jamesfredley commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4364488513
##########
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
+ // key answers nearly every lookup without a walk; where the two
disagree the flat key wins.
+ value = configMap.get(key);
Review Comment:
`merge(['a.b.c': 1])`, then `merge([a: 'scalar'])`, then `merge(['a.b.d':
2])` leaves the nested `a.b` as `[d: 2]` while the flat alias stays `[c: 1]`.
`getProperty('a.b', Map)` returns the old block.
Replace an existing map alias when the branch is recreated, before flat
lookup treats it as current.
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -336,12 +348,14 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
newPathList.addAll(currentMap.getPath())
newPathList.add(pathElement)
- Map<String, Object> newMap = new
NavigableMap(currentMap.getRootConfig(), newPathList.asImmutable())
+ NavigableMap newMap = new
NavigableMap(currentMap.getRootConfig(), newPathList.asImmutable())
currentMap.put(pathElement, newMap)
- def fullPath = accumulatedPath.toString()
- if (!rootMap.containsKey(fullPath)) {
- rootMap.put(fullPath, newMap)
+ // the flat alias of a new block lives at the configuration
root under the block's full path,
+ // whichever map the walk started on; a scalar an earlier
merge left there is replaced
+ NavigableMap root = currentMap.getRootConfig()
Review Comment:
Cloning an empty map and merging `['new.block.value': 1]` into the copy
inserts `new` and `new.block` into the original, because the alias write uses
`getRootConfig()` and the clone still points at the source.
Write those aliases on the copy. The original should stay empty.
--
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]
