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 3efc25a10..06ac632a1 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,6 +15,7 @@ */ package org.springframework.data.jpa.repository.support; +import java.io.Serializable; import java.lang.reflect.Method; import java.util.Collections; import java.util.HashMap; @@ -27,10 +28,15 @@ 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; @@ -46,7 +52,7 @@ import org.springframework.util.ClassUtils; * {@link RepositoryProxyPostProcessor} that sets up interceptors to read metadata information from the invoked method. * This is necessary to allow redeclaration of CRUD methods in repository interfaces and configure locking information * or query hints on them. - * + * * @author Oliver Gierke * @author Thomas Darimont * @author Christoph Strobl @@ -55,7 +61,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B private ClassLoader classLoader = ClassUtils.getDefaultClassLoader(); - /* + /* * (non-Javadoc) * @see org.springframework.beans.factory.BeanClassLoaderAware#setBeanClassLoader(java.lang.ClassLoader) */ @@ -65,12 +71,14 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B } - /* + /* * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryProxyPostProcessor#postProcess(org.springframework.aop.framework.ProxyFactory, org.springframework.data.repository.core.RepositoryInformation) */ @Override public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) { + + factory.addAdvisor(ExposeRepositoryInvocationInterceptor.ADVISOR); factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE); } @@ -91,18 +99,18 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B /** * {@link MethodInterceptor} to build and cache {@link DefaultCrudMethodMetadata} instances for the invoked methods. * Will bind the found information to a {@link TransactionSynchronizationManager} for later lookup. - * + * * @see DefaultCrudMethodMetadata * @author Oliver Gierke * @author Thomas Darimont */ - static enum CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor { + enum CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor { INSTANCE; private final ConcurrentMap metadataCache = new ConcurrentHashMap(); - /* + /* * (non-Javadoc) * @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation) */ @@ -139,7 +147,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B /** * Default implementation of {@link CrudMethodMetadata} that will inspect the backing method for annotations. - * + * * @author Oliver Gierke * @author Thomas Darimont */ @@ -152,7 +160,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B /** * Creates a new {@link DefaultCrudMethodMetadata} for the given {@link Method}. - * + * * @param method must not be {@literal null}. */ public DefaultCrudMethodMetadata(Method method) { @@ -196,7 +204,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B return Collections.unmodifiableMap(queryHints); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jpa.repository.support.CrudMethodMetadata#getLockModeType() */ @@ -205,7 +213,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B return lockModeType; } - /* + /* * (non-Javadoc) * @see org.springframework.data.jpa.repository.support.CrudMethodMetadata#getQueryHints() */ @@ -214,7 +222,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B return queryHints; } - /* + /* * (non-Javadoc) * @see org.springframework.data.jpa.repository.support.CrudMethodMetadata#getEntityGraph() */ @@ -223,7 +231,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B return entityGraph; } - /* + /* * (non-Javadoc) * @see org.springframework.data.jpa.repository.support.CrudMethodMetadata#getMethod() */ @@ -235,7 +243,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B private static class ThreadBoundTargetSource implements TargetSource { - /* + /* * (non-Javadoc) * @see org.springframework.aop.TargetSource#getTargetClass() */ @@ -244,7 +252,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B return CrudMethodMetadata.class; } - /* + /* * (non-Javadoc) * @see org.springframework.aop.TargetSource#isStatic() */ @@ -253,22 +261,113 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B return false; } - /* + /* * (non-Javadoc) * @see org.springframework.aop.TargetSource#getTarget() */ @Override public Object getTarget() throws Exception { - MethodInvocation invocation = ExposeInvocationInterceptor.currentInvocation(); + MethodInvocation invocation = ExposeRepositoryInvocationInterceptor.currentInvocation(); return TransactionSynchronizationManager.getResource(invocation.getMethod()); } - /* + /* * (non-Javadoc) * @see org.springframework.aop.TargetSource#releaseTarget(java.lang.Object) */ @Override public void releaseTarget(Object target) throws Exception {} } + + /** + * 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 1.11.13 + * @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 08d618492..b84528c26 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 @@ -30,15 +30,16 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.aop.framework.ProxyFactory; -import org.springframework.aop.interceptor.ExposeInvocationInterceptor; 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.transaction.support.TransactionSynchronizationManager; /** * Unit tests for {@link CrudMethodMetadataPopulatingMethodInterceptor}. * * @author Oliver Gierke + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { @@ -56,8 +57,8 @@ public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { assertThat(TransactionSynchronizationManager.getResource(method), is(nullValue())); } - @Test // DATAJPA-839 - public void looksUpCrudMethodMetadataForEveryInvocation() throws Throwable { + @Test // DATAJPA-839, DATAJPA-1368 + public void looksUpCrudMethodMetadataForEveryInvocation() { CrudMethodMetadata metadata = new CrudMethodMetadataPostProcessor().getCrudMethodMetadata(); @@ -68,7 +69,7 @@ public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { private Method prepareMethodInvocation(String name) throws Throwable { Method method = Sample.class.getMethod(name); - ExposeInvocationInterceptor.INSTANCE.invoke(invocation); + ExposeRepositoryInvocationInterceptor.INSTANCE.invoke(invocation); when(invocation.getMethod()).thenReturn(method); return method; @@ -78,7 +79,7 @@ public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { ProxyFactory factory = new ProxyFactory(new Object()); factory.addInterface(Sample.class); - factory.addAdvice(ExposeInvocationInterceptor.INSTANCE); + factory.addAdvice(ExposeRepositoryInvocationInterceptor.INSTANCE); factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE); factory.addAdvice(new MethodInterceptor() {