diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/callback/RecoveryRetryCallback.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/callback/RecoveryRetryCallback.java
index 1d8f3ba85..4985d778f 100644
--- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/callback/RecoveryRetryCallback.java
+++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/callback/RecoveryRetryCallback.java
@@ -16,8 +16,6 @@
package org.springframework.batch.retry.callback;
-import org.springframework.batch.item.ItemRecoverer;
-import org.springframework.batch.item.ItemWriter;
import org.springframework.batch.retry.RecoveryCallback;
import org.springframework.batch.retry.RetryCallback;
import org.springframework.batch.retry.RetryContext;
@@ -26,8 +24,8 @@ import org.springframework.batch.retry.policy.RecoveryCallbackRetryPolicy;
/**
* A {@link RetryCallback} that knows about and caches an item, and attempts to
- * process it using an {@link ItemWriter}. Used by the
- * {@link RecoveryCallbackRetryPolicy} to enable external retry of the item
+ * process it using a delegate {@link RetryCallback}. Used by the
+ * {@link RecoveryCallbackRetryPolicy} to enable stateful retry of the
* processing.
*
* @author Dave Syer
@@ -52,12 +50,12 @@ public class RecoveryRetryCallback implements RetryCallback {
* Constructor with mandatory properties. The key will be set to the item.
*
* @param item the item to process
- * @param writer the writer to use to process it
+ * @param callback the delegate to use to process it
*/
- public RecoveryRetryCallback(Object item, RetryCallback writer) {
+ public RecoveryRetryCallback(Object item, RetryCallback callback) {
super();
this.item = item;
- this.callback = writer;
+ this.callback = callback;
this.key = item;
}
@@ -65,12 +63,12 @@ public class RecoveryRetryCallback implements RetryCallback {
* Constructor with mandatory properties.
*
* @param item the item to process
- * @param writer the writer to use to process it
+ * @param callback the delegate to use to process it
*/
- public RecoveryRetryCallback(Object item, RetryCallback writer, Object key) {
+ public RecoveryRetryCallback(Object item, RetryCallback callback, Object key) {
super();
this.item = item;
- this.callback = writer;
+ this.callback = callback;
this.key = key;
}
@@ -84,10 +82,7 @@ public class RecoveryRetryCallback implements RetryCallback {
}
/**
- * Setter for injecting optional recovery handler. If it is not injected but
- * the reader or writer implement {@link ItemRecoverer}, one of those will
- * be used instead (preferring the reader to the writer if both would be
- * appropriate).
+ * Setter for injecting optional recovery handler.
*
* @param recoverer
*/
@@ -126,9 +121,9 @@ public class RecoveryRetryCallback implements RetryCallback {
}
/**
- * Accessor for the {@link ItemRecoverer}.
+ * Accessor for the {@link RecoveryCallback}.
*
- * @return the {@link ItemRecoverer}.
+ * @return the {@link RecoveryCallback}.
*/
public RecoveryCallback getRecoveryCallback() {
return recoverer;
diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptor.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptor.java
index 245c306cc..f2fac4317 100644
--- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptor.java
+++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptor.java
@@ -23,17 +23,37 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.batch.item.ItemKeyGenerator;
import org.springframework.batch.item.ItemRecoverer;
import org.springframework.batch.item.NewItemIdentifier;
+import org.springframework.batch.retry.ExhaustedRetryException;
import org.springframework.batch.retry.RecoveryCallback;
import org.springframework.batch.retry.RetryCallback;
import org.springframework.batch.retry.RetryContext;
+import org.springframework.batch.retry.RetryOperations;
import org.springframework.batch.retry.RetryPolicy;
import org.springframework.batch.retry.callback.RecoveryRetryCallback;
-import org.springframework.batch.retry.policy.RecoveryCallbackRetryPolicy;
import org.springframework.batch.retry.policy.NeverRetryPolicy;
+import org.springframework.batch.retry.policy.RecoveryCallbackRetryPolicy;
import org.springframework.batch.retry.support.RetryTemplate;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
+/**
+ * A {@link MethodInterceptor} that can be used to automatically retry calls to
+ * a method on a service if it fails. The argument to the service method is
+ * treated as an item to be remembered in case the call fails. So the retry
+ * operation is stateful, and the item that failed is tracked by its unique key
+ * (via {@link ItemKeyGenerator}) until the retry is exhausted, at which point
+ * the {@link ItemRecoverer} is called.
+ *
+ * The main use case for this is where the service is transactional, via a
+ * transaction interceptor on the interceptor chain. In this case the retry (and
+ * recovery on exhausted) always happens in a new transaction.
+ *
+ * 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}.
+ *
+ * @author Dave Syer
+ */
public class StatefulRetryOperationsInterceptor implements MethodInterceptor {
private transient Log logger = LogFactory.getLog(getClass());
@@ -58,7 +78,10 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor {
* Public setter for the {@link ItemRecoverer} to use if the retry is
* exhausted. The recoverer should be able to return an object of the same
* type as the target object because its return value will be used to return
- * to the caller in the case of a recovery.
+ * to the caller in the case of a recovery.
+ *
+ * If no recoverer is set then an exhausted retry will result in an
+ * {@link ExhaustedRetryException}.
*
* @param recoverer the {@link ItemRecoverer} to set
*/
@@ -71,7 +94,9 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor {
}
/**
- * Public setter for the retryPolicy.
+ * Public setter for the retryPolicy. The value provided should be a normal
+ * stateless policy, which is wrapped into a stateful policy inside this
+ * method.
* @param retryPolicy the retryPolicy to set
*/
public void setRetryPolicy(RetryPolicy retryPolicy) {
@@ -94,10 +119,14 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor {
* re-thrown. The only time it is not re-thrown is when retry is exhausted
* and the recovery path is taken (though the {@link ItemRecoverer} provided
* if there is one). In that case the value returned from the method
- * invocation will be null, or if primitive then "0" (e.g. Boolean.FALSE, 0L
- * etc.).
+ * invocation will be the value returned by the recoverer (so the return
+ * type for that should be the same as the intercepted method).
*
* @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation)
+ * @see ItemRecoverer#recover(Object, Throwable)
+ *
+ * @throws ExhaustedRetryException if the retry is exhausted and no
+ * {@link ItemRecoverer} is provided.
*/
public Object invoke(final MethodInvocation invocation) throws Throwable {
@@ -172,11 +201,7 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor {
if (recoverer != null) {
return recoverer.recover(item, context.getLastThrowable());
}
- // TODO: This sucks big time because the method invocation almost
- // certainly does not return an object of this type. It would be
- // better to return null (but then method invocations that return
- // primitive values would barf).
- return item;
+ throw new ExhaustedRetryException("Retry was exhausted but there was no recovery path.");
}
}
diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptorTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptorTests.java
index 7fe6b3f32..7a7fa3cd5 100644
--- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptorTests.java
+++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptorTests.java
@@ -28,6 +28,7 @@ import org.springframework.aop.framework.Advised;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.aop.target.SingletonTargetSource;
import org.springframework.batch.item.ItemRecoverer;
+import org.springframework.batch.retry.ExhaustedRetryException;
import org.springframework.batch.retry.policy.AlwaysRetryPolicy;
import org.springframework.batch.retry.policy.NeverRetryPolicy;
import org.springframework.batch.retry.policy.SimpleRetryPolicy;
@@ -134,7 +135,30 @@ public class StatefulRetryOperationsInterceptorTests extends TestCase {
assertEquals(1, result.size());
}
- public void testRetryExceptionAfterTooManyAttempts() throws Exception {
+ public void testRetryExceptionAfterTooManyAttemptsWithNoRecovery() throws Exception {
+ ((Advised) service).addAdvice(interceptor);
+ interceptor.setRetryPolicy(new NeverRetryPolicy());
+ try {
+ service.service("foo");
+ fail("Expected Exception.");
+ }
+ catch (Exception e) {
+ String message = e.getMessage();
+ assertTrue("Wrong message: " + message, message.startsWith("Not enough calls"));
+ }
+ assertEquals(1, count);
+ try {
+ service.service("foo");
+ fail("Expected ExhaustedRetryException");
+ } catch (ExhaustedRetryException e) {
+ // expected
+ String message = e.getMessage();
+ assertTrue("Wrong message: " + message, message.startsWith("Retry was exhausted but there was no recover"));
+ }
+ assertEquals(1, count);
+ }
+
+ public void testRecoveryAfterTooManyAttempts() throws Exception {
((Advised) service).addAdvice(interceptor);
interceptor.setRetryPolicy(new NeverRetryPolicy());
try {