diff --git a/infrastructure/.springBeans b/infrastructure/.springBeans index bc8aa601a..da47de4bd 100644 --- a/infrastructure/.springBeans +++ b/infrastructure/.springBeans @@ -6,6 +6,7 @@ src/test/resources/org/springframework/batch/io/file/support/mapping/bean-wrapper.xml src/test/resources/org/springframework/batch/io/sql/data-source-context.xml + src/test/resources/org/springframework/batch/retry/aop/retry-transaction-test.xml diff --git a/infrastructure/pom.xml b/infrastructure/pom.xml index 5453fe0ed..42eda75ea 100644 --- a/infrastructure/pom.xml +++ b/infrastructure/pom.xml @@ -23,6 +23,16 @@ junit junit + + aspectj + aspectjrt + test + + + aspectj + aspectjweaver + test + cglib cglib-nodep diff --git a/infrastructure/src/main/java/org/springframework/batch/retry/aop/RetryOperationsInterceptor.java b/infrastructure/src/main/java/org/springframework/batch/retry/aop/RetryOperationsInterceptor.java index e86b992df..6e52aea58 100644 --- a/infrastructure/src/main/java/org/springframework/batch/retry/aop/RetryOperationsInterceptor.java +++ b/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}.
+ * + * 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"); diff --git a/infrastructure/src/test/java/org/springframework/batch/retry/aop/RetryOperationsInterceptorTests.java b/infrastructure/src/test/java/org/springframework/batch/retry/aop/RetryOperationsInterceptorTests.java index 2fef895d5..0c303873a 100644 --- a/infrastructure/src/test/java/org/springframework/batch/retry/aop/RetryOperationsInterceptorTests.java +++ b/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"); + } + } + } } diff --git a/infrastructure/src/test/resources/org/springframework/batch/retry/aop/retry-transaction-test.xml b/infrastructure/src/test/resources/org/springframework/batch/retry/aop/retry-transaction-test.xml new file mode 100644 index 000000000..01f9f7733 --- /dev/null +++ b/infrastructure/src/test/resources/org/springframework/batch/retry/aop/retry-transaction-test.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + +