jdaugherty commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4382924033
##########
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:
The read that changes with no `null` or exception to point at it is a typed
one with a default, where the application set the parent to a boolean as a
shorthand for the block:
```groovy
// x.y: false
config.getProperty('x.y.enabled', Boolean, true) // 9.0.x: false this
PR: true
```
Please add that as an example here and in the skill. The default takes over
silently, so nothing else points at the change.
##########
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:
**[DISCUSS] Read a numeric segment after a list as an index, as Spring Boot
does**
Spring Boot treats `server.ports.0` and `server.ports[0]` as the same
property once `ConfigurationPropertySources` is attached to the environment,
which `SpringApplication` always does. With `server.ports: [8080, 8443]` loaded
through Grails' `YamlPropertySourceLoader` into an attached
`StandardEnvironment`, and a `PropertySourcesConfig` built over the same
sources:
| | 9.0.x | this PR |
|---|---|---|
| `environment.getProperty('server.ports.0')` | `8080` | `8080` |
| `config.getProperty('server.ports.0')` | the whole list, `'8080,8443'` |
`null` |
| `config.getProperty('server.ports[0]')` | `8080` | `8080` |
Neither branch gives the environment's answer. Should Grails 9 read a
numeric segment after a list as an index, so that
`config.getProperty('server.ports.0')` is `8080`, rather than making the key
absent?
- For: the config and the environment of one application agree on the same
key, and an application that reads a list element this way keeps working, now
with the element instead of the list.
- Against: `server.ports[0]` already works, and `navigate('server', 'ports',
'0')` would have to index too, or the two would disagree.
The walk would index only when it reaches a list, so a map with a `'0'` key
reads as it does now. The answer decides what section 20 and these assertions
say about a list, so settling it before this merges would avoid documenting one
answer and then changing it.
##########
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:
This holds while nothing else sets the deeper key. When a lower-precedence
property source does, `getProperty` now answers with that key, where 9.0.x
answered with the value above it:
```groovy
def sources = new MutablePropertySources()
sources.addLast(new MapPropertySource('higher', ['a.b': 'x']))
sources.addLast(new MapPropertySource('lower', ['a.b.c': 'y']))
def config = new PropertySourcesConfig(sources)
```
| | 9.0.x | this PR |
|---|---|---|
| `config.getProperty('a.b.c')` | `x` | `y` |
| `config.containsProperty('a.b.c')` | `true` | `true` |
| `config.navigate('a', 'b', 'c')` | `x` | `null` |
`y` is the right answer, and it is what a Spring `Environment` over the same
two sources answers. But it is a second change an application can observe, and
this sentence says the key is absent. Please describe it here and in the skill,
and pin it in `PropertySourcesConfigSpec` with two property sources like the
ones above.
##########
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:
@matrei @sbglasius @jamesfredley I'd love your thoughts here
--
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]
