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


##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -374,15 +375,119 @@ class NavigableMap implements Map<String, Object>, 
Cloneable {
     Map<String, Object> toFlatConfig() {
         Map<String, Object> flatConfig = [:]
         flattenKeys(flatConfig, this, [], false)
+        removeDottedDuplicates(flatConfig)
         flatConfig
     }
 
     Properties toProperties() {
         Properties properties = new Properties()
         flattenKeys((Map<Object, Object>) properties, this, [], true)
+        removeDottedDuplicates((Map<Object, Object>) properties)
         properties
     }
 
+    /**
+     * Removes each name that only writes a bracketed name with dots, such as 
{@code app.routes./api/**} beside
+     * {@code app.routes[/api/**]}, which Spring Boot would bind as the key 
{@code api}. A map written by map subscripts
+     * holds its entries under their bracketed names and is walked under their 
dotted ones too, so each entry is
+     * flattened under both. A dotted name is removed only when every 
bracketed name that it writes with dots holds the
+     * same value; where they differ, as when a later write changed one of 
them, every name stays, as it was before. A
+     * bracketed key with a dot in it, {@code a[b.c]}, names a key of {@code 
a}, so the dotted name {@code a.b.c} is
+     * a property of its own, and it stays when the map holds it as a key, as 
a put of {@code a.b.c} does, or the nested
+     * maps hold a value under that path.
+     */
+    private void removeDottedDuplicates(Map<? extends Object, Object> 
flatConfig) {
+        Map<String, List<Object>> bracketedValues = [:]
+        Set<String> pathsToCheck = [] as Set<String>
+        for (Map.Entry<? extends Object, Object> entry in 
flatConfig.entrySet()) {
+            String name = String.valueOf(entry.key)
+            if (name.indexOf('[') > -1) {
+                for (Map.Entry<String, Boolean> dottedName in 
dottedNamesOf(name).entrySet()) {
+                    List<Object> values = bracketedValues.get(dottedName.key)
+                    if (values == null) {
+                        values = []
+                        bracketedValues.put(dottedName.key, values)
+                    }
+                    values.add(entry.value)
+                    if (dottedName.value) {
+                        pathsToCheck.add(dottedName.key)
+                    }
+                }
+            }
+        }
+        for (Map.Entry<String, List<Object>> candidate in 
bracketedValues.entrySet()) {
+            if (!flatConfig.containsKey(candidate.key)) {
+                continue
+            }
+            Object value = flatConfig.get(candidate.key)
+            if (!candidate.value.every { Object bracketedValue -> 
bracketedValue == value }) {
+                continue
+            }
+            if (candidate.key in pathsToCheck && (containsKey(candidate.key) 
|| holdsPath(candidate.key.tokenize('.')))) {

Review Comment:
   Fixed in e541a498af, with the lookup matrei suggested in his review. 
`holdsPath` now tries every dotted start of the name that the map holds as a 
key, the whole name included, and walks the rest through maps, still requiring 
the last segment. After `put('a.b', [c: 'x'])` and `merge(['a[b.c]': 'x'], 
true)`, in either order, the export holds both `a.b.c=x` and `a[b.c]=x`. 
`a[b.c]` alone, and `routes[/v1]` beside `routes[/v1.0/**]`, still export only 
the bracketed names.
   
   Both orders are rows in the exact-export table in `ConfigMapSpec`; both fail 
at a3a02b0618 with only `a[b.c]=x`. The `grails-bootstrap` suite and the 
`org.grails.config` and `grails.config` specs in grails-core pass, and so does 
`codeStyle` for both modules.
   
   The one shape matrei noted stays as he described: `put('a', ['b.c': 'x'])` 
stores `a` as `[b.c: x]`, which is what merging `a[b.c]` stores, so after both 
the export holds only `a[b.c]`. The javadoc of `removeDottedDuplicates` now 
says so.
   



##########
grails-core/src/main/groovy/org/grails/config/CompositeConfig.java:
##########
@@ -41,6 +41,11 @@
  * A {@link Config} composed of other Configs. A lookup answers from the first 
config, in order of precedence,
  * that has the key, and the {@link Map} views, {@link #flatten()} and {@link 
#toProperties()} resolve a key
  * shared by several configs the same way, so each key appears once with its 
highest-precedence value.
+ * <p>
+ * One case differs: {@link #flatten()} and {@link #toProperties()} combine 
the names each config exports, and a

Review Comment:
   Taken as suggested in a4290530f5. A probe at this head bears the reason out: 
`high.flatten()` holds `app.routes`, `app.routes[k]` and `app`, never 
`app.routes.k`, so `composite.flatten()` answers `low` under the dotted name, 
and only the export changed, so it did before this PR too.
   



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