-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fail fast on duplicate configuration resources (with configurable merge/first-match) #12383
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
base: 5.0.x
Are you sure you want to change the base?
Conversation
Introduce ConfigurationLoadStrategy and ConfigurationLoadStrategyType to configure how duplicate configuration resources are handled.
Allow configuring the ConfigurationLoadStrategy via ApplicationContextBuilder and propagate it through DefaultApplicationContextBuilder and Micronaut.
Resolve constant property sources dynamically so default loaders reflect StaticOptimizations changes in tests and early environment construction.
Add configurable strategies (fail fast, first match, merge all) when the same application configuration resource appears multiple times on the classpath.
Cover default fail-fast behavior and the FIRST_MATCH and MERGE_ALL strategies, including mergeOrder and environment-specific duplicates.
Document the new default fail-fast behavior and how to configure FIRST_MATCH or MERGE_ALL using snippet-backed examples.
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.
Pull request overview
Adds a configurable configuration-resource loading strategy to Micronaut so duplicate config files on the classpath can either fail fast (new default), fall back to first-match behavior, or be merged deterministically (optionally with merge ordering).
Changes:
- Introduces
ConfigurationLoadStrategy/ConfigurationLoadStrategyTypeand exposes it viaApplicationContextBuilder(andMicronaut) to control duplicate handling. - Updates
DefaultEnvironmentconfig loading to detect duplicates, optionally warn, fail fast, or merge resources (with optional regex-basedmergeOrder). - Adds docs snippets + guide updates and new Spock tests for the new behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test-suite/src/test/java/io/micronaut/docs/context/env/ConfigurationLoadStrategySnippet.java | Java docs snippet demonstrating strategy configuration |
| test-suite-kotlin/src/test/kotlin/io/micronaut/docs/context/env/ConfigurationLoadStrategySnippet.kt | Kotlin docs snippet demonstrating strategy configuration |
| test-suite-groovy/src/test/groovy/io/micronaut/docs/context/env/ConfigurationLoadStrategySnippet.groovy | Groovy docs snippet demonstrating strategy configuration |
| test-suite-property-source/src/test/groovy/io/micronaut/context/env/ConstantPropertySourceSpec.groovy | Updates test setup to explicitly restore legacy FIRST_MATCH behavior |
| test-suite-property-source/src/test/groovy/io/micronaut/context/env/ConfigurationLoadStrategySpec.groovy | New tests for default fail-fast, FIRST_MATCH, MERGE_ALL, mergeOrder, env-specific duplicates |
| src/main/docs/guide/config/propertySource.adoc | Documents duplicate configuration resource handling and strategy options |
| src/main/docs/guide/appendix/breaks.adoc | Adds Micronaut 5 breaking change note for default fail-fast behavior |
| inject/src/main/java/io/micronaut/context/env/DefaultEnvironment.java | Implements duplicate detection + strategy application during config loading |
| inject/src/main/java/io/micronaut/context/env/ConstantPropertySourceLoader.java | Adjusts constant property source loader initialization behavior |
| inject/src/main/java/io/micronaut/context/env/ConfigurationLoadStrategyType.java | New enum defining strategy types |
| inject/src/main/java/io/micronaut/context/env/ConfigurationLoadStrategy.java | New record + builder defining strategy configuration |
| inject/src/main/java/io/micronaut/context/DefaultApplicationContextBuilder.java | Stores/applies strategy configured via builder hook |
| inject/src/main/java/io/micronaut/context/ApplicationContextConfiguration.java | Adds strategy to context configuration API with default |
| inject/src/main/java/io/micronaut/context/ApplicationContextBuilder.java | Adds builder hook for configuring strategy |
| context/src/main/java/io/micronaut/runtime/Micronaut.java | Propagates new builder hook through Micronaut |
inject/src/main/java/io/micronaut/context/env/DefaultEnvironment.java
Outdated
Show resolved
Hide resolved
...roperty-source/src/test/groovy/io/micronaut/context/env/ConfigurationLoadStrategySpec.groovy
Show resolved
Hide resolved
...roperty-source/src/test/groovy/io/micronaut/context/env/ConfigurationLoadStrategySpec.groovy
Outdated
Show resolved
Hide resolved
inject/src/main/java/io/micronaut/context/env/DefaultEnvironment.java
Outdated
Show resolved
Hide resolved
inject/src/main/java/io/micronaut/context/env/ConfigurationLoadStrategy.java
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@graemerocher I've opened a new pull request, #12384, to work on those changes. Once the pull request is ready, I'll request review from you. |
…ing (#12384) * Fix ConfigurationLoadStrategy to always defensively copy mergeOrder Co-authored-by: graemerocher <[email protected]> * Optimize performance for FIRST_MATCH with no warnings Co-authored-by: graemerocher <[email protected]> * Add cleanup for temp directories/JARs in tests Co-authored-by: graemerocher <[email protected]> * Add cross-extension duplicate detection in DefaultEnvironment Co-authored-by: graemerocher <[email protected]> * Address code review feedback and add documentation Co-authored-by: graemerocher <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: graemerocher <[email protected]>
|
|
|
the test failure is unrelated |
|
I would prefer for this to be implemented in the same way as Spring has it with classpath: - Locates only the first resource found at the specified path within the classpath. classpath*: - Locates all resources that match the given name across all classpath entries (directories and JARs). And this can be used with |
|
I don't understand, in this case the user doesn't specify where or how config is loaded. If there are duplicate |
inject/src/main/java/io/micronaut/context/env/ConfigurationLoadStrategy.java
Show resolved
Hide resolved
sdelamo
left a comment
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.
I think the feature is useful but the PR does not contain tests for the new logic in DefaultEnvironment::loadPropertySourceFromAbstractLoader
inject/src/main/java/io/micronaut/context/env/ConfigurationLoadStrategy.java
Show resolved
Hide resolved
inject/src/main/java/io/micronaut/context/ApplicationContextBuilder.java
Outdated
Show resolved
Hide resolved
|
Handle non-conforming ResourceLoader#getResources implementations and avoid assigning null under NullMarked in ConfigurationLoadStrategy.Builder.
Cover MERGE_ALL merging and mergeOrder behavior, invalid mergeOrder patterns, and FAIL_ON_DUPLICATE duplicates within the same extension. Add a context-level test to exercise Micronaut fluent configurationLoadingStrategy.
Address Sonar Groovy convention findings.
| return propertySourceLoader.loadEnv(name, resourceLoader, activeEnvironment); | ||
| } | ||
|
|
||
| private Optional<PropertySource> loadPropertySourceFromAbstractLoader(String fileName, |
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.
Can you move all of it into a new method to ClassPathResourceLoader with an option to merge of fail? I don't think it should be included here
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.
I don't understand, ClassPathResourceLoader is in the core module and has nothing to do with environment configuration. Why would it go there?
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.
Because the main issue is about the classpath files being loaded and the strategies resolving the conflicts etc. I don't think the DefaultEnvironment should be responsible for that logic. Considering there might be different consumers of the classpath files that might also want to react to similar problems. One of them being reported is actually the database flyway migrations being in multiple jars. Not sure if it's mostly about the properties or if some of the frameworks what to load multiple different files/configurations of the same name from the classpath.




Fixes #11703
AI was used in the production for this PR.
Summary
application.yml,application.properties).ConfigurationExceptionlisting all conflicting resource locations.mergeOrderby jar name patterns).Key changes
ConfigurationLoadStrategy/ConfigurationLoadStrategyTypeApplicationContextBuilder#configurationLoadingStrategy(...)(propagated throughMicronaut)mergeOrder), and env-specific resourcesMigration / behavior change
If you relied on “first match wins” when duplicates exist, configure: