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.

Author: odrotbohm

src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java

@@ -27,9 +27,9 @@
 
 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 @@ public JpaEntityGraph getEntityGraph() {
 		}
 	}
 
-	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 {}
 	}
 }

src/test/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPopulatingMethodInterceptorUnitTests.java

@@ -23,11 +23,14 @@
 
 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,18 +51,59 @@ 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);
 
 		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();
 	}
 }