From 17cd644506872c1b8e3f9ddf657371580c35e4e6 Mon Sep 17 00:00:00 2001 From: Yann Ann Date: Thu, 13 Oct 2022 09:50:31 +0800 Subject: [PATCH] [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 --- .../impl/ProcessInstanceServiceImpl.java | 4 +- .../service/ProcessInstanceServiceTest.java | 149 +++++++++++++++--- .../common/utils/DateUtils.java | 4 + .../common/utils/DateUtilsTest.java | 3 + .../dao/utils/WorkflowUtils.java | 43 +++++ .../dao/utils/WorkflowUtilsTest.java | 51 ++++++ 6 files changed, 234 insertions(+), 20 deletions(-) create mode 100644 dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtils.java create mode 100644 dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtilsTest.java diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java index c344badd29..e41543ff4e 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java +++ b/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()); diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java index a3b604066e..c2de44783a 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java +++ b/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 proejctAuthFailRes = processInstanceService + Map 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 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 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 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 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 proejctAuthFailRes = - processInstanceService.queryProcessInstanceById(loginUser, projectCode, 1); - Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS)); + Map 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 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 proejctAuthFailRes = - processInstanceService.queryTaskListByProcessId(loginUser, projectCode, 1); - Assert.assertEquals(Status.PROJECT_NOT_FOUND, proejctAuthFailRes.get(Constants.STATUS)); + Map 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 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 proejctAuthFailRes = + Map 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 proejctAuthFailRes = processInstanceService.updateProcessInstance(loginUser, projectCode, 1, + Map 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 proejctAuthFailRes = + Map 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 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 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 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 successRes = processInstanceService.viewVariables(1L, 1); Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS)); + + when(processInstanceMapper.queryDetailById(1)).thenReturn(null); + Map 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 successRes = processInstanceService.viewGantt(0L, 1); Assert.assertEquals(Status.SUCCESS, successRes.get(Constants.STATUS)); + + when(processInstanceMapper.queryDetailById(1)).thenReturn(null); + Map processNotExist = processInstanceService.viewVariables(1L, 1); + Assert.assertEquals(Status.PROCESS_INSTANCE_NOT_EXIST, processNotExist.get(Constants.STATUS)); } /** diff --git a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/DateUtils.java b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/DateUtils.java index f6e38ac503..08890e86fb 100644 --- a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/DateUtils.java +++ b/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)); } diff --git a/dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/DateUtilsTest.java b/dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/DateUtilsTest.java index 0b0b7c84fd..e4cbff3223 100644 --- a/dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/DateUtilsTest.java +++ b/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"); diff --git a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtils.java b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtils.java new file mode 100644 index 0000000000..e2c2ccb31c --- /dev/null +++ b/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()); + } + +} + diff --git a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtilsTest.java b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/utils/WorkflowUtilsTest.java new file mode 100644 index 0000000000..985066cbdb --- /dev/null +++ b/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); + } +} \ No newline at end of file