[Improvement-17267] Throw exception if deserialize json failed in JSONUtils (#17243)

Author: ruanwenjun

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-email/src/test/java/org/apache/dolphinscheduler/plugin/alert/email/ExcelUtilsTest.java

@@ -17,8 +17,6 @@
 
 package org.apache.dolphinscheduler.plugin.alert.email;
 
-import org.apache.dolphinscheduler.plugin.alert.email.exception.AlertEmailException;
-
 import java.io.File;
 import java.nio.file.Path;
 
@@ -35,7 +33,7 @@ public class ExcelUtilsTest {
     private String xlsFilePath;
 
     @BeforeEach
-    public void setUp() throws Exception {
+    public void setUp() {
         xlsFilePath = testFolder.toString();
     }
 
@@ -58,9 +56,8 @@ public void testGenExcelFile() {
         Assertions.assertTrue(xlsFile.exists());
 
         // Invoke genExcelFile with incorrectContent, will cause RuntimeException
-        Assertions.assertThrows(AlertEmailException.class, () -> {
-            ExcelUtils.genExcelFile(incorrectContent1, title, xlsFilePath);
-        });
+        Assertions.assertThrows(IllegalArgumentException.class,
+                () -> ExcelUtils.genExcelFile(incorrectContent1, title, xlsFilePath));
 
     }
 

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-feishu/src/test/java/org/apache/dolphinscheduler/plugin/alert/feishu/FeiShuSenderTest.java

@@ -76,24 +76,22 @@ public void testFormatContent() {
     public void testSendWithFormatException() {
         AlertData alertData = new AlertData();
         alertData.setTitle("feishu test title");
-        alertData.setContent("feishu test content");
-        FeiShuSender feiShuSender = new FeiShuSender(feiShuConfig);
-        String alertResult = feiShuSender.formatContent(alertData);
-        Assertions.assertEquals(alertResult, alertData.getTitle() + alertData.getContent());
+        alertData.setContent("[{\"content\":\"feishu test content\"}]");
+        String alertResult = FeiShuSender.formatContent(alertData);
+        Assertions.assertEquals(alertResult, "`feishu test title`\ncontent:feishu test content\n");
     }
 
     @Test
     public void testCheckSendFeiShuSendMsgResult() {
 
-        FeiShuSender feiShuSender = new FeiShuSender(feiShuConfig);
-        AlertResult alertResult = feiShuSender.checkSendFeiShuSendMsgResult("");
+        AlertResult alertResult = FeiShuSender.checkSendFeiShuSendMsgResult("");
         Assertions.assertFalse(alertResult.isSuccess());
-        AlertResult alertResult2 = feiShuSender.checkSendFeiShuSendMsgResult("123");
-        Assertions.assertEquals("send feishu msg error: feishu server resp parse error is null.",
+        AlertResult alertResult2 = FeiShuSender.checkSendFeiShuSendMsgResult("{\"code\":123}");
+        Assertions.assertEquals("alert send feishu msg error: null",
                 alertResult2.getMessage());
 
         String response = "{\"code\":\"0\",\"data\":{},\"msg\":\"success\"}";
-        AlertResult alertResult3 = feiShuSender.checkSendFeiShuSendMsgResult(response);
+        AlertResult alertResult3 = FeiShuSender.checkSendFeiShuSendMsgResult(response);
         Assertions.assertTrue(alertResult3.isSuccess());
     }
 }

dolphinscheduler-alert/dolphinscheduler-alert-server/src/test/java/org/apache/dolphinscheduler/alert/runner/AlertSenderTest.java

@@ -96,11 +96,10 @@ void testSyncHandler() {
 
         // 2.alert plugin does not exist
         int pluginDefineId = 1;
-        String pluginInstanceParams = "alert-instance-mail-params";
         String pluginInstanceName = "alert-instance-mail";
         List<AlertPluginInstance> alertInstanceList = new ArrayList<>();
         AlertPluginInstance alertPluginInstance = new AlertPluginInstance(
-                pluginDefineId, pluginInstanceParams, pluginInstanceName);
+                pluginDefineId, PLUGIN_INSTANCE_PARAMS, pluginInstanceName);
         alertPluginInstance.setId(1);
         alertInstanceList.add(alertPluginInstance);
         when(alertDao.listInstanceByAlertGroupId(1)).thenReturn(alertInstanceList);

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java

@@ -475,10 +475,11 @@ protected void checkParams(String name, String config, String workerGroups) {
         if (StringUtils.isEmpty(config)) {
             throw new ServiceException(Status.ENVIRONMENT_CONFIG_IS_NULL);
         }
-        if (!StringUtils.isEmpty(workerGroups)) {
-            List<String> workerGroupList = JSONUtils.parseObject(workerGroups, new TypeReference<List<String>>() {
-            });
-            if (Objects.isNull(workerGroupList)) {
+        if (StringUtils.isNotEmpty(workerGroups)) {
+            try {
+                JSONUtils.parseObject(workerGroups, new TypeReference<List<String>>() {
+                });
+            } catch (IllegalArgumentException e) {
                 throw new ServiceException(Status.ENVIRONMENT_WORKER_GROUPS_IS_INVALID);
             }
         }

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java

@@ -28,6 +28,7 @@
 import org.apache.dolphinscheduler.api.utils.PageInfo;
 import org.apache.dolphinscheduler.api.utils.Result;
 import org.apache.dolphinscheduler.common.constants.Constants;
+import org.apache.dolphinscheduler.common.constants.SystemConstants;
 import org.apache.dolphinscheduler.common.enums.AuthorizationType;
 import org.apache.dolphinscheduler.common.enums.Flag;
 import org.apache.dolphinscheduler.common.enums.UserType;
@@ -63,7 +64,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.TimeZone;
 import java.util.stream.Collectors;
 
 import lombok.extern.slf4j.Slf4j;
@@ -891,7 +891,7 @@ public Map<String, Object> getUserInfo(User loginUser) {
 
         // add system default timezone if not user timezone
         if (StringUtils.isEmpty(user.getTimeZone())) {
-            user.setTimeZone(TimeZone.getDefault().toZoneId().getId());
+            user.setTimeZone(SystemConstants.DEFAULT_TIME_ZONE.toZoneId().getId());
         }
 
         // remove password

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkflowInstanceServiceTest.java

@@ -480,6 +480,7 @@ public void testQueryTaskListByWorkflowInstanceId() throws IOException {
         taskInstanceContext.setContextType(ContextType.DEPENDENT_RESULT_CONTEXT);
         DependentResultTaskInstanceContext dependentResultTaskInstanceContext =
                 new DependentResultTaskInstanceContext();
+        dependentResultTaskInstanceContext.setContextType(ContextType.DEPENDENT_RESULT_CONTEXT);
         dependentResultTaskInstanceContext.setProjectCode(projectCode);
         dependentResultTaskInstanceContext.setDependentResult(DependResult.SUCCESS);
         taskInstanceContext.setTaskInstanceContext(

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/constants/SystemConstants.java

@@ -0,0 +1,32 @@
+/*
+ * 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.common.constants;
+
+import java.util.TimeZone;
+
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+public class SystemConstants {
+
+    public static final TimeZone DEFAULT_TIME_ZONE = TimeZone.getDefault();
+
+    static {
+        log.info("init timezone: {}", DEFAULT_TIME_ZONE);
+    }
+}

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/JSONUtils.java

@@ -25,6 +25,8 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.dolphinscheduler.common.constants.DateConstants.YYYY_MM_DD_HH_MM_SS;
 
+import org.apache.dolphinscheduler.common.constants.SystemConstants;
+
 import org.apache.commons.lang3.StringUtils;
 
 import java.io.IOException;
@@ -69,10 +71,6 @@
 @Slf4j
 public final class JSONUtils {
 
-    static {
-        log.info("init timezone: {}", TimeZone.getDefault());
-    }
-
     private static final ObjectMapper objectMapper = JsonMapper.builder()
             .configure(FAIL_ON_UNKNOWN_PROPERTIES, false)
             .configure(ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT, true)
@@ -83,7 +81,7 @@ public final class JSONUtils {
             .addModule(new SimpleModule()
                     .addSerializer(LocalDateTime.class, new LocalDateTimeSerializer())
                     .addDeserializer(LocalDateTime.class, new LocalDateTimeDeserializer()))
-            .defaultTimeZone(TimeZone.getDefault())
+            .defaultTimeZone(SystemConstants.DEFAULT_TIME_ZONE)
             .defaultDateFormat(new SimpleDateFormat(YYYY_MM_DD_HH_MM_SS))
             .build();
 
@@ -110,7 +108,7 @@ public static JsonNode toJsonNode(Object obj) {
     /**
      * json representation of object
      *
-     * @param object object
+     * @param object  object
      * @param feature feature
      * @return object to json string
      */
@@ -133,31 +131,34 @@ public static String toJsonString(Object object, SerializationFeature feature) {
      * the fields of the specified object are generics, just the object itself should not be a
      * generic type.
      *
-     * @param json the string from which the object is to be deserialized
+     * @param json  the string from which the object is to be deserialized
      * @param clazz the class of T
-     * @param <T> T
+     * @param <T>   T
      * @return an object of type T from the string
      * classOfT
      */
     public static @Nullable <T> T parseObject(String json, Class<T> clazz) {
+        if (clazz == null) {
+            throw new IllegalArgumentException("Class type cannot be null");
+        }
+
         if (Strings.isNullOrEmpty(json)) {
             return null;
         }
 
         try {
             return objectMapper.readValue(json, clazz);
         } catch (Exception e) {
-            log.error("Parse object exception, jsonStr: {}, class: {}", json, clazz, e);
+            throw new IllegalArgumentException("Parse json: " + json + " to class: " + clazz.getName() + " failed", e);
         }
-        return null;
     }
 
     /**
-     *  deserialize
+     * deserialize
      *
-     * @param src byte array
+     * @param src   byte array
      * @param clazz class
-     * @param <T> deserialize type
+     * @param <T>   deserialize type
      * @return deserialize type
      */
     public static <T> T parseObject(byte[] src, Class<T> clazz) {
@@ -171,12 +172,16 @@ public static <T> T parseObject(byte[] src, Class<T> clazz) {
     /**
      * json to list
      *
-     * @param json json string
+     * @param json  json string
      * @param clazz class
-     * @param <T> T
+     * @param <T>   T
      * @return list
      */
     public static <T> List<T> toList(String json, Class<T> clazz) {
+        if (clazz == null) {
+            throw new IllegalArgumentException("Class type cannot be null");
+        }
+
         if (Strings.isNullOrEmpty(json)) {
             return Collections.emptyList();
         }
@@ -185,10 +190,10 @@ public static <T> List<T> toList(String json, Class<T> clazz) {
             CollectionType listType = objectMapper.getTypeFactory().constructCollectionType(ArrayList.class, clazz);
             return objectMapper.readValue(json, listType);
         } catch (Exception e) {
-            log.error("parse list exception!", e);
+            throw new IllegalArgumentException(
+                    "Parse json: " + json + " to list of class: " + clazz.getName() + " failed", e);
         }
 
-        return Collections.emptyList();
     }
 
     /**
@@ -222,7 +227,7 @@ public static boolean checkJsonValid(String json, Boolean logFlag) {
      * node or its child nodes, and returning value it has.
      * If no matching field is found in this node or its descendants, returns null.
      *
-     * @param jsonNode json node
+     * @param jsonNode  json node
      * @param fieldName Name of field to look for
      * @return Value of first matching node found, if any; null if none
      */
@@ -238,7 +243,6 @@ public static String findValue(JsonNode jsonNode, String fieldName) {
 
     /**
      * json to map
-     * {@link #toMap(String, Class, Class)}
      *
      * @param json json
      * @return json to map
@@ -248,34 +252,10 @@ public static Map<String, String> toMap(String json) {
         });
     }
 
-    /**
-     * json to map
-     *
-     * @param json json
-     * @param classK classK
-     * @param classV classV
-     * @param <K> K
-     * @param <V> V
-     * @return to map
-     */
-    public static <K, V> Map<K, V> toMap(String json, Class<K> classK, Class<V> classV) {
-        if (Strings.isNullOrEmpty(json)) {
-            return Collections.emptyMap();
-        }
-
-        try {
-            return objectMapper.readValue(json, new TypeReference<Map<K, V>>() {
-            });
-        } catch (Exception e) {
-            log.error("json to map exception!", e);
-        }
-
-        return Collections.emptyMap();
-    }
-
     /**
      * from the key-value generated json  to get the str value no matter the real type of value
-     * @param json the json str
+     *
+     * @param json     the json str
      * @param nodeName key
      * @return the str value of key
      */
@@ -305,13 +285,15 @@ public static <T> T parseObject(String json, TypeReference<T> type) {
             return null;
         }
 
+        if (type == null) {
+            throw new IllegalArgumentException("Type reference cannot be null");
+        }
+
         try {
             return objectMapper.readValue(json, type);
         } catch (Exception e) {
-            log.error("json to map exception!", e);
+            throw new IllegalArgumentException("Parse json: " + json + " to type: " + type.getType() + " failed", e);
         }
-
-        return null;
     }
 
     /**
@@ -347,14 +329,12 @@ public static <T> byte[] toJsonByteArray(T obj) {
         if (obj == null) {
             return null;
         }
-        String json = "";
         try {
-            json = toJsonString(obj);
+            return toJsonString(obj).getBytes(UTF_8);
         } catch (Exception e) {
-            log.error("json serialize exception.", e);
+            throw new IllegalArgumentException("Object: " + obj + " to json serialization exception.", e);
         }
 
-        return json.getBytes(UTF_8);
     }
 
     public static ObjectNode parseObject(String text) {