DATAJPA-1575 - Limit CrudMethodMetadataPostProcessor to implementation methods.

Also, merge CrudMethodMetadataPostProcessor with ExposeRepositoryInvocationInterceptor as exposing the MethodInvocation is only required for CrudMethodMetadata.
This commit is contained in:
Mark Paluch
2019-07-23 10:58:15 +02:00
parent dfc79193b4
commit 6e2032e7b6
2 changed files with 79 additions and 127 deletions

View File

@@ -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<MethodInvocation> currentInvocation = new NamedThreadLocal<>(
"Current AOP method invocation");
private final ConcurrentMap<Method, CrudMethodMetadata> metadataCache = new ConcurrentHashMap<>();
private final Set<Method> 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<MethodInvocation> 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.
* <p>
* Alternative to overriding the {@code equals} method.
*/
private Object readResolve() {
return INSTANCE;
}
}
}

View File

@@ -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;