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


##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -420,6 +435,81 @@ class NavigableMap implements Map<String, Object>, 
Cloneable {
         }
     }
 
+    /**
+     * Whether a bracketed flat key exports the entry, so that the walk skips 
its dotted name. The entry {@code /api/**} of
+     * {@code routes} is exported by {@code routes[/api/**]}, held by the 
nearest enclosing nested map, when that key
+     * exports every name and value the entry would, and the walked map holds 
no flat key under the entry's dotted name,
+     * {@code app.routes./api/**}, or under the dotted name of a value below 
the entry, with another value. A flat key of
+     * the map itself, such as {@code app.routes./api/**}, is exported by the 
bracketed key that splits it at one of its
+     * dots, {@code app.routes[/api/**]}, when that key exports the same value 
and no flat key below it disagrees. Where the bracketed key holds less, or a 
value a later write replaced, or the dotted names hold
+     * values that differ, the dotted names are exported too, and the one 
written last is the one exported under them.
+     */
+    private boolean isExportedInBrackets(NavigableMap owner, List<String> 
path, List<String> relativePath, String key, Object value) {
+        if (relativePath) {
+            String bracketedKey = 
"${relativePath.join('.')}[${key}]".toString()
+            if (!owner.containsKey(bracketedKey) || 
!exportsAll(owner.get(bracketedKey), value)) {
+                return false
+            }
+            return agreesWithFlatKeys("${path.join('.')}.${key}".toString(), 
value)
+        }
+        for (int dot = key.indexOf('.'); dot > 0; dot = key.indexOf('.', dot + 
1)) {
+            String bracketedKey = "${key.substring(0, 
dot)}[${key.substring(dot + 1)}]".toString()
+            if (owner.containsKey(bracketedKey) && 
exportsAll(owner.get(bracketedKey), value)) {

Review Comment:
   Fixed in a0ce568a1a, by changing the approach rather than adding a check. 
Each round has found an order of writes where one of the walk's checks trusted 
the wrong alias, so I took the decision out of the walk.
   
   The walk is now 7.0.x's again. `toFlatConfig()` and `toProperties()` then 
remove from its result only the dotted names that duplicate a bracketed one. A 
name such as `app.routes./api/**` is removed only when every bracketed name in 
the result that spells it with dots, here `app.routes[/api/**]`, holds the same 
value. Every value the export holds is therefore the one 7.0.x exported under 
that name, so no order of writes can lose or change one. Where the names 
disagree, all of them stay, as on 7.0.x. Your order exports `a.b.c=new`, 
`a.b[c]=old` and `a[b.c]=new`, as 7.0.x does.
   
   `a[b.c]` names a key of `a`, so the `a.b.c` it spells is a property of its 
own. That dotted name stays whenever the nested maps hold a value at that path.
   
   I checked it against 7.0.x over every sequence of up to three writes from 
eleven kinds: dotted, bracketed at each level, a bracketed key with a dot, a 
nested map, `put`, a map in brackets, a null in brackets, special characters 
and a sibling. In all 1463 sequences, the export equals 7.0.x's with exactly 
those duplicates removed: 959 of them, in 806 sequences.
   
   `ConfigMapSpec` now holds the orders from every round in one `where:` table 
of exact `toProperties()` and `toFlatConfig()` results, your order included. It 
fails at 8fb4cad282, 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]

Reply via email to