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


##########
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:
   `merge(['a.rows[0].name': 'old'], true)` then `merge(['a.rows': [[name: 
'new']]], true)` then `merge(['a.rows[0].port': '8080'], true)` returns 
`[[name: 'old', port: '8080']]`. The old element map is reused. Before these 
commits both lookups returned `name: 'new'`.
   
   Drop the indexed aliases when the list is replaced. Please cover that 
three-step merge.



-- 
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