codeconsole commented on code in PR #16771:
URL: https://github.com/apache/grails-core/pull/16771#discussion_r4390231339
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -171,6 +177,84 @@ 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. Each element that is a map is held as a
nested map, which the owner also holds
+ * under the indexed name, {@code rows[0]}, and which holds the element as
application.yml gives it. 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.
+ */
+ 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)
+ String written = indexedKey.substring(subscriptStart + 1, subscriptEnd)
+ if (container instanceof List) {
+ List flatList = (List) container
+ List held = new HeldList()
+ for (int i = 0; i < flatList.size(); i++) {
+ held.add(heldElement(owner, "${name}[${i}]".toString(),
flatList.get(i), String.valueOf(i) == written))
+ }
+ owner.put(name, held)
+ targetMap.put(containerKey, held)
+ } else if (container instanceof Map) {
+ Object current = owner.get(name)
+ NavigableMap held = current instanceof NavigableMap ?
(NavigableMap) current : new NavigableMap(owner.getRootConfig(),
(owner.getPath() + [name]).asImmutable())
+ for (Map.Entry entry in ((Map) container).entrySet()) {
+ String key = String.valueOf(entry.key)
+ held.put(key, heldElement(owner, "${name}[${key}]".toString(),
entry.value, key == written))
+ }
+ owner.put(name, held)
+ targetMap.put(containerKey, held)
+ }
+ }
+
+ /**
+ * The element to hold, which the owner also holds under its indexed name.
An element that is a map is held as a
+ * nested map with the entries of the element, built again for the element
just written, so that it holds every
+ * field the element holds, such as {@code db.url} beside the {@code
db.port} just written below it, and none of an
+ * element the list held before it was replaced. A nested map already held
for another element is kept.
+ */
+ private static Object heldElement(NavigableMap owner, String indexedName,
Object element, boolean written) {
+ if (!(element instanceof Map)) {
+ return element
+ }
+ NavigableMap held
+ if (element instanceof NavigableMap && !written) {
+ held = (NavigableMap) element
+ } else {
+ held = new NavigableMap(owner.getRootConfig(), (owner.getPath() +
[indexedName]).asImmutable())
Review Comment:
Fixed in d6b55cba4d.
**The placeholder.** The rebuilt element took its values from the list's
element, which the subscript write fills with the raw value. Now the field just
written takes the value held by the nested map the merge built for the element.
That nested map was written through the hook that resolves placeholders. With
`x=resolved`, `getProperty('app.rows[0].name')`, `getProperty('app.rows',
List)`, `navigate`, `config['app.rows[0].name']` and dot notation all read
`resolved`. `getProperty('app.rows', List)` gave `${x}` on 7.0.x too, so the
list agrees now where it didn't before.
**The flat alias from your review.** The rebuilt element replaced the
owner's `rows[0]`, but not the keys the merge wrote for it. Those keys,
`a.rows[0]` and `a.rows[0].db`, now hold the rebuilt element and its maps. Only
keys that already exist are touched, so the names the environment sees don't
change.
Tests:
- `PropertySourcesConfigSpec` has a `where:` with the placeholder written
alone, beside other fields, and after the list was replaced. Each row asserts
that the indexed key, the list, the nested maps and the flat key agree, and
that `config['app.rows[0]']` is the element `navigate` returns.
- The `db` feature in `NavigableMapSpec` asserts that `a.rows[0]` and
`a.rows[0].db` are the list's maps.
All of them fail at bed3ae249f, the previous head. Both modules' suites pass.
--
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]
