codeconsole commented on code in PR #16771:
URL: https://github.com/apache/grails-core/pull/16771#discussion_r4384027345
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -169,6 +171,71 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
}
}
+ /**
+ * Holds the list or map that an indexed key with a property after the
index, such as {@code a.rows[0].name}, writes
+ * under its flat key, {@code a.rows}, in the nested map that owns it too,
so that {@code navigate('a', 'rows')} and
+ * {@code config.a.rows} read it: a list or map of the nested maps that
splitting the key at every dot built there for
+ * each element, {@code rows[0]}, which hold the element as
application.yml gives it and are left as they are. The flat
+ * key {@code a.rows} then holds the same list or map, as the flat key of
a nested map holds that map, so a dotted key
+ * merged into it later is read through both. The flat keys of the
elements are left as they are.
+ */
+ private void holdIndexedContainer(NavigableMap targetMap, String
indexedKey) {
+ int subscriptStart = indexedKey.indexOf('[')
+ int subscriptEnd = subscriptStart > -1 ? indexedKey.indexOf(']',
subscriptStart) : -1
+ if (subscriptEnd < 0 || indexedKey.indexOf('.', subscriptEnd) < 0) {
+ return
+ }
+ String containerKey = indexedKey.substring(0, subscriptStart)
+ int lastDot = containerKey.lastIndexOf('.')
+ if (lastDot < 0) {
+ // the write of the container has put it in the map that owns it
already
+ return
+ }
+ NavigableMap owner =
targetMap.navigateSubMap(containerKey.substring(0, lastDot).tokenize('.'),
false)
+ Object container = targetMap.get(containerKey)
+ if (owner == null) {
+ return
+ }
+ String name = containerKey.substring(lastDot + 1)
+ if (container instanceof List) {
+ List flatList = (List) container
+ List held = []
+ for (int i = 0; i < flatList.size(); i++) {
+ held.add(heldElement(owner.get("${name}[${i}]".toString()),
flatList.get(i)))
Review Comment:
Fixed in e47c7dabd5. A write that replaces a list now removes the keys its
elements were written under: `a.rows[0]` and `a.rows[0].name` at the root, and
`rows[0]` in the `a` map. The same applies when a list or map is replaced by a
value that is not a map. A map merged into a map keeps them, since its entries
stay. After your three steps, `navigate('a', 'rows')`, `getProperty('a.rows',
List)`, `config['a.rows']`, `getProperty('a.rows[0].name')` and
`config.a.rows[0].name` all give `[name: 'new', port: '8080']`.
One change from 7.0.x comes with it: after `merge(['a.rows[0].name':
'old'])` and `merge(['a.rows': [[name: 'new']]])`,
`getProperty('a.rows[0].name')` is now null where 7.0.x still answered `'old'`.
That is what a list merged on its own gives, since it has no indexed keys.
Tests:
- `NavigableMapSpec` covers the three-step merge with the list replaced as a
dotted key and in a nested map.
- `NavigableMapSpec` checks that no key is left at the root or in `a` when a
list is replaced by a list or a value, and when a map is replaced by a value.
- `PropertySourcesConfigSpec` reads your case through every path.
All of them fail without the commit. 383a3a776b merges 7.0.x in, now that
#16770 has landed.
--
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]
