jamesfredley commented on code in PR #16798:
URL: https://github.com/apache/grails-core/pull/16798#discussion_r4420900297
##########
grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java:
##########
@@ -56,13 +57,93 @@ public abstract class NavigableMapConfig implements Config {
protected ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
protected ConfigurableConversionService conversionService = new
DefaultConversionService();
protected NavigableMap configMap = new NavigableMap() {
+
+ /**
+ * A value resolved for a dotted key, which the merge writes twice in
a row: under the whole key and then,
+ * in the nested map, under its last part. The second write takes the
value the first one resolved.
+ */
+ private Object pendingSource;
+ private Object pendingResolved;
+
+ /**
+ * Resolves the placeholders in a list, which is stored as it is, and
in a value written under an indexed key,
+ * {@code rows[0]} or {@code rows[0].url}, which is written into its
list or map without the hook below.
+ */
+ @Override
+ protected void mergeMapEntry(NavigableMap rootMap, String path,
NavigableMap targetMap, String sourceKey,
+ Object sourceValue, boolean parseFlatKeys, boolean
isNestedSet) {
+ Object value = pendingSource != null && pendingSource ==
sourceValue ? pendingResolved : resolvedBeforeMerge(sourceKey, sourceValue);
Review Comment:
A YAML list element `${random.uuid}` is resolved once for the whole list,
again for the indexed key, and again for the nested map. Those sources merge
with flat keys off, so the pending cache does not cover them.
`config['app.hosts[0]']` and `getProperty('app.hosts[0]')` can then differ.
Resolve each element once and reuse that value.
##########
grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java:
##########
@@ -56,13 +57,93 @@ public abstract class NavigableMapConfig implements Config {
protected ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
protected ConfigurableConversionService conversionService = new
DefaultConversionService();
protected NavigableMap configMap = new NavigableMap() {
+
+ /**
+ * A value resolved for a dotted key, which the merge writes twice in
a row: under the whole key and then,
+ * in the nested map, under its last part. The second write takes the
value the first one resolved.
+ */
+ private Object pendingSource;
+ private Object pendingResolved;
+
+ /**
+ * Resolves the placeholders in a list, which is stored as it is, and
in a value written under an indexed key,
+ * {@code rows[0]} or {@code rows[0].url}, which is written into its
list or map without the hook below.
+ */
+ @Override
+ protected void mergeMapEntry(NavigableMap rootMap, String path,
NavigableMap targetMap, String sourceKey,
+ Object sourceValue, boolean parseFlatKeys, boolean
isNestedSet) {
+ Object value = pendingSource != null && pendingSource ==
sourceValue ? pendingResolved : resolvedBeforeMerge(sourceKey, sourceValue);
+ pendingSource = null;
+ pendingResolved = null;
+ if (value != sourceValue && parseFlatKeys &&
sourceKey.split("\\.").length > 1) {
+ pendingSource = sourceValue;
+ pendingResolved = value;
+ }
+ super.mergeMapEntry(rootMap, path, targetMap, sourceKey, value,
parseFlatKeys, isNestedSet);
+ }
+
@Override
protected Object mergeMapEntry(NavigableMap targetMap, String
sourceKey, Object newValue) {
if (newValue instanceof CharSequence) {
newValue = resolvePlaceholders(newValue.toString());
}
return super.mergeMapEntry(targetMap, sourceKey, newValue);
}
+
+ private Object resolvedBeforeMerge(String sourceKey, Object value) {
+ if (value instanceof List) {
Review Comment:
With `host=secret`, a `MapPropertySource` list containing `\${host}` used to
stay the literal `${host}`. `processAndEvaluate()` already resolved it and
removed the escape. Resolving that result again stores `[secret]`. Keep list
and index placeholders, and do not resolve a value this path already handled.
--
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]
