codeconsole commented on code in PR #16771:
URL: https://github.com/apache/grails-core/pull/16771#discussion_r4416009530


##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -171,6 +177,204 @@ 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
+     * in the nested map that owns it, so that {@code navigate('a', 'rows')} 
and {@code config.a.rows} read it. The list
+     * the owner already holds is the one held, as written element by element 
from application.yml or by a profile's
+     * list over it; only when it holds none is it built from the list under 
the flat key. An 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. Only the element just written is 
updated, so each indexed key costs the same
+     * however long the list is. The flat keys are left as they are, so the 
environment presents the same names and
+     * values as before.
+     */
+    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)
+        if (owner == null) {
+            return
+        }
+        String name = containerKey.substring(lastDot + 1)
+        String written = indexedKey.substring(subscriptStart + 1, subscriptEnd)
+        String writtenField = indexedKey.substring(subscriptEnd + 2)
+        Object container = targetMap.get(containerKey)
+        Object current = owner.get(name)
+        if (container instanceof List) {
+            if (!written.isInteger()) {
+                return
+            }
+            HeldList held
+            if (current instanceof HeldList) {
+                held = (HeldList) current
+            } else {
+                boolean fromFlatKey = !(current instanceof List)
+                List source = fromFlatKey ? (List) container : (List) current
+                held = new HeldList()
+                for (int i = 0; i < source.size(); i++) {
+                    String index = String.valueOf(i)
+                    Object element = fromFlatKey ? nestedOrElement(owner, 
"${name}[${index}]".toString(), source.get(i)) : source.get(i)
+                    held.add(index == written ? element : 
heldElement(targetMap, owner, containerKey, name, index, element, null))
+                }
+            }
+            int index = written.toInteger()
+            Object element = index < held.size() ? held.get(index) : null
+            if (element == null && index < ((List) container).size()) {
+                element = ((List) container).get(index)
+            }
+            fill(held, index, null)
+            held.set(index, heldElement(targetMap, owner, containerKey, name, 
written, element, writtenField))
+            owner.put(name, held)
+        } else if (container instanceof Map) {
+            NavigableMap held
+            if (current instanceof NavigableMap) {
+                held = (NavigableMap) current
+            } else {
+                boolean fromFlatKey = !(current instanceof Map)
+                Map source = fromFlatKey ? (Map) container : (Map) current
+                held = new NavigableMap(owner.getRootConfig(), 
(owner.getPath() + [name]).asImmutable())
+                for (Map.Entry entry in ((Map) source).entrySet()) {
+                    String key = String.valueOf(entry.key)
+                    Object element = fromFlatKey ? nestedOrElement(owner, 
"${name}[${key}]".toString(), entry.value) : entry.value
+                    held.put(key, key == written ? element : 
heldElement(targetMap, owner, containerKey, name, key, element, null))
+                }
+            }
+            Object element = held.containsKey(written) ? held.get(written) : 
((Map) container).get(written)
+            held.put(written, heldElement(targetMap, owner, containerKey, 
name, written, element, writtenField))
+            owner.put(name, held)
+        }
+    }
+
+    /**
+     * The element to hold, which the owner also holds under its indexed name, 
{@code rows[0]}. An element that is a
+     * nested map is kept, and so is the nested map the owner holds for the 
element just written when it is that same
+     * map, since the merge has written the field into it. Otherwise an 
element that is a map is held as a nested map
+     * built from it, 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. The field just written then
+     * takes the value the owner's nested map holds for it, which the merge 
stored through
+     * {@link #mergeMapEntry(NavigableMap, String, Object)}, so a placeholder 
in it is resolved as in every other entry,
+     * and the keys of the map the indexed key was merged into that held the 
element, or a map inside it, such as
+     * {@code a.rows[0]} and {@code a.rows[0].db}, then hold the rebuilt 
element and its maps.
+     */
+    private static Object heldElement(NavigableMap targetMap, NavigableMap 
owner, String containerKey, String name, String index,
+                                      Object element, String writtenField) {
+        String indexedName = "${name}[${index}]".toString()
+        Object written = writtenField != null ? owner.get(indexedName) : null
+        if (written instanceof NavigableMap && (element == null || 
element.is(written))) {
+            Object stored = valueAt(written, writtenField)
+            if (stored != null && writtenField.indexOf('.') > -1) {
+                ((NavigableMap) written).merge([(writtenField): stored], true)
+            }
+            return written

Review Comment:
   Fixed in 3f917458a6. An element that is not a map is now replaced by the map 
the indexed key wrote. After your two merges, `navigate('a', 'rows')` and 
`getProperty('a.rows', List)` give `[[name: 'first'], [name: 'second']]`, and 
dot notation and `getProperty('a.rows[1].name')` read `second`. Your sequence 
is covered in `NavigableMapSpec` and `PropertySourcesConfigSpec`, and both fail 
at f02b0f1287. The `grails-bootstrap`, `grails-core` and 
`grails-web-url-mappings` 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]

Reply via email to