GH-231: Support Retry Based on Result

Resolves https://github.com/spring-projects/spring-retry/issues/231

Enhance `RetryListener` to allow it to examine the result to determine
whether the call should be accepted, or retried according to the retry
policy.

* Restore missing period in doc.

README.md

@@ -345,20 +345,26 @@ public interface RetryListener {
 
     void open(RetryContext context, RetryCallback<T> callback);
 
+    void onSuccess(RetryContext context, T result);
+
     void onError(RetryContext context, RetryCallback<T> callback, Throwable e);
 
     void close(RetryContext context, RetryCallback<T> callback, Throwable e);
+
 }
 ```
 
-The `open` and `close` callbacks come before and after the entire retry in the simplest
-case, and `onError` applies to the individual `RetryCallback` calls. The close method
-might also receive a `Throwable`. If there has been an error, it is the last one thrown by
-the `RetryCallback`.
+The `open` and `close` callbacks come before and after the entire retry in the simplest case, and `onSuccess`, `onError` apply to the individual `RetryCallback` calls; the current retry count can be obtained from the `RetryContext`.
+The close method might also receive a `Throwable`.
+Starting with version 2.0, the `onSuccess` method is called after a successful call to the callback.
+This allows the listener to examine the result and throw an exception if the result doesn't match some expected criteria.
+The type of the exception thrown is then used to determine whether the call should be retried or not, based on the retry policy.
+If there has been an error, it is the last one thrown by the `RetryCallback`.
 
 Note that when there is more than one listener, they are in a list, so there is an order.
-In this case, `open` is called in the same order, while `onError` and `close` are called
-in reverse order.
+In this case, `open` is called in the same order, while `onSuccess`, `onError`, and `close` are called in reverse order.
+
+`RetryListenerSupport` is provided, with no-op implementations; you can extend this class if you don't need to implement all of the `RetryListener` methods.
 
 ### Listeners for Reflective Method Invocations
 
@@ -366,6 +372,8 @@ When dealing with methods that are annotated with `@Retryable` or with Spring AO
 
 Such a scenario could be particularly useful when there is a need to monitor how often a certain method call has been retried and expose it with detailed tagging information (such as class name, method name, or even parameter values in some exotic cases).
 
+Starting with version 2.0, the `MethodInvocationRetryListenerSupport` has a new method `doOnSuccess`.
+
 The following example registers such a listener:
 
 ```java
@@ -383,6 +391,17 @@ template.registerListener(new MethodInvocationRetryListenerSupport() {
 
         // register a monitoring counter with appropriate tags
         // ...
+
+        @Override
+        protected <T, E extends Throwable> void doOnSuccess(RetryContext context,
+                MethodInvocationRetryCallback<T, E> callback, T result) {
+
+            Object[] arguments = callback.getInvocation().getArguments();
+
+            // decide whether the result for the given arguments should be accepted
+            // or retried according to the retry policy
+        }
+
       }
     });
 ```

src/main/java/org/springframework/retry/RetryListener.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2007 the original author or authors.
+ * Copyright 2006-2022 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.
@@ -22,6 +22,7 @@ package org.springframework.retry;
  * lifecycle.
  *
  * @author Dave Syer
+ * @author Gary Russell
  *
  */
 public interface RetryListener {
@@ -40,8 +41,8 @@ public interface RetryListener {
 	<T, E extends Throwable> boolean open(RetryContext context, RetryCallback<T, E> callback);
 
 	/**
-	 * Called after the final attempt (successful or not). Allow the interceptor to clean
-	 * up any resource it is holding before control returns to the retry caller.
+	 * Called after the final attempt (successful or not). Allow the listener to clean up
+	 * any resource it is holding before control returns to the retry caller.
 	 * @param context the current {@link RetryContext}.
 	 * @param callback the current {@link RetryCallback}.
 	 * @param throwable the last exception that was thrown by the callback.
@@ -50,6 +51,19 @@ public interface RetryListener {
 	 */
 	<T, E extends Throwable> void close(RetryContext context, RetryCallback<T, E> callback, Throwable throwable);
 
+	/**
+	 * Called after a successful attempt; allow the listener to throw a new exception to
+	 * cause a retry (according to the retry policy), based on the result returned by the
+	 * {@link RetryCallback#doWithRetry(RetryContext)}
+	 * @param <T> the return type.
+	 * @param context the current {@link RetryContext}.
+	 * @param callback the current {@link RetryCallback}.
+	 * @param result the result returned by the callback method.
+	 * @since 2.0
+	 */
+	default <T, E extends Throwable> void onSuccess(RetryContext context, RetryCallback<T, E> callback, T result) {
+	}
+
 	/**
 	 * Called after every unsuccessful attempt at a retry.
 	 * @param context the current {@link RetryContext}.

src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java

@@ -19,6 +19,8 @@ package org.springframework.retry.listener;
 import org.springframework.retry.RetryCallback;
 import org.springframework.retry.RetryContext;
 import org.springframework.retry.RetryListener;
+import org.springframework.retry.RetryOperations;
+import org.springframework.retry.TerminatedRetryException;
 import org.springframework.retry.interceptor.MethodInvocationRetryCallback;
 
 /**
@@ -35,6 +37,7 @@ import org.springframework.retry.interceptor.MethodInvocationRetryCallback;
  */
 public class MethodInvocationRetryListenerSupport implements RetryListener {
 
+	@Override
 	public <T, E extends Throwable> void close(RetryContext context, RetryCallback<T, E> callback,
 			Throwable throwable) {
 		if (callback instanceof MethodInvocationRetryCallback) {
@@ -43,6 +46,15 @@ public class MethodInvocationRetryListenerSupport implements RetryListener {
 		}
 	}
 
+	@Override
+	public <T, E extends Throwable> void onSuccess(RetryContext context, RetryCallback<T, E> callback, T result) {
+		if (callback instanceof MethodInvocationRetryCallback) {
+			MethodInvocationRetryCallback<T, E> methodInvocationRetryCallback = (MethodInvocationRetryCallback<T, E>) callback;
+			doOnSuccess(context, methodInvocationRetryCallback, result);
+		}
+	}
+
+	@Override
 	public <T, E extends Throwable> void onError(RetryContext context, RetryCallback<T, E> callback,
 			Throwable throwable) {
 		if (callback instanceof MethodInvocationRetryCallback) {
@@ -51,6 +63,7 @@ public class MethodInvocationRetryListenerSupport implements RetryListener {
 		}
 	}
 
+	@Override
 	public <T, E extends Throwable> boolean open(RetryContext context, RetryCallback<T, E> callback) {
 		if (callback instanceof MethodInvocationRetryCallback) {
 			MethodInvocationRetryCallback<T, E> methodInvocationRetryCallback = (MethodInvocationRetryCallback<T, E>) callback;
@@ -61,14 +74,56 @@ public class MethodInvocationRetryListenerSupport implements RetryListener {
 		return true;
 	}
 
+	/**
+	 * Called after the final attempt (successful or not). Allow the listener to clean up
+	 * any resource it is holding before control returns to the retry caller.
+	 * @param context the current {@link RetryContext}.
+	 * @param callback the current {@link RetryCallback}.
+	 * @param throwable the last exception that was thrown by the callback.
+	 * @param <E> the exception type
+	 * @param <T> the return value
+	 */
 	protected <T, E extends Throwable> void doClose(RetryContext context, MethodInvocationRetryCallback<T, E> callback,
 			Throwable throwable) {
 	}
 
+	/**
+	 * Called after a successful attempt; allow the listener to throw a new exception to
+	 * cause a retry (according to the retry policy), based on the result returned by the
+	 * {@link RetryCallback#doWithRetry(RetryContext)}
+	 * @param <T> the return type.
+	 * @param context the current {@link RetryContext}.
+	 * @param callback the current {@link RetryCallback}.
+	 * @param result the result returned by the callback method.
+	 * @since 2.0
+	 */
+	protected <T, E extends Throwable> void doOnSuccess(RetryContext context,
+			MethodInvocationRetryCallback<T, E> callback, T result) {
+	}
+
+	/**
+	 * Called after every unsuccessful attempt at a retry.
+	 * @param context the current {@link RetryContext}.
+	 * @param callback the current {@link RetryCallback}.
+	 * @param throwable the last exception that was thrown by the callback.
+	 * @param <T> the return value
+	 * @param <E> the exception to throw
+	 */
 	protected <T, E extends Throwable> void doOnError(RetryContext context,
 			MethodInvocationRetryCallback<T, E> callback, Throwable throwable) {
 	}
 
+	/**
+	 * Called before the first attempt in a retry. For instance, implementers can set up
+	 * state that is needed by the policies in the {@link RetryOperations}. The whole
+	 * retry can be vetoed by returning false from this method, in which case a
+	 * {@link TerminatedRetryException} will be thrown.
+	 * @param <T> the type of object returned by the callback
+	 * @param <E> the type of exception it declares may be thrown
+	 * @param context the current {@link RetryContext}.
+	 * @param callback the current {@link RetryCallback}.
+	 * @return true if the retry should proceed.
+	 */
 	protected <T, E extends Throwable> boolean doOpen(RetryContext context,
 			MethodInvocationRetryCallback<T, E> callback) {
 		return true;

src/main/java/org/springframework/retry/support/RetryTemplate.java

@@ -326,7 +326,9 @@ 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 retryCallback.doWithRetry(context);
+					T result = retryCallback.doWithRetry(context);
+					doOnSuccessInterceptors(retryCallback, context, result);
+					return result;
 				}
 				catch (Throwable e) {
 
@@ -590,6 +592,13 @@ public class RetryTemplate implements RetryOperations {
 		}
 	}
 
+	private <T, E extends Throwable> void doOnSuccessInterceptors(RetryCallback<T, E> callback, RetryContext context,
+			T result) {
+		for (int i = this.listeners.length; i-- > 0;) {
+			this.listeners[i].onSuccess(context, callback, result);
+		}
+	}
+
 	private <T, E extends Throwable> void doOnErrorInterceptors(RetryCallback<T, E> callback, RetryContext context,
 			Throwable throwable) {
 		for (int i = this.listeners.length; i-- > 0;) {

src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java

@@ -129,9 +129,11 @@ public class RetryOperationsInterceptorTests {
 		final String methodTagName = "method";
 		final String labelTagName = "label";
 		final Map<String, String> monitoringTags = new HashMap<>();
+		AtomicBoolean argumentsAsExpected = new AtomicBoolean();
 		RetryTemplate template = new RetryTemplate();
 		template.setRetryPolicy(new SimpleRetryPolicy(2));
 		template.registerListener(new MethodInvocationRetryListenerSupport() {
+
 			@Override
 			protected <T, E extends Throwable> void doClose(RetryContext context,
 					MethodInvocationRetryCallback<T, E> callback, Throwable throwable) {
@@ -140,6 +142,14 @@ public class RetryOperationsInterceptorTests {
 				monitoringTags.put(classTagName, method.getDeclaringClass().getSimpleName());
 				monitoringTags.put(methodTagName, method.getName());
 			}
+
+			@Override
+			protected <T, E extends Throwable> void doOnSuccess(RetryContext context,
+					MethodInvocationRetryCallback<T, E> callback, T result) {
+
+				argumentsAsExpected.set(callback.getInvocation().getArguments().length == 0);
+			}
+
 		});
 
 		this.interceptor.setLabel(label);
@@ -153,6 +163,7 @@ public class RetryOperationsInterceptorTests {
 		assertThat(monitoringTags.get(classTagName),
 				equalTo(RetryOperationsInterceptorTests.Service.class.getSimpleName()));
 		assertThat(monitoringTags.get(methodTagName), equalTo("service"));
+		assertTrue(argumentsAsExpected.get());
 	}
 
 	@Test

src/test/java/org/springframework/retry/support/RetryTemplateTests.java

@@ -17,6 +17,7 @@
 package org.springframework.retry.support;
 
 import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.junit.Test;
@@ -29,6 +30,7 @@ import org.springframework.retry.backoff.BackOffContext;
 import org.springframework.retry.backoff.BackOffInterruptedException;
 import org.springframework.retry.backoff.BackOffPolicy;
 import org.springframework.retry.backoff.StatelessBackOffPolicy;
+import org.springframework.retry.listener.RetryListenerSupport;
 import org.springframework.retry.policy.NeverRetryPolicy;
 import org.springframework.retry.policy.SimpleRetryPolicy;
 
@@ -346,6 +348,30 @@ public class RetryTemplateTests {
 		verify(bop).start(any());
 	}
 
+	@Test
+	public void testRetryOnBadResult() {
+		RetryTemplate template = new RetryTemplate();
+		template.registerListener(new RetryListenerSupport() {
+
+			@Override
+			public <T, E extends Throwable> void onSuccess(RetryContext context, RetryCallback<T, E> callback,
+					T result) {
+
+				if (result.equals("bad")) {
+					throw new IllegalStateException("test");
+				}
+			}
+
+		});
+		AtomicBoolean first = new AtomicBoolean(true);
+		AtomicInteger callCount = new AtomicInteger();
+		template.execute((ctx) -> {
+			callCount.incrementAndGet();
+			return first.getAndSet(false) ? "bad" : "good";
+		});
+		assertEquals(2, callCount.get());
+	}
+
 	private static class MockRetryCallback implements RetryCallback<Object, Exception> {
 
 		private int attempts;