Copilot commented on code in PR #16798:
URL: https://github.com/apache/grails-core/pull/16798#discussion_r4424980134
##########
grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java:
##########
@@ -55,13 +58,99 @@ public abstract class NavigableMapConfig implements Config {
protected static final Logger LOG =
LoggerFactory.getLogger(NavigableMapConfig.class);
protected ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
protected ConfigurableConversionService conversionService = new
DefaultConversionService();
+
+ /**
+ * Whether the merge resolves the placeholders in the values it writes. It
does not while it merges values that are
+ * resolved already, so a placeholder an escape kept as text is not
resolved a second time.
+ */
+ private boolean resolvingPlaceholders = true;
+
protected NavigableMap configMap = new NavigableMap() {
+
+ /**
+ * The texts the merge in progress has resolved, by the full name of
the entry and the text. The merge writes one
+ * entry under several keys: a dotted key under the whole key and in
its nested map, and a list element in its
+ * list in the nested map, in its list under the flat key and under
its indexed key. Every write takes one value,
+ * even for {@code ${random.uuid}}.
+ */
+ private Map<List<String>, String> resolvedTexts;
+
+ @Override
+ public void merge(Map sourceMap, boolean parseFlatKeys) {
+ resolvedTexts = new HashMap<>();
+ try {
+ super.merge(sourceMap, parseFlatKeys);
+ } finally {
+ resolvedTexts = null;
+ }
+ }
+
+ /**
+ * Resolves the placeholders in a value before it is written: in a
string, in the strings of a list, through
+ * the maps and lists it holds, and in a map written under an indexed
key, {@code rows[0]}, which is written into
+ * its list as it is. A map written under a plain key is merged entry
by entry, through this method.
+ */
@Override
- protected Object mergeMapEntry(NavigableMap targetMap, String
sourceKey, Object newValue) {
- if (newValue instanceof CharSequence) {
- newValue = resolvePlaceholders(newValue.toString());
+ protected void mergeMapEntry(NavigableMap rootMap, String path,
NavigableMap targetMap, String sourceKey,
+ Object sourceValue, boolean parseFlatKeys, boolean
isNestedSet) {
+ Object value = sourceValue;
+ if (resolvingPlaceholders && (sourceValue instanceof CharSequence
|| sourceValue instanceof List || sourceValue instanceof Map &&
isIndexed(sourceKey))) {
+ value = resolved(path == null || path.isEmpty() ? sourceKey :
path + '.' + sourceKey, sourceValue);
}
- return super.mergeMapEntry(targetMap, sourceKey, newValue);
+ super.mergeMapEntry(rootMap, path, targetMap, sourceKey, value,
parseFlatKeys, isNestedSet);
+ }
+
+ private boolean isIndexed(String sourceKey) {
+ int subscriptStart = sourceKey.indexOf('[');
+ return subscriptStart > -1 && sourceKey.indexOf(']') >
subscriptStart;
+ }
+
+ /**
+ * The value with the placeholders in its strings resolved, through
the lists and maps it holds. A list or map
+ * holding no placeholder is the one given, and any other is a copy of
the same kind, so the property source
+ * keeps what it holds.
+ */
+ @SuppressWarnings("unchecked")
+ private Object resolved(String name, Object value) {
+ if (value instanceof CharSequence) {
+ return resolvedText(name, value.toString());
+ }
+ if (value instanceof List) {
+ List<?> list = (List<?>) value;
+ List<Object> resolved = null;
+ for (int i = 0; i < list.size(); i++) {
+ Object element = resolved(name + '[' + i + ']',
list.get(i));
+ if (resolved == null && !Objects.equals(element,
list.get(i))) {
+ resolved = new ArrayList<>(list);
+ }
+ if (resolved != null) {
+ resolved.set(i, element);
+ }
+ }
+ return resolved != null ? resolved : list;
+ }
+ if (value instanceof Map) {
+ Map<?, ?> map = (Map<?, ?>) value;
+ Map<Object, Object> resolved = null;
+ for (Map.Entry<?, ?> entry : map.entrySet()) {
+ Object element = resolved(name + '.' + entry.getKey(),
entry.getValue());
Review Comment:
For YAML such as `app.rows: [{routes: {'[x]': '${random.uuid}'}}]`, the
loaded element contains both `routes.x` and its `routes[x]` alias, which this
traversal resolves under different cache keys. The same leaf is therefore
resolved twice, so `getProperty('app.rows[0].routes[x]')` and
`getProperty('app.rows', List)[0].routes.x` can return different UUIDs. Make
these aliases share a resolution without conflating genuinely distinct dotted
keys, and add this YAML shape to the counting-source regression tests.
--
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]
