jdaugherty commented on code in PR #16771:
URL: https://github.com/apache/grails-core/pull/16771#discussion_r4423887657
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -171,6 +177,265 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
}
}
+ /**
+ * Holds the list or map that an indexed key with a property after the
index, such as {@code a.rows[0].name}, writes
+ * in the nested map that owns it, so that {@code navigate('a', 'rows')}
and {@code config.a.rows} read it. The list
+ * the owner already holds is the one held, as written element by element
from application.yml or by a profile's
+ * list over it; only when it holds none is it built from the list under
the flat key. An element that is a map is
+ * held as a nested map, which the owner also holds under the indexed
name, {@code rows[0]}, and which holds the
+ * element as application.yml gives it. Only the element just written is
updated, so each indexed key costs the same
+ * however long the list is. The flat keys are left as they are, so the
environment presents the same names and
+ * values as before.
+ */
+ private void holdIndexedContainer(NavigableMap targetMap, String
indexedKey) {
+ int subscriptStart = indexedKey.indexOf('[')
+ int subscriptEnd = subscriptStart > -1 ? indexedKey.indexOf(']',
subscriptStart) : -1
+ if (subscriptEnd < 0 || indexedKey.indexOf('.', subscriptEnd) < 0) {
+ return
+ }
+ String containerKey = indexedKey.substring(0, subscriptStart)
+ int lastDot = containerKey.lastIndexOf('.')
+ if (lastDot < 0 && targetMap.getPath().isEmpty()) {
+ // the root holds the container under its own key, as the
subscript wrote it, as 7.0.x always did
+ return
+ }
+ // a map below the root, such as an element of a list, owns a
container it holds itself, cols of rows[0].cols[0]
+ NavigableMap owner = lastDot < 0 ? targetMap :
targetMap.navigateSubMap(containerKey.substring(0, lastDot).tokenize('.'),
false)
+ if (owner == null) {
+ return
+ }
+ String name = containerKey.substring(lastDot + 1)
+ String written = indexedKey.substring(subscriptStart + 1, subscriptEnd)
+ String writtenField = indexedKey.substring(subscriptEnd + 2)
+ Object container = targetMap.get(containerKey)
+ Object current = owner.get(name)
+ if (container instanceof List) {
+ if (!written.isInteger()) {
+ return
+ }
+ HeldList held
+ if (current instanceof HeldList) {
+ held = (HeldList) current
+ } else {
+ boolean fromFlatKey = !(current instanceof List)
+ List source = fromFlatKey ? (List) container : (List) current
+ held = new HeldList()
+ for (int i = 0; i < source.size(); i++) {
+ String index = String.valueOf(i)
+ Object element = fromFlatKey ? nestedOrElement(owner,
"${name}[${index}]".toString(), source.get(i)) : source.get(i)
+ held.add(index == written ? element :
heldElement(targetMap, owner, containerKey, name, index, element, null))
+ }
+ }
+ int index = written.toInteger()
+ Object element = index < held.size() ? held.get(index) : null
+ if (element == null && index < ((List) container).size()) {
+ element = ((List) container).get(index)
+ }
+ fill(held, index, null)
+ held.set(index, heldElement(targetMap, owner, containerKey, name,
written, element, writtenField))
+ owner.put(name, held)
+ } else if (container instanceof Map) {
+ NavigableMap held
+ if (current instanceof NavigableMap) {
+ held = (NavigableMap) current
+ } else {
+ boolean fromFlatKey = !(current instanceof Map)
+ Map source = fromFlatKey ? (Map) container : (Map) current
+ held = new NavigableMap(owner.getRootConfig(),
(owner.getPath() + [name]).asImmutable())
+ for (Map.Entry entry in ((Map) source).entrySet()) {
+ String key = String.valueOf(entry.key)
+ Object element = fromFlatKey ? nestedOrElement(owner,
"${name}[${key}]".toString(), entry.value) : entry.value
+ held.put(key, key == written ? element :
heldElement(targetMap, owner, containerKey, name, key, element, null))
+ }
+ }
+ Object element = held.containsKey(written) ? held.get(written) :
((Map) container).get(written)
+ held.put(written, heldElement(targetMap, owner, containerKey,
name, written, element, writtenField))
+ owner.put(name, held)
+ }
+ }
+
+ /**
+ * The element to hold, which the owner also holds under its indexed name,
{@code rows[0]}. An element that is a
+ * nested map is kept, and so is the nested map the owner holds for the
element just written when it is that same
+ * map, since the merge has written the field into it. An element that is
a value that is not a map, or none, is
+ * replaced by the map the indexed key wrote, as {@code rows[1].name}
replaces {@code 'two'}, rebuilt from the
+ * element under the flat key, so that a field written before the list was
replaced or cleared does not come back;
+ * a map the owner holds under the indexed name of any other element that
is not a map is removed. Otherwise an
+ * element that is a map is held as a nested map
+ * built from it, so that it holds every field the element holds, such as
{@code db.url} beside the {@code db.port}
+ * just written below it, and none of an element the list held before it
was replaced. The field just written then
+ * takes the value the owner's nested map holds for it, which the merge
stored through
+ * {@link #mergeMapEntry(NavigableMap, String, Object)}, so a placeholder
in it is resolved as in every other entry,
+ * and the keys of the map the indexed key was merged into that held the
element, or a map inside it, such as
+ * {@code a.rows[0]} and {@code a.rows[0].db}, then hold the rebuilt
element and its maps.
+ */
+ private static Object heldElement(NavigableMap targetMap, NavigableMap
owner, String containerKey, String name, String index,
+ Object element, String writtenField) {
+ String indexedName = "${name}[${index}]".toString()
+ Object nested = owner.get(indexedName)
+ Object written = writtenField != null ? nested : null
+ if (written instanceof NavigableMap && element != null &&
element.is(written)) {
+ Object stored = valueAt(written, writtenField)
Review Comment:
Fixed in 8b637a920f and 3d3339b73a.
8b637a920f is the one-liner from the comment above: when the key in brackets
has a dot in it, the held map takes the field from the flat element every time
it is written, not only when it holds none yet. After `maxAge` is merged as
`10` and then `20`, `navigate('a', 'mappings')['/api/v1.0/**'].maxAge` and
`get('a.mappings')['/api/v1.0/**'].maxAge` are both `20`. With the two
Boot-style sources, `getProperty('grails.cors.mappings', Map)`, `navigate`, dot
notation, `get('grails.cors.mappings')` and
`getProperty('grails.cors.mappings[/api/v1.0/**].maxAge')` all give `20`,
before and after `refresh()`.
3d3339b73a then moves the source of the field. The flat element holds the
value as given, so a placeholder in a field of a dotted bracketed key read
unresolved through `navigate`, dot notation and `getProperty(key, Map)`, where
`[/api/**]` read the resolved value. `heldElement` now takes the field just
written from the maps the merge wrote it into, for every key, through
`storedValue`: the nested map held for the element for a key without a dot, and
the maps named by the parts of the key, `mappings[/api/v1` and `0/**]`, for one
with a dot. For a key without a dot that is the same map as before.
Tests: in `NavigableMapSpec`, "a field of a map key in brackets written
again holds the last value written" merges the field twice; in
`PropertySourcesConfigSpec`, "a field of a map key in brackets written by a
second source holds the value of that source" writes it from two sources, as
asked, before and after `refresh()`, and "a placeholder in a field of a map key
in brackets is resolved alike for every read of it" writes a placeholder once,
over an earlier value and beside another field. Every test runs for `/api/**`
and `/api/v1.0/**`; the `/api/v1.0/**` rows of the first two fail at 66a010e175
and those of the third at 8b637a920f. `NavigableMapSpec`,
`PropertySourcesConfigSpec`, `PropertySourceConfigSpec`,
`NavigableMapPropertySourceSpec`, `GrailsCorsConfigurationSpec` and `codeStyle`
for `grails-bootstrap` and `grails-core` pass locally; the full suites are on
CI.
--
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]
