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

Simplify RetryPolicy interface now that they are all stateless

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

@@ -35,29 +35,20 @@ public interface RetryPolicy {
 	 */
 	boolean canRetry(RetryContext context);
 
-	/**
-	 * @param context the current context.
-	 * @return true if the policy determines that the last exception should be
-	 * re-thrown.
-	 */
-	boolean shouldRethrow(RetryContext context);
-
 	/**
 	 * Acquire resources needed for the retry operation. The callback is passed
 	 * in so that marker interfaces can be used and a manager can collaborate
 	 * with the callback to set up some state in the status token.
-	 * 
-	 * @param callback the {@link RetryCallback} that will execute the unit of
-	 * work for this retry.
 	 * @param parent the parent context if we are in a nested retry.
+	 * 
 	 * @return a {@link RetryContext} object specific to this manager.
 	 * 
 	 */
-	RetryContext open(RetryCallback callback, RetryContext parent);
+	RetryContext open(RetryContext parent);
 
 	/**
 	 * @param context a retry status created by the
-	 * {@link #open(RetryCallback, RetryContext)} method of this manager.
+	 * {@link #open(RetryContext)} method of this manager.
 	 * @param succeeded true if the retry callback succeeded
 	 */
 	void close(RetryContext context, boolean succeeded);
@@ -72,16 +63,4 @@ public interface RetryPolicy {
 	 */
 	void registerThrowable(RetryContext context, Exception throwable) throws TerminatedRetryException;
 
-	/**
-	 * Handle an exhausted retry. Default will be to throw an exception, but
-	 * implementations may provide recovery path.
-	 * 
-	 * @param context the current retry context.
-	 * @return an appropriate value possibly from the callback.
-	 * 
-	 * @throws ExhaustedRetryException if there is no recovery path.
-	 * @throws Exception in rare cases where the policy wants to propagate the
-	 * retryable exception
-	 */
-	Object handleRetryExhausted(RetryContext context) throws ExhaustedRetryException, Exception;
 }

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

@@ -1,56 +0,0 @@
-/*
- * 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.policy;
-
-import org.springframework.batch.retry.RetryContext;
-import org.springframework.batch.retry.RetryPolicy;
-
-/**
- * Base class for "normal" retry policies: those that operate in the context of
- * a callback that is called repeatedly in a loop until it succeeds, or the
- * policy decides to terminate. There is no need for such policies to store
- * state outside the context.
- * 
- * @see RetryPolicy#handleRetryExhausted(RetryContext)
- * 
- * @author Dave Syer
- * 
- * @deprecated TODO: remove this base class
- * 
- */
-public abstract class AbstractStatelessRetryPolicy implements RetryPolicy {
-
-	/**
-	 * Just returns the negative of {@link RetryPolicy#canRetry(RetryContext)},
-	 * i.e. if we cannot retry then the exception should be thrown.
-	 * 
-	 * @see org.springframework.batch.retry.RetryPolicy#shouldRethrow(org.springframework.batch.retry.RetryContext)
-	 */
-	public boolean shouldRethrow(RetryContext context) {
-		return !canRetry(context);
-	}
-
-	/**
-	 * Throw an exception.
-	 * 
-	 * @see org.springframework.batch.retry.RetryPolicy#handleRetryExhausted(org.springframework.batch.retry.RetryContext)
-	 */
-	public Object handleRetryExhausted(RetryContext context) throws Exception {
-		throw context.getLastThrowable();
-	}
-
-}

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

@@ -19,7 +19,6 @@ package org.springframework.batch.retry.policy;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.springframework.batch.retry.RetryCallback;
 import org.springframework.batch.retry.RetryContext;
 import org.springframework.batch.retry.RetryPolicy;
 import org.springframework.batch.retry.TerminatedRetryException;
@@ -32,7 +31,7 @@ import org.springframework.batch.retry.context.RetryContextSupport;
  * @author Dave Syer
  * 
  */
-public class CompositeRetryPolicy extends AbstractStatelessRetryPolicy {
+public class CompositeRetryPolicy implements RetryPolicy {
 
 	RetryPolicy[] policies = new RetryPolicy[0];
 
@@ -93,13 +92,12 @@ public class CompositeRetryPolicy extends AbstractStatelessRetryPolicy {
 	 * Creates a new context that copies the existing policies and keeps a list
 	 * of the contexts from each one.
 	 * 
-	 * @see org.springframework.batch.retry.RetryPolicy#open(org.springframework.batch.retry.RetryCallback,
-	 * RetryContext)
+	 * @see org.springframework.batch.retry.RetryPolicy#open(RetryContext)
 	 */
-	public RetryContext open(RetryCallback callback, RetryContext parent) {
+	public RetryContext open(RetryContext parent) {
 		List<RetryContext> list = new ArrayList<RetryContext>();
 		for (int i = 0; i < policies.length; i++) {
-			list.add(policies[i].open(callback, parent));
+			list.add(policies[i].open(parent));
 		}
 		return new CompositeRetryContext(parent, list);
 	}

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

@@ -19,7 +19,6 @@ package org.springframework.batch.retry.policy;
 import java.util.HashMap;
 import java.util.Map;
 
-import org.springframework.batch.retry.RetryCallback;
 import org.springframework.batch.retry.RetryContext;
 import org.springframework.batch.retry.RetryPolicy;
 import org.springframework.batch.retry.TerminatedRetryException;
@@ -35,7 +34,7 @@ import org.springframework.util.Assert;
  * @author Dave Syer
  * 
  */
-public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy {
+public class ExceptionClassifierRetryPolicy implements RetryPolicy {
 
 	private Classifier<Throwable, String> exceptionClassifier = new ExceptionClassifierSupport();
 
@@ -92,11 +91,10 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 	 * Create an active context that proxies a retry policy by chosing a target
 	 * from the policy map.
 	 * 
-	 * @see org.springframework.batch.retry.RetryPolicy#open(org.springframework.batch.retry.RetryCallback,
-	 * RetryContext)
+	 * @see org.springframework.batch.retry.RetryPolicy#open(RetryContext)
 	 */
-	public RetryContext open(RetryCallback callback, RetryContext parent) {
-		return new ExceptionClassifierRetryContext(parent, exceptionClassifier).open(callback, parent);
+	public RetryContext open(RetryContext parent) {
+		return new ExceptionClassifierRetryContext(parent, exceptionClassifier).open(parent);
 	}
 
 	/**
@@ -121,9 +119,6 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 		// Dynamic: depends on the policy:
 		RetryContext context;
 
-		// The same for the life of the context:
-		RetryCallback callback;
-
 		Map<RetryPolicy, RetryContext> contexts = new HashMap<RetryPolicy, RetryContext>();
 
 		public ExceptionClassifierRetryContext(RetryContext parent, Classifier<Throwable,String> exceptionClassifier) {
@@ -142,10 +137,6 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 			return policy.canRetry(this.context);
 		}
 
-		public boolean shouldRethrow(RetryContext context) {
-			return policy.shouldRethrow(context);
-		}
-
 		public void close(RetryContext context, boolean succeeded) {
 			// Only close those policies that have been used (opened):
 			for (RetryPolicy policy : contexts.keySet()) {
@@ -153,8 +144,7 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 			}
 		}
 
-		public RetryContext open(RetryCallback callback, RetryContext parent) {
-			this.callback = callback;
+		public RetryContext open(RetryContext parent) {
 			return this;
 		}
 
@@ -167,7 +157,7 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 		private RetryContext getContext(RetryPolicy policy) {
 			RetryContext context = contexts.get(policy);
 			if (context == null) {
-				context = policy.open(callback, null);
+				context = policy.open(null);
 				contexts.put(policy, context);
 			}
 			return context;
@@ -179,11 +169,6 @@ public class ExceptionClassifierRetryPolicy extends AbstractStatelessRetryPolicy
 			return result;
 		}
 
-		public Object handleRetryExhausted(RetryContext context) throws UnsupportedOperationException {
-			// Not called...
-			throw new UnsupportedOperationException("Not supported - this code should be unreachable.");
-		}
-
 	}
 
 }

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

@@ -16,7 +16,6 @@
 
 package org.springframework.batch.retry.policy;
 
-import org.springframework.batch.retry.RetryCallback;
 import org.springframework.batch.retry.RetryContext;
 import org.springframework.batch.retry.RetryPolicy;
 import org.springframework.batch.retry.TerminatedRetryException;
@@ -30,7 +29,7 @@ import org.springframework.batch.retry.context.RetryContextSupport;
  * @author Dave Syer
  * 
  */
-public class NeverRetryPolicy extends AbstractStatelessRetryPolicy {
+public class NeverRetryPolicy implements RetryPolicy {
 
 	/**
 	 * Returns false after the first exception. So there is always one try, and
@@ -55,9 +54,9 @@ public class NeverRetryPolicy extends AbstractStatelessRetryPolicy {
 	 * Return a context that can respond to early termination requests, but does
 	 * nothing else.
 	 * 
-	 * @see org.springframework.batch.retry.RetryPolicy#open(org.springframework.batch.retry.RetryCallback, RetryContext)
+	 * @see org.springframework.batch.retry.RetryPolicy#open(RetryContext)
 	 */
-	public RetryContext open(RetryCallback callback, RetryContext parent) {
+	public RetryContext open(RetryContext parent) {
 		return new NeverRetryContext(parent);
 	}
 

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

@@ -19,8 +19,8 @@ package org.springframework.batch.retry.policy;
 import java.util.Collection;
 import java.util.HashSet;
 
-import org.springframework.batch.retry.RetryCallback;
 import org.springframework.batch.retry.RetryContext;
+import org.springframework.batch.retry.RetryPolicy;
 import org.springframework.batch.retry.context.RetryContextSupport;
 import org.springframework.batch.support.BinaryExceptionClassifier;
 
@@ -41,7 +41,7 @@ import org.springframework.batch.support.BinaryExceptionClassifier;
  * @author Rob Harrop
  * 
  */
-public class SimpleRetryPolicy extends AbstractStatelessRetryPolicy {
+public class SimpleRetryPolicy implements RetryPolicy {
 
 	/**
 	 * The default limit to the number of attempts for a new policy.
@@ -143,10 +143,9 @@ public class SimpleRetryPolicy extends AbstractStatelessRetryPolicy {
 	 * Get a status object that can be used to track the current operation
 	 * according to this policy. Has to be aware of the latest exception and the
 	 * number of attempts.
-	 * @see org.springframework.batch.retry.RetryPolicy#open(org.springframework.batch.retry.RetryCallback,
-	 * RetryContext)
+	 * @see org.springframework.batch.retry.RetryPolicy#open(RetryContext)
 	 */
-	public RetryContext open(RetryCallback callback, RetryContext parent) {
+	public RetryContext open(RetryContext parent) {
 		return new SimpleRetryContext(parent);
 	}
 

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

@@ -16,7 +16,6 @@
 
 package org.springframework.batch.retry.policy;
 
-import org.springframework.batch.retry.RetryCallback;
 import org.springframework.batch.retry.RetryContext;
 import org.springframework.batch.retry.RetryPolicy;
 import org.springframework.batch.retry.TerminatedRetryException;
@@ -24,12 +23,12 @@ import org.springframework.batch.retry.context.RetryContextSupport;
 
 /**
  * A {@link RetryPolicy} that allows a retry only if it hasn't timed out. The
- * clock is started on a call to {@link #open(RetryCallback, RetryContext)}.
+ * clock is started on a call to {@link #open(RetryContext)}.
  * 
  * @author Dave Syer
  * 
  */
-public class TimeoutRetryPolicy extends AbstractStatelessRetryPolicy {
+public class TimeoutRetryPolicy implements RetryPolicy {
 
 	/**
 	 * Default value for timeout (milliseconds).
@@ -59,7 +58,7 @@ public class TimeoutRetryPolicy extends AbstractStatelessRetryPolicy {
 	public void close(RetryContext context, boolean succeeded) {
 	}
 
-	public RetryContext open(RetryCallback callback, RetryContext parent) {
+	public RetryContext open(RetryContext parent) {
 		return new TimeoutRetryContext(parent, timeout);
 	}
 

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

@@ -195,8 +195,7 @@ public class RetryTemplate implements RetryOperations {
 		BackOffPolicy backOffPolicy = this.backOffPolicy;
 
 		// Allow the retry policy to initialise itself...
-		// TODO: catch and rethrow abnormal retry exception?
-		RetryContext context = open(retryCallback, retryPolicy, state);
+		RetryContext context = open(retryPolicy, state);
 
 		// Make sure the context is available globally for clients who need
 		// it...
@@ -323,21 +322,18 @@ public class RetryTemplate implements RetryOperations {
 	}
 
 	/**
-	 * @param retryCallback
 	 * @param retryPolicy
 	 * @return a retry context
 	 */
-	protected RetryContext open(RetryCallback retryCallback, RetryPolicy retryPolicy, RetryState state) {
-
-		// TODO: we don't need the callback here
+	protected RetryContext open(RetryPolicy retryPolicy, RetryState state) {
 
 		if (state == null) {
-			return doOpenInternal(retryCallback, retryPolicy);
+			return doOpenInternal(retryPolicy);
 		}
 
 		Object key = state.getKey();
 		if (state.isForceRefresh()) {
-			return doOpenInternal(retryCallback, retryPolicy);
+			return doOpenInternal(retryPolicy);
 		}
 		else if (retryContextCache.containsKey(key)) {
 
@@ -352,20 +348,19 @@ public class RetryTemplate implements RetryOperations {
 		}
 		else {
 
-			// The cache is only ued if there is a failure.
-			return doOpenInternal(retryCallback, retryPolicy);
+			// The cache is only used if there is a failure.
+			return doOpenInternal(retryPolicy);
 
 		}
 
 	}
 
 	/**
-	 * @param retryCallback
 	 * @param retryPolicy
 	 * @return
 	 */
-	private RetryContext doOpenInternal(RetryCallback retryCallback, RetryPolicy retryPolicy) {
-		return retryPolicy.open(retryCallback, RetrySynchronizationManager.getContext());
+	private RetryContext doOpenInternal(RetryPolicy retryPolicy) {
+		return retryPolicy.open(RetrySynchronizationManager.getContext());
 	}
 
 	/**