jamesfredley commented on code in PR #16485:
URL: https://github.com/apache/grails-core/pull/16485#discussion_r4174334284
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/proxy/ByteBuddyGroovyInterceptorSpec.groovy:
##########
@@ -142,15 +142,14 @@ class ByteBuddyGroovyInterceptorSpec extends
HibernateGormDatastoreSpec {
!Hibernate.isInitialized(proxy)
}
- void "getIdentifier() on uninitialized proxy returns identifier without
initialization"() {
+ void "a user-defined getIdentifier() invokes the entity implementation"() {
given:
def proxy = manager.hibernateSession.getReference(Location, savedId)
Review Comment:
This feature still obtains the proxy through
`manager.hibernateSession.getReference(...)` rather than the public GORM API.
`ProxyIdentifierAccessSpec` covers business `getIdentifier()` through `load()`,
so this is not blocking. When this spec is next touched,
`Location.load(savedId)` would match the rule that tests go through the public
surface.
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormInstanceApi.groovy:
##########
@@ -273,7 +276,27 @@ class GormInstanceApi<D> extends AbstractGormApi<D>
implements GormInstanceOpera
@Override
Serializable ident(D instance) {
- (Serializable)InvokerHelper.getProperty(instance, 'id')
+ PersistentEntity entity =
mappingContext.getPersistentEntity(persistentClass.name)
+ if (entity == null) {
+ return (Serializable) InvokerHelper.getProperty(instance, 'id')
+ }
+ PersistentProperty identity = entity.identity
+ if (identity != null) {
+ return (Serializable) InvokerHelper.getProperty(instance,
identity.name)
+ }
+
+ PersistentProperty[] idProperties = entity.compositeIdentity
+ if (idProperties != null) {
+ def identifier = entity.newInstance()
+ if (identifier instanceof Serializable) {
+ EntityReflector reflector = entity.reflector
+ for (PersistentProperty property : idProperties) {
+ reflector.setProperty(identifier, property.name,
reflector.getProperty(instance, property.name))
Review Comment:
Reading `identity.name` fixes renamed identifiers on a real instance, but it
regresses lazy `ident()` for non-Hibernate proxies.
`EntityProxyMethodHandler` returns the cached proxy key for property `id`
and for `getId()` without resolving the target. It does not shortcut a renamed
property such as `code` or `slug`. `InvokerHelper.getProperty(instance,
identity.name)` therefore falls through to `resolveDelegate()` and loads the
entity. The composite loop below has the same effect:
`reflector.getProperty(instance, property.name)` initializes the proxy for each
key field.
On 8.0.x, `ident()` always read `id`, so an uninitialized MongoDB, Neo4j, or
Redis proxy returned the key it already held. After this change, `ident()` on
an uninitialized renamed-key proxy hits the datastore and can fail if that row
is gone.
Hibernate is not hit the same way. `GroovyProxyInterceptorLogic` still
short-circuits the method name `ident` before this body runs. Javassist proxies
do not.
Please return `ProxyHandler.getIdentifier()` when the instance is an
uninitialized proxy, before reading the mapped property. Add a public-API test
that an uninitialized renamed-key proxy returns that key and does not
initialize.
##########
grails-doc/src/en/ref/Domain Classes/ident.adoc:
##########
@@ -25,7 +25,9 @@ under the License.
=== Purpose
-Returns the value of the identity property of the domain class regardless of
the name of the identity property itself
+Returns the value of the mapped identity property of the domain class,
regardless of its name. For example, when the mapping uses `id name: 'code'`,
`ident()` returns the value of `code`. A generated identifier remains `null`
until it has been assigned.
+
+For a composite identity, `ident()` returns a separate serializable instance
of the domain class with the mapped key properties copied into it, including
any associations that form part of the key. Other properties retain their
new-instance defaults. Pass the returned identifier to `get()` or `load()` to
retrieve the entity.
Review Comment:
This describes every composite `ident()` call as building a separate
key-only prototype. That is true for a non-proxy instance. An uninitialized
Hibernate proxy returns the identifier it was created with, including any
non-key state already on that object. `Domain.load(key).ident()` can return the
supplied key rather than a freshly copied prototype.
Please qualify the prototype wording so it applies to a non-proxy entity,
and note that an uninitialized proxy returns its existing identifier without
loading.
--
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]
