Skip to content

Commit

Permalink
feat(engine): don't fail on change variables async when process insta…
Browse files Browse the repository at this point in the history 
…nce does not exist

Related to #4795
  • Loading branch information
@joaquinfelici
joaquinfelici committed Nov 21, 2024
1 parent d27a7be commit 4b101ea
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 48 deletions.
2 changes: 1 addition & 1 deletion ...e/src/main/java/org/camunda/bpm/engine/impl/batch/variables/BatchSetVariablesHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void executeHandler(BatchConfiguration batchConfiguration,

for (String processInstanceId : processInstanceIds) {
commandContext.executeWithOperationLogPrevented(
new SetExecutionVariablesCmd(processInstanceId, variables, false, true));
new SetExecutionVariablesCmd(processInstanceId, variables, false, true, false));
}
}

Expand Down
12 changes: 6 additions & 6 deletions engine/src/main/java/org/camunda/bpm/engine/impl/cmd/AbstractVariableCmd.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ public Void execute(CommandContext commandContext) {

AbstractVariableScope scope = getEntity();

executeOperation(scope);

onSuccess(scope);

if(!preventLogUserOperation) {
logVariableOperation(scope);
if (scope != null) {
executeOperation(scope);
onSuccess(scope);
if(!preventLogUserOperation) {
logVariableOperation(scope);
}
}

return null;
Expand Down
19 changes: 17 additions & 2 deletions engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetExecutionVariablesCmd.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,24 @@ public class SetExecutionVariablesCmd extends AbstractSetVariableCmd {

private static final long serialVersionUID = 1L;

protected boolean failIfNotExists = true;

public SetExecutionVariablesCmd(String executionId,
Map<String, ?> variables,
boolean isLocal,
boolean skipJavaSerializationFormatCheck) {
super(executionId, variables, isLocal, skipJavaSerializationFormatCheck);
}

public SetExecutionVariablesCmd(String executionId,
Map<String, ?> variables,
boolean isLocal,
boolean skipJavaSerializationFormatCheck,
boolean failIfNotExists) {
this(executionId, variables, isLocal, skipJavaSerializationFormatCheck);
this.failIfNotExists = failIfNotExists;
}

public SetExecutionVariablesCmd(String executionId, Map<String, ? extends Object> variables, boolean isLocal) {
super(executionId, variables, isLocal, false);
}
Expand All @@ -52,9 +63,13 @@ protected ExecutionEntity getEntity() {
.getExecutionManager()
.findExecutionById(entityId);

ensureNotNull("execution " + entityId + " doesn't exist", "execution", execution);
if (failIfNotExists) {
ensureNotNull("execution " + entityId + " doesn't exist", "execution", execution);
}

checkSetExecutionVariables(execution);
if(execution != null) {
checkSetExecutionVariables(execution);
}

return execution;
}
Expand Down
18 changes: 17 additions & 1 deletion engine/src/test/java/org/camunda/bpm/engine/test/api/runtime/RuntimeServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package org.camunda.bpm.engine.test.api.runtime;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.camunda.bpm.engine.test.util.ActivityInstanceAssert.assertThat;
import static org.camunda.bpm.engine.test.util.ActivityInstanceAssert.describeActivityInstanceTree;
import static org.camunda.bpm.engine.test.util.ExecutableProcessUtil.USER_TASK_PROCESS;
import static org.camunda.bpm.engine.variable.Variables.createVariables;
import static org.camunda.bpm.engine.variable.Variables.objectValue;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -110,7 +112,6 @@
public class RuntimeServiceTest {

public static final String TESTING_INSTANCE_DELETION = "testing instance deletion";
public static final String A_STREAM = "aStream";

@ClassRule
public static ProcessEngineBootstrapRule bootstrapRule = new ProcessEngineBootstrapRule(configuration ->
Expand Down Expand Up @@ -1045,6 +1046,21 @@ public void testSetVariablesNullExecutionId() {
}
}


@Test
public void setVariablesSyncOnCompletedProcessInstance() {
// given completed process instance
testRule.deploy(USER_TASK_PROCESS);
String id = runtimeService.startProcessInstanceByKey("process").getId();
Task task = engineRule.getTaskService().createTaskQuery().processInstanceId(id).singleResult();
engineRule.getTaskService().complete(task.getId());

// when setting variables then exception is thrown
assertThatThrownBy(() -> runtimeService.setVariables(id, Variables.createVariables().putValue("foo", "bar")))
.isInstanceOf(NullValueException.class)
.hasMessage("execution " + id + " doesn't exist: execution is null");
}

private void checkHistoricVariableUpdateEntity(String variableName, String processInstanceId) {
if (processEngineConfiguration.getHistoryLevel().equals(HistoryLevel.HISTORY_LEVEL_FULL)) {
boolean deletedVariableUpdateFound = false;
Expand Down
119 changes: 95 additions & 24 deletions engine/src/test/java/org/camunda/bpm/engine/test/api/runtime/SetVariablesBatchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.tuple;
import static org.camunda.bpm.engine.test.util.ExecutableProcessUtil.USER_TASK_PROCESS;

import java.util.Arrays;
import java.util.Collections;
Expand All @@ -40,11 +41,11 @@
import org.camunda.bpm.engine.history.HistoricProcessInstanceQuery;
import org.camunda.bpm.engine.history.UserOperationLogEntry;
import org.camunda.bpm.engine.impl.util.ClockUtil;
import org.camunda.bpm.engine.management.JobDefinition;
import org.camunda.bpm.engine.repository.Deployment;
import org.camunda.bpm.engine.runtime.Job;
import org.camunda.bpm.engine.runtime.ProcessInstanceQuery;
import org.camunda.bpm.engine.runtime.VariableInstanceQuery;
import org.camunda.bpm.engine.task.Task;
import org.camunda.bpm.engine.test.ProcessEngineRule;
import org.camunda.bpm.engine.test.RequiredHistoryLevel;
import org.camunda.bpm.engine.test.dmn.businessruletask.TestPojo;
Expand All @@ -70,11 +71,11 @@ public class SetVariablesBatchTest {

protected ProcessEngineRule engineRule = new ProvidedProcessEngineRule();
protected ProcessEngineTestRule engineTestRule = new ProcessEngineTestRule(engineRule);
protected BatchRule rule = new BatchRule(engineRule, engineTestRule);
protected BatchRule batchRule = new BatchRule(engineRule, engineTestRule);
protected BatchHelper helper = new BatchHelper(engineRule);

@Rule
public RuleChain ruleChain = RuleChain.outerRule(engineRule).around(engineTestRule).around(rule);
public RuleChain ruleChain = RuleChain.outerRule(engineRule).around(engineTestRule).around(batchRule);

protected RuntimeService runtimeService;
protected HistoryService historyService;
Expand Down Expand Up @@ -122,7 +123,7 @@ public void shouldSetByIds() {
.containsExactly(tuple(null, "foo", "bar", batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -152,7 +153,7 @@ public void shouldSetByIds_TypedValue() {
.containsExactly(tuple(null, "foo", "bar", batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -188,7 +189,7 @@ public void shouldSetByIds_MixedTypedAndUntypedValues() {
);

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -223,7 +224,7 @@ public void shouldSetByIds_ObjectValue() {
.containsExactly(tuple(null, "foo", pojo, batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -260,7 +261,7 @@ public void shouldSetByIds_MultipleVariables() {
);

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -301,7 +302,7 @@ public void shouldSetByIds_VariablesAsMap() {
);

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -332,7 +333,7 @@ public void shouldSetByRuntimeQuery() {
.containsExactly(tuple(null, "foo", "bar", batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -372,7 +373,7 @@ public void shouldSetByIdsAndQueries() {
.containsExactly(tuple(null, "foo", "bar", batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -402,7 +403,7 @@ public void shouldSetByHistoryQuery() {
.containsExactly(tuple(null, "foo", "bar", batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -433,7 +434,7 @@ public void shouldSetByHistoryQuery_WithFinishedInstances() {
.containsExactly(tuple(null, "foo", "bar", batch.getId()));

// when
rule.syncExec(batch);
batchRule.syncExec(batch);

// then
assertThat(query.list())
Expand Down Expand Up @@ -539,10 +540,10 @@ public void shouldCreateDeploymentAwareBatchJobs_ByIds() {
// when
Batch batch = runtimeService.setVariablesAsync(processInstanceIds, SINGLE_VARIABLE);

rule.executeSeedJobs(batch);
batchRule.executeSeedJobs(batch);

// then
List<Job> executionJobs = rule.getExecutionJobs(batch);
List<Job> executionJobs = batchRule.getExecutionJobs(batch);
assertThat(executionJobs)
.extracting("deploymentId")
.containsExactlyInAnyOrder(deploymentIdOne, deploymentIdTwo);
Expand Down Expand Up @@ -570,10 +571,10 @@ public void shouldCreateDeploymentAwareBatchJobs_ByRuntimeQuery() {
// when
Batch batch = runtimeService.setVariablesAsync(runtimeQuery, SINGLE_VARIABLE);

rule.executeSeedJobs(batch);
batchRule.executeSeedJobs(batch);

// then
List<Job> executionJobs = rule.getExecutionJobs(batch);
List<Job> executionJobs = batchRule.getExecutionJobs(batch);
assertThat(executionJobs)
.extracting("deploymentId")
.containsExactlyInAnyOrder(deploymentIdOne, deploymentIdTwo);
Expand Down Expand Up @@ -602,10 +603,10 @@ public void shouldCreateDeploymentAwareBatchJobs_ByHistoryQuery() {
// when
Batch batch = runtimeService.setVariablesAsync(historyQuery, SINGLE_VARIABLE);

rule.executeSeedJobs(batch);
batchRule.executeSeedJobs(batch);

// then
List<Job> executionJobs = rule.getExecutionJobs(batch);
List<Job> executionJobs = batchRule.getExecutionJobs(batch);
assertThat(executionJobs)
.extracting("deploymentId")
.containsExactlyInAnyOrder(deploymentIdOne, deploymentIdTwo);
Expand Down Expand Up @@ -659,7 +660,7 @@ public void shouldNotLogInstanceOperation() {
engineRule.getIdentityService()
.setAuthenticatedUserId("demo");

rule.syncExec(batch);
batchRule.syncExec(batch);

// then
List<UserOperationLogEntry> logs = historyService.createUserOperationLogQuery()
Expand All @@ -682,10 +683,10 @@ public void shouldCreateProcessInstanceRelatedBatchJobsForSingleInvocations() {
// when
Batch batch = runtimeService.setVariablesAsync(processInstanceIds, SINGLE_VARIABLE);

rule.executeSeedJobs(batch);
batchRule.executeSeedJobs(batch);

// then
List<Job> executionJobs = rule.getExecutionJobs(batch);
List<Job> executionJobs = batchRule.getExecutionJobs(batch);
assertThat(executionJobs)
.extracting("processInstanceId")
.containsExactlyInAnyOrder(processInstanceIdOne, processInstanceIdTwo);
Expand All @@ -707,10 +708,10 @@ public void shouldNotCreateProcessInstanceRelatedBatchJobsForMultipleInvocations
// when
Batch batch = runtimeService.setVariablesAsync(processInstanceIds, SINGLE_VARIABLE);

rule.executeSeedJobs(batch);
batchRule.executeSeedJobs(batch);

// then
List<Job> executionJobs = rule.getExecutionJobs(batch);
List<Job> executionJobs = batchRule.getExecutionJobs(batch);
assertThat(executionJobs)
.extracting("processInstanceId")
.containsOnlyNulls();
Expand Down Expand Up @@ -743,4 +744,74 @@ public void shouldSetExecutionStartTimeInBatchAndHistory() {
managementService.deleteBatch(batch.getId(), true);
}

@Test
public void setVariablesAsyncOnCompletedProcessInstance() {
// given set variables on completed process instance
engineTestRule.deploy(USER_TASK_PROCESS);
String id = runtimeService.startProcessInstanceByKey(PROCESS_KEY).getId();
Batch batch = runtimeService.setVariablesAsync(List.of(id), SINGLE_VARIABLE);
Task task = engineRule.getTaskService().createTaskQuery().processInstanceId(id).singleResult();
engineRule.getTaskService().complete(task.getId());

// when executing batch then no exception is thrown
batchRule.syncExec(batch);
}

@Test
public void setVariablesAsyncOnCompletedProcessInstanceWithQuery() {
// given set variables on completed process instance
engineTestRule.deploy(USER_TASK_PROCESS);
String id = runtimeService.startProcessInstanceByKey(PROCESS_KEY).getId();
Batch batch = runtimeService.setVariablesAsync(runtimeService.createProcessInstanceQuery().processInstanceId(id), SINGLE_VARIABLE);
Task task = engineRule.getTaskService().createTaskQuery().processInstanceId(id).singleResult();
engineRule.getTaskService().complete(task.getId());

// when executing batch then no exception is thrown
batchRule.syncExec(batch);
}

@Test
public void setVariablesAsyncOnCompletedProcessInstanceWithHistoricQuery() {
// given set variables on completed process instance
engineTestRule.deploy(USER_TASK_PROCESS);
String id = runtimeService.startProcessInstanceByKey(PROCESS_KEY).getId();
Batch batch = runtimeService.setVariablesAsync(historyService.createHistoricProcessInstanceQuery().processInstanceId(id), SINGLE_VARIABLE);
Task task = engineRule.getTaskService().createTaskQuery().processInstanceId(id).singleResult();
engineRule.getTaskService().complete(task.getId());

// when executing batch then no exception is thrown
batchRule.syncExec(batch);
}

@Test
public void setVariablesSyncOnCompletedProcessInstance() {
// given completed process
engineTestRule.deploy(USER_TASK_PROCESS);
String id = runtimeService.startProcessInstanceByKey(PROCESS_KEY).getId();
Task task = engineRule.getTaskService().createTaskQuery().processInstanceId(id).singleResult();
engineRule.getTaskService().complete(task.getId());

// when setting variables then exception is thrown
assertThatThrownBy(() -> runtimeService.setVariables(id, SINGLE_VARIABLE))
.isInstanceOf(NullValueException.class);
}


@Test
public void setVariablesAsyncOnBatchWithOneCompletedInstance() {
// given set variables batch with one completed process instance
engineTestRule.deploy(USER_TASK_PROCESS);
String id1 = runtimeService.startProcessInstanceByKey(PROCESS_KEY).getId();
String id2 = runtimeService.startProcessInstanceByKey(PROCESS_KEY).getId();
Batch batch = runtimeService.setVariablesAsync(List.of(id1, id2), SINGLE_VARIABLE);
Task task = engineRule.getTaskService().createTaskQuery().processInstanceId(id1).singleResult();
engineRule.getTaskService().complete(task.getId());

// when executing the bacth
batchRule.syncExec(batch);

// then no exception is thrown and the variables are set for the existing process
assertThat(runtimeService.getVariables(id2).equals(SINGLE_VARIABLE));
}

}
Loading

0 comments on commit 4b101ea

Please sign in to comment.