chore(engine): adjust TTL warnings (#4039)

* remove the 180 days default value information * if the global engine configuration is set and the a Model with HTTL > as a global engine flag is deployed, add a warning that the global flag is overwritten for this process #4031
camunda · Feb 20, 2024 · 036acf9 · 036acf9
1 parent 5be182b
commit 036acf9
Show file tree

Hide file tree

Showing 5 changed files with 181 additions and 88 deletions.
diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/HistoryTimeToLiveParser.java b/engine/src/main/java/org/camunda/bpm/engine/impl/HistoryTimeToLiveParser.java
@@ -25,7 +25,6 @@
 import org.camunda.bpm.engine.impl.cfg.ConfigurationLogger;
 import org.camunda.bpm.engine.impl.cfg.ProcessEngineConfigurationImpl;
 import org.camunda.bpm.engine.impl.context.Context;
-import org.camunda.bpm.engine.impl.dmn.entity.repository.DecisionRequirementsDefinitionEntity;
 import org.camunda.bpm.engine.impl.interceptor.CommandContext;
 import org.camunda.bpm.engine.impl.util.ParseUtil;
 import org.camunda.bpm.engine.impl.util.xml.Element;
@@ -34,16 +33,11 @@
 
 /**
  * Class that encapsulates the business logic of parsing HistoryTimeToLive of different deployable resources (process, definition, case).
- * <p>
- * Furthermore, it considers notifying the users with a logging message when parsing historyTimeToLive values that are
- * the same with the default camunda modeler TTL value (see {@code CAMUNDA_MODELER_TTL_DEFAULT_VALUE}).
  */
 public class HistoryTimeToLiveParser {
 
   protected static final ConfigurationLogger LOG = ConfigurationLogger.CONFIG_LOGGER;
 
-  protected static final int CAMUNDA_MODELER_TTL_DEFAULT_VALUE = 180; // This value is hardcoded into camunda modeler
-
   protected final boolean enforceNonNullValue;
   protected final String httlConfigValue;
 
@@ -112,11 +106,11 @@ protected Integer parseAndValidate(String historyTimeToLiveString, String defini
 
     if (!skipEnforceTtl) {
       if (result.isInValidAgainstConfig()) {
-        throw new NotValidException("History Time To Live cannot be null");
+        throw LOG.logErrorNoTTLConfigured();
       }
 
-      if (result.shouldBeLogged()) {
-        LOG.logHistoryTimeToLiveDefaultValueWarning(definitionKey);
+      if (result.hasLongerModelValueThanGlobalConfig()) {
+        LOG.logModelHTTLLongerThanGlobalConfiguration(definitionKey);
       }
     }
 
@@ -139,10 +133,11 @@ protected boolean isInValidAgainstConfig() {
       return enforceNonNullValue && (valueAsInteger == null);
     }
 
-    protected boolean shouldBeLogged() {
+    protected boolean hasLongerModelValueThanGlobalConfig() {
       return !systemDefaultConfigWillBeUsed // only values originating from models make sense to be logged
           && valueAsInteger != null
-          && valueAsInteger == CAMUNDA_MODELER_TTL_DEFAULT_VALUE;
+          && httlConfigValue != null && !httlConfigValue.isEmpty()
+          && valueAsInteger > ParseUtil.parseHistoryTimeToLive(httlConfigValue);
     }
   }
 }
diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ConfigurationLogger.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ConfigurationLogger.java
@@ -18,6 +18,7 @@
 
 import javax.naming.NamingException;
 import org.camunda.bpm.engine.ProcessEngineException;
+import org.camunda.bpm.engine.exception.NotValidException;
 import org.camunda.bpm.engine.impl.ProcessEngineLogger;
 
 /**
@@ -114,28 +115,26 @@ public void invalidPropertyValue(Exception e) {
   }
 
   /**
-   * Method for logging message when default history time to live is configured.
+   * Method for logging message when model TTL longer than global TTL.
    *
-   * @param definitionKey the correlated definition key with which history time to live is related to.
+   * @param definitionKey the correlated definition key with which history TTL is related to.
    *                      For processes related to httl, that is the processDefinitionKey, for cases the case definition key
    *                      whereas for decisions is the decision definition key.
    */
-  public void logHistoryTimeToLiveDefaultValueWarning(String definitionKey) {
-    String message = formatHTTLDefaultValueMessage(definitionKey);
-
-    logWarn("016", message);
+  public void logModelHTTLLongerThanGlobalConfiguration(String definitionKey) {
+    logWarn(
+        "017", "definitionKey: {}; "
+            + "The specified Time To Live (TTL) in the model is longer than the global TTL configuration. "
+            + "The historic data related to this model will be cleaned up at later point comparing to the other processes.",
+            definitionKey);
   }
 
-  protected String formatHTTLDefaultValueMessage(String definitionKey) {
-    String result = "You are using the default TTL (Time To Live) of 180 days (six months); "
-        + "the history clean-up feature will delete your data after six months. "
-        + "We recommend adjusting the TTL configuration property aligned with your specific requirements.";
-
-    if (definitionKey != null) {
-      result = "definitionKey: " + definitionKey + "; " + result;
-    }
-
-    return result;
+  public NotValidException logErrorNoTTLConfigured() {
+    return new NotValidException(exceptionMessage("018",
+        "History Time To Live (TTL) cannot be null. "
+        + "TTL is necessary for the History Cleanup to work. The following options are possible:\n"
+        + "* Set historyTimeToLive in the model\n"
+        + "* Set a default historyTimeToLive as a global process engine configuration\n"
+        + "* (Not recommended) Deactivate the enforceTTL config to disable this check"));
   }
-
 }
diff --git a/...test/java/org/camunda/bpm/engine/test/api/repository/HistoryTimeToLiveDeploymentTest.java b/...test/java/org/camunda/bpm/engine/test/api/repository/HistoryTimeToLiveDeploymentTest.java
@@ -35,6 +35,7 @@
 import org.camunda.bpm.engine.test.ProcessEngineRule;
 import org.camunda.bpm.engine.test.util.ProcessEngineTestRule;
 import org.camunda.bpm.engine.test.util.ProvidedProcessEngineRule;
+import org.camunda.bpm.model.bpmn.Bpmn;
 import org.camunda.commons.testing.ProcessEngineLoggingRule;
 import org.junit.After;
 import org.junit.Before;
@@ -46,12 +47,12 @@ public class HistoryTimeToLiveDeploymentTest {
 
   protected static final String CONFIG_LOGGER = "org.camunda.bpm.engine.cfg";
 
-  protected static final String EXPECTED_DEFAULT_CONFIG_MSG =
-      "You are using the default TTL (Time To Live) of 180 days (six months); "
-      + "the history clean-up feature will delete your data after six months. "
-      + "We recommend adjusting the TTL configuration property aligned with your specific requirements.";
+  protected static final String EXPECTED_DEFAULT_CONFIG_MSG = "History Time To Live (TTL) cannot be null. ";
 
-  protected static final String DEFAULT_HTTL_CONFIG_VALUE = "180";
+  protected static final String EXPECTED_LONGER_TTL_MSG = "The specified Time To Live (TTL) in the model is longer than the global TTL configuration. "
+      + "The historic data related to this model will be cleaned up at later point comparing to the other processes.";
+
+  protected static final String HTTL_CONFIG_VALUE = "180";
 
   protected ProcessEngineRule engineRule = new ProvidedProcessEngineRule();
   protected ProcessEngineTestRule testRule = new ProcessEngineTestRule(engineRule);
@@ -98,7 +99,11 @@ public void processWithoutHTTLShouldFail() {
 
         // then
         .isInstanceOf(ParseException.class)
-        .hasMessageContaining("History Time To Live cannot be null");
+        .hasMessageContaining(EXPECTED_DEFAULT_CONFIG_MSG)
+        .hasMessageContaining("TTL is necessary for the History Cleanup to work. The following options are possible:")
+        .hasMessageContaining("* Set historyTimeToLive in the model")
+        .hasMessageContaining("* Set a default historyTimeToLive as a global process engine configuration")
+        .hasMessageContaining("* (Not recommended) Deactivate the enforceTTL config to disable this check");
   }
 
   @Test
@@ -137,7 +142,7 @@ public void caseWithoutHTTLShouldFail() {
         // then
         .isInstanceOf(ProcessEngineException.class)
         .hasCauseInstanceOf(NotValidException.class)
-        .hasStackTraceContaining("History Time To Live cannot be null");
+        .hasStackTraceContaining(EXPECTED_DEFAULT_CONFIG_MSG);
   }
 
   @Test
@@ -164,7 +169,7 @@ public void decisionWithoutHTTLShouldFail() {
         // then
         .isInstanceOf(ProcessEngineException.class)
         .hasCauseInstanceOf(DmnTransformException.class)
-        .hasStackTraceContaining("History Time To Live cannot be null");
+        .hasStackTraceContaining(EXPECTED_DEFAULT_CONFIG_MSG);
   }
 
   @Test
@@ -185,7 +190,7 @@ public void shouldDeploySuccessfullyDueToProcessEngineConfigFallback() {
   @Test
   public void shouldNotLogMessageOnDefaultConfigOriginatingFromConfig() {
     // given
-    processEngineConfiguration.setHistoryTimeToLive(DEFAULT_HTTL_CONFIG_VALUE);
+    processEngineConfiguration.setHistoryTimeToLive(HTTL_CONFIG_VALUE);
 
     // when
     testRule.deploy(repositoryService.createDeployment()
@@ -196,105 +201,99 @@ public void shouldNotLogMessageOnDefaultConfigOriginatingFromConfig() {
   }
 
   @Test
-  public void shouldLogMessageOnDefaultValueOfProcessModel() {
+  public void shouldGetDeployedProcess() {
     // given
-    String nonDefaultValue = "179";
-    processEngineConfiguration.setHistoryTimeToLive(nonDefaultValue);
+    processEngineConfiguration.setEnforceHistoryTimeToLive(false);
 
     // when
-    testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/version3.bpmn20.xml"));
+    DeploymentWithDefinitions definitions = testRule.deploy(repositoryService.createDeployment()
+        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/version1.bpmn20.xml"));
 
     // then
-    assertThat(loggingRule.getFilteredLog("definitionKey: process; " + EXPECTED_DEFAULT_CONFIG_MSG)).hasSize(1);
+    processEngineConfiguration.setEnforceHistoryTimeToLive(true);
+    processEngineConfiguration.getDeploymentCache().purgeCache();
+    repositoryService.getProcessDefinition(definitions.getDeployedProcessDefinitions().get(0).getId());
   }
 
   @Test
-  public void shouldLogMessageOnDefaultValueOfCaseModel() {
+  public void shouldGetDeployedDecision() {
     // given
-    String nonDefaultValue = "179";
-    processEngineConfiguration.setHistoryTimeToLive(nonDefaultValue);
+    processEngineConfiguration.setEnforceHistoryTimeToLive(false);
 
     // when
-    testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/case_with_180_httl.cmmn"));
+    DeploymentWithDefinitions definitions = testRule.deploy(repositoryService.createDeployment()
+        .addClasspathResource("org/camunda/bpm/engine/test/api/dmn/Another_Example.dmn"));
 
     // then
-    assertThat(loggingRule.getFilteredLog("definitionKey: testCase; " + EXPECTED_DEFAULT_CONFIG_MSG)).hasSize(1);
+    processEngineConfiguration.setEnforceHistoryTimeToLive(true);
+    processEngineConfiguration.getDeploymentCache().purgeCache();
+    repositoryService.getDecisionDefinition(definitions.getDeployedDecisionDefinitions().get(0).getId());
   }
 
   @Test
-  public void shouldLogMessageOnDefaultValueOfDecisionModel() {
+  public void shouldGetDeployedCase() {
     // given
-    String nonDefaultValue = "179";
-    processEngineConfiguration.setHistoryTimeToLive(nonDefaultValue);
+    processEngineConfiguration.setEnforceHistoryTimeToLive(false);
 
     // when
-    testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/decision_with_180_httl.dmn"));
+    DeploymentWithDefinitions definitions = testRule.deploy(repositoryService.createDeployment()
+        .addClasspathResource("org/camunda/bpm/engine/test/api/cmmn/oneTaskCase2.cmmn"));
 
     // then
-    assertThat(loggingRule.getFilteredLog("definitionKey: testDecision; " + EXPECTED_DEFAULT_CONFIG_MSG)).hasSize(1);
+    processEngineConfiguration.setEnforceHistoryTimeToLive(true);
+    processEngineConfiguration.getDeploymentCache().purgeCache();
+    repositoryService.getCaseDefinition(definitions.getDeployedCaseDefinitions().get(0).getId());
   }
 
   @Test
-  public void shouldNotLogAnyMessageOnNonDefaultConfig() {
-    String nonDefaultValue = "179";
-
+  public void shouldLogMessageOnLongerTTLInProcessModel() {
     // given
+    String nonDefaultValue = "179";
     processEngineConfiguration.setHistoryTimeToLive(nonDefaultValue);
 
     // when
-    testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/version1.bpmn20.xml"));
+    deployProcessDefinitions();
 
     // then
-    assertThat(loggingRule.getFilteredLog(EXPECTED_DEFAULT_CONFIG_MSG)).hasSize(0);
+    assertThat(loggingRule.getFilteredLog("definitionKey: process; " + EXPECTED_LONGER_TTL_MSG)).hasSize(1);
   }
 
   @Test
-  public void shouldGetDeployedProcess() {
+  public void shouldLogMessageOnLongerTTLInfCaseModel() {
     // given
-    processEngineConfiguration.setEnforceHistoryTimeToLive(false);
+    String nonDefaultValue = "179";
+    processEngineConfiguration.setHistoryTimeToLive(nonDefaultValue);
 
     // when
-    DeploymentWithDefinitions definitions = testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/version1.bpmn20.xml"));
+    testRule.deploy(repositoryService.createDeployment()
+        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/case_with_365_httl.cmmn"));
 
     // then
-    processEngineConfiguration.setEnforceHistoryTimeToLive(true);
-    processEngineConfiguration.getDeploymentCache().purgeCache();
-    repositoryService.getProcessDefinition(definitions.getDeployedProcessDefinitions().get(0).getId());
+    assertThat(loggingRule.getFilteredLog("definitionKey: testCase; " + EXPECTED_LONGER_TTL_MSG)).hasSize(1);
   }
 
   @Test
-  public void shouldGetDeployedDecision() {
+  public void shouldLogMessageOnLongerTTLInDecisionModel() {
     // given
-    processEngineConfiguration.setEnforceHistoryTimeToLive(false);
+    String nonDefaultValue = "179";
+    processEngineConfiguration.setHistoryTimeToLive(nonDefaultValue);
 
     // when
-    DeploymentWithDefinitions definitions = testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/dmn/Another_Example.dmn"));
+    testRule.deploy(repositoryService.createDeployment()
+        .addClasspathResource("org/camunda/bpm/engine/test/api/repository/decision_with_365_httl.dmn"));
 
     // then
-    processEngineConfiguration.setEnforceHistoryTimeToLive(true);
-    processEngineConfiguration.getDeploymentCache().purgeCache();
-    repositoryService.getDecisionDefinition(definitions.getDeployedDecisionDefinitions().get(0).getId());
+    assertThat(loggingRule.getFilteredLog("definitionKey: testDecision; " + EXPECTED_LONGER_TTL_MSG)).hasSize(1);
   }
 
-  @Test
-  public void shouldGetDeployedCase() {
-    // given
-    processEngineConfiguration.setEnforceHistoryTimeToLive(false);
-
-    // when
-    DeploymentWithDefinitions definitions = testRule.deploy(repositoryService.createDeployment()
-        .addClasspathResource("org/camunda/bpm/engine/test/api/cmmn/oneTaskCase2.cmmn"));
-
-    // then
-    processEngineConfiguration.setEnforceHistoryTimeToLive(true);
-    processEngineConfiguration.getDeploymentCache().purgeCache();
-    repositoryService.getCaseDefinition(definitions.getDeployedCaseDefinitions().get(0).getId());
+  protected void deployProcessDefinitions() {
+    testRule.deploy(
+      Bpmn.createExecutableProcess("process")
+        .camundaHistoryTimeToLive(365)
+        .startEvent()
+        .userTask()
+        .endEvent()
+        .done());
   }
 
 }
diff --git a/engine/src/test/resources/org/camunda/bpm/engine/test/api/repository/case_with_365_httl.cmmn b/engine/src/test/resources/org/camunda/bpm/engine/test/api/repository/case_with_365_httl.cmmn
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="ISO-8859-1" standalone="yes"?>
+<definitions id="_7f0c94c0-2a22-445d-b4b7-4fd181e08248"
+             xmlns="http://www.omg.org/spec/CMMN/20151109/MODEL"
+             xmlns:camunda="http://camunda.org/schema/1.0/cmmn"
+             targetNamespace="Examples">
+  <case id="testCase" name="One Task Case" camunda:historyTimeToLive="365">
+
+    <casePlanModel id="CasePlanModel_1">
+      <planItem id="PI_HumanTask_1" definitionRef="HumanTask_1" />
+      <humanTask id="HumanTask_1" name="A HumanTask" />
+    </casePlanModel>
+  </case>
+
+</definitions>