codeconsole commented on code in PR #16771:
URL: https://github.com/apache/grails-core/pull/16771#discussion_r4383232035
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -169,6 +171,28 @@ 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; splitting the key at every dot wrote
only an entry named after the element there,
+ * {@code rows[0]}, which the flat keys of the element are read through
and which is left as it is.
+ */
+ 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)
+ Object container = targetMap.get(containerKey)
+ int lastDot = containerKey.lastIndexOf('.')
+ NavigableMap owner = lastDot > -1 ?
targetMap.navigateSubMap(containerKey.substring(0, lastDot).tokenize('.'),
false) : targetMap
+ if (owner == null || owner.is(owner.getRootConfig()) || !(container
instanceof List || container instanceof Map)) {
+ return
+ }
+ owner.put(containerKey.substring(lastDot + 1), container)
Review Comment:
Reworked in f74073598c and c95c114cb5. The held list now holds, for each
element, the nested `rows[i]` map the merge builds, and that map also takes the
entries of the flat key's element. An element is therefore `[name:first,
db:[url:jdbc:x], db.url:jdbc:x]`, the same as from application.yml. With your
Boot-style source, `config.app.rows[0].db.url` is `jdbc:x`, and
`config.app.rows[1].missing.deeper ?: 'default'` is `'default'` again. These
features cover it:
- "an element of a list written by indexed keys is read through dot notation
as deep as it goes" (your example)
- "a list of maps written by indexed keys gives the same list as
application.yml"
In 82d4fdde6a the flat key `app.rows` holds that same list as well, so
`config['app.rows']` matches application.yml too.
One change to flag: `getProperty(key, List)` gave the flat key's plain maps
for indexed keys, and the nested maps for application.yml. To keep the
indexed-keys result that `PropertySourceConfigSpec` pins, `NavigableMapConfig`
now gives the `NavigableMap` elements of a list as plain maps, the way
`getProperty(key, Map)` gives a map. That also changes application.yml lists
read through `getProperty(key, List)`. "A list of maps is given as plain maps
by getProperty and read through dot notation" covers both sources.
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -169,6 +171,28 @@ 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; splitting the key at every dot wrote
only an entry named after the element there,
+ * {@code rows[0]}, which the flat keys of the element are read through
and which is left as it is.
+ */
+ 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)
+ Object container = targetMap.get(containerKey)
+ int lastDot = containerKey.lastIndexOf('.')
+ NavigableMap owner = lastDot > -1 ?
targetMap.navigateSubMap(containerKey.substring(0, lastDot).tokenize('.'),
false) : targetMap
+ if (owner == null || owner.is(owner.getRootConfig()) || !(container
instanceof List || container instanceof Map)) {
Review Comment:
Covered in f74073598c and 82d4fdde6a. A map written by subscripts is now
held as a nested map, so a later dotted key writes into it. The flat key
`app.mappings` holds that same map, as the flat key of a nested map does, so
both reads agree. There are two tests:
- `NavigableMapSpec`, "a map written by an indexed key with a property after
the index keeps its entries when a dotted key is merged after it", merges
`a.mappings[foo].name` and then `a.mappings.bar`. It asserts that
`navigate('a', 'mappings')` is `[foo: [name: 'x'], bar: 'y']` and that
`get('a.mappings')` is the same instance.
- `PropertySourcesConfigSpec`, "a map written by indexed keys is read
through dot notation, and a key it does not hold is null-safe", asserts
`config.app.mappings.foo.name == 'x'` and that `config.app.mappings.bar.deeper`
is a null-safe miss. After `app.mappings.bar` is merged, it asserts that
`navigate` and `config['app.mappings']` both hold `foo` and `bar`.
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -169,6 +171,28 @@ 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; splitting the key at every dot wrote
only an entry named after the element there,
+ * {@code rows[0]}, which the flat keys of the element are read through
and which is left as it is.
+ */
+ 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)
+ Object container = targetMap.get(containerKey)
+ int lastDot = containerKey.lastIndexOf('.')
+ NavigableMap owner = lastDot > -1 ?
targetMap.navigateSubMap(containerKey.substring(0, lastDot).tokenize('.'),
false) : targetMap
Review Comment:
Done in f74073598c: `holdIndexedContainer` returns when `lastDot < 0`, which
covers both branches. I removed the "in a nested map" row, since it passed
without the change.
--
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]
