[CAM-9676] feat(engine): Implemented OR queries for ProcessInstanceQuery

[funfried]

OR queries extension for ProcessInstanceQuery based on the already existing OR queries for tasks from tag 7.10.0. Also added a JUnit test for it and extended the query DTO for the REST API.

See also CAM-9676 in Jira

[ThorbenLindhauer]

Hi @funfried,

Since this is a bigger contribution, it may take us a while to review the code. I hope that is ok for you.

Best regards,
Thorben

[funfried]

Hi @ThorbenLindhauer

Sure, no problem, take your time. I think I'll have to wait for the next community release until it might get published anyway, so I've built my own patch release which I'm gonna use till then ☺️

Regards,
Fabian

[tasso94]

Hi @funfried,

thanks for your contribution and for your patience.

It looks good so far. However, I have some review hints for you:

• Please consolidate all the validation code in one central method #checkQueryOk. This method should be executed in #executeCount and #executeList. Like this, it is easier to maintain the business logic which is in charge for the validation of the query.
• All OR scenarios which require a table join will currently lead to wrong results. For such cases ProcessEngineExceptions should be thrown.

Please adjust the Pull Requests #321 and #319 as well.

If you have any questions, please do not hesitate to ask.

Cheers,
Tassilo

[funfried]

Hello @tasso94,

thanks for reviewing!

Regarding your comments:

• I just used the #checkQueryOk method from the AbstractQuery class. So do you suggest to overwrite that method and call the super implementation and also the methods #ensureOrExpressionsEvaluated and #ensureVariablesInitialized or what do you mean by "business logic which is in charge for the validation of the query"?
• Not sure what you mean with "All OR scenarios which require a table join will currently lead to wrong results". Is there something wrong in the code or do I just miss some throws notations for ProcessEngineExceptions? Could you give me an example of what is wrong or missing?

Best regards,
Fabian

[tasso94]

Hi @funfried,

Re 1: Please consider this example code. Let's move this and all other validations to the separate method #checkQueryOk.

Re 2: Let's assume we have two historical task instances the first has an assignee "foo" and the second has a process instance business key "bar". If you apply the filter #taskAssignee("foo") and #processInstanceBusinessKey("bar") within one OR query, you will not get the expected result of two but only one historical task instance instead. This is because an inner join between the historical task instance table and the historical process instance table is performed which will never lead to the expected result. This is why a ProcessEngineException should be thrown when combining filter criteria where joining tables is necessary.

Cheers,
Tassilo

[funfried]

Hello @tasso94,

sorry for responding so late, I'm kind of busy at the moment, but I will definitely implement the requested changes as soon as possible. I hope I get to it next week.

Regards,
Fabian

[funfried]

Hello @tasso94 ,

I updated the PR, please check again. I will also update the other two PRs.

Regards,
Fabian

[funfried]

@tasso94

Regarding the OR scenarios with table joins, are you sure it won't work in any cases? Because there are tests for some of the cases which were actually correct, e.g. definitionName and definitionKey in the historic task instance query. I think in this case it should be fine, or?

Cheers,
Fabian

[ThorbenLindhauer]

HI Fabian,

Tassilo is currently out of office until the 29th of April. He will get back to you then.

Cheers,
Thorben

[funfried]

Hi @ThorbenLindhauer,

sure, no worries, thanks for letting me know.

Regards,
Fabian

[tasso94]

Hi Fabian,

thanks for revising your pull request.

I will look into your changes in the course of this week.

Cheers,
Tassilo

[tasso94]

Hi Fabian,

thanks for your patience.

The following bug issue (https://app.camunda.com/jira/browse/CAM-9114) is scheduled to be fixed soon and we consider to fix the problem for your contribution as well. When it is decided, we will take care of fixing it for your contribution as well.

Right now we are busy with the 7.11 release (scheduled for 31st of May) and will come back to you as soon as possible.

Stay tuned!

Cheers,
Tassilo

[funfried]

Hello Tassilo,

sorry for the late response, I had parental leave the last few weeks and so my PC wasn't turned on that much 😊. Anyways, good luck with the release and talk to you afterwards.

Regards,
Fabian

[funfried]

Hi @tasso94,

sorry to bother, but have you been able to check my PRs?

Regards,
Fabian

[tasso94]

Hi Fabian,

sorry for my late response. We've fixed the issue regarding table joins for the task query recently [1] and will now look into your contribution again.

Cheers,
Tassilo

[1] https://app.camunda.com/jira/browse/CAM-9114

[funfried]

Hi @tasso94,

that sounds great, thanks. Looking forward to it.

Regards,
Fabian