historyCleanupDefaultNumberOfRetries cannot be configured from tomcat xml configurations

[psavidis]

Environment (Required on creation)

Tomcat Distrubution of : 7.19.7-ee, 7.20.0

Description (Required on creation; please attach any relevant screenshots, stacktraces, log files, etc. to the ticket)

The newly added flag historyCleanupDefaultNumberOfRetries which controls the customisation of retries for cleanup jobs separately, once configured from a tomcat xml configuration file makes the engine startup to fail.

Steps to reproduce (Required on creation)

1. Go to camunda-bpm-platform/distro/tomcat/assembly/src/conf/bpm-platform.xml

2. After the property historyCleanupBatchWindowStartTime, copy paste the following indicative config:

   <property name="historyCleanupDefaultNumberOfRetries">5</property> (The above config is only indicative, any int value will do)

3. Build The tomcat distro & unzip camunda-bpm-tomcat-7.xx.zip.
4. Use start-camunda.sh (or .bat) to start tomcat

Observed Behavior (Required on creation)

Open camunda-bpm-platform/distro/tomcat/distro/target/camunda-bpm-tomcat-7.xx-SNAPSHOT/server/apache-tomcat-9.0.75/logs/catalina.out; Notice the stacktrace failing with the respective message that historyCleanupDefaultNumberOfRetries cannot be initialised.

Expected behavior (Required on creation)

The application should startup and the ProcessEngineConfiguration should be populated with the respective value (in the indicative example the value should be 5)

Root Cause (Required on prioritization)

org.camunda.bpm.container.impl.metadata.PropertyHelper#convertToClass supports only primitive types. Thus, the type of historyCleanupDefaultNumberOfRetries fetched from the reflection code of the method is java.lang.Integer, not matching with any of the primitive types of the current conditionals and the output type is determined to be java.lang.String.

Solution Ideas

• Make org.camunda.bpm.container.impl.metadata.PropertyHelper.convertToClass support java.lang.Integer autoboxing type.

• Likewise, the rest of the conditionals can be complemented with their respective autoboxing types Long, Float, Boolean.

• The primitive type conditionals can be extended with Double, just in case any other configuration with these types might be added in the future & avoid fixing the same problem again.

• byte, short, char primitive types can be excluded since: a) First one is not likely to be ever exposed for config purposes b) Short is already covered by int c) Character can cause config-confusion with String. Also String can cover characters too & the likelihood of using a char in configuration is low.

Hints

Links

• https://jira.camunda.com/browse/SUPPORT-13684
• https://jira.camunda.com/browse/SUPPORT-18739
• https://github.com/camunda/camunda-bpm-platform/issues/2826

Breakdown

### Latest
- [ ] https://github.com/camunda/camunda-bpm-platform/pull/3806

### Backports
- [ ] https://github.com/camunda/camunda-bpm-platform-maintenance/pull/1090
- [ ] https://github.com/camunda/camunda-bpm-platform-maintenance/pull/1089

Dev2QA handover

• [ ] Does this ticket need a QA test and the testing goals are not clear from the description? Add a Dev2QA handover comment

[psavidis]

TODO: Add Fix to 7.20 once respective Release Process branch is created.

[psavidis]

Update:

The solution passes all AS integration Tests besides WebLogic. Last run that failed can be found here.

The exact reason that the WebLogic ITs fail is not identified.

Hints for troubleshooting:

• The new config file boxed-properties-config.xml adds a new engine called boxed-config-engine for the different AS & used by the test TestConfigParsing that covers historyCleanupDefaultNumberOfRetries parsing.
• The engine is found in all AS besides WebLogic. For that case, the engine fetched by the test is null, cannot be found.
• boxed-config-engine reference cannot be found in any WLS log file as if the engine is not registered at all

[psavidis]

The above issue stands as an obstacle at the moment for moving forward. Also, the new implementation will require extra testing for ALL the possible config scenarios.

In this regard, the decision is to move forward with simply using an int type for historyCleanupDefaultNumberOfRetries and use a default value of Integer.MIN_VALUE and adapt the implementation (see HistoryCleanupHelper#getMaxRetries) accordingly.

[mboskamp]

@psavidis, the code looks good 👍

🔧 Please add the correct version labels, DRI, and reviewer before you merge.