Add a test for retry interceptor that shows how to layer the transaction boundary

infrastructure/.springBeans

@@ -6,6 +6,7 @@
 	<configs>
 		<config>src/test/resources/org/springframework/batch/io/file/support/mapping/bean-wrapper.xml</config>
 		<config>src/test/resources/org/springframework/batch/io/sql/data-source-context.xml</config>
+		<config>src/test/resources/org/springframework/batch/retry/aop/retry-transaction-test.xml</config>
 	</configs>
 	<configSets>
 	</configSets>

infrastructure/pom.xml

@@ -23,6 +23,16 @@
 			<groupId>junit</groupId>
 			<artifactId>junit</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>aspectj</groupId>
+			<artifactId>aspectjrt</artifactId>
+			<scope>test</scope>
+		</dependency>
+		<dependency>
+			<groupId>aspectj</groupId>
+			<artifactId>aspectjweaver</artifactId>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>cglib</groupId>
 			<artifactId>cglib-nodep</artifactId>

infrastructure/src/main/java/org/springframework/batch/retry/aop/RetryOperationsInterceptor.java

@@ -29,7 +29,14 @@ import org.springframework.util.Assert;
  * A {@link MethodInterceptor} that can be used to automatically retry calls to
  * a method on a service if it fails. The injected {@link RetryOperations} is
  * used to control the number of retries. By default it will retry a fixed
- * number of times, according to the defaults in {@link RetryTemplate}.
+ * number of times, according to the defaults in {@link RetryTemplate}.<br/>
+ * 
+ * Hint about transaction boundaries. If you want to retry a failed transaction
+ * you need to make sure that the transaction boundary is inside the retry,
+ * otherwise the successful attempt will roll back with the whole transaction.
+ * If the method being intercepted is also transactional, then use the ordering
+ * hints in the advice declarations to ensure that this one is before the
+ * transaction interceptor in the advice chain.
  * 
  * @author Rob Harrop
  * @author Dave Syer
@@ -58,7 +65,8 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
 				 * implementation come along?).
 				 */
 				if (invocation instanceof ProxyMethodInvocation) {
-					return ((ProxyMethodInvocation) invocation).invocableClone().proceed();
+					return ((ProxyMethodInvocation) invocation)
+							.invocableClone().proceed();
 				} else {
 					throw new IllegalStateException(
 							"MethodInvocation of the wrong type detected - this should not happen with Spring AOP, so please raise an issue if you see this exception");

infrastructure/src/test/java/org/springframework/batch/retry/aop/RetryOperationsInterceptorTests.java

@@ -31,6 +31,10 @@ import org.springframework.aop.target.SingletonTargetSource;
 import org.springframework.batch.retry.policy.NeverRetryPolicy;
 import org.springframework.batch.retry.policy.SimpleRetryPolicy;
 import org.springframework.batch.retry.support.RetryTemplate;
+import org.springframework.context.support.ClassPathXmlApplicationContext;
+import org.springframework.transaction.support.TransactionSynchronizationAdapter;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
+import org.springframework.util.ClassUtils;
 
 public class RetryOperationsInterceptorTests extends TestCase {
 
@@ -39,18 +43,25 @@ public class RetryOperationsInterceptorTests extends TestCase {
 	private Service service;
 
 	private ServiceImpl target;
+	
+	private static int count;
+
+	private static int transactionCount;
 
 	protected void setUp() throws Exception {
 		super.setUp();
 		interceptor = new RetryOperationsInterceptor();
 		target = new ServiceImpl();
-		service = (Service) ProxyFactory.getProxy(Service.class, new SingletonTargetSource(target));
+		service = (Service) ProxyFactory.getProxy(Service.class,
+				new SingletonTargetSource(target));
+		count = 0;
+		transactionCount = 0;
 	}
 
 	public void testDefaultInterceptorSunnyDay() throws Exception {
 		((Advised) service).addAdvice(interceptor);
 		service.service();
-		assertEquals(2, target.count);
+		assertEquals(2, count);
 	}
 
 	public void testInterceptorChainWithRetry() throws Exception {
@@ -66,7 +77,7 @@ public class RetryOperationsInterceptorTests extends TestCase {
 		template.setRetryPolicy(new SimpleRetryPolicy(2));
 		interceptor.setRetryTemplate(template);
 		service.service();
-		assertEquals(2, target.count);
+		assertEquals(2, count);
 		assertEquals(2, list.size());
 	}
 
@@ -78,13 +89,26 @@ public class RetryOperationsInterceptorTests extends TestCase {
 		try {
 			service.service();
 			fail("Expected Exception.");
-		}
-		catch (Exception e) {
+		} catch (Exception e) {
 			assertTrue(e.getMessage().startsWith("Not enough calls"));
 		}
-		assertEquals(1, target.count);
+		assertEquals(1, count);
 	}
-	
+
+	public void testOutsideTransaction() throws Exception {
+		ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext(
+				ClassUtils.addResourcePathToPackagePath(getClass(),
+						"retry-transaction-test.xml"));
+		Object object = context.getBean("bean");
+		assertNotNull(object);
+		assertTrue(object instanceof Service);
+		Service bean = (Service) object ;
+		bean.doTansactional();
+		assertEquals(2, count);
+		// Expect 2 separate transactions...
+		assertEquals(2, transactionCount);
+	}
+
 	public void testIllegalMethodInvocationType() throws Throwable {
 		try {
 			interceptor.invoke(new MethodInvocation() {
@@ -116,12 +140,14 @@ public class RetryOperationsInterceptorTests extends TestCase {
 		}
 	}
 
-	private interface Service {
+	public static interface Service {
 		void service() throws Exception;
+		void doTansactional() throws Exception;
 	}
 
-	private static class ServiceImpl implements Service {
-		private int count = 0;
+	public static class ServiceImpl implements Service {
+		
+		private boolean enteredTransaction = false;
 
 		public void service() throws Exception {
 			count++;
@@ -129,5 +155,21 @@ public class RetryOperationsInterceptorTests extends TestCase {
 				throw new Exception("Not enough calls: " + count);
 			}
 		}
+		public void doTansactional() throws Exception {
+			if (TransactionSynchronizationManager.isActualTransactionActive() && !enteredTransaction) {
+				transactionCount++;
+				TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() {
+					public void beforeCompletion() {
+						enteredTransaction = false;
+					}
+				});
+				enteredTransaction  = true;
+			}
+			count++;
+			if (count==1) {
+				throw new RuntimeException("Rollback please");
+			}
+		}
+
 	}
 }

infrastructure/src/test/resources/org/springframework/batch/retry/aop/retry-transaction-test.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	xmlns:aop="http://www.springframework.org/schema/aop"
+	xmlns:tx="http://www.springframework.org/schema/tx"
+	xmlns:p="http://www.springframework.org/schema/p"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:schemaLocation="
+		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd
+		http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop-2.0.xsd
+		http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx-2.0.xsd">
+
+	<aop:config>
+		<aop:pointcut id="transactional"
+			expression="execution(* org.springframework.batch..RetryOperationsInterceptorTests.Service.doTansactional(..))" />
+		<aop:advisor pointcut-ref="transactional"
+			advice-ref="retryAdvice" order="-1"/>
+		<aop:advisor pointcut-ref="transactional" advice-ref="txAdvice" order="0"/>
+	</aop:config>
+
+	<bean id="bean"
+		class="org.springframework.batch.retry.aop.RetryOperationsInterceptorTests$ServiceImpl" />
+
+	<bean id="retryAdvice"
+		class="org.springframework.batch.retry.aop.RetryOperationsInterceptor"/>
+
+	<tx:advice id="txAdvice" transaction-manager="transactionManager">
+		<tx:attributes>
+			<tx:method name="*" />
+		</tx:attributes>
+	</tx:advice>
+
+	<bean id="transactionManager"
+		class="org.springframework.batch.support.transaction.ResourcelessTransactionManager" />
+
+</beans>