Fix bug: stage plan item instance automatically being completed when …

…setting variable through runtimeService in a lifecycleListener

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) {

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;

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();

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.
      */

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);
+
 }

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();

modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/test/AbstractFlowableCmmnTestCase.java

@@ -267,7 +267,8 @@ protected void assertPlanItemInstanceState(List<PlanItemInstance> planItemInstan
             .collect(Collectors.toList());
 
         if (planItemInstanceStates.isEmpty()) {
-            fail("No plan item instances found with name " + name);
+            List<String> 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());

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);
+    }
+
+}

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<PlanItemInstance> 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);
+    }
+
+}

modules/flowable-cmmn-engine/src/test/resources/flowable.cmmn.cfg.xml

@@ -59,6 +59,7 @@
                 </entry>
             </map>
         </property>
+
     </bean>
 
     <bean id="testBean" class="org.flowable.cmmn.test.delegate.TestBean" />
@@ -70,5 +71,7 @@
     <bean id="timerBean" class="org.flowable.cmmn.test.bean.TimerBean" />
 
     <bean id="assignmentBean" class="org.flowable.cmmn.test.delegate.AssignmentBean" />
-
+
+    <bean id="setVariableBean" class="org.flowable.cmmn.test.delegate.TestSetVariableBean" />
+
 </beans>