jdaugherty commented on code in PR #16694:
URL: https://github.com/apache/grails-core/pull/16694#discussion_r4337240241


##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1592,6 +1592,32 @@ Subscript assignment, `attrs['gspTagSyntaxCall'] = 
value`, now stores an attribu
 
 NOTE: When adding a class that extends `grails.util.TypeConvertingMap`, do not 
declare a no-argument `getX()` or `isX()` method on it, or the entry `x` 
becomes unreachable for the same reason. Statically compiled callers bind 
directly to the accessor, so the collision cannot be corrected at runtime.
 
+===== 28.4 Map entries named `properties` or `metaClass`
+
+The accessor-first resolution described above applies to every `Map`, not only 
to a class that declares an accessor of its own, because every Groovy object 
has the `properties` and `metaClass` properties.
+In Groovy 4 `map.properties` and `map['properties']` read the map entry.
+In Groovy 5 both return the Groovy meta-properties of the map object, in 
dynamic and in statically compiled code alike, and `map['properties'] = value` 
throws `groovy.lang.ReadOnlyPropertyException`.
+`metaClass` behaves the same way, and assigning to it replaces the metaclass 
of the map.
+`class` and `empty` are exempt and still address the map entry.
+
+Such keys are common in data an application does not control; the `properties` 
member of a GeoJSON `Feature` is one.
+Grails 8 stores and loads them through the `Map` API, so they round-trip 
through a `Map` property or an embedded map of a GORM for MongoDB entity, and 
through data binding of a `Map` property, whether the value arrives as a nested 
map or as an indexed key such as `attributes[properties]`.
+
+Application code that reads one of these keys from a `Map` must call `get`:
+
+[source,groovy]
+----
+Map feature = place.geoJson    // a Map property holding a GeoJSON Feature
+
+feature.get('properties')      // the entry
+feature.properties             // the meta-properties of the map object
+feature['properties']          // the same, on Groovy 5
+----
+
+The map a loaded GORM `Map` property comes back as is a dirty-checking wrapper 
with accessors of its own, named `target`, `parent`, `property`, `originalSize` 
and `assigned`.
+The wrapper resolves those five names to the map entry, in dotted and 
subscript form, so an entry such as `parent` reads and writes through the 
wrapper as it does through the plain map, and as it did in Grails 7.
+The one form that still reaches the accessor is dotted access from statically 
compiled code, which binds at compile time as it always has; read such an entry 
with `get` there.

Review Comment:
   Applied in 03fe486da6 with the suggested wording. I confirmed the getters on 
hibernate-core 7.4.10: `getKey`, `getValue`, `getOwner`, `getRole`, `getSize`, 
`getSession`, `isDirty` and the rest you listed are all declared on 
`AbstractPersistentCollection` or `PersistentMap`. The PR description now says 
the same about Hibernate.



##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1592,6 +1592,32 @@ Subscript assignment, `attrs['gspTagSyntaxCall'] = 
value`, now stores an attribu
 
 NOTE: When adding a class that extends `grails.util.TypeConvertingMap`, do not 
declare a no-argument `getX()` or `isX()` method on it, or the entry `x` 
becomes unreachable for the same reason. Statically compiled callers bind 
directly to the accessor, so the collision cannot be corrected at runtime.
 
+===== 28.4 Map entries named `properties` or `metaClass`
+
+The accessor-first resolution described above applies to every `Map`, not only 
to a class that declares an accessor of its own, because every Groovy object 
has the `properties` and `metaClass` properties.
+In Groovy 4 `map.properties` and `map['properties']` read the map entry.
+In Groovy 5 both return the Groovy meta-properties of the map object, in 
dynamic and in statically compiled code alike, and `map['properties'] = value` 
throws `groovy.lang.ReadOnlyPropertyException`.
+`metaClass` behaves the same way, and assigning to it replaces the metaclass 
of the map.

Review Comment:
   Applied in 03fe486da6. Confirmed on 4.0.33 and 5.1.3: `map['metaClass'] = 
'x'` throws `GroovyCastException` on both, in dynamic code and under 
`@CompileStatic`, and only the read changed. The table in the description now 
has a read row and the corrected write row, and the "Affects" line says which 
half is new in Groovy 5.



##########
grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MapKeysNamedLikeBeanPropertiesSpec.groovy:
##########
@@ -0,0 +1,128 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.grails.datastore.gorm.mongo
+
+import grails.persistence.Entity
+import org.apache.grails.data.mongo.core.GrailsDataMongoTckManager
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.bson.Document
+import org.bson.types.ObjectId
+import spock.lang.Issue
+
+/**
+ * A Map property whose entries are named like bean properties of the map, as 
the {@code properties}
+ * member of a GeoJSON Feature is, is stored and loaded like any other entry. 
Groovy 5 resolves a
+ * String subscript on a Map as property access, so the codecs go through the 
Map API instead, and
+ * the dirty-checking wrapper a loaded map comes back as resolves its own 
accessor names
+ * ({@code target}, {@code parent}, {@code property}, {@code originalSize}, 
{@code assigned}) to
+ * the entries.
+ */
+@Issue('https://github.com/apache/grails-core/issues/16675')
+class MapKeysNamedLikeBeanPropertiesSpec extends 
GrailsDataTckSpec<GrailsDataMongoTckManager> {
+
+    static final Map FEATURE = [
+            type        : 'Feature',
+            geometry    : [type: 'Point', coordinates: [100.0d, 0.0d]],
+            properties  : [name: 'Test', metaClass: 'nested'],
+            metaClass   : 'top',
+            firstKey    : 'first',
+            target      : 't',
+            parent      : 'p',
+            property    : 'q',
+            originalSize: 42,
+            assigned    : 'yes'
+    ]
+
+    void setupSpec() {
+        manager.registerDomainClasses(GeoFeature)
+    }
+
+    void "a Map property saved with a key named properties is read back with 
the entry intact"() {
+        when: "an entity whose map holds keys named like bean properties is 
saved and reloaded"
+        new GeoFeature(name: 'saved', geoJson: new 
LinkedHashMap(FEATURE)).save(flush: true, failOnError: true)
+        manager.session.clear()
+        GeoFeature loaded = GeoFeature.findByName('saved')
+
+        then: "every entry survives the round trip"
+        loaded.geoJson.get('properties') == [name: 'Test', metaClass: 'nested']
+        loaded.geoJson.get('metaClass') == 'top'
+        loaded.geoJson.get('firstKey') == 'first'
+        loaded.geoJson == FEATURE
+
+        and: "entries named like the accessors of the loaded map's wrapper 
read as entries"
+        loaded.geoJson.target == 't'

Review Comment:
   Applied in 03fe486da6, with the import. The spec passes locally against the 
MongoDB container with the premise asserted.



-- 
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]

Reply via email to