diff --git a/modules/flowable-cmmn-engine-configurator/pom.xml b/modules/flowable-cmmn-engine-configurator/pom.xml index 3aee5bbd81b..cf8c0efca62 100644 --- a/modules/flowable-cmmn-engine-configurator/pom.xml +++ b/modules/flowable-cmmn-engine-configurator/pom.xml @@ -62,6 +62,11 @@ mockito-core test + + org.apache.groovy + groovy-jsr223 + test + net.javacrumbs.json-unit json-unit-assertj diff --git a/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java index ec4c05fe97e..a86a11567aa 100644 --- a/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java +++ b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java @@ -15,7 +15,9 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.Date; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import org.flowable.cmmn.api.CmmnHistoryService; @@ -52,6 +54,8 @@ public abstract class AbstractProcessEngineIntegrationTest { protected static CmmnEngineConfiguration cmmnEngineConfiguration; protected static ProcessEngine processEngine; + protected static Map beans = new HashMap<>(); + protected CmmnRepositoryService cmmnRepositoryService; protected CmmnRuntimeService cmmnRuntimeService; protected CmmnTaskService cmmnTaskService; @@ -69,7 +73,9 @@ public abstract class AbstractProcessEngineIntegrationTest { @BeforeClass public static void bootProcessEngine() { if (processEngine == null) { - processEngine = ProcessEngineConfiguration.createProcessEngineConfigurationFromResource("flowable.cfg.xml").buildProcessEngine(); + ProcessEngineConfiguration processEngineConfiguration = ProcessEngineConfiguration.createProcessEngineConfigurationFromResource("flowable.cfg.xml"); + processEngineConfiguration.setBeans(beans); + processEngine = processEngineConfiguration.buildProcessEngine(); cmmnEngineConfiguration = (CmmnEngineConfiguration) processEngine.getProcessEngineConfiguration() .getEngineConfigurations().get(EngineConfigurationConstants.KEY_CMMN_ENGINE_CONFIG); CmmnTestRunner.setCmmnEngineConfiguration(cmmnEngineConfiguration); @@ -91,6 +97,12 @@ public void setupServices() { this.processEngineHistoryService = processEngine.getHistoryService(); this.processEngineConfiguration = processEngine.getProcessEngineConfiguration(); this.processEngineDynamicBpmnService = processEngine.getDynamicBpmnService(); + + beans.put("cmmnRepositoryService", cmmnEngineConfiguration.getCmmnRepositoryService()); + beans.put("cmmnRuntimeService", cmmnEngineConfiguration.getCmmnRuntimeService()); + beans.put("cmmnTaskService", cmmnEngineConfiguration.getCmmnTaskService()); + beans.put("cmmnHistoryService", cmmnEngineConfiguration.getCmmnHistoryService()); + beans.put("cmmnManagementService", cmmnEngineConfiguration.getCmmnManagementService()); } @After @@ -102,6 +114,8 @@ public void cleanup() { for (CmmnDeployment deployment : cmmnRepositoryService.createDeploymentQuery().list()) { cmmnRepositoryService.deleteDeployment(deployment.getId(), true); } + + beans.clear(); } protected Date setCmmnClockFixedToCurrentTime() { diff --git a/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java new file mode 100644 index 00000000000..29885b477ae --- /dev/null +++ b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java @@ -0,0 +1,62 @@ +/* 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; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import org.flowable.cmmn.api.runtime.CaseInstance; +import org.flowable.cmmn.engine.test.CmmnDeployment; +import org.flowable.cmmn.engine.test.impl.CmmnHistoryTestHelper; +import org.flowable.common.engine.impl.history.HistoryLevel; +import org.flowable.engine.test.Deployment; +import org.flowable.variable.api.history.HistoricVariableInstance; +import org.junit.Test; + +/** + * @author Joram Barrez + */ +public class VariablesTest extends AbstractProcessEngineIntegrationTest { + + @Test + @Deployment + @CmmnDeployment + public void testSettingAndRemovingVariableThroughCmmnRuntimeService() { + processEngineRepositoryService.createDeployment() + .addClasspathResource("org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml") + .deploy(); + + CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder() + .caseDefinitionKey("varSyncTestCase") + .variable("loopNum", 100) + .variable("round", 5) + .start(); + + // The case instance ending means all variable lookups succeeded + assertCaseInstanceEnded(caseInstance); + assertThat(cmmnRuntimeService.getVariables(caseInstance.getId())).isEmpty(); + + if (CmmnHistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, cmmnEngineConfiguration)) { + List historicVariables = cmmnHistoryService.createHistoricVariableInstanceQuery() + .caseInstanceId(caseInstance.getId()).list(); + + // The variables are recreated each loop with a non-null value + assertThat(historicVariables).hasSize(102); // 100 from the variables and 2 for round and loopNum + for (HistoricVariableInstance historicVariable : historicVariables) { + assertThat(historicVariable.getValue()).isNotNull(); + } + } + } + +} diff --git a/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml new file mode 100644 index 00000000000..78909e1ab08 --- /dev/null +++ b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml @@ -0,0 +1,230 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0}]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn new file mode 100644 index 00000000000..c5abe5afb98 --- /dev/null +++ b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java index 71e5cb47885..1457bd0e9f4 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java @@ -38,8 +38,12 @@ public Object execute(CommandContext commandContext) { if (caseInstanceId == null) { throw new FlowableIllegalArgumentException("caseInstanceId is null"); } - + CmmnEngineConfiguration cmmnEngineConfiguration = CommandContextUtil.getCmmnEngineConfiguration(commandContext); + + // In the BPMN engine, this is done by getting the variable on the execution. + // However, doing the same in CMMN will fetch the case instance and non-completed plan item instances in one query. + // Hence, why here a direct query is done here (which is cached). VariableInstanceEntity variableInstanceEntity = cmmnEngineConfiguration.getVariableServiceConfiguration().getVariableService() .createInternalVariableInstanceQuery() .scopeId(caseInstanceId) @@ -49,7 +53,7 @@ public Object execute(CommandContext commandContext) { .singleResult(); if (variableInstanceEntity != null) { return variableInstanceEntity.getValue(); - } + } return null; } diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java index 84644bf60d2..84b690dc365 100644 --- a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java @@ -31,6 +31,7 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; +import org.flowable.cmmn.api.CmmnRuntimeService; import org.flowable.cmmn.api.delegate.DelegatePlanItemInstance; import org.flowable.cmmn.api.delegate.PlanItemJavaDelegate; import org.flowable.cmmn.api.history.HistoricMilestoneInstance; @@ -881,6 +882,22 @@ public void testUpdateMetaInfo() { } + @Test + @CmmnDeployment + public void testSettingGettingMultipleTimesInSameTransaction() { + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = true; + CaseInstance caseInstance1 = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("testSettingGettingMultipleTimesInSameTransaction").start(); + assertThat(cmmnRuntimeService.getVariables(caseInstance1.getId())).isEmpty(); + + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = false; + CaseInstance caseInstance2 = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("testSettingGettingMultipleTimesInSameTransaction").start(); + Map variables = cmmnRuntimeService.getVariables(caseInstance2.getId()); + assertThat(variables).hasSize(100); + for (String variableName : variables.keySet()) { + assertThat(variables.get(variableName)).isNotNull(); + } + } + protected void addVariableTypeIfNotExists(VariableType variableType) { // We can't remove the VariableType after every test since it would cause the test // to fail due to not being able to get the variable value during deleting @@ -1062,4 +1079,39 @@ public CustomTestVariable(String someValue, int someInt) { } } + public static class TestSetGetVariablesDelegate implements PlanItemJavaDelegate { + + public static boolean REMOVE_VARS_IN_LAST_ROUND = true; + + @Override + public void execute(DelegatePlanItemInstance planItemInstance) { + String caseInstanceId = planItemInstance.getCaseInstanceId(); + CmmnRuntimeService cmmnRuntimeService = CommandContextUtil.getCmmnRuntimeService(); + + int nrOfLoops = 100; + for (int nrOfRounds = 0; nrOfRounds < 4; nrOfRounds++) { + + // Set + for (int i = 0; i < nrOfLoops; i++) { + cmmnRuntimeService.setVariable(caseInstanceId, "test_" + i, i); + } + + // Get + for (int i = 0; i < nrOfLoops; i++) { + if (cmmnRuntimeService.getVariable(caseInstanceId, "test_" + i) == null) { + throw new RuntimeException("This exception shouldn't happen"); + } + } + + // Remove + if (REMOVE_VARS_IN_LAST_ROUND && nrOfRounds == 3) { + for (int i = 0; i < nrOfLoops; i++) { + cmmnRuntimeService.removeVariable(caseInstanceId, "test_" + i); + } + } + + } + } + } + } diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn new file mode 100644 index 00000000000..686dffd6cdb --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn @@ -0,0 +1,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java b/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java index 2c67c960e39..c39f20af33b 100644 --- a/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java +++ b/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java @@ -98,9 +98,18 @@ public List findInCache(Class entityClass) { } if (classCache != null) { - List entities = new ArrayList<>(classCache.size()); + ArrayList entities = new ArrayList<>(classCache.size()); for (CachedEntity cachedObject : classCache.values()) { - entities.add((T) cachedObject.getEntity()); + + // Non-deleted entities go first in the returned list, + // while deleted ones go at the end. + // This way users of this method will first get the 'active' entities. + if (!cachedObject.getEntity().isDeleted()) { + entities.add(0, (T) cachedObject.getEntity()); + } else { + entities.add((T) cachedObject.getEntity()); + } + } return entities; } diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java index 0149da7cb99..ac82123f767 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java @@ -29,6 +29,7 @@ import org.assertj.core.groups.Tuple; import org.flowable.common.engine.impl.history.HistoryLevel; +import org.flowable.engine.RuntimeService; import org.flowable.engine.delegate.DelegateExecution; import org.flowable.engine.delegate.JavaDelegate; import org.flowable.engine.impl.test.HistoryTestHelper; @@ -733,6 +734,28 @@ public void testCreateAndUpdateWithValue() { } } + @Test + @Deployment + public void testSettingGettingMultipleTimesInSameTransaction() { + + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = true; + + ProcessInstance processInstance1 = runtimeService.createProcessInstanceBuilder() + .processDefinitionKey("testSettingGettingMultipleTimesInSameTransaction") + .start(); + assertThat(runtimeService.getVariables(processInstance1.getId())).isEmpty(); + + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = false; + ProcessInstance processInstance2 = runtimeService.createProcessInstanceBuilder() + .processDefinitionKey("testSettingGettingMultipleTimesInSameTransaction") + .start(); + Map variables = runtimeService.getVariables(processInstance2.getId()); + assertThat(variables).hasSize(100); + for (String variableName : variables.keySet()) { + assertThat(variables.get(variableName)).isNotNull(); + } + } + // Class to test variable serialization public static class TestSerializableVariable implements Serializable { @@ -906,4 +929,39 @@ public void execute(DelegateExecution execution) { } } + public static class TestSetGetVariablesDelegate implements JavaDelegate { + + public static boolean REMOVE_VARS_IN_LAST_ROUND = true; + + @Override + public void execute(DelegateExecution execution) { + String processInstanceId = execution.getProcessInstanceId(); + RuntimeService runtimeService = CommandContextUtil.getProcessEngineConfiguration().getRuntimeService(); + + int nrOfLoops = 100; + for (int nrOfRounds = 0; nrOfRounds < 4; nrOfRounds++) { + + // Set + for (int i = 0; i < nrOfLoops; i++) { + runtimeService.setVariable(processInstanceId, "test_" + i, i); + } + + // Get + for (int i = 0; i < nrOfLoops; i++) { + if (runtimeService.getVariable(processInstanceId, "test_" + i) == null) { + throw new RuntimeException("This exception shouldn't happen"); + } + } + + // Remove + if (REMOVE_VARS_IN_LAST_ROUND && nrOfRounds == 3) { + for (int i = 0; i < nrOfLoops; i++) { + runtimeService.removeVariable(processInstanceId, "test_" + i); + } + } + + } + } + } + } diff --git a/modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml b/modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml new file mode 100644 index 00000000000..68cc58ed050 --- /dev/null +++ b/modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml @@ -0,0 +1,77 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file