codeconsole commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4365148952
##########
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:
Fixed in 850c97ba0f. `navigateSubMap` kept the root alias of a block it
created whenever the alias already held a map, which is right only while that
map is the nested block; after `merge([a: 'scalar'])` destroyed the nested
branch the alias `a.b` still held the old block, and the recreation left it
there. A block merged as a map value already replaced its alias
unconditionally, and the dotted key path now does the same: whatever the alias
holds when the nested block has to be created is stale. After `merge(['a.b.c':
1])`, `merge([a: 'scalar'])`, `merge(['a.b.d': 2])`, `getProperty('a.b', Map)`
and `navigate('a', 'b')` are both `[d: 2]`. The flat `a.b.c` of the destroyed
block stays readable as `1`, as the description says a lower-precedence key
does when a higher-precedence scalar replaces its parent; the recreated block
does not pick it up. Pinned in `PropertySourcesConfigSpec` and
`NavigableMapSpec`.
##########
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:
Fixed in f4f6485171. `clone()` copied the entries into a map whose
`rootConfig` was still the original, and shared the nested maps with it, so
once the alias write followed `getRootConfig()` a merge into the copy wrote the
original's root, and a dot-notation write below a shared block, `copy.a.b.d =
2`, had always written the original's nested map and its root alias. The copy
of a root is now the root of its own configuration and its nested maps are
copies under it, with a map the original holds twice, as the nested entry and
the flat alias of a block, copied once so the copy keeps both representations
on one map; the copy of a nested map stays under the root of the original, as
before. Merging `['new.block.value': 1]` into the clone of an empty map leaves
the original empty, and `copy.a.b.d = 2`, `copy.merge(['a.e.f': 3])` and
`copy.put('g', 4)` leave the original's nested maps and flat keys untouched
while the copy reads each through both representations. Pinned in `NavigableMap
Spec`; `CodeGenConfig.clone()` builds its copy by merging and is unchanged.
--
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]
