TransactionAttributeSourcePointcut pointcut skips target classes with TransactionalProxy marker (e.g. Spring Data proxies)

This 4.2 commit also makes TransactionProxyFactoryBean expose the TransactionalProxy marker and refines SpringProxy marker handling.

Issue: SPR-13109

spring-aop/src/main/java/org/springframework/aop/framework/AbstractSingletonProxyFactoryBean.java

@@ -173,6 +173,8 @@ public abstract class AbstractSingletonProxyFactoryBean extends ProxyConfig
 					ClassUtils.getAllInterfacesForClass(targetSource.getTargetClass(), this.proxyClassLoader));
 		}
 
+		postProcessProxyFactory(proxyFactory);
+
 		this.proxy = proxyFactory.getProxy(this.proxyClassLoader);
 	}
 
@@ -191,6 +193,15 @@ public abstract class AbstractSingletonProxyFactoryBean extends ProxyConfig
 		}
 	}
 
+	/**
+	 * A hook for subclasses to post-process the {@link ProxyFactory}
+	 * before creating the proxy instance with it.
+	 * @param proxyFactory the AOP ProxyFactory about to be used
+	 * @since 4.2
+	 */
+	protected void postProcessProxyFactory(ProxyFactory proxyFactory) {
+	}
+
 
 	@Override
 	public Object getObject() {

spring-aop/src/main/java/org/springframework/aop/framework/DefaultAopProxyFactory.java

@@ -70,8 +70,8 @@ public class DefaultAopProxyFactory implements AopProxyFactory, Serializable {
 	 * (or no proxy interfaces specified at all).
 	 */
 	private boolean hasNoUserSuppliedProxyInterfaces(AdvisedSupport config) {
-		Class<?>[] interfaces = config.getProxiedInterfaces();
-		return (interfaces.length == 0 || (interfaces.length == 1 && SpringProxy.class == interfaces[0]));
+		Class<?>[] ifcs = config.getProxiedInterfaces();
+		return (ifcs.length == 0 || (ifcs.length == 1 && SpringProxy.class.isAssignableFrom(ifcs[0])));
 	}
 
 }

spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -34,6 +34,9 @@ abstract class TransactionAttributeSourcePointcut extends StaticMethodMatcherPoi
 
 	@Override
 	public boolean matches(Method method, Class<?> targetClass) {
+		if (TransactionalProxy.class.isAssignableFrom(targetClass)) {
+			return false;
+		}
 		TransactionAttributeSource tas = getTransactionAttributeSource();
 		return (tas == null || tas.getTransactionAttribute(method, targetClass) != null);
 	}

spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionProxyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -20,6 +20,7 @@ import java.util.Properties;
 
 import org.springframework.aop.Pointcut;
 import org.springframework.aop.framework.AbstractSingletonProxyFactoryBean;
+import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
@@ -199,4 +200,13 @@ public class TransactionProxyFactoryBean extends AbstractSingletonProxyFactoryBe
 		}
 	}
 
+	/**
+	 * As of 4.2, this method adds {@link TransactionalProxy} to the set of
+	 * proxy interfaces in order to avoid re-processing of transaction metadata.
+	 */
+	@Override
+	protected void postProcessProxyFactory(ProxyFactory proxyFactory) {
+		proxyFactory.addInterface(TransactionalProxy.class);
+	}
+
 }

spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionalProxy.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2002-2015 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 the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.transaction.interceptor;
+
+import org.springframework.aop.SpringProxy;
+
+/**
+ * A marker interface for manually created transactional proxies.
+ *
+ * <p>{@link TransactionAttributeSourcePointcut} will ignore such existing
+ * transactional proxies during AOP auto-proxying and therefore avoid
+ * re-processing transaction metadata on them.
+ *
+ * @author Juergen Hoeller
+ * @since 4.1.7
+ */
+public interface TransactionalProxy extends SpringProxy {
+
+}

spring-tx/src/test/java/org/springframework/transaction/interceptor/BeanFactoryTransactionTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -20,9 +20,10 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.util.Map;
 
-import junit.framework.TestCase;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
+import org.junit.Before;
+import org.junit.Test;
 
 import org.springframework.aop.support.AopUtils;
 import org.springframework.aop.support.StaticMethodMatcherPointcut;
@@ -40,44 +41,56 @@ import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.TransactionException;
 import org.springframework.transaction.TransactionStatus;
 
+import static org.junit.Assert.*;
 import static org.mockito.BDDMockito.*;
 
+
 /**
  * Test cases for AOP transaction management.
  *
  * @author Rod Johnson
+ * @author Juergen Hoeller
  * @since 23.04.2003
  */
-public class BeanFactoryTransactionTests extends TestCase {
+public class BeanFactoryTransactionTests {
 
 	private DefaultListableBeanFactory factory;
 
-	@Override
+
+	@Before
 	public void setUp() {
 		this.factory = new DefaultListableBeanFactory();
 		new XmlBeanDefinitionReader(this.factory).loadBeanDefinitions(
 				new ClassPathResource("transactionalBeanFactory.xml", getClass()));
 	}
 
+
+	@Test
 	public void testGetsAreNotTransactionalWithProxyFactory1() throws NoSuchMethodException {
 		ITestBean testBean = (ITestBean) factory.getBean("proxyFactory1");
 		assertTrue("testBean is a dynamic proxy", Proxy.isProxyClass(testBean.getClass()));
+		assertFalse(testBean instanceof TransactionalProxy);
 		doTestGetsAreNotTransactional(testBean);
 	}
 
+	@Test
 	public void testGetsAreNotTransactionalWithProxyFactory2DynamicProxy() throws NoSuchMethodException {
 		this.factory.preInstantiateSingletons();
 		ITestBean testBean = (ITestBean) factory.getBean("proxyFactory2DynamicProxy");
 		assertTrue("testBean is a dynamic proxy", Proxy.isProxyClass(testBean.getClass()));
+		assertTrue(testBean instanceof TransactionalProxy);
 		doTestGetsAreNotTransactional(testBean);
 	}
 
+	@Test
 	public void testGetsAreNotTransactionalWithProxyFactory2Cglib() throws NoSuchMethodException {
 		ITestBean testBean = (ITestBean) factory.getBean("proxyFactory2Cglib");
 		assertTrue("testBean is CGLIB advised", AopUtils.isCglibProxy(testBean));
+		assertTrue(testBean instanceof TransactionalProxy);
 		doTestGetsAreNotTransactional(testBean);
 	}
 
+	@Test
 	public void testProxyFactory2Lazy() throws NoSuchMethodException {
 		ITestBean testBean = (ITestBean) factory.getBean("proxyFactory2Lazy");
 		assertFalse(factory.containsSingleton("target"));
@@ -85,9 +98,11 @@ public class BeanFactoryTransactionTests extends TestCase {
 		assertTrue(factory.containsSingleton("target"));
 	}
 
+	@Test
 	public void testCglibTransactionProxyImplementsNoInterfaces() throws NoSuchMethodException {
 		ImplementsNoInterfaces ini = (ImplementsNoInterfaces) factory.getBean("cglibNoInterfaces");
 		assertTrue("testBean is CGLIB advised", AopUtils.isCglibProxy(ini));
+		assertTrue(ini instanceof TransactionalProxy);
 		String newName = "Gordon";
 
 		// Install facade
@@ -99,9 +114,11 @@ public class BeanFactoryTransactionTests extends TestCase {
 		assertEquals(2, ptm.commits);
 	}
 
+	@Test
 	public void testGetsAreNotTransactionalWithProxyFactory3() throws NoSuchMethodException {
 		ITestBean testBean = (ITestBean) factory.getBean("proxyFactory3");
 		assertTrue("testBean is a full proxy", testBean instanceof DerivedTestBean);
+		assertTrue(testBean instanceof TransactionalProxy);
 		InvocationCounterPointcut txnCounter = (InvocationCounterPointcut) factory.getBean("txnInvocationCounterPointcut");
 		InvocationCounterInterceptor preCounter = (InvocationCounterInterceptor) factory.getBean("preInvocationCounterInterceptor");
 		InvocationCounterInterceptor postCounter = (InvocationCounterInterceptor) factory.getBean("postInvocationCounterInterceptor");
@@ -158,6 +175,7 @@ public class BeanFactoryTransactionTests extends TestCase {
 		assertTrue(testBean.getAge() == age);
 	}
 
+	@Test
 	public void testGetBeansOfTypeWithAbstract() {
 		Map<String, ITestBean> beansOfType = factory.getBeansOfType(ITestBean.class, true, true);
 		assertNotNull(beansOfType);
@@ -166,6 +184,7 @@ public class BeanFactoryTransactionTests extends TestCase {
 	/**
 	 * Check that we fail gracefully if the user doesn't set any transaction attributes.
 	 */
+	@Test
 	public void testNoTransactionAttributeSource() {
 		try {
 			DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
@@ -181,6 +200,7 @@ public class BeanFactoryTransactionTests extends TestCase {
 	/**
 	 * Test that we can set the target to a dynamic TargetSource.
 	 */
+	@Test
 	public void testDynamicTargetSource() throws NoSuchMethodException {
 		// Install facade
 		CallCountingTransactionManager txMan = new CallCountingTransactionManager();