Add proxy package test coverage and fix IntelliJ inspections in grails-datamapping-core

grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy

@@ -18,6 +18,7 @@
  */
 package org.grails.datastore.gorm.proxy
 
+import groovy.transform.CompileDynamic
 import groovy.transform.CompileStatic
 import org.codehaus.groovy.runtime.HandleMetaClass
 import org.codehaus.groovy.runtime.InvokerHelper
@@ -70,7 +71,8 @@ class GroovyProxyFactory implements ProxyFactory {
         }
     }
 
-    @groovy.transform.CompileDynamic
+    @CompileDynamic
+    @SuppressWarnings('GrMethodMayBeStatic')
     protected Serializable getIdDynamic(obj) {
         if (obj.respondsTo('getId')) {
             return (Serializable)obj.invokeMethod('getId', null)
@@ -90,7 +92,7 @@ class GroovyProxyFactory implements ProxyFactory {
     @Override
     <T> T createProxy(Session session, Class<T> type, Serializable key) {
         EntityPersister persister = (EntityPersister) session.getPersister(type)
-        T proxy = type.newInstance()
+        T proxy = type.getDeclaredConstructor().newInstance()
         if (persister != null) {
             persister.setObjectIdentifier(proxy, key)
         } else {
@@ -121,7 +123,8 @@ class GroovyProxyFactory implements ProxyFactory {
         return proxy
     }
 
-    @groovy.transform.CompileDynamic
+    @CompileDynamic
+    @SuppressWarnings('GrMethodMayBeStatic')
     protected void setMetaClassDynamic(Object proxy, MetaClass proxyMc) {
         proxy.setMetaClass(proxyMc)
     }
@@ -161,6 +164,7 @@ class GroovyProxyFactory implements ProxyFactory {
         return object
     }
 
+    @SuppressWarnings('GrMethodMayBeStatic')
     protected MetaClass unwrapHandleMetaClass(MetaClass mc) {
         if (mc instanceof HandleMetaClass) {
             return ((HandleMetaClass) mc).getAdaptee()

grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClass.java

@@ -46,15 +46,15 @@ public class ProxyInstanceMetaClass extends DelegatingMetaClass {
     /**
      * Session to fetch from, if we need to.
      */
-    private Session session;
+    private final Session session;
     /**
      * The loaded instance we're proxying, or null if it hasn't been loaded.
      */
     private Object proxyTarget;
     /**
      * The key of the object.
      */
-    private Serializable key;
+    private final Serializable key;
 
     public ProxyInstanceMetaClass(MetaClass delegate, Session session, Serializable key) {
         super(delegate);
@@ -66,6 +66,7 @@ public ProxyInstanceMetaClass(MetaClass delegate, Session session, Serializable
      * Load the target from the DB.
      * @return target.
      */
+    @SuppressWarnings("unchecked")
     public Object getProxyTarget() {
         if (proxyTarget == null) {
             proxyTarget = session.retrieve(getTheClass(), getKey());
@@ -82,9 +83,9 @@ public Object getProxyTarget() {
     /**
      * Handle method calls on our proxy.
      * @param o The proxy.
-     * @param methodName
-     * @param arguments
-     * @return
+     * @param methodName The name of the method being invoked.
+     * @param arguments The arguments passed to the method.
+     * @return The result of invoking the method, resolving the proxy target first if required.
      */
     @Override
     public Object invokeMethod(Object o, String methodName, Object[] arguments) {
@@ -118,44 +119,33 @@ public boolean isProxyInitiated() {
 
     @Override
     public Object getProperty(Object object, String property) {
-        if (property.equals("id")) {
-            return getKey();
-        } else if (property.equals("proxy")) {
-            return true;
-        } else if (property.equals("initialized")) {
-            return isProxyInitiated();
-        } else if (property.equals("target")) {
-            return getProxyTarget();
-        } else if (property.equals("metaClass")) {
-            return this;
-        } else if (property.equals("class") || property.equals("domainClass")) {
-            // return correct class only if loaded, otherwise hope for the best
-            return delegate.getProperty(isProxyInitiated() ? proxyTarget : object, property);
-        } else {
-            return delegate.getProperty(getProxyTarget(), property);
-        }
+        return switch (property) {
+            case "id" -> getKey();
+            case "proxy" -> true;
+            case "initialized" -> isProxyInitiated();
+            case "target" -> getProxyTarget();
+            case "metaClass" -> this;
+            case "class", "domainClass" ->
+                // return correct class only if loaded, otherwise hope for the best
+                delegate.getProperty(isProxyInitiated() ? proxyTarget : object, property);
+            default -> delegate.getProperty(getProxyTarget(), property);
+        };
     }
 
     @Override
     public void setProperty(Object object, String property, Object newValue) {
-        boolean resolveTarget = true;
-        if (property.equals("metaClass") && (newValue == null || newValue instanceof MetaClass)) {
-            resolveTarget = false;
-        }
+        boolean resolveTarget = !property.equals("metaClass") || (newValue != null && !(newValue instanceof MetaClass));
         delegate.setProperty(resolveTarget ? getProxyTarget() : object, property, newValue);
     }
 
     @Override
     public Object getAttribute(Object object, String attribute) {
-        if (attribute.equals("id")) {
-            return getKey();
-        } else if (attribute.equals("initialized")) {
-            return isProxyInitiated();
-        } else if (attribute.equals("target")) {
-            return getProxyTarget();
-        } else {
-            return delegate.getAttribute(getProxyTarget(), attribute);
-        }
+        return switch (attribute) {
+            case "id" -> getKey();
+            case "initialized" -> isProxyInitiated();
+            case "target" -> getProxyTarget();
+            default -> delegate.getAttribute(getProxyTarget(), attribute);
+        };
     }
 
     @Override

grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactorySpec.groovy

@@ -0,0 +1,148 @@
+/*
+ *  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.proxy
+
+import org.grails.datastore.mapping.core.Session
+import org.grails.datastore.mapping.engine.AssociationQueryExecutor
+import org.grails.datastore.mapping.engine.EntityPersister
+import spock.lang.Specification
+
+class GroovyProxyFactorySpec extends Specification {
+
+    GroovyProxyFactory proxyFactory = new GroovyProxyFactory()
+
+    void "createProxy returns an initialized-looking instance whose identifier is available without loading"() {
+        given:
+        Session session = Mock(Session)
+        session.getPersister(ProxyFactoryTestDomain) >> null
+        session.getMappingContext() >> null
+
+        when:
+        ProxyFactoryTestDomain proxy = proxyFactory.createProxy(session, ProxyFactoryTestDomain, 42L)
+
+        then:
+        proxyFactory.isProxy(proxy)
+        proxyFactory.getIdentifier(proxy) == 42L
+        !proxyFactory.isInitialized(proxy)
+        0 * session.retrieve(_, _)
+    }
+
+    void "createProxy uses the session's persister to set the object identifier when available"() {
+        given:
+        Session session = Mock(Session)
+        EntityPersister persister = Mock(EntityPersister)
+        session.getPersister(ProxyFactoryTestDomain) >> persister
+
+        when:
+        ProxyFactoryTestDomain proxy = proxyFactory.createProxy(session, ProxyFactoryTestDomain, 99L)
+
+        then:
+        1 * persister.setObjectIdentifier(_, 99L)
+        proxyFactory.isProxy(proxy)
+    }
+
+    void "unwrap loads and returns the target for a proxy, caching it as initialized"() {
+        given:
+        Session session = Mock(Session)
+        session.getPersister(ProxyFactoryTestDomain) >> null
+        ProxyFactoryTestDomain target = new ProxyFactoryTestDomain(id: 7L, name: 'loaded')
+        ProxyFactoryTestDomain proxy = proxyFactory.createProxy(session, ProxyFactoryTestDomain, 7L)
+
+        when:
+        Object result = proxyFactory.unwrap(proxy)
+
+        then:
+        1 * session.retrieve(ProxyFactoryTestDomain, 7L) >> target
+        result.is(target)
+
+        and: 'the proxy is now considered initialized without a second retrieve'
+        proxyFactory.isInitialized(proxy)
+        0 * session.retrieve(_, _)
+    }
+
+    void "unwrap returns the object unchanged when it is not a proxy"() {
+        given:
+        ProxyFactoryTestDomain plain = new ProxyFactoryTestDomain(id: 1L)
+
+        expect:
+        proxyFactory.unwrap(plain).is(plain)
+        !proxyFactory.isProxy(plain)
+        proxyFactory.isInitialized(plain)
+    }
+
+    void "getIdentifier falls back to invoking getId() on a non-proxied object"() {
+        given:
+        ProxyFactoryTestDomain plain = new ProxyFactoryTestDomain(id: 5L)
+
+        expect:
+        proxyFactory.getIdentifier(plain) == 5L
+    }
+
+    void "getProxiedClass returns the runtime class regardless of proxy state"() {
+        given:
+        ProxyFactoryTestDomain plain = new ProxyFactoryTestDomain(id: 1L)
+        Session session = Mock(Session)
+        session.getPersister(ProxyFactoryTestDomain) >> null
+        ProxyFactoryTestDomain proxy = proxyFactory.createProxy(session, ProxyFactoryTestDomain, 2L)
+
+        expect:
+        proxyFactory.getProxiedClass(plain) == ProxyFactoryTestDomain
+        proxyFactory.getProxiedClass(proxy) == ProxyFactoryTestDomain
+    }
+
+    void "initialize eagerly resolves the proxy target"() {
+        given:
+        Session session = Mock(Session)
+        session.getPersister(ProxyFactoryTestDomain) >> null
+        ProxyFactoryTestDomain target = new ProxyFactoryTestDomain(id: 3L)
+        ProxyFactoryTestDomain proxy = proxyFactory.createProxy(session, ProxyFactoryTestDomain, 3L)
+
+        when:
+        proxyFactory.initialize(proxy)
+
+        then:
+        1 * session.retrieve(ProxyFactoryTestDomain, 3L) >> target
+        proxyFactory.isInitialized(proxy)
+    }
+
+    void "association proxies are not supported"() {
+        given:
+        Session session = Mock(Session)
+        AssociationQueryExecutor executor = Mock(AssociationQueryExecutor)
+
+        when:
+        proxyFactory.createProxy(session, executor, 1L)
+
+        then:
+        thrown(UnsupportedOperationException)
+    }
+
+    void "isInitialized(object, associationName) treats a null association as initialized"() {
+        given:
+        ProxyFactoryTestDomain owner = new ProxyFactoryTestDomain(id: 1L, name: null)
+
+        expect:
+        proxyFactory.isInitialized(owner, 'name')
+    }
+}
+
+class ProxyFactoryTestDomain {
+    Long id
+    String name
+}