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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
