[Fix-#11669][Workflow Instance Page] Fix the duration in Workflow Instance page. (#12264)

* fix process duration issue

* use processInstance.getState().isFinished() replace new isFinish function

* add WorkflowUtils

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

@@ -63,6 +63,7 @@ import org.apache.dolphinscheduler.dao.mapper.TaskDefinitionMapper;
 import org.apache.dolphinscheduler.dao.mapper.TaskInstanceMapper;
 import org.apache.dolphinscheduler.dao.mapper.TenantMapper;
 import org.apache.dolphinscheduler.dao.repository.ProcessInstanceDao;
+import org.apache.dolphinscheduler.dao.utils.WorkflowUtils;
 import org.apache.dolphinscheduler.plugin.task.api.enums.DependResult;
 import org.apache.dolphinscheduler.plugin.task.api.model.Property;
 import org.apache.dolphinscheduler.plugin.task.api.parameters.ParametersNode;
@@ -320,8 +321,7 @@ public class ProcessInstanceServiceImpl extends BaseServiceImpl implements Proce
         }
 
         for (ProcessInstance processInstance : processInstances) {
-            processInstance.setDuration(
-                    DateUtils.format2Duration(processInstance.getStartTime(), processInstance.getEndTime()));
+            processInstance.setDuration(WorkflowUtils.getWorkflowInstanceDuration(processInstance));
             User executor = idToUserMap.get(processInstance.getExecutorId());
             if (null != executor) {
                 processInstance.setExecutorName(executor.getUserName());

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

@@ -270,24 +270,64 @@ public class ProcessInstanceServiceTest {
         // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
-        Map<String, Object> proejctAuthFailRes = processInstanceService
+        Map<String, Object> projectAuthFailRes = processInstanceService
                 .queryTopNLongestRunningProcessInstance(loginUser, projectCode, size, startTime, endTime);
-        Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS));
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
 
         // project auth success
         putMsg(result, Status.SUCCESS, projectCode);
         ProcessInstance processInstance = getProcessInstance();
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
+        projectAuthFailRes = processInstanceService
+            .queryTopNLongestRunningProcessInstance(loginUser, projectCode, -1, startTime, endTime);
+        Assert.assertEquals(Status.NEGTIVE_SIZE_NUMBER_ERROR, projectAuthFailRes.get(Constants.STATUS));
+
+        putMsg(result, Status.SUCCESS, projectCode);
+        when(projectMapper.queryByCode(projectCode)).thenReturn(project);
+        when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
         when(usersService.queryUser(loginUser.getId())).thenReturn(loginUser);
         when(usersService.getUserIdByName(loginUser.getUserName())).thenReturn(loginUser.getId());
         when(usersService.queryUser(processInstance.getExecutorId())).thenReturn(loginUser);
         Map<String, Object> successRes = processInstanceService.queryTopNLongestRunningProcessInstance(loginUser,
-                projectCode, size, startTime, endTime);
+            projectCode, size, startTime, endTime);
 
         Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS));
     }
 
+    @Test
+    public void testTopNLongestRunningProcessInstanceFailure() {
+        long projectCode = 1L;
+        User loginUser = getAdminUser();
+        Project project = getProject(projectCode);
+        Map<String, Object> result = new HashMap<>(5);
+        putMsg(result, Status.PROJECT_NOT_FOUND, projectCode);
+        int size = 10;
+        String startTime = "2020-01-01 00:00:00";
+        String endTime = "2020-08-02 00:00:00";
+
+        putMsg(result, Status.SUCCESS, projectCode);
+        when(projectMapper.queryByCode(projectCode)).thenReturn(project);
+        when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
+        Map<String, Object> startTimeBiggerFailRes = processInstanceService
+            .queryTopNLongestRunningProcessInstance(loginUser, projectCode, size, endTime, startTime);
+        Assert.assertEquals(Status.START_TIME_BIGGER_THAN_END_TIME_ERROR, startTimeBiggerFailRes.get(Constants.STATUS));
+
+        putMsg(result, Status.SUCCESS, projectCode);
+        when(projectMapper.queryByCode(projectCode)).thenReturn(project);
+        when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
+        Map<String, Object> dataNullFailRes = processInstanceService
+            .queryTopNLongestRunningProcessInstance(loginUser, projectCode, size, null, endTime);
+        Assert.assertEquals(Status.DATA_IS_NULL, dataNullFailRes.get(Constants.STATUS));
+
+        putMsg(result, Status.SUCCESS, projectCode);
+        when(projectMapper.queryByCode(projectCode)).thenReturn(project);
+        when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
+        dataNullFailRes = processInstanceService
+            .queryTopNLongestRunningProcessInstance(loginUser, projectCode, size, startTime, null);
+        Assert.assertEquals(Status.DATA_IS_NULL, dataNullFailRes.get(Constants.STATUS));
+    }
+
     @Test
     public void testQueryProcessInstanceById() {
         long projectCode = 1L;
@@ -299,9 +339,9 @@ public class ProcessInstanceServiceTest {
         // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
-        Map<String, Object> proejctAuthFailRes =
-                processInstanceService.queryProcessInstanceById(loginUser, projectCode, 1);
-        Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS));
+        Map<String, Object> projectAuthFailRes =
+            processInstanceService.queryProcessInstanceById(loginUser, projectCode, 1);
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
 
         // project auth success
         ProcessInstance processInstance = getProcessInstance();
@@ -325,6 +365,11 @@ public class ProcessInstanceServiceTest {
         WorkerGroup workerGroup = getWorkGroup();
         Map<String, Object> workerExistRes = processInstanceService.queryProcessInstanceById(loginUser, projectCode, 1);
         Assert.assertEquals(Status.SUCCESS, workerExistRes.get(Constants.STATUS));
+
+        when(processService.findProcessDefinition(processInstance.getProcessDefinitionCode(),
+            processInstance.getProcessDefinitionVersion())).thenReturn(null);;
+        workerExistRes = processInstanceService.queryProcessInstanceById(loginUser, projectCode, 1);
+        Assert.assertEquals(Status.PROCESS_DEFINE_NOT_EXIST, workerExistRes.get(Constants.STATUS));
     }
 
     @Test
@@ -338,9 +383,9 @@ public class ProcessInstanceServiceTest {
         // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
-        Map<String, Object> proejctAuthFailRes =
-                processInstanceService.queryTaskListByProcessId(loginUser, projectCode, 1);
-        Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS));
+        Map<String, Object> projectAuthFailRes =
+            processInstanceService.queryTaskListByProcessId(loginUser, projectCode, 1);
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
 
         // project auth success
         putMsg(result, Status.SUCCESS, projectCode);
@@ -374,6 +419,11 @@ public class ProcessInstanceServiceTest {
         Map<String, DependResult> resultMap =
                 processInstanceService.parseLogForDependentResult(logString);
         Assert.assertEquals(1, resultMap.size());
+
+        resultMap.clear();
+        resultMap = processInstanceService.parseLogForDependentResult("");
+        Assert.assertEquals(0, resultMap.size());
+
     }
 
     @Test
@@ -387,9 +437,9 @@ public class ProcessInstanceServiceTest {
         // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
-        Map<String, Object> proejctAuthFailRes =
+        Map<String, Object> projectAuthFailRes =
                 processInstanceService.querySubProcessInstanceByTaskId(loginUser, projectCode, 1);
-        Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS));
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
 
         // task null
         putMsg(result, Status.SUCCESS, projectCode);
@@ -413,6 +463,13 @@ public class ProcessInstanceServiceTest {
                 processInstanceService.querySubProcessInstanceByTaskId(loginUser, projectCode, 1);
         Assert.assertEquals(Status.TASK_INSTANCE_NOT_SUB_WORKFLOW_INSTANCE, notSubprocessRes.get(Constants.STATUS));
 
+        putMsg(result, Status.SUCCESS, projectCode);
+        taskDefinition.setProjectCode(0L);
+        notSubprocessRes = processInstanceService.querySubProcessInstanceByTaskId(loginUser, projectCode, 1);
+        Assert.assertEquals(Status.TASK_INSTANCE_NOT_EXISTS, notSubprocessRes.get(Constants.STATUS));
+
+        taskDefinition.setProjectCode(projectCode);
+        when(taskDefinitionMapper.queryByCode(taskInstance.getTaskCode())).thenReturn(taskDefinition);
         // sub process not exist
         TaskInstance subTask = getTaskInstance();
         subTask.setTaskType("SUB_PROCESS");
@@ -445,9 +502,9 @@ public class ProcessInstanceServiceTest {
         // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, INSTANCE_UPDATE)).thenReturn(result);
-        Map<String, Object> proejctAuthFailRes = processInstanceService.updateProcessInstance(loginUser, projectCode, 1,
+        Map<String, Object> projectAuthFailRes = processInstanceService.updateProcessInstance(loginUser, projectCode, 1,
                 shellJson, taskJson, "2020-02-21 00:00:00", true, "", "", 0, "");
-        Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS));
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
 
         // process instance null
         putMsg(result, Status.SUCCESS, projectCode);
@@ -522,9 +579,9 @@ public class ProcessInstanceServiceTest {
         // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, WORKFLOW_INSTANCE)).thenReturn(result);
-        Map<String, Object> proejctAuthFailRes =
+        Map<String, Object> projectAuthFailRes =
                 processInstanceService.queryParentInstanceBySubId(loginUser, projectCode, 1);
-        Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS));
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
 
         // process instance null
         putMsg(result, Status.SUCCESS, projectCode);
@@ -570,11 +627,58 @@ public class ProcessInstanceServiceTest {
         Project project = getProject(projectCode);
         Map<String, Object> result = new HashMap<>();
         putMsg(result, Status.PROJECT_NOT_FOUND, projectCode);
-        // process instance null
-        putMsg(result, Status.SUCCESS, projectCode);
+
+        // project auth fail
         when(projectMapper.queryByCode(projectCode)).thenReturn(project);
         when(projectService.checkProjectAndAuth(loginUser, project, projectCode, INSTANCE_DELETE)).thenReturn(result);
-        when(processService.findProcessInstanceDetailById(1)).thenReturn(null);
+        Map<String, Object> projectAuthFailRes =
+            processInstanceService.deleteProcessInstanceById(loginUser, projectCode, 1);
+        Assert.assertEquals(Status.PROJECT_NOT_FOUND, projectAuthFailRes.get(Constants.STATUS));
+
+        // not sub process
+        ProcessInstance processInstance = getProcessInstance();
+        processInstance.setIsSubProcess(Flag.NO);
+        processInstance.setState(WorkflowExecutionStatus.RUNNING_EXECUTION);
+        putMsg(result, Status.SUCCESS, projectCode);
+        when(processService.findProcessInstanceDetailById(1)).thenReturn(Optional.ofNullable(processInstance));
+        try {
+            processInstanceService.deleteProcessInstanceById(loginUser, projectCode, 1);
+            Assert.fail();
+        } catch (ServiceException ex) {
+            Assert.assertEquals(Status.PROCESS_INSTANCE_STATE_OPERATION_ERROR.getCode(), ex.getCode());
+        }
+
+        processInstance.setState(WorkflowExecutionStatus.SUCCESS);
+        processInstance.setState(WorkflowExecutionStatus.SUCCESS);
+        processInstance.setTimeout(3000);
+        processInstance.setCommandType(CommandType.STOP);
+        processInstance.setProcessDefinitionCode(46L);
+        processInstance.setProcessDefinitionVersion(1);
+        ProcessDefinition processDefinition = getProcessDefinition();
+        processDefinition.setId(1);
+        processDefinition.setUserId(1);
+        processDefinition.setProjectCode(0L);
+        when(processDefineMapper.queryByCode(46L)).thenReturn(processDefinition);
+        try {
+            processInstanceService.deleteProcessInstanceById(loginUser, projectCode, 1);
+            Assert.fail();
+        } catch (ServiceException ex) {
+            Assert.assertEquals(Status.PROCESS_INSTANCE_NOT_EXIST.getCode(), ex.getCode());
+        }
+
+        processDefinition.setProjectCode(projectCode);
+        when(processService.deleteWorkProcessInstanceById(1)).thenReturn(1);
+        Map<String, Object> successRes =
+            processInstanceService.deleteProcessInstanceById(loginUser, projectCode, 1);
+        Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS));
+
+        when(processService.deleteWorkProcessInstanceById(1)).thenReturn(0);
+        try {
+            processInstanceService.deleteProcessInstanceById(loginUser, projectCode, 1);
+            Assert.fail();
+        } catch (ServiceException ex) {
+            Assert.assertEquals(Status.DELETE_PROCESS_INSTANCE_BY_ID_ERROR.getCode(), ex.getCode());
+        }
     }
 
     @Test
@@ -587,6 +691,11 @@ public class ProcessInstanceServiceTest {
         when(processInstanceMapper.queryDetailById(1)).thenReturn(processInstance);
         Map<String, Object> successRes = processInstanceService.viewVariables(1L, 1);
         Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS));
+
+        when(processInstanceMapper.queryDetailById(1)).thenReturn(null);
+        Map<String, Object> processNotExist = processInstanceService.viewVariables(1L, 1);
+        Assert.assertEquals(Status.PROCESS_INSTANCE_NOT_EXIST, processNotExist.get(Constants.STATUS));
+
     }
 
     @Test
@@ -611,6 +720,10 @@ public class ProcessInstanceServiceTest {
 
         Map<String, Object> successRes = processInstanceService.viewGantt(0L, 1);
         Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS));
+
+        when(processInstanceMapper.queryDetailById(1)).thenReturn(null);
+        Map<String, Object> processNotExist = processInstanceService.viewVariables(1L, 1);
+        Assert.assertEquals(Status.PROCESS_INSTANCE_NOT_EXIST, processNotExist.get(Constants.STATUS));
     }
 
     /**

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

@@ -353,6 +353,10 @@ public final class DateUtils {
         if (end == null) {
             end = new Date();
         }
+        if (start.after(end)) {
+            logger.warn("start Time {} is later than end Time {}", start, end);
+            return null;
+        }
         return format2Duration(differMs(start, end));
     }
 

dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/DateUtilsTest.java

@@ -176,6 +176,9 @@ public class DateUtilsTest {
         String duration = DateUtils.format2Duration(start, end);
         Assert.assertEquals("1d 1h 10m 10s", duration);
 
+        duration = DateUtils.format2Duration(end, start);
+        Assert.assertNull(duration);
+
         // hours minutes seconds
         start = DateUtils.stringToDate("2020-01-20 11:00:00");
         end = DateUtils.stringToDate("2020-01-20 12:10:10");

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtils.java

@@ -0,0 +1,43 @@
+/*
+ * 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.dao.utils;
+
+import java.util.Date;
+import org.apache.dolphinscheduler.common.utils.DateUtils;
+import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
+
+/**
+ * workflow utils
+ */
+public class WorkflowUtils {
+
+    /**
+     * get workflow duration
+     * if processInstance is running, the endTime will be the current time
+     *
+     * @param processInstance workflow instance
+     * @return workflow duration
+     */
+    public static String getWorkflowInstanceDuration (ProcessInstance processInstance) {
+        return processInstance.getState() != null && processInstance.getState().isFinished() ?
+            DateUtils.format2Duration(processInstance.getStartTime(), processInstance.getEndTime()) :
+            DateUtils.format2Duration(processInstance.getStartTime(), new Date());
+    }
+
+}
+

dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtilsTest.java

@@ -0,0 +1,51 @@
+/*
+ * 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.dao.utils;
+
+import java.util.Date;
+import org.apache.dolphinscheduler.common.enums.WorkflowExecutionStatus;
+import org.apache.dolphinscheduler.common.utils.DateUtils;
+import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class WorkflowUtilsTest {
+
+    @Test
+    public void testGetWorkflowInstanceDuration() {
+        ProcessInstance processInstance = new ProcessInstance();
+        processInstance.setId(1);
+        processInstance.setState(null);
+        Date start = DateUtils.stringToDate("2020-01-20 11:00:00");
+        Date end = DateUtils.stringToDate("2020-01-21 12:10:10");
+        processInstance.setStartTime(start);
+        processInstance.setEndTime(end);
+
+        String noStateDuration = WorkflowUtils.getWorkflowInstanceDuration(processInstance);
+        System.currentTimeMillis();
+        Assertions.assertNotEquals("1d 1h 10m 10s", noStateDuration);
+
+        processInstance.setState(WorkflowExecutionStatus.RUNNING_EXECUTION);
+        String notFinishDuration = WorkflowUtils.getWorkflowInstanceDuration(processInstance);
+        Assertions.assertNotEquals("1d 1h 10m 10s", notFinishDuration);
+
+        processInstance.setState(WorkflowExecutionStatus.SUCCESS);
+        String successDuration = WorkflowUtils.getWorkflowInstanceDuration(processInstance);
+        Assertions.assertEquals("1d 1h 10m 10s", successDuration);
+    }
+}