RESOLVED - BATCH-994: BackOffPolicy is not applied for exceptions that cause rollback

backOff before deciding whether to rethrow

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java

@@ -236,7 +236,18 @@ public class RetryTemplate implements RetryOperations {
 					doOnErrorInterceptors(retryCallback, context, e);
 
 					registerThrowable(retryPolicy, state, context, e);
-
+					
+					try {
+						backOffPolicy.backOff(backOffContext);
+					}
+					catch (BackOffInterruptedException ex) {
+						lastException = e;
+						// back off was prevented by another thread - fail the
+						// retry
+						logger.debug("Abort retry because interrupted: count=" + context.getRetryCount());
+						throw ex;
+					}
+					
 					if (shouldRethrow(retryPolicy, context, state)) {
 						logger.debug("Rethrow in retry for policy: count=" + context.getRetryCount());
 						throw e;
@@ -244,17 +255,6 @@ public class RetryTemplate implements RetryOperations {
 
 				}
 
-				try {
-					backOffPolicy.backOff(backOffContext);
-				}
-				catch (BackOffInterruptedException e) {
-					lastException = e;
-					// back off was prevented by another thread - fail the
-					// retry
-					logger.debug("Abort retry because interrupted: count=" + context.getRetryCount());
-					throw e;
-				}
-
 				/*
 				 * A stateful attempt that can retry should have rethrown the
 				 * exception by now - i.e. we shouldn't get this far for a

spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/RetryTemplateTests.java

@@ -22,6 +22,8 @@ import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.fail;
 
+import static org.easymock.EasyMock.*;
+
 import java.util.Collections;
 
 import org.junit.Test;
@@ -129,7 +131,7 @@ public class RetryTemplateTests {
 		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(attempts));
 		BinaryExceptionClassifier classifier = new BinaryExceptionClassifier(Collections
 				.<Class<? extends Throwable>> singleton(IllegalArgumentException.class), false);
-		retryTemplate.execute(callback, new DefaultRetryState("foo",classifier));
+		retryTemplate.execute(callback, new DefaultRetryState("foo", classifier));
 		assertEquals(attempts, callback.attempts);
 	}
 
@@ -256,6 +258,50 @@ public class RetryTemplateTests {
 		}
 	}
 
+	/**
+	 * {@link BackOffPolicy} should apply also for exceptions that are
+	 * re-thrown.
+	 */
+	@Test
+	public void testBackOffForRethrownException() throws Exception {
+		
+		RetryTemplate tested = new RetryTemplate();
+		tested.setRetryPolicy(new SimpleRetryPolicy(1));
+
+		BackOffPolicy bop = createStrictMock(BackOffPolicy.class);
+		BackOffContext backOffContext = new BackOffContext() {
+		};
+		tested.setBackOffPolicy(bop);
+
+		expect(bop.start(isA(RetryContext.class))).andReturn(backOffContext);
+		bop.backOff(backOffContext);
+		expectLastCall().once();
+		replay(bop);
+
+		try {
+			tested.execute(new RetryCallback<Object>() {
+
+				public Object doWithRetry(RetryContext context) throws Exception {
+					throw new Exception("maybe next time!");
+				}
+
+			}, null, new DefaultRetryState(tested) {
+
+				@Override
+				public boolean rollbackFor(Exception exception) {
+					return true;
+				}
+
+			});
+			fail();
+		}
+		catch (Exception expected) {
+			assertEquals("maybe next time!", expected.getMessage());
+		}
+		
+		verify(bop);
+	}
+
 	private static class MockRetryCallback implements RetryCallback<Object> {
 
 		private int attempts;