From b1ab9a717a4c1576f6f43ca40782f07620eaeed1 Mon Sep 17 00:00:00 2001 From: BertMatthys <84137009+BertMatthys@users.noreply.github.com> Date: Mon, 19 Feb 2024 10:41:59 +0100 Subject: [PATCH 1/7] Make sure NotFoundException for decision execution is thrown before possible NPE in restApiInterceptor --- .../api/history/BaseHistoricDecisionExecutionResource.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/flowable-dmn-rest/src/main/java/org/flowable/dmn/rest/service/api/history/BaseHistoricDecisionExecutionResource.java b/modules/flowable-dmn-rest/src/main/java/org/flowable/dmn/rest/service/api/history/BaseHistoricDecisionExecutionResource.java index e6a53797bf2..97680b56100 100644 --- a/modules/flowable-dmn-rest/src/main/java/org/flowable/dmn/rest/service/api/history/BaseHistoricDecisionExecutionResource.java +++ b/modules/flowable-dmn-rest/src/main/java/org/flowable/dmn/rest/service/api/history/BaseHistoricDecisionExecutionResource.java @@ -51,13 +51,14 @@ protected DmnHistoricDecisionExecution getHistoricDecisionExecutionFromRequest(S DmnHistoricDecisionExecution decisionExecution = historicDecisionExecutionQuery.singleResult(); + if (decisionExecution == null) { + throw new FlowableObjectNotFoundException("Could not find a decision execution with id '" + decisionExecutionId + "'"); + } + if (restApiInterceptor != null) { restApiInterceptor.accessDecisionHistoryInfoById(decisionExecution); } - if (decisionExecution == null) { - throw new FlowableObjectNotFoundException("Could not find a decision execution with id '" + decisionExecutionId + "'"); - } return decisionExecution; } From 5928971ae2fe38cd36516d2645070395a83dc93a Mon Sep 17 00:00:00 2001 From: Tijs Rademakers Date: Mon, 19 Feb 2024 12:22:20 +0100 Subject: [PATCH 2/7] Add support for local variables for stages for case instance migration --- ...eToAvailablePlanItemDefinitionMapping.java | 18 ++ .../PlanItemDefinitionMappingBuilder.java | 6 + .../runtime/ChangePlanItemStateBuilder.java | 6 + .../engine/impl/agenda/CmmnEngineAgenda.java | 2 + .../impl/agenda/DefaultCmmnEngineAgenda.java | 8 + .../AbstractEvaluationCriteriaOperation.java | 16 +- .../operation/EvaluateCriteriaOperation.java | 14 +- .../CaseInstanceMigrationManagerImpl.java | 2 +- .../entity/PlanItemInstanceEntity.java | 3 + .../entity/PlanItemInstanceEntityImpl.java | 11 + .../AbstractCmmnDynamicStateManager.java | 12 +- .../ChangePlanItemStateBuilderImpl.java | 6 + .../engine/interceptor/MigrationContext.java | 9 + .../migration/CaseInstanceMigrationTest.java | 263 +++++++++++++++++- .../stage-local-variables-extra-task.cmmn.xml | 27 ++ .../migration/stage-local-variables.cmmn.xml | 25 ++ ...tition-local-variables-extra-task.cmmn.xml | 42 +++ .../stage-repetition-local-variables.cmmn.xml | 45 +++ .../migration/task-local-variables.cmmn.xml | 9 + 19 files changed, 510 insertions(+), 14 deletions(-) create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables-extra-task.cmmn.xml create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables.cmmn.xml create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables-extra-task.cmmn.xml create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables.cmmn.xml create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/task-local-variables.cmmn.xml diff --git a/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/MoveToAvailablePlanItemDefinitionMapping.java b/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/MoveToAvailablePlanItemDefinitionMapping.java index 90101f14c96..dd8b8e1145c 100644 --- a/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/MoveToAvailablePlanItemDefinitionMapping.java +++ b/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/MoveToAvailablePlanItemDefinitionMapping.java @@ -12,9 +12,27 @@ */ package org.flowable.cmmn.api.migration; +import java.util.LinkedHashMap; +import java.util.Map; + public class MoveToAvailablePlanItemDefinitionMapping extends PlanItemDefinitionMapping { + + protected Map withLocalVariables = new LinkedHashMap<>(); public MoveToAvailablePlanItemDefinitionMapping(String planItemDefinitionId) { super(planItemDefinitionId); } + + public MoveToAvailablePlanItemDefinitionMapping(String planItemDefinitionId, Map withLocalVariables) { + super(planItemDefinitionId); + this.withLocalVariables = withLocalVariables; + } + + public Map getWithLocalVariables() { + return withLocalVariables; + } + + public void setWithLocalVariables(Map withLocalVariables) { + this.withLocalVariables = withLocalVariables; + } } diff --git a/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/PlanItemDefinitionMappingBuilder.java b/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/PlanItemDefinitionMappingBuilder.java index 190a92c4d0b..0bc49afde71 100644 --- a/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/PlanItemDefinitionMappingBuilder.java +++ b/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/migration/PlanItemDefinitionMappingBuilder.java @@ -38,6 +38,12 @@ public static MoveToAvailablePlanItemDefinitionMapping createMoveToAvailablePlan return new MoveToAvailablePlanItemDefinitionMapping(planItemDefinitionId); } + public static MoveToAvailablePlanItemDefinitionMapping createMoveToAvailablePlanItemDefinitionMappingFor( + String planItemDefinitionId, Map withLocalVariables) { + + return new MoveToAvailablePlanItemDefinitionMapping(planItemDefinitionId, withLocalVariables); + } + public static WaitingForRepetitionPlanItemDefinitionMapping createWaitingForRepetitionPlanItemDefinitionMappingFor(String planItemDefinitionId) { return new WaitingForRepetitionPlanItemDefinitionMapping(planItemDefinitionId); } diff --git a/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/runtime/ChangePlanItemStateBuilder.java b/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/runtime/ChangePlanItemStateBuilder.java index d361ef8791c..232960515d7 100644 --- a/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/runtime/ChangePlanItemStateBuilder.java +++ b/modules/flowable-cmmn-api/src/main/java/org/flowable/cmmn/api/runtime/ChangePlanItemStateBuilder.java @@ -16,6 +16,7 @@ import java.util.Map; import org.flowable.cmmn.api.migration.ActivatePlanItemDefinitionMapping; +import org.flowable.cmmn.api.migration.MoveToAvailablePlanItemDefinitionMapping; import org.flowable.common.engine.api.FlowableException; import org.flowable.common.engine.api.FlowableObjectNotFoundException; @@ -63,6 +64,11 @@ public interface ChangePlanItemStateBuilder { */ ChangePlanItemStateBuilder changeToAvailableStateByPlanItemDefinitionIds(List planItemDefinitionIds); + /** + * Set a plan item to available state by definition mapping. + */ + ChangePlanItemStateBuilder changeToAvailableStateByPlanItemDefinition(MoveToAvailablePlanItemDefinitionMapping planItemDefinitionMapping); + /** * Terminate a plan item by definition id without terminating another plan item instance. */ diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/CmmnEngineAgenda.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/CmmnEngineAgenda.java index a9f8debf953..5423f6eff63 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/CmmnEngineAgenda.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/CmmnEngineAgenda.java @@ -92,6 +92,8 @@ public interface CmmnEngineAgenda extends Agenda { void planEvaluateCriteriaOperation(String caseInstanceEntityId, PlanItemLifeCycleEvent lifeCycleEvent); + void planEvaluateCriteriaOperation(String caseInstanceEntityId, MigrationContext migrationContext); + void planEvaluateVariableEventListenersOperation(String caseInstanceEntityId); } diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java index ac9ce880e92..66aae05a5fd 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/DefaultCmmnEngineAgenda.java @@ -144,6 +144,14 @@ public void planEvaluateCriteriaOperation(String caseInstanceEntityId, PlanItemL internalPlanEvaluateCriteria(caseInstanceEntityId, lifeCycleEvent, false); } + @Override + public void planEvaluateCriteriaOperation(String caseInstanceEntityId, MigrationContext migrationContext) { + EvaluateCriteriaOperation evaluateCriteriaOperation = new EvaluateCriteriaOperation(commandContext, caseInstanceEntityId, null); + evaluateCriteriaOperation.setEvaluateStagesAndCaseInstanceCompletion(false); + evaluateCriteriaOperation.setMigrationContext(migrationContext); + addOperation(evaluateCriteriaOperation); + } + protected void internalPlanEvaluateCriteria(String caseInstanceEntityId, PlanItemLifeCycleEvent planItemLifeCycleEvent, boolean evaluateCaseInstanceCompleted) { EvaluateCriteriaOperation evaluateCriteriaOperation = new EvaluateCriteriaOperation(commandContext, caseInstanceEntityId, planItemLifeCycleEvent); evaluateCriteriaOperation.setEvaluateStagesAndCaseInstanceCompletion(evaluateCaseInstanceCompleted); diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java index 79eadcd96d2..152be2b31b2 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/AbstractEvaluationCriteriaOperation.java @@ -44,6 +44,7 @@ import org.flowable.cmmn.engine.impl.util.ExpressionUtil; import org.flowable.cmmn.engine.impl.util.PlanItemInstanceContainerUtil; import org.flowable.cmmn.engine.impl.util.PlanItemInstanceUtil; +import org.flowable.cmmn.engine.interceptor.MigrationContext; import org.flowable.cmmn.model.Criterion; import org.flowable.cmmn.model.EventListener; import org.flowable.cmmn.model.HasExitCriteria; @@ -107,6 +108,7 @@ public void evaluateForActivation(PlanItemInstanceEntity planItemInstanceEntity, // if we need to activate the plan item, mark the result as some criteria changed and plan the activation of the plan item by adding // this as an operation to the agenda if (activatePlanItemInstance) { + planItemInstanceEntity.setPlannedForActivationInMigration(true); evaluationResult.markCriteriaChanged(); CommandContextUtil.getAgenda(commandContext) .planActivatePlanItemInstanceOperation(planItemInstanceEntity, satisfiedEntryCriterion != null ? satisfiedEntryCriterion.getId() : null); @@ -144,7 +146,7 @@ public boolean evaluateForCompletion(PlanItemInstanceEntity planItemInstanceEnti } else if (planItem.getPlanItemDefinition() instanceof Stage) { if (PlanItemInstanceState.ACTIVE.equals(state)) { - boolean criteriaChangeOrActiveChildrenForStage = evaluatePlanItemsCriteria(planItemInstanceEntity); + boolean criteriaChangeOrActiveChildrenForStage = evaluatePlanItemsCriteria(planItemInstanceEntity, null); if (criteriaChangeOrActiveChildrenForStage) { evaluationResult.markCriteriaChanged(); planItemInstanceEntity.setCompletable(false); // an active child = stage cannot be completed anymore @@ -189,15 +191,21 @@ public boolean evaluateForCompletion(PlanItemInstanceEntity planItemInstanceEnti * Returns false if no sentry changes happened and none of the passed plan item instances are active. * This means that the parent of these plan item instances also now can change its state. */ - protected boolean evaluatePlanItemsCriteria(PlanItemInstanceContainer planItemInstanceContainer) { + protected boolean evaluatePlanItemsCriteria(PlanItemInstanceContainer planItemInstanceContainer, MigrationContext migrationContext) { List planItemInstances = planItemInstanceContainer.getChildPlanItemInstances(); // needed because when doing case instance migration the child plan item instances can be null - if (planItemInstances == null && planItemInstanceContainer instanceof CaseInstanceEntity) { + if ((planItemInstances == null || (migrationContext != null && migrationContext.isFetchPlanItemInstances())) && + planItemInstanceContainer instanceof CaseInstanceEntity) { + PlanItemInstanceEntityManager planItemInstanceEntityManager = CommandContextUtil.getPlanItemInstanceEntityManager(commandContext); CaseInstanceEntity caseInstance = (CaseInstanceEntity) planItemInstanceContainer; planItemInstances = planItemInstanceEntityManager.findByCaseInstanceId(caseInstance.getId()); planItemInstanceContainer.setChildPlanItemInstances(planItemInstances); + + if (migrationContext != null && migrationContext.isFetchPlanItemInstances()) { + migrationContext.setFetchPlanItemInstances(false); + } } // create an evaluation result object, holding all evaluation results as well as a list of newly created child plan items, as to avoid concurrent @@ -212,7 +220,7 @@ protected boolean evaluatePlanItemsCriteria(PlanItemInstanceContainer planItemIn String state = planItemInstanceEntity.getState(); // check, if the plan item is in an evaluation state (e.g. available or waiting for repetition) to check for its activation - if (PlanItemInstanceState.EVALUATE_ENTRY_CRITERIA_STATES.contains(state)) { + if (PlanItemInstanceState.EVALUATE_ENTRY_CRITERIA_STATES.contains(state) && !planItemInstanceEntity.isPlannedForActivationInMigration()) { evaluateForActivation(planItemInstanceEntity, planItemInstanceContainer, evaluationResult); } diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/EvaluateCriteriaOperation.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/EvaluateCriteriaOperation.java index ec109c15779..5395af2efa4 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/EvaluateCriteriaOperation.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/agenda/operation/EvaluateCriteriaOperation.java @@ -15,6 +15,7 @@ import org.flowable.cmmn.api.runtime.CaseInstanceState; import org.flowable.cmmn.engine.impl.criteria.PlanItemLifeCycleEvent; import org.flowable.cmmn.engine.impl.util.CommandContextUtil; +import org.flowable.cmmn.engine.interceptor.MigrationContext; import org.flowable.cmmn.model.Criterion; import org.flowable.common.engine.impl.interceptor.CommandContext; import org.slf4j.Logger; @@ -27,6 +28,8 @@ public class EvaluateCriteriaOperation extends AbstractEvaluationCriteriaOperation { private static final Logger LOGGER = LoggerFactory.getLogger(EvaluateCriteriaOperation.class); + + protected MigrationContext migrationContext; public EvaluateCriteriaOperation(CommandContext commandContext, String caseInstanceEntityId) { super(commandContext, caseInstanceEntityId, null, null); @@ -53,7 +56,7 @@ public void run() { satisfiedExitCriterion.getExitType(), satisfiedExitCriterion.getExitEventType()); } else { - boolean criteriaChangeOrActiveChildren = evaluatePlanItemsCriteria(caseInstanceEntity); + boolean criteriaChangeOrActiveChildren = evaluatePlanItemsCriteria(caseInstanceEntity, migrationContext); if (evaluateStagesAndCaseInstanceCompletion && evaluatePlanModelComplete() && !criteriaChangeOrActiveChildren @@ -91,5 +94,12 @@ public String toString() { return stringBuilder.toString(); } - + + public MigrationContext getMigrationContext() { + return migrationContext; + } + + public void setMigrationContext(MigrationContext migrationContext) { + this.migrationContext = migrationContext; + } } diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/migration/CaseInstanceMigrationManagerImpl.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/migration/CaseInstanceMigrationManagerImpl.java index 2b1dde9173a..ec189f7ef67 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/migration/CaseInstanceMigrationManagerImpl.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/migration/CaseInstanceMigrationManagerImpl.java @@ -371,7 +371,7 @@ protected ChangePlanItemStateBuilderImpl prepareChangeStateBuilder(CaseInstance } for (MoveToAvailablePlanItemDefinitionMapping planItemDefinitionMapping : document.getMoveToAvailablePlanItemDefinitionMappings()) { - changePlanItemStateBuilder.changeToAvailableStateByPlanItemDefinitionId(planItemDefinitionMapping.getPlanItemDefinitionId()); + changePlanItemStateBuilder.changeToAvailableStateByPlanItemDefinition(planItemDefinitionMapping); } for (WaitingForRepetitionPlanItemDefinitionMapping planItemDefinitionMapping : document.getWaitingForRepetitionPlanItemDefinitionMappings()) { diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java index b4dc7a33c03..0ab87907f19 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntity.java @@ -26,4 +26,7 @@ public interface PlanItemInstanceEntity extends Entity, HasRevision, DelegatePla VariableScope getParentVariableScope(); + boolean isPlannedForActivationInMigration(); + + void setPlannedForActivationInMigration(boolean plannedForActivationInMigration); } diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java index c6cb0be4463..94df68d8934 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/persistence/entity/PlanItemInstanceEntityImpl.java @@ -92,6 +92,7 @@ public class PlanItemInstanceEntityImpl extends AbstractCmmnEngineVariableScopeE protected PlanItemInstanceLifecycleListener currentLifecycleListener; // Only set when executing an plan item lifecycle listener protected FlowableListener currentFlowableListener; // Only set when executing an plan item lifecycle listener + protected boolean plannedForActivationInMigration; public PlanItemInstanceEntityImpl() { } @@ -646,6 +647,16 @@ public void setLocalizedName(String localizedName) { this.localizedName = localizedName; } + @Override + public boolean isPlannedForActivationInMigration() { + return plannedForActivationInMigration; + } + + @Override + public void setPlannedForActivationInMigration(boolean plannedForActivationInMigration) { + this.plannedForActivationInMigration = plannedForActivationInMigration; + } + @Override public String toString() { StringBuilder stringBuilder = new StringBuilder(); diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/AbstractCmmnDynamicStateManager.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/AbstractCmmnDynamicStateManager.java index 9ad975fb52e..338b632e2dc 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/AbstractCmmnDynamicStateManager.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/AbstractCmmnDynamicStateManager.java @@ -147,7 +147,9 @@ protected void doMovePlanItemState(CaseInstanceChangeState caseInstanceChangeSta executeRemoveWaitingForRepetitionPlanItemInstances(caseInstanceChangeState, caseInstance, commandContext); CmmnEngineAgenda agenda = CommandContextUtil.getAgenda(commandContext); - agenda.planEvaluateCriteriaOperation(caseInstance.getId()); + MigrationContext migrationContext = new MigrationContext(); + migrationContext.setFetchPlanItemInstances(true); + agenda.planEvaluateCriteriaOperation(caseInstance.getId(), migrationContext); } protected void executeChangePlanItemIds(CaseInstanceChangeState caseInstanceChangeState, String originalCaseDefinitionId, CommandContext commandContext) { @@ -228,6 +230,10 @@ protected void executeActivatePlanItemInstances(CaseInstanceChangeState caseInst PlanItemInstanceEntity newPlanItemInstance = createStagesAndPlanItemInstances(planItem, caseInstance, caseInstanceChangeState, commandContext); + + if (planItemDefinitionMapping.getWithLocalVariables() != null && !planItemDefinitionMapping.getWithLocalVariables().isEmpty()) { + newPlanItemInstance.setVariablesLocal(planItemDefinitionMapping.getWithLocalVariables()); + } CmmnEngineAgenda agenda = CommandContextUtil.getAgenda(commandContext); if (planItemDefinitionMapping.getNewAssignee() != null && planItem.getPlanItemDefinition() instanceof HumanTask) { @@ -316,6 +322,10 @@ protected void executeChangePlanItemInstancesToAvailableState(CaseInstanceChange caseInstanceChangeState.addCreatedStageInstance(planItemDefinitionMapping.getPlanItemDefinitionId(), availablePlanItemInstance); } + if (planItemDefinitionMapping.getWithLocalVariables() != null && !planItemDefinitionMapping.getWithLocalVariables().isEmpty()) { + availablePlanItemInstance.setVariablesLocal(planItemDefinitionMapping.getWithLocalVariables()); + } + CmmnHistoryManager cmmnHistoryManager = cmmnEngineConfiguration.getCmmnHistoryManager(); cmmnHistoryManager.recordPlanItemInstanceCreated(availablePlanItemInstance); diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/ChangePlanItemStateBuilderImpl.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/ChangePlanItemStateBuilderImpl.java index 71e4f04a9ec..72205d50e02 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/ChangePlanItemStateBuilderImpl.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/runtime/ChangePlanItemStateBuilderImpl.java @@ -103,6 +103,12 @@ public ChangePlanItemStateBuilder changeToAvailableStateByPlanItemDefinitionIds( return this; } + @Override + public ChangePlanItemStateBuilder changeToAvailableStateByPlanItemDefinition(MoveToAvailablePlanItemDefinitionMapping planItemDefinitionMapping) { + changeToAvailableStatePlanItemDefinitions.add(planItemDefinitionMapping); + return this; + } + @Override public ChangePlanItemStateBuilder terminatePlanItemDefinitionId(String planItemDefinitionId) { terminatePlanItemDefinitions.add(new TerminatePlanItemDefinitionMapping(planItemDefinitionId)); diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/interceptor/MigrationContext.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/interceptor/MigrationContext.java index babe86db66c..531701e69e9 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/interceptor/MigrationContext.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/interceptor/MigrationContext.java @@ -14,7 +14,16 @@ public class MigrationContext { + protected boolean fetchPlanItemInstances; protected String assignee; + + public boolean isFetchPlanItemInstances() { + return fetchPlanItemInstances; + } + + public void setFetchPlanItemInstances(boolean fetchPlanItemInstances) { + this.fetchPlanItemInstances = fetchPlanItemInstances; + } public String getAssignee() { return assignee; diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/migration/CaseInstanceMigrationTest.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/migration/CaseInstanceMigrationTest.java index a7a4c39ff06..dc258466912 100644 --- a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/migration/CaseInstanceMigrationTest.java +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/migration/CaseInstanceMigrationTest.java @@ -404,18 +404,18 @@ void withTwoTasksIntroducingANewStageAroundSecondTask() { assertThat(planItem2.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); assertThat(planItem2.getName()).isEqualTo("Task 2"); assertThat(planItem2.getStageInstanceId()).isNotNull(); - assertThat(planItem2.getState()).isEqualTo(PlanItemInstanceState.AVAILABLE); + assertThat(planItem2.getState()).isEqualTo(PlanItemInstanceState.ACTIVE); PlanItemInstance planItem3 = planItemsByElementId.get("planItem3").get(0); assertThat(planItem3.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); assertThat(planItem3.getPlanItemDefinitionId()).isEqualTo("expandedStage1"); assertThat(planItem3.getState()).isEqualTo(PlanItemInstanceState.AVAILABLE); - Task task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).singleResult(); + Task task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("humanTask1").singleResult(); assertThat(task.getTaskDefinitionKey()).isEqualTo("humanTask1"); assertThat(task.getScopeDefinitionId()).isEqualTo(destinationDefinition.getId()); cmmnTaskService.complete(task.getId()); - task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).singleResult(); + task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("humanTask2").singleResult(); assertThat(task.getTaskDefinitionKey()).isEqualTo("humanTask2"); assertThat(task.getScopeDefinitionId()).isEqualTo(destinationDefinition.getId()); cmmnTaskService.complete(task.getId()); @@ -4395,10 +4395,261 @@ void activatePlanItemWithCompletedStage() { assertThat(historicPlanItemInstances.size()).isEqualTo(1); assertThat(historicPlanItemInstances.get(0).getState()).isEqualTo("completed"); } + + @Test + void migrateCaseInstancesWithLocalVariablesForStage() { + // Arrange + deployCaseDefinition("test1", "org/flowable/cmmn/test/migration/stage-local-variables.cmmn.xml"); + CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("example-stage-case").start(); + CaseDefinition destinationDefinition = deployCaseDefinition("test1", "org/flowable/cmmn/test/migration/stage-local-variables-extra-task.cmmn.xml"); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + + Map localStageVarMap = new HashMap<>(); + localStageVarMap.put("stageNr", 1); + + // Act + cmmnMigrationService.createCaseInstanceMigrationBuilder() + .migrateToCaseDefinition(destinationDefinition.getId()) + .addActivatePlanItemDefinitionMapping(PlanItemDefinitionMappingBuilder.createActivatePlanItemDefinitionMappingFor("extra-task")) + .addActivatePlanItemDefinitionMapping(PlanItemDefinitionMappingBuilder.createActivatePlanItemDefinitionMappingFor("stage", null, localStageVarMap)) + .migrateCaseInstances(caseInstance.getCaseDefinitionId()); + + // Assert + CaseInstance caseInstanceAfterMigration = cmmnRuntimeService.createCaseInstanceQuery() + .caseInstanceId(caseInstance.getId()) + .singleResult(); + assertThat(caseInstanceAfterMigration.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); + List planItemInstances = cmmnRuntimeService.createPlanItemInstanceQuery() + .caseInstanceId(caseInstance.getId()) + .includeEnded() + .list(); + assertThat(planItemInstances).hasSize(4); + assertThat(planItemInstances) + .extracting(PlanItemInstance::getCaseDefinitionId) + .containsOnly(destinationDefinition.getId()); + assertThat(planItemInstances) + .extracting(PlanItemInstance::getName) + .containsExactlyInAnyOrder("Start Task", "Stage", "Stage Task 1", "Extra Task"); + assertThat(planItemInstances) + .filteredOn("name", "Start Task") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(planItemInstances) + .filteredOn("name", "Stage") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(planItemInstances) + .filteredOn("name", "Stage Task 1") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(planItemInstances) + .filteredOn("name", "Extra Task") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(3); + + Task task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("stage-task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(2); + + task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + + task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("extra-task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnRuntimeService.createCaseInstanceQuery().caseInstanceId(caseInstance.getId()).count()).isZero(); + + if (CmmnHistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, cmmnEngineConfiguration)) { + assertThat(cmmnHistoryService.createHistoricCaseInstanceQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + assertThat(cmmnHistoryService.createHistoricCaseInstanceQuery().caseInstanceId(caseInstance.getId()).singleResult().getCaseDefinitionId()) + .isEqualTo(destinationDefinition.getId()); + + List historicPlanItemInstances = cmmnHistoryService.createHistoricPlanItemInstanceQuery() + .planItemInstanceCaseInstanceId(caseInstance.getId()).list(); + assertThat(historicPlanItemInstances).hasSize(4); + for (HistoricPlanItemInstance historicPlanItemInstance : historicPlanItemInstances) { + assertThat(historicPlanItemInstance.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); + } + + List historicTasks = cmmnHistoryService.createHistoricTaskInstanceQuery().caseInstanceId(caseInstance.getId()).list(); + assertThat(historicTasks).hasSize(3); + for (HistoricTaskInstance historicTask : historicTasks) { + assertThat(historicTask.getScopeDefinitionId()).isEqualTo(destinationDefinition.getId()); + } + } + } + + @Test + void migrateCaseInstancesWithLocalVariablesForRepeatingStage() { + // Arrange + deployCaseDefinition("test1", "org/flowable/cmmn/test/migration/stage-repetition-local-variables.cmmn.xml"); + CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("example-stage-case").start(); + CaseDefinition destinationDefinition = deployCaseDefinition("test1", "org/flowable/cmmn/test/migration/stage-repetition-local-variables-extra-task.cmmn.xml"); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(2); + + Map localStageVarMap = new HashMap<>(); + localStageVarMap.put("stageNr", 1); + + // Act + cmmnMigrationService.createCaseInstanceMigrationBuilder() + .migrateToCaseDefinition(destinationDefinition.getId()) + .addActivatePlanItemDefinitionMapping(PlanItemDefinitionMappingBuilder.createActivatePlanItemDefinitionMappingFor("extra-task")) + .addActivatePlanItemDefinitionMapping(PlanItemDefinitionMappingBuilder.createActivatePlanItemDefinitionMappingFor("repeating-stage", null, localStageVarMap)) + .migrateCaseInstances(caseInstance.getCaseDefinitionId()); + + // Assert + CaseInstance caseInstanceAfterMigration = cmmnRuntimeService.createCaseInstanceQuery() + .caseInstanceId(caseInstance.getId()) + .singleResult(); + assertThat(caseInstanceAfterMigration.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); + List planItemInstances = cmmnRuntimeService.createPlanItemInstanceQuery() + .caseInstanceId(caseInstance.getId()) + .includeEnded() + .list(); + assertThat(planItemInstances).hasSize(6); + assertThat(planItemInstances) + .extracting(PlanItemInstance::getCaseDefinitionId) + .containsOnly(destinationDefinition.getId()); + assertThat(planItemInstances) + .extracting(PlanItemInstance::getName) + .containsExactlyInAnyOrder("Exit Task", "Start Repeating Task", "Repeating Stage", "Repeating Stage", "Stage repeating Task 1", "Extra Task"); + assertThat(planItemInstances) + .filteredOn("name", "Start Repeating Task") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(planItemInstances) + .filteredOn("name", "Repeating Stage") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE, PlanItemInstanceState.WAITING_FOR_REPETITION); + + assertThat(planItemInstances) + .filteredOn("name", "Stage repeating Task 1") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(planItemInstances) + .filteredOn("name", "Extra Task") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(4); + + Task task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("stage-repeating-task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(3); + + task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("exit-task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnRuntimeService.createCaseInstanceQuery().caseInstanceId(caseInstance.getId()).count()).isZero(); + + if (CmmnHistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, cmmnEngineConfiguration)) { + assertThat(cmmnHistoryService.createHistoricCaseInstanceQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + assertThat(cmmnHistoryService.createHistoricCaseInstanceQuery().caseInstanceId(caseInstance.getId()).singleResult().getCaseDefinitionId()) + .isEqualTo(destinationDefinition.getId()); + + List historicPlanItemInstances = cmmnHistoryService.createHistoricPlanItemInstanceQuery() + .planItemInstanceCaseInstanceId(caseInstance.getId()).list(); + assertThat(historicPlanItemInstances).hasSize(6); + for (HistoricPlanItemInstance historicPlanItemInstance : historicPlanItemInstances) { + assertThat(historicPlanItemInstance.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); + } + + List historicTasks = cmmnHistoryService.createHistoricTaskInstanceQuery().caseInstanceId(caseInstance.getId()).list(); + assertThat(historicTasks).hasSize(4); + for (HistoricTaskInstance historicTask : historicTasks) { + assertThat(historicTask.getScopeDefinitionId()).isEqualTo(destinationDefinition.getId()); + } + } + } + + @Test + void migrateCaseInstancesWithLocalVariablesForAvailableStage() { + // Arrange + deployCaseDefinition("test1", "org/flowable/cmmn/test/migration/task-local-variables.cmmn.xml"); + CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("example-stage-case").start(); + CaseDefinition destinationDefinition = deployCaseDefinition("test1", "org/flowable/cmmn/test/migration/stage-local-variables.cmmn.xml"); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + + Map localStageVarMap = new HashMap<>(); + localStageVarMap.put("stageNr", 1); + + // Act + cmmnMigrationService.createCaseInstanceMigrationBuilder() + .migrateToCaseDefinition(destinationDefinition.getId()) + .addMoveToAvailablePlanItemDefinitionMapping(PlanItemDefinitionMappingBuilder.createMoveToAvailablePlanItemDefinitionMappingFor("stage", localStageVarMap)) + .migrateCaseInstances(caseInstance.getCaseDefinitionId()); - // with sentries - // with stages - // with new expected case variables + // Assert + CaseInstance caseInstanceAfterMigration = cmmnRuntimeService.createCaseInstanceQuery() + .caseInstanceId(caseInstance.getId()) + .singleResult(); + assertThat(caseInstanceAfterMigration.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); + List planItemInstances = cmmnRuntimeService.createPlanItemInstanceQuery() + .caseInstanceId(caseInstance.getId()) + .includeEnded() + .list(); + assertThat(planItemInstances).hasSize(2); + assertThat(planItemInstances) + .extracting(PlanItemInstance::getCaseDefinitionId) + .containsOnly(destinationDefinition.getId()); + assertThat(planItemInstances) + .extracting(PlanItemInstance::getName) + .containsExactlyInAnyOrder("Start Task", "Stage"); + assertThat(planItemInstances) + .filteredOn("name", "Start Task") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.ACTIVE); + + assertThat(planItemInstances) + .filteredOn("name", "Stage") + .extracting(PlanItemInstance::getState) + .containsOnly(PlanItemInstanceState.AVAILABLE); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + + Task task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + + task = cmmnTaskService.createTaskQuery().caseInstanceId(caseInstance.getId()).taskDefinitionKey("stage-task").singleResult(); + cmmnTaskService.complete(task.getId()); + + assertThat(cmmnRuntimeService.createCaseInstanceQuery().caseInstanceId(caseInstance.getId()).count()).isZero(); + + if (CmmnHistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, cmmnEngineConfiguration)) { + assertThat(cmmnHistoryService.createHistoricCaseInstanceQuery().caseInstanceId(caseInstance.getId()).count()).isEqualTo(1); + assertThat(cmmnHistoryService.createHistoricCaseInstanceQuery().caseInstanceId(caseInstance.getId()).singleResult().getCaseDefinitionId()) + .isEqualTo(destinationDefinition.getId()); + + List historicPlanItemInstances = cmmnHistoryService.createHistoricPlanItemInstanceQuery() + .planItemInstanceCaseInstanceId(caseInstance.getId()).list(); + assertThat(historicPlanItemInstances).hasSize(3); + for (HistoricPlanItemInstance historicPlanItemInstance : historicPlanItemInstances) { + assertThat(historicPlanItemInstance.getCaseDefinitionId()).isEqualTo(destinationDefinition.getId()); + } + + List historicTasks = cmmnHistoryService.createHistoricTaskInstanceQuery().caseInstanceId(caseInstance.getId()).list(); + assertThat(historicTasks).hasSize(2); + for (HistoricTaskInstance historicTask : historicTasks) { + assertThat(historicTask.getScopeDefinitionId()).isEqualTo(destinationDefinition.getId()); + } + } + } protected class CustomTenantProvider implements DefaultTenantProvider { diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables-extra-task.cmmn.xml b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables-extra-task.cmmn.xml new file mode 100644 index 00000000000..3394cee14ae --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables-extra-task.cmmn.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + complete + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables.cmmn.xml b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables.cmmn.xml new file mode 100644 index 00000000000..518ea058f8e --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-local-variables.cmmn.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + complete + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables-extra-task.cmmn.xml b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables-extra-task.cmmn.xml new file mode 100644 index 00000000000..f026e045bc3 --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables-extra-task.cmmn.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + complete + + + + + complete + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables.cmmn.xml b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables.cmmn.xml new file mode 100644 index 00000000000..03187f0f013 --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/stage-repetition-local-variables.cmmn.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + + + + complete + + + + + complete + + + + + + + + + + + + complete + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/task-local-variables.cmmn.xml b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/task-local-variables.cmmn.xml new file mode 100644 index 00000000000..cb40e1d86f6 --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/migration/task-local-variables.cmmn.xml @@ -0,0 +1,9 @@ + + + + + + + + + \ No newline at end of file From f9e3e1e09ec0e1520de2a7e86fe2b7d71b696288 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 22 Feb 2024 08:14:56 +0100 Subject: [PATCH 3/7] Bump org.postgresql:postgresql from 42.5.4 to 42.7.2 (#3850) Bumps [org.postgresql:postgresql](https://github.com/pgjdbc/pgjdbc) from 42.5.4 to 42.7.2. - [Release notes](https://github.com/pgjdbc/pgjdbc/releases) - [Changelog](https://github.com/pgjdbc/pgjdbc/blob/master/CHANGELOG.md) - [Commits](https://github.com/pgjdbc/pgjdbc/commits) --- updated-dependencies: - dependency-name: org.postgresql:postgresql dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index dc69cd518ae..7b9aa064b33 100644 --- a/pom.xml +++ b/pom.xml @@ -389,7 +389,7 @@ org.postgresql postgresql - 42.5.4 + 42.7.2 xerces From dc2b970df94f52556f2f29972ee3819f360dd345 Mon Sep 17 00:00:00 2001 From: Joram Barrez Date: Mon, 4 Mar 2024 12:55:44 +0100 Subject: [PATCH 4/7] Fix bug in CmmnRuntimeService#getVariable when used in nested command context, where during looping a wrongly cached variable instance is returned. More specifically: the iteration order of the hashmap changes depending on the amount of variables stored during the same transaction. --- .../flowable-cmmn-engine-configurator/pom.xml | 5 + .../AbstractProcessEngineIntegrationTest.java | 16 +- .../org/flowable/cmmn/test/VariablesTest.java | 62 +++++ ...riableThroughCmmnRuntimeService.bpmn20.xml | 230 ++++++++++++++++++ ...vingVariableThroughCmmnRuntimeService.cmmn | 35 +++ .../cmmn/engine/impl/cmd/GetVariableCmd.java | 8 +- .../cmmn/test/runtime/VariablesTest.java | 52 ++++ ...GettingMultipleTimesInSameTransaction.cmmn | 43 ++++ .../persistence/cache/EntityCacheImpl.java | 13 +- .../test/api/variables/VariablesTest.java | 58 +++++ ...gMultipleTimesInSameTransaction.bpmn20.xml | 77 ++++++ 11 files changed, 594 insertions(+), 5 deletions(-) create mode 100644 modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java create mode 100644 modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml create mode 100644 modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn create mode 100644 modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn create mode 100644 modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml diff --git a/modules/flowable-cmmn-engine-configurator/pom.xml b/modules/flowable-cmmn-engine-configurator/pom.xml index 3aee5bbd81b..cf8c0efca62 100644 --- a/modules/flowable-cmmn-engine-configurator/pom.xml +++ b/modules/flowable-cmmn-engine-configurator/pom.xml @@ -62,6 +62,11 @@ mockito-core test + + org.apache.groovy + groovy-jsr223 + test + net.javacrumbs.json-unit json-unit-assertj diff --git a/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java index ec4c05fe97e..a86a11567aa 100644 --- a/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java +++ b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/AbstractProcessEngineIntegrationTest.java @@ -15,7 +15,9 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.Date; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import org.flowable.cmmn.api.CmmnHistoryService; @@ -52,6 +54,8 @@ public abstract class AbstractProcessEngineIntegrationTest { protected static CmmnEngineConfiguration cmmnEngineConfiguration; protected static ProcessEngine processEngine; + protected static Map beans = new HashMap<>(); + protected CmmnRepositoryService cmmnRepositoryService; protected CmmnRuntimeService cmmnRuntimeService; protected CmmnTaskService cmmnTaskService; @@ -69,7 +73,9 @@ public abstract class AbstractProcessEngineIntegrationTest { @BeforeClass public static void bootProcessEngine() { if (processEngine == null) { - processEngine = ProcessEngineConfiguration.createProcessEngineConfigurationFromResource("flowable.cfg.xml").buildProcessEngine(); + ProcessEngineConfiguration processEngineConfiguration = ProcessEngineConfiguration.createProcessEngineConfigurationFromResource("flowable.cfg.xml"); + processEngineConfiguration.setBeans(beans); + processEngine = processEngineConfiguration.buildProcessEngine(); cmmnEngineConfiguration = (CmmnEngineConfiguration) processEngine.getProcessEngineConfiguration() .getEngineConfigurations().get(EngineConfigurationConstants.KEY_CMMN_ENGINE_CONFIG); CmmnTestRunner.setCmmnEngineConfiguration(cmmnEngineConfiguration); @@ -91,6 +97,12 @@ public void setupServices() { this.processEngineHistoryService = processEngine.getHistoryService(); this.processEngineConfiguration = processEngine.getProcessEngineConfiguration(); this.processEngineDynamicBpmnService = processEngine.getDynamicBpmnService(); + + beans.put("cmmnRepositoryService", cmmnEngineConfiguration.getCmmnRepositoryService()); + beans.put("cmmnRuntimeService", cmmnEngineConfiguration.getCmmnRuntimeService()); + beans.put("cmmnTaskService", cmmnEngineConfiguration.getCmmnTaskService()); + beans.put("cmmnHistoryService", cmmnEngineConfiguration.getCmmnHistoryService()); + beans.put("cmmnManagementService", cmmnEngineConfiguration.getCmmnManagementService()); } @After @@ -102,6 +114,8 @@ public void cleanup() { for (CmmnDeployment deployment : cmmnRepositoryService.createDeploymentQuery().list()) { cmmnRepositoryService.deleteDeployment(deployment.getId(), true); } + + beans.clear(); } protected Date setCmmnClockFixedToCurrentTime() { diff --git a/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java new file mode 100644 index 00000000000..29885b477ae --- /dev/null +++ b/modules/flowable-cmmn-engine-configurator/src/test/java/org/flowable/cmmn/test/VariablesTest.java @@ -0,0 +1,62 @@ +/* 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.cmmn.test; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import org.flowable.cmmn.api.runtime.CaseInstance; +import org.flowable.cmmn.engine.test.CmmnDeployment; +import org.flowable.cmmn.engine.test.impl.CmmnHistoryTestHelper; +import org.flowable.common.engine.impl.history.HistoryLevel; +import org.flowable.engine.test.Deployment; +import org.flowable.variable.api.history.HistoricVariableInstance; +import org.junit.Test; + +/** + * @author Joram Barrez + */ +public class VariablesTest extends AbstractProcessEngineIntegrationTest { + + @Test + @Deployment + @CmmnDeployment + public void testSettingAndRemovingVariableThroughCmmnRuntimeService() { + processEngineRepositoryService.createDeployment() + .addClasspathResource("org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml") + .deploy(); + + CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder() + .caseDefinitionKey("varSyncTestCase") + .variable("loopNum", 100) + .variable("round", 5) + .start(); + + // The case instance ending means all variable lookups succeeded + assertCaseInstanceEnded(caseInstance); + assertThat(cmmnRuntimeService.getVariables(caseInstance.getId())).isEmpty(); + + if (CmmnHistoryTestHelper.isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, cmmnEngineConfiguration)) { + List historicVariables = cmmnHistoryService.createHistoricVariableInstanceQuery() + .caseInstanceId(caseInstance.getId()).list(); + + // The variables are recreated each loop with a non-null value + assertThat(historicVariables).hasSize(102); // 100 from the variables and 2 for round and loopNum + for (HistoricVariableInstance historicVariable : historicVariables) { + assertThat(historicVariable.getValue()).isNotNull(); + } + } + } + +} diff --git a/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml new file mode 100644 index 00000000000..78909e1ab08 --- /dev/null +++ b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.bpmn20.xml @@ -0,0 +1,230 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0}]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn new file mode 100644 index 00000000000..c5abe5afb98 --- /dev/null +++ b/modules/flowable-cmmn-engine-configurator/src/test/resources/org/flowable/cmmn/test/VariablesTest.testSettingAndRemovingVariableThroughCmmnRuntimeService.cmmn @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java index 71e5cb47885..1457bd0e9f4 100644 --- a/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java +++ b/modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/cmd/GetVariableCmd.java @@ -38,8 +38,12 @@ public Object execute(CommandContext commandContext) { if (caseInstanceId == null) { throw new FlowableIllegalArgumentException("caseInstanceId is null"); } - + CmmnEngineConfiguration cmmnEngineConfiguration = CommandContextUtil.getCmmnEngineConfiguration(commandContext); + + // In the BPMN engine, this is done by getting the variable on the execution. + // However, doing the same in CMMN will fetch the case instance and non-completed plan item instances in one query. + // Hence, why here a direct query is done here (which is cached). VariableInstanceEntity variableInstanceEntity = cmmnEngineConfiguration.getVariableServiceConfiguration().getVariableService() .createInternalVariableInstanceQuery() .scopeId(caseInstanceId) @@ -49,7 +53,7 @@ public Object execute(CommandContext commandContext) { .singleResult(); if (variableInstanceEntity != null) { return variableInstanceEntity.getValue(); - } + } return null; } diff --git a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java index 84644bf60d2..84b690dc365 100644 --- a/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java +++ b/modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/runtime/VariablesTest.java @@ -31,6 +31,7 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; +import org.flowable.cmmn.api.CmmnRuntimeService; import org.flowable.cmmn.api.delegate.DelegatePlanItemInstance; import org.flowable.cmmn.api.delegate.PlanItemJavaDelegate; import org.flowable.cmmn.api.history.HistoricMilestoneInstance; @@ -881,6 +882,22 @@ public void testUpdateMetaInfo() { } + @Test + @CmmnDeployment + public void testSettingGettingMultipleTimesInSameTransaction() { + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = true; + CaseInstance caseInstance1 = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("testSettingGettingMultipleTimesInSameTransaction").start(); + assertThat(cmmnRuntimeService.getVariables(caseInstance1.getId())).isEmpty(); + + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = false; + CaseInstance caseInstance2 = cmmnRuntimeService.createCaseInstanceBuilder().caseDefinitionKey("testSettingGettingMultipleTimesInSameTransaction").start(); + Map variables = cmmnRuntimeService.getVariables(caseInstance2.getId()); + assertThat(variables).hasSize(100); + for (String variableName : variables.keySet()) { + assertThat(variables.get(variableName)).isNotNull(); + } + } + protected void addVariableTypeIfNotExists(VariableType variableType) { // We can't remove the VariableType after every test since it would cause the test // to fail due to not being able to get the variable value during deleting @@ -1062,4 +1079,39 @@ public CustomTestVariable(String someValue, int someInt) { } } + public static class TestSetGetVariablesDelegate implements PlanItemJavaDelegate { + + public static boolean REMOVE_VARS_IN_LAST_ROUND = true; + + @Override + public void execute(DelegatePlanItemInstance planItemInstance) { + String caseInstanceId = planItemInstance.getCaseInstanceId(); + CmmnRuntimeService cmmnRuntimeService = CommandContextUtil.getCmmnRuntimeService(); + + int nrOfLoops = 100; + for (int nrOfRounds = 0; nrOfRounds < 4; nrOfRounds++) { + + // Set + for (int i = 0; i < nrOfLoops; i++) { + cmmnRuntimeService.setVariable(caseInstanceId, "test_" + i, i); + } + + // Get + for (int i = 0; i < nrOfLoops; i++) { + if (cmmnRuntimeService.getVariable(caseInstanceId, "test_" + i) == null) { + throw new RuntimeException("This exception shouldn't happen"); + } + } + + // Remove + if (REMOVE_VARS_IN_LAST_ROUND && nrOfRounds == 3) { + for (int i = 0; i < nrOfLoops; i++) { + cmmnRuntimeService.removeVariable(caseInstanceId, "test_" + i); + } + } + + } + } + } + } diff --git a/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn new file mode 100644 index 00000000000..686dffd6cdb --- /dev/null +++ b/modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/runtime/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.cmmn @@ -0,0 +1,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java b/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java index 2c67c960e39..c39f20af33b 100644 --- a/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java +++ b/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java @@ -98,9 +98,18 @@ public List findInCache(Class entityClass) { } if (classCache != null) { - List entities = new ArrayList<>(classCache.size()); + ArrayList entities = new ArrayList<>(classCache.size()); for (CachedEntity cachedObject : classCache.values()) { - entities.add((T) cachedObject.getEntity()); + + // Non-deleted entities go first in the returned list, + // while deleted ones go at the end. + // This way users of this method will first get the 'active' entities. + if (!cachedObject.getEntity().isDeleted()) { + entities.add(0, (T) cachedObject.getEntity()); + } else { + entities.add((T) cachedObject.getEntity()); + } + } return entities; } diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java index 0149da7cb99..ac82123f767 100644 --- a/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/api/variables/VariablesTest.java @@ -29,6 +29,7 @@ import org.assertj.core.groups.Tuple; import org.flowable.common.engine.impl.history.HistoryLevel; +import org.flowable.engine.RuntimeService; import org.flowable.engine.delegate.DelegateExecution; import org.flowable.engine.delegate.JavaDelegate; import org.flowable.engine.impl.test.HistoryTestHelper; @@ -733,6 +734,28 @@ public void testCreateAndUpdateWithValue() { } } + @Test + @Deployment + public void testSettingGettingMultipleTimesInSameTransaction() { + + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = true; + + ProcessInstance processInstance1 = runtimeService.createProcessInstanceBuilder() + .processDefinitionKey("testSettingGettingMultipleTimesInSameTransaction") + .start(); + assertThat(runtimeService.getVariables(processInstance1.getId())).isEmpty(); + + TestSetGetVariablesDelegate.REMOVE_VARS_IN_LAST_ROUND = false; + ProcessInstance processInstance2 = runtimeService.createProcessInstanceBuilder() + .processDefinitionKey("testSettingGettingMultipleTimesInSameTransaction") + .start(); + Map variables = runtimeService.getVariables(processInstance2.getId()); + assertThat(variables).hasSize(100); + for (String variableName : variables.keySet()) { + assertThat(variables.get(variableName)).isNotNull(); + } + } + // Class to test variable serialization public static class TestSerializableVariable implements Serializable { @@ -906,4 +929,39 @@ public void execute(DelegateExecution execution) { } } + public static class TestSetGetVariablesDelegate implements JavaDelegate { + + public static boolean REMOVE_VARS_IN_LAST_ROUND = true; + + @Override + public void execute(DelegateExecution execution) { + String processInstanceId = execution.getProcessInstanceId(); + RuntimeService runtimeService = CommandContextUtil.getProcessEngineConfiguration().getRuntimeService(); + + int nrOfLoops = 100; + for (int nrOfRounds = 0; nrOfRounds < 4; nrOfRounds++) { + + // Set + for (int i = 0; i < nrOfLoops; i++) { + runtimeService.setVariable(processInstanceId, "test_" + i, i); + } + + // Get + for (int i = 0; i < nrOfLoops; i++) { + if (runtimeService.getVariable(processInstanceId, "test_" + i) == null) { + throw new RuntimeException("This exception shouldn't happen"); + } + } + + // Remove + if (REMOVE_VARS_IN_LAST_ROUND && nrOfRounds == 3) { + for (int i = 0; i < nrOfLoops; i++) { + runtimeService.removeVariable(processInstanceId, "test_" + i); + } + } + + } + } + } + } diff --git a/modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml b/modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml new file mode 100644 index 00000000000..68cc58ed050 --- /dev/null +++ b/modules/flowable-engine/src/test/resources/org/flowable/engine/test/api/variables/VariablesTest.testSettingGettingMultipleTimesInSameTransaction.bpmn20.xml @@ -0,0 +1,77 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From 01ca40bbd6f3ba77880edcdc331f90de1ef9d09f Mon Sep 17 00:00:00 2001 From: Joram Barrez Date: Thu, 7 Mar 2024 14:39:58 +0100 Subject: [PATCH 5/7] Improve previous commit for ordering the returned list of cached entities. --- .../impl/persistence/cache/EntityCacheImpl.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java b/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java index c39f20af33b..c8844ff494d 100644 --- a/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java +++ b/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/cache/EntityCacheImpl.java @@ -98,19 +98,24 @@ public List findInCache(Class entityClass) { } if (classCache != null) { + + // Having two ArrayLists and merge them later is faster than doing add(0, element) ArrayList entities = new ArrayList<>(classCache.size()); + ArrayList deletedEntities = new ArrayList<>(classCache.size()); for (CachedEntity cachedObject : classCache.values()) { - // Non-deleted entities go first in the returned list, - // while deleted ones go at the end. - // This way users of this method will first get the 'active' entities. if (!cachedObject.getEntity().isDeleted()) { - entities.add(0, (T) cachedObject.getEntity()); - } else { entities.add((T) cachedObject.getEntity()); + } else { + deletedEntities.add((T) cachedObject.getEntity()); } } + + // Non-deleted entities go first in the returned list, + // while deleted ones go at the end. + // This way users of this method will first get the 'active' entities. + entities.addAll(deletedEntities); return entities; } From 23a64a403d6267b9a0ecf079c2be698ef4b19637 Mon Sep 17 00:00:00 2001 From: Tijs Rademakers Date: Sun, 10 Mar 2024 18:38:36 +0100 Subject: [PATCH 6/7] Update notice.txt --- distro/src/notice.txt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/distro/src/notice.txt b/distro/src/notice.txt index 73a45701d33..4bcd443a5c8 100644 --- a/distro/src/notice.txt +++ b/distro/src/notice.txt @@ -104,9 +104,9 @@ org.liquibase liquibase-core 4.5.0 Apac org.mybatis mybatis 3.5.13 The Apache Software License, Version 2.0 org.mybatis mybatis-spring 3.0.3 The Apache Software License, Version 2.0 org.mvel mvel2 2.2.6.Final The Apache Software License, Version 2.0 -org.slf4j jcl-over-slf4j 2.0.11 MIT License -org.slf4j slf4j-api 2.0.11 MIT License -org.slf4j slf4j-log4j12 2.0.11 MIT License +org.slf4j jcl-over-slf4j 2.0.11 MIT License +org.slf4j slf4j-api 2.0.11 MIT License +org.slf4j slf4j-log4j12 2.0.11 MIT License org.springframework spring-beans 6.1.3 The Apache Software License, Version 2.0 org.springframework spring-core 6.1.3 The Apache Software License, Version 2.0 org.springframework spring-context 6.1.3 The Apache Software License, Version 2.0 @@ -127,3 +127,11 @@ org.tinyjee.jgraphx jgraphx 1.10.4.1 JGra org.yaml snakeyaml 1.17 The Apache Software License, Version 2.0 xerces xercesImpl 2.12.1 The Apache Software License, Version 2.0 xml-apis xml-apis 1.4.01 The Apache Software License, Version 2.0-The SAX License-The W3C License + +When using the Flowable Rest app, there are a number of additional libraries included, which are otherwise not a dependency of Flowable: + +org.eclipse.angus angus-mail 2.0.2 EPL 2.0, GPL with classpath exception +jakarta.annotation jakarta.annotation-api 2.1.1 EPL 2.0, GPL with classpath exception +jakarta.jms jakarta.jms-api 3.1.0 EPL 2.0, GPL with classpath exception +jakarta.mail jakarta.mail-api 2.1.2 EPL 2.0, GPL with classpath exception +org.openjdk.nashorn nashorn-core 15.4 GPL with classpath exception From 98dead964836146ae7237f2f41e97ab4de4c53d1 Mon Sep 17 00:00:00 2001 From: Christopher Welsch <75045836+WelschChristopher@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:11:45 +0100 Subject: [PATCH 7/7] Move variable deletion check to HistoryManager (#3862) Always execute DB historic process instance query lookup since history level can be configured on a process deifnition level --- .../DefaultHistoryConfigurationSettings.java | 5 +- .../impl/history/DefaultHistoryManager.java | 6 +- .../history/HistoryConfigurationSettings.java | 3 +- ...toricProcessInstanceEntityManagerImpl.java | 16 +- .../DeleteHistoricProcessInstanceTest.java | 188 ++++++++++++++++++ 5 files changed, 198 insertions(+), 20 deletions(-) create mode 100644 modules/flowable-engine/src/test/java/org/flowable/engine/test/history/DeleteHistoricProcessInstanceTest.java diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryConfigurationSettings.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryConfigurationSettings.java index a6faa53ed47..2be47fd6c62 100644 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryConfigurationSettings.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryConfigurationSettings.java @@ -19,7 +19,6 @@ import org.flowable.bpmn.model.Process; import org.flowable.common.engine.api.scope.ScopeTypes; import org.flowable.common.engine.impl.history.HistoryLevel; -import org.flowable.engine.history.HistoricProcessInstance; import org.flowable.engine.impl.cfg.ProcessEngineConfigurationImpl; import org.flowable.engine.impl.persistence.entity.ExecutionEntity; import org.flowable.engine.impl.util.ProcessDefinitionUtil; @@ -284,8 +283,8 @@ public boolean isHistoryEnabledForVariableInstance(String processDefinitionId, V } @Override - public boolean isHistoryEnabledForVariables(HistoricProcessInstance historicProcessInstance) { - return processEngineConfiguration.getHistoryLevel().isAtLeast(HistoryLevel.ACTIVITY); + public boolean isHistoryEnabledForVariables(String processDefinitionId) { + return isHistoryLevelAtLeast(HistoryLevel.ACTIVITY, processDefinitionId); } @Override diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryManager.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryManager.java index c6b6b9df43e..da18dedd7b2 100644 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryManager.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/DefaultHistoryManager.java @@ -129,8 +129,10 @@ public void recordProcessInstanceDeleted(String processInstanceId, String proces HistoricProcessInstanceEntity historicProcessInstance = getHistoricProcessInstanceEntityManager().findById(processInstanceId); getHistoricDetailEntityManager().deleteHistoricDetailsByProcessInstanceId(processInstanceId); - if (getHistoryConfigurationSettings().isHistoryEnabledForVariables(historicProcessInstance)) { - processEngineConfiguration.getVariableServiceConfiguration().getHistoricVariableService().deleteHistoricVariableInstancesByProcessInstanceId(processInstanceId); + + if (getHistoryConfigurationSettings().isHistoryEnabledForVariables(processDefinitionId)) { + processEngineConfiguration.getVariableServiceConfiguration().getHistoricVariableService() + .deleteHistoricVariableInstancesByProcessInstanceId(processInstanceId); } getHistoricActivityInstanceEntityManager().deleteHistoricActivityInstancesByProcessInstanceId(processInstanceId); TaskHelper.deleteHistoricTaskInstancesByProcessInstanceId(processInstanceId); diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/HistoryConfigurationSettings.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/HistoryConfigurationSettings.java index d8bf38015e8..b69e34053f0 100644 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/HistoryConfigurationSettings.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/HistoryConfigurationSettings.java @@ -13,7 +13,6 @@ package org.flowable.engine.impl.history; import org.flowable.common.engine.impl.history.HistoryLevel; -import org.flowable.engine.history.HistoricProcessInstance; import org.flowable.engine.impl.persistence.entity.ExecutionEntity; import org.flowable.engine.runtime.ActivityInstance; import org.flowable.entitylink.service.impl.persistence.entity.EntityLinkEntity; @@ -86,7 +85,7 @@ public interface HistoryConfigurationSettings { /** * Returns whether history is enabled for variables for the provided historic process instance. */ - boolean isHistoryEnabledForVariables(HistoricProcessInstance historicProcessInstance); + boolean isHistoryEnabledForVariables(String processDefinitionId); /** * Returns whether variable history is enabled for the provided historic task instance. diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/persistence/entity/HistoricProcessInstanceEntityManagerImpl.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/persistence/entity/HistoricProcessInstanceEntityManagerImpl.java index 41294cc0eb7..e5a1ad8a9ae 100644 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/persistence/entity/HistoricProcessInstanceEntityManagerImpl.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/persistence/entity/HistoricProcessInstanceEntityManagerImpl.java @@ -14,7 +14,6 @@ package org.flowable.engine.impl.persistence.entity; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Map; @@ -43,28 +42,19 @@ public HistoricProcessInstanceEntity create(ExecutionEntity processInstanceExecu @Override public long findHistoricProcessInstanceCountByQueryCriteria(HistoricProcessInstanceQueryImpl historicProcessInstanceQuery) { - if (getHistoryManager().isHistoryEnabled()) { - return dataManager.findHistoricProcessInstanceCountByQueryCriteria(historicProcessInstanceQuery); - } - return 0; + return dataManager.findHistoricProcessInstanceCountByQueryCriteria(historicProcessInstanceQuery); } @Override @SuppressWarnings("unchecked") public List findHistoricProcessInstancesByQueryCriteria(HistoricProcessInstanceQueryImpl historicProcessInstanceQuery) { - if (getHistoryManager().isHistoryEnabled()) { - return dataManager.findHistoricProcessInstancesByQueryCriteria(historicProcessInstanceQuery); - } - return Collections.EMPTY_LIST; + return dataManager.findHistoricProcessInstancesByQueryCriteria(historicProcessInstanceQuery); } @Override @SuppressWarnings("unchecked") public List findHistoricProcessInstancesAndVariablesByQueryCriteria(HistoricProcessInstanceQueryImpl historicProcessInstanceQuery) { - if (getHistoryManager().isHistoryEnabled()) { - return dataManager.findHistoricProcessInstancesAndVariablesByQueryCriteria(historicProcessInstanceQuery); - } - return Collections.EMPTY_LIST; + return dataManager.findHistoricProcessInstancesAndVariablesByQueryCriteria(historicProcessInstanceQuery); } @Override diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/history/DeleteHistoricProcessInstanceTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/history/DeleteHistoricProcessInstanceTest.java new file mode 100644 index 00000000000..ba3f97c0f89 --- /dev/null +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/history/DeleteHistoricProcessInstanceTest.java @@ -0,0 +1,188 @@ +/* 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.history; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; + +import java.util.List; + +import org.flowable.common.engine.impl.history.HistoryLevel; +import org.flowable.engine.history.HistoricProcessInstance; +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.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +/** + * @author Christopher Welsch + */ +public class DeleteHistoricProcessInstanceTest extends PluggableFlowableTestCase { + + protected HistoryLevel engineHistoryLevel; + + @BeforeEach + public void prepare() { + engineHistoryLevel = processEngineConfiguration.getHistoryLevel(); + } + + @AfterEach + public void cleanup() { + processEngineConfiguration.setHistoryLevel(engineHistoryLevel); + } + + @ParameterizedTest + @EnumSource + @Deployment(resources = { "org/flowable/engine/test/api/history/oneTaskHistoryLevelNoneProcess.bpmn20.xml" }) + public void testDeleteVariableInstancesWithHistoryLevelNone(HistoryLevel historyLevel) { + processEngineConfiguration.setHistoryLevel(historyLevel); + + ProcessInstance processInstance = runtimeService.createProcessInstanceBuilder().processDefinitionKey("oneTaskProcess") + .variable("testVariable", "testValue").start(); + + List variableInstances = runtimeService.createVariableInstanceQuery().processInstanceId(processInstance.getId()).list(); + assertThat(variableInstances).extracting(VariableInstance::getName, VariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + taskService.complete(taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult().getId()); + + List historicVariableInstances = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()).list(); + + assertThat(historicVariableInstances).isEmpty(); + assertThat(historyService.createHistoricProcessInstanceQuery().processInstanceId(processInstance.getId()).singleResult()).isNull(); + } + + @ParameterizedTest + @EnumSource + @Deployment(resources = { "org/flowable/engine/test/api/history/oneTaskHistoryLevelTaskProcess.bpmn20.xml" }) + public void testDeleteVariableInstancesWithHistoryLevelTask(HistoryLevel historyLevel) { + processEngineConfiguration.setHistoryLevel(historyLevel); + ProcessInstance processInstance = runtimeService.createProcessInstanceBuilder().processDefinitionKey("oneTaskProcess") + .variable("testVariable", "testValue").start(); + + List variableInstances = runtimeService.createVariableInstanceQuery().processInstanceId(processInstance.getId()).list(); + assertThat(variableInstances).extracting(VariableInstance::getName, VariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + taskService.complete(taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult().getId()); + + List historicVariableInstances = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()).list(); + + assertThat(historicVariableInstances).isEmpty(); + + HistoricProcessInstance historicProcessInstance = historyService.createHistoricProcessInstanceQuery().processInstanceId(processInstance.getId()) + .singleResult(); + assertThat(historicProcessInstance).isNotNull(); + assertThat(historicProcessInstance.getId()).isEqualTo(processInstance.getId()); + + } + + @ParameterizedTest + @EnumSource + @Deployment(resources = { "org/flowable/engine/test/api/history/oneTaskHistoryLevelInstanceProcess.bpmn20.xml" }) + public void testDeleteVariableInstancesWithHistoryLevelInstance(HistoryLevel historyLevel) { + processEngineConfiguration.setHistoryLevel(historyLevel); + + ProcessInstance processInstance = runtimeService.createProcessInstanceBuilder().processDefinitionKey("oneTaskProcess") + .variable("testVariable", "testValue").start(); + + List variableInstances = runtimeService.createVariableInstanceQuery().processInstanceId(processInstance.getId()).list(); + assertThat(variableInstances).extracting(VariableInstance::getName, VariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + taskService.complete(taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult().getId()); + + List historicVariableInstances = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()).list(); + + assertThat(historicVariableInstances).isEmpty(); + + HistoricProcessInstance historicProcessInstance = historyService.createHistoricProcessInstanceQuery().processInstanceId(processInstance.getId()) + .singleResult(); + assertThat(historicProcessInstance).isNotNull(); + + } + + @ParameterizedTest + @EnumSource + @Deployment(resources = { "org/flowable/engine/test/api/history/oneTaskHistoryLevelActivityProcess.bpmn20.xml" }) + public void testDeleteVariableInstancesWithHistoryLevelActivity(HistoryLevel historyLevel) { + processEngineConfiguration.setHistoryLevel(historyLevel); + ProcessInstance processInstance = runtimeService.createProcessInstanceBuilder().processDefinitionKey("oneTaskProcess") + .variable("testVariable", "testValue").start(); + + List variableInstances = runtimeService.createVariableInstanceQuery().processInstanceId(processInstance.getId()).list(); + assertThat(variableInstances).extracting(VariableInstance::getName, VariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + taskService.complete(taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult().getId()); + + List historicVariableInstances = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()).list(); + + assertThat(historicVariableInstances).extracting(HistoricVariableInstance::getVariableName, HistoricVariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + } + + @ParameterizedTest + @EnumSource + @Deployment(resources = { "org/flowable/engine/test/api/history/oneTaskHistoryLevelAuditProcess.bpmn20.xml" }) + public void testDeleteVariableInstancesWithHistoryLevelAudit(HistoryLevel historyLevel) { + processEngineConfiguration.setHistoryLevel(historyLevel); + ProcessInstance processInstance = runtimeService.createProcessInstanceBuilder().processDefinitionKey("oneTaskProcess") + .variable("testVariable", "testValue").start(); + + List variableInstances = runtimeService.createVariableInstanceQuery().processInstanceId(processInstance.getId()).list(); + assertThat(variableInstances).extracting(VariableInstance::getName, VariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + taskService.complete(taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult().getId()); + + List historicVariableInstances = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()).list(); + + assertThat(historicVariableInstances).extracting(HistoricVariableInstance::getVariableName, HistoricVariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + } + + @ParameterizedTest + @EnumSource + @Deployment(resources = { "org/flowable/engine/test/api/history/oneTaskHistoryLevelFullProcess.bpmn20.xml" }) + public void testDeleteVariableInstancesWithHistoryLevelFull(HistoryLevel historyLevel) { + processEngineConfiguration.setHistoryLevel(historyLevel); + ProcessInstance processInstance = runtimeService.createProcessInstanceBuilder().processDefinitionKey("oneTaskProcess") + .variable("testVariable", "testValue").start(); + + List variableInstances = runtimeService.createVariableInstanceQuery().processInstanceId(processInstance.getId()).list(); + assertThat(variableInstances).extracting(VariableInstance::getName, VariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + taskService.complete(taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult().getId()); + + List historicVariableInstances = historyService.createHistoricVariableInstanceQuery() + .processInstanceId(processInstance.getId()).list(); + + assertThat(historicVariableInstances).extracting(HistoricVariableInstance::getVariableName, HistoricVariableInstance::getValue).contains( + tuple("testVariable", "testValue") + ); + } +}