diff --git a/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java b/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java index e3953fa1c..281208be2 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java @@ -15,12 +15,13 @@ */ package org.springframework.data.jpa.repository.support; -import java.io.Serializable; import java.lang.reflect.Method; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Predicate; @@ -30,14 +31,11 @@ import javax.persistence.QueryHint; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -import org.springframework.aop.Advisor; + import org.springframework.aop.TargetSource; import org.springframework.aop.framework.ProxyFactory; -import org.springframework.aop.interceptor.ExposeInvocationInterceptor; -import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.core.NamedThreadLocal; -import org.springframework.core.PriorityOrdered; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.jpa.repository.EntityGraph; @@ -49,6 +47,7 @@ import org.springframework.lang.Nullable; import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; /** * {@link RepositoryProxyPostProcessor} that sets up interceptors to read metadata information from the invoked method. @@ -80,9 +79,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B */ @Override public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) { - - factory.addAdvisor(ExposeRepositoryInvocationInterceptor.ADVISOR); - factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE); + factory.addAdvice(new CrudMethodMetadataPopulatingMethodInterceptor(repositoryInformation)); } /** @@ -107,11 +104,37 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B * @author Oliver Gierke * @author Thomas Darimont */ - enum CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor { + static class CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor { - INSTANCE; + private static final ThreadLocal currentInvocation = new NamedThreadLocal<>( + "Current AOP method invocation"); private final ConcurrentMap metadataCache = new ConcurrentHashMap<>(); + private final Set implementations = new HashSet<>(); + + CrudMethodMetadataPopulatingMethodInterceptor(RepositoryInformation repositoryInformation) { + + ReflectionUtils.doWithMethods(repositoryInformation.getRepositoryInterface(), implementations::add, + method -> !repositoryInformation.isQueryMethod(method)); + } + + /** + * Return the AOP Alliance {@link MethodInvocation} object associated with the current invocation. + * + * @return the invocation object associated with the current invocation. + * @throws IllegalStateException if there is no AOP invocation in progress, or if the + * {@link CrudMethodMetadataPopulatingMethodInterceptor} was not added to this interceptor chain. + */ + static MethodInvocation currentInvocation() throws IllegalStateException { + + MethodInvocation mi = currentInvocation.get(); + + if (mi == null) + throw new IllegalStateException( + "No MethodInvocation found: Check that an AOP invocation is in progress, and that the " + + "CrudMethodMetadataPopulatingMethodInterceptor is upfront in the interceptor chain."); + return mi; + } /* * (non-Javadoc) @@ -121,30 +144,43 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B public Object invoke(MethodInvocation invocation) throws Throwable { Method method = invocation.getMethod(); - CrudMethodMetadata metadata = (CrudMethodMetadata) TransactionSynchronizationManager.getResource(method); - if (metadata != null) { + if (!implementations.contains(method)) { return invocation.proceed(); } - CrudMethodMetadata methodMetadata = metadataCache.get(method); - - if (methodMetadata == null) { - - methodMetadata = new DefaultCrudMethodMetadata(method); - CrudMethodMetadata tmp = metadataCache.putIfAbsent(method, methodMetadata); - - if (tmp != null) { - methodMetadata = tmp; - } - } - - TransactionSynchronizationManager.bindResource(method, methodMetadata); + MethodInvocation oldInvocation = currentInvocation.get(); + currentInvocation.set(invocation); try { - return invocation.proceed(); + + CrudMethodMetadata metadata = (CrudMethodMetadata) TransactionSynchronizationManager.getResource(method); + + if (metadata != null) { + return invocation.proceed(); + } + + CrudMethodMetadata methodMetadata = metadataCache.get(method); + + if (methodMetadata == null) { + + methodMetadata = new DefaultCrudMethodMetadata(method); + CrudMethodMetadata tmp = metadataCache.putIfAbsent(method, methodMetadata); + + if (tmp != null) { + methodMetadata = tmp; + } + } + + TransactionSynchronizationManager.bindResource(method, methodMetadata); + + try { + return invocation.proceed(); + } finally { + TransactionSynchronizationManager.unbindResource(method); + } } finally { - TransactionSynchronizationManager.unbindResource(method); + currentInvocation.set(oldInvocation); } } } @@ -285,7 +321,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B @Override public Object getTarget() { - MethodInvocation invocation = ExposeRepositoryInvocationInterceptor.currentInvocation(); + MethodInvocation invocation = CrudMethodMetadataPopulatingMethodInterceptor.currentInvocation(); return TransactionSynchronizationManager.getResource(invocation.getMethod()); } @@ -296,95 +332,4 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B @Override public void releaseTarget(Object target) {} } - - /** - * Own copy of {@link ExposeInvocationInterceptor} scoped to repository proxy method usage to not conflict with - * {@link ExposeInvocationInterceptor} that might expose nested proxy calls to e.g. proxied transaction managers. - * - * @author Mark Paluch - * @since 2.2.0 - * @see ExposeInvocationInterceptor - */ - @SuppressWarnings("serial") - static class ExposeRepositoryInvocationInterceptor implements MethodInterceptor, PriorityOrdered, Serializable { - - /** - * Singleton instance of this class - */ - static final ExposeRepositoryInvocationInterceptor INSTANCE = new ExposeRepositoryInvocationInterceptor(); - - private static final ThreadLocal invocation = new NamedThreadLocal<>( - "Current AOP method invocation"); - - /** - * Singleton advisor for this class. Use in preference to {@code INSTANCE} when using Spring AOP, as it prevents the - * need to create a new Advisor to wrap the instance. - */ - static final Advisor ADVISOR = new DefaultPointcutAdvisor(INSTANCE) { - @Override - public String toString() { - return ExposeRepositoryInvocationInterceptor.class.getName() + ".ADVISOR"; - } - }; - - /** - * Ensures that only the canonical instance can be created. - */ - private ExposeRepositoryInvocationInterceptor() {} - - /** - * Return the AOP Alliance {@link MethodInvocation} object associated with the current invocation. - * - * @return the invocation object associated with the current invocation. - * @throws IllegalStateException if there is no AOP invocation in progress, or if the - * {@link ExposeRepositoryInvocationInterceptor} was not added to this interceptor chain. - */ - static MethodInvocation currentInvocation() throws IllegalStateException { - - MethodInvocation mi = invocation.get(); - - if (mi == null) - throw new IllegalStateException( - "No MethodInvocation found: Check that an AOP invocation is in progress, and that the " - + "ExposeRepositoryInvocationInterceptor is upfront in the interceptor chain. Specifically, note that " - + "advices with order HIGHEST_PRECEDENCE will execute before ExposeRepositoryMethodInvocationInterceptor!"); - return mi; - } - - /* - * (non-Javadoc) - * @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation) - */ - @Override - public Object invoke(MethodInvocation mi) throws Throwable { - - MethodInvocation oldInvocation = invocation.get(); - invocation.set(mi); - - try { - return mi.proceed(); - } finally { - invocation.set(oldInvocation); - } - } - - /* - * (non-Javadoc) - * @see org.springframework.core.Ordered#getOrder() - */ - @Override - public int getOrder() { - return PriorityOrdered.HIGHEST_PRECEDENCE + 1; - } - - /** - * Required to support serialization. Replaces with canonical instance on deserialization, protecting Singleton - * pattern. - *

- * Alternative to overriding the {@code equals} method. - */ - private Object readResolve() { - return INSTANCE; - } - } } diff --git a/src/test/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPopulatingMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPopulatingMethodInterceptorUnitTests.java index 49b0c93e3..e4aba5671 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPopulatingMethodInterceptorUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPopulatingMethodInterceptorUnitTests.java @@ -31,7 +31,7 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.aop.framework.ProxyFactory; import org.springframework.data.jpa.repository.Lock; import org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor.CrudMethodMetadataPopulatingMethodInterceptor; -import org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor.ExposeRepositoryInvocationInterceptor; +import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.transaction.support.TransactionSynchronizationManager; /** @@ -45,13 +45,14 @@ import org.springframework.transaction.support.TransactionSynchronizationManager public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { @Mock MethodInvocation invocation; + @Mock RepositoryInformation information; - private static Sample expectLockModeType(final CrudMethodMetadata metadata, final LockModeType type) { + private static Sample expectLockModeType(CrudMethodMetadata metadata, RepositoryInformation information, + LockModeType type) { ProxyFactory factory = new ProxyFactory(new Object()); factory.addInterface(Sample.class); - factory.addAdvice(ExposeRepositoryInvocationInterceptor.INSTANCE); - factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE); + factory.addAdvice(new CrudMethodMetadataPopulatingMethodInterceptor(information)); factory.addAdvice(new MethodInterceptor() { @Override @@ -65,29 +66,35 @@ public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { } @Test // DATAJPA-268 + @SuppressWarnings("unchecked") public void cleansUpBoundResources() throws Throwable { Method method = prepareMethodInvocation("someMethod"); + when(information.isQueryMethod(method)).thenReturn(false); + when(information.getRepositoryInterface()).thenReturn((Class) Sample.class); - CrudMethodMetadataPopulatingMethodInterceptor interceptor = CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE; + CrudMethodMetadataPopulatingMethodInterceptor interceptor = new CrudMethodMetadataPopulatingMethodInterceptor( + information); interceptor.invoke(invocation); assertThat(TransactionSynchronizationManager.getResource(method)).isNull(); } @Test // DATAJPA-839, DATAJPA-1368 + @SuppressWarnings("unchecked") public void looksUpCrudMethodMetadataForEveryInvocation() { CrudMethodMetadata metadata = new CrudMethodMetadataPostProcessor().getCrudMethodMetadata(); + when(information.isQueryMethod(any())).thenReturn(false); + when(information.getRepositoryInterface()).thenReturn((Class) Sample.class); - expectLockModeType(metadata, LockModeType.OPTIMISTIC).someMethod(); - expectLockModeType(metadata, LockModeType.PESSIMISTIC_READ).someOtherMethod(); + expectLockModeType(metadata, information, LockModeType.OPTIMISTIC).someMethod(); + expectLockModeType(metadata, information, LockModeType.PESSIMISTIC_READ).someOtherMethod(); } private Method prepareMethodInvocation(String name) throws Throwable { Method method = Sample.class.getMethod(name); - ExposeRepositoryInvocationInterceptor.INSTANCE.invoke(invocation); when(invocation.getMethod()).thenReturn(method); return method;