fix(engine): Fix DecisionDefinition historyTimeToLive

* fix(engine): Adjust History Levels on DecisionDefinitionTest * fix(engine): Fix Broken DecisionDefinitionHandler to be Stateless Related to: camunda#2496
vivekchand · Jun 15, 2023 · 46c97a8 · 46c97a8
1 parent a3213bd
commit 46c97a8
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 19 deletions.
diff --git a/...c/main/java/org/camunda/bpm/dmn/engine/impl/spi/transform/DmnElementTransformHandler.java b/...c/main/java/org/camunda/bpm/dmn/engine/impl/spi/transform/DmnElementTransformHandler.java
@@ -19,7 +19,9 @@
 import org.camunda.bpm.model.dmn.instance.DmnModelElementInstance;
 
 /**
- * Handler to transform a DMN model element.
+ * Handler to transform a DMN model element. By design, all handler implementations have to be stateless since they are
+ * stored by the static context of DefaultElementTransformHandlerRegistry & can be shared across different
+ * DmnEngineConfigurations or ProcessEngineConfiguration.
  *
  * @param <Source> the type of the transformation input
  * @param <Target> the type of the transformation output

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java
@@ -2641,8 +2641,7 @@ protected void initDmnEngine() {
           .dmnHistoryEventProducer(dmnHistoryEventProducer)
           .scriptEngineResolver(scriptingEngines)
           .feelCustomFunctionProviders(dmnFeelCustomFunctionProviders)
-          .enableFeelLegacyBehavior(dmnFeelEnableLegacyBehavior)
-          .historyTimeToLive(historyTimeToLive);
+          .enableFeelLegacyBehavior(dmnFeelEnableLegacyBehavior);
 
       if (dmnElProvider != null) {
         dmnEngineConfigurationBuilder.elProvider(dmnElProvider);

diff --git a/...ain/java/org/camunda/bpm/engine/impl/dmn/configuration/DmnEngineConfigurationBuilder.java b/...ain/java/org/camunda/bpm/engine/impl/dmn/configuration/DmnEngineConfigurationBuilder.java
@@ -49,7 +49,6 @@ public class DmnEngineConfigurationBuilder {
   protected DmnScriptEngineResolver scriptEngineResolver;
   protected ElProvider elProvider;
   protected List<FeelCustomFunctionProvider> feelCustomFunctionProviders;
-  protected String historyTimeToLive;
 
   /**
    * Creates a new builder to modify the given DMN engine configuration.
@@ -95,7 +94,7 @@ public DefaultDmnEngineConfiguration build() {
     // override the decision table handler
     DmnTransformer dmnTransformer = dmnEngineConfiguration.getTransformer();
     dmnTransformer.getElementTransformHandlerRegistry().addHandler(Definitions.class, new DecisionRequirementsDefinitionTransformHandler());
-    dmnTransformer.getElementTransformHandlerRegistry().addHandler(Decision.class, new DecisionDefinitionHandler(historyTimeToLive));
+    dmnTransformer.getElementTransformHandlerRegistry().addHandler(Decision.class, new DecisionDefinitionHandler());
 
     // do not override the script engine resolver if set
     if (dmnEngineConfiguration.getScriptEngineResolver() == null) {
@@ -136,9 +135,4 @@ public DmnEngineConfigurationBuilder enableFeelLegacyBehavior(boolean dmnFeelEna
         .enableFeelLegacyBehavior(dmnFeelEnableLegacyBehavior);
     return this;
   }
-
-  public DmnEngineConfigurationBuilder historyTimeToLive(String historyTimeToLive) {
-    this.historyTimeToLive = historyTimeToLive;
-    return this;
-  }
 }
diff --git a/.../src/main/java/org/camunda/bpm/engine/impl/dmn/transformer/DecisionDefinitionHandler.java b/.../src/main/java/org/camunda/bpm/engine/impl/dmn/transformer/DecisionDefinitionHandler.java
@@ -19,18 +19,13 @@
 import org.camunda.bpm.dmn.engine.impl.DmnDecisionImpl;
 import org.camunda.bpm.dmn.engine.impl.spi.transform.DmnElementTransformContext;
 import org.camunda.bpm.dmn.engine.impl.transform.DmnDecisionTransformHandler;
+import org.camunda.bpm.engine.impl.context.Context;
 import org.camunda.bpm.engine.impl.dmn.entity.repository.DecisionDefinitionEntity;
 import org.camunda.bpm.engine.impl.util.ParseUtil;
 import org.camunda.bpm.model.dmn.instance.Decision;
 
 public class DecisionDefinitionHandler extends DmnDecisionTransformHandler {
 
-  protected final Integer configuredHistoryTTL;
-
-  public DecisionDefinitionHandler(String configuredHistoryTTL) {
-    this.configuredHistoryTTL = ParseUtil.parseHistoryTimeToLive(configuredHistoryTTL);
-  }
-
   @Override
   protected DmnDecisionImpl createDmnElement() {
     return new DecisionDefinitionEntity();
@@ -55,6 +50,7 @@ private void setHistoryTTL(Decision decision, DecisionDefinitionEntity decisionD
     if (localHistoryTimeToLive != null) {
       decisionDefinition.setHistoryTimeToLive(localHistoryTimeToLive);
     } else {
+      Integer configuredHistoryTTL = ParseUtil.parseHistoryTimeToLive(Context.getProcessEngineConfiguration().getHistoryTimeToLive());
       decisionDefinition.setHistoryTimeToLive(configuredHistoryTTL);
     }
 

diff --git a/engine/src/test/java/org/camunda/bpm/engine/test/dmn/deployment/DecisionDefinitionTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/dmn/deployment/DecisionDefinitionTest.java
@@ -25,11 +25,13 @@
 import java.util.Map;
 import org.camunda.bpm.engine.DecisionService;
 import org.camunda.bpm.engine.HistoryService;
+import org.camunda.bpm.engine.ProcessEngineConfiguration;
 import org.camunda.bpm.engine.RepositoryService;
 import org.camunda.bpm.engine.history.HistoricDecisionInstance;
 import org.camunda.bpm.engine.impl.util.ClockUtil;
 import org.camunda.bpm.engine.repository.DeploymentBuilder;
 import org.camunda.bpm.engine.repository.DeploymentWithDefinitions;
+import org.camunda.bpm.engine.test.RequiredHistoryLevel;
 import org.camunda.bpm.engine.test.util.ProcessEngineBootstrapRule;
 import org.camunda.bpm.engine.test.util.ProcessEngineTestRule;
 import org.camunda.bpm.engine.test.util.ProvidedProcessEngineRule;
@@ -45,22 +47,25 @@
 import org.camunda.bpm.model.dmn.instance.Output;
 import org.camunda.bpm.model.dmn.instance.Text;
 import org.junit.Before;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
 
 public class DecisionDefinitionTest {
 
-  protected ProcessEngineBootstrapRule bootstrapRule = new ProcessEngineBootstrapRule(configuration -> {
+  @ClassRule
+  public static ProcessEngineBootstrapRule BOOTSTRAP_RULE = new ProcessEngineBootstrapRule(configuration -> {
     configuration.setHistoryTimeToLive("P30D");
   });
 
-  protected ProvidedProcessEngineRule engineRule = new ProvidedProcessEngineRule(bootstrapRule);
+  protected ProvidedProcessEngineRule engineRule = new ProvidedProcessEngineRule(BOOTSTRAP_RULE);
 
   protected ProcessEngineTestRule testRule = new ProcessEngineTestRule(engineRule);
 
   @Rule
-  public RuleChain ruleChain = RuleChain.outerRule(bootstrapRule).around(engineRule).around(testRule);
+  public RuleChain ruleChain = RuleChain.outerRule(engineRule)
+      .around(testRule);
 
   protected RepositoryService repositoryService;
   protected DecisionService decisionService;
@@ -103,6 +108,7 @@ public void shouldNotOverrideWithGlobalConfigOnDecisionHistoryTTLPresence() {
     assertThat(deployment.getDeployedDecisionDefinitions().get(0).getHistoryTimeToLive()).isEqualTo(10);
   }
 
+  @RequiredHistoryLevel(ProcessEngineConfiguration.HISTORY_FULL)
   @Test
   public void shouldApplyHistoryTTLOnRemovalTimeOfDecisionInstanceLocal() {
     // given
@@ -130,6 +136,7 @@ public void shouldApplyHistoryTTLOnRemovalTimeOfDecisionInstanceLocal() {
     assertThat(result.getRemovalTime()).isInSameDayAs(expectedRemovalDate);
   }
 
+  @RequiredHistoryLevel(ProcessEngineConfiguration.HISTORY_FULL)
   @Test
   public void shouldApplyHistoryTTLOnRemovalTimeOfDecisionInstanceGlobal() {
     // given