Add PropertyUtilsTest, the UT of PropertyUtils (#1528)

1. Add PropertyUtilsTest
2. Add null check and formatparse check in PropertyUtils
3. Add UT path in parent.pom

dolphinscheduler-alert/src/main/java/org/apache/dolphinscheduler/alert/utils/PropertyUtils.java

@@ -17,15 +17,16 @@
 package org.apache.dolphinscheduler.alert.utils;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Properties;
+import java.util.regex.PatternSyntaxException;
 
 import static org.apache.dolphinscheduler.alert.utils.Constants.ALERT_PROPERTIES_PATH;
-import static org.apache.dolphinscheduler.alert.utils.Constants.DATA_SOURCE_PROPERTIES_PATH;
 
 /**
  * property utils
@@ -72,7 +73,10 @@ public class PropertyUtils {
      * @return the value
      */
     public static String getString(String key) {
-        return properties.getProperty(key);
+        if (StringUtils.isEmpty(key)) {
+            return null;
+        }
+        return properties.getProperty(key.trim());
     }
 
     /**
@@ -82,6 +86,7 @@ public class PropertyUtils {
      * @return  get property int value , if key == null, then return -1
      */
     public static int getInt(String key) {
+
         return getInt(key, -1);
     }
 
@@ -111,6 +116,11 @@ public class PropertyUtils {
      * @return  the boolean result value
      */
     public static Boolean getBoolean(String key) {
+
+        if (StringUtils.isEmpty(key)) {
+            return false;
+        }
+
         String value = properties.getProperty(key.trim());
         if(null != value){
             return Boolean.parseBoolean(value);
@@ -135,10 +145,30 @@ public class PropertyUtils {
      * @return the value related the key or the default value if the key not existed
      */
     public static long getLong(String key, long defaultVal) {
+
         String val = getString(key);
-        return val == null ? defaultVal : Long.parseLong(val);
+        if (val == null) {
+            return defaultVal;
+        }
+
+        try {
+            return Long.parseLong(val);
+        } catch (NumberFormatException e) {
+            logger.info(e.getMessage(),e);
+        }
+
+        return defaultVal;
     }
 
+    /**
+     * get double value
+     * @param key the key
+     * @return if the value not existed, return -1.0, or will return the related value
+     */
+    public static double getDouble(String key) {
+        String val = getString(key);
+        return getDouble(key,-1.0);
+    }
 
     /**
      * get double value
@@ -146,9 +176,20 @@ public class PropertyUtils {
      * @param defaultVal the default value
      * @return the value related the key or the default value if the key not existed
      */
-    public double getDouble(String key, double defaultVal) {
+    public static double getDouble(String key, double defaultVal) {
+
         String val = getString(key);
-        return val == null ? defaultVal : Double.parseDouble(val);
+        if (val == null) {
+            return defaultVal;
+        }
+
+        try {
+            return Double.parseDouble(val);
+        } catch (NumberFormatException e) {
+            logger.info(e.getMessage(),e);
+        }
+
+        return defaultVal;
     }
 
 
@@ -160,13 +201,13 @@ public class PropertyUtils {
      */
     public static String[] getArray(String key, String splitStr) {
         String value = getString(key);
-        if (value == null) {
+        if (value == null || StringUtils.isEmpty(splitStr)) {
             return null;
         }
         try {
             String[] propertyArray = value.split(splitStr);
             return propertyArray;
-        } catch (NumberFormatException e) {
+        } catch (PatternSyntaxException e) {
             logger.info(e.getMessage(),e);
         }
         return null;
@@ -180,9 +221,19 @@ public class PropertyUtils {
      * @param <T> the generic class type
      * @return  get enum value
      */
-    public <T extends Enum<T>> T getEnum(String key, Class<T> type,
+    public static <T extends Enum<T>> T getEnum(String key, Class<T> type,
                                          T defaultValue) {
         String val = getString(key);
-        return val == null ? defaultValue : Enum.valueOf(type, val);
+        if (val == null) {
+            return defaultValue;
+        }
+
+        try {
+            return Enum.valueOf(type, val);
+        } catch (IllegalArgumentException e) {
+            logger.info(e.getMessage(),e);
+        }
+
+        return defaultValue;
     }
 }

dolphinscheduler-alert/src/test/java/org/apache/dolphinscheduler/alert/utils/PropertyUtilsTest.java

+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.alert.utils;
+
+import org.apache.dolphinscheduler.common.enums.ZKNodeType;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.*;
+
+/**
+ * Test PropertyUtils
+ * and the resource path is src/test/resources/alert.properties.
+ */
+public class PropertyUtilsTest {
+
+    private static final Logger logger = LoggerFactory.getLogger(PropertyUtilsTest.class);
+
+    /**
+     * Test getString
+     */
+    @Test
+    public void testGetString() {
+
+        //Expected "EMAIL"
+        String result = PropertyUtils.getString("alert.type");
+        logger.info(result);
+        assertEquals(result, "EMAIL");
+
+        //Expected "xxx.xxx.test"
+        result = PropertyUtils.getString("mail.server.host");
+        assertEquals(result, "xxx.xxx.test");
+
+        //If key is undefine in alert.properties, then return null
+        result = PropertyUtils.getString("abc");
+        assertNull(result);
+
+        //If key is null, then return null
+        result = PropertyUtils.getString(null);
+        assertNull(result);
+    }
+
+
+    /**
+     * Test getBoolean
+     */
+    @Test
+    public void testGetBoolean() {
+
+        //Expected true
+        Boolean result = PropertyUtils.getBoolean("mail.smtp.starttls.enable");
+        assertTrue(result);
+
+        //Expected false
+        result = PropertyUtils.getBoolean("mail.smtp.ssl.enable");
+        assertFalse(result);
+
+        //If key is undefine in alert.properties, then return null
+        result = PropertyUtils.getBoolean("abc");
+        assertFalse(result);
+
+        //If key is null, then return false
+        result = PropertyUtils.getBoolean(null);
+        assertFalse(result);
+    }
+
+    /**
+     * Test getLong
+     */
+    @Test
+    public void testGetLong() {
+
+        //Expected 25
+        long result = PropertyUtils.getLong("mail.server.port");
+        assertSame(result, 25L);
+
+        //If key is null, then return -1
+        result = PropertyUtils.getLong(null);
+        assertSame(result, -1L);
+
+        //If key is undefine in alert.properties, then return -1
+        result = PropertyUtils.getLong("abc");
+        assertSame(result, -1L);
+
+        //If key is undefine in alert.properties, and there is a defaultval, then return defaultval
+        result = PropertyUtils.getLong("abc", 200);
+        assertEquals(result, 200L);
+
+        //If the value can not parse to long ,it will log the error and return -1L
+        result = PropertyUtils.getLong("test.server.testnumber");
+        assertSame(result, -1L);
+    }
+
+    /**
+     * Test getDouble
+     */
+    @Test
+    public void testGetDouble() {
+
+        //Expected 3.0
+        double result = PropertyUtils.getDouble("test.server.factor");
+        assertEquals(result, 3.0, 0);
+
+        //If key is null, then return -1.0
+        result = PropertyUtils.getDouble(null);
+        assertEquals(result, -1.0, 0);
+
+        //If key is undefine in alert.properties, then return -1
+        result = PropertyUtils.getDouble("abc");
+        assertEquals(result, -1.0, 0);
+
+        //If key is undefine in alert.properties, and there is a defaultval, then return defaultval
+        result = PropertyUtils.getDouble("abc", 5.0);
+        assertEquals(result, 5.0, 0);
+
+        //If the value can not parse to double ,it will log the error and return -1.0
+        result = PropertyUtils.getDouble("test.server.testnumber");
+        assertEquals(result, -1.0, 0);
+    }
+
+    /**
+     * Test getArray
+     */
+    @Test
+    public void testGetArray() {
+
+        //Expected length 3
+        String[] result = PropertyUtils.getArray("test.server.list", ",");
+        assertEquals(result.length, 3);
+
+        //Equal array values
+        assertEquals(result[0], "xxx.xxx.test1");
+        assertEquals(result[1], "xxx.xxx.test2");
+        assertEquals(result[2], "xxx.xxx.test3");
+
+        //If key is null, then return -1
+        result = PropertyUtils.getArray(null, ",");
+        assertNull(result);
+
+        //If key is undefine in alert.properties, then return null
+        result = PropertyUtils.getArray("abc", ",");
+        assertNull(result);
+
+        //If splitStr is null, then return null
+        result = PropertyUtils.getArray("test.server.list", null);
+        assertNull(result);
+    }
+
+    /**
+     * test getInt
+     */
+    @Test
+    public void testGetInt() {
+
+        //Expected 25
+        int result = PropertyUtils.getInt("mail.server.port");
+        assertSame(result, 25);
+
+        //If key is null, then return -1
+        result = PropertyUtils.getInt(null);
+        assertSame(result, -1);
+
+        //If key is undefine in alert.properties, then return -1
+        result = PropertyUtils.getInt("abc");
+        assertSame(result, -1);
+
+        //If key is undefine in alert.properties, and there is a defaultval, then return defaultval
+        result = PropertyUtils.getInt("abc", 300);
+        assertEquals(result, 300);
+
+        //If the value can not parse to int ,it will log the error and return -1
+        result = PropertyUtils.getInt("test.server.testnumber");
+        assertSame(result, -1);
+    }
+
+    /**
+     * Test getEnum
+     */
+    @Test
+    public void testGetEnum() {
+
+        //Expected MASTER
+        ZKNodeType zkNodeType = PropertyUtils.getEnum("test.server.enum1", ZKNodeType.class,ZKNodeType.WORKER);
+        assertEquals(zkNodeType, ZKNodeType.MASTER);
+
+        //Expected DEAD_SERVER
+        zkNodeType = PropertyUtils.getEnum("test.server.enum2", ZKNodeType.class,ZKNodeType.WORKER);
+        assertEquals(zkNodeType, ZKNodeType.DEAD_SERVER);
+
+        //If key is null, then return defaultval
+        zkNodeType = PropertyUtils.getEnum(null, ZKNodeType.class,ZKNodeType.WORKER);
+        assertEquals(zkNodeType, ZKNodeType.WORKER);
+
+        //If the value doesn't define in enum ,it will log the error and return -1
+        zkNodeType = PropertyUtils.getEnum("test.server.enum3", ZKNodeType.class,ZKNodeType.WORKER);
+        assertEquals(zkNodeType, ZKNodeType.WORKER);
+    }
+
+}
\ No newline at end of file

dolphinscheduler-alert/src/test/resources/alert.properties

+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# For unit test
+
+#alert type is EMAIL/SMS
+alert.type=EMAIL
+
+# mail server configuration
+mail.protocol=SMTP
+mail.server.host=xxx.xxx.test
+mail.server.port=25
+mail.sender=xxx@xxx.com
+mail.user=xxx@xxx.com
+mail.passwd=111111
+
+# Test double
+test.server.factor=3.0
+
+
+# Test NumberFormat
+test.server.testnumber=abc
+
+# Test array
+test.server.list=xxx.xxx.test1,xxx.xxx.test2,xxx.xxx.test3
+
+# Test enum
+test.server.enum1=MASTER
+test.server.enum2=DEAD_SERVER
+test.server.enum3=abc
+
+# TLS
+mail.smtp.starttls.enable=true
+# SSL
+mail.smtp.ssl.enable=false
+mail.smtp.ssl.trust=xxx.xxx.com
+
+#xls file path,need create if not exist
+xls.file.path=/tmp/xls
+
+# Enterprise WeChat configuration
+enterprise.wechat.enable=false
+enterprise.wechat.corp.id=xxxxxxx
+enterprise.wechat.secret=xxxxxxx
+enterprise.wechat.agent.id=xxxxxxx
+enterprise.wechat.users=xxxxxxx
+enterprise.wechat.token.url=https://qyapi.weixin.qq.com/cgi-bin/gettoken?corpid=$corpId&corpsecret=$secret
+enterprise.wechat.push.url=https://qyapi.weixin.qq.com/cgi-bin/message/send?access_token=$token
+enterprise.wechat.team.send.msg={\"toparty\":\"$toParty\",\"agentid\":\"$agentId\",\"msgtype\":\"text\",\"text\":{\"content\":\"$msg\"},\"safe\":\"0\"}
+enterprise.wechat.user.send.msg={\"touser\":\"$toUser\",\"agentid\":\"$agentId\",\"msgtype\":\"markdown\",\"markdown\":{\"content\":\"$msg\"}}
+
+
+

pom.xml

@@ -652,6 +652,7 @@
 						<include>**/alert/utils/ExcelUtilsTest.java</include>
 						<include>**/alert/utils/FuncUtilsTest.java</include>
 						<include>**/alert/utils/JSONUtilsTest.java</include>
+						<include>**/alert/utils/PropertyUtilsTest.java</include>
 					</includes>
 					<!-- <skip>true</skip> -->
 				</configuration>