jamesfredley commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4367891308
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -199,32 +246,120 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
mergeMaps(rootMap, newPath , subMap, (Map) sourceValue,
parseFlatKeys)
newValue = subMap
} else {
- newValue = sourceValue
+ newValue = mergedValue(sourceValue)
}
if (isNestedSet && newValue == null) {
- if (path) {
-
- def subMap = rootMap.get(path)
- if (subMap instanceof Map) {
- subMap.remove(sourceKey)
- }
- def keysToRemove = rootMap.keySet().findAll() { String key
->
- key.startsWith("${path}.")
- }
- for (key in keysToRemove) {
- rootMap.remove(key)
- }
+ // the flat key of the removed property, those below it and
its indexed aliases go with it;
+ // the flat keys of its siblings stay
+ String removedKey = path ? "${path}.${sourceKey}".toString() :
sourceKey
+ List<String> keysToRemove = rootMap.keySet().findAll { String
key ->
+ key == removedKey || key.startsWith(removedKey + '.') ||
key.startsWith(removedKey + '[')
+ } as List<String>
+ for (String key in keysToRemove) {
+ rootMap.remove(key)
}
targetMap.remove(sourceKey)
} else {
+ String flatKey = path ? "${path}.${sourceKey}".toString() :
sourceKey
if (path) {
- rootMap.put("${path}.${sourceKey}".toString(), newValue)
+ mergeMapEntry(rootMap, flatKey, newValue)
}
mergeMapEntry(targetMap, sourceKey, newValue)
+ if (newValue instanceof List) {
+ writeElementKeys(rootMap, flatKey, (List) newValue)
+ }
}
}
}
+ /**
+ * Writes the flat keys of the elements of a list merged as a value,
{@code a.rows[0]} and, for an element
+ * that is a map, {@code a.rows[0].name}, as the indexed keys of a
configuration file write them, so that a
+ * list is read alike however it was written. The element keys of the list
the value replaces go first: they
+ * name elements the new list does not hold.
+ */
+ private void writeElementKeys(NavigableMap rootMap, String flatKey, List
list) {
+ List<String> staleKeys = rootMap.keySet().findAll { String key ->
key.startsWith(flatKey + '[') } as List<String>
+ for (String key in staleKeys) {
+ rootMap.remove(key)
+ }
+ writeElements(rootMap, flatKey, list)
+ }
+
+ private void writeElements(NavigableMap rootMap, String flatKey, Object
value) {
+ if (value instanceof List) {
+ int index = 0
+ for (Object element in (List) value) {
+ String elementKey = "${flatKey}[${index++}]".toString()
+ mergeMapEntry(rootMap, elementKey, element)
+ writeElements(rootMap, elementKey, element)
+ }
+ } else if (value instanceof Map && !(value instanceof NavigableMap)) {
+ for (Entry entry in ((Map) value).entrySet()) {
+ String entryKey = "${flatKey}.${entry.key}".toString()
+ mergeMapEntry(rootMap, entryKey, entry.value)
+ writeElements(rootMap, entryKey, entry.value)
+ }
+ }
+ }
+
+ /** The index of the first subscript of a key such as {@code
rows[0].name}, or -1 when the key has none. */
+ private static int subscriptStart(String key) {
+ int start = key.indexOf('[')
+ start > -1 && key.indexOf(']', start) > start ? start : -1
+ }
+
+ /**
+ * Merges a key with a subscript, such as {@code rows[0].name}, {@code
groups[0][1]} or {@code mappings[/api/**]}.
+ * The container named before the first subscript, a list for a numeric
index and a map otherwise, is held by
+ * the nested map that owns it and, under its flat key, by the root, as
one object in both places. The root
+ * also holds a flat key for each element the subscripts step through and
for the value itself, so that
+ * {@code a.rows}, {@code a.rows[0]} and {@code a.rows[0].name} are each
read without a walk. The dots of the
+ * container's name are a path only when the merge parses flat keys, as
for any other key.
+ */
+ private void mergeIndexedEntry(NavigableMap rootMap, String path,
NavigableMap targetMap, String containerName,
+ String subscripts, Object sourceValue,
boolean parseFlatKeys) {
+ NavigableMap containerParent = targetMap
+ String containerKey = containerName
+ if (parseFlatKeys && containerName.contains('.')) {
+ List<String> nameParts = containerName.split(/\./) as List<String>
+ containerParent = targetMap.navigateSubMap(nameParts[0..-2], true)
+ containerKey = nameParts[-1]
+ }
+ String containerPath = path ? "${path}.${containerName}".toString() :
containerName
+ int subscriptEnd = subscripts.indexOf(']')
+ Object value = mergedValue(sourceValue)
+ Object container =
setSubscriptValue(containerParent.get(containerKey), subscripts.substring(1,
subscriptEnd),
Review Comment:
`application.groovy` can supply `['one'].asImmutable()`. Replaying the
generated element alias writes into that list and throws
`UnsupportedOperationException`, so startup fails. Those aliases did not exist
before this change.
Copy an immutable list before replaying its aliases. Please start with that
list and assert the application comes up.
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -199,32 +246,120 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
mergeMaps(rootMap, newPath , subMap, (Map) sourceValue,
parseFlatKeys)
newValue = subMap
} else {
- newValue = sourceValue
+ newValue = mergedValue(sourceValue)
}
if (isNestedSet && newValue == null) {
- if (path) {
-
- def subMap = rootMap.get(path)
- if (subMap instanceof Map) {
- subMap.remove(sourceKey)
- }
- def keysToRemove = rootMap.keySet().findAll() { String key
->
- key.startsWith("${path}.")
- }
- for (key in keysToRemove) {
- rootMap.remove(key)
- }
+ // the flat key of the removed property, those below it and
its indexed aliases go with it;
+ // the flat keys of its siblings stay
+ String removedKey = path ? "${path}.${sourceKey}".toString() :
sourceKey
+ List<String> keysToRemove = rootMap.keySet().findAll { String
key ->
+ key == removedKey || key.startsWith(removedKey + '.') ||
key.startsWith(removedKey + '[')
+ } as List<String>
+ for (String key in keysToRemove) {
+ rootMap.remove(key)
}
targetMap.remove(sourceKey)
} else {
+ String flatKey = path ? "${path}.${sourceKey}".toString() :
sourceKey
if (path) {
- rootMap.put("${path}.${sourceKey}".toString(), newValue)
+ mergeMapEntry(rootMap, flatKey, newValue)
}
mergeMapEntry(targetMap, sourceKey, newValue)
+ if (newValue instanceof List) {
+ writeElementKeys(rootMap, flatKey, (List) newValue)
+ }
}
}
}
+ /**
+ * Writes the flat keys of the elements of a list merged as a value,
{@code a.rows[0]} and, for an element
+ * that is a map, {@code a.rows[0].name}, as the indexed keys of a
configuration file write them, so that a
+ * list is read alike however it was written. The element keys of the list
the value replaces go first: they
+ * name elements the new list does not hold.
+ */
+ private void writeElementKeys(NavigableMap rootMap, String flatKey, List
list) {
+ List<String> staleKeys = rootMap.keySet().findAll { String key ->
key.startsWith(flatKey + '[') } as List<String>
+ for (String key in staleKeys) {
+ rootMap.remove(key)
+ }
+ writeElements(rootMap, flatKey, list)
+ }
+
+ private void writeElements(NavigableMap rootMap, String flatKey, Object
value) {
+ if (value instanceof List) {
+ int index = 0
+ for (Object element in (List) value) {
+ String elementKey = "${flatKey}[${index++}]".toString()
+ mergeMapEntry(rootMap, elementKey, element)
+ writeElements(rootMap, elementKey, element)
+ }
+ } else if (value instanceof Map && !(value instanceof NavigableMap)) {
+ for (Entry entry in ((Map) value).entrySet()) {
+ String entryKey = "${flatKey}.${entry.key}".toString()
+ mergeMapEntry(rootMap, entryKey, entry.value)
+ writeElements(rootMap, entryKey, entry.value)
+ }
+ }
+ }
+
+ /** The index of the first subscript of a key such as {@code
rows[0].name}, or -1 when the key has none. */
+ private static int subscriptStart(String key) {
+ int start = key.indexOf('[')
+ start > -1 && key.indexOf(']', start) > start ? start : -1
+ }
+
+ /**
+ * Merges a key with a subscript, such as {@code rows[0].name}, {@code
groups[0][1]} or {@code mappings[/api/**]}.
+ * The container named before the first subscript, a list for a numeric
index and a map otherwise, is held by
+ * the nested map that owns it and, under its flat key, by the root, as
one object in both places. The root
+ * also holds a flat key for each element the subscripts step through and
for the value itself, so that
+ * {@code a.rows}, {@code a.rows[0]} and {@code a.rows[0].name} are each
read without a walk. The dots of the
+ * container's name are a path only when the merge parses flat keys, as
for any other key.
+ */
+ private void mergeIndexedEntry(NavigableMap rootMap, String path,
NavigableMap targetMap, String containerName,
+ String subscripts, Object sourceValue,
boolean parseFlatKeys) {
+ NavigableMap containerParent = targetMap
+ String containerKey = containerName
+ if (parseFlatKeys && containerName.contains('.')) {
+ List<String> nameParts = containerName.split(/\./) as List<String>
+ containerParent = targetMap.navigateSubMap(nameParts[0..-2], true)
+ containerKey = nameParts[-1]
+ }
+ String containerPath = path ? "${path}.${containerName}".toString() :
containerName
+ int subscriptEnd = subscripts.indexOf(']')
+ Object value = mergedValue(sourceValue)
+ Object container =
setSubscriptValue(containerParent.get(containerKey), subscripts.substring(1,
subscriptEnd),
+ subscripts.substring(subscriptEnd + 1), value)
+ mergeMapEntry(containerParent, containerKey, container)
+ if (!containerParent.is(rootMap)) {
+ mergeMapEntry(rootMap, containerPath, container)
+ }
+ Object element = container
+ String elementPath = containerPath
+ String remaining = subscripts
+ while (remaining.startsWith('[') && remaining.indexOf(']') > 0) {
+ int end = remaining.indexOf(']')
+ String index = remaining.substring(1, end)
+ element = index.isNumber() ? ((List)
element).get(index.toInteger()) : ((Map) element).get(index)
+ elementPath += remaining.substring(0, end + 1)
Review Comment:
`merge([a: [rows: [[name: 'old']]]])` then `merge(['a.rows[0]': [name:
'new']])` updates the element but leaves `a.rows[0].name` as `'old'`.
`getProperty('a.rows[0].name')` returns the old value.
Drop and rebuild the child aliases when an indexed element is replaced.
--
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]
