Fix Switch logic task doesn't check the branch exist (#15755)

dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/task/switchtask/SwitchLogicTask.java

@@ -18,10 +18,8 @@
 package org.apache.dolphinscheduler.server.master.runner.task.switchtask;
 
 import org.apache.dolphinscheduler.common.utils.JSONUtils;
-import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
 import org.apache.dolphinscheduler.dao.entity.TaskInstance;
 import org.apache.dolphinscheduler.plugin.task.api.TaskExecutionContext;
-import org.apache.dolphinscheduler.plugin.task.api.enums.DependResult;
 import org.apache.dolphinscheduler.plugin.task.api.enums.TaskExecutionStatus;
 import org.apache.dolphinscheduler.plugin.task.api.model.Property;
 import org.apache.dolphinscheduler.plugin.task.api.model.SwitchResultVo;
@@ -34,7 +32,6 @@ import org.apache.dolphinscheduler.server.master.runner.task.BaseSyncLogicTask;
 import org.apache.dolphinscheduler.server.master.utils.SwitchTaskUtils;
 
 import org.apache.commons.collections4.CollectionUtils;
-import org.apache.commons.lang3.StringUtils;
 
 import java.util.List;
 import java.util.Map;
@@ -47,7 +44,7 @@ public class SwitchLogicTask extends BaseSyncLogicTask<SwitchParameters> {
 
     public static final String TASK_TYPE = "SWITCH";
 
-    private final ProcessInstance processInstance;
+    private final WorkflowExecuteRunnable workflowExecuteRunnable;
     private final TaskInstance taskInstance;
 
     public SwitchLogicTask(TaskExecutionContext taskExecutionContext,
@@ -59,9 +56,8 @@ public class SwitchLogicTask extends BaseSyncLogicTask<SwitchParameters> {
                         .orElseThrow(() -> new LogicTaskInitializeException(
                                 "Cannot find the task instance in workflow execute runnable"))
                         .getSwitchDependency());
-        WorkflowExecuteRunnable workflowExecuteRunnable =
+        this.workflowExecuteRunnable =
                 processInstanceExecCacheManager.getByProcessInstanceId(taskExecutionContext.getProcessInstanceId());
-        this.processInstance = workflowExecuteRunnable.getWorkflowExecuteContext().getWorkflowInstance();
         this.taskInstance = workflowExecuteRunnable.getTaskInstance(taskExecutionContext.getTaskInstanceId())
                 .orElseThrow(() -> new LogicTaskInitializeException(
                         "Cannot find the task instance in workflow execute runnable"));
@@ -69,84 +65,86 @@ public class SwitchLogicTask extends BaseSyncLogicTask<SwitchParameters> {
 
     @Override
     public void handle() throws MasterTaskExecuteException {
-        DependResult conditionResult = calculateConditionResult();
+        // Calculate the condition result and get the next node
-        TaskExecutionStatus status =
+        if (CollectionUtils.isEmpty(taskParameters.getDependTaskList())) {
-                (conditionResult == DependResult.SUCCESS) ? TaskExecutionStatus.SUCCESS : TaskExecutionStatus.FAILURE;
+            moveToDefaultBranch();
-        log.info("Switch task execute finished, condition result is: {}, task status is: {}", conditionResult,
+        } else {
-                status.name());
+            calculateSwitchBranch();
-        taskExecutionContext.setCurrentExecutionStatus(status);
+        }
+        taskInstance.setSwitchDependency(taskParameters);
+        log.info("Switch task execute finished");
+        taskExecutionContext.setCurrentExecutionStatus(TaskExecutionStatus.SUCCESS);
     }
 
-    // todo: don't use depend result, use switch result
+    private void moveToDefaultBranch() {
-    private DependResult calculateConditionResult() {
+        checkIfBranchExist(taskParameters.getNextNode());
-        DependResult conditionResult = DependResult.SUCCESS;
 
         List<SwitchResultVo> switchResultVos = taskParameters.getDependTaskList();
+        switchResultVos.add(new SwitchResultVo(null, taskParameters.getNextNode()));
+        taskParameters.setResultConditionLocation(switchResultVos.size() - 1);
 
-        SwitchResultVo switchResultVo = new SwitchResultVo();
+        log.info("The condition is not satisfied, move to the default branch: {}",
-        switchResultVo.setNextNode(taskParameters.getNextNode());
+                taskParameters.getNextNode().stream().map(node -> workflowExecuteRunnable.getWorkflowExecuteContext()
-        switchResultVos.add(switchResultVo);
+                        .getWorkflowGraph().getDag().getNode(node).getName()).collect(Collectors.toList()));
-        // todo: refactor these calculate code
+    }
-        int finalConditionLocation = switchResultVos.size() - 1;
-        int i = 0;
 
-        Map<String, Property> globalParams = JSONUtils
+    private void calculateSwitchBranch() {
-                .toList(processInstance.getGlobalParams(), Property.class)
+        List<SwitchResultVo> switchResultVos = taskParameters.getDependTaskList();
-                .stream()
+        if (CollectionUtils.isEmpty(switchResultVos)) {
-                .collect(Collectors.toMap(Property::getProp, Property -> Property));
+            moveToDefaultBranch();
+        }
+        Map<String, Property> globalParams = taskExecutionContext.getPrepareParamsMap();
         Map<String, Property> varParams = JSONUtils
                 .toList(taskInstance.getVarPool(), Property.class)
                 .stream()
                 .collect(Collectors.toMap(Property::getProp, Property -> Property));
 
-        for (SwitchResultVo info : switchResultVos) {
+        int finalConditionLocation = -1;
-            log.info("Begin to execute {} condition: {} ", (i + 1), info.getCondition());
+        for (int i = 0; i < switchResultVos.size(); i++) {
-            if (StringUtils.isEmpty(info.getCondition())) {
+            SwitchResultVo switchResultVo = switchResultVos.get(i);
-                finalConditionLocation = i;
+            log.info("Begin to execute {} condition: {} ", i, switchResultVo.getCondition());
-                break;
+            String content = SwitchTaskUtils.generateContentWithTaskParams(switchResultVo.getCondition(), globalParams,
-            }
+                    varParams);
-            String content =
-                    SwitchTaskUtils.generateContentWithTaskParams(info.getCondition(), globalParams, varParams);
             log.info("Format condition sentence::{} successfully", content);
-            Boolean result;
+            boolean result;
             try {
                 result = SwitchTaskUtils.evaluate(content);
                 log.info("Execute condition sentence: {} successfully: {}", content, result);
+                if (result) {
+                    finalConditionLocation = i;
+                }
             } catch (Exception e) {
                 log.info("Execute condition sentence: {} failed", content, e);
-                conditionResult = DependResult.FAILED;
-                break;
-            }
-            if (result) {
-                finalConditionLocation = i;
-                break;
             }
-            i++;
         }
-        taskParameters.setDependTaskList(switchResultVos);
+        if (finalConditionLocation >= 0) {
-        taskParameters.setResultConditionLocation(finalConditionLocation);
+            checkIfBranchExist(switchResultVos.get(finalConditionLocation).getNextNode());
-        taskInstance.setSwitchDependency(taskParameters);
+            log.info("The condition is satisfied, move to the branch: {}",
-
+                    switchResultVos.get(finalConditionLocation).getNextNode().stream()
-        if (!isValidSwitchResult(switchResultVos.get(finalConditionLocation))) {
+                            .map(node -> workflowExecuteRunnable.getWorkflowExecuteContext().getWorkflowGraph().getDag()
-            conditionResult = DependResult.FAILED;
+                                    .getNode(node).getName())
-            log.error("The switch task depend result is invalid, result:{}, switch branch:{}", conditionResult,
+                            .collect(Collectors.toList()));
-                    finalConditionLocation);
+            taskParameters.setResultConditionLocation(finalConditionLocation);
+        } else {
+            log.info("All conditions are not satisfied, move to the default branch");
+            moveToDefaultBranch();
         }
-
-        log.info("The switch task depend result:{}, switch branch:{}", conditionResult, finalConditionLocation);
-        return conditionResult;
     }
 
-    private boolean isValidSwitchResult(SwitchResultVo switchResult) {
+    private void checkIfBranchExist(List<Long> branchNode) {
-        if (CollectionUtils.isEmpty(switchResult.getNextNode())) {
+        if (CollectionUtils.isEmpty(branchNode)) {
-            return false;
+            throw new IllegalArgumentException("The branchNode is empty, please check the switch task configuration");
         }
-        for (Long nextNode : switchResult.getNextNode()) {
+        for (Long branch : branchNode) {
-            if (nextNode == null) {
+            if (branch == null) {
-                return false;
+                throw new IllegalArgumentException("The branch is empty, please check the switch task configuration");
+            }
+            if (!workflowExecuteRunnable.getWorkflowExecuteContext().getWorkflowGraph().getDag().containsNode(branch)) {
+                throw new IllegalArgumentException(
+                        "The branch(code= " + branchNode
+                                + ") is not in the dag, please check the switch task configuration");
             }
         }
-        return true;
     }
 
 }