codeconsole commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4391788874
##########
grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy:
##########
@@ -132,9 +132,33 @@ class PropertySourcesConfigSpec extends Specification {
config.getProperty('.plain.nested.value.') == 'found'
config.getProperty('...') == null
- and: "a key that runs past a scalar answers with that scalar, as it
always has"
- // Grails 8 behaviour, pinned on purpose: apache/grails-core#16732
makes such a key absent on 9.0.x
- config.getProperty('plain.nested.value.deeper') == 'found'
- config.containsProperty('plain.nested.value.deeper')
+ and: "a key that runs past a scalar is absent, where Grails 8 answered
with the scalar"
+ config.getProperty('plain.nested.value.deeper') == null
+ !config.containsProperty('plain.nested.value.deeper')
+ }
+
+ void 'a dotted key that runs past a value that is not a map is absent'() {
+ given: "a scalar, a list and a closure, the value shapes
application.yml and application.groovy produce"
+ def constraints = { nullable true }
+ def config = new PropertySourcesConfig([
+ server: [url: 'http://localhost', ports: [8080, 8443]],
+ grails: [gorm: ['default': [constraints: constraints]]]
+ ])
+
+ expect: "each value resolves itself"
+ config.getProperty('server.url') == 'http://localhost'
+ config.getProperty('server.ports', List) == [8080, 8443]
+ config.getProperty('grails.gorm.default.constraints',
Closure).is(constraints)
+
+ and: "a key that names a child of any of them is absent rather than
the value"
+ config.getProperty('server.url.port') == null
+ config.getProperty('server.url.port', String, 'none') == 'none'
+ config.getProperty('server.ports.0') == null
Review Comment:
On reflection I think Grails 9 should do what Spring Boot does, in this PR.
- **It is the reason for the PR.** The case for making the key absent is
that the config then agrees with the `Environment` beside it. For a list,
absent is the one answer where they disagree: per your table, the environment
answers `server.ports.0` with `8080`, and this PR answers `null`.
- **The element is what such a read meant.** Code that reads
`server.ports.0` almost certainly wants the first port. Grails 8 handed it the
whole list, which was never designed, as you found for the walk in #11042.
`null` breaks that code as much as the element would, and only the element does
what it asked for.
- **There is only one change if it goes in here.** sbglasius's point stands
for a split: going from `null` to the element later is compatible, and going
back is not. If this PR goes from the list straight to the element, Grails 9
never answers `null` for an index, and section 20 documents one answer.
- **`server.ports[0]` isn't the fallback it looks like.** It names the
element only where the source wrote indexed keys, as `application.yml` and
`.properties` files do. For a list from `application.groovy` or a merged map,
`config.getProperty('server.ports[0]')` is `null` on 9.0.x and here.
What I'd do:
- `getProperty` and `navigate` step into a list when the next segment is a
whole number within the list, `server.ports.0`, and read `ports[0]` the same
way, so both spellings answer for every list.
- A segment that is not an index, or is out of range, leaves the key absent,
as for any other value that is not a map.
- The walk indexes only when it reaches a list, so a map with a `'0'` key
reads as it does now.
- `containsProperty` follows `getProperty`.
- A list of maps reads through: `app.rows.0.name`.
- Section 20 and the skill would say that `server.ports.0` is the first
element, as in Spring Boot, where Grails 8 answered with the list.
One difference from Boot: Boot resolves both spellings against the indexed
keys a YAML or `.properties` source holds, while Grails would also index a list
held whole, as `application.groovy` gives it. That seems the right side to err
on, since the config already answers with the whole list there.
If you agree, I'll add it here before this merges.
--
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]
