From c1adb7bf82ea2dba6488edd0e3c66b2edbd4b150 Mon Sep 17 00:00:00 2001 From: Joram Barrez Date: Fri, 30 Aug 2024 14:43:53 +0200 Subject: [PATCH] Fix bug: stage plan item instance automatically being completed when setting variable through runtimeService in a lifecycleListener --- .../impl/agenda/DefaultCmmnEngineAgenda.java | 2 + ...tChangePlanItemInstanceStateOperation.java | 9 + .../AbstractEvaluationCriteriaOperation.java | 2 +- .../impl/agenda/operation/CmmnOperation.java | 7 + .../entity/PlanItemInstanceEntity.java | 5 +- .../entity/PlanItemInstanceEntityImpl.java | 12 ++ .../test/AbstractFlowableCmmnTestCase.java | 3 +- .../test/delegate/TestSetVariableBean.java | 28 +++ .../test/runtime/StageCompletionTest.java | 65 ++++++ .../src/test/resources/flowable.cmmn.cfg.xml | 5 +- ...estSetVariableInLifecycleListener.cmmn.xml | 193 ++++++++++++++++++ 11 files changed, 327 insertions(+), 4 deletions(-) create mode 100644 modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/delegate/TestSetVariableBean.java create mode 100644 modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/StageCompletionTest.java create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/StageCompletionTest.testSetVariableInLifecycleListener.cmmn.xml diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java index 66aae05a5fd..b4438edfe9d 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java @@ -69,6 +69,8 @@ public DefaultCmmnEngineAgenda(CommandContext commandContext) { } public void addOperation(CmmnOperation operation) { + + operation.onPlanned(); int operationIndex = getOperationIndex(operation); if (operationIndex >= 0) { diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractChangePlanItemInstanceStateOperation.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractChangePlanItemInstanceStateOperation.java index 2616ea28d2b..745d095ec96 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractChangePlanItemInstanceStateOperation.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractChangePlanItemInstanceStateOperation.java @@ -34,6 +34,12 @@ public AbstractChangePlanItemInstanceStateOperation(CommandContext commandContex super(commandContext, planItemInstanceEntity); } + @Override + public void onPlanned() { + // The plan item is marked as being 'in flux'. After the state is changed, the flag is changed back (see below). + this.planItemInstanceEntity.setStateChangeUnprocessed(true); + } + @Override public void run() { String oldState = planItemInstanceEntity.getState(); @@ -53,12 +59,15 @@ public void run() { } planItemInstanceEntity.setState(newState); + CmmnEngineConfiguration cmmnEngineConfiguration =CommandContextUtil.getCmmnEngineConfiguration(commandContext); cmmnEngineConfiguration.getListenerNotificationHelper().executeLifecycleListeners( commandContext, planItemInstanceEntity, oldState, getNewState()); CommandContextUtil.getAgenda(commandContext).planEvaluateCriteriaOperation(planItemInstanceEntity.getCaseInstanceId(), createPlanItemLifeCycleEvent()); internalExecute(); + + planItemInstanceEntity.setStateChangeUnprocessed(false); if (CommandContextUtil.getCmmnEngineConfiguration(commandContext).isLoggingSessionEnabled()) { String loggingType = null; diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java index 152be2b31b2..7f8f4e4dcb7 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java @@ -145,7 +145,7 @@ public boolean evaluateForCompletion(PlanItemInstanceEntity planItemInstanceEnti } else if (planItem.getPlanItemDefinition() instanceof Stage) { - if (PlanItemInstanceState.ACTIVE.equals(state)) { + if (PlanItemInstanceState.ACTIVE.equals(state) && !planItemInstanceEntity.isStateChangeUnprocessed()) { boolean criteriaChangeOrActiveChildrenForStage = evaluatePlanItemsCriteria(planItemInstanceEntity, null); if (criteriaChangeOrActiveChildrenForStage) { evaluationResult.markCriteriaChanged(); diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/CmmnOperation.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/CmmnOperation.java index b81ed4f42ce..e4f8f2e6fbf 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/CmmnOperation.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/CmmnOperation.java @@ -53,6 +53,13 @@ public CmmnOperation(CommandContext commandContext) { this.commandContext = commandContext; } + /** + * Called when the operation is planned on the agenda (but not yet executed) + */ + public void onPlanned() { + // No-op by default + } + /** * @return The id of the case instance related to this operation. */ diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java index 0ab87907f19..2d1553a9b85 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java @@ -27,6 +27,9 @@ public interface PlanItemInstanceEntity extends Entity, HasRevision, DelegatePla VariableScope getParentVariableScope(); boolean isPlannedForActivationInMigration(); - void setPlannedForActivationInMigration(boolean plannedForActivationInMigration); + + boolean isStateChangeUnprocessed(); + void setStateChangeUnprocessed(boolean stateChangeUnprocessed); + } diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java index 94df68d8934..ef9409dc6cd 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java @@ -94,6 +94,8 @@ public class PlanItemInstanceEntityImpl extends AbstractCmmnEngineVariableScopeE protected FlowableListener currentFlowableListener; // Only set when executing an plan item lifecycle listener protected boolean plannedForActivationInMigration; + protected boolean stateChangeUnprocessed; // only set to true when an agenda operation is planned and this has not been executed yet + public PlanItemInstanceEntityImpl() { } @@ -657,6 +659,16 @@ public void setPlannedForActivationInMigration(boolean plannedForActivationInMig this.plannedForActivationInMigration = plannedForActivationInMigration; } + @Override + public boolean isStateChangeUnprocessed() { + return stateChangeUnprocessed; + } + + @Override + public void setStateChangeUnprocessed(boolean stateChangeUnprocessed) { + this.stateChangeUnprocessed = stateChangeUnprocessed; + } + @Override public String toString() { StringBuilder stringBuilder = new StringBuilder(); diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/test/AbstractFlowableCmmnTestCase.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/test/AbstractFlowableCmmnTestCase.java index 80a43c4c8ec..5c7a4f34aa6 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/test/AbstractFlowableCmmnTestCase.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/test/AbstractFlowableCmmnTestCase.java @@ -267,7 +267,8 @@ protected void assertPlanItemInstanceState(List planItemInstan .collect(Collectors.toList()); if (planItemInstanceStates.isEmpty()) { - fail("No plan item instances found with name " + name); + List planItemInstanceNames = planItemInstances.stream().map(PlanItemInstance::getName).collect(Collectors.toList()); + fail("No plan item instances found with name " + name + ", following names were found:" + String.join(",", planItemInstanceNames)); } assertEquals("Incorrect number of states found: " + planItemInstanceStates, states.length, planItemInstanceStates.size()); diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/delegate/TestSetVariableBean.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/delegate/TestSetVariableBean.java new file mode 100644 index 00000000000..90540b6421c --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/delegate/TestSetVariableBean.java @@ -0,0 +1,28 @@ +/* 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.flowable.cmmn.test.delegate; + +import org.flowable.cmmn.api.CmmnRuntimeService; +import org.flowable.cmmn.engine.impl.util.CommandContextUtil; + +/** + * @author Joram Barrez + */ +public class TestSetVariableBean { + + public void setVariable(String caseInstanceId, String variableName, Object variableValue) { + CmmnRuntimeService cmmnRuntimeService = CommandContextUtil.getCmmnEngineConfiguration().getCmmnRuntimeService(); + cmmnRuntimeService.setVariable(caseInstanceId, variableName, variableValue); + } + +} diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/StageCompletionTest.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/StageCompletionTest.java new file mode 100644 index 00000000000..9eeb192c0cf --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/StageCompletionTest.java @@ -0,0 +1,65 @@ +/* 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.flowable.cmmn.test.runtime; + +import java.util.List; + +import org.flowable.cmmn.api.runtime.CaseInstance; +import org.flowable.cmmn.api.runtime.PlanItemInstance; +import org.flowable.cmmn.api.runtime.PlanItemInstanceState; +import org.flowable.cmmn.engine.test.CmmnDeployment; +import org.flowable.cmmn.engine.test.FlowableCmmnTestCase; +import org.flowable.task.api.Task; +import org.junit.Test; + +/** + * @author Joram Barrez + */ +public class StageCompletionTest extends FlowableCmmnTestCase { + + @Test + @CmmnDeployment + public void testSetVariableInLifecycleListener() { + + /* + * This test has a case which has a stage with a lifecycle listener that sets a variable through the cmmnRuntimeService. + * Before introducing the flag 'isStateChangeUnprocessed' on PlanItemInstanceEntity, this would lead to the following problem: + * + * startCaseInstance --> new CommandContext --> plan operations for initialization + moving stage plan item instance to state 'active' + * setVariable, through a service, will reuse the existing commandContext. However, the SetVariableCmd plans an explicit evaluation operation (for good reasons) + * + * When the evaluation operations executes, the stage has moved into the 'active' state, however when the lifecycle listener executes + * no child plan item instances are created yet. The logic deems correctly that this constellation means the stage should complete. + * + * Introducing the stateChangeUnprocessed flag on the stage plan item instance fixes this problem: it avoids looking into stages that + * are still being initialized but have a setup that would otherwise automatically complete the stage plan item instance. + */ + + CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("myCase").start(); + List planItemInstances = cmmnRuntimeService.createPlanItemInstanceQuery().caseInstanceId(caseInstance.getId()).list(); + assertPlanItemInstanceState(caseInstance, "stageWithLifecycleListener", PlanItemInstanceState.AVAILABLE); + + // Triggering the user event listener activates the stage + cmmnRuntimeService.completeUserEventListenerInstance(planItemInstances.stream() + .filter(planItemInstance -> "A".equalsIgnoreCase(planItemInstance.getName())).findAny().get().getId()); + + assertPlanItemInstanceState(caseInstance, "stageWithLifecycleListener", PlanItemInstanceState.ACTIVE); + + // Completing the user tasks should complete the case instance + Task task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).singleResult(); + cmmnTaskService.complete(task.getId()); + + assertCaseInstanceEnded(caseInstance); + } + +} diff --git a/modules/flowable-cmmn-engine/src/test/resources/flowable.cmmn.cfg.xml b/modules/flowable-cmmn-engine/src/test/resources/flowable.cmmn.cfg.xml index 6c244497dcd..0f8bce19e70 100644 --- a/modules/flowable-cmmn-engine/src/test/resources/flowable.cmmn.cfg.xml +++ b/modules/flowable-cmmn-engine/src/test/resources/flowable.cmmn.cfg.xml @@ -59,6 +59,7 @@ + @@ -70,5 +71,7 @@ - + + + diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/StageCompletionTest.testSetVariableInLifecycleListener.cmmn.xml b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/StageCompletionTest.testSetVariableInLifecycleListener.cmmn.xml new file mode 100644 index 00000000000..d357244d747 --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/StageCompletionTest.testSetVariableInLifecycleListener.cmmn.xml @@ -0,0 +1,193 @@ + + + + + + + + + + + + + + + + + + + + + + + complete + + + + + + + + complete + + + + + + + + occur + + + + + + + + occur + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file