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


##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -271,6 +351,13 @@ class NavigableMap implements Map<String, Object>, 
Cloneable {
                 }
                 targetMap.remove(sourceKey)
             } else {
+                if (currentValue instanceof Collection || (currentValue 
instanceof Map && !(sourceValue instanceof Map))) {

Review Comment:
   YAML `app.rows: [{name: first}]` makes `getProperty('app.rows[0].name')` 
return null. Importing the source copies `rows[0]` and then this cleanup 
deletes it.
   
   Keep that alias. Please load that YAML and assert the name is `first`.



##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -169,6 +171,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: 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)))
+            }
+            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()) {
+                held.put(String.valueOf(entry.key), 
heldElement(owner.get("${name}[${entry.key}]".toString()), entry.value))
+            }
+            owner.put(name, held)
+            targetMap.put(containerKey, held)
+        }
+    }
+
+    /**
+     * Removes the keys that indexed keys and map subscripts wrote for the 
entries of a list or map, such as
+     * {@code rows[0]} and {@code rows[0].name} for {@code rows}, so that a 
subscript written after the list or map is
+     * replaced does not find the entries it replaced.
+     */
+    private static void removeSubscriptKeys(NavigableMap map, String key) {
+        String prefix = "${key}[".toString()
+        List<String> subscriptKeys = map.keySet().findAll { String k -> 
k.startsWith(prefix) } as List<String>
+        for (String subscriptKey in subscriptKeys) {
+            map.remove(subscriptKey)
+        }
+    }
+
+    /**
+     * The element to hold: the nested map built for it, which also takes the 
entries of the flat key's element, such as
+     * {@code db.url} beside {@code db}, as an element from application.yml 
holds both, or the flat key's element itself.
+     */
+    private static Object heldElement(Object nested, Object flat) {
+        if (!(nested instanceof NavigableMap)) {
+            return flat
+        }
+        NavigableMap element = (NavigableMap) nested
+        if (flat instanceof Map) {
+            for (Map.Entry entry in ((Map) flat).entrySet()) {
+                String key = String.valueOf(entry.key)
+                if (!element.containsKey(key)) {

Review Comment:
   `merge(['a.rows': [[db: [url: 'jdbc:new', user: 'me']]]], true)` then 
`merge(['a.rows[0].db.port': '8080'], true)` drops `db.url`. The new element 
already has `db`, so the old subtree is skipped.
   
   Keep the sibling fields. Please assert `url` is still there.



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