jdaugherty commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4416718756
##########
grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java:
##########
@@ -380,20 +380,61 @@ private Object
getValueWithDotNotatedKeySupport(NavigableMap configMap, String k
}
if (end > start) {
String segment = key.substring(start, end);
- if (first) {
- value = configMap.get(segment);
- first = false;
- } else if (value instanceof Map) {
- value = ((Map<?, ?>) value).get(segment);
- } else {
- break;
+ value = childOf(first ? configMap : value, segment);
+ first = false;
+ if (value == null) {
+ return null;
}
}
start = end + 1;
}
return value;
}
+ /**
+ * The child a segment of a key names: the entry of a map, or the element
of a list at a whole-number segment, as
+ * Spring Boot reads {@code server.ports.0}. A segment with an index after
a name, {@code ports[0]}, names the element
+ * of the list under {@code ports} when the map holds no key of that name
itself. Anything else below a value that
+ * is not a map, such as a child of a scalar, does not exist.
+ */
+ private static Object childOf(Object value, String segment) {
+ if (value instanceof Map) {
+ Map<?, ?> map = (Map<?, ?>) value;
+ if (map.containsKey(segment)) {
+ return map.get(segment);
+ }
+ int bracket = segment.indexOf('[');
+ if (bracket > 0 && segment.endsWith("]")) {
+ Object child = map.get(segment.substring(0, bracket));
+ for (String index : segment.substring(bracket + 1,
segment.length() - 1).split("\\]\\[", -1)) {
+ child = childOf(child, index);
+ if (child == null) {
+ return null;
+ }
+ }
+ return child;
+ }
+ return null;
+ }
+ if (value instanceof List<?> list) {
+ int index = listIndex(segment);
+ return index >= 0 && index < list.size() ? list.get(index) : null;
+ }
+ return null;
+ }
+
+ private static int listIndex(String segment) {
Review Comment:
Nit: a leading zero reads as an index here, so `app.ports.01` is the second
element, where the `Environment` answers `null`. Rejecting a segment that
starts with `0` unless it is exactly `0` would keep the two in step. Optional.
##########
grails-doc/src/en/guide/upgrading/upgrading90x.adoc:
##########
@@ -1241,3 +1241,36 @@ Grails no longer registers a character encoding filter
of its own. It used to, a
* `spring.servlet.encoding.enabled: false` turns the encoding filter off.
* The CAS plugin's single sign-out filter, which reads a parameter of every
POST, is registered right after the encoding filter.
* A filter of your own that reads request parameters needs a later order than
`Ordered.HIGHEST_PRECEDENCE`, or form input reaches controllers decoded as
ISO-8859-1. One registered at exactly `Ordered.HIGHEST_PRECEDENCE` through a
`FilterRegistrationBean` runs before the encoding filter.
+
+==== 21. A Configuration Key Below a Value That Is Not a Map Is Absent
+
+In Grails 8, `getProperty` on `grailsApplication.config` answered a dotted key
that runs past a value that is not a map with that value: `server.url.port`
when `server.url` is a string, `server.ports.0` when `server.ports` is a list,
or `grails.gorm.default.constraints.nullable` when
`grails.gorm.default.constraints` is a closure from `application.groovy`.
`containsProperty` of such a key was `true`. In Grails 9 the key is absent:
`getProperty` returns `null` or the default, `containsProperty` returns
`false`, and `navigate` answers `null`.
+
+Below a list, a whole-number segment names an element instead, as Spring Boot
reads it, and so does an index after the name of the list, whether the list
comes from `application.yml` or from `application.groovy`:
Review Comment:
This holds for the element's value, but not for a placeholder in it. The
config stores a list whole and its merge hook resolves placeholders only in
string values, so an element written as
`${EXAMPLE_ORIGIN:https://default.example.com}` is returned as written, where
the `Environment` resolves it:
| | 9.0.x | this PR | `Environment` |
|---|---|---|---|
| `config.getProperty('app.origins.1')` | the whole list |
`${EXAMPLE_ORIGIN:…}` | `https://default.example.com` |
| `config.getProperty('app.origins[1]')` | `${EXAMPLE_ORIGIN:…}` |
`${EXAMPLE_ORIGIN:…}` | `https://default.example.com` |
Pre-existing: the whole-list read, `getProperty('app.origins', List)`, has
the same raw elements on 9.0.x, and both loaders take this path. Not a blocker
here. Please add a sentence here and in the skill that a placeholder in a list
element is not resolved by `config`, and resolve list elements in the hook as a
separate PR against the oldest branch it affects, like the other merge
follow-ups.
--
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]
