jamesfredley commented on issue #16122:
URL: https://github.com/apache/grails-core/issues/16122#issuecomment-5245025245
### Shim feasibility: this one looks small and self-contained
Following the compatibility-shim precedents in #16011 (restore a deprecated
contract behind an opt-in) and #16101 (narrow, default-off behavioural flag), I
looked at what restoring these two access forms would actually require. It
appears to be a very small change, because the class has already kept most of
the Map-like surface.
The runtime class is `grails/util/Metadata.groovy`, which lives in the
**`grails-gradle/model`** build rather than in `grails-core` - worth noting up
front, since that is a separately published artifact that `grails-core`
consumes.
Its current shape:
```groovy
@CompileStatic
class Metadata {
<T> T getProperty(String key, Class<T> targetType, T defaultValue) {
return this.propertyResolver.getProperty(key, targetType,
defaultValue)
}
boolean containsKey(Object key) {
return this.propertyResolver.containsProperty((String) key)
}
Object getOrDefault(Object key, Object defaultValue) {
return getProperty(key.toString(), Object, defaultValue)
}
Object navigate(String... path) {
return this.propertyResolver.getProperty(path.join('.').toString(),
Object, null)
}
}
```
So `containsKey(Object)` and `getOrDefault(Object, Object)` - both Map-style
accessors - are still present and still delegate to the resolver. The class is
not a `Map` (it does not implement `Map`), and what is missing is specifically:
- `get(Object)` - which is what `Metadata.current.get('some.key')` binds to
- a single-argument property read - which is what
`Metadata.current.'some.key'` binds to
Both have an exact one-line expression already in use elsewhere in the
class, namely `getProperty(key.toString(), Object, defaultValue)`.
Restoring them would therefore be roughly:
```groovy
@Deprecated
Object get(Object key) {
return getProperty(key.toString(), Object, null)
}
@Deprecated
Object propertyMissing(String name) {
return getProperty(name, Object, null)
}
```
`propertyMissing` still works despite `@CompileStatic` on the class, because
the failing call sites are dynamic Groovy in the *calling* plugin, so dispatch
goes through the receiver's MetaClass at runtime.
That would make both previously-working forms return the value or `null`
again, rather than throwing, and it keeps the supported typed accessor
`getProperty(key, Class, default)` as the documented path.
### Why this may be worth shimming rather than only documenting
The two removed forms fail at **runtime**, not at build time, so an
unchanged Grails 7 binary plugin resolves, loads, and boots before throwing on
the first request that touches the code path - as `grails-web-console:7.1.0`
does with a 500 on `GET /console`. Unlike the other Groovy-level breaks I have
found in this sweep (#16123, #16126, #16128), this one does not require the
plugin to be recompiled to be fixed; the framework can simply answer the call
again.
If restoring them unconditionally is not desired, the #16101 pattern would
fit: keep them removed by default and allow opting in, so a legacy application
can run today while plugin authors migrate to `getProperty(key, Class,
default)`.
I have not raised a PR for this - flagging the assessment in case it is
useful.
--
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]
