[GitHub] [incubator-dolphinscheduler] zhuangchong commented on a change in pull request #4237: [FIX-3177]Task time parameter parsing error

[GitBox <gi...@apache.org>]

zhuangchong commented on a change in pull request #4237:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4237#discussion_r543966468



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -66,11 +66,13 @@ public static String convertParameterPlaceholders(String parameterString, Map<St
         }
         Date cronTime;
         if (parameterMap != null && !parameterMap.isEmpty()) {
+            // replace variable ${} form,refers to the replacement of system variables and custom variables
+            parameterString = PlaceholderUtils.replacePlaceholders(parameterString, parameterMap, true);
+        }
+        if (parameterMap != null && null != parameterMap.get(Constants.PARAMETER_DATETIME)) {

Review comment:
       I think you should make sure that the parameters within the Dateutils.parse method are not null
   
   ```
   if (StringUtils.isBlank(date) || StringUtils.isBlank(format)) {
               return null;
           }
   ```
   
   ```
    public static String convertParameterPlaceholders(String parameterString, Map<String, String> parameterMap) {
           if (StringUtils.isEmpty(parameterString)) {
               return parameterString;
           }
   
           if (parameterMap == null || parameterMap.isEmpty()) {
               return dateTemplateParse(parameterString, new Date());
           }
   
           //Get current time, schedule execute time
           String cronTimeStr = parameterMap.get(Constants.PARAMETER_DATETIME);
           Date cronTime = DateUtils.parse(cronTimeStr, Constants.PARAMETER_FORMAT_TIME);
           if (cronTime == null) {
               return parameterString;
           }
           // replace variable ${} form,refers to the replacement of system variables and custom variables
           parameterString = PlaceholderUtils.replacePlaceholders(parameterString, parameterMap, true);
           return dateTemplateParse(parameterString, cronTime);
       }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org