diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/async/AsyncCmmnHistoryTest.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/async/AsyncCmmnHistoryTest.java index ec1c1a05610..c7ac0230333 100644 --- a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/async/AsyncCmmnHistoryTest.java +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/async/AsyncCmmnHistoryTest.java @@ -67,6 +67,7 @@ import org.flowable.task.api.history.HistoricTaskLogEntryBuilder; import org.flowable.task.api.history.HistoricTaskLogEntryType; import org.flowable.variable.api.history.HistoricVariableInstance; +import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.junit.Assert; import org.junit.Test; @@ -1579,7 +1580,12 @@ public void testVariableChanges() { VariableInstanceEntity variableInstanceEntity = variablesInstances.get(0); variableInstanceEntity.setMetaInfo("test meta info"); - cmmnEngineConfiguration.getVariableServiceConfiguration().getVariableInstanceEntityManager().updateWithHistoricVariableSync(variableInstanceEntity); + VariableServiceConfiguration variableServiceConfiguration = cmmnEngineConfiguration.getVariableServiceConfiguration(); + variableServiceConfiguration.getVariableInstanceEntityManager().update(variableInstanceEntity); + if (variableServiceConfiguration.getInternalHistoryVariableManager() != null) { + variableServiceConfiguration.getInternalHistoryVariableManager() + .recordVariableUpdate(variableInstanceEntity, commandContext.getClock().getCurrentTime()); + } return null; }); diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariableInstanceValueModifierCmmnTest.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariableInstanceValueModifierCmmnTest.java index 6ca6eb3e8a4..8aa170522fc 100644 --- a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariableInstanceValueModifierCmmnTest.java +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariableInstanceValueModifierCmmnTest.java @@ -23,6 +23,7 @@ import org.flowable.common.engine.api.FlowableIllegalArgumentException; import org.flowable.task.api.Task; import org.flowable.variable.api.persistence.entity.VariableInstance; +import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.DefaultVariableInstanceValueModifier; import org.flowable.variable.service.impl.VariableInstanceValueModifier; import org.junit.After; @@ -53,19 +54,8 @@ public void tearDown() { @Test @CmmnDeployment(resources = { "org/flowable/cmmn/test/one-human-task-model.cmmn" }) public void testCompleteTaskWithExceptionInPostSetVariable() { - DefaultVariableInstanceValueModifier modifier = new DefaultVariableInstanceValueModifier(cmmnEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - if (variableInstance.getName().equals("orderId")) { - if (((Number) value).longValue() < 0) { - throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); - } - } - super.setOrUpdateValue(variableInstance, value, tenantId); - } - }; - cmmnEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(modifier); + cmmnEngineConfiguration.getVariableServiceConfiguration() + .setVariableInstanceValueModifier(new TestOrderIdValidatingValueModifier(cmmnEngineConfiguration.getVariableServiceConfiguration())); Map variables = new HashMap<>(); variables.put("orderId", 1L); @@ -87,18 +77,8 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, @Test @CmmnDeployment(resources = { "org/flowable/cmmn/test/one-human-task-model.cmmn" }) public void testTransientVariables() { - DefaultVariableInstanceValueModifier modifier = new DefaultVariableInstanceValueModifier(cmmnEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - if (variableInstance.getName().equals("orderId")) { - if (((Number) value).longValue() < 0) { - throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); - } - } - } - }; - cmmnEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(modifier); + cmmnEngineConfiguration.getVariableServiceConfiguration() + .setVariableInstanceValueModifier(new TestOrderIdValidatingValueModifier(cmmnEngineConfiguration.getVariableServiceConfiguration())); Map variables = new HashMap<>(); variables.put("orderId", -1L); @@ -108,4 +88,31 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, cmmnRuntimeService.createCaseInstanceBuilder().transientVariables(variables).caseDefinitionKey("oneTaskCase").start(); }).isInstanceOf(FlowableIllegalArgumentException.class).hasMessage("Invalid type: value should be larger than zero"); } + + static class TestOrderIdValidatingValueModifier extends DefaultVariableInstanceValueModifier { + + public TestOrderIdValidatingValueModifier(VariableServiceConfiguration serviceConfiguration) { + super(serviceConfiguration); + } + + @Override + public void setVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + validateValue(variableInstance, value); + super.setVariableValue(variableInstance, value, tenantId); + } + + @Override + public void updateVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + validateValue(variableInstance, value); + super.updateVariableValue(variableInstance, value, tenantId); + } + + protected void validateValue(VariableInstance variableInstance, Object value) { + if (variableInstance.getName().equals("orderId")) { + if (((Number) value).longValue() < 0) { + throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); + } + } + } + } } 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 9bc9a0183e7..310f5464656 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 @@ -50,6 +50,7 @@ import org.flowable.variable.api.persistence.entity.VariableInstance; import org.flowable.variable.api.types.ValueFields; import org.flowable.variable.api.types.VariableType; +import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.junit.Rule; import org.junit.Test; @@ -811,7 +812,12 @@ public void testUpdateMetaInfo() { VariableInstanceEntity variableInstanceEntity = variablesInstances.get(0); variableInstanceEntity.setMetaInfo("test meta info"); - cmmnEngineConfiguration.getVariableServiceConfiguration().getVariableInstanceEntityManager().updateWithHistoricVariableSync(variableInstanceEntity); + VariableServiceConfiguration variableServiceConfiguration = cmmnEngineConfiguration.getVariableServiceConfiguration(); + variableServiceConfiguration.getVariableInstanceEntityManager().update(variableInstanceEntity); + if (variableServiceConfiguration.getInternalHistoryVariableManager() != null) { + variableServiceConfiguration.getInternalHistoryVariableManager() + .recordVariableUpdate(variableInstanceEntity, commandContext.getClock().getCurrentTime()); + } return null; }); diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariableInstanceValueModifierBpmnTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariableInstanceValueModifierBpmnTest.java index 575689875d1..92c543f626a 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariableInstanceValueModifierBpmnTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariableInstanceValueModifierBpmnTest.java @@ -14,31 +14,38 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.entry; import static org.assertj.core.groups.Tuple.tuple; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.flowable.common.engine.api.FlowableIllegalArgumentException; +import org.flowable.common.engine.impl.history.HistoryLevel; +import org.flowable.engine.history.HistoricProcessInstance; +import org.flowable.engine.impl.test.HistoryTestHelper; import org.flowable.engine.impl.test.PluggableFlowableTestCase; import org.flowable.engine.runtime.ProcessInstance; import org.flowable.engine.test.Deployment; import org.flowable.engine.test.api.event.TestVariableEventListener; import org.flowable.task.api.Task; import org.flowable.variable.api.event.FlowableVariableEvent; +import org.flowable.variable.api.history.HistoricVariableInstance; import org.flowable.variable.api.persistence.entity.VariableInstance; import org.flowable.variable.api.types.ValueFields; import org.flowable.variable.api.types.VariableType; import org.flowable.variable.api.types.VariableTypes; +import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.DefaultVariableInstanceValueModifier; import org.flowable.variable.service.impl.VariableInstanceValueModifier; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.flowable.variable.service.impl.types.IntegerType; -import org.flowable.variable.service.impl.types.LongType; import org.flowable.variable.service.impl.types.StringType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -81,18 +88,8 @@ public void tearDown() { @Test @Deployment(resources = { "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml" }) public void testCompleteTaskWithExceptionInPostSetVariable() { - DefaultVariableInstanceValueModifier variableValueModifier = new DefaultVariableInstanceValueModifier(processEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - public void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - if (variableInstance.getName().equals("orderId")) { - if (((Number) value).longValue() < 0) { - throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); - } - } - } - }; - processEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(variableValueModifier); + processEngineConfiguration.getVariableServiceConfiguration() + .setVariableInstanceValueModifier(new TestOrderIdValidatingValueModifier(processEngineConfiguration.getVariableServiceConfiguration())); Map variables = new HashMap<>(); variables.put("orderId", 1L); @@ -120,20 +117,20 @@ public void testChangeVariableType() { processEngineConfiguration.getVariableServiceConfiguration()) { @Override - public VariableType determineVariableType(VariableTypes typeRegistry, VariableInstance variableInstance, Object value, String tenantId) { + public void setVariableValue(VariableInstance variableInstance, Object value, String tenantId) { if (isIntegralNumber(value)) { - // We always use 'long' as the type for integral numbers - return typeRegistry.getVariableType(LongType.TYPE_NAME); + super.setVariableValue(variableInstance, Long.parseLong(value.toString()), tenantId); + } else { + super.setVariableValue(variableInstance, value, tenantId); } - return super.determineVariableType(typeRegistry, variableInstance, value, tenantId); } @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { + public void updateVariableValue(VariableInstance variableInstance, Object value, String tenantId) { if (isIntegralNumber(value)) { - variableInstance.setValue(((Number) value).longValue()); + super.updateVariableValue(variableInstance, Long.parseLong(value.toString()), tenantId); } else { - super.setOrUpdateValue(variableInstance, value, tenantId); + super.updateVariableValue(variableInstance, value, tenantId); } } @@ -153,6 +150,16 @@ boolean isIntegralNumber(Object value) { assertThat(orderIdInstance.getTypeName()).isEqualTo("string"); assertThat(orderIdInstance.getValue()).isEqualTo("ABC"); + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricProcessInstance historicProcessInstance = historyService.createHistoricProcessInstanceQuery() + .processInstanceId(processInstance.getId()) + .includeProcessVariables() + .singleResult(); + assertThat(historicProcessInstance).isNotNull(); + assertThat(historicProcessInstance.getProcessVariables()) + .contains(entry("orderId", "ABC")); + } + Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); // now we change it to be an integral number variables.put("orderId", 1); @@ -168,6 +175,16 @@ boolean isIntegralNumber(Object value) { .extracting(FlowableVariableEvent::getVariableName, FlowableVariableEvent::getVariableValue) .containsExactly(tuple("orderId", "ABC"), tuple("orderId", 1L)); + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricProcessInstance historicProcessInstance = historyService.createHistoricProcessInstanceQuery() + .processInstanceId(processInstance.getId()) + .includeProcessVariables() + .singleResult(); + assertThat(historicProcessInstance).isNotNull(); + assertThat(historicProcessInstance.getProcessVariables()) + .contains(entry("orderId", 1L)); + } + } @Test @@ -177,27 +194,8 @@ public void testWrapVariableWithCustomType() { customType = new WrappedIntegerCustomType(); variableTypes.addTypeBefore(customType, "integer"); try { - DefaultVariableInstanceValueModifier variableValueModifier = new DefaultVariableInstanceValueModifier(processEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - public VariableType determineVariableType(VariableTypes typeRegistry, VariableInstance variableInstance, Object value, String tenantId) { - if ((value instanceof Integer) && ((Integer) value) > 1000) { - return typeRegistry.getVariableType(WrappedIntegerCustomType.TYPE_NAME); - } - return super.determineVariableType(typeRegistry, variableInstance, value, tenantId); - } - - @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - if (variableInstance.getTypeName().equals(WrappedIntegerCustomType.TYPE_NAME)) { - variableInstance.setMetaInfo(value + "meta"); - variableInstance.setValue(new WrappedIntegerValue((Integer) value, variableInstance.getMetaInfo())); - } else { - super.setOrUpdateValue(variableInstance, value, tenantId); - } - } - }; - processEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(variableValueModifier); + processEngineConfiguration.getVariableServiceConfiguration() + .setVariableInstanceValueModifier(new WrappedIntegerValueModifier(processEngineConfiguration.getVariableServiceConfiguration())); Map variables = new HashMap<>(); variables.put("simpleInteger", 1000); @@ -221,6 +219,21 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, assertThat(wrappedIntegerValue.metaInfo).isEqualTo("1001meta"); }); + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricVariableInstance historicVariableInstance = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()) + .variableName("wrappedInteger") + .singleResult(); + assertThat(historicVariableInstance).isNotNull(); + assertThat(historicVariableInstance.getValue()) + .isInstanceOfSatisfying(WrappedIntegerValue.class, wrappedIntegerValue -> { + assertThat(wrappedIntegerValue.value).isEqualTo(1001); + assertThat(wrappedIntegerValue.metaInfo).isEqualTo("1001meta"); + }); + assertThat(historicVariableInstance.getVariableTypeName()).isEqualTo("wrappedIntegerType"); + assertThat(historicVariableInstance.getMetaInfo()).isEqualTo("1001meta"); + } + Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); variables.put("wrappedInteger", 1002); taskService.complete(task.getId(), variables); @@ -244,6 +257,21 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, tuple("wrappedInteger", "wrappedIntegerType", wrappedInteger.getValue(), "VARIABLE_CREATED"), tuple("simpleInteger", "integer", 1000, "VARIABLE_UPDATED"), // task completion triggers 'update' of this variable tuple("wrappedInteger", "wrappedIntegerType", wrappedIntegerTaskUpdate.getValue(), "VARIABLE_UPDATED")); + + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricVariableInstance historicVariableInstance = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()) + .variableName("wrappedInteger") + .singleResult(); + assertThat(historicVariableInstance).isNotNull(); + assertThat(historicVariableInstance.getValue()) + .isInstanceOfSatisfying(WrappedIntegerValue.class, wrappedIntegerValue -> { + assertThat(wrappedIntegerValue.value).isEqualTo(1002); + assertThat(wrappedIntegerValue.metaInfo).isEqualTo("1002meta"); + }); + assertThat(historicVariableInstance.getVariableTypeName()).isEqualTo("wrappedIntegerType"); + assertThat(historicVariableInstance.getMetaInfo()).isEqualTo("1002meta"); + } } finally { // We have to delete here. Otherwise the annotation deployment delete operation will fail, due to the missing custom type deleteDeployments(); @@ -257,27 +285,8 @@ public void testWrapVariableWithCustomTypeChangeToSimpleType() { customType = new WrappedIntegerCustomType(); try { variableTypes.addTypeBefore(customType, "integer"); - DefaultVariableInstanceValueModifier variableValueModifier = new DefaultVariableInstanceValueModifier(processEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - public VariableType determineVariableType(VariableTypes typeRegistry, VariableInstance variableInstance, Object value, String tenantId) { - if ((value instanceof Integer) && ((Integer) value) > 1000) { - return typeRegistry.getVariableType(WrappedIntegerCustomType.TYPE_NAME); - } - return super.determineVariableType(typeRegistry, variableInstance, value, tenantId); - } - - @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - if (variableInstance.getTypeName().equals(WrappedIntegerCustomType.TYPE_NAME)) { - variableInstance.setMetaInfo(value + "meta"); - variableInstance.setValue(new WrappedIntegerValue((Integer) value, variableInstance.getMetaInfo())); - } else { - super.setOrUpdateValue(variableInstance, value, tenantId); - } - } - }; - processEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(variableValueModifier); + processEngineConfiguration.getVariableServiceConfiguration() + .setVariableInstanceValueModifier(new WrappedIntegerValueModifier(processEngineConfiguration.getVariableServiceConfiguration())); Map variables = new HashMap<>(); variables.put("wrappedInteger", 1001); @@ -307,6 +316,21 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, assertThat(wrappedIntegerValue.metaInfo).isEqualTo("1001meta"); }); + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricVariableInstance historicVariableInstance = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()) + .variableName("wrappedInteger") + .singleResult(); + assertThat(historicVariableInstance).isNotNull(); + assertThat(historicVariableInstance.getValue()) + .isInstanceOfSatisfying(WrappedIntegerValue.class, wrappedIntegerValue -> { + assertThat(wrappedIntegerValue.value).isEqualTo(1001); + assertThat(wrappedIntegerValue.metaInfo).isEqualTo("1001meta"); + }); + assertThat(historicVariableInstance.getVariableTypeName()).isEqualTo("wrappedIntegerType"); + assertThat(historicVariableInstance.getMetaInfo()).isEqualTo("1001meta"); + } + // Change back to simple integer type by setting value to 1000 Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); variables.put("wrappedInteger", 1000); @@ -315,6 +339,7 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, VariableInstanceEntity wrappedIntegerTaskUpdate = (VariableInstanceEntity) runtimeService.getVariableInstance(processInstance.getId(), "wrappedInteger"); assertThat(wrappedIntegerTaskUpdate.getValue()).isInstanceOf(Integer.class).isEqualTo(1000); + assertThat(wrappedIntegerTaskUpdate.getMetaInfo()).isNull(); assertThat(wrappedIntegerTaskUpdate.getType()).isInstanceOf(IntegerType.class); assertThat(wrappedIntegerTaskUpdate.getTypeName()).isEqualTo(IntegerType.TYPE_NAME); @@ -329,6 +354,17 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, tuple("wrappedInteger", "wrappedIntegerType", wrappedInteger.getValue(), "VARIABLE_CREATED"), tuple("wrappedInteger", "integer", 1000, "VARIABLE_UPDATED")); + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricVariableInstance historicVariableInstance = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()) + .variableName("wrappedInteger") + .singleResult(); + assertThat(historicVariableInstance).isNotNull(); + assertThat(historicVariableInstance.getValue()).isEqualTo(1000); + assertThat(historicVariableInstance.getVariableTypeName()).isEqualTo(IntegerType.TYPE_NAME); + assertThat(historicVariableInstance.getMetaInfo()).isNull(); + } + } finally { // We have to delete here. Otherwise the annotation deployment delete operation will fail, due to the missing custom type deleteDeployments(); @@ -338,52 +374,31 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, @Test @Deployment(resources = { "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml" }, tenantId = "myTenant") public void testCompleteTaskWithExceptionInPostSetVariableWithTenantId() { - List setOrUpdateValueTenantIds = new LinkedList<>(); - DefaultVariableInstanceValueModifier enhancer = new DefaultVariableInstanceValueModifier(processEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - setOrUpdateValueTenantIds.add(tenantId); - if (variableInstance.getName().equals("orderId") && ((Long) value) < 0) { - throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); - } - super.setOrUpdateValue(variableInstance, value, tenantId); - } - - }; - processEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(enhancer); + TestOrderIdValidatingValueModifier valueModifier = new TestOrderIdValidatingValueModifier(processEngineConfiguration.getVariableServiceConfiguration()); + processEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(valueModifier); Map variables = new HashMap<>(); variables.put("orderId", 1L); ProcessInstance processInstance = runtimeService .startProcessInstanceByKeyAndTenantId("oneTaskProcess", variables, "myTenant"); + assertThat(valueModifier.setVariableTenantIds).containsExactly("myTenant"); + assertThatThrownBy(() -> { Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult(); variables.put("orderId", -1L); taskService.complete(task.getId(), variables); }).isInstanceOf(FlowableIllegalArgumentException.class).hasMessage("Invalid type: value should be larger than zero"); - assertThat(setOrUpdateValueTenantIds).containsExactly("myTenant", "myTenant"); + assertThat(valueModifier.setVariableTenantIds).containsExactly("myTenant"); + assertThat(valueModifier.updateVariableTenantIds).containsExactly("myTenant"); } @Test @Deployment(resources = { "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml" }) public void testTransientVariables() { - List setOrUpdateValueTenantIds = new LinkedList<>(); - DefaultVariableInstanceValueModifier enhancer = new DefaultVariableInstanceValueModifier(processEngineConfiguration.getVariableServiceConfiguration()) { - - @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - setOrUpdateValueTenantIds.add(tenantId); - if (variableInstance.getName().equals("orderId") && ((Long) value) < 0) { - throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); - } - super.setOrUpdateValue(variableInstance, value, tenantId); - } - - }; - processEngineConfiguration.getVariableServiceConfiguration().setVariableInstanceValueModifier(enhancer); + processEngineConfiguration.getVariableServiceConfiguration() + .setVariableInstanceValueModifier(new TestOrderIdValidatingValueModifier(processEngineConfiguration.getVariableServiceConfiguration())); Map variables = new HashMap<>(); variables.put("orderId", -1L); @@ -399,30 +414,43 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, void testEnhanceVariableWithMetaInfo() { ObjectMapper objectMapper = processEngineConfiguration.getObjectMapper(); - List preSetValueCalls = new LinkedList<>(); + List preSetValueCalls = new ArrayList<>(); DefaultVariableInstanceValueModifier modifier = new DefaultVariableInstanceValueModifier(processEngineConfiguration.getVariableServiceConfiguration()) { @Override - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - if (variableInstance instanceof VariableInstanceEntity) { - preSetValueCalls.add(value); - assertThat(variableInstance.getProcessInstanceId()).isNotNull(); - assertThat(variableInstance.getProcessDefinitionId()).isNotNull(); - if (value instanceof String) { - VariableMeta variableMeta = new VariableMeta(); - variableMeta.byteLength = String.valueOf(((String) value).getBytes().length); - try { - variableInstance.setMetaInfo(objectMapper.writeValueAsString(variableMeta)); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - variableInstance.setValue(value + "Enhanced"); - } else { - variableInstance.setValue(value); + public void setVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + preSetValueCalls.add(value); + assertThat(variableInstance.getProcessInstanceId()).isNotNull(); + assertThat(variableInstance.getProcessDefinitionId()).isNotNull(); + Pair valueAndMeta = determineValueAndMetaInfo(value); + super.setVariableValue(variableInstance, valueAndMeta.getLeft(), tenantId); + variableInstance.setMetaInfo(valueAndMeta.getRight()); + } + + @Override + public void updateVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + preSetValueCalls.add(value); + assertThat(variableInstance.getProcessInstanceId()).isNotNull(); + assertThat(variableInstance.getProcessDefinitionId()).isNotNull(); + Pair valueAndMeta = determineValueAndMetaInfo(value); + super.updateVariableValue(variableInstance, valueAndMeta.getLeft(), tenantId); + variableInstance.setMetaInfo(valueAndMeta.getRight()); + } + + Pair determineValueAndMetaInfo(Object value) { + if (value instanceof String) { + VariableMeta variableMeta = new VariableMeta(); + variableMeta.byteLength = String.valueOf(((String) value).getBytes().length); + String metaInfo; + try { + metaInfo = objectMapper.writeValueAsString(variableMeta); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); } - } else { - variableInstance.setValue(value); + return Pair.of(value + "Enhanced", metaInfo); } + + return Pair.of(value, null); } }; @@ -458,6 +486,25 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value, assertThat(intVariableInstance.getMetaInfo()).isNull(); assertThat(preSetValueCalls).containsExactlyInAnyOrder("myValue1", 1); + + if (HistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processEngineConfiguration)) { + HistoricVariableInstance historicVariableInstance = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()) + .variableName("myEnhancedStringVariable") + .singleResult(); + assertThat(historicVariableInstance).isNotNull(); + assertThat(historicVariableInstance.getValue()).isEqualTo("myValue1Enhanced"); + assertThat(historicVariableInstance.getVariableTypeName()).isEqualTo("string"); + assertThat(historicVariableInstance.getMetaInfo()).isEqualTo("{\"byteLength\":\"8\"}"); + + historicVariableInstance = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()) + .variableName("myIntVariable") + .singleResult(); + assertThat(historicVariableInstance).isNotNull(); + assertThat(historicVariableInstance.getValue()).isEqualTo(1); + assertThat(historicVariableInstance.getMetaInfo()).isNull(); + } } static class VariableMeta { @@ -489,6 +536,8 @@ public void setValue(Object value, ValueFields valueFields) { public Object getValue(ValueFields valueFields) { if (valueFields instanceof VariableInstance) { return new WrappedIntegerValue(valueFields.getLongValue(), ((VariableInstance) valueFields).getMetaInfo()); + } else if (valueFields instanceof HistoricVariableInstance) { + return new WrappedIntegerValue(valueFields.getLongValue(), ((HistoricVariableInstance) valueFields).getMetaInfo()); } return super.getValue(valueFields); } @@ -530,4 +579,66 @@ public int hashCode() { } } + static class TestOrderIdValidatingValueModifier extends DefaultVariableInstanceValueModifier { + + final Collection setVariableTenantIds = new ArrayList<>(); + final Collection updateVariableTenantIds = new ArrayList<>(); + + TestOrderIdValidatingValueModifier(VariableServiceConfiguration serviceConfiguration) { + super(serviceConfiguration); + } + + @Override + public void setVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + setVariableTenantIds.add(tenantId); + validateValue(variableInstance, value); + super.setVariableValue(variableInstance, value, tenantId); + } + + @Override + public void updateVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + updateVariableTenantIds.add(tenantId); + validateValue(variableInstance, value); + super.updateVariableValue(variableInstance, value, tenantId); + } + + protected void validateValue(VariableInstance variableInstance, Object value) { + if (variableInstance.getName().equals("orderId")) { + if (((Number) value).longValue() < 0) { + throw new FlowableIllegalArgumentException("Invalid type: value should be larger than zero"); + } + } + } + } + + static class WrappedIntegerValueModifier extends DefaultVariableInstanceValueModifier { + + WrappedIntegerValueModifier(VariableServiceConfiguration serviceConfiguration) { + super(serviceConfiguration); + } + + + @Override + public void setVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + Pair valueAndMeta = determineValueAndMeta(value); + super.setVariableValue(variableInstance, valueAndMeta.getLeft(), tenantId); + variableInstance.setMetaInfo(valueAndMeta.getRight()); + } + + @Override + public void updateVariableValue(VariableInstance variableInstance, Object value, String tenantId) { + Pair valueAndMeta = determineValueAndMeta(value); + super.updateVariableValue(variableInstance, valueAndMeta.getLeft(), tenantId); + variableInstance.setMetaInfo(valueAndMeta.getRight()); + } + + Pair determineValueAndMeta(Object value) { + if (value instanceof Integer && ((Integer) value) > 1000) { + String metaInfo = value + "meta"; + return Pair.of(new WrappedIntegerValue((Number) value, metaInfo), metaInfo); + } + return Pair.of(value, null); + } + } + } 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 5c88b9c3ad7..7a2c27988b8 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 @@ -40,6 +40,7 @@ import org.flowable.variable.api.history.HistoricVariableInstance; import org.flowable.variable.api.persistence.entity.VariableInstance; import org.flowable.variable.api.types.ValueFields; +import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.persistence.entity.HistoricVariableInstanceEntity; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.joda.time.DateTime; @@ -636,7 +637,12 @@ public void testUpdateMetaInfo() { VariableInstanceEntity variableInstanceEntity = variablesInstances.get(0); variableInstanceEntity.setMetaInfo("test meta info"); - processEngineConfiguration.getVariableServiceConfiguration().getVariableInstanceEntityManager().updateWithHistoricVariableSync(variableInstanceEntity); + VariableServiceConfiguration variableServiceConfiguration = processEngineConfiguration.getVariableServiceConfiguration(); + variableServiceConfiguration.getVariableInstanceEntityManager().update(variableInstanceEntity); + if (variableServiceConfiguration.getInternalHistoryVariableManager() != null) { + variableServiceConfiguration.getInternalHistoryVariableManager() + .recordVariableUpdate(variableInstanceEntity, commandContext.getClock().getCurrentTime()); + } return null; }); @@ -671,7 +677,14 @@ public void testCreateAndUpdateWithValue() { .containsExactly(Tuple.tuple("myVariable", "myStringValue", "string")); VariableInstanceEntity variableInstanceEntity = variablesInstances.get(0); - CommandContextUtil.getVariableService(commandContext).updateVariableInstanceWithValue(variableInstanceEntity, 42, "myTenantId"); + VariableServiceConfiguration variableServiceConfiguration = CommandContextUtil.getProcessEngineConfiguration(commandContext) + .getVariableServiceConfiguration(); + variableServiceConfiguration.getVariableInstanceValueModifier().updateVariableValue(variableInstanceEntity, 42, "myTenantId"); + variableServiceConfiguration.getVariableInstanceEntityManager().update(variableInstanceEntity); + if (variableServiceConfiguration.getInternalHistoryVariableManager() != null) { + variableServiceConfiguration.getInternalHistoryVariableManager() + .recordVariableUpdate(variableInstanceEntity, commandContext.getClock().getCurrentTime()); + } return null; }); diff --git a/modules/flowable-engine/src/test/java/org/flowable/standalone/history/async/AsyncHistoryTest.java b/modules/flowable-engine/src/test/java/org/flowable/standalone/history/async/AsyncHistoryTest.java index 88e4a40ae9b..0a6db230661 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/standalone/history/async/AsyncHistoryTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/standalone/history/async/AsyncHistoryTest.java @@ -56,6 +56,7 @@ import org.flowable.task.api.history.HistoricTaskLogEntry; import org.flowable.task.api.history.HistoricTaskLogEntryBuilder; import org.flowable.task.api.history.HistoricTaskLogEntryType; +import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.junit.Assert; import org.junit.jupiter.api.AfterEach; @@ -1030,7 +1031,12 @@ public void testVariableChanges() { VariableInstanceEntity variableInstanceEntity = variablesInstances.get(0); variableInstanceEntity.setMetaInfo("test meta info"); - processEngineConfiguration.getVariableServiceConfiguration().getVariableInstanceEntityManager().updateWithHistoricVariableSync(variableInstanceEntity); + VariableServiceConfiguration variableServiceConfiguration = processEngineConfiguration.getVariableServiceConfiguration(); + variableServiceConfiguration.getVariableInstanceEntityManager().update(variableInstanceEntity); + if (variableServiceConfiguration.getInternalHistoryVariableManager() != null) { + variableServiceConfiguration.getInternalHistoryVariableManager() + .recordVariableUpdate(variableInstanceEntity, commandContext.getClock().getCurrentTime()); + } return null; }); diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/VariableService.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/VariableService.java index bd80f6ec514..4887ab781c2 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/VariableService.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/VariableService.java @@ -14,7 +14,6 @@ import java.util.List; -import org.flowable.variable.service.impl.VariableInstanceValueModifier; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; /** @@ -57,15 +56,6 @@ default List findVariableInstanceBySubScopeIdAndScopeTyp */ void insertVariableInstanceWithValue(VariableInstanceEntity variable, Object value, String tenantId); - /** - * Updates variable instance with the new value. Makes sure the {@link VariableInstanceValueModifier} is called. - * - * @param variable variable to update - * @param newValue new value to set - * @param tenantId the tenant id of the variable instance - */ - void updateVariableInstanceWithValue(VariableInstanceEntity variable, Object newValue, String tenantId); - void deleteVariableInstance(VariableInstanceEntity variable); void deleteVariablesByExecutionId(String executionId); diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/DefaultVariableInstanceValueModifier.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/DefaultVariableInstanceValueModifier.java index 017087c7864..d94ed56e4a7 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/DefaultVariableInstanceValueModifier.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/DefaultVariableInstanceValueModifier.java @@ -14,7 +14,6 @@ import org.flowable.variable.api.persistence.entity.VariableInstance; import org.flowable.variable.api.types.VariableType; -import org.flowable.variable.api.types.VariableTypes; import org.flowable.variable.service.VariableServiceConfiguration; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; @@ -35,10 +34,10 @@ public DefaultVariableInstanceValueModifier(VariableServiceConfiguration service public void setVariableValue(VariableInstance variableInstance, Object value, String tenantId) { if (variableInstance instanceof VariableInstanceEntity) { VariableInstanceEntity variableInstanceEntity = (VariableInstanceEntity) variableInstance; - VariableType variableType = determineVariableType(serviceConfiguration.getVariableTypes(), variableInstance, value, tenantId); + VariableType variableType = determineVariableType(value); setVariableType(variableInstanceEntity, variableType); } - setOrUpdateValue(variableInstance, value, tenantId); + variableInstance.setValue(value); } @Override @@ -48,19 +47,23 @@ public void updateVariableValue(VariableInstance variableInstance, Object value, */ if (variableInstance instanceof VariableInstanceEntity) { VariableInstanceEntity variableInstanceEntity = (VariableInstanceEntity) variableInstance; - VariableType variableType = determineVariableType(serviceConfiguration.getVariableTypes(), variableInstance, value, tenantId); + VariableType variableType = determineVariableType(value); if (!variableType.equals(variableInstanceEntity.getType())) { - // variable type has changed - variableInstance.setValue(null); - setVariableType(variableInstanceEntity, variableType); - variableInstanceEntity.forceUpdate(); + updateVariableType(variableInstanceEntity, variableType); } } - setOrUpdateValue(variableInstance, value, tenantId); + variableInstance.setValue(value); } - protected VariableType determineVariableType(VariableTypes variableTypes, VariableInstance variableInstance, Object value, String tenantId) { - return variableTypes.findVariableType(value); + protected void updateVariableType(VariableInstanceEntity variableInstance, VariableType variableType) { + // variable type has changed + variableInstance.setValue(null); + setVariableType(variableInstance, variableType); + variableInstance.forceUpdate(); + } + + protected VariableType determineVariableType(Object value) { + return serviceConfiguration.getVariableTypes().findVariableType(value); } /** @@ -73,15 +76,4 @@ protected void setVariableType(VariableInstanceEntity variableInstance, Variable variableInstance.setType(type); } - /** - * Central hook method for all modifications of a value for the variable instance. - * Please note, that transient variable instances are passed here as well. - * - * @param variableInstance the variable instance to be modified - * @param value the value to be set for the variable instance - * @param tenantId the ID of the tenant the variable instance belongs to - */ - protected void setOrUpdateValue(VariableInstance variableInstance, Object value, String tenantId) { - variableInstance.setValue(value); - } } diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceValueModifier.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceValueModifier.java index 35d62e5b85b..1d941e9e2f0 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceValueModifier.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceValueModifier.java @@ -24,8 +24,9 @@ public interface VariableInstanceValueModifier { /** - * Sets the value of a persistent or transient variable instance. - + * Sets the value of a new persistent or transient variable instance. + * This is invoked when a variable instance is created. + * * @param variableInstance the variable instance to be modified * @param value the new value to be set for the variable instance. * @param tenantId the ID of the tenant the variable instance belongs to @@ -34,6 +35,7 @@ public interface VariableInstanceValueModifier { /** * Updates the value of a variable instance. + * This is invoked when a variable instance already exists and its value is being updated. * * @param variableInstance the variable instance to be modified * @param value the value to be set for the updated variable instance. diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableServiceImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableServiceImpl.java index 56dd9abd76d..f3d050c3c5a 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableServiceImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableServiceImpl.java @@ -49,11 +49,6 @@ public void insertVariableInstanceWithValue(VariableInstanceEntity variable, Obj getVariableInstanceEntityManager().insertWithValue(variable, value, tenantId); } - @Override - public void updateVariableInstanceWithValue(VariableInstanceEntity variable, Object newValue, String tenantId) { - getVariableInstanceEntityManager().updateWithHistoricVariableSync(tenantId, variable, newValue); - } - @Override public void deleteVariableInstance(VariableInstanceEntity variable) { getVariableInstanceEntityManager().delete(variable); diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManager.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManager.java index 5ae099ceb67..6f4f0df925f 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManager.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManager.java @@ -42,18 +42,6 @@ public interface VariableInstanceEntityManager extends EntityManager findVariableInstancesByQueryCriteria(VariableInstanceQueryImpl variableInstanceQuery); diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManagerImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManagerImpl.java index d7f27aaefa4..5a37f979513 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManagerImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/persistence/entity/VariableInstanceEntityManagerImpl.java @@ -52,19 +52,6 @@ public void insertWithValue(VariableInstanceEntity variable, Object value, Strin insert(variable); } - @Override - public void updateWithHistoricVariableSync(VariableInstanceEntity variableInstanceEntity) { - update(variableInstanceEntity, true); - serviceConfiguration.getInternalHistoryVariableManager().recordVariableUpdate(variableInstanceEntity, serviceConfiguration.getClock().getCurrentTime()); - } - - @Override - public void updateWithHistoricVariableSync(String tenantId, VariableInstanceEntity variableInstanceEntity, Object newValue) { - serviceConfiguration.getVariableInstanceValueModifier().updateVariableValue(variableInstanceEntity, newValue, tenantId); - update(variableInstanceEntity, true); - serviceConfiguration.getInternalHistoryVariableManager().recordVariableUpdate(variableInstanceEntity, serviceConfiguration.getClock().getCurrentTime()); - } - @Override public InternalVariableInstanceQuery createInternalVariableInstanceQuery() { return new InternalVariableInstanceQueryImpl(dataManager);