-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
feature(engine): add activityIdIn filter to HistoricProcessInstanceQuery #4619
feature(engine): add activityIdIn filter to HistoricProcessInstanceQuery #4619
Conversation
a571bce
to
c088a99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You are only testing the use case with a single activityId but not with multiple ids.
- Can we test multiple activities in different scopes with and without incidents?
- ❓ Does it make sense to copy some of these tests over to
HistoricProcessInstanceTest
(please scroll in the code excerpt to see all the tests)
Lines 1997 to 2192 in 1c5a126
@Test public void testQueryByNullActivityId() { try { runtimeService.createProcessInstanceQuery() .activityIdIn((String) null); fail("exception expected"); } catch (NullValueException e) { assertThat(e.getMessage()).contains("activity ids contains null value"); } } @Test public void testQueryByNullActivityIds() { try { runtimeService.createProcessInstanceQuery() .activityIdIn((String[]) null); fail("exception expected"); } catch (NullValueException e) { assertThat(e.getMessage()).contains("activity ids is null"); } } @Test public void testQueryByUnknownActivityId() { ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery() .activityIdIn("unknown"); assertNoProcessInstancesReturned(query); } @Test public void testQueryByLeafActivityId() { // given ProcessDefinition oneTaskDefinition = testHelper.deployAndGetDefinition(ProcessModels.ONE_TASK_PROCESS); ProcessDefinition gatewaySubProcessDefinition = testHelper.deployAndGetDefinition(FORK_JOIN_SUB_PROCESS_MODEL); // when ProcessInstance oneTaskInstance1 = runtimeService.startProcessInstanceById(oneTaskDefinition.getId()); ProcessInstance oneTaskInstance2 = runtimeService.startProcessInstanceById(oneTaskDefinition.getId()); ProcessInstance gatewaySubProcessInstance1 = runtimeService.startProcessInstanceById(gatewaySubProcessDefinition.getId()); ProcessInstance gatewaySubProcessInstance2 = runtimeService.startProcessInstanceById(gatewaySubProcessDefinition.getId()); Task task = engineRule.getTaskService().createTaskQuery() .processInstanceId(gatewaySubProcessInstance2.getId()) .taskName("completeMe") .singleResult(); engineRule.getTaskService().complete(task.getId()); // then ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery().activityIdIn("userTask"); assertReturnedProcessInstances(query, oneTaskInstance1, oneTaskInstance2); query = runtimeService.createProcessInstanceQuery().activityIdIn("userTask1", "userTask2"); assertReturnedProcessInstances(query, gatewaySubProcessInstance1, gatewaySubProcessInstance2); query = runtimeService.createProcessInstanceQuery().activityIdIn("userTask", "userTask1"); assertReturnedProcessInstances(query, oneTaskInstance1, oneTaskInstance2, gatewaySubProcessInstance1); query = runtimeService.createProcessInstanceQuery().activityIdIn("userTask", "userTask1", "userTask2"); assertReturnedProcessInstances(query, oneTaskInstance1, oneTaskInstance2, gatewaySubProcessInstance1, gatewaySubProcessInstance2); query = runtimeService.createProcessInstanceQuery().activityIdIn("join"); assertReturnedProcessInstances(query, gatewaySubProcessInstance2); } @Test public void testQueryByNonLeafActivityId() { // given ProcessDefinition processDefinition = testHelper.deployAndGetDefinition(FORK_JOIN_SUB_PROCESS_MODEL); // when runtimeService.startProcessInstanceById(processDefinition.getId()); // then ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery().activityIdIn("subProcess", "fork"); assertNoProcessInstancesReturned(query); } @Test public void testQueryByAsyncBeforeActivityId() { // given ProcessDefinition testProcess = testHelper.deployAndGetDefinition(ProcessModels.newModel() .startEvent("start").camundaAsyncBefore() .subProcess("subProcess").camundaAsyncBefore() .embeddedSubProcess() .startEvent() .serviceTask("task").camundaAsyncBefore().camundaExpression("${true}") .endEvent() .subProcessDone() .endEvent("end").camundaAsyncBefore() .done() ); // when ProcessInstance instanceBeforeStart = runtimeService.startProcessInstanceById(testProcess.getId()); ProcessInstance instanceBeforeSubProcess = runtimeService.startProcessInstanceById(testProcess.getId()); executeJobForProcessInstance(instanceBeforeSubProcess); ProcessInstance instanceBeforeTask = runtimeService.startProcessInstanceById(testProcess.getId()); executeJobForProcessInstance(instanceBeforeTask); executeJobForProcessInstance(instanceBeforeTask); ProcessInstance instanceBeforeEnd = runtimeService.startProcessInstanceById(testProcess.getId()); executeJobForProcessInstance(instanceBeforeEnd); executeJobForProcessInstance(instanceBeforeEnd); executeJobForProcessInstance(instanceBeforeEnd); // then ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery().activityIdIn("start"); assertReturnedProcessInstances(query, instanceBeforeStart); query = runtimeService.createProcessInstanceQuery().activityIdIn("subProcess"); assertReturnedProcessInstances(query, instanceBeforeSubProcess); query = runtimeService.createProcessInstanceQuery().activityIdIn("task"); assertReturnedProcessInstances(query, instanceBeforeTask); query = runtimeService.createProcessInstanceQuery().activityIdIn("end"); assertReturnedProcessInstances(query, instanceBeforeEnd); } @Test public void testQueryByAsyncAfterActivityId() { // given ProcessDefinition testProcess = testHelper.deployAndGetDefinition(ProcessModels.newModel() .startEvent("start").camundaAsyncAfter() .subProcess("subProcess").camundaAsyncAfter() .embeddedSubProcess() .startEvent() .serviceTask("task").camundaAsyncAfter().camundaExpression("${true}") .endEvent() .subProcessDone() .endEvent("end").camundaAsyncAfter() .done() ); // when ProcessInstance instanceAfterStart = runtimeService.startProcessInstanceById(testProcess.getId()); ProcessInstance instanceAfterTask = runtimeService.startProcessInstanceById(testProcess.getId()); executeJobForProcessInstance(instanceAfterTask); ProcessInstance instanceAfterSubProcess = runtimeService.startProcessInstanceById(testProcess.getId()); executeJobForProcessInstance(instanceAfterSubProcess); executeJobForProcessInstance(instanceAfterSubProcess); ProcessInstance instanceAfterEnd = runtimeService.startProcessInstanceById(testProcess.getId()); executeJobForProcessInstance(instanceAfterEnd); executeJobForProcessInstance(instanceAfterEnd); executeJobForProcessInstance(instanceAfterEnd); // then ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery().activityIdIn("start"); assertReturnedProcessInstances(query, instanceAfterStart); query = runtimeService.createProcessInstanceQuery().activityIdIn("task"); assertReturnedProcessInstances(query, instanceAfterTask); query = runtimeService.createProcessInstanceQuery().activityIdIn("subProcess"); assertReturnedProcessInstances(query, instanceAfterSubProcess); query = runtimeService.createProcessInstanceQuery().activityIdIn("end"); assertReturnedProcessInstances(query, instanceAfterEnd); } @Test public void testQueryByActivityIdBeforeCompensation() { // given ProcessDefinition testProcess = testHelper.deployAndGetDefinition(CompensationModels.COMPENSATION_ONE_TASK_SUBPROCESS_MODEL); // when runtimeService.startProcessInstanceById(testProcess.getId()); testHelper.completeTask("userTask1"); // then ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery().activityIdIn("subProcess"); assertNoProcessInstancesReturned(query); } @Test public void testQueryByActivityIdDuringCompensation() { // given ProcessDefinition testProcess = testHelper.deployAndGetDefinition(CompensationModels.COMPENSATION_ONE_TASK_SUBPROCESS_MODEL); // when ProcessInstance processInstance = runtimeService.startProcessInstanceById(testProcess.getId()); testHelper.completeTask("userTask1"); testHelper.completeTask("userTask2"); // then ProcessInstanceQuery query = runtimeService.createProcessInstanceQuery().activityIdIn("subProcess"); assertReturnedProcessInstances(query, processInstance); query = runtimeService.createProcessInstanceQuery().activityIdIn("compensationEvent"); assertReturnedProcessInstances(query, processInstance); query = runtimeService.createProcessInstanceQuery().activityIdIn("compensationHandler"); assertReturnedProcessInstances(query, processInstance); } - One candidate would be passing
null
but also some others might make sense.
- One candidate would be passing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Do we need to test call activities? I mean an incident in a call activity.
...t/src/main/java/org/camunda/bpm/engine/rest/dto/history/HistoricProcessInstanceQueryDto.java
Outdated
Show resolved
Hide resolved
@@ -626,6 +636,21 @@ | |||
) | |||
</if> | |||
|
|||
<if test="query.activityIds != null and query.activityIds.length > 0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Do we need to consider adding some indexes here? To ensure performance for the modification. Or the existing indexes will be good enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's be careful with indices on history tables. I would only add them when we get feedback. Adding a new index for existing databases with tables that have huge amounts of data is a costly operation, and it slows down process execution since history insertion is slower.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✍️ To add to Tassilo's suggestions further.
We need to have tests for filtering historic instances where some of them have been already completed/deleted.
Similar to what was done in #4624 test coverage. Since that will be the usual case to do modification.
|
Added test when some instances are deleted and completed |
4168695
to
7fb55ec
Compare
ActivityIdIn filter returns different processInstances for runtimeQuery and HistoricQuery in special cases around |
48bac24
to
86488bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
engine-rest/engine-rest-openapi/src/main/templates/lib/commons/history-process-instance.ftl
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/camunda/bpm/engine/history/HistoricProcessInstanceQuery.java
Outdated
Show resolved
Hide resolved
@@ -626,6 +636,21 @@ | |||
) | |||
</if> | |||
|
|||
<if test="query.activityIds != null and query.activityIds.length > 0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's be careful with indices on history tables. I would only add them when we get feedback. Adding a new index for existing databases with tables that have huge amounts of data is a costly operation, and it slows down process execution since history insertion is slower.
…est/dto/history/HistoricProcessInstanceQueryDto.java Co-authored-by: yanavasileva <[email protected]>
… Compensation is used" This reverts commit 4168695.
0b8956c
to
e6dc437
Compare
related to: #4618