OPEN - issue BATCH-777: Parametrise RetryCallback and related interfaces

Extend RetryOperations to include recovery case

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

@@ -64,6 +64,6 @@ public interface RetryContext extends AttributeAccessor {
 	 * be null if this is the first attempt, but also if the enclosing policy
 	 * decides not to provide it (e.g. because of concerns about memory usage).
 	 */
-	Throwable getLastThrowable();
+	Exception getLastThrowable();
 
 }

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

@@ -27,7 +27,7 @@ public interface RetryOperations {
 
 	/**
 	 * Execute the supplied {@link RetryCallback} with the configured retry
-	 * semantics.  See implementations for configuration details.
+	 * semantics. See implementations for configuration details.
 	 * 
 	 * @return the value returned by the {@link RetryCallback} upon successful
 	 * invocation.
@@ -36,4 +36,15 @@ public interface RetryOperations {
 	 */
 	Object execute(RetryCallback retryCallback) throws Exception;
 
+	/**
+	 * Execute the supplied {@link RetryCallback} with a fallback on exhausted
+	 * retry to the {@link RecoveryCallback}. See implementations for configuration details.
+	 * 
+	 * @return the value returned by the {@link RetryCallback} upon successful
+	 * invocation, and that returned by the {@link RecoveryCallback} otherwise.
+	 * @throws Exception any {@link Exception} raised by the
+	 * {@link RecoveryCallback} upon unsuccessful retry.
+	 */
+	Object execute(RetryCallback retryCallback, RecoveryCallback recoveryCallback) throws Exception;
+
 }

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

@@ -70,7 +70,7 @@ public interface RetryPolicy {
 	 * @throws TerminatedRetryException if the status is set to terminate only.
 	 * 
 	 */
-	void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException;
+	void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException;
 
 	/**
 	 * Handle an exhausted retry. Default will be to throw an exception, but

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/context/RetryContextSupport.java

@@ -26,7 +26,7 @@ public class RetryContextSupport extends AttributeAccessorSupport implements Ret
 
 	private int count;
 
-	private Throwable lastException;
+	private Exception lastException;
 
 	private RetryContext parent;
 
@@ -51,7 +51,7 @@ public class RetryContextSupport extends AttributeAccessorSupport implements Ret
 		return count;
 	}
 
-	public Throwable getLastThrowable() {
+	public Exception getLastThrowable() {
 		return lastException;
 	}
 
@@ -61,7 +61,7 @@ public class RetryContextSupport extends AttributeAccessorSupport implements Ret
 	 * 
 	 * All {@link RetryPolicy} implementations should use this method when they
 	 * register the throwable. It should only be called once per retry attempt
-	 * because it increments the conter.<br/>
+	 * because it increments a counter.<br/>
 	 * 
 	 * Use of this method is not enforced by the framework - it is a service
 	 * provider contract for authors of policies.
@@ -69,7 +69,7 @@ public class RetryContextSupport extends AttributeAccessorSupport implements Ret
 	 * @param throwable the exception that caused the current retry attempt to
 	 * fail.
 	 */
-	public void registerThrowable(Throwable throwable) {
+	public void registerThrowable(Exception throwable) {
 		this.lastException = throwable;
 		if (throwable != null)
 			count++;

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/AbstractStatelessRetryPolicy.java

@@ -16,7 +16,6 @@
 
 package org.springframework.batch.retry.policy;
 
-import org.springframework.batch.retry.ExhaustedRetryException;
 import org.springframework.batch.retry.RetryContext;
 import org.springframework.batch.retry.RetryPolicy;
 
@@ -49,9 +48,8 @@ public abstract class AbstractStatelessRetryPolicy implements RetryPolicy {
 	 * 
 	 * @see org.springframework.batch.retry.RetryPolicy#handleRetryExhausted(org.springframework.batch.retry.RetryContext)
 	 */
-	public Object handleRetryExhausted(RetryContext context) throws ExhaustedRetryException {
-		throw new ExhaustedRetryException("Retry exhausted after last attempt with no recovery path.", context
-				.getLastThrowable());
+	public Object handleRetryExhausted(RetryContext context) throws Exception {
+		throw context.getLastThrowable();
 	}
 
 }

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/CompositeRetryPolicy.java

@@ -110,7 +110,7 @@ public class CompositeRetryPolicy extends AbstractStatelessRetryPolicy {
 	 * 
 	 * @see org.springframework.batch.retry.RetryPolicy#close(org.springframework.batch.retry.RetryContext, boolean)
 	 */
-	public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+	public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 		RetryContext[] contexts = ((CompositeRetryContext) context).contexts;
 		RetryPolicy[] policies = ((CompositeRetryContext) context).policies;
 		for (int i = 0; i < contexts.length; i++) {

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicy.java

@@ -103,9 +103,9 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 	 * Delegate to the policy currently activated in the context.
 	 * 
 	 * @see org.springframework.batch.retry.RetryPolicy#registerThrowable(org.springframework.batch.retry.RetryContext,
-	 * java.lang.Throwable)
+	 * Exception)
 	 */
-	public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+	public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 		RetryPolicy policy = (RetryPolicy) context;
 		policy.registerThrowable(context, throwable);
 		((RetryContextSupport) context).registerThrowable(throwable);
@@ -158,7 +158,7 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 			return this;
 		}
 
-		public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+		public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 			policy = getPolicy(exceptionClassifier.classify(throwable));
 			this.context = getContext(policy);
 			policy.registerThrowable(this.context, throwable);

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/NeverRetryPolicy.java

@@ -64,9 +64,9 @@ public class NeverRetryPolicy extends AbstractStatelessRetryPolicy {
 	/**
 	 * Do nothing.
 	 * @see org.springframework.batch.retry.RetryPolicy#registerThrowable(org.springframework.batch.retry.RetryContext,
-	 * java.lang.Throwable)
+	 * Exception)
 	 */
-	public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+	public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 		((NeverRetryContext) context).setFinished();
 		((RetryContextSupport) context).registerThrowable(throwable);
 	}

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/RecoveryCallbackRetryPolicy.java

@@ -115,9 +115,9 @@ public class RecoveryCallbackRetryPolicy extends AbstractStatefulRetryPolicy {
 	 * implemented by subclasses), and remove the current item from the history.
 	 * 
 	 * @see org.springframework.batch.retry.RetryPolicy#registerThrowable(org.springframework.batch.retry.RetryContext,
-	 * java.lang.Throwable)
+	 * Exception)
 	 */
-	public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+	public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 		((RetryPolicy) context).registerThrowable(context, throwable);
 		// The throwable is stored in the delegate context.
 	}
@@ -185,7 +185,7 @@ public class RecoveryCallbackRetryPolicy extends AbstractStatefulRetryPolicy {
 			return null;
 		}
 
-		public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+		public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 			// TODO: this comparison assumes that hashCode is the limiting
 			// factor. Actually the cache should be able to decide for us.
 			if (this.initialHashCode != key.hashCode()) {
@@ -213,7 +213,7 @@ public class RecoveryCallbackRetryPolicy extends AbstractStatefulRetryPolicy {
 			return null;
 		}
 
-		public Throwable getLastThrowable() {
+		public Exception getLastThrowable() {
 			return delegateContext.getLastThrowable();
 		}
 

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/SimpleRetryPolicy.java

@@ -75,7 +75,6 @@ public class SimpleRetryPolicy extends AbstractStatelessRetryPolicy {
 		classes.add(Exception.class);
 		setRetryableExceptionClasses(classes);
 		classes = new HashSet<Class<? extends Throwable>>();
-		classes.add(Error.class);
 		setFatalExceptionClasses(classes);
 		this.maxAttempts = maxAttempts;
 	}
@@ -136,9 +135,9 @@ public class SimpleRetryPolicy extends AbstractStatelessRetryPolicy {
 	 * Update the status with another attempted retry and the latest exception.
 	 * 
 	 * @see org.springframework.batch.retry.RetryPolicy#registerThrowable(org.springframework.batch.retry.RetryContext,
-	 * java.lang.Throwable)
+	 * Exception)
 	 */
-	public void registerThrowable(RetryContext context, Throwable throwable) {
+	public void registerThrowable(RetryContext context, Exception throwable) {
 		SimpleRetryContext simpleContext = ((SimpleRetryContext) context);
 		simpleContext.registerThrowable(throwable);
 	}

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/TimeoutRetryPolicy.java

@@ -63,7 +63,7 @@ public class TimeoutRetryPolicy extends AbstractStatelessRetryPolicy {
 		return new TimeoutRetryContext(parent, timeout);
 	}
 
-	public void registerThrowable(RetryContext context, Throwable throwable) throws TerminatedRetryException {
+	public void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException {
 		((RetryContextSupport) context).registerThrowable(throwable);
 		// otherwise no-op - we only time out, otherwise retry everything...
 	}

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

@@ -22,6 +22,8 @@ import java.util.List;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+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.RetryListener;
@@ -120,18 +122,40 @@ public class RetryTemplate implements RetryOperations {
 	 * terminated through the {@link RetryContext}.
 	 */
 	public final Object execute(RetryCallback callback) throws Exception {
-
-		/*
-		 * Read all needed data into local variables to prevent any
-		 * reference/primitive changes on other threads affecting this retry
-		 * attempt.
-		 */
-		BackOffPolicy backOffPolicy = this.backOffPolicy;
 		RetryPolicy retryPolicy = this.retryPolicy;
+		return doExecute(callback, null, retryPolicy);
+	}
 
+	/**
+	 * Keep executing the callback until it either succeeds or the policy
+	 * dictates that we stop, in which case the recovery callback will be
+	 * executed.
+	 * 
+	 * @see org.springframework.batch.retry.RetryOperations#execute(org.springframework.batch.retry.RetryCallback,
+	 * org.springframework.batch.retry.RecoveryCallback)
+	 * 
+	 * @throws TerminatedRetryException if the retry has been manually
+	 * terminated through the {@link RetryContext}.
+	 */
+	public final Object execute(RetryCallback retryCallback, RecoveryCallback recoveryCallback) throws Exception {
+		RetryPolicy retryPolicy = this.retryPolicy;
+		return doExecute(retryCallback, recoveryCallback, retryPolicy);
+	}
+
+	/**
+	 * @param retryCallback
+	 * @param recoveryCallback
+	 * @param retryPolicy
+	 * @return the result of the callback
+	 * @throws Exception
+	 */
+	protected Object doExecute(RetryCallback retryCallback, RecoveryCallback recoveryCallback, RetryPolicy retryPolicy)
+			throws Exception {
+
+		BackOffPolicy backOffPolicy = this.backOffPolicy;
 		// Allow the retry policy to initialise itself...
 		// TODO: catch and rethrow abnormal retry exception?
-		RetryContext context = retryPolicy.open(callback, RetrySynchronizationManager.getContext());
+		RetryContext context = retryPolicy.open(retryCallback, RetrySynchronizationManager.getContext());
 
 		// Make sure the context is available globally for clients who need
 		// it...
@@ -142,7 +166,7 @@ public class RetryTemplate implements RetryOperations {
 		try {
 
 			// Give clients a chance to enhance the context...
-			boolean running = doOpenInterceptors(callback, context);
+			boolean running = doOpenInterceptors(retryCallback, context);
 
 			if (!running) {
 				throw new TerminatedRetryException("Retry terminated abnormally by interceptor before first attempt");
@@ -164,17 +188,17 @@ public class RetryTemplate implements RetryOperations {
 					// Reset the last exception, so if we are successful
 					// the close interceptors will not think we failed...
 					lastException = null;
-					return callback.doWithRetry(context);
+					return retryCallback.doWithRetry(context);
 				}
 				catch (Exception e) {
 
 					lastException = e;
 
-					doOnErrorInterceptors(callback, context, e);
+					doOnErrorInterceptors(retryCallback, context, e);
 
 					retryPolicy.registerThrowable(context, e);
 
-					if (retryPolicy.shouldRethrow(context)) {
+					if (shouldRethrow(context)) {
 						logger.debug("Rethrow in retry for policy: count=" + context.getRetryCount());
 						throw e;
 					}
@@ -200,14 +224,50 @@ public class RetryTemplate implements RetryOperations {
 			}
 
 			logger.debug("Retry failed last attempt: count=" + context.getRetryCount());
-			return retryPolicy.handleRetryExhausted(context);
+
+			if (context.isExhaustedOnly()) {
+				throw new ExhaustedRetryException("Retry exhausted after last attempt with no recovery path.", context
+						.getLastThrowable());
+			}
+
+			return handleRetryExhausted(recoveryCallback, context);
 
 		}
 		finally {
-			retryPolicy.close(context, lastException==null);
-			doCloseInterceptors(callback, context, lastException);
+			retryPolicy.close(context, lastException == null);
+			doCloseInterceptors(retryCallback, context, lastException);
 			RetrySynchronizationManager.clear();
 		}
+
+	}
+
+	/**
+	 * @param recoveryCallback the callback for recovery (might be null)
+	 * @param context the current retry context
+	 * @throws Exception if the callback does, and if there is no callback then
+	 * definitely the last exception from the context
+	 */
+	private Object handleRetryExhausted(RecoveryCallback recoveryCallback, RetryContext context) throws Exception {
+		return retryPolicy.handleRetryExhausted(context);
+//		if (recoveryCallback != null) {
+//			return recoveryCallback.recover(context);
+//		}
+//		logger.debug("Retry exhausted after last attempt with no recovery path.");
+//		throw context.getLastThrowable();
+	}
+
+	/**
+	 * Extension point for subclasses to decide on behaviour after catching an
+	 * exception in a {@link RetryCallback}. Normal stateless behaviour is not
+	 * to rethrow.
+	 * 
+	 * @param context the current {@link RetryContext}
+	 * 
+	 * @return false but subclasses might choose otherwise
+	 */
+	protected boolean shouldRethrow(RetryContext context) {
+		// TODO: return false
+		return retryPolicy.shouldRethrow(context);
 	}
 
 	private boolean doOpenInterceptors(RetryCallback callback, RetryContext context) {

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

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2006-2007 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.batch.retry.support;
+
+import org.springframework.batch.retry.RecoveryCallback;
+import org.springframework.batch.retry.RetryCallback;
+import org.springframework.batch.retry.RetryContext;
+import org.springframework.batch.retry.RetryPolicy;
+
+/**
+ * 
+ * 
+ * @author Dave Syer
+ */
+public class StatefulRetryTemplate extends RetryTemplate {
+
+	/**
+	 * @param retryCallback
+	 * @param recoveryCallback
+	 * @param retryPolicy
+	 * @return the result of the callback
+	 * @throws Exception
+	 */
+	protected Object doExecute(RetryCallback retryCallback, RecoveryCallback recoveryCallback, RetryPolicy retryPolicy)
+			throws Exception {
+
+		return super.doExecute(retryCallback, recoveryCallback, retryPolicy);
+
+	}
+
+	/**
+	 * Extension point for subclasses to decide on behaviour after catching an
+	 * exception in a {@link RetryCallback}. Normal stateless behaviour is not
+	 * to rethrow.
+	 * 
+	 * @param context the current {@link RetryContext}
+	 * 
+	 * @return false but subclasses might choose otherwise
+	 */
+	protected boolean shouldRethrow(RetryContext context) {
+		return false;
+	}
+
+}