matrei commented on code in PR #16798:
URL: https://github.com/apache/grails-core/pull/16798#discussion_r4440130948
##########
grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java:
##########
@@ -53,15 +57,111 @@
@Deprecated
public abstract class NavigableMapConfig implements Config {
protected static final Logger LOG =
LoggerFactory.getLogger(NavigableMapConfig.class);
+
+ /**
+ * A key in brackets that is not an index and holds no dot, {@code [x]} of
{@code routes[x]}.
+ */
+ private static final Pattern NAMED_SUBSCRIPT =
Pattern.compile("\\[([^.\\[\\]]*[^.\\[\\]0-9][^.\\[\\]]*)]");
protected ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
protected ConfigurableConversionService conversionService = new
DefaultConversionService();
+
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 (sourceValue instanceof CharSequence || sourceValue instanceof
List ||
+ (sourceValue instanceof Map && isIndexed(sourceKey))) {
+ value = resolved(path == null || path.isEmpty() ? sourceKey :
path + '.' + sourceKey, sourceValue);
+ }
+ 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
Review Comment:
Nit: "a copy of the same kind" holds for maps, but a list with a placeholder
is always copied into an `ArrayList`, so a `LinkedList` from
`application.groovy` becomes an `ArrayList` once it holds a placeholder, while
one without a placeholder stays a `LinkedList` via `copyOf()`. Either reword
the comment, or copy the list the way `shallowCopyOf()` does, if keeping the
kind matters.
##########
grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy:
##########
@@ -208,6 +209,257 @@ 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 = '''\
+app:
+ origins:
+ - https://a.example.com
+ - '${EXAMPLE_ORIGIN:https://default.example.com}'
+ rows:
+ - url: '${EXAMPLE_ORIGIN:https://default.example.com}'
+ name: first
+ db:
+ host: '${EXAMPLE_ORIGIN:https://default.example.com}'
+ tags:
+ - '${EXAMPLE_ORIGIN:https://default.example.com}'
+'''
+ def source = new YamlPropertySourceLoader().load('application.yml',
new ByteArrayResource(yml.bytes)).first()
+ def environment = new StandardEnvironment()
+ environment.propertySources.addFirst(source)
+ ConfigurationPropertySources.attach(environment)
+ def config = new PropertySourcesConfig(environment.propertySources)
+
+ expect: "the environment resolves each element"
+ environment.getProperty('app.origins[1]') ==
'https://default.example.com'
+ environment.getProperty('app.rows[0].url') ==
'https://default.example.com'
+
+ and: "the config answers the same, by index and in the whole list"
+ config.getProperty('app.origins[1]') == 'https://default.example.com'
+ config.getProperty('app.origins', List) == ['https://a.example.com',
'https://default.example.com']
+ config.navigate('app', 'origins') == ['https://a.example.com',
'https://default.example.com']
+ config.getProperty('app.rows[0].url') == 'https://default.example.com'
+ config.navigate('app', 'rows')[0].url == 'https://default.example.com'
+ config.navigate('app', 'rows')[0].name == 'first'
+
+ and: "so does a map or a list inside an element"
+ environment.getProperty('app.rows[0].db.host') ==
'https://default.example.com'
+ config.getProperty('app.rows[0].db.host') ==
'https://default.example.com'
+ config.navigate('app', 'rows')[0].db.host ==
'https://default.example.com'
+ config.navigate('app', 'rows')[0].tags ==
['https://default.example.com']
+ }
+
+ void 'a placeholder in a list held whole is resolved'() {
+ given: "a list held whole, as application.groovy gives it, of values,
of maps and of lists"
+ def map = new NavigableMap()
+ map.merge([app: [origins: ['https://a.example.com',
'${EXAMPLE_ORIGIN:https://default.example.com}'],
+ rows : [[url:
'${EXAMPLE_ORIGIN:https://default.example.com}', port: 8080]],
+ groups :
[['${EXAMPLE_ORIGIN:https://default.example.com}']]]], true)
+ def config = new PropertySourcesConfig(new
NavigableMapPropertySource('application.groovy', map))
+
+ expect:
+ config.getProperty('app.origins', List) == ['https://a.example.com',
'https://default.example.com']
+ config.navigate('app', 'origins') == ['https://a.example.com',
'https://default.example.com']
+ config.navigate('app', 'rows') == [[url:
'https://default.example.com', port: 8080]]
+ config.navigate('app', 'groups') == [['https://default.example.com']]
+
+ and: "the property source keeps the list as written"
+ map.navigate('app', 'origins') == ['https://a.example.com',
'${EXAMPLE_ORIGIN:https://default.example.com}']
+ }
+
+ void 'a list element follows the value its placeholder names when the
config is refreshed, and #description keeps the placeholder'() {
+ given: "a list with a placeholder, and a property source holding the
value it names"
+ def values = [exampleHost: 'first']
+ def propertySources = new MutablePropertySources()
+ propertySources.addFirst(new MapPropertySource('values', values))
+ propertySources.addLast(source)
+ def config = new PropertySourcesConfig(propertySources)
+
+ expect:
+ config.getProperty('app.origins', List) == ['http://first']
+
+ when: "the value changes and the config is refreshed"
+ values.exampleHost = 'second'
+ config.refresh()
+
+ then: "the list element follows it, in the whole list and by
navigation"
+ config.getProperty('app.origins', List) == ['http://second']
+ config.navigate('app', 'origins') == ['http://second']
+
+ and: "the property source still holds the placeholder"
+ source.getProperty('app.origins') == ['http://${exampleHost}']
+
+ where:
+ description | source
+ 'application.yml' | new
YamlPropertySourceLoader().load('application.yml', new
ByteArrayResource('app:\n origins:\n -
http://${exampleHost}\n'.bytes)).first()
+ 'application.groovy' | new
NavigableMapPropertySource('application.groovy', new NavigableMap().tap {
merge([app: [origins: ['http://${exampleHost}']]], true) })
+ 'an ordinary property source' | new MapPropertySource('ordinary',
['app.origins': ['http://${exampleHost}']])
+ }
+
+ void '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'() {
+ given:
+ def propertySources = new MutablePropertySources()
+ propertySources.addFirst(new MapPropertySource('higher',
['app.origins[1]': 'https://c.example.com']))
+ propertySources.addLast(source)
+
+ when:
+ def config = new PropertySourcesConfig(propertySources)
+
+ then: "the config holds the written element under each name of the
list"
+ config.getProperty('app.origins', List) == ['https://a.example.com',
'https://c.example.com']
+ config['app.origins'] == ['https://a.example.com',
'https://c.example.com']
+ config.navigate('app', 'origins') == ['https://a.example.com',
'https://c.example.com']
+
+ and: "a list of the same kind, while the source keeps its own"
+ config.navigate('app', 'origins').getClass() ==
source.getProperty('app.origins').getClass()
+ source.getProperty('app.origins') == ['https://a.example.com',
'https://b.example.com']
Review Comment:
About not being able to reproduce the in-place change for `application.yml`:
on 7.0.x it's the source's nested list that changes, not the flat key
`getProperty` reads. The YAML loader's map holds `app.origins` and `app →
origins` as two separate lists, and the indexed write lands in the nested one.
On the next merge the indexed keys write `b` back, which is why a rebuilt
config never shows it. 17ff33c8a3 fixes it either way, but this row passes on
7.0.x as written. Asserting the nested list too makes it fail there and pass
here (I checked both):
```groovy
source.getSource().navigate('app', 'origins') == ['https://a.example.com',
'https://b.example.com']
```
It only applies to the two `NavigableMapPropertySource` rows, so it would
need its own feature or a guard.
--
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]
