From cd2f5c5d754e4b8203a5a3a951f8b0cc5f63ebe7 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 17 Dec 2015 17:25:14 +0100 Subject: [PATCH] DATAJPA-839 - Fixed thread-bound lookups of CrudMethodMetadata. CrudMethodMetadata exposed by the CrudMethodMetadataPostProcessor previously used an AbstractLazyCreationTargetSource to lookup the thread-bound instance. That instance however is cached and never released so that all subsequent calls to it returned the same (and in most cases wrong) instance. We're now implementing TargetSource directly to make sure we obtain a fresh instance on every access of the CrudMethodMetadata proxy. --- .../CrudMethodMetadataPostProcessor.java | 33 +++++++++++-- ...aPopulatingMethodInterceptorUnitTests.java | 48 ++++++++++++++++++- 2 files changed, 75 insertions(+), 6 deletions(-) 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 dbcaf1958..104f3cc48 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 @@ -27,9 +27,9 @@ import javax.persistence.QueryHint; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.springframework.aop.TargetSource; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; -import org.springframework.aop.target.AbstractLazyCreationTargetSource; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.jpa.repository.EntityGraph; @@ -224,17 +224,42 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B } } - private static class ThreadBoundTargetSource extends AbstractLazyCreationTargetSource { + private static class ThreadBoundTargetSource implements TargetSource { /* * (non-Javadoc) - * @see org.springframework.aop.target.AbstractLazyCreationTargetSource#createObject() + * @see org.springframework.aop.TargetSource#getTargetClass() */ @Override - protected Object createObject() throws Exception { + public Class getTargetClass() { + return CrudMethodMetadata.class; + } + + /* + * (non-Javadoc) + * @see org.springframework.aop.TargetSource#isStatic() + */ + @Override + public boolean isStatic() { + return false; + } + + /* + * (non-Javadoc) + * @see org.springframework.aop.TargetSource#getTarget() + */ + @Override + public Object getTarget() throws Exception { MethodInvocation invocation = ExposeInvocationInterceptor.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 {} } } 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 fbcc8d605..def986ad4 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 @@ -23,11 +23,14 @@ import java.lang.reflect.Method; import javax.persistence.LockModeType; +import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.Test; 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.transaction.support.TransactionSynchronizationManager; @@ -48,8 +51,7 @@ public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { @Test public void cleansUpBoundResources() throws Throwable { - Method method = Sample.class.getMethod("someMethod"); - when(invocation.getMethod()).thenReturn(method); + Method method = prepareMethodInvocation("someMethod"); CrudMethodMetadataPopulatingMethodInterceptor interceptor = CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE; interceptor.invoke(invocation); @@ -57,9 +59,51 @@ public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests { assertThat(TransactionSynchronizationManager.getResource(method), is(nullValue())); } + /** + * @see DATAJPA-839 + */ + @Test + public void looksUpCrudMethodMetadataForEveryInvocation() throws Throwable { + + CrudMethodMetadata metadata = new CrudMethodMetadataPostProcessor().getCrudMethodMetadata(); + + expectLockModeType(metadata, LockModeType.OPTIMISTIC).someMethod(); + expectLockModeType(metadata, LockModeType.PESSIMISTIC_READ).someOtherMethod(); + } + + private Method prepareMethodInvocation(String name) throws Throwable { + + Method method = Sample.class.getMethod(name); + ExposeInvocationInterceptor.INSTANCE.invoke(invocation); + when(invocation.getMethod()).thenReturn(method); + + return method; + } + + private static Sample expectLockModeType(final CrudMethodMetadata metadata, final LockModeType type) { + + ProxyFactory factory = new ProxyFactory(new Object()); + factory.addInterface(Sample.class); + factory.addAdvice(ExposeInvocationInterceptor.INSTANCE); + factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE); + factory.addAdvice(new MethodInterceptor() { + + @Override + public Object invoke(MethodInvocation invocation) { + assertThat(metadata.getLockModeType(), is(type)); + return null; + } + }); + + return (Sample) factory.getProxy(); + } + interface Sample { @Lock(LockModeType.OPTIMISTIC) void someMethod(); + + @Lock(LockModeType.PESSIMISTIC_READ) + void someOtherMethod(); } }