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


##########
grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java:
##########
@@ -217,67 +222,29 @@ public <T> T getProperty(String key, Class<T> targetType, 
T defaultValue, List<T
         return value;
     }
 
+    /**
+     * Reads the process environment again, so that a variable installed after 
this config was
+     * created, as a test can do reflectively, is seen by the lookups that 
follow.
+     */
+    protected void refreshSystemEnvironment() {
+        systemEnvironment = new SystemEnvironmentIndex(System.getenv());
+    }
+
     @Override
     public <T> T getProperty(String key, Class<T> targetType, T defaultValue) {
-        Object value = findInSystemEnvironment(key);
+        Object value = systemEnvironment.get(key);
         if (value == null) {
-            value = getValueWithDotNotatedKeySupport(configMap, key);
+            // merge and dot-notation assignment write the flat key as well as 
the nested entry, so the flat

Review Comment:
   `config.merge(['app.items[0]': 'old'])` then `config.merge([app: 
['items[0]': 'new']])` leaves `getProperty('app.items', List)` as `['old']` and 
`navigate('app', 'items')` as `['new']`. The second merge updates only the 
nested list.
   
   Keep the indexed list and its flat alias on the same value before the 
flat-key lookup.



##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -218,7 +217,7 @@ class NavigableMap implements Map<String, Object>, 
Cloneable {
                 targetMap.remove(sourceKey)
             } else {
                 if (path) {
-                    rootMap.put("${path}.${sourceKey}".toString(), newValue)
+                    mergeMapEntry(rootMap, "${path}.${sourceKey}".toString(), 
newValue)

Review Comment:
   Merging `[auth: [secret: '${random.uuid}']]` resolves the placeholder once 
for the flat entry and again for the nested entry. `getProperty('auth.secret')` 
and `getProperty('auth', Map).secret` are different values.
   
   Resolve the placeholder once and store that same value in both places.



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