You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jdaugherty (via GitHub)" <gi...@apache.org> on 2026/10/07 18:03:55 UTC
[PR] Resolve placeholders in scalar configuration lists [grails-core]
jdaugherty opened a new pull request, #16552:
URL: https://github.com/apache/grails-core/pull/16552
## Description
Fixes #16550.
Expose non-empty scalar configuration lists through indexed properties so Spring Boot resolves each element's placeholders when binding `@ConfigurationProperties`. Previously the binder received the whole list and copied string elements without resolving their placeholders.
- Extend the existing indexed object-list handling to scalar and nested scalar lists, while preserving empty-list binding and whole-list access through Grails `Config`.
- Use Spring Boot's binder for external configuration locations, banner version options, and bridged Spring Security user roles so these consumers continue to work with indexed lists.
- Cover YAML and Groovy configuration, placeholder defaults and environment overrides, typed and nested lists, empty values, list precedence, banner output, external configuration loading, and user-role binding.
- Document the behavior and the migration from whole-list `Environment.getProperty` lookups in the configuration guide and Grails 8 upgrade notes.
### Validation
- Full affected-module tests passed for `grails-core`, `grails-spring-security`, and the external-configuration example, including its integration tests.
- Repository-wide pre-commit validation passed: `./gradlew clean aggregateViolations :grails-test-report:check --continue --max-workers=4 -PmaxTestParallel=2 --console=plain`.
- 23,468 tests reported: zero failures or errors, 420 skipped.
- Checkstyle, CodeNarc, repository conventions, and enabled PMD checks are clean. SpotBugs is disabled.
- `git diff --check`.
- Additional non-test validation is still running locally: `buildHealth`, followed by `:grails-doc:publishGuide -x aggregateGroovydoc` and `validateRepositoryConventions`. These results are pending; they are not included in the passed checks above.
## Contributor Checklist
### Issue and Scope
- [x] This PR is linked to an existing issue that has been **acknowledged or approved** by the project team. If no approved issue exists, please give background on why this change is necessary. Tickets are preferred for release change log history.
- [x] This PR addresses the **complete scope** of the linked issue. Partial implementations or unfinished work should not be submitted for review.
- [x] This PR contains a **single, focused change**. Unrelated changes should be submitted as separate pull requests.
- [x] This PR targets the **correct branch** for the type of change:
- **Patch release branches** (e.g., `7.0.x`): Bug fixes only. No new features or API changes.
- **Minor release branches** (e.g., `7.1.x`): New features are welcome, but breaking existing APIs must be avoided.
- **Major release branches** (e.g., `8.0.x`): Reserved for major changes. Breaking API changes are permitted.
### Code Quality
- [x] I have **added or updated tests** that cover the changes introduced in this PR. All code contributions are expected to include appropriate test coverage.
- [ ] I have verified that all existing tests pass by running `./gradlew build --rerun-tasks`.
- The full test suite passed via `./gradlew clean aggregateViolations :grails-test-report:check --continue` with the worker limits shown above. CI will perform the build checks.
- [x] My code follows the project's **code style** guidelines. The affected modules' `codeStyle` tasks and repository-wide `aggregateViolations` checks passed.
- [x] This PR does **not** include mass reformatting, style-only changes, or large-scale refactoring unless it was **explicitly approved** in the linked issue. Unsolicited reformatting will not be accepted.
### Licensing and Attribution
- [x] All contributed code is provided under the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0), and new source files include the appropriate **Apache license header**.
- [x] I have the necessary rights to submit this contribution and confirm it is my own original work (see [Legal Notice](https://github.com/apache/grails-core/blob/8.0.x/CONTRIBUTING.md#i-want-to-contribute)).
### Documentation
- [x] If this PR introduces user-facing changes, I have included or updated the relevant documentation.
- [x] If this PR adds a new feature, I have updated the **What's New** section of the Grails Guide. — Not applicable: bug fix.
- [x] If this PR introduces breaking changes or changes that require user action during an upgrade, I have updated the **Upgrade Notes** for the corresponding version in the Grails Guide.
- [x] The PR description clearly explains **what** was changed and **why**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16552:
URL: https://github.com/apache/grails-core/pull/16552#discussion_r4214778842
##########
grails-core/src/main/groovy/grails/config/external/ExternalConfigRunListener.groovy:
##########
@@ -92,10 +94,11 @@ class ExternalConfigRunListener implements SpringApplicationRunListener {
// Resolve final locations, taking into account user home prefix and file wildcards
private List<Object> getLocations(ConfigurableEnvironment environment) {
- List<Object> locations = environment.getProperty('grails.config.locations', List, []) as List<Object>
+ Binder binder = Binder.get(environment)
+ List<Object> locations = binder.bind('grails.config.locations', Bindable.listOf(Object)).orElse([])
// See if grails.config.locations is defined in an environments block like 'development' or 'test'
String environmentString = "environments.${Environment.current.name}.grails.config.locations"
- locations = environment.getProperty(environmentString, List, locations)
+ locations = binder.bind(environmentString, Bindable.listOf(Object)).orElse(locations)
Review Comment:
This bind uses the current environment name as a property segment. A supported custom environment such as `-Dgrails.env=UAT` makes Binder throw InvalidConfigurationPropertyNameException, because the name is not a canonical lowercase property. Camel-case and underscore names fail the same way. The listener runs this bind even when no external locations are configured, so those applications fail during startup. Look the locations up in a way that still accepts custom environment names.
##########
grails-core/src/test/groovy/org/grails/config/YamlPropertySourceLoaderSpec.groovy:
##########
@@ -157,6 +164,53 @@ class YamlPropertySourceLoaderSpec extends Specification {
config.getProperty('app.names', List) == ['p', 'q']
}
+ def "resolves placeholders in YAML scalar lists bound to configuration properties with environment #variables"() {
+ given:
+ def source = load('''\
+ app:
+ allowedOrigins:
+ - https://static.example.com
+ - "${EXAMPLE_ALLOWED_ORIGIN:https://default.example.com}"
+ ports: [8080, "${EXAMPLE_PORT:9090}"]
+ flags: [true, "${EXAMPLE_FLAG:false}"]
+ groups: [["${EXAMPLE_GROUP:primary}"], [secondary]]
Review Comment:
This nested list does not survive flattening. The loader uses Spring keys such as `app.groups[0][0]`, and NavigableMap.mergeMapEntry() keeps the text after the first index at subscriptEnd + 2. Consecutive indexes drop the second `[`, so the property source exposes `app.groups[0].0]` instead of `app.groups[0][0]`, and Boot cannot bind List<List<String>>. Fix consecutive-index parsing, and keep a test that fails when the nested list is flattened incorrectly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #16552:
URL: https://github.com/apache/grails-core/pull/16552
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16552:
URL: https://github.com/apache/grails-core/pull/16552#issuecomment-6045387275
## 🚨 All tests passed but jobs failed 🚨
### Failed Jobs without Test Failures
❌ [Build Health / Core Projects](https://github.com/apache/grails-core/actions/runs/37663932396/job/112938221050?pr=16552)
🏷️ Commit: 7fd5b1e82b2dd65c700b2faadfcb3d47ee87e245
▶️ Tests: 109576 executed
🟡 Checks: 78/84 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16552:
URL: https://github.com/apache/grails-core/pull/16552#discussion_r4220949473
##########
grails-core/src/main/groovy/grails/config/external/ExternalConfigRunListener.groovy:
##########
@@ -92,10 +94,11 @@ class ExternalConfigRunListener implements SpringApplicationRunListener {
// Resolve final locations, taking into account user home prefix and file wildcards
private List<Object> getLocations(ConfigurableEnvironment environment) {
- List<Object> locations = environment.getProperty('grails.config.locations', List, []) as List<Object>
+ Binder binder = Binder.get(environment)
+ List<Object> locations = binder.bind('grails.config.locations', Bindable.listOf(Object)).orElse([])
// See if grails.config.locations is defined in an environments block like 'development' or 'test'
String environmentString = "environments.${Environment.current.name}.grails.config.locations"
- locations = environment.getProperty(environmentString, List, locations)
+ locations = binder.bind(environmentString, Bindable.listOf(Object)).orElse(locations)
Review Comment:
Custom environment names are adapted before they are bound, including uppercase, camel-case, and underscore forms.
##########
grails-core/src/test/groovy/org/grails/config/YamlPropertySourceLoaderSpec.groovy:
##########
@@ -157,6 +164,53 @@ class YamlPropertySourceLoaderSpec extends Specification {
config.getProperty('app.names', List) == ['p', 'q']
}
+ def "resolves placeholders in YAML scalar lists bound to configuration properties with environment #variables"() {
+ given:
+ def source = load('''\
+ app:
+ allowedOrigins:
+ - https://static.example.com
+ - "${EXAMPLE_ALLOWED_ORIGIN:https://default.example.com}"
+ ports: [8080, "${EXAMPLE_PORT:9090}"]
+ flags: [true, "${EXAMPLE_FLAG:false}"]
+ groups: [["${EXAMPLE_GROUP:primary}"], [secondary]]
Review Comment:
Nested list indexes are rebuilt from the Spring-style keys, and the test covers forms such as app.groups[0][1].
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16552:
URL: https://github.com/apache/grails-core/pull/16552#discussion_r4218420987
##########
grails-core/src/main/groovy/grails/config/external/ExternalConfigRunListener.groovy:
##########
@@ -92,10 +94,11 @@ class ExternalConfigRunListener implements SpringApplicationRunListener {
// Resolve final locations, taking into account user home prefix and file wildcards
private List<Object> getLocations(ConfigurableEnvironment environment) {
- List<Object> locations = environment.getProperty('grails.config.locations', List, []) as List<Object>
+ Binder binder = Binder.get(environment)
+ List<Object> locations = binder.bind('grails.config.locations', Bindable.listOf(Object)).orElse([])
// See if grails.config.locations is defined in an environments block like 'development' or 'test'
String environmentString = "environments.${Environment.current.name}.grails.config.locations"
- locations = environment.getProperty(environmentString, List, locations)
+ locations = binder.bind(environmentString, Bindable.listOf(Object)).orElse(locations)
Review Comment:
Fixed in 8c08cab4a7. The environment-specific lookup now binds `ConfigurationPropertyName.adapt("environments.<env>.grails.config.locations", '.')`, which accepts mixed-case and underscore names, so custom environments no longer throw. `ExternalConfigSpec` now loads environment-specific locations for `UAT`, `myCustomEnv`, and `custom_env`, and starts a `UAT` environment with no locations configured; all four fail against the previous code. The upgrade note now also says that `Binder` needs canonical names and to use `ConfigurationPropertyName.adapt` for names built from values such as the environment name.
##########
grails-core/src/test/groovy/org/grails/config/YamlPropertySourceLoaderSpec.groovy:
##########
@@ -157,6 +164,53 @@ class YamlPropertySourceLoaderSpec extends Specification {
config.getProperty('app.names', List) == ['p', 'q']
}
+ def "resolves placeholders in YAML scalar lists bound to configuration properties with environment #variables"() {
+ given:
+ def source = load('''\
+ app:
+ allowedOrigins:
+ - https://static.example.com
+ - "${EXAMPLE_ALLOWED_ORIGIN:https://default.example.com}"
+ ports: [8080, "${EXAMPLE_PORT:9090}"]
+ flags: [true, "${EXAMPLE_FLAG:false}"]
+ groups: [["${EXAMPLE_GROUP:primary}"], [secondary]]
Review Comment:
Fixed in 8c08cab4a7. `NavigableMap.mergeMapEntry` now handles any number of consecutive subscripts, with or without a trailing property (`[0][1]`, `[0][1].name`, `[api][0]`), so `[[a, b], [c]]` becomes a list of lists and is exposed as `app.groups[0][0]`, `app.groups[0][1]`, `app.groups[1][0]`. The old test only passed because Boot's lenient name adaptation strips the stray `]`. The new `YamlPropertySourceLoaderSpec` feature asserts the exact flattened property names (including a three-level list and objects inside a nested list), environment reads by indexed name, `List<List<…>>` binding, and whole-list reads through the Grails config. `NavigableMapSpec` covers the merge directly. All of these fail against the previous parser.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16552:
URL: https://github.com/apache/grails-core/pull/16552#issuecomment-6044580871
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16552?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 59.5784%. Comparing base ([`2d6e2b4`](https://app.codecov.io/gh/apache/grails-core/commit/2d6e2b46aa669aa2682c7616ee6e4b1d13192407?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7fd5b1e`](https://app.codecov.io/gh/apache/grails-core/commit/7fd5b1e82b2dd65c700b2faadfcb3d47ee87e245?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16552 +/- ##
==================================================
+ Coverage 59.5580% 59.5784% +0.0204%
- Complexity 25045 25051 +6
==================================================
Files 2175 2175
Lines 108694 108692 -2
Branches 19835 19834 -1
==================================================
+ Hits 64736 64757 +21
+ Misses 35010 34985 -25
- Partials 8948 8950 +2
```
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16552?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...re/src/main/groovy/grails/boot/GrailsBanner.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16552?src=pr&el=tree&filepath=grails-core%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fboot%2FGrailsBanner.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L2dyYWlscy9ib290L0dyYWlsc0Jhbm5lci5ncm9vdnk=) | `80.7910% <100.0000%> (+6.2147%)` | :arrow_up: |
| [...s/config/external/ExternalConfigRunListener.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16552?src=pr&el=tree&filepath=grails-core%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fconfig%2Fexternal%2FExternalConfigRunListener.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L2dyYWlscy9jb25maWcvZXh0ZXJuYWwvRXh0ZXJuYWxDb25maWdSdW5MaXN0ZW5lci5ncm9vdnk=) | `18.6667% <100.0000%> (+1.0991%)` | :arrow_up: |
| [...rg/grails/config/NavigableMapPropertySource.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16552?src=pr&el=tree&filepath=grails-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fconfig%2FNavigableMapPropertySource.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvY29uZmlnL05hdmlnYWJsZU1hcFByb3BlcnR5U291cmNlLmdyb292eQ==) | `88.3721% <100.0000%> (+1.4156%)` | :arrow_up: |
... and [4 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16552/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve placeholders in scalar configuration lists [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16552:
URL: https://github.com/apache/grails-core/pull/16552#issuecomment-6047689560
## ✅ All tests passed ✅
🏷️ Commit: d76cfd59c9ddd300eaa781c41af09111f7638593
▶️ Tests: 109805 executed
⚪️ Checks: 91/91 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org