Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix get historic process instance api orquery for mutiple statues and… #4515

Closed
wants to merge 1 commit into from
Closed

Fix get historic process instance api orquery for mutiple statues and… #4515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions ...rg/camunda/bpm/engine/rest/history/HistoricProcessInstanceRestServiceInteractionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.HashSet;
import javax.ws.rs.core.Response.Status;
import org.camunda.bpm.engine.AuthorizationException;
import org.camunda.bpm.engine.BadUserRequestException;
Expand Down Expand Up @@ -549,11 +550,7 @@ public void testOrQuery() {
// given
HistoricProcessInstanceQueryImpl mockedQuery = mock(HistoricProcessInstanceQueryImpl.class);
when(historyServiceMock.createHistoricProcessInstanceQuery()).thenReturn(mockedQuery);

String payload = "{ \"orQueries\": [{" +
"\"processDefinitionKey\": \"aKey\", " +
"\"processInstanceBusinessKey\": \"aBusinessKey\"}] }";

String payload = "{\"orQueries\": [{ \"processDefinitionKey\": \"aKey\", \"processInstanceBusinessKey\": \"aBusinessKey\", \"completed\": true ,\"active\": true}]}";
// when
given()
.contentType(POST_JSON_CONTENT_TYPE)
Expand All @@ -572,6 +569,7 @@ public void testOrQuery() {
// then
assertThat(argument.getValue().getProcessDefinitionKey()).isEqualTo("aKey");
assertThat(argument.getValue().getBusinessKey()).isEqualTo("aBusinessKey");
assertThat(argument.getValue().getState()).isEqualTo(new HashSet<>(Arrays.asList("COMPLETED", "ACTIVE")));
}

protected void verifyBatchJson(String batchJson) {
Expand Down
36 changes: 23 additions & 13 deletions engine/src/main/java/org/camunda/bpm/engine/impl/HistoricProcessInstanceQueryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import static org.camunda.bpm.engine.impl.util.EnsureUtil.ensureNotContainsNull;
import static org.camunda.bpm.engine.impl.util.EnsureUtil.ensureNotEmpty;
import static org.camunda.bpm.engine.impl.util.EnsureUtil.ensureNotNull;
import static org.camunda.bpm.engine.impl.util.EnsureUtil.ensureNull;
import static org.camunda.bpm.engine.impl.util.EnsureUtil.ensureEmpty;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -91,7 +91,7 @@ public class HistoricProcessInstanceQueryImpl extends AbstractVariableQueryImpl<
protected boolean isTenantIdSet;
protected String[] executedActivityIds;
protected String[] activeActivityIds;
protected String state;
protected Set<String> state = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes related to multiple state handling don't seem to be described or explained in the issue. Only the unexpected behavior of the combination of withIncidents and orQueries is described there. Could you please document this as well?


protected String caseInstanceId;

Expand Down Expand Up @@ -599,7 +599,7 @@ public String getIncidentStatus() {
return incidentStatus;
}

public String getState() {
public Set<String> getState() {
return state;
}

Expand Down Expand Up @@ -772,36 +772,46 @@ public HistoricProcessInstanceQuery activeActivityIdIn(String... ids) {

@Override
public HistoricProcessInstanceQuery active() {
ensureNull(BadUserRequestException.class, "Already querying for historic process instance with another state", state, state);
state = HistoricProcessInstance.STATE_ACTIVE;
if(!isOrQueryActive) {
ensureEmpty(BadUserRequestException.class, "Already querying for historic process instance with another state", state);
}
state.add(HistoricProcessInstance.STATE_ACTIVE);
return this;
}

@Override
public HistoricProcessInstanceQuery suspended() {
ensureNull(BadUserRequestException.class, "Already querying for historic process instance with another state", state, state);
state = HistoricProcessInstance.STATE_SUSPENDED;
if(!isOrQueryActive) {
ensureEmpty(BadUserRequestException.class, "Already querying for historic process instance with another state", state);
}
state.add(HistoricProcessInstance.STATE_SUSPENDED);
return this;
}

@Override
public HistoricProcessInstanceQuery completed() {
ensureNull(BadUserRequestException.class, "Already querying for historic process instance with another state", state, state);
state = HistoricProcessInstance.STATE_COMPLETED;
if(!isOrQueryActive) {
ensureEmpty(BadUserRequestException.class, "Already querying for historic process instance with another state", state);
}
state.add(HistoricProcessInstance.STATE_COMPLETED);
return this;
}

@Override
public HistoricProcessInstanceQuery externallyTerminated() {
ensureNull(BadUserRequestException.class, "Already querying for historic process instance with another state", state, state);
state = HistoricProcessInstance.STATE_EXTERNALLY_TERMINATED;
if(!isOrQueryActive) {
ensureEmpty(BadUserRequestException.class, "Already querying for historic process instance with another state", state);
}
state.add(HistoricProcessInstance.STATE_EXTERNALLY_TERMINATED);
return this;
}

@Override
public HistoricProcessInstanceQuery internallyTerminated() {
ensureNull(BadUserRequestException.class, "Already querying for historic process instance with another state", state, state);
state = HistoricProcessInstance.STATE_INTERNALLY_TERMINATED;
if(!isOrQueryActive) {
ensureEmpty(BadUserRequestException.class, "Already querying for historic process instance with another state", state);
}
state.add(HistoricProcessInstance.STATE_INTERNALLY_TERMINATED);
return this;
}

Expand Down
8 changes: 8 additions & 0 deletions engine/src/main/java/org/camunda/bpm/engine/impl/util/EnsureUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ public static void ensureNotEmpty(Class<? extends ProcessEngineException> except
}
}

@SuppressWarnings("rawtypes")
public static void ensureEmpty(Class<? extends ProcessEngineException> exceptionClass, String message, Collection collection) {
if (collection != null && !collection.isEmpty()) {
String variableName = collection.iterator().next().toString();
throw generateException(exceptionClass, message, variableName, "is not empty");
}
}

@SuppressWarnings("rawtypes")
public static void ensureNotEmpty(String variableName, Map map) {
ensureNotEmpty("", variableName, map);
Expand Down
17 changes: 12 additions & 5 deletions ...src/main/resources/org/camunda/bpm/engine/impl/mapping/entity/HistoricProcessInstance.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,12 +402,12 @@
<bind name="JOIN_TYPE" value="'inner join'" />

<foreach collection="queries" item="query">
<if test="query.isOrQueryActive">
<bind name="JOIN_TYPE" value="'left join'" />
</if>

<if test="query != null &amp;&amp; (query.withIncidents || query.withRootIncidents || query.incidentStatus != null || query.incidentMessage != null || query.incidentMessageLike != null || query.incidentType != null)">
<bind name="INC_JOIN" value="true" />
<if test="query.isOrQueryActive">
<bind name="JOIN_TYPE" value="'left join'" />
</if>
</if>

<if test="query != null &amp;&amp; (query.executedActivityIds != null and query.executedActivityIds.length > 0) || (query.activeActivityIds != null and query.activeActivityIds.length > 0)">
Expand Down Expand Up @@ -516,8 +516,15 @@
</foreach>
)
</if>
<if test="query.state != null">
${queryType} SELF.STATE_ = #{query.state}

<if test="query.state != null and !query.state.isEmpty()">
${queryType} (
SELF.STATE_ IS NOT NULL
AND SELF.STATE_ IN
<foreach item="state" index="index" collection="query.state" open="(" separator="," close=")">
#{state}
</foreach>
)
</if>


Expand Down
185 changes: 185 additions & 0 deletions ...src/test/java/org/camunda/bpm/engine/test/history/HistoricProcessInstanceQueryOrTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,106 @@ public void shouldReturnHistoricProcInstWithProcessDefinitionNameOrProcessDefini
assertEquals(2, processInstances.size());
}

@Test
public void shouldReturnHistoricProcInstWithMultipleStates() {
// given
BpmnModelInstance aProcessDefinition = Bpmn.createExecutableProcess("aProcessDefinition")
.name("process1")
.startEvent()
.userTask()
.endEvent()
.done();

String deploymentId = repositoryService
.createDeployment()
.addModelInstance("foo.bpmn", aProcessDefinition)
.deploy()
.getId();

deploymentIds.add(deploymentId);

ProcessInstance processInstance1 = runtimeService.startProcessInstanceByKey("aProcessDefinition");

BpmnModelInstance anotherProcessDefinition = Bpmn.createExecutableProcess("anotherProcessDefinition")
.startEvent()
.userTask()
.endEvent()
.done();

deploymentId = repositoryService
.createDeployment()
.addModelInstance("foo.bpmn", anotherProcessDefinition)
.deploy()
.getId();

deploymentIds.add(deploymentId);

runtimeService.startProcessInstanceByKey("anotherProcessDefinition");
runtimeService.updateProcessInstanceSuspensionState()
.byProcessInstanceId(processInstance1.getId()).suspend();

// when
List<HistoricProcessInstance> processInstances = historyService.createHistoricProcessInstanceQuery()
.or()
.active()
.suspended()
.endOr()
.list();

// then
assertEquals(2, processInstances.size());
}

@Test
public void shouldReturnHistoricProcInstWithMatchingState() {
// given
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the whole given block of this test and the one above is exactly the same, please extract it into a method with a significant name and reuse the code.

BpmnModelInstance aProcessDefinition = Bpmn.createExecutableProcess("aProcessDefinition")
.name("process1")
.startEvent()
.userTask()
.endEvent()
.done();

String deploymentId = repositoryService
.createDeployment()
.addModelInstance("foo.bpmn", aProcessDefinition)
.deploy()
.getId();

deploymentIds.add(deploymentId);

ProcessInstance processInstance1 = runtimeService.startProcessInstanceByKey("aProcessDefinition");

BpmnModelInstance anotherProcessDefinition = Bpmn.createExecutableProcess("anotherProcessDefinition")
.startEvent()
.userTask()
.endEvent()
.done();

deploymentId = repositoryService
.createDeployment()
.addModelInstance("foo.bpmn", anotherProcessDefinition)
.deploy()
.getId();

deploymentIds.add(deploymentId);

runtimeService.startProcessInstanceByKey("anotherProcessDefinition");
runtimeService.updateProcessInstanceSuspensionState()
.byProcessInstanceId(processInstance1.getId()).suspend();

// when
List<HistoricProcessInstance> processInstances = historyService.createHistoricProcessInstanceQuery()
.or()
.active()
.endOr()
.list();

// then
assertEquals(1, processInstances.size());
assertEquals("ACTIVE", processInstances.get(0).getState());
}

@Test
public void shouldReturnHistoricProcInstWithBusinessKeyOrBusinessKeyLike() {
// given
Expand Down Expand Up @@ -817,4 +917,89 @@ public void shouldReturnByProcessDefinitionIdOrIncidentType() {
assertThat(processInstances.size()).isEqualTo(2);
}

@Test
@Deployment(resources={"org/camunda/bpm/engine/test/api/oneTaskProcess.bpmn20.xml"})
public void shouldReturnHistoricProcInstWithVarValue1OrVarValue21() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned here, this and the test below require FULL history level (more info).

// given
Map<String, Object> vars = new HashMap<String, Object>();
vars.put("stringVar", "abcdef");
runtimeService.startProcessInstanceByKey("oneTaskProcess", vars);

runtimeService.startProcessInstanceByKey("oneTaskProcess");

String processKey = "process";
deployFailingProcess(processKey);

vars = new HashMap<String, Object>();
vars.put("stringVar", "ghijkl");
runtimeService.startProcessInstanceByKey(processKey, vars);

String jobId = managementService.createJobQuery().singleResult().getId();

managementService.setJobRetries(jobId, 0);

// when
List<HistoricProcessInstance> processInstances = historyService.createHistoricProcessInstanceQuery()
.withIncidents()
.or()
.variableValueEquals("stringVar", "abcdef")
.variableValueEquals("stringVar", "ghijkl")
.endOr()
.list();

// then
assertEquals(1, processInstances.size());
}

protected void deployFailingProcess(String processKey) {
BpmnModelInstance aProcessDefinition = Bpmn.createExecutableProcess(processKey)
.startEvent()
.serviceTask()
.camundaClass("org.camunda.bpm.engine.test.jobexecutor.FailingDelegate")
.camundaAsyncBefore()
.endEvent()
.done();

String deploymentId = repositoryService
.createDeployment()
.addModelInstance("foo.bpmn", aProcessDefinition)
.deploy()
.getId();

deploymentIds.add(deploymentId);
}

@Test
@Deployment(resources={"org/camunda/bpm/engine/test/api/oneTaskProcess.bpmn20.xml"})
public void shouldReturnHistoricProcInstWithVarValue1OrVarValue23() {
// given
Map<String, Object> vars = new HashMap<String, Object>();
vars.put("stringVar", "abcdef");
runtimeService.startProcessInstanceByKey("oneTaskProcess", vars);

runtimeService.startProcessInstanceByKey("oneTaskProcess");

String processKey = "process";
deployFailingProcess(processKey);

vars = new HashMap<String, Object>();
vars.put("stringVar", "ghijkl");
runtimeService.startProcessInstanceByKey(processKey, vars);

String jobId = managementService.createJobQuery().singleResult().getId();

managementService.setJobRetries(jobId, 0);

// when
List<HistoricProcessInstance> processInstances = historyService.createHistoricProcessInstanceQuery()
.or()
.withIncidents()
.variableValueEquals("stringVar", "abcdef")
.endOr()
.list();

// then
assertEquals(1, processInstances.size());
}

}