diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java new file mode 100644 index 00000000000..bcfa860e388 --- /dev/null +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java @@ -0,0 +1,199 @@ +/* Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.flowable.engine.test.impl; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; + +import org.flowable.common.engine.api.scope.ScopeTypes; +import org.flowable.common.engine.impl.db.AbstractDataManager; +import org.flowable.engine.impl.test.PluggableFlowableTestCase; +import org.flowable.engine.runtime.ProcessInstance; +import org.flowable.engine.test.Deployment; +import org.flowable.variable.api.history.HistoricVariableInstance; +import org.flowable.variable.api.persistence.entity.VariableInstance; +import org.flowable.variable.service.InternalVariableInstanceQuery; +import org.flowable.variable.service.VariableService; +import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * Tests that the variable queries split id collections which exceed {@link AbstractDataManager#MAX_ENTRIES_IN_CLAUSE} + * into multiple in() clauses, since not every database supports more entries in a single in() clause. + * + * @author Filip Hrisafov + */ +class VariableInstanceQueryLargeIdCollectionTest extends PluggableFlowableTestCase { + + protected final List variableIds = new ArrayList<>(); + + @AfterEach + void tearDown() { + managementService.executeCommand(commandContext -> { + VariableService variableService = processEngineConfiguration.getVariableServiceConfiguration().getVariableService(); + for (String variableId : variableIds) { + VariableInstanceEntity variable = variableService.createInternalVariableInstanceQuery().id(variableId).singleResult(); + if (variable != null) { + variableService.deleteVariableInstance(variable); + } + } + return null; + }); + variableIds.clear(); + } + + @Test + void internalQueryWithLargeTaskIdCollection() { + createVariable("taskVar", variable -> variable.setTaskId("task-1")); + createVariable("otherTaskVar", variable -> variable.setTaskId("task-2")); + createVariable("ignoredTaskVar", variable -> variable.setTaskId("task-3")); + + assertThat(findVariables(query -> query.taskIds(idsWith("task-1", "task-2")))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("taskVar", "otherTaskVar"); + } + + @Test + @Deployment(resources = "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml") + void internalQueryWithLargeExecutionIdCollection() { + // Variables are linked to an execution with a foreign key, so real process instances are needed here + ProcessInstance firstInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("executionVar", "first")); + ProcessInstance secondInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("otherExecutionVar", "second")); + runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("ignoredExecutionVar", "third")); + + assertThat(findVariables(query -> query.executionIds(idsWith(firstInstance.getId(), secondInstance.getId())))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("executionVar", "otherExecutionVar"); + } + + @Test + void internalQueryWithLargeScopeIdCollection() { + createVariable("scopeVar", variable -> { + variable.setScopeId("scope-1"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("otherScopeVar", variable -> { + variable.setScopeId("scope-2"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("ignoredScopeVar", variable -> { + variable.setScopeId("scope-3"); + variable.setScopeType(ScopeTypes.CMMN); + }); + + assertThat(findVariables(query -> query.scopeIds(idsWith("scope-1", "scope-2")).scopeType(ScopeTypes.CMMN))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("scopeVar", "otherScopeVar"); + } + + @Test + void internalQueryWithLargeSubScopeIdCollection() { + createVariable("subScopeVar", variable -> { + variable.setSubScopeId("subScope-1"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("otherSubScopeVar", variable -> { + variable.setSubScopeId("subScope-2"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("ignoredSubScopeVar", variable -> { + variable.setSubScopeId("subScope-3"); + variable.setScopeType(ScopeTypes.CMMN); + }); + + assertThat(findVariables(query -> query.subScopeIds(idsWith("subScope-1", "subScope-2")).scopeType(ScopeTypes.CMMN))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("subScopeVar", "otherSubScopeVar"); + } + + @Test + void variableInstanceQueryWithLargeTaskIdCollection() { + createVariable("taskVar", variable -> variable.setTaskId("task-1")); + createVariable("otherTaskVar", variable -> variable.setTaskId("task-2")); + createVariable("ignoredTaskVar", variable -> variable.setTaskId("task-3")); + + assertThat(runtimeService.createVariableInstanceQuery().taskIds(idsWith("task-1", "task-2")).list()) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("taskVar", "otherTaskVar"); + } + + @Test + @Deployment(resources = "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml") + void variableInstanceQueryWithLargeExecutionIdCollection() { + ProcessInstance firstInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("executionVar", "first")); + ProcessInstance secondInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("otherExecutionVar", "second")); + runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("ignoredExecutionVar", "third")); + + assertThat(runtimeService.createVariableInstanceQuery().executionIds(idsWith(firstInstance.getId(), secondInstance.getId())).list()) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("executionVar", "otherExecutionVar"); + } + + @Test + @Deployment(resources = "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml") + void historicVariableInstanceQueryWithLargeExecutionIdCollection() { + ProcessInstance firstInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("executionVar", "first")); + ProcessInstance secondInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("otherExecutionVar", "second")); + runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("ignoredExecutionVar", "third")); + + assertThat(historyService.createHistoricVariableInstanceQuery().executionIds(idsWith(firstInstance.getId(), secondInstance.getId())).list()) + .extracting(HistoricVariableInstance::getVariableName) + .containsExactlyInAnyOrder("executionVar", "otherExecutionVar"); + } + + protected List findVariables(Consumer queryCustomizer) { + return managementService.executeCommand(commandContext -> { + InternalVariableInstanceQuery query = processEngineConfiguration.getVariableServiceConfiguration() + .getVariableService() + .createInternalVariableInstanceQuery(); + queryCustomizer.accept(query); + return query.list(); + }); + } + + protected void createVariable(String name, Consumer variableCustomizer) { + String variableId = managementService.executeCommand(commandContext -> { + VariableService variableService = processEngineConfiguration.getVariableServiceConfiguration().getVariableService(); + VariableInstanceEntity variable = variableService.createVariableInstance(name); + variableCustomizer.accept(variable); + variableService.insertVariableInstanceWithValue(variable, name + "-value", null); + return variable.getId(); + }); + variableIds.add(variableId); + } + + /** + * Returns a set which contains the given ids, padded with dummy ids so the total exceeds the maximum number of entries in a single in() clause. + */ + protected Set idsWith(String... ids) { + Set allIds = new LinkedHashSet<>(List.of(ids)); + allIds.addAll(generateIds("dummy", AbstractDataManager.MAX_ENTRIES_IN_CLAUSE + 500)); + return allIds; + } + + protected Collection generateIds(String prefix, int amount) { + List ids = new ArrayList<>(amount); + for (int i = 0; i < amount; i++) { + ids.add(prefix + "-generated-" + i); + } + return ids; + } +} diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java index d85ec647dcb..3621376c951 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java @@ -422,6 +422,10 @@ public Set getExecutionIds() { return executionIds; } + public List> getSafeExecutionIds() { + return getSafeList(executionIds); + } + public boolean isExcludeTaskRelated() { return excludeTaskRelated; } diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java index 4e07a42e71c..14b967524cd 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java @@ -17,9 +17,11 @@ import org.apache.commons.lang3.StringUtils; import org.flowable.common.engine.api.FlowableIllegalArgumentException; +import org.flowable.common.engine.impl.db.AbstractDataManager; import org.flowable.common.engine.impl.db.SingleCachedEntityMatcher; import org.flowable.common.engine.impl.persistence.cache.CachedEntity; import org.flowable.common.engine.impl.persistence.cache.CachedEntityMatcher; +import org.flowable.common.engine.impl.util.CollectionUtil; import org.flowable.variable.service.InternalVariableInstanceQuery; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.flowable.variable.service.impl.persistence.entity.data.VariableInstanceDataManager; @@ -225,6 +227,10 @@ public Collection getTaskIds() { return taskIds; } + public List> getSafeTaskIds() { + return getSafeList(taskIds); + } + public String getProcessInstanceId() { return processInstanceId; } @@ -237,6 +243,10 @@ public Collection getExecutionIds() { return executionIds; } + public List> getSafeExecutionIds() { + return getSafeList(executionIds); + } + public boolean isWithoutTaskId() { return withoutTaskId; } @@ -249,6 +259,10 @@ public Collection getScopeIds() { return scopeIds; } + public List> getSafeScopeIds() { + return getSafeList(scopeIds); + } + public String getSubScopeId() { return subScopeId; } @@ -257,6 +271,10 @@ public Collection getSubScopeIds() { return subScopeIds; } + public List> getSafeSubScopeIds() { + return getSafeList(subScopeIds); + } + public boolean isWithoutSubScopeId() { return withoutSubScopeId; } @@ -277,6 +295,11 @@ public Collection getNames() { return names; } + protected List> getSafeList(Collection values) { + // need to split into different parts due to some dbs not supporting more than MAX_ENTRIES_IN_CLAUSE for in() + return CollectionUtil.partition(values, AbstractDataManager.MAX_ENTRIES_IN_CLAUSE); + } + // This method is needed because we have a different way of querying list and single objects via MyBatis. // Querying lists wraps the object in a ListQueryParameterObject public InternalVariableInstanceQueryImpl getParameter() { @@ -304,7 +327,7 @@ public boolean isRetained(VariableInstanceEntity entity, InternalVariableInstanc return false; } - if (param.scopeIds != null && !param.scopeIds.contains(entity.getScopeId())) { + if (param.scopeIds != null && !param.scopeIds.isEmpty() && !param.scopeIds.contains(entity.getScopeId())) { return false; } @@ -324,7 +347,7 @@ public boolean isRetained(VariableInstanceEntity entity, InternalVariableInstanc return false; } - if (param.subScopeIds != null && !param.subScopeIds.contains(entity.getSubScopeId())) { + if (param.subScopeIds != null && !param.subScopeIds.isEmpty() && !param.subScopeIds.contains(entity.getSubScopeId())) { return false; } diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java index f580f2de1b1..18da8a20bb8 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java @@ -372,6 +372,10 @@ public Set getTaskIds() { return taskIds; } + public List> getSafeTaskIds() { + return getSafeList(taskIds); + } + public String getExecutionId() { return executionId; } @@ -380,6 +384,10 @@ public Set getExecutionIds() { return executionIds; } + public List> getSafeExecutionIds() { + return getSafeList(executionIds); + } + public boolean isExcludeTaskRelated() { return excludeTaskRelated; } diff --git a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml index 6b80b43e7ad..de8372787ee 100644 --- a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml +++ b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml @@ -421,10 +421,14 @@ and RES.EXECUTION_ID_ = #{executionId, jdbcType=NVARCHAR} - and RES.EXECUTION_ID_ in - - #{executionId, jdbcType=NVARCHAR} + and ( + + RES.EXECUTION_ID_ in + + #{executionId, jdbcType=NVARCHAR} + + ) and RES.TASK_ID_ = #{taskId, jdbcType=NVARCHAR} diff --git a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml index 72d6bc3b7f0..9ca9de4b886 100644 --- a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml +++ b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml @@ -196,11 +196,15 @@ AND TASK_ID_ = #{parameter.taskId, jdbcType=NVARCHAR} - - AND TASK_ID_ in - - #{taskId, jdbcType=NVARCHAR} + + AND ( + + TASK_ID_ in + + #{taskId, jdbcType=NVARCHAR} + + ) AND PROC_INST_ID_ = #{parameter.processInstanceId, jdbcType=NVARCHAR} @@ -208,11 +212,15 @@ AND EXECUTION_ID_ = #{parameter.executionId, jdbcType=NVARCHAR} - - AND EXECUTION_ID_ in - - #{executionId, jdbcType=NVARCHAR} + + AND ( + + EXECUTION_ID_ in + + #{executionId, jdbcType=NVARCHAR} + + ) AND TASK_ID_ is null @@ -221,19 +229,27 @@ AND SCOPE_ID_ = #{parameter.scopeId, jdbcType=NVARCHAR} - AND SCOPE_ID_ in - - #{scopeId, jdbcType=NVARCHAR} + AND ( + + SCOPE_ID_ in + + #{scopeId, jdbcType=NVARCHAR} + + ) AND SUB_SCOPE_ID_ = #{parameter.subScopeId, jdbcType=NVARCHAR} - AND SUB_SCOPE_ID_ in - - #{subScopeId, jdbcType=NVARCHAR} + AND ( + + SUB_SCOPE_ID_ in + + #{subScopeId, jdbcType=NVARCHAR} + + ) AND SUB_SCOPE_ID_ is null @@ -287,19 +303,27 @@ and RES.EXECUTION_ID_ = #{executionId, jdbcType=NVARCHAR} - and RES.EXECUTION_ID_ in - - #{executionId, jdbcType=NVARCHAR} + and ( + + RES.EXECUTION_ID_ in + + #{executionId, jdbcType=NVARCHAR} + + ) and RES.TASK_ID_ = #{taskId, jdbcType=NVARCHAR} - and RES.TASK_ID_ in - - #{taskId, jdbcType=NVARCHAR} + and ( + + RES.TASK_ID_ in + + #{taskId, jdbcType=NVARCHAR} + + ) and RES.TASK_ID_ is NULL