From aed39de54bed72306503c9d1bfaf849caba00442 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 19 Apr 2022 14:17:21 -0400 Subject: [PATCH] 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 | 31 +++++++++-- .../springframework/retry/RetryListener.java | 20 ++++++- .../MethodInvocationRetryListenerSupport.java | 55 +++++++++++++++++++ .../retry/support/RetryTemplate.java | 11 +++- .../RetryOperationsInterceptorTests.java | 11 ++++ .../retry/support/RetryTemplateTests.java | 26 +++++++++ 6 files changed, 144 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 542cb14..e2f18dc 100644 --- a/README.md +++ b/README.md @@ -345,20 +345,26 @@ public interface RetryListener { void open(RetryContext context, RetryCallback callback); + void onSuccess(RetryContext context, T result); + void onError(RetryContext context, RetryCallback callback, Throwable e); void close(RetryContext context, RetryCallback 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 void doOnSuccess(RetryContext context, + MethodInvocationRetryCallback 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 + } + } }); ``` diff --git a/src/main/java/org/springframework/retry/RetryListener.java b/src/main/java/org/springframework/retry/RetryListener.java index 7f33f2b..a2c3af1 100644 --- a/src/main/java/org/springframework/retry/RetryListener.java +++ b/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 { boolean open(RetryContext context, RetryCallback 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 { */ void close(RetryContext context, RetryCallback 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 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 void onSuccess(RetryContext context, RetryCallback callback, T result) { + } + /** * Called after every unsuccessful attempt at a retry. * @param context the current {@link RetryContext}. diff --git a/src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java b/src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java index ff4667c..601370d 100644 --- a/src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java +++ b/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 void close(RetryContext context, RetryCallback callback, Throwable throwable) { if (callback instanceof MethodInvocationRetryCallback) { @@ -43,6 +46,15 @@ public class MethodInvocationRetryListenerSupport implements RetryListener { } } + @Override + public void onSuccess(RetryContext context, RetryCallback callback, T result) { + if (callback instanceof MethodInvocationRetryCallback) { + MethodInvocationRetryCallback methodInvocationRetryCallback = (MethodInvocationRetryCallback) callback; + doOnSuccess(context, methodInvocationRetryCallback, result); + } + } + + @Override public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { if (callback instanceof MethodInvocationRetryCallback) { @@ -51,6 +63,7 @@ public class MethodInvocationRetryListenerSupport implements RetryListener { } } + @Override public boolean open(RetryContext context, RetryCallback callback) { if (callback instanceof MethodInvocationRetryCallback) { MethodInvocationRetryCallback methodInvocationRetryCallback = (MethodInvocationRetryCallback) 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 the exception type + * @param the return value + */ protected void doClose(RetryContext context, MethodInvocationRetryCallback 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 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 void doOnSuccess(RetryContext context, + MethodInvocationRetryCallback 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 the return value + * @param the exception to throw + */ protected void doOnError(RetryContext context, MethodInvocationRetryCallback 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 the type of object returned by the callback + * @param 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 boolean doOpen(RetryContext context, MethodInvocationRetryCallback callback) { return true; diff --git a/src/main/java/org/springframework/retry/support/RetryTemplate.java b/src/main/java/org/springframework/retry/support/RetryTemplate.java index cc7e6af..6c183c2 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplate.java +++ b/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 void doOnSuccessInterceptors(RetryCallback callback, RetryContext context, + T result) { + for (int i = this.listeners.length; i-- > 0;) { + this.listeners[i].onSuccess(context, callback, result); + } + } + private void doOnErrorInterceptors(RetryCallback callback, RetryContext context, Throwable throwable) { for (int i = this.listeners.length; i-- > 0;) { diff --git a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java index 474592f..e9bc569 100644 --- a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java +++ b/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 monitoringTags = new HashMap<>(); + AtomicBoolean argumentsAsExpected = new AtomicBoolean(); RetryTemplate template = new RetryTemplate(); template.setRetryPolicy(new SimpleRetryPolicy(2)); template.registerListener(new MethodInvocationRetryListenerSupport() { + @Override protected void doClose(RetryContext context, MethodInvocationRetryCallback callback, Throwable throwable) { @@ -140,6 +142,14 @@ public class RetryOperationsInterceptorTests { monitoringTags.put(classTagName, method.getDeclaringClass().getSimpleName()); monitoringTags.put(methodTagName, method.getName()); } + + @Override + protected void doOnSuccess(RetryContext context, + MethodInvocationRetryCallback 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 diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java index 69fab13..2040a0d 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java +++ b/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 void onSuccess(RetryContext context, RetryCallback 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 { private int attempts;