Skip to content

Commit

Permalink
Remove not needed update methods from VariableService
Browse files Browse the repository at this point in the history
  • Loading branch information
@filiphr
filiphr committed May 11, 2023
1 parent aedf85e commit 8960137
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 47 deletions.
8 changes: 7 additions & 1 deletion ...flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/async/AsyncCmmnHistoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
});

Expand Down
8 changes: 7 additions & 1 deletion modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
});
Expand Down
110 changes: 108 additions & 2 deletions ...st/java/org/flowable/engine/test/api/variables/VariableInstanceValueModifierBpmnTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

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.HashMap;
Expand All @@ -24,16 +25,21 @@

import org.apache.commons.lang3.StringUtils;
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;
Expand Down Expand Up @@ -153,6 +159,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);
Expand All @@ -168,6 +184,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
Expand Down Expand Up @@ -221,6 +247,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);
Expand All @@ -244,6 +285,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();
Expand Down Expand Up @@ -307,6 +363,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);
Expand All @@ -329,6 +400,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();
Expand Down Expand Up @@ -370,12 +452,10 @@ protected void setOrUpdateValue(VariableInstance variableInstance, Object value,
@Test
@Deployment(resources = { "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml" })
public void testTransientVariables() {
List<String> 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");
}
Expand Down Expand Up @@ -458,6 +538,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 {
Expand Down Expand Up @@ -530,4 +629,11 @@ public int hashCode() {
}
}

static class TestValueModifier extends DefaultVariableInstanceValueModifier {

public TestValueModifier(VariableServiceConfiguration serviceConfiguration) {
super(serviceConfiguration);
}
}

}
17 changes: 15 additions & 2 deletions ...s/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
});

Expand Down Expand Up @@ -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;
});
Expand Down
8 changes: 7 additions & 1 deletion ...flowable-engine/src/test/java/org/flowable/standalone/history/async/AsyncHistoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
});

Expand Down
10 changes: 0 additions & 10 deletions ...lowable-variable-service/src/main/java/org/flowable/variable/service/VariableService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import java.util.List;

import org.flowable.variable.service.impl.VariableInstanceValueModifier;
import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity;

/**
Expand Down Expand Up @@ -57,15 +56,6 @@ default List<VariableInstanceEntity> 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);
Expand Down
Loading

0 comments on commit 8960137

Please sign in to comment.