From f8587e32913e77d2ad7f384a08fc999da8d19fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20Moreira?= Date: Tue, 21 Feb 2023 19:23:06 +0000 Subject: [PATCH] fix/MNT-23416_boundary_event_sub_subprocess (#4235) * MNT-23416: test where main-process doesn't receive an error event thrown by a subprocess within a subprocess * fix error propagation between child process and parent call activity execution * polish findCatchingEventsAndExecuteCatchForCallActivity method * polish ErrorPropagation with try-finally * Refactor methods to improve code readability --------- Co-authored-by: Igor Dianov Co-authored-by: Elias Ricken de Medeiros <26007058+erdemedeiros@users.noreply.github.com> --- .../engine/delegate/DelegateExecution.java | 6 + .../impl/bpmn/helper/ErrorPropagation.java | 122 ++++++++++-------- .../entity/ExecutionEntityImpl.java | 7 +- .../entity/ExecutionEntityImplTest.java | 55 ++++++++ .../event/error/BoundaryErrorEventTest.java | 18 +++ ...t.subprocess2ndLevelThrowsError.bpmn20.xml | 34 +++++ 6 files changed, 188 insertions(+), 54 deletions(-) create mode 100644 activiti-core/activiti-engine/src/test/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImplTest.java create mode 100644 activiti-core/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.subprocess2ndLevelThrowsError.bpmn20.xml diff --git a/activiti-core/activiti-engine/src/main/java/org/activiti/engine/delegate/DelegateExecution.java b/activiti-core/activiti-engine/src/main/java/org/activiti/engine/delegate/DelegateExecution.java index b480f998c6..e7943329b8 100644 --- a/activiti-core/activiti-engine/src/main/java/org/activiti/engine/delegate/DelegateExecution.java +++ b/activiti-core/activiti-engine/src/main/java/org/activiti/engine/delegate/DelegateExecution.java @@ -44,6 +44,12 @@ public interface DelegateExecution extends VariableScope { */ String getRootProcessInstanceId(); + /** + * Determines if the current execution is the root one + * @return true if the current execution is the root one; false otherwise + */ + boolean isRootExecution(); + /** * Will contain the event name in case this execution is passed in for an {@link ExecutionListener}. */ diff --git a/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ErrorPropagation.java b/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ErrorPropagation.java index 11e933c54c..e75ca3b67b 100644 --- a/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ErrorPropagation.java +++ b/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ErrorPropagation.java @@ -51,59 +51,13 @@ public class ErrorPropagation { public static void propagateError(String errorRef, DelegateExecution execution) { Map> eventMap = findCatchingEventsForProcess(execution.getProcessDefinitionId(), errorRef); - if (eventMap.size() > 0) { - executeCatch(eventMap, execution, errorRef); - } else if (!execution.getProcessInstanceId().equals(execution.getRootProcessInstanceId())) { // Call activity - ExecutionEntityManager executionEntityManager = Context.getCommandContext().getExecutionEntityManager(); - ExecutionEntity processInstanceExecution = executionEntityManager.findById(execution.getProcessInstanceId()); - if (processInstanceExecution != null) { - - ExecutionEntity parentExecution = processInstanceExecution.getSuperExecution(); - - Set toDeleteProcessInstanceIds = new HashSet(); - toDeleteProcessInstanceIds.add(execution.getProcessInstanceId()); - - while (parentExecution != null && eventMap.size() == 0) { - eventMap = findCatchingEventsForProcess(parentExecution.getProcessDefinitionId(), errorRef); - if (eventMap.size() > 0) { - - for (String processInstanceId : toDeleteProcessInstanceIds) { - ExecutionEntity processInstanceEntity = executionEntityManager.findById(processInstanceId); - - // Delete - executionEntityManager.deleteProcessInstanceExecutionEntity(processInstanceEntity.getId(), - execution.getCurrentFlowElement() != null ? execution.getCurrentFlowElement().getId() : null, - "ERROR_EVENT " + errorRef, - false, false); - - // Event - if (Context.getProcessEngineConfiguration() != null && Context.getProcessEngineConfiguration().getEventDispatcher().isEnabled()) { - Context.getProcessEngineConfiguration().getEventDispatcher() - .dispatchEvent(ActivitiEventBuilder.createEntityEvent(ActivitiEventType.PROCESS_COMPLETED_WITH_ERROR_END_EVENT, processInstanceEntity)); - } - } - executeCatch(eventMap, parentExecution, errorRef); - - } else { - toDeleteProcessInstanceIds.add(parentExecution.getProcessInstanceId()); - ExecutionEntity superExecution = parentExecution.getSuperExecution(); - if (superExecution != null) { - parentExecution = superExecution; - } else if (!parentExecution.getId().equals(parentExecution.getRootProcessInstanceId())) { // stop at the root - parentExecution = parentExecution.getProcessInstance(); - } else { - parentExecution = null; - } - } + try { + executeCatch(eventMap, execution, errorRef); + } finally { + if (eventMap.size() == 0) { + throw new BpmnError(errorRef, "No catching boundary event found for error with errorCode '" + errorRef + "', neither in same process nor in parent process"); } - - } - - } - - if (eventMap.size() == 0) { - throw new BpmnError(errorRef, "No catching boundary event found for error with errorCode '" + errorRef + "', neither in same process nor in parent process"); } } @@ -163,12 +117,74 @@ public class ErrorPropagation { if (matchingEvent != null && parentExecution != null) { executeEventHandler(matchingEvent, parentExecution, currentExecution, errorId); - } else { + } + else if (isCallActivity(delegateExecution)) { + eventMap.putAll(findCatchingEventsAndExecuteCatchForCallActivity(errorId, delegateExecution)); + } + else { throw new ActivitiException("No matching parent execution for error code " + errorId + " found"); } } - protected static void executeEventHandler(Event event, ExecutionEntity parentExecution, ExecutionEntity currentExecution, String errorId) { + private static boolean isCallActivity(DelegateExecution delegateExecution) { + return !delegateExecution.getProcessInstanceId() + .equals(delegateExecution.getRootProcessInstanceId()); + } + + protected static Map> findCatchingEventsAndExecuteCatchForCallActivity(String errorRef, DelegateExecution execution) { + ExecutionEntityManager executionEntityManager = Context.getCommandContext().getExecutionEntityManager(); + ExecutionEntity processInstanceExecution = executionEntityManager.findById(execution.getProcessInstanceId()); + + Map> eventMap = Collections.emptyMap(); + if (processInstanceExecution != null) { + + ExecutionEntity parentExecution = processInstanceExecution.getSuperExecution(); + + Set toDeleteProcessInstanceIds = new HashSet<>(); + toDeleteProcessInstanceIds.add(execution.getProcessInstanceId()); + + while (!parentExecution.isRootExecution() && eventMap.isEmpty()) { + eventMap = findCatchingEventsForProcess(parentExecution.getProcessDefinitionId(), errorRef); + if (!eventMap.isEmpty()) { + for (String processInstanceId : toDeleteProcessInstanceIds) { + deleteProcessInstanceEntity(errorRef, execution, executionEntityManager, processInstanceId); + } + executeCatch(eventMap, parentExecution, errorRef); + } else { + toDeleteProcessInstanceIds.add(parentExecution.getProcessInstanceId()); + ExecutionEntity superExecution = parentExecution.getSuperExecution(); + if (superExecution != null) { + parentExecution = superExecution; + } else { + parentExecution = parentExecution.getProcessInstance(); + } + } + } + } + + return eventMap; + } + + private static void deleteProcessInstanceEntity(String errorRef, DelegateExecution execution, + ExecutionEntityManager executionEntityManager, String processInstanceId) { + ExecutionEntity processInstanceEntity = executionEntityManager.findById(processInstanceId); + + executionEntityManager.deleteProcessInstanceExecutionEntity(processInstanceEntity.getId(), + execution.getCurrentFlowElement() != null ? execution.getCurrentFlowElement().getId() : null, + "ERROR_EVENT " + errorRef, + false, false); + dispatchProcessErroredEvent(processInstanceEntity); + } + + private static void dispatchProcessErroredEvent(ExecutionEntity processInstanceEntity) { + if (Context.getProcessEngineConfiguration() != null && Context.getProcessEngineConfiguration().getEventDispatcher().isEnabled()) { + Context.getProcessEngineConfiguration().getEventDispatcher() + .dispatchEvent(ActivitiEventBuilder.createEntityEvent(ActivitiEventType.PROCESS_COMPLETED_WITH_ERROR_END_EVENT, + processInstanceEntity)); + } + } + + protected static void executeEventHandler(Event event, ExecutionEntity parentExecution, ExecutionEntity currentExecution, String errorId) { if (Context.getProcessEngineConfiguration() != null && Context.getProcessEngineConfiguration().getEventDispatcher().isEnabled()) { BpmnModel bpmnModel = ProcessDefinitionUtil.getBpmnModel(parentExecution.getProcessDefinitionId()); if (bpmnModel != null) { diff --git a/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImpl.java b/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImpl.java index 2ef6672312..a01646e0cf 100644 --- a/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImpl.java +++ b/activiti-core/activiti-engine/src/main/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImpl.java @@ -496,7 +496,12 @@ public class ExecutionEntityImpl extends VariableScopeImpl implements ExecutionE this.rootProcessInstanceId = rootProcessInstanceId; } - // scopes /////////////////////////////////////////////////////////////////// + @Override + public boolean isRootExecution() { + return id != null && id.equals(rootProcessInstanceId); + } + + // scopes /////////////////////////////////////////////////////////////////// public boolean isScope() { return isScope; diff --git a/activiti-core/activiti-engine/src/test/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImplTest.java b/activiti-core/activiti-engine/src/test/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImplTest.java new file mode 100644 index 0000000000..ac3349a27e --- /dev/null +++ b/activiti-core/activiti-engine/src/test/java/org/activiti/engine/impl/persistence/entity/ExecutionEntityImplTest.java @@ -0,0 +1,55 @@ +/* + * Copyright 2010-2020 Alfresco Software, Ltd. + * + * Licensed 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.activiti.engine.impl.persistence.entity; + + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; + +public class ExecutionEntityImplTest { + + public static final String ROOT_ID = "rootId"; + + @Test + public void isRootExecution_should_returnTrue_whenIdIsSameAsRoot() { + final ExecutionEntityImpl executionEntity = new ExecutionEntityImpl(); + executionEntity.setId(ROOT_ID); + executionEntity.setRootProcessInstanceId(ROOT_ID); + + assertThat(executionEntity.isRootExecution()).isTrue(); + + } + + @Test + public void isRootExecution_should_returnFalse_whenIdIsNotSameAsRoot() { + final ExecutionEntityImpl executionEntity = new ExecutionEntityImpl(); + executionEntity.setId("anotherId"); + executionEntity.setRootProcessInstanceId(ROOT_ID); + + assertThat(executionEntity.isRootExecution()).isFalse(); + + } + + @Test + public void isRootExecution_should_returnFalse_whenIdIsNull() { + final ExecutionEntityImpl executionEntity = new ExecutionEntityImpl(); + executionEntity.setId(null); + + assertThat(executionEntity.isRootExecution()).isFalse(); + } +} diff --git a/activiti-core/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.java b/activiti-core/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.java index 0a166c8766..fa497ea3b6 100644 --- a/activiti-core/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.java +++ b/activiti-core/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.java @@ -212,6 +212,24 @@ public class BoundaryErrorEventTest extends PluggableActivitiTestCase { assertProcessEnded(procId); } + @Deployment(resources = { "org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.testCatchErrorOnCallActivity-parent.bpmn20.xml", + "org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.subprocess2ndLevelThrowsError.bpmn20.xml" }) + public void testCatchErrorOnCallActivityWithSubprocess() { + String procId = runtimeService.startProcessInstanceByKey("catchErrorOnCallActivity").getId(); + Task task = taskService.createTaskQuery().singleResult(); + assertThat(task.getName()).isEqualTo("Task in subprocess"); + + // Completing the task will reach the end error event, + // which is caught on the call activity boundary + taskService.complete(task.getId()); + task = taskService.createTaskQuery().singleResult(); + assertThat(task.getName()).isEqualTo("Escalated Task"); + + // Completing the task will end the process instance + taskService.complete(task.getId()); + assertProcessEnded(procId); + } + @Deployment(resources = { "org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.subprocess.bpmn20.xml" }) public void testUncaughtError() { runtimeService.startProcessInstanceByKey("simpleSubProcess"); diff --git a/activiti-core/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.subprocess2ndLevelThrowsError.bpmn20.xml b/activiti-core/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.subprocess2ndLevelThrowsError.bpmn20.xml new file mode 100644 index 0000000000..f360a3a9de --- /dev/null +++ b/activiti-core/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/event/error/BoundaryErrorEventTest.subprocess2ndLevelThrowsError.bpmn20.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + -- GitLab