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 index 1bf1398afe8..f4855c8a12e 100644 --- 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). - *

- * 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 index 4b2d4ce53ea..0fdd3b86db0 100644 --- 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/engine/src/test/java/org/camunda/bpm/engine/test/api/repository/HistoryTimeToLiveDeploymentTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/api/repository/HistoryTimeToLiveDeploymentTest.java index 52d6f872218..124832900d9 100644 --- a/engine/src/test/java/org/camunda/bpm/engine/test/api/repository/HistoryTimeToLiveDeploymentTest.java +++ b/engine/src/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 new file mode 100644 index 00000000000..c74a2647170 --- /dev/null +++ b/engine/src/test/resources/org/camunda/bpm/engine/test/api/repository/case_with_365_httl.cmmn @@ -0,0 +1,14 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/engine/src/test/resources/org/camunda/bpm/engine/test/api/repository/decision_with_365_httl.dmn b/engine/src/test/resources/org/camunda/bpm/engine/test/api/repository/decision_with_365_httl.dmn new file mode 100644 index 00000000000..1d7a8145366 --- /dev/null +++ b/engine/src/test/resources/org/camunda/bpm/engine/test/api/repository/decision_with_365_httl.dmn @@ -0,0 +1,86 @@ + + + + + + + status + + + "bronze","silver","gold" + + + + + sum + + + + + "ok","notok" + + + + + + "bronze" + + + + + + "notok" + + + + + + + + "silver" + + + + + + "ok" + + + "you little fish will get what you want" + + + + + "silver" + + + = 1000]]> + + + "notok" + + + "you took too much man, you took too much!" + + + + + "gold" + + + + + + "ok" + + + "you get anything you want" + + + + +