diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy index 3b14dc1b0d2..0e12422c7f3 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy +++ b/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 createProxy(Session session, Class 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() diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClass.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClass.java index 36d5b841d6f..03670514ce0 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClass.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClass.java @@ -46,7 +46,7 @@ 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. */ @@ -54,7 +54,7 @@ public class ProxyInstanceMetaClass extends DelegatingMetaClass { /** * 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 diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactorySpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactorySpec.groovy new file mode 100644 index 00000000000..ded7a2d5d9e --- /dev/null +++ b/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 +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClassSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClassSpec.groovy new file mode 100644 index 00000000000..7ac384afe1b --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClassSpec.groovy @@ -0,0 +1,360 @@ +/* + * 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.springframework.dao.DataIntegrityViolationException +import spock.lang.Specification + +class ProxyInstanceMetaClassSpec extends Specification { + + Session session = Mock(Session) + MetaClass delegate = Mock(MetaClass) + ProxyInstanceTestTarget target = new ProxyInstanceTestTarget() + Object proxy = new Object() + + void setup() { + delegate.getTheClass() >> ProxyInstanceTestTarget + } + + ProxyInstanceMetaClass newMetaClass() { + new ProxyInstanceMetaClass(delegate, session, 11L) + } + + void "getKey returns the identifier without resolving the target"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + Serializable key = metaClass.getKey() + boolean initiated = metaClass.isProxyInitiated() + + then: + key == 11L + !initiated + 0 * session.retrieve(_, _) + } + + void "getProxyTarget lazily loads and caches the target from the session"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + Object first = metaClass.getProxyTarget() + Object second = metaClass.getProxyTarget() + + then: + 1 * session.retrieve(ProxyInstanceTestTarget, 11L) >> target + first.is(target) + second.is(target) + metaClass.isProxyInitiated() + } + + void "getProxyTarget throws DataIntegrityViolationException when the associated instance no longer exists"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> null + + when: + metaClass.getProxyTarget() + + then: + thrown(DataIntegrityViolationException) + } + + void "invokeMethod handles proxy-aware methods without resolving the target"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + Object isProxyResult = metaClass.invokeMethod(proxy, 'isProxy', [] as Object[]) + Object getIdResult = metaClass.invokeMethod(proxy, 'getId', [] as Object[]) + Object isInitializedResult = metaClass.invokeMethod(proxy, 'isInitialized', [] as Object[]) + Object getMetaClassResult = metaClass.invokeMethod(proxy, 'getMetaClass', [] as Object[]) + + then: + isProxyResult == true + getIdResult == 11L + isInitializedResult == false + getMetaClassResult.is(metaClass) + 0 * session.retrieve(_, _) + } + + void "invokeMethod resolves the target and delegates for getTarget/initialize"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + delegate.invokeMethod(target, methodName, [] as Object[]) >> target + + expect: + metaClass.invokeMethod(proxy, methodName, [] as Object[]).is(target) + + where: + methodName << ['getTarget', 'initialize'] + } + + void "invokeMethod delegates other calls against the resolved target"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + delegate.invokeMethod(target, 'toString', [] as Object[]) >> 'resolved' + + expect: + metaClass.invokeMethod(proxy, 'toString', [] as Object[]) == 'resolved' + } + + void "invokeMethod for getClass/getDomainClass only resolves once the proxy is already initiated"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + delegate.invokeMethod(proxy, methodName, [] as Object[]) >> ProxyInstanceTestTarget + + when: 'not yet initiated, so the delegate is called against the uninitialized proxy' + Object result = metaClass.invokeMethod(proxy, methodName, [] as Object[]) + + then: + result == ProxyInstanceTestTarget + 0 * session.retrieve(_, _) + + where: + methodName << ['getClass', 'getDomainClass'] + } + + void "invokeMethod for getClass/getDomainClass resolves the target once already initiated"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + metaClass.getProxyTarget() + delegate.invokeMethod(target, 'getClass', [] as Object[]) >> ProxyInstanceTestTarget + + expect: + metaClass.invokeMethod(proxy, 'getClass', [] as Object[]) == ProxyInstanceTestTarget + } + + void "invokeMethod does not resolve the target for setMetaClass with a MetaClass argument"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + MetaClass newMetaClassArg = Mock(MetaClass) + delegate.invokeMethod(proxy, 'setMetaClass', [newMetaClassArg] as Object[]) >> null + + when: + metaClass.invokeMethod(proxy, 'setMetaClass', [newMetaClassArg] as Object[]) + + then: + 0 * session.retrieve(_, _) + } + + void "invokeMethod resolves the target for setMetaClass calls with a non-MetaClass argument"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + delegate.invokeMethod(target, 'setMetaClass', ['not-a-metaclass'] as Object[]) >> null + + when: + metaClass.invokeMethod(proxy, 'setMetaClass', ['not-a-metaclass'] as Object[]) + + then: + 1 * session.retrieve(ProxyInstanceTestTarget, 11L) >> target + } + + void "invokeMethod resolves the target for setMetaClass calls with an unexpected argument count"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + delegate.invokeMethod(target, 'setMetaClass', [] as Object[]) >> null + + when: + metaClass.invokeMethod(proxy, 'setMetaClass', [] as Object[]) + + then: + 1 * session.retrieve(ProxyInstanceTestTarget, 11L) >> target + } + + void "invokeMethod does not resolve the target for setMetaClass with a null argument"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + delegate.invokeMethod(proxy, 'setMetaClass', [null] as Object[]) >> null + + when: + metaClass.invokeMethod(proxy, 'setMetaClass', [null] as Object[]) + + then: + 0 * session.retrieve(_, _) + } + + void "getProperty exposes proxy metadata without resolving the target"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + Object idResult = metaClass.getProperty(proxy, 'id') + Object proxyResult = metaClass.getProperty(proxy, 'proxy') + Object initializedResult = metaClass.getProperty(proxy, 'initialized') + Object metaClassResult = metaClass.getProperty(proxy, 'metaClass') + + then: + idResult == 11L + proxyResult == true + initializedResult == false + metaClassResult.is(metaClass) + 0 * session.retrieve(_, _) + } + + void "getProperty for class/domainClass only resolves once the proxy is already initiated"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + delegate.getProperty(proxy, propertyName) >> ProxyInstanceTestTarget + + when: + Object result = metaClass.getProperty(proxy, propertyName) + + then: + result == ProxyInstanceTestTarget + 0 * session.retrieve(_, _) + + where: + propertyName << ['class', 'domainClass'] + } + + void "getProperty resolves the target for the target property"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + + expect: + metaClass.getProperty(proxy, 'target').is(target) + } + + void "getProperty for class/domainClass resolves the target once already initiated"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + metaClass.getProxyTarget() + delegate.getProperty(target, 'class') >> ProxyInstanceTestTarget + + expect: + metaClass.getProperty(proxy, 'class') == ProxyInstanceTestTarget + } + + void "getProperty resolves the target for regular properties"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + delegate.getProperty(target, 'name') >> 'resolved-name' + + expect: + metaClass.getProperty(proxy, 'name') == 'resolved-name' + } + + void "setProperty does not resolve the target when replacing the metaClass"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + MetaClass newMetaClassArg = Mock(MetaClass) + + when: + metaClass.setProperty(proxy, 'metaClass', newMetaClassArg) + + then: + 1 * delegate.setProperty(proxy, 'metaClass', newMetaClassArg) + 0 * session.retrieve(_, _) + } + + void "setProperty does not resolve the target when clearing the metaClass"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + metaClass.setProperty(proxy, 'metaClass', null) + + then: + 1 * delegate.setProperty(proxy, 'metaClass', null) + 0 * session.retrieve(_, _) + } + + void "setProperty resolves the target when setting metaClass to a non-MetaClass value"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + metaClass.setProperty(proxy, 'metaClass', 'not-a-metaclass') + + then: + 1 * session.retrieve(ProxyInstanceTestTarget, 11L) >> target + 1 * delegate.setProperty(target, 'metaClass', 'not-a-metaclass') + } + + void "setProperty resolves the target for regular properties"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + + when: + metaClass.setProperty(proxy, 'name', 'new-name') + + then: + 1 * delegate.setProperty(target, 'name', 'new-name') + } + + void "getAttribute exposes proxy metadata without resolving the target"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + + when: + Object idResult = metaClass.getAttribute(proxy, 'id') + Object initializedResult = metaClass.getAttribute(proxy, 'initialized') + + then: + idResult == 11L + initializedResult == false + 0 * session.retrieve(_, _) + } + + void "getAttribute resolves the target for the target attribute"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + + expect: + metaClass.getAttribute(proxy, 'target').is(target) + } + + void "getAttribute resolves the target for other attributes"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + delegate.getAttribute(target, 'name') >> 'resolved-name' + + expect: + metaClass.getAttribute(proxy, 'name') == 'resolved-name' + } + + void "setAttribute always resolves and delegates to the target"() { + given: + ProxyInstanceMetaClass metaClass = newMetaClass() + session.retrieve(ProxyInstanceTestTarget, 11L) >> target + + when: + metaClass.setAttribute(proxy, 'name', 'new-name') + + then: + 1 * delegate.setAttribute(target, 'name', 'new-name') + } +} + +class ProxyInstanceTestTarget { + Long id + String name +}