-
Notifications
You must be signed in to change notification settings - Fork 694
[Backport stable/8.7] ArchUnit rule to support both camelCase and kebab-case in ConditionalOnProperty #37052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
1 task
af89187
to
97d12da
Compare
97d12da
to
1d85a7d
Compare
Adds a new module that can be used to specify ArchUnit tests for the orchestration cluster. These tests have access to the entire production code through a dependency on dist (io.camunda:camunda-zeebe). The tests need to be run by surefire rather than failsafe to ensure correct execution and access to loaded classes. With failsafe, tests are executed but failures are not detected somehow. I did not understand why this is. Note that the parent module (io.camunda:camunda-qa) attempts to run all tests with failsafe rather than surefire so the archunit-tests's pom explicitly re-configures this. The choice to create a new module was made as it: - allows to easily run these tests together - depending on dist ensures it runs on all production code, including modules added later without needing to change dependencies - helps others to find a fitting place for their ArchUnit tests - groups together ArchUnit tests for easily gaining an overview The downsides are: - long feedback loop - I value this less than the long term ease of maintenance. - doesn't run on non-prod code - No need to solve this now. See: https://camunda.slack.com/archives/C08F5RD5X8B/p1755527617038109 (cherry picked from commit 4399511)
The ConditionalOnProperty annotation requires kebab-case in its attributes in order to correctly match them using relaxed binding to spring loaded properties. In other words, to match to both my-example and myExample, the @ConditionalOnProperty must refer to my-example. If it refers to myExample, then it only works explicitly on myExample and relaxed binding to my-example doesn't work. To avoid this, we add an ArchUnit test that verifies that none of the attributes of ConditionalOnProperty use camelCase. It does this simply by checking for capitalized letters. If it fails, the error message includes all the needed details, like the location of the forbidden annotation, the specific attribute, and the failure. To overcome the problem, simply switch to using kebab-case in the attributes of ConditionalOnProperty. (cherry picked from commit d67a9f8)
ConditionalOnProperty requires specifying names of properties using kebab-case. This ensures that camunda.tasklist.webapp-enabled can be used rather than requiring camunda.tasklist.webappEnabled explicitly. Note that the docs document it as camunda.tasklist.webappEnabled, but this change is backward compatible, as camunda.tasklist.webappEnabled can still be matched due to relaxed binding. We decided to use kebab-case for configs in the docs, but they have not been adjusted yet. (cherry picked from commit d6f200e)
ConditionalOnProperty requires specifying names of properties using kebab-case. This ensures that camunda.operate.webapp-enabled can be used rather than requiring camunda.operate.webappEnabled explicitly. Note that the docs document it as camunda.operate.webappEnabled, but this change is backward compatible, as camunda.operate.webappEnabled can still be matched due to relaxed binding. We decided to use kebab-case for configs in the docs, but they have not been adjusted yet. (cherry picked from commit d8b567a)
ConditionalOnProperty requires specifying names of properties using kebab-case. This ensures that camunda.tasklist.importer-enabled can be used rather than requiring camunda.tasklist.importerEnabled explicitly. Note that the docs document it as camunda.tasklist.importerEnabled, but this change is backward compatible, as camunda.tasklist.importerEnabled can still be matched due to relaxed binding. We decided to use kebab-case for configs in the docs, but they have not been adjusted yet. (cherry picked from commit feaf933)
ConditionalOnProperty requires specifying names of properties using kebab-case. This ensures that camunda.operate.importer-enabled can be used rather than requiring camunda.operate.importerEnabled explicitly. Note that the docs document it as camunda.operate.importerEnabled, but this change is backward compatible, as camunda.operate.importerEnabled can still be matched due to relaxed binding. We decided to use kebab-case for configs in the docs, but they have not been adjusted yet. (cherry picked from commit b50ce94)
ConditionalOnProperty requires specifying names of properties using kebab-case. This ensures that camunda.tasklist.feature-flag.process-public-endpoints can be used rather than requiring camunda.tasklist.featureFlag.processPublicEndpoints explicitly. (cherry picked from commit eba1343)
Without this, the qa modules are not used during `./mvnw test` or `./mvnw verify`. (cherry picked from commit 3a39089)
The condition checks against uppercase characters only. This means it accepts kebab-case and not camelCase or PascalCase. Just staying not camelCase is thus not entirely correct. We can easily resolve this by removing the mention of camelCase here. Note that the validation already describes the actual violation (no uppercase allowed). I also took the chance to align both descriptions. (cherry picked from commit 833b6b7)
This change moves the RequireKebabCaseInConditionalOnPropertyArchTest to the io.camunda package as it covers all those classes. This makes it easier for other ArchTests to also align to the structure of the codebase that they work with. The end goal is a module that organizes its test in way where it's easy to see at a glance what code is covered by these tests. (cherry picked from commit 42ce60f)
1d85a7d
to
0f6c6a4
Compare
korthout
approved these changes
Aug 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
component/operate
Related to the Operate component/team
component/optimize
Related to Optimize component/team
component/tasklist
Related to the Tasklist component/team
component/zeebe
Related to the Zeebe component/team
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Backport of #36872 to
stable/8.7
.relates to #36869