jdaugherty commented on code in PR #16281:
URL: https://github.com/apache/grails-core/pull/16281#discussion_r3904805389
##########
grails-core/src/main/groovy/grails/util/AbstractTypeConvertingMap.java:
##########
@@ -349,6 +351,72 @@ public Set entrySet() {
return wrappedMap.entrySet();
}
+ /**
+ * Subscript access ({@code map['name']}) always addresses a map entry,
never a JavaBean
+ * property of this class.
+ *
+ * <p>Groovy 5 changed runtime method selection for classes implementing
{@link Map}: for a
+ * {@code String} key it prefers {@code DefaultGroovyMethods.getAt(Object,
String)}, which
+ * reads a bean property, over {@code DefaultGroovyMethods.getAt(Map,
Object)}, which reads a
+ * map entry. Declaring the {@code String} overload here keeps map
semantics, so an entry that
+ * happens to share its name with a getter on this class remains reachable.
+ *
+ * @param key the map key
+ * @return the value stored under the key, or {@code null}
+ * @since 8.0.0
+ */
+ public Object getAt(String key) {
+ return get(key);
+ }
+
+ /**
+ * Subscript assignment ({@code map['name'] = value}) always writes a map
entry, never a
+ * JavaBean property of this class. See {@link #getAt(String)} for why the
overload is needed.
+ *
+ * @param key the map key
+ * @param value the value to store
+ * @return the previous value stored under the key, or {@code null}
+ * @since 8.0.0
+ */
+ public Object putAt(String key, Object value) {
+ return put(key, value);
+ }
+
+ /**
+ * Property access ({@code map.name}) reads a map entry rather than a
JavaBean property, so
+ * that entries whose names collide with a getter on this class remain
reachable. Getters such
+ * as {@code getRequest()} are still callable as methods. The {@code
metaClass} property is
+ * excluded because Groovy relies on it.
+ *
+ * @param name the map key
+ * @return the value stored under the key, or {@code null}
+ * @since 8.0.0
+ */
+ @Override
+ public Object getProperty(String name) {
+ if (METACLASS_PROPERTY.equals(name)) {
+ return super.getProperty(name);
+ }
+ return get(name);
+ }
+
+ /**
+ * Property assignment ({@code map.name = value}) writes a map entry
rather than a JavaBean
+ * property of this class. See {@link #getProperty(String)}.
+ *
+ * @param name the map key
+ * @param value the value to store
+ * @since 8.0.0
+ */
+ @Override
+ public void setProperty(String name, Object value) {
Review Comment:
`setProperty` routes every non-`metaClass` name to `put`, which also changes
assignment for names backed by a *real settable* bean property — not only the
read-only getters this PR is about. That is a dynamic-path change, independent
of the static-compilation caveat documented in c9e3c0f.
The only such property in this hierarchy is
`GroovyPageAttributes.gspTagSyntaxCall`. Verified with a replica carrying a
settable `flag` property:
```
Groovy 4.0.33 (no override)
dynamic m.flag = false -> field=false, mapEntry=null # setter
invoked
dynamic m['flag'] = false -> field=false, mapEntry=null # setter
invoked
Groovy 5.1.0 (this PR)
dynamic m.flag = false -> field=true, mapEntry=false # setter
skipped
dynamic m['flag'] = false -> field=true, mapEntry=false # setter
skipped
```
So on Grails 7 both forms called `setGspTagSyntaxCall`; here both write a
map entry and leave the flag at its default. Reads were already map-first on
Groovy 4, so this is specifically a write-side change and it is not in §28.3.
I checked how the framework sets the flag: only the two-arg constructor
(`new GroovyPageAttributes(attrs, false)` in `TagOutput`) and the explicit
`setGspTagSyntaxCall`/`isGspTagSyntaxCall` methods. So nothing inside
grails-core regresses. The exposure is third-party taglib or plugin code using
the property form — it would silently stop suppressing auto-printing, and the
stray `gspTagSyntaxCall` entry then travels with the attribute map into
whatever renders remaining attributes.
Either exempting names that have a real setter, or documenting this in §28.3
and pinning it with a test, would close it. A test that passes today and
captures the difference is suggested on `GroovyPageAttributesTests`.
Minor, same method: the `metaClass` branch could be `return getMetaClass();`
/ `setMetaClass(...)` instead of round-tripping through
`super.getProperty(name)`, which re-enters the metaclass to resolve a property
this class now intercepts. Cheaper per read and avoids the self-referential
path.
##########
grails-core/src/main/groovy/grails/util/AbstractTypeConvertingMap.java:
##########
@@ -349,6 +351,72 @@ public Set entrySet() {
return wrappedMap.entrySet();
}
+ /**
+ * Subscript access ({@code map['name']}) always addresses a map entry,
never a JavaBean
+ * property of this class.
+ *
+ * <p>Groovy 5 changed runtime method selection for classes implementing
{@link Map}: for a
+ * {@code String} key it prefers {@code DefaultGroovyMethods.getAt(Object,
String)}, which
+ * reads a bean property, over {@code DefaultGroovyMethods.getAt(Map,
Object)}, which reads a
+ * map entry. Declaring the {@code String} overload here keeps map
semantics, so an entry that
+ * happens to share its name with a getter on this class remains reachable.
+ *
+ * @param key the map key
+ * @return the value stored under the key, or {@code null}
+ * @since 8.0.0
+ */
+ public Object getAt(String key) {
+ return get(key);
+ }
+
+ /**
+ * Subscript assignment ({@code map['name'] = value}) always writes a map
entry, never a
+ * JavaBean property of this class. See {@link #getAt(String)} for why the
overload is needed.
+ *
+ * @param key the map key
+ * @param value the value to store
+ * @return the previous value stored under the key, or {@code null}
+ * @since 8.0.0
+ */
+ public Object putAt(String key, Object value) {
+ return put(key, value);
+ }
+
+ /**
+ * Property access ({@code map.name}) reads a map entry rather than a
JavaBean property, so
+ * that entries whose names collide with a getter on this class remain
reachable. Getters such
+ * as {@code getRequest()} are still callable as methods. The {@code
metaClass} property is
+ * excluded because Groovy relies on it.
+ *
+ * @param name the map key
+ * @return the value stored under the key, or {@code null}
+ * @since 8.0.0
+ */
+ @Override
+ public Object getProperty(String name) {
Review Comment:
c9e3c0f documents the static-compilation caveat well for `params`, and I
verified `testStaticCompilationBindsPropertySyntaxToTheAccessor` passes. Two
notes on scope, since this override is the shared base for both maps:
**`attrs` is not covered, and it fails more quietly than `params`.** Both
the new NOTE and the new test scope themselves to `params`.
`GroovyPageAttributesTests` has no `@CompileStatic` coverage at all. The
difference matters: on `params`, statically compiled assignment to a colliding
name fails at compile time, which is loud. On `attrs`, `gspTagSyntaxCall` has a
real setter, so it compiles and silently does the wrong thing. Verified against
the real class on Groovy 5.1.0:
```
dynamic attrs.gspTagSyntaxCall = false -> mapEntry=false, field=true
# as intended
@CompileStatic attrs.gspTagSyntaxCall = false -> mapEntry=null, field=false
# setter invoked, map untouched
dynamic attrs.gspTagSyntaxCall (read) -> the map entry
@CompileStatic attrs.gspTagSyntaxCall (read) -> the boolean field
```
The subscript form is correct in both modes, as with `params`.
**Worth stating the underlying rule once, rather than per name.** The reason
the caveat exists is structural: `getProperty`/`setProperty` are runtime hooks,
so the static compiler binds to the declared accessor and never reaches them,
while `getAt(String)`/`putAt(String, Object)` *are* declared methods it
resolves directly. That is why subscript is the portable form for any colliding
name on either map — including ones a subclass adds later. A sentence to that
effect in the Javadoc here would keep the guidance from having to be re-derived
for each new accessor.
For reference, measured on Groovy 4.0.33 vs 5.1.0 with the real classes —
the Groovy 4 static and dynamic columns were identical, so the divergent rows
are Grails 7 → 8 changes rather than pre-existing quirks:
| expression | Grails 7 (Groovy 4) | this PR, dynamic | this PR,
`@CompileStatic` |
|---|---|---|---|
| `params.identifier` read | map entry | map entry | `getIdentifier()` → the
`id` param |
| `params.request` read | map entry | map entry | the `HttpServletRequest` |
| `params.identifier = 'x'` | map put | map put | compile error |
| `params['identifier']` read/write | map | map | map |
| `attrs.gspTagSyntaxCall` read | map entry | map entry | the boolean field |
| `attrs.gspTagSyntaxCall = false` | setter invoked | map put | setter
invoked, map untouched |
##########
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectInstantiationSpec.groovy:
##########
@@ -126,6 +126,34 @@ class CommandObjectInstantiationSpec extends Specification
implements Controller
where:
requestMethod << ['POST', 'PUT', 'GET', 'DELETE']
}
+
+ @Issue('https://github.com/apache/grails-core/issues/16280')
+ void 'Test a parameter named identifier does not divert domain command
object resolution'() {
+ given:
+ def target = new DomainClassCommandObject(name: 'Target').save()
+ def decoy = new DomainClassCommandObject(name: 'Decoy').save()
+
+ expect:
+ target.id != null
+ decoy.id != null
+ target.id != decoy.id
+
+ when: 'a request carries both an id and a form field named identifier'
+ request.method = 'POST'
+ params.id = target.id
+ params.identifier = decoy.id
+ controller.domainCommandObject()
+
+ then: 'the command object is resolved from id, not from the identifier
field'
+ response.status == HttpServletResponse.SC_OK
+ model.commandObject.id == target.id
Review Comment:
These two conditions are not actually sensitive to `getIdentifier()`, so the
integration half of this test does not exercise the mechanism its name
describes.
I verified by mutation — changing `GrailsParameterMap.getIdentifier()` to
`return get("identifier")` and re-running. `model.commandObject.id ==
target.id` and `model.commandObject.name == 'Target'` both still passed; the
only condition that failed was the direct `params.getIdentifier() == target.id`
further down.
The reason is that `Controller.initializeCommandObject` reads
`commandObjectBindingSource.getIdentifierValue()` first and only falls back to
`params.getIdentifier()` when that returns null.
`DefaultDataBindingSourceCreator` builds that source as `new
SimpleMapDataBindingSource(grailsWebRequest.getParams())`, whose
`getIdentifierValue()` returns `map['id']` — already `target.id` here — so the
fallback never runs.
So the coverage comes entirely from the direct assertion, which duplicates
`GrailsParameterMapTests.testGetIdentifierAlwaysReadsTheIdParameter`. To pin
the mechanism at this level, drive the fallback — e.g. a binding source with no
`id` key, so `getIdentifierValue()` returns null and `getIdentifier()` is
actually consulted. Otherwise the spec is fine but the "does not divert domain
command object resolution" claim rests on the unit test rather than on this one.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1488,6 +1488,47 @@ def value = configObject.containsKey(key) ?
configObject.get(key) : null
Grails applies this guard internally when resolving Spring profile activation
keys; you only need it in application code that navigates a raw `ConfigObject`
by subscript.
+===== 28.3 `params` and `attrs` entries that share a name with an accessor
+
+Groovy 5 changed runtime method selection for classes that implement `Map`.
+For a `String` subscript key, and for dotted property access, Groovy now
prefers a JavaBean property of the class over a map entry.
+On `params` (`GrailsParameterMap`) and on tag library `attrs`
(`GroovyPageAttributes`) that made an entry named after an accessor
unreachable: `params['request']` returned the `HttpServletRequest` instead of
the submitted parameter, and `params['identifier'] = value` threw a
`ReadOnlyPropertyException`.
+
+Grails 8 restores the Grails 7 rule.
+In dynamically compiled Groovy, `params['name']`, `params.name`,
`attrs['name']` and `attrs.name` all address a map entry; call the accessor
method when you want the framework object:
+
+[source,groovy]
+----
+params['request'] // the request parameter named "request", or null when
none was submitted
+params.request // the same parameter
+params.getRequest() // the HttpServletRequest
+----
+
+[NOTE]
+====
+This applies to dynamically compiled Groovy.
+Under `@CompileStatic` or `@GrailsCompileStatic` the compiler binds property
syntax to the declared accessor rather than routing through `getProperty`, so
for the two names that collide with an accessor on `params`,
`params.identifier` calls `getIdentifier()` and `params.request` calls
`getRequest()` instead of reading the map entry.
+Writing them that way does not compile: `params.identifier = value` fails with
`Cannot set read-only property: identifier`, and `params['request'] = value`
fails with `Cannot assign value of type java.lang.String to variable of type
jakarta.servlet.http.HttpServletRequest`.
+
+In statically compiled code, read these two parameters with
`params['identifier']` and `params['request']`, and write them with
`params.put('identifier', value)` and `params.put('request', value)`:
+
+[source,groovy]
+----
+@GrailsCompileStatic
+class BookController {
+ def save() {
+ params.put('request', 'submitted value') // params['request'] = ...
does not compile
+ def submitted = params['request'] // the parameter;
params.request is the HttpServletRequest
+ }
+}
+----
+
+Every other parameter name is unaffected, and the subscript form behaves the
same under both compilation modes.
+====
+
+One behavior differs from Grails 7: `params.metaClass` always returns the
Groovy `MetaClass`.
Review Comment:
Worth extending this paragraph to the write side, which the `metaClass`
guard makes asymmetric. Verified on Groovy 5.1.0:
```groovy
params['metaClass'] = 'mc' // stores a map entry
params.metaClass = 'mc' // GroovyCastException: Cannot cast object 'mc'
... to class 'groovy.lang.MetaClass'
```
As written the paragraph only tells readers about reading
`params['metaClass']`, so the exception on the dotted form is a surprise. One
sentence covers it.
##########
grails-gsp/grails-taglib/src/test/groovy/org/grails/taglib/GroovyPageAttributesTests.groovy:
##########
@@ -94,6 +94,38 @@ class GroovyPageAttributesTests {
assert '[one:foo]' == attrs.toString()
}
+ // https://github.com/apache/grails-core/issues/16280
+ @Test
+ void testGspTagSyntaxCallAttributeIsAMapEntry() {
+ def attrs = toGroovyPageAttributes([:])
+
+ attrs['gspTagSyntaxCall'] = 'value'
+
+ assertEquals 'value', attrs['gspTagSyntaxCall']
+ assertEquals 'value', attrs.gspTagSyntaxCall
+ assertTrue attrs.isGspTagSyntaxCall()
+
+ attrs.setGspTagSyntaxCall(false)
+
+ assertFalse attrs.isGspTagSyntaxCall()
+ assertEquals 'value', attrs['gspTagSyntaxCall']
+ }
Review Comment:
This covers the read side and the explicit `setGspTagSyntaxCall(false)`
method form, but never `attrs.gspTagSyntaxCall = false` — the
property-assignment form, which is what the new `setProperty` override changes.
Since this is the only settable bean property in the hierarchy, it is the case
worth pinning.
Two tests, both of which I ran against c9e3c0f. The first **passes** and
captures the dynamic write-side change from Grails 7:
```groovy
// https://github.com/apache/grails-core/issues/16280
@Test
void testPropertyAssignmentDoesNotInvokeTheSetter() {
def attrs = toGroovyPageAttributes([:])
attrs.gspTagSyntaxCall = false
assertEquals false, attrs['gspTagSyntaxCall']
assertTrue attrs.isGspTagSyntaxCall()
}
```
On Groovy 4 that same line invoked `setGspTagSyntaxCall(false)` and stored
nothing in the map, so this is the assertion that documents the difference.
The second **fails** today, and is the `attrs` counterpart of
`testStaticCompilationBindsPropertySyntaxToTheAccessor`:
```groovy
// https://github.com/apache/grails-core/issues/16280
@Test
void testStaticallyCompiledAttributeAccessAddressesTheMap() {
def attrs = toGroovyPageAttributes([:])
StaticallyCompiledAccess.write(attrs)
assertEquals false, attrs['gspTagSyntaxCall']
assertTrue attrs.isGspTagSyntaxCall()
}
@CompileStatic
static class StaticallyCompiledAccess {
static void write(GroovyPageAttributes attrs) {
attrs.gspTagSyntaxCall = false
}
}
```
Both need `import groovy.transform.CompileStatic`. The second fails with
`expected: <false> but was: <null>` — the map entry was never written and the
field was.
If the `attrs` static behavior is accepted as a documented limitation like
`params`, then inverting that second assertion (asserting the field was written
and the map was not) is the right form — it still locks the behavior down and
makes it discoverable instead of silent. Either way I'd not leave it untested,
since this is the one case with no compile error to warn the caller.
##########
grails-gsp/grails-taglib/build.gradle:
##########
@@ -84,6 +84,7 @@ dependencies {
testImplementation 'org.junit.jupiter:junit-jupiter-api'
testImplementation 'org.slf4j:slf4j-nop' // Get rid of warning about
missing slf4j implementation during compilation and tests
testImplementation 'org.spockframework:spock-core'
+ testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine' // Required to
discover the JUnit 5 tests in this module
Review Comment:
Confirmed this is a real gap and not a no-op: reverting only this line makes
`:grails-taglib:test` report BUILD SUCCESSFUL while discovering zero tests, so
`GroovyPageAttributesTests` had never executed.
Worth considering whether it belongs in `gradle/test-config.gradle` instead.
That convention already adds `testRuntimeOnly
'org.junit.platform:junit-platform-launcher'` and calls `useJUnitPlatform()`,
but never adds the Jupiter engine — which is why the failure mode existed at
all. Every other module with JUnit 5 tests gets the engine only transitively,
through `runtimeOnly 'org.junit.jupiter:junit-jupiter-engine'` in
`grails-testing-support-core`; I checked the six modules that declare
`junit-jupiter-api` without an explicit engine and they all resolve it that way
today. If that transitive edge is ever narrowed, the same silent-skip returns
elsewhere with a green build.
Fine to keep the module-local fix and do the convention change separately,
but it would be the fix for the class of problem rather than this instance.
##########
grails-web-common/src/test/groovy/grails/web/servlet/mvc/GrailsParameterMapTests.groovy:
##########
@@ -394,6 +395,212 @@ class GrailsParameterMapTests {
assertEquals "1", theMap['test']
}
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testAddingIdentifierParam() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ theMap['identifier'] = 'id1'
+
+ assertEquals 'id1', theMap['identifier']
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testSubscriptAccessForNamesThatCollideWithGetters() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ ['identifier', 'request', 'empty', 'class', 'metaClass', 'plain'].each
{ String name ->
+ theMap[name] = "value of $name".toString()
+ }
+
+ ['identifier', 'request', 'empty', 'class', 'metaClass', 'plain'].each
{ String name ->
+ assertEquals "value of $name".toString(), theMap[name], "subscript
access for [$name] should read the parameter"
+ assertTrue theMap.containsKey(name), "[$name] should be a key of
the map"
+ }
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testPropertyAccessForNamesThatCollideWithGetters() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ theMap.identifier = 'id1'
+ theMap.request = 'r1'
+ theMap.empty = 'e1'
+
+ assertEquals 'id1', theMap.identifier
+ assertEquals 'r1', theMap.request
+ assertEquals 'e1', theMap.empty
+ assertEquals 'id1', theMap['identifier']
+ assertEquals 'r1', theMap['request']
+ assertEquals 'e1', theMap['empty']
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testGetIdentifierAlwaysReadsTheIdParameter() {
+ mockRequest.addParameter("id", "abc")
+ theMap = new GrailsParameterMap(mockRequest)
+
+ assertEquals 'abc', theMap.getIdentifier()
+
+ // a parameter named "identifier" is an ordinary map entry and must
not divert
+ // getIdentifier(), which Controller.initializeCommandObject() uses to
load a domain object
+ theMap['identifier'] = 'other'
+
+ assertEquals 'abc', theMap.getIdentifier()
+ assertEquals 'other', theMap['identifier']
+ assertEquals 'other', theMap.identifier
+ assertEquals 'abc', theMap['id']
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testGetIdentifierIsNullWhenNoIdParameterWasSubmitted() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ assertNull theMap.getIdentifier()
+
+ theMap['identifier'] = 'other'
+
+ assertNull theMap.getIdentifier()
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testGetRequestStillReturnsTheRequest() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ theMap['request'] = 'r1'
+
+ assertSame mockRequest, theMap.getRequest()
+ assertEquals 'r1', theMap['request']
+ assertEquals 'r1', theMap.request
+ }
+
+ @Test
+ @CompileStatic
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testStaticCompilationBindsPropertySyntaxToTheAccessor() {
+ GrailsParameterMap params = new GrailsParameterMap(mockRequest)
+
+ // subscript access addresses the map under @CompileStatic as well,
because getAt(String)
+ // and putAt(String, Object) are declared methods the static compiler
resolves directly
+ params['identifier'] = 'id1'
+ assertEquals 'id1', params['identifier']
+ assertNull params['request']
+
+ // put() is the portable way to write a parameter whose name collides
with an accessor;
+ // params['request'] = 'r1' does not compile, because the static
compiler binds it to the
+ // HttpServletRequest-typed "request" property
+ params.put('request', 'r1')
+ assertEquals 'r1', params['request']
+
+ // property syntax binds to the declared accessor at compile time and
bypasses
+ // getProperty, so it does NOT read the map entry under static
compilation.
+ // Writing it does not compile at all: "params.identifier = value"
fails with
+ // "Cannot set read-only property: identifier".
+ assertNull params.identifier // getIdentifier() reads the
absent "id" parameter
+ assertSame mockRequest, params.request // getRequest()
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testCollidingNamesAreNullWhenNoSuchParameterWasSubmitted() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ // "request" and "identifier" name parameters, so with no such
parameter submitted they
+ // read as null rather than falling back to
getRequest()/getIdentifier()
+ assertNull theMap['request']
+ assertNull theMap.request
+ assertNull theMap['identifier']
+ assertNull theMap.identifier
+ assertNull theMap.nonexistent
+
+ // the accessors themselves are unaffected
+ assertSame mockRequest, theMap.getRequest()
+ assertNull theMap.getIdentifier() // no "id" parameter was submitted
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testCollidingParameterNamesArrivingFromTheRequest() {
+ mockRequest.addParameter("identifier", "id1")
+ mockRequest.addParameter("request", "r1")
+ theMap = new GrailsParameterMap(mockRequest)
+
+ assertEquals 'id1', theMap['identifier']
+ assertEquals 'r1', theMap['request']
+ assertEquals 'id1', theMap.identifier
+ assertEquals 'r1', theMap.request
+ assertSame mockRequest, theMap.getRequest()
+ }
+
+ @Test
+ @Issue("https://github.com/apache/grails-core/issues/16280";)
+ void testMetaClassPropertyIsNotShadowedByTheMap() {
+ theMap = new GrailsParameterMap(mockRequest)
+
+ theMap['metaClass'] = 'mc'
+
+ assertEquals 'mc', theMap['metaClass']
+ assertNotNull theMap.metaClass
+ assertNotEquals 'mc', theMap.metaClass
Review Comment:
Nit: this does catch the `metaClass` guard being removed — the property
would then return the string `'mc'` — but only because of the specific value
stored above. With any other value it would be vacuous, since a `String` is
never equal to a `MetaClass`.
The grails-core twin states the intent directly:
```groovy
assert map.metaClass instanceof MetaClass
```
Worth matching here for consistency.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1488,6 +1488,47 @@ def value = configObject.containsKey(key) ?
configObject.get(key) : null
Grails applies this guard internally when resolving Spring profile activation
keys; you only need it in application code that navigates a raw `ConfigObject`
by subscript.
+===== 28.3 `params` and `attrs` entries that share a name with an accessor
+
+Groovy 5 changed runtime method selection for classes that implement `Map`.
+For a `String` subscript key, and for dotted property access, Groovy now
prefers a JavaBean property of the class over a map entry.
+On `params` (`GrailsParameterMap`) and on tag library `attrs`
(`GroovyPageAttributes`) that made an entry named after an accessor
unreachable: `params['request']` returned the `HttpServletRequest` instead of
the submitted parameter, and `params['identifier'] = value` threw a
`ReadOnlyPropertyException`.
+
+Grails 8 restores the Grails 7 rule.
+In dynamically compiled Groovy, `params['name']`, `params.name`,
`attrs['name']` and `attrs.name` all address a map entry; call the accessor
method when you want the framework object:
+
+[source,groovy]
+----
+params['request'] // the request parameter named "request", or null when
none was submitted
+params.request // the same parameter
+params.getRequest() // the HttpServletRequest
+----
+
+[NOTE]
+====
+This applies to dynamically compiled Groovy.
+Under `@CompileStatic` or `@GrailsCompileStatic` the compiler binds property
syntax to the declared accessor rather than routing through `getProperty`, so
for the two names that collide with an accessor on `params`,
`params.identifier` calls `getIdentifier()` and `params.request` calls
`getRequest()` instead of reading the map entry.
+Writing them that way does not compile: `params.identifier = value` fails with
`Cannot set read-only property: identifier`, and `params['request'] = value`
fails with `Cannot assign value of type java.lang.String to variable of type
jakarta.servlet.http.HttpServletRequest`.
+
+In statically compiled code, read these two parameters with
`params['identifier']` and `params['request']`, and write them with
`params.put('identifier', value)` and `params.put('request', value)`:
+
+[source,groovy]
+----
+@GrailsCompileStatic
+class BookController {
+ def save() {
+ params.put('request', 'submitted value') // params['request'] = ...
does not compile
+ def submitted = params['request'] // the parameter;
params.request is the HttpServletRequest
+ }
+}
+----
+
+Every other parameter name is unaffected, and the subscript form behaves the
same under both compilation modes.
Review Comment:
This NOTE is a good addition and matches what I measured. Two gaps:
**It only addresses `params`.** `attrs` (`GroovyPageAttributes`) has the
same static-compilation behavior, and there it is quieter, because
`gspTagSyntaxCall` has a real setter — `attrs.gspTagSyntaxCall = false` under
`@CompileStatic` compiles and silently writes the field. Since the section
header covers both `params` and `attrs`, a reader will reasonably assume the
NOTE does too. Suggest either naming `attrs` here or generalising: subscript is
the portable form for any accessor-colliding name on either map.
**"Every other parameter name is unaffected" is true only for reads.**
Assignment to a name backed by a real setter also changed, in dynamically
compiled code: on Grails 7 `attrs.gspTagSyntaxCall = false` and
`attrs['gspTagSyntaxCall'] = false` both invoked `setGspTagSyntaxCall`, and
both now write a map entry instead. That is a second difference from Grails 7
alongside the `metaClass` read, and it is currently undocumented.
--
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]
