From 7516219c445d2ae492943d5247ee369827a999dc Mon Sep 17 00:00:00 2001 From: e-ivaldi Date: Fri, 17 Nov 2023 20:15:31 +0000 Subject: [PATCH] GH-395: Expose maximum number of attempts property Fixes https://github.com/spring-projects/spring-retry/issues/395 It is sometimes useful to be aware, within a retry context, whether the current attempt is the last attempt when using a 'maximum number of attempts before failure' type of retry policy. In order to achieve the desired result, for all policies, a new attribute is going to be added to the context named "context.max-attempts" that will contain the number of max attempts set in the policy, if any, or -1 otherwise. * GH-395: make the retry context aware of the policies that have a maximum number of attempts set * GH-395: add author details to all modified classes that already had an author list --- .../springframework/retry/RetryContext.java | 8 ++++++ .../springframework/retry/RetryPolicy.java | 17 ++++++++++++ .../retry/policy/CompositeRetryPolicy.java | 17 ++++++++++++ .../retry/policy/MaxAttemptsRetryPolicy.java | 1 + .../retry/policy/SimpleRetryPolicy.java | 2 ++ .../retry/support/RetryTemplate.java | 5 ++++ .../retry/annotation/EnableRetryTests.java | 3 ++- .../policy/CompositeRetryPolicyTests.java | 19 +++++++++++++ .../retry/support/RetryTemplateTests.java | 27 +++++++++++++++++++ 9 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/retry/RetryContext.java b/src/main/java/org/springframework/retry/RetryContext.java index cec6ac5..0450ec5 100644 --- a/src/main/java/org/springframework/retry/RetryContext.java +++ b/src/main/java/org/springframework/retry/RetryContext.java @@ -23,6 +23,7 @@ import org.springframework.core.AttributeAccessor; * used to alter the course of the retry, e.g. force an early termination. * * @author Dave Syer + * @author Emanuele Ivaldi * */ public interface RetryContext extends AttributeAccessor { @@ -61,6 +62,13 @@ public interface RetryContext extends AttributeAccessor { */ String NO_RECOVERY = "context.no-recovery"; + /** + * Retry context attribute that represent the maximum number of attempts for policies + * that provide a maximum number of attempts before failure. For other policies the + * value returned is {@link RetryPolicy#NO_MAXIMUM_ATTEMPTS_SET} + */ + String MAX_ATTEMPTS = "context.max-attempts"; + /** * Signal to the framework that no more attempts should be made to try or retry the * current {@link RetryCallback}. diff --git a/src/main/java/org/springframework/retry/RetryPolicy.java b/src/main/java/org/springframework/retry/RetryPolicy.java index f333014..7185fd0 100644 --- a/src/main/java/org/springframework/retry/RetryPolicy.java +++ b/src/main/java/org/springframework/retry/RetryPolicy.java @@ -26,10 +26,17 @@ import java.io.Serializable; * API for a range of different platforms for the external context. * * @author Dave Syer + * @author Emanuele Ivaldi * */ public interface RetryPolicy extends Serializable { + /** + * The value returned by {@link RetryPolicy#getMaxAttempts()} when the policy doesn't + * provide a maximum number of attempts before failure + */ + int NO_MAXIMUM_ATTEMPTS_SET = -1; + /** * @param context the current retry status * @return true if the operation can proceed @@ -59,4 +66,14 @@ public interface RetryPolicy extends Serializable { */ void registerThrowable(RetryContext context, Throwable throwable); + /** + * Called to understand if the policy has a fixed number of maximum attempts before + * failure + * @return -1 if the policy doesn't provide a fixed number of maximum attempts before + * failure, the number of maximum attempts before failure otherwise + */ + default int getMaxAttempts() { + return NO_MAXIMUM_ATTEMPTS_SET; + } + } diff --git a/src/main/java/org/springframework/retry/policy/CompositeRetryPolicy.java b/src/main/java/org/springframework/retry/policy/CompositeRetryPolicy.java index 6796178..6a97d15 100644 --- a/src/main/java/org/springframework/retry/policy/CompositeRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/CompositeRetryPolicy.java @@ -30,6 +30,7 @@ import org.springframework.retry.context.RetryContextSupport; * * @author Dave Syer * @author Michael Minella + * @author Emanuele Ivaldi * */ @SuppressWarnings("serial") @@ -145,6 +146,22 @@ public class CompositeRetryPolicy implements RetryPolicy { ((RetryContextSupport) context).registerThrowable(throwable); } + /** + * @return the lower 'maximum number of attempts before failure' between all policies + * that have a 'maximum number of attempts before failure' set, if at least one is + * present among the policies, return {@link RetryPolicy#NO_MAXIMUM_ATTEMPTS_SET} + * otherwise + */ + @Override + public int getMaxAttempts() { + return Arrays.stream(policies) + .map(RetryPolicy::getMaxAttempts) + .filter(maxAttempts -> maxAttempts != NO_MAXIMUM_ATTEMPTS_SET) + .sorted() + .findFirst() + .orElse(NO_MAXIMUM_ATTEMPTS_SET); + } + private static class CompositeRetryContext extends RetryContextSupport { RetryContext[] contexts; diff --git a/src/main/java/org/springframework/retry/policy/MaxAttemptsRetryPolicy.java b/src/main/java/org/springframework/retry/policy/MaxAttemptsRetryPolicy.java index 78a4de3..06db30d 100644 --- a/src/main/java/org/springframework/retry/policy/MaxAttemptsRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/MaxAttemptsRetryPolicy.java @@ -74,6 +74,7 @@ public class MaxAttemptsRetryPolicy implements RetryPolicy { * The maximum number of attempts before failure. * @return the maximum number of attempts */ + @Override public int getMaxAttempts() { return this.maxAttempts; } diff --git a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java index 3916cc2..bede505 100644 --- a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java @@ -58,6 +58,7 @@ import org.springframework.util.ClassUtils; * @author Gary Russell * @author Aleksandr Shamukov * @author Artem Bilan + * @author Emanuele Ivaldi */ @SuppressWarnings("serial") public class SimpleRetryPolicy implements RetryPolicy { @@ -194,6 +195,7 @@ public class SimpleRetryPolicy implements RetryPolicy { * The maximum number of attempts before failure. * @return the maximum number of attempts */ + @Override public int getMaxAttempts() { if (this.maxAttemptsSupplier != null) { return this.maxAttemptsSupplier.get(); diff --git a/src/main/java/org/springframework/retry/support/RetryTemplate.java b/src/main/java/org/springframework/retry/support/RetryTemplate.java index abc1d90..ac715fe 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplate.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplate.java @@ -75,6 +75,7 @@ import org.springframework.retry.policy.SimpleRetryPolicy; * @author Artem Bilan * @author Josh Long * @author Aleksandr Shamukov + * @author Emanuele Ivaldi */ public class RetryTemplate implements RetryOperations { @@ -296,6 +297,10 @@ public class RetryTemplate implements RetryOperations { throw new TerminatedRetryException("Retry terminated abnormally by interceptor before first attempt"); } + if (!context.hasAttribute(RetryContext.MAX_ATTEMPTS)) { + context.setAttribute(RetryContext.MAX_ATTEMPTS, retryPolicy.getMaxAttempts()); + } + // Get or Start the backoff context... BackOffContext backOffContext = null; Object resource = context.getAttribute("backOffContext"); diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index 2d20b0f..a8a3ad9 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -60,6 +60,7 @@ import static org.mockito.Mockito.verify; * @author Henning Pöttker * @author Yanming Zhou * @author Anton Aharkau + * @author Emanuele Ivaldi * @since 1.1 */ public class EnableRetryTests { @@ -295,7 +296,7 @@ public class EnableRetryTests { ExpressionService service = context.getBean(ExpressionService.class); service.service6(); RuntimeConfigs runtime = context.getBean(RuntimeConfigs.class); - verify(runtime, times(5)).getMaxAttempts(); + verify(runtime, times(6)).getMaxAttempts(); verify(runtime, times(1)).getInitial(); verify(runtime, times(2)).getMax(); verify(runtime, times(2)).getMult(); diff --git a/src/test/java/org/springframework/retry/policy/CompositeRetryPolicyTests.java b/src/test/java/org/springframework/retry/policy/CompositeRetryPolicyTests.java index ed60bd1..4c56209 100644 --- a/src/test/java/org/springframework/retry/policy/CompositeRetryPolicyTests.java +++ b/src/test/java/org/springframework/retry/policy/CompositeRetryPolicyTests.java @@ -160,4 +160,23 @@ public class CompositeRetryPolicyTests { assertThat(policy.canRetry(context)).isTrue(); } + @Test + public void testMaximumAttemptsForNonSuitablePolicies() { + CompositeRetryPolicy policy = new CompositeRetryPolicy(); + policy.setOptimistic(true); + policy.setPolicies(new RetryPolicy[] { new NeverRetryPolicy(), new NeverRetryPolicy() }); + + assertThat(policy.getMaxAttempts()).isEqualTo(RetryPolicy.NO_MAXIMUM_ATTEMPTS_SET); + } + + @Test + public void testMaximumAttemptsForSuitablePolicies() { + CompositeRetryPolicy policy = new CompositeRetryPolicy(); + policy.setOptimistic(true); + policy.setPolicies( + new RetryPolicy[] { new SimpleRetryPolicy(6), new SimpleRetryPolicy(3), new SimpleRetryPolicy(4) }); + + assertThat(policy.getMaxAttempts()).isEqualTo(3); + } + } diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java index 104e827..1828cb3 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java @@ -26,11 +26,13 @@ import org.springframework.classify.BinaryExceptionClassifier; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; import org.springframework.retry.RetryListener; +import org.springframework.retry.RetryPolicy; import org.springframework.retry.TerminatedRetryException; 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.policy.AlwaysRetryPolicy; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.policy.SimpleRetryPolicy; @@ -49,6 +51,7 @@ import static org.mockito.Mockito.verify; * @author Dave Syer * @author Gary Russell * @author Henning Pöttker + * @author Emanuele Ivaldi */ public class RetryTemplateTests { @@ -333,6 +336,30 @@ public class RetryTemplateTests { assertThat(callCount.get()).isEqualTo(2); } + @Test + public void testContextForPolicyWithMaximumNumberOfAttempts() throws Throwable { + RetryTemplate retryTemplate = new RetryTemplate(); + RetryPolicy retryPolicy = new SimpleRetryPolicy(2); + retryTemplate.setRetryPolicy(retryPolicy); + + Integer result = retryTemplate.execute((RetryCallback) context -> (Integer) context + .getAttribute(RetryContext.MAX_ATTEMPTS), context -> RetryPolicy.NO_MAXIMUM_ATTEMPTS_SET); + + assertThat(result).isEqualTo(2); + } + + @Test + public void testContextForPolicyWithNoMaximumNumberOfAttempts() throws Throwable { + RetryTemplate retryTemplate = new RetryTemplate(); + RetryPolicy retryPolicy = new AlwaysRetryPolicy(); + retryTemplate.setRetryPolicy(retryPolicy); + + Integer result = retryTemplate.execute((RetryCallback) context -> (Integer) context + .getAttribute(RetryContext.MAX_ATTEMPTS), context -> RetryPolicy.NO_MAXIMUM_ATTEMPTS_SET); + + assertThat(result).isEqualTo(RetryPolicy.NO_MAXIMUM_ATTEMPTS_SET); + } + private static class MockRetryCallback implements RetryCallback { private int attempts;