sbglasius opened a new pull request, #16281:
URL: https://github.com/apache/grails-core/pull/16281
Fixes #16280
## Problem
Groovy 5 changed runtime method selection for classes that implement `Map`.
For a `String` subscript key, `DefaultGroovyMethods.putAt(Object, String,
Object)` (→ `setProperty`) now wins over `putAt(Map, K, V)` (→ `put`), and
`getAt(Object, String)` (→ `getProperty`) over `getAt(Map, Object)`. Because
`GrailsParameterMap` declares `getIdentifier()` and `getRequest()` with no
setters, those names became read-only *properties* rather than ordinary map
keys:
```
groovy.lang.ReadOnlyPropertyException: Cannot set read-only property:
identifier
at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2837)
at
org.codehaus.groovy.runtime.InvokerHelper.setProperty(InvokerHelper.java:187)
at
org.codehaus.groovy.runtime.DefaultGroovyMethods.putAt(DefaultGroovyMethods.java:12191)
```
| Expression | Groovy 4.0.30 (Grails 7) | Groovy 5.x before this PR |
|---|---|---|
| `params['identifier'] = 'x'` | map put | throws
`ReadOnlyPropertyException` |
| `params['request'] = 'x'` | map put | throws `ReadOnlyPropertyException` |
| `params['identifier']` | map value | `getIdentifier()` → `params.id` |
| `params['request']` | map value | the `HttpServletRequest` |
| `params.identifier` / `params.request` | map value | getter result |
`GroovyPageAttributes` is affected the same way: `attrs['gspTagSyntaxCall']
= x` silently wrote the boolean field instead of the map.
## Fix
`AbstractTypeConvertingMap` — the shared base of `GrailsParameterMap` and
`GroovyPageAttributes` — declares `getAt(String)` / `putAt(String, Object)` and
map-first `getProperty` / `setProperty`, so subscript and property access
always address a map entry. This restores the Grails 7 rule.
Declaring `getAt(Object)` / `putAt(Object, Object)` does **not** work; DGM's
`String` overload remains the closer match. The `String` overloads are what fix
it, verified against both Groovy 4.0.30 and Groovy 5.0.6.
`metaClass` is exempt from the property override so Groovy's own machinery
keeps working. It is still readable as a map entry through
`params['metaClass']`.
Per review feedback, `getIdentifier()` now prefers a parameter literally
named `identifier` and otherwise falls back to the conventional `id` parameter.
`getRequest()` keeps its `HttpServletRequest` return type — widening it to
`Object` would break every caller and would let a submitted form field flow
into framework code expecting a servlet request.
## Also in this PR
`grails-taglib` had no `junit-jupiter-engine` on its test runtime classpath,
so `GroovyPageAttributesTests` — the module's only JUnit 5 test — was never
discovered. Added the engine; all tests in that module now run and pass.
## Documentation
- `upgrading80x.adoc` §28.3, alongside the existing Groovy 5 behavior-change
notes, covering the restored rule and the two deltas from Grails 7
(`getIdentifier()` precedence, `params.metaClass`).
- `controllersAndScopes.adoc` — the everyday rule for `params` and `attrs`
access.
## Testing
- `GrailsParameterMapTests`: 13 new tests (35 total). Collision names via
subscript and dot syntax; collisions arriving from the HTTP request;
`getIdentifier()` precedence, fallback and null cases; `getRequest()`
unaffected; absent keys reading as `null`; `metaClass` not shadowed; `GString`
/ `Integer` / `null` keys and values; `remove` / `containsKey`; cloning;
`toQueryString`; nested maps.
- `TypeConvertingMapTests`: 4 new tests at the level where the fix lives.
- `GroovyPageAttributesTests`: 2 new tests.
Verified green: `grails-core`, `grails-web-common`, `grails-controllers`,
`grails-web-mvc`, `grails-url-mappings`, `grails-gsp`, `grails-web-taglib`,
`grails-taglib`, `grails-shell-cli`.
`aggregateViolations` reports no Checkstyle, CodeNarc, PMD or SpotBugs
issues.
Opened as a draft to confirm CI is green before requesting review.
--
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]
