DATACMNS-464 - Transaction attribute lookup now considers target class as fallback.

In case of a standalone repository interface declaring CRUD methods manually we didn't inspect the invocation target method for transactional settings. That might be needed if e.g. a save(…) method is declared but not annotated at all.

src/main/java/org/springframework/data/repository/core/support/PersistenceExceptionTranslationRepositoryProxyPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@ package org.springframework.data.repository.core.support;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.dao.support.PersistenceExceptionTranslationInterceptor;
+import org.springframework.data.repository.core.RepositoryInformation;
 import org.springframework.util.Assert;
 
 /**
@@ -47,9 +48,9 @@ public class PersistenceExceptionTranslationRepositoryProxyPostProcessor impleme
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.core.support.RepositoryProxyPostProcessor#postProcess(org.springframework.aop.framework.ProxyFactory)
+	 * @see org.springframework.data.repository.core.support.RepositoryProxyPostProcessor#postProcess(org.springframework.aop.framework.ProxyFactory, org.springframework.data.repository.core.RepositoryInformation)
 	 */
-	public void postProcess(ProxyFactory factory) {
+	public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) {
 		factory.addAdvice(interceptor);
 	}
 }

src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java

@@ -155,7 +155,7 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware {
 		result.setInterfaces(new Class[] { repositoryInterface, Repository.class });
 
 		for (RepositoryProxyPostProcessor processor : postProcessors) {
-			processor.postProcess(result);
+			processor.postProcess(result, information);
 		}
 
 		result.addAdvice(new QueryExecutorMethodInterceptor(information, customImplementation, target));

src/main/java/org/springframework/data/repository/core/support/RepositoryProxyPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2010 the original author or authors.
+ * Copyright 2008-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 package org.springframework.data.repository.core.support;
 
 import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.data.repository.core.RepositoryInformation;
 
 /**
  * Callback interface used during repository proxy creation. Allows manipulating the {@link ProxyFactory} creating the
@@ -28,7 +29,8 @@ public interface RepositoryProxyPostProcessor {
 	/**
 	 * Manipulates the {@link ProxyFactory}, e.g. add further interceptors to it.
 	 * 
-	 * @param factory
+	 * @param factory will never be {@literal null}.
+	 * @param repositoryInformation will never be {@literal null}.
 	 */
-	void postProcess(ProxyFactory factory);
-}
+	void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation);
+}

src/main/java/org/springframework/data/repository/core/support/TransactionalRepositoryProxyPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2013 the original author or authors.
+ * Copyright 2008-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,9 +28,11 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.core.BridgeMethodResolver;
 import org.springframework.dao.support.PersistenceExceptionTranslationInterceptor;
+import org.springframework.data.repository.core.RepositoryInformation;
 import org.springframework.transaction.annotation.Ejb3TransactionAnnotationParser;
 import org.springframework.transaction.annotation.SpringTransactionAnnotationParser;
 import org.springframework.transaction.annotation.TransactionAnnotationParser;
@@ -52,7 +54,8 @@ import org.springframework.util.ObjectUtils;
  */
 class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostProcessor {
 
-	private final TransactionInterceptor transactionInterceptor;
+	private final BeanFactory beanFactory;
+	private final String transactionManagerName;
 
 	/**
 	 * Creates a new {@link TransactionalRepositoryProxyPostProcessor} using the given {@link ListableBeanFactory} and
@@ -66,17 +69,24 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr
 		Assert.notNull(beanFactory);
 		Assert.notNull(transactionManagerName);
 
-		this.transactionInterceptor = new TransactionInterceptor(null, new CustomAnnotationTransactionAttributeSource());
-		this.transactionInterceptor.setTransactionManagerBeanName(transactionManagerName);
-		this.transactionInterceptor.setBeanFactory(beanFactory);
-		this.transactionInterceptor.afterPropertiesSet();
+		this.beanFactory = beanFactory;
+		this.transactionManagerName = transactionManagerName;
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.repository.core.support.RepositoryProxyPostProcessor#postProcess(org.springframework.aop.framework.ProxyFactory)
+	 * @see org.springframework.data.repository.core.support.RepositoryProxyPostProcessor#postProcess(org.springframework.aop.framework.ProxyFactory, org.springframework.data.repository.core.RepositoryInformation)
 	 */
-	public void postProcess(ProxyFactory factory) {
+	public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) {
+
+		CustomAnnotationTransactionAttributeSource transactionAttributeSource = new CustomAnnotationTransactionAttributeSource();
+		transactionAttributeSource.setRepositoryInformation(repositoryInformation);
+
+		TransactionInterceptor transactionInterceptor = new TransactionInterceptor(null, transactionAttributeSource);
+		transactionInterceptor.setTransactionManagerBeanName(transactionManagerName);
+		transactionInterceptor.setBeanFactory(beanFactory);
+		transactionInterceptor.afterPropertiesSet();
+
 		factory.addAdvice(transactionInterceptor);
 	}
 
@@ -248,6 +258,15 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr
 		 */
 		final Map<Object, TransactionAttribute> attributeCache = new ConcurrentHashMap<Object, TransactionAttribute>();
 
+		private RepositoryInformation repositoryInformation;
+
+		/**
+		 * @param repositoryInformation the repositoryInformation to set
+		 */
+		public void setRepositoryInformation(RepositoryInformation repositoryInformation) {
+			this.repositoryInformation = repositoryInformation;
+		}
+
 		/**
 		 * Determine the transaction attribute for this method invocation.
 		 * <p>
@@ -349,8 +368,26 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr
 				return txAtt;
 			}
 
-			// End: Implementation class check block
+			// Fallback to implementation class transaction settings of nothing found
+			// return findTransactionAttribute(method);
+			Method targetClassMethod = repositoryInformation.getTargetClassMethod(method);
+
+			if (targetClassMethod.equals(method)) {
+				return null;
+			}
+
+			txAtt = findTransactionAttribute(targetClassMethod);
+			if (txAtt != null) {
+				return txAtt;
+			}
+
+			txAtt = findTransactionAttribute(targetClassMethod.getDeclaringClass());
+			if (txAtt != null) {
+				return txAtt;
+			}
+
 			return null;
+			// End: Implementation class check block
 		}
 
 		/**

src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -124,6 +124,14 @@ public class DefaultCrudMethodsUnitTests {
 		assertFindAllMethodOn(type, CrudRepository.class.getDeclaredMethod("findAll"));
 	}
 
+	/**
+	 * @see DATACMNS-464
+	 */
+	@Test
+	public void detectsCustomSaveMethod() throws Exception {
+		assertSaveMethodOn(RepositoryWithCustomSave.class, RepositoryWithCustomSave.class.getMethod("save", Domain.class));
+	}
+
 	private static CrudMethods getMethodsFor(Class<?> repositoryInterface) {
 
 		RepositoryMetadata metadata = new DefaultRepositoryMetadata(repositoryInterface);
@@ -179,6 +187,11 @@ public class DefaultCrudMethodsUnitTests {
 
 	interface DomainPagingAndSortingRepository extends PagingAndSortingRepository<Domain, Long> {}
 
+	interface RepositoryWithCustomSave extends Repository<Domain, Serializable> {
+
+		Domain save(Domain domain);
+	}
+
 	interface RepositoryWithCustomSortingAndPagingFindAll extends Repository<Domain, Serializable> {
 
 		Iterable<Domain> findAll(Sort sort);

src/test/java/org/springframework/data/repository/core/support/PersistenceExceptionTranslationRepositoryProxyPostProcessorUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2014 the original author or authors.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -40,10 +40,8 @@ import org.springframework.dao.support.PersistenceExceptionTranslator;
 @RunWith(MockitoJUnitRunner.class)
 public class PersistenceExceptionTranslationRepositoryProxyPostProcessorUnitTests {
 
-	@Mock
-	ListableBeanFactory beanFactory;
-	@Mock
-	ProxyFactory proxyFactory;
+	@Mock ListableBeanFactory beanFactory;
+	@Mock ProxyFactory proxyFactory;
 
 	@Before
 	public void setUp() {
@@ -54,19 +52,25 @@ public class PersistenceExceptionTranslationRepositoryProxyPostProcessorUnitTest
 				beans);
 	}
 
+	/**
+	 * @see DATACMNS-318
+	 */
 	@Test(expected = IllegalArgumentException.class)
 	public void rejectsNullBeanFactory() throws Exception {
 
 		new PersistenceExceptionTranslationRepositoryProxyPostProcessor(null);
 	}
 
+	/**
+	 * @see DATACMNS-318
+	 */
 	@Test
 	public void setsUpBasicInstance() throws Exception {
 
 		RepositoryProxyPostProcessor postProcessor = new PersistenceExceptionTranslationRepositoryProxyPostProcessor(
 				beanFactory);
 
-		postProcessor.postProcess(proxyFactory);
+		postProcessor.postProcess(proxyFactory, null);
 
 		verify(proxyFactory).addAdvice(isA(PersistenceExceptionTranslationInterceptor.class));
 	}

src/test/java/org/springframework/data/repository/core/support/TransactionRepositoryProxyPostProcessorUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2013 the original author or authors.
+ * Copyright 2008-2014 the original author or authors.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -15,12 +15,16 @@
  */
 package org.springframework.data.repository.core.support;
 
+import static org.junit.Assert.*;
 import static org.mockito.Matchers.*;
 import static org.mockito.Mockito.*;
 
+import java.io.Serializable;
+import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -29,6 +33,11 @@ import org.mockito.runners.MockitoJUnitRunner;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.dao.support.PersistenceExceptionTranslator;
+import org.springframework.data.repository.Repository;
+import org.springframework.data.repository.core.RepositoryInformation;
+import org.springframework.data.repository.core.support.TransactionalRepositoryProxyPostProcessor.CustomAnnotationTransactionAttributeSource;
+import org.springframework.transaction.annotation.Transactional;
+import org.springframework.transaction.interceptor.TransactionAttribute;
 import org.springframework.transaction.interceptor.TransactionInterceptor;
 
 /**
@@ -39,10 +48,9 @@ import org.springframework.transaction.interceptor.TransactionInterceptor;
 @RunWith(MockitoJUnitRunner.class)
 public class TransactionRepositoryProxyPostProcessorUnitTests {
 
-	@Mock
-	ListableBeanFactory beanFactory;
-	@Mock
-	ProxyFactory proxyFactory;
+	@Mock ListableBeanFactory beanFactory;
+	@Mock ProxyFactory proxyFactory;
+	@Mock RepositoryInformation repositoryInformation;
 
 	@Before
 	public void setUp() {
@@ -67,8 +75,63 @@ public class TransactionRepositoryProxyPostProcessorUnitTests {
 	public void setsUpBasicInstance() throws Exception {
 
 		RepositoryProxyPostProcessor postProcessor = new TransactionalRepositoryProxyPostProcessor(beanFactory, "txManager");
-		postProcessor.postProcess(proxyFactory);
+		postProcessor.postProcess(proxyFactory, repositoryInformation);
 
 		verify(proxyFactory).addAdvice(isA(TransactionInterceptor.class));
 	}
+
+	/**
+	 * @see DATACMNS-464
+	 */
+	@Test
+	public void fallsBackToTargetMethodTransactionSettings() throws Exception {
+		assertTransactionAttributeFor(SampleImplementation.class);
+	}
+
+	/**
+	 * @see DATACMNS-464
+	 */
+	@Test
+	public void fallsBackToTargetClassTransactionSettings() throws Exception {
+		assertTransactionAttributeFor(SampleImplementationWithClassAnnotation.class);
+	}
+
+	private void assertTransactionAttributeFor(Class<?> implementationClass) throws Exception {
+
+		Method repositorySaveMethod = SampleRepository.class.getMethod("save", Sample.class);
+		Method implementationClassMethod = implementationClass.getMethod("save", Object.class);
+
+		when(repositoryInformation.getTargetClassMethod(repositorySaveMethod)).thenReturn(implementationClassMethod);
+
+		CustomAnnotationTransactionAttributeSource attributeSource = new CustomAnnotationTransactionAttributeSource();
+		attributeSource.setRepositoryInformation(repositoryInformation);
+
+		TransactionAttribute attribute = attributeSource.getTransactionAttribute(repositorySaveMethod,
+				SampleImplementation.class);
+
+		assertThat(attribute, Matchers.is(Matchers.notNullValue()));
+	}
+
+	static class Sample {}
+
+	interface SampleRepository extends Repository<Sample, Serializable> {
+
+		Sample save(Sample object);
+	}
+
+	static class SampleImplementation<T> {
+
+		@Transactional
+		public <S extends T> S save(S object) {
+			return null;
+		}
+	}
+
+	@Transactional
+	static class SampleImplementationWithClassAnnotation<T> {
+
+		public <S extends T> S save(S object) {
+			return null;
+		}
+	}
 }