codeconsole commented on code in PR #16769:
URL: https://github.com/apache/grails-core/pull/16769#discussion_r4383230845
##########
grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy:
##########
@@ -116,4 +116,21 @@ class PropertySourcesConfigSpec extends Specification {
then: 'the second value is not merged with the first value'
cfg.foo.bar == ['two': '2']
}
+
+ void 'a dotted key inside a nested map is exported under its full path and
overwrites no other property'() {
+ given: "a map holding a property and, in a nested map, a dotted key
that alone names the same property"
+ def config = new PropertySourcesConfig([a: [b: 'root'], x: ['a.b':
'nested'], logging: [level: ['org.springframework': 'DEBUG']]])
+
+ expect: "each property is read and exported under its own name"
+ config.getProperty('a.b') == 'root'
Review Comment:
Added in 20ca350112. This feature now also asserts `config['a.b'] ==
'root'`, so its overwrite half fails without the fix as well. A second feature,
"a dotted key inside a nested map writes no property that the nested maps do
not hold", builds `[a: [c: '1'], x: ['a.b': 'nested']]` and asserts that
`getProperty('a.b')`, `toProperties().getProperty('a.b')` and `navigate('a',
'b')` are all null.
The description is corrected too. I didn't rewrite f54b426242 on a branch
already under review, so 20ca350112's message says what the first one got
wrong: that `getProperty('a.b')` and `toProperties()` already answered `root`
for that example, and that `config['a.b']` is the read the overwrite broke.
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -159,7 +159,9 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
def pathParts = keyParts[0..-2]
Map actualTarget = targetMap.navigateSubMap(pathParts as
List, true)
sourceKey = keyParts[-1]
- mergeMapEntry(rootMap, pathParts.join('.'), actualTarget,
sourceKey, sourceValue, parseFlatKeys)
+ // the flat key of the entry is its full path: that of the
map holding the dotted key, then the key
+ String nestedPath = path ?
"${path}.${pathParts.join('.')}" : pathParts.join('.')
+ mergeMapEntry(rootMap, nestedPath, actualTarget,
sourceKey, sourceValue, parseFlatKeys)
Review Comment:
Covered in 20ca350112. "A dotted key inside a nested map holding
#description writes the flat keys of its full path only" is a `where:` table in
`NavigableMapSpec` that asserts the root key set for each shape:
| merged | root keys |
|---|---|
| `[x: ['a.b': [c: 'v']]]` | `x`, `x.a.b`, `x.a.b.c` |
| `[x: ['a.b': ['c.d': 'v']]]` | `x`, `x.a.b`, `x.a.b.c.d` |
| `[x: ['a.b.c': 'v']]` | `x`, `x.a.b.c` |
| `[a: [c: '1'], x: ['a.b': 'v']]` | `a`, `a.c`, `x`, `x.a.b` |
A separate feature asserts that root `x.a.b` is the instance `navigate('x',
'a', 'b')` returns.
--
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]
