codeconsole commented on code in PR #16798:
URL: https://github.com/apache/grails-core/pull/16798#discussion_r4432346088
##########
grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy:
##########
@@ -203,6 +207,194 @@ class PropertySourcesConfigSpec extends Specification {
!config.containsKey('gorm.url')
}
+ void 'a placeholder in a list element from application.yml is resolved as
the environment resolves it'() {
+ given: "application.yml with a placeholder in a list of values and in
a list of maps"
+ def yml = """\
Review Comment:
Done in 8109e4516e.
##########
grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java:
##########
@@ -115,15 +115,13 @@ private void
mergeEnumerablePropertySource(EnumerablePropertySource enumerablePr
final String[] propertyNames =
enumerablePropertySource.getPropertyNames();
for (String propertyName : propertyNames) {
Object value =
enumerablePropertySource.getProperty(propertyName);
- if (value instanceof ConfigObject) {
- if (((ConfigObject) value).isEmpty()) continue;
- } else {
- value = processAndEvaluate(value);
+ if (value instanceof ConfigObject && ((ConfigObject)
value).isEmpty()) {
+ continue;
}
- map.put(propertyName, value);
+ map.put(propertyName, processAndEvaluate(value));
}
- configMap.merge(map, true);
+ mergeResolved(map);
Review Comment:
No reason to keep them. c76316bd9e removes `processAndEvaluate()`,
`mergeResolved()` and the flag. The values of an ordinary source are merged as
they are read, and the merge resolves them once per entry, as it does for
application.yml. The escaped-placeholder feature passes unchanged.
The copy does matter, so it is now its own step, `copyOf()`, and it resolves
nothing. Without it, a list of a `MapPropertySource` is held as given, so
`app.origins[1]` from a source of higher precedence changes the source's own
list. After that source is removed, a rebuilt config still reads the written
element.
Checking this turned up the same change in place on 7.0.x for
`application.groovy`, whose lists the config held as the source's own. I
couldn't reproduce it for a list from Grails' YAML loader: in the same probe,
the source's list stayed as written.
- 17ff33c8a3 copies these sources too. Each list and map is copied once, so
the config still holds one list under the flat key and in the nested map.
- 628fb25fdd copies an `ArrayList` or a `LinkedList` as its own kind, as the
config held it before.
- `a list from #description is held as a copy, so an indexed write from a
source of higher precedence leaves the source as it is` covers application.yml,
application.groovy and an ordinary source. The application.groovy row fails at
34220363f1, and the ordinary row fails with the raw merge.
Without the flag, the #16771 merge-up needs a different change. #16771
writes `db.host` into the element `app.rows[0]` through the hook a second time,
with the value already resolved, under a name relative to the element. This
keeps it at one resolution:
- name the entry by the target map's path, so `app.rows[0]` plus `db.host`;
- record each result as resolved for its entry.
```java
List<String> targetPath = targetMap.getPath();
value = resolved(targetPath.isEmpty() ? sourceKey : String.join(".",
targetPath) + '.' + sourceKey, sourceValue);
```
```java
String entry = entryName(name);
String resolved = resolvedTexts.computeIfAbsent(Arrays.asList(entry, text),
key -> resolvePlaceholders(text));
resolvedTexts.putIfAbsent(Arrays.asList(entry, resolved), resolved);
return resolved;
```
I merged #16771 at 66a010e175 into this head and ran the config specs of
grails-core and grails-bootstrap and `GrailsCorsConfigurationSpec`: with the
change, all 524 tests pass. They include an escaped placeholder in a map inside
an element of a YAML list, read by index, by navigation, as the whole list and
in the export. Without the change, the whole list and the export give the value
of `host`. "Merging up" in the description now has it.
--
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]
