codeconsole commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4391788138
##########
grails-doc/src/en/guide/upgrading/upgrading90x.adoc:
##########
@@ -1229,3 +1229,11 @@ See <<asyncPromises,Promises>> and <<events,Events>>.
`grails-async-rxjava3` carries a `null` value as the RxJava 1 factory and
every other promise factory do: `createPromise()` and `createPromise(Class)`
give a promise whose `get()` returns `null`, a task that returns `null`
resolves to `null`, and a `null` value keeps its place in the list that
`onComplete`, `onError` and `waitAll` deliver.
`grails-async-rxjava2` does not: there `createPromise()` and `onComplete` over
a list holding a bound promise of `null` fail with a `NullPointerException`,
and a task that returns `null` resolves to the `java.lang.Void` class.
+
+==== 20. 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`.
Review Comment:
Done in 07a31f2ec3. Section 20 and the skill now say that a property source
of lower precedence that sets the deeper key now answers it. With `a.b: x`
above `a.b.c: y`, `getProperty('a.b.c')` answers `y` where Grails 8 answered
`x`, as a Spring `Environment` over the same sources does, `containsProperty`
stays `true`, and `navigate('a', 'b', 'c')` answers `null`.
`PropertySourcesConfigSpec` builds the config over your two
`MapPropertySource`s and a `StandardEnvironment` over the same two, and asserts
all four. It fails on 9.0.x.
##########
grails-doc/src/en/guide/upgrading/upgrading90x.adoc:
##########
@@ -1229,3 +1229,11 @@ See <<asyncPromises,Promises>> and <<events,Events>>.
`grails-async-rxjava3` carries a `null` value as the RxJava 1 factory and
every other promise factory do: `createPromise()` and `createPromise(Class)`
give a promise whose `get()` returns `null`, a task that returns `null`
resolves to `null`, and a `null` value keeps its place in the list that
`onComplete`, `onError` and `waitAll` deliver.
`grails-async-rxjava2` does not: there `createPromise()` and `onComplete` over
a list holding a bound promise of `null` fail with a `NullPointerException`,
and a task that returns `null` resolves to the `java.lang.Void` class.
+
+==== 20. 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`.
+
+===== Required Actions
+
+Code that reads a child of a string, a list or a closure and relied on being
answered with the parent reads the parent instead.
Review Comment:
Done in 07a31f2ec3. Section 20 has the `x.y: false` example with
`getProperty('x.y.enabled', Boolean, true)` (Grails 8: `false`, Grails 9:
`true`), saying that the default takes over with nothing to point at the
change, plus a required action for it. The skill has the same example and, as
sbglasius asked, a checklist entry: no typed read with a default names a key
below a value that is not a map. `PropertySourcesConfigSpec` pins the read, and
it fails on 9.0.x.
##########
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:
I'd keep it absent here, for the reason sbglasius gives: going from `null`
to the element later is compatible, and going back from the element is not.
Indexing would also have to be done in `navigate`, which is more than this PR
is for.
`server.ports[0]` isn't the spelling that always works, though. 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.
`getProperty('server.ports', List)` works for both. So in 07a31f2ec3 section 20
and the skill tell applications to read the element from the list,
`config.getProperty('server.ports', List)[0]`, and mention `server.ports[0]`
only for a list from `application.yml`.
If you'd rather have Grails 9 index a list as Spring Boot does, I'd do it in
this PR, `navigate` included, so section 20 only ever documents one answer.
--
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]
