diff --git a/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java index def93cb7c2..bb253ffea3 100644 --- a/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java @@ -34,7 +34,6 @@ import org.springframework.core.retry.RetryPolicy; import org.springframework.core.retry.RetryTemplate; import org.springframework.core.retry.Retryable; import org.springframework.util.ClassUtils; -import org.springframework.util.backoff.ExponentialBackOff; /** * Abstract retry interceptor implementation, adapting a given @@ -89,26 +88,17 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { } } - RetryTemplate retryTemplate = new RetryTemplate(); - - RetryPolicy.Builder policyBuilder = RetryPolicy.builder(); - for (Class include : spec.includes()) { - policyBuilder.includes(include); - } - for (Class exclude : spec.excludes()) { - policyBuilder.excludes(exclude); - } - policyBuilder.predicate(spec.predicate().forMethod(method)); - policyBuilder.maxAttempts(spec.maxAttempts()); - retryTemplate.setRetryPolicy(policyBuilder.build()); - - ExponentialBackOff backOff = new ExponentialBackOff(); - backOff.setInitialInterval(spec.delay()); - backOff.setJitter(spec.jitter()); - backOff.setMultiplier(spec.multiplier()); - backOff.setMaxInterval(spec.maxDelay()); - backOff.setMaxAttempts(spec.maxAttempts()); - retryTemplate.setBackOffPolicy(backOff); + RetryPolicy retryPolicy = RetryPolicy.builder() + .includes(spec.includes()) + .excludes(spec.excludes()) + .predicate(spec.predicate().forMethod(method)) + .maxAttempts(spec.maxAttempts()) + .delay(Duration.ofMillis(spec.delay())) + .maxDelay(Duration.ofMillis(spec.maxDelay())) + .jitter(Duration.ofMillis(spec.jitter())) + .multiplier(spec.multiplier()) + .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); try { return retryTemplate.execute(new Retryable<>() { diff --git a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java index eb599cba00..7c697af0aa 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java @@ -16,15 +16,13 @@ package org.springframework.core.retry; -import java.time.Duration; -import java.time.LocalDateTime; import java.util.Set; import java.util.StringJoiner; import java.util.function.Predicate; import org.jspecify.annotations.Nullable; -import org.springframework.util.Assert; +import org.springframework.util.backoff.BackOff; /** * Default {@link RetryPolicy} created by {@link RetryPolicy.Builder}. @@ -35,44 +33,58 @@ import org.springframework.util.Assert; */ class DefaultRetryPolicy implements RetryPolicy { - private final int maxAttempts; - - private final @Nullable Duration maxDuration; - private final Set> includes; private final Set> excludes; private final @Nullable Predicate predicate; + private final BackOff backOff; - DefaultRetryPolicy(int maxAttempts, @Nullable Duration maxDuration, Set> includes, - Set> excludes, @Nullable Predicate predicate) { - Assert.isTrue((maxAttempts > 0 || maxDuration != null), "Max attempts or max duration must be specified"); - this.maxAttempts = maxAttempts; - this.maxDuration = maxDuration; + DefaultRetryPolicy(Set> includes, Set> excludes, + @Nullable Predicate predicate, BackOff backOff) { + this.includes = includes; this.excludes = excludes; this.predicate = predicate; + this.backOff = backOff; } @Override - public RetryExecution start() { - return new DefaultRetryPolicyExecution(); + public boolean shouldRetry(Throwable throwable) { + if (!this.excludes.isEmpty()) { + for (Class excludedType : this.excludes) { + if (excludedType.isInstance(throwable)) { + return false; + } + } + } + if (!this.includes.isEmpty()) { + boolean included = false; + for (Class includedType : this.includes) { + if (includedType.isInstance(throwable)) { + included = true; + break; + } + } + if (!included) { + return false; + } + } + return this.predicate == null || this.predicate.test(throwable); + } + + @Override + public BackOff getBackOff() { + return this.backOff; } @Override public String toString() { StringJoiner result = new StringJoiner(", ", "DefaultRetryPolicy[", "]"); - if (this.maxAttempts > 0) { - result.add("maxAttempts=" + this.maxAttempts); - } - if (this.maxDuration != null) { - result.add("maxDuration=" + this.maxDuration.toMillis() + "ms"); - } if (!this.includes.isEmpty()) { result.add("includes=" + names(this.includes)); } @@ -82,6 +94,7 @@ class DefaultRetryPolicy implements RetryPolicy { if (this.predicate != null) { result.add("predicate=" + this.predicate.getClass().getSimpleName()); } + result.add("backOff=" + this.backOff); return result.toString(); } @@ -95,73 +108,4 @@ class DefaultRetryPolicy implements RetryPolicy { return result.toString(); } - - /** - * {@link RetryExecution} for {@link DefaultRetryPolicy}. - */ - private class DefaultRetryPolicyExecution implements RetryExecution { - - private final LocalDateTime retryStartTime = LocalDateTime.now(); - - private int retryCount; - - - @Override - public boolean shouldRetry(Throwable throwable) { - if (DefaultRetryPolicy.this.maxAttempts > 0 && - this.retryCount++ >= DefaultRetryPolicy.this.maxAttempts) { - return false; - } - if (DefaultRetryPolicy.this.maxDuration != null) { - Duration retryDuration = Duration.between(this.retryStartTime, LocalDateTime.now()); - if (retryDuration.compareTo(DefaultRetryPolicy.this.maxDuration) > 0) { - return false; - } - } - if (!DefaultRetryPolicy.this.excludes.isEmpty()) { - for (Class excludedType : DefaultRetryPolicy.this.excludes) { - if (excludedType.isInstance(throwable)) { - return false; - } - } - } - if (!DefaultRetryPolicy.this.includes.isEmpty()) { - boolean included = false; - for (Class includedType : DefaultRetryPolicy.this.includes) { - if (includedType.isInstance(throwable)) { - included = true; - break; - } - } - if (!included) { - return false; - } - } - return DefaultRetryPolicy.this.predicate == null || DefaultRetryPolicy.this.predicate.test(throwable); - } - - @Override - public String toString() { - StringJoiner result = new StringJoiner(", ", "DefaultRetryPolicyExecution[", "]"); - if (DefaultRetryPolicy.this.maxAttempts > 0) { - result.add("maxAttempts=" + DefaultRetryPolicy.this.maxAttempts); - result.add("retryCount=" + this.retryCount); - } - if (DefaultRetryPolicy.this.maxDuration != null) { - result.add("maxDuration=" + DefaultRetryPolicy.this.maxDuration.toMillis() + "ms"); - result.add("retryStartTime=" + this.retryStartTime); - } - if (!DefaultRetryPolicy.this.includes.isEmpty()) { - result.add("includes=" + names(DefaultRetryPolicy.this.includes)); - } - if (!DefaultRetryPolicy.this.excludes.isEmpty()) { - result.add("excludes=" + names(DefaultRetryPolicy.this.excludes)); - } - if (DefaultRetryPolicy.this.predicate != null) { - result.add("predicate=" + DefaultRetryPolicy.this.predicate.getClass().getSimpleName()); - } - return result.toString(); - } - } - } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryExecution.java b/spring-core/src/main/java/org/springframework/core/retry/RetryExecution.java deleted file mode 100644 index 3a1bce777a..0000000000 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryExecution.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright 2002-present 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 - * - * https://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.core.retry; - -/** - * Strategy interface to define a retry execution created for a given - * {@link RetryPolicy}. - * - *

A {@code RetryExecution} is effectively an executable instance of a given - * {@code RetryPolicy}. - * - *

Implementations may be stateful but do not need to be thread-safe. - * - * @author Mahmoud Ben Hassine - * @since 7.0 - */ -public interface RetryExecution { - - /** - * Specify if the operation should be retried based on the given throwable. - * @param throwable the exception that caused the operation to fail - * @return {@code true} if the operation should be retried, {@code false} otherwise - */ - boolean shouldRetry(Throwable throwable); - -} diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java b/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java index 3f7fdaf7a3..7b0946122c 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java @@ -21,12 +21,14 @@ import org.jspecify.annotations.Nullable; import org.springframework.core.retry.support.CompositeRetryListener; /** - * An extension point that allows to inject code during key retry phases. + * {@code RetryListener} defines a listener API for reacting to events + * published during the execution of a {@link Retryable} operation. * *

Typically registered in a {@link RetryTemplate}, and can be composed using * a {@link CompositeRetryListener}. * * @author Mahmoud Ben Hassine + * @author Sam Brannen * @since 7.0 * @see CompositeRetryListener */ @@ -34,33 +36,37 @@ public interface RetryListener { /** * Called before every retry attempt. - * @param retryExecution the retry execution + * @param retryPolicy the {@link RetryPolicy} + * @param retryable the {@link Retryable} operation */ - default void beforeRetry(RetryExecution retryExecution) { + default void beforeRetry(RetryPolicy retryPolicy, Retryable retryable) { } /** * Called after the first successful retry attempt. - * @param retryExecution the retry execution - * @param result the result of the {@link Retryable} + * @param retryPolicy the {@link RetryPolicy} + * @param retryable the {@link Retryable} operation + * @param result the result of the {@code Retryable} operation */ - default void onRetrySuccess(RetryExecution retryExecution, @Nullable Object result) { + default void onRetrySuccess(RetryPolicy retryPolicy, Retryable retryable, @Nullable Object result) { } /** * Called every time a retry attempt fails. - * @param retryExecution the retry execution - * @param throwable the exception thrown by the {@link Retryable} + * @param retryPolicy the {@link RetryPolicy} + * @param retryable the {@link Retryable} operation + * @param throwable the exception thrown by the {@code Retryable} operation */ - default void onRetryFailure(RetryExecution retryExecution, Throwable throwable) { + default void onRetryFailure(RetryPolicy retryPolicy, Retryable retryable, Throwable throwable) { } /** * Called if the {@link RetryPolicy} is exhausted. - * @param retryExecution the retry execution - * @param throwable the last exception thrown by the {@link Retryable} + * @param retryPolicy the {@code RetryPolicy} + * @param retryable the {@code Retryable} operation + * @param throwable the last exception thrown by the {@link Retryable} operation */ - default void onRetryPolicyExhaustion(RetryExecution retryExecution, Throwable throwable) { + default void onRetryPolicyExhaustion(RetryPolicy retryPolicy, Retryable retryable, Throwable throwable) { } } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java index 6f6c34dd11..018731119b 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java @@ -17,6 +17,7 @@ package org.springframework.core.retry; import java.time.Duration; +import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; @@ -25,45 +26,82 @@ import java.util.function.Predicate; import org.jspecify.annotations.Nullable; import org.springframework.util.Assert; +import org.springframework.util.backoff.BackOff; +import org.springframework.util.backoff.ExponentialBackOff; +import org.springframework.util.backoff.FixedBackOff; /** * Strategy interface to define a retry policy. * *

Also provides factory methods and a fluent builder API for creating retry - * policies with common configurations. See {@link #withMaxAttempts(int)}, - * {@link #withMaxDuration(Duration)}, {@link #builder()}, and the configuration - * options in {@link Builder} for details. + * policies with common configurations. See {@link #withDefaults()}, + * {@link #withMaxAttempts(int)}, {@link #withMaxElapsedTime(Duration)}, + * {@link #builder()}, and the configuration options in {@link Builder} for details. * * @author Sam Brannen * @author Mahmoud Ben Hassine * @since 7.0 - * @see RetryExecution + * @see Retryable + * @see RetryTemplate + * @see BackOff */ public interface RetryPolicy { /** - * Start a new execution for this retry policy. - * @return a new {@link RetryExecution} + * Specify if the {@link Retryable} operation should be retried based on the + * given throwable. + * @param throwable the exception that caused the operation to fail + * @return {@code true} if the operation should be retried, {@code false} otherwise */ - RetryExecution start(); + boolean shouldRetry(Throwable throwable); /** - * Create a {@link RetryPolicy} configured with a maximum number of retry attempts. - * @param maxAttempts the maximum number of retry attempts; must be greater than zero - * @see Builder#maxAttempts(int) + * Get the {@link BackOff} strategy to use for this retry policy. + *

Defaults to a fixed backoff of {@value Builder#DEFAULT_DELAY} milliseconds + * and maximum {@value Builder#DEFAULT_MAX_ATTEMPTS} retry attempts. + * @return the {@code BackOff} strategy to use + * @see FixedBackOff */ - static RetryPolicy withMaxAttempts(int maxAttempts) { - return builder().maxAttempts(maxAttempts).build(); + default BackOff getBackOff() { + return new FixedBackOff(Builder.DEFAULT_DELAY, Builder.DEFAULT_MAX_ATTEMPTS); + } + + + /** + * Create a {@link RetryPolicy} with default configuration. + *

The returned policy applies to all exception types, uses a fixed backoff + * of {@value Builder#DEFAULT_DELAY} milliseconds, and supports maximum + * {@value Builder#DEFAULT_MAX_ATTEMPTS} retry attempts. + * @see FixedBackOff + */ + static RetryPolicy withDefaults() { + return throwable -> true; } /** - * Create a {@link RetryPolicy} configured with a maximum retry {@link Duration}. - * @param maxDuration the maximum retry duration; must be positive - * @see Builder#maxDuration(Duration) + * Create a {@link RetryPolicy} configured with a maximum number of retry attempts. + *

The returned policy uses a fixed backoff of {@value Builder#DEFAULT_DELAY} + * milliseconds. + * @param maxAttempts the maximum number of retry attempts; must be greater than zero + * @see Builder#maxAttempts(int) + * @see FixedBackOff */ - static RetryPolicy withMaxDuration(Duration maxDuration) { - return builder().maxDuration(maxDuration).build(); + static RetryPolicy withMaxAttempts(int maxAttempts) { + Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero"); + return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build(); + } + + /** + * Create a {@link RetryPolicy} configured with a maximum elapsed time. + *

The returned policy uses a fixed backoff of {@value Builder#DEFAULT_DELAY} + * milliseconds. + * @param maxElapsedTime the maximum elapsed time; must be positive + * @see Builder#maxElapsedTime(Duration) + * @see FixedBackOff + */ + static RetryPolicy withMaxElapsedTime(Duration maxElapsedTime) { + return builder().maxElapsedTime(maxElapsedTime).build(); } /** @@ -81,9 +119,41 @@ public interface RetryPolicy { */ final class Builder { + /** + * The default {@linkplain #maxAttempts(int) max attempts}: {@value}. + */ + public static final int DEFAULT_MAX_ATTEMPTS = 3; + + /** + * The default {@linkplain #delay(Duration) delay}: {@value} ms. + */ + public static final long DEFAULT_DELAY = 1000; + + /** + * The default {@linkplain #maxDelay(Duration) max delay}: {@value} ms. + * @see Long#MAX_VALUE + */ + public static final long DEFAULT_MAX_DELAY = Long.MAX_VALUE; + + /** + * The default {@linkplain #multiplier(double) multiplier}: {@value}. + */ + public static final double DEFAULT_MULTIPLIER = 1.0; + + + private @Nullable BackOff backOff; + private int maxAttempts; - private @Nullable Duration maxDuration; + private @Nullable Duration delay; + + private @Nullable Duration jitter; + + private double multiplier; + + private @Nullable Duration maxDelay; + + private @Nullable Duration maxElapsedTime; private final Set> includes = new LinkedHashSet<>(); @@ -97,10 +167,30 @@ public interface RetryPolicy { } + /** + * Specify the {@link BackOff} strategy to use. + *

The supplied value will override any previously configured value. + *

WARNING: If you configure a custom {@code BackOff} + * strategy, you should not configure any of the following: + * {@link #maxAttempts(int) maxAttempts}, {@link #delay(Duration) delay}, + * {@link #jitter(Duration) jitter}, {@link #multiplier(double) multiplier}, + * {@link #maxDelay(Duration) maxDelay}, or {@link #maxElapsedTime(Duration) + * maxElapsedTime}. + * @param backOff the {@code BackOff} strategy + * @return this {@code Builder} instance for chained method invocations + */ + public Builder backOff(BackOff backOff) { + Assert.notNull(backOff, "BackOff must not be null"); + this.backOff = backOff; + return this; + } + /** * Specify the maximum number of retry attempts. - *

If a {@code maxAttempts} value has already been configured, the - * supplied value will override the existing value. + *

The default is {@value #DEFAULT_MAX_ATTEMPTS}. + *

The supplied value will override any previously configured value. + *

You should not specify this configuration option if you have + * configured a custom {@link #backOff(BackOff) BackOff} strategy. * @param maxAttempts the maximum number of retry attempts; must be * greater than zero * @return this {@code Builder} instance for chained method invocations @@ -112,15 +202,106 @@ public interface RetryPolicy { } /** - * Specify the maximum retry {@link Duration}. - *

If a {@code maxDuration} value has already been configured, the - * supplied value will override the existing value. - * @param maxDuration the maximum retry duration; must be positive + * Specify the base delay after the initial invocation. + *

If a {@linkplain #multiplier(double) multiplier} is specified, this + * serves as the initial delay to multiply from. + *

The default is {@value #DEFAULT_DELAY} milliseconds. + *

The supplied value will override any previously configured value. + *

You should not specify this configuration option if you have + * configured a custom {@link #backOff(BackOff) BackOff} strategy. + * @param delay the base delay, typically in milliseconds or seconds; + * must be positive + * @return this {@code Builder} instance for chained method invocations + * @see #jitter(Duration) + * @see #multiplier(double) + * @see #maxDelay(Duration) + */ + public Builder delay(Duration delay) { + assertIsPositive("delay", delay); + this.delay = delay; + return this; + } + + /** + * Specify a jitter value for the base retry attempt, randomly subtracted + * or added to the calculated delay, resulting in a value between + * {@code delay - jitter} and {@code delay + jitter} but never below the + * {@linkplain #delay(Duration) base delay} or above the + * {@linkplain #maxDelay(Duration) max delay}. + *

If a {@linkplain #multiplier(double) multiplier} is specified, it + * is applied to the jitter value as well. + *

The default is no jitter. + *

The supplied value will override any previously configured value. + *

You should not specify this configuration option if you have + * configured a custom {@link #backOff(BackOff) BackOff} strategy. + * @param jitter the jitter value, typically in milliseconds; must be positive + * @return this {@code Builder} instance for chained method invocations + * @see #delay(Duration) + * @see #multiplier(double) + * @see #maxDelay(Duration) + */ + public Builder jitter(Duration jitter) { + Assert.isTrue(!jitter.isNegative(), + () -> "Invalid jitter (%dms): must be >= 0.".formatted(jitter.toMillis())); + this.jitter = jitter; + return this; + } + + /** + * Specify a multiplier for a delay for the next retry attempt, applied + * to the previous delay (starting with the initial + * {@linkplain #delay(Duration) delay}) as well as to the applicable + * {@linkplain #jitter(Duration) jitter} for each attempt. + *

The default is {@value Builder#DEFAULT_MULTIPLIER}, effectively + * resulting in a fixed delay. + *

The supplied value will override any previously configured value. + *

You should not specify this configuration option if you have + * configured a custom {@link #backOff(BackOff) BackOff} strategy. + * @return this {@code Builder} instance for chained method invocations + * @see #delay(Duration) + * @see #jitter(Duration) + * @see #maxDelay(Duration) + */ + public Builder multiplier(double multiplier) { + Assert.isTrue(multiplier >= 1, () -> "Invalid multiplier '" + multiplier + "': " + + "must be greater than or equal to 1. A multiplier of 1 is equivalent to a fixed delay."); + this.multiplier = multiplier; + return this; + } + + /** + * Specify the maximum delay for any retry attempt, limiting how far + * {@linkplain #jitter(Duration) jitter} and the + * {@linkplain #multiplier(double) multiplier} can increase the + * {@linkplain #delay(Duration) delay}. + *

The default is unlimited. + *

The supplied value will override any previously configured value. + *

You should not specify this configuration option if you have + * configured a custom {@link #backOff(BackOff) BackOff} strategy. + * @param maxDelay the maximum delay; must be positive + * @return this {@code Builder} instance for chained method invocations + * @see #delay(Duration) + * @see #jitter(Duration) + * @see #multiplier(double) + */ + public Builder maxDelay(Duration maxDelay) { + assertIsPositive("max delay", maxDelay); + this.maxDelay = maxDelay; + return this; + } + + /** + * Specify the maximum elapsed time. + *

The default is unlimited. + *

The supplied value will override any previously configured value. + *

You should not specify this configuration option if you have + * configured a custom {@link #backOff(BackOff) BackOff} strategy. + * @param maxElapsedTime the maximum elapsed time; must be positive * @return this {@code Builder} instance for chained method invocations */ - public Builder maxDuration(Duration maxDuration) { - Assert.isTrue(!maxDuration.isNegative() && !maxDuration.isZero(), "Max duration must be positive"); - this.maxDuration = maxDuration; + public Builder maxElapsedTime(Duration maxElapsedTime) { + assertIsPositive("max elapsed time", maxElapsedTime); + this.maxElapsedTime = maxElapsedTime; return this; } @@ -130,10 +311,14 @@ public interface RetryPolicy { *

Defaults to all exception types. *

If included exception types have already been configured, the supplied * types will be added to the existing list of included types. - *

This can be combined with {@link #excludes(Class...)} and - * {@link #predicate(Predicate)}. + *

This can be combined with other {@code includes}, {@code excludes}, + * and a custom {@code predicate}. * @param types the types of exceptions to include in the policy * @return this {@code Builder} instance for chained method invocations + * @see #includes(Collection) + * @see #excludes(Class...) + * @see #excludes(Collection) + * @see #predicate(Predicate) */ @SafeVarargs // Making the method final allows us to use @SafeVarargs. @SuppressWarnings("varargs") @@ -142,15 +327,39 @@ public interface RetryPolicy { return this; } + /** + * Specify the types of exceptions for which the {@link RetryPolicy} + * should retry a failed operation. + *

Defaults to all exception types. + *

If included exception types have already been configured, the supplied + * types will be added to the existing list of included types. + *

This can be combined with other {@code includes}, {@code excludes}, + * and a custom {@code predicate}. + * @param types the types of exceptions to include in the policy + * @return this {@code Builder} instance for chained method invocations + * @see #includes(Class...) + * @see #excludes(Class...) + * @see #excludes(Collection) + * @see #predicate(Predicate) + */ + public Builder includes(Collection> types) { + this.includes.addAll(types); + return this; + } + /** * Specify the types of exceptions for which the {@link RetryPolicy} * should not retry a failed operation. *

If excluded exception types have already been configured, the supplied * types will be added to the existing list of excluded types. - *

This can be combined with {@link #includes(Class...)} and - * {@link #predicate(Predicate)}. + *

This can be combined with {@code includes}, other {@code excludes}, + * and a custom {@code predicate}. * @param types the types of exceptions to exclude from the policy * @return this {@code Builder} instance for chained method invocations + * @see #includes(Class...) + * @see #includes(Collection) + * @see #excludes(Collection) + * @see #predicate(Predicate) */ @SafeVarargs // Making the method final allows us to use @SafeVarargs. @SuppressWarnings("varargs") @@ -159,6 +368,25 @@ public interface RetryPolicy { return this; } + /** + * Specify the types of exceptions for which the {@link RetryPolicy} + * should not retry a failed operation. + *

If excluded exception types have already been configured, the supplied + * types will be added to the existing list of excluded types. + *

This can be combined with {@code includes}, other {@code excludes}, + * and a custom {@code predicate}. + * @param types the types of exceptions to exclude from the policy + * @return this {@code Builder} instance for chained method invocations + * @see #includes(Class...) + * @see #includes(Collection) + * @see #excludes(Class...) + * @see #predicate(Predicate) + */ + public Builder excludes(Collection> types) { + this.excludes.addAll(types); + return this; + } + /** * Specify a custom {@link Predicate} that the {@link RetryPolicy} will * use to determine whether to retry a failed operation based on a given @@ -166,10 +394,13 @@ public interface RetryPolicy { *

If a predicate has already been configured, the supplied predicate * will be {@linkplain Predicate#and(Predicate) combined} with the * existing predicate. - *

This can be combined with {@link #includes(Class...)} and - * {@link #excludes(Class...)}. + *

This can be combined with {@code includes} and {@code excludes}. * @param predicate a custom predicate * @return this {@code Builder} instance for chained method invocations + * @see #includes(Class...) + * @see #includes(Collection) + * @see #excludes(Class...) + * @see #excludes(Collection) */ public Builder predicate(Predicate predicate) { this.predicate = (this.predicate != null ? this.predicate.and(predicate) : predicate); @@ -177,11 +408,37 @@ public interface RetryPolicy { } /** - * Build the {@link RetryPolicy} configured via this {@code Builder}. + * Build the configured {@link RetryPolicy}. */ public RetryPolicy build() { - return new DefaultRetryPolicy(this.maxAttempts, this.maxDuration, - this.includes, this.excludes, this.predicate); + BackOff backOff = this.backOff; + if (backOff != null) { + boolean misconfigured = (this.maxAttempts != 0) || (this.delay != null) || (this.jitter != null) || + (this.multiplier != 0) || (this.maxDelay != null) || (this.maxElapsedTime != null); + Assert.state(!misconfigured, """ + The following configuration options are not supported with a custom BackOff strategy: \ + maxAttempts, delay, jitter, multiplier, maxDelay, or maxElapsedTime."""); + } + else { + ExponentialBackOff exponentialBackOff = new ExponentialBackOff(); + exponentialBackOff.setMaxAttempts(this.maxAttempts > 0 ? this.maxAttempts : DEFAULT_MAX_ATTEMPTS); + exponentialBackOff.setInitialInterval(this.delay != null ? this.delay.toMillis() : DEFAULT_DELAY); + exponentialBackOff.setMaxInterval(this.maxDelay != null ? this.maxDelay.toMillis() : DEFAULT_MAX_DELAY); + exponentialBackOff.setMultiplier(this.multiplier > 1 ? this.multiplier : DEFAULT_MULTIPLIER); + if (this.jitter != null) { + exponentialBackOff.setJitter(this.jitter.toMillis()); + } + if (this.maxElapsedTime != null) { + exponentialBackOff.setMaxElapsedTime(this.maxElapsedTime.toMillis()); + } + backOff = exponentialBackOff; + } + return new DefaultRetryPolicy(this.includes, this.excludes, this.predicate, backOff); + } + + private static void assertIsPositive(String name, Duration duration) { + Assert.isTrue((!duration.isNegative() && !duration.isZero()), + () -> "Invalid duration (%dms): %s must be positive.".formatted(duration.toMillis(), name)); } } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java index 6e78284394..09329303f5 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java @@ -26,18 +26,16 @@ import org.springframework.core.log.LogAccessor; import org.springframework.util.Assert; import org.springframework.util.backoff.BackOff; import org.springframework.util.backoff.BackOffExecution; -import org.springframework.util.backoff.FixedBackOff; /** * A basic implementation of {@link RetryOperations} that executes and potentially - * retries a {@link Retryable} operation based on a configured {@link RetryPolicy} - * and {@link BackOff} policy. + * retries a {@link Retryable} operation based on a configured {@link RetryPolicy}. * *

By default, a retryable operation will be retried at most 3 times with a * fixed backoff of 1 second. * *

A {@link RetryListener} can be {@linkplain #setRetryListener(RetryListener) - * registered} to intercept and inject behavior during key retry phases (before a + * registered} to react to events published during key retry phases (before a * retry attempt, after a retry attempt, etc.). * *

All retry actions performed by this template are logged at debug level, using @@ -58,9 +56,7 @@ public class RetryTemplate implements RetryOperations { private static final LogAccessor logger = new LogAccessor(RetryTemplate.class); - private RetryPolicy retryPolicy = RetryPolicy.withMaxAttempts(3); - - private BackOff backOffPolicy = new FixedBackOff(Duration.ofSeconds(1)); + private RetryPolicy retryPolicy = RetryPolicy.withDefaults(); private RetryListener retryListener = new RetryListener() {}; @@ -68,13 +64,13 @@ public class RetryTemplate implements RetryOperations { /** * Create a new {@code RetryTemplate} with maximum 3 retry attempts and a * fixed backoff of 1 second. + * @see RetryPolicy#withDefaults() */ public RetryTemplate() { } /** - * Create a new {@code RetryTemplate} with a custom {@link RetryPolicy} and a - * fixed backoff of 1 second. + * Create a new {@code RetryTemplate} with the supplied {@link RetryPolicy}. * @param retryPolicy the retry policy to use */ public RetryTemplate(RetryPolicy retryPolicy) { @@ -82,25 +78,14 @@ public class RetryTemplate implements RetryOperations { this.retryPolicy = retryPolicy; } - /** - * Create a new {@code RetryTemplate} with a custom {@link RetryPolicy} and - * {@link BackOff} policy. - * @param retryPolicy the retry policy to use - * @param backOffPolicy the backoff policy to use - */ - public RetryTemplate(RetryPolicy retryPolicy, BackOff backOffPolicy) { - this(retryPolicy); - Assert.notNull(backOffPolicy, "BackOff policy must not be null"); - this.backOffPolicy = backOffPolicy; - } - /** * Set the {@link RetryPolicy} to use. - *

Defaults to {@code RetryPolicy.withMaxAttempts(3)}. + *

Defaults to {@code RetryPolicy.withDefaults()}. * @param retryPolicy the retry policy to use + * @see RetryPolicy#withDefaults() * @see RetryPolicy#withMaxAttempts(int) - * @see RetryPolicy#withMaxDuration(Duration) + * @see RetryPolicy#withMaxElapsedTime(Duration) * @see RetryPolicy#builder() */ public void setRetryPolicy(RetryPolicy retryPolicy) { @@ -108,17 +93,6 @@ public class RetryTemplate implements RetryOperations { this.retryPolicy = retryPolicy; } - /** - * Set the {@link BackOff} policy to use. - *

Defaults to {@code new FixedBackOff(Duration.ofSeconds(1))}. - * @param backOffPolicy the backoff policy to use - * @see FixedBackOff - */ - public void setBackOffPolicy(BackOff backOffPolicy) { - Assert.notNull(backOffPolicy, "BackOff policy must not be null"); - this.backOffPolicy = backOffPolicy; - } - /** * Set the {@link RetryListener} to use. *

If multiple listeners are needed, use a @@ -157,13 +131,12 @@ public class RetryTemplate implements RetryOperations { () -> "Execution of retryable operation '%s' failed; initiating the retry process" .formatted(retryableName)); // Retry process starts here - RetryExecution retryExecution = this.retryPolicy.start(); - BackOffExecution backOffExecution = this.backOffPolicy.start(); + BackOffExecution backOffExecution = this.retryPolicy.getBackOff().start(); Deque exceptions = new ArrayDeque<>(); exceptions.add(initialException); Throwable retryException = initialException; - while (retryExecution.shouldRetry(retryException)) { + while (this.retryPolicy.shouldRetry(retryException)) { try { long duration = backOffExecution.nextBackOff(); if (duration == BackOffExecution.STOP) { @@ -181,9 +154,9 @@ public class RetryTemplate implements RetryOperations { } logger.debug(() -> "Preparing to retry operation '%s'".formatted(retryableName)); try { - this.retryListener.beforeRetry(retryExecution); + this.retryListener.beforeRetry(this.retryPolicy, retryable); R result = retryable.execute(); - this.retryListener.onRetrySuccess(retryExecution, result); + this.retryListener.onRetrySuccess(this.retryPolicy, retryable, result); logger.debug(() -> "Retryable operation '%s' completed successfully after retry" .formatted(retryableName)); return result; @@ -191,7 +164,7 @@ public class RetryTemplate implements RetryOperations { catch (Throwable currentAttemptException) { logger.debug(() -> "Retry attempt for operation '%s' failed due to '%s'" .formatted(retryableName, currentAttemptException)); - this.retryListener.onRetryFailure(retryExecution, currentAttemptException); + this.retryListener.onRetryFailure(this.retryPolicy, retryable, currentAttemptException); exceptions.add(currentAttemptException); retryException = currentAttemptException; } @@ -203,7 +176,7 @@ public class RetryTemplate implements RetryOperations { "Retry policy for operation '%s' exhausted; aborting execution".formatted(retryableName), exceptions.removeLast()); exceptions.forEach(finalException::addSuppressed); - this.retryListener.onRetryPolicyExhaustion(retryExecution, finalException); + this.retryListener.onRetryPolicyExhaustion(this.retryPolicy, retryable, finalException); throw finalException; } } diff --git a/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java b/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java index 0eb25852ce..f71d94d5f8 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java +++ b/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java @@ -21,9 +21,10 @@ import java.util.List; import org.jspecify.annotations.Nullable; -import org.springframework.core.retry.RetryExecution; import org.springframework.core.retry.RetryListener; +import org.springframework.core.retry.RetryPolicy; import org.springframework.core.retry.RetryTemplate; +import org.springframework.core.retry.Retryable; import org.springframework.util.Assert; /** @@ -66,23 +67,23 @@ public class CompositeRetryListener implements RetryListener { @Override - public void beforeRetry(RetryExecution retryExecution) { - this.listeners.forEach(retryListener -> retryListener.beforeRetry(retryExecution)); + public void beforeRetry(RetryPolicy retryPolicy, Retryable retryable) { + this.listeners.forEach(retryListener -> retryListener.beforeRetry(retryPolicy, retryable)); } @Override - public void onRetrySuccess(RetryExecution retryExecution, @Nullable Object result) { - this.listeners.forEach(listener -> listener.onRetrySuccess(retryExecution, result)); + public void onRetrySuccess(RetryPolicy retryPolicy, Retryable retryable, @Nullable Object result) { + this.listeners.forEach(listener -> listener.onRetrySuccess(retryPolicy, retryable, result)); } @Override - public void onRetryFailure(RetryExecution retryExecution, Throwable throwable) { - this.listeners.forEach(listener -> listener.onRetryFailure(retryExecution, throwable)); + public void onRetryFailure(RetryPolicy retryPolicy, Retryable retryable, Throwable throwable) { + this.listeners.forEach(listener -> listener.onRetryFailure(retryPolicy, retryable, throwable)); } @Override - public void onRetryPolicyExhaustion(RetryExecution retryExecution, Throwable throwable) { - this.listeners.forEach(listener -> listener.onRetryPolicyExhaustion(retryExecution, throwable)); + public void onRetryPolicyExhaustion(RetryPolicy retryPolicy, Retryable retryable, Throwable throwable) { + this.listeners.forEach(listener -> listener.onRetryPolicyExhaustion(retryPolicy, retryable, throwable)); } } diff --git a/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsDefaultRetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsDefaultRetryPolicyTests.java deleted file mode 100644 index 7027d4ed70..0000000000 --- a/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsDefaultRetryPolicyTests.java +++ /dev/null @@ -1,188 +0,0 @@ -/* - * Copyright 2002-present 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 - * - * https://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.core.retry; - -import java.io.FileNotFoundException; -import java.io.IOException; -import java.nio.file.FileSystemException; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.mockito.Mockito.mock; - -/** - * Max attempts tests for {@link DefaultRetryPolicy} and its {@link RetryExecution}. - * - * @author Mahmoud Ben Hassine - * @author Sam Brannen - * @since 7.0 - */ -class MaxAttemptsDefaultRetryPolicyTests { - - @Test - void invalidMaxAttempts() { - assertThatIllegalArgumentException() - .isThrownBy(() -> RetryPolicy.withMaxAttempts(0)) - .withMessage("Max attempts must be greater than zero"); - assertThatIllegalArgumentException() - .isThrownBy(() -> RetryPolicy.withMaxAttempts(-1)) - .withMessage("Max attempts must be greater than zero"); - } - - @Test - void maxAttempts() { - var retryPolicy = RetryPolicy.withMaxAttempts(2); - var retryExecution = retryPolicy.start(); - var throwable = mock(Throwable.class); - - assertThat(retryExecution.shouldRetry(throwable)).isTrue(); - assertThat(retryExecution.shouldRetry(throwable)).isTrue(); - - assertThat(retryExecution.shouldRetry(throwable)).isFalse(); - assertThat(retryExecution.shouldRetry(throwable)).isFalse(); - } - - @Test - void maxAttemptsAndPredicate() { - var retryPolicy = RetryPolicy.builder() - .maxAttempts(4) - .predicate(NumberFormatException.class::isInstance) - .build(); - - var retryExecution = retryPolicy.start(); - - // 4 retries - assertThat(retryExecution.shouldRetry(new NumberFormatException())).isTrue(); - assertThat(retryExecution.shouldRetry(new IllegalStateException())).isFalse(); - assertThat(retryExecution.shouldRetry(new IllegalStateException())).isFalse(); - assertThat(retryExecution.shouldRetry(new CustomNumberFormatException())).isTrue(); - - // After policy exhaustion - assertThat(retryExecution.shouldRetry(new NumberFormatException())).isFalse(); - assertThat(retryExecution.shouldRetry(new IllegalStateException())).isFalse(); - } - - @Test - void maxAttemptsWithIncludesAndExcludes() { - var policy = RetryPolicy.builder() - .maxAttempts(6) - .includes(RuntimeException.class, IOException.class) - .excludes(FileNotFoundException.class, CustomFileSystemException.class) - .build(); - - var retryExecution = policy.start(); - - // 6 retries - assertThat(retryExecution.shouldRetry(new IOException())).isTrue(); - assertThat(retryExecution.shouldRetry(new RuntimeException())).isTrue(); - assertThat(retryExecution.shouldRetry(new FileNotFoundException())).isFalse(); - assertThat(retryExecution.shouldRetry(new FileSystemException("file"))).isTrue(); - assertThat(retryExecution.shouldRetry(new CustomFileSystemException("file"))).isFalse(); - assertThat(retryExecution.shouldRetry(new IOException())).isTrue(); - - // After policy exhaustion - assertThat(retryExecution.shouldRetry(new IOException())).isFalse(); - } - - @Test - void toStringImplementations() { - var policy = RetryPolicy.builder() - .maxAttempts(6) - .includes(RuntimeException.class, IOException.class) - .excludes(FileNotFoundException.class, CustomFileSystemException.class) - .build(); - - assertThat(policy).asString().isEqualTo(""" - DefaultRetryPolicy[\ - maxAttempts=6, \ - includes=[java.lang.RuntimeException, java.io.IOException], \ - excludes=[java.io.FileNotFoundException, \ - org.springframework.core.retry.MaxAttemptsDefaultRetryPolicyTests.CustomFileSystemException]]"""); - - var template = """ - DefaultRetryPolicyExecution[\ - maxAttempts=6, \ - retryCount=%d, \ - includes=[java.lang.RuntimeException, java.io.IOException], \ - excludes=[java.io.FileNotFoundException, \ - org.springframework.core.retry.MaxAttemptsDefaultRetryPolicyTests.CustomFileSystemException]]"""; - var retryExecution = policy.start(); - var count = 0; - - assertThat(retryExecution).asString().isEqualTo(template, count++); - retryExecution.shouldRetry(new IOException()); - assertThat(retryExecution).asString().isEqualTo(template, count++); - retryExecution.shouldRetry(new IOException()); - assertThat(retryExecution).asString().isEqualTo(template, count++); - } - - @Test - void toStringImplementationsWithPredicateAsClass() { - var policy = RetryPolicy.builder() - .maxAttempts(1) - .predicate(new NumberFormatExceptionMatcher()) - .build(); - assertThat(policy).asString() - .isEqualTo("DefaultRetryPolicy[maxAttempts=1, predicate=NumberFormatExceptionMatcher]"); - - var retryExecution = policy.start(); - assertThat(retryExecution).asString() - .isEqualTo("DefaultRetryPolicyExecution[maxAttempts=1, retryCount=0, predicate=NumberFormatExceptionMatcher]"); - } - - @Test - void toStringImplementationsWithPredicateAsLambda() { - var policy = RetryPolicy.builder() - .maxAttempts(2) - .predicate(NumberFormatException.class::isInstance) - .build(); - assertThat(policy).asString() - .matches("DefaultRetryPolicy\\[maxAttempts=2, predicate=MaxAttemptsDefaultRetryPolicyTests.+?Lambda.+?]"); - - var retryExecution = policy.start(); - assertThat(retryExecution).asString() - .matches("DefaultRetryPolicyExecution\\[maxAttempts=2, retryCount=0, predicate=MaxAttemptsDefaultRetryPolicyTests.+?Lambda.+?]"); - - retryExecution.shouldRetry(new NumberFormatException()); - assertThat(retryExecution).asString() - .matches("DefaultRetryPolicyExecution\\[maxAttempts=2, retryCount=1, predicate=MaxAttemptsDefaultRetryPolicyTests.+?Lambda.+?]"); - - retryExecution.shouldRetry(new NumberFormatException()); - assertThat(retryExecution).asString() - .matches("DefaultRetryPolicyExecution\\[maxAttempts=2, retryCount=2, predicate=MaxAttemptsDefaultRetryPolicyTests.+?Lambda.+?]"); - - retryExecution.shouldRetry(new NumberFormatException()); - assertThat(retryExecution).asString() - .matches("DefaultRetryPolicyExecution\\[maxAttempts=2, retryCount=3, predicate=MaxAttemptsDefaultRetryPolicyTests.+?Lambda.+?]"); - } - - - @SuppressWarnings("serial") - private static class CustomNumberFormatException extends NumberFormatException { - } - - @SuppressWarnings("serial") - private static class CustomFileSystemException extends FileSystemException { - - CustomFileSystemException(String file) { - super(file); - } - } - -} diff --git a/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java new file mode 100644 index 0000000000..8a28d519d9 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java @@ -0,0 +1,125 @@ +/* + * Copyright 2002-present 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 + * + * https://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.core.retry; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.FileSystemException; +import java.time.Duration; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.springframework.util.backoff.BackOffExecution.STOP; + +/** + * Max attempts {@link RetryPolicy} tests. + * + * @author Mahmoud Ben Hassine + * @author Sam Brannen + * @since 7.0 + */ +class MaxAttemptsRetryPolicyTests { + + @Test + void maxAttempts() { + var retryPolicy = RetryPolicy.builder().maxAttempts(2).delay(Duration.ofMillis(1)).build(); + var backOffExecution = retryPolicy.getBackOff().start(); + var throwable = mock(Throwable.class); + + assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + + assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); + assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP); + assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); + assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP); + } + + @Test + void maxAttemptsAndPredicate() { + var retryPolicy = RetryPolicy.builder() + .maxAttempts(4) + .delay(Duration.ofMillis(1)) + .predicate(NumberFormatException.class::isInstance) + .build(); + + var backOffExecution = retryPolicy.getBackOff().start(); + + // 4 retries + assertThat(retryPolicy.shouldRetry(new NumberFormatException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new CustomNumberFormatException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + + // After policy exhaustion + assertThat(retryPolicy.shouldRetry(new NumberFormatException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP); + assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse(); + assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP); + } + + @Test + void maxAttemptsWithIncludesAndExcludes() { + var retryPolicy = RetryPolicy.builder() + .maxAttempts(6) + .includes(RuntimeException.class, IOException.class) + .excludes(FileNotFoundException.class, CustomFileSystemException.class) + .build(); + + var backOffExecution = retryPolicy.getBackOff().start(); + + // 6 retries + assertThat(retryPolicy.shouldRetry(new IOException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new RuntimeException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new FileNotFoundException())).isFalse(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new FileSystemException("file"))).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new CustomFileSystemException("file"))).isFalse(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(retryPolicy.shouldRetry(new IOException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + + // After policy exhaustion + assertThat(retryPolicy.shouldRetry(new IOException())).isTrue(); + assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP); + } + + + @SuppressWarnings("serial") + private static class CustomNumberFormatException extends NumberFormatException { + } + + @SuppressWarnings("serial") + private static class CustomFileSystemException extends FileSystemException { + + CustomFileSystemException(String file) { + super(file); + } + } + +} diff --git a/spring-core/src/test/java/org/springframework/core/retry/MaxDurationDefaultRetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/MaxDurationDefaultRetryPolicyTests.java deleted file mode 100644 index 0ceb99e6ba..0000000000 --- a/spring-core/src/test/java/org/springframework/core/retry/MaxDurationDefaultRetryPolicyTests.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright 2002-present 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 - * - * https://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.core.retry; - -import java.time.Duration; - -import org.junit.jupiter.api.Test; - -import static java.time.Duration.ofSeconds; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; - -/** - * Max duration tests for {@link DefaultRetryPolicy} and its {@link RetryExecution}. - * - * @author Mahmoud Ben Hassine - * @author Sam Brannen - * @since 7.0 - */ -class MaxDurationDefaultRetryPolicyTests { - - @Test - void invalidMaxDuration() { - assertThatIllegalArgumentException() - .isThrownBy(() -> RetryPolicy.withMaxDuration(Duration.ZERO)) - .withMessage("Max duration must be positive"); - assertThatIllegalArgumentException() - .isThrownBy(() -> RetryPolicy.withMaxDuration(ofSeconds(-1))) - .withMessage("Max duration must be positive"); - } - - @Test - void toStringImplementations() { - var policy1 = RetryPolicy.withMaxDuration(ofSeconds(3)); - var policy2 = RetryPolicy.builder() - .maxDuration(ofSeconds(1)) - .predicate(new NumberFormatExceptionMatcher()) - .build(); - - assertThat(policy1).asString() - .isEqualTo("DefaultRetryPolicy[maxDuration=3000ms]"); - assertThat(policy2).asString() - .isEqualTo("DefaultRetryPolicy[maxDuration=1000ms, predicate=NumberFormatExceptionMatcher]"); - - assertThat(policy1.start()).asString() - .matches("DefaultRetryPolicyExecution\\[maxDuration=3000ms, retryStartTime=.+]"); - assertThat(policy2.start()).asString() - .matches("DefaultRetryPolicyExecution\\[maxDuration=1000ms, retryStartTime=.+, predicate=NumberFormatExceptionMatcher]"); - } - -} diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java new file mode 100644 index 0000000000..b0e4568401 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java @@ -0,0 +1,455 @@ +/* + * Copyright 2002-present 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 + * + * https://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.core.retry; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.FileSystemException; +import java.time.Duration; +import java.util.List; +import java.util.StringJoiner; + +import org.assertj.core.api.ThrowingConsumer; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import org.springframework.util.backoff.ExponentialBackOff; +import org.springframework.util.backoff.FixedBackOff; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.InstanceOfAssertFactories.type; +import static org.mockito.Mockito.mock; + +/** + * Unit tests for {@link RetryPolicy} and its builder. + * + * @author Sam Brannen + * @since 7.0 + * @see RetryTemplateTests + */ +class RetryPolicyTests { + + @Nested + class FactoryMethodTests { + + @Test + void withDefaults() { + var policy = RetryPolicy.withDefaults(); + + assertThat(policy.shouldRetry(new AssertionError())).isTrue(); + assertThat(policy.shouldRetry(new IOException())).isTrue(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(FixedBackOff.class)) + .satisfies(backOff -> { + assertThat(backOff.getMaxAttempts()).isEqualTo(3); + assertThat(backOff.getInterval()).isEqualTo(1000); + }); + } + + @Test + void withMaxAttemptsPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.withMaxAttempts(0)) + .withMessage("Max attempts must be greater than zero"); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.withMaxAttempts(-1)) + .withMessage("Max attempts must be greater than zero"); + } + + @Test + void withMaxAttempts() { + var policy = RetryPolicy.withMaxAttempts(5); + + assertThat(policy.shouldRetry(new AssertionError())).isTrue(); + assertThat(policy.shouldRetry(new IOException())).isTrue(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(FixedBackOff.class)) + .satisfies(backOff -> { + assertThat(backOff.getMaxAttempts()).isEqualTo(5); + assertThat(backOff.getInterval()).isEqualTo(1000); + }); + } + + @Test + void withMaxElapsedTimePreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.withMaxElapsedTime(Duration.ofMillis(0))) + .withMessage("Invalid duration (0ms): max elapsed time must be positive."); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.withMaxElapsedTime(Duration.ofMillis(-1))) + .withMessage("Invalid duration (-1ms): max elapsed time must be positive."); + } + + @Test + void withMaxElapsedTime() { + var policy = RetryPolicy.withMaxElapsedTime(Duration.ofMillis(42)); + + assertThat(policy.shouldRetry(new AssertionError())).isTrue(); + assertThat(policy.shouldRetry(new IOException())).isTrue(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()) + .extracting(ExponentialBackOff::getMaxElapsedTime).isEqualTo(42L); + } + + } + + @Nested + class BuilderTests { + + @Test + void backOffPlusConflictingConfig() { + assertThatIllegalStateException() + .isThrownBy(() -> RetryPolicy.builder().backOff(mock()).delay(Duration.ofMillis(10)).build()) + .withMessage(""" + The following configuration options are not supported with a custom BackOff strategy: \ + maxAttempts, delay, jitter, multiplier, maxDelay, or maxElapsedTime."""); + } + + @Test + void backOff() { + var backOff = new FixedBackOff(); + var policy = RetryPolicy.builder().backOff(backOff).build(); + + assertThat(policy.getBackOff()).isEqualTo(backOff); + + assertThat(policy).asString() + .isEqualTo("DefaultRetryPolicy[backOff=FixedBackOff[interval=5000, maxAttempts=unlimited]]"); + } + + @Test + void maxAttemptsPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().maxAttempts(0)) + .withMessage("Max attempts must be greater than zero"); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().maxAttempts(-1)) + .withMessage("Max attempts must be greater than zero"); + } + + @Test + void maxAttempts() { + var policy = RetryPolicy.builder().maxAttempts(5).build(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(backOff -> { + assertThat(backOff.getMaxAttempts()).isEqualTo(5); + assertThat(backOff.getInitialInterval()).isEqualTo(1000); + }); + + assertToString(policy, 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 5); + } + + @Test + void delayPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(0))) + .withMessage("Invalid duration (0ms): delay must be positive."); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(-1))) + .withMessage("Invalid duration (-1ms): delay must be positive."); + } + + @Test + void delay() { + var policy = RetryPolicy.builder().delay(Duration.ofMillis(42)).build(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(backOff -> { + assertThat(backOff.getInitialInterval()).isEqualTo(42); + assertThat(backOff.getMaxAttempts()).isEqualTo(3); + }); + + assertToString(policy, 42, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void jitterPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().jitter(Duration.ofMillis(-1))) + .withMessage("Invalid jitter (-1ms): must be >= 0."); + } + + @Test + void jitter() { + var policy = RetryPolicy.builder().jitter(Duration.ofMillis(42)).build(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()) + .extracting(ExponentialBackOff::getJitter).isEqualTo(42L); + + assertToString(policy, 1000, 42, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void multiplierPreconditions() { + String template = """ + Invalid multiplier '%s': must be greater than or equal to 1. \ + A multiplier of 1 is equivalent to a fixed delay."""; + + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().multiplier(-1)) + .withMessage(template, "-1.0"); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().multiplier(0)) + .withMessage(template, "0.0"); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().multiplier(0.5)) + .withMessage(template, "0.5"); + } + + @Test + void multiplier() { + var policy = RetryPolicy.builder().multiplier(1.5).build(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()) + .extracting(ExponentialBackOff::getMultiplier).isEqualTo(1.5); + + assertToString(policy, 1000, 0, 1.5, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void maxDelayPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ofMillis(0))) + .withMessage("Invalid duration (0ms): max delay must be positive."); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ofMillis(-1))) + .withMessage("Invalid duration (-1ms): max delay must be positive."); + } + + @Test + void maxDelay() { + var policy = RetryPolicy.builder().maxDelay(Duration.ofMillis(42)).build(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()) + .extracting(ExponentialBackOff::getMaxInterval).isEqualTo(42L); + + assertToString(policy, 1000, 0, 1, 42, Long.MAX_VALUE, 3); + } + + @Test + void maxElapsedTimePreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(0))) + .withMessage("Invalid duration (0ms): max elapsed time must be positive."); + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(-1))) + .withMessage("Invalid duration (-1ms): max elapsed time must be positive."); + } + + @Test + void maxElapsedTime() { + var policy = RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(42)).build(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()) + .extracting(ExponentialBackOff::getMaxElapsedTime).isEqualTo(42L); + + assertToString(policy, 1000, 0, 1, Long.MAX_VALUE, 42, 3); + } + + @Test + void includes() { + var policy = RetryPolicy.builder() + .includes(FileNotFoundException.class, IllegalArgumentException.class) + .includes(List.of(NumberFormatException.class, AssertionError.class)) + .build(); + + assertThat(policy.shouldRetry(new FileNotFoundException())).isTrue(); + assertThat(policy.shouldRetry(new IllegalArgumentException())).isTrue(); + assertThat(policy.shouldRetry(new NumberFormatException())).isTrue(); + assertThat(policy.shouldRetry(new AssertionError())).isTrue(); + + assertThat(policy.shouldRetry(new Throwable())).isFalse(); + assertThat(policy.shouldRetry(new FileSystemException("fs"))).isFalse(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()); + + String filters = "includes=" + names(FileNotFoundException.class, IllegalArgumentException.class, + NumberFormatException.class, AssertionError.class) + ", "; + assertToString(policy, filters, 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void includesSubtypeMatching() { + var policy = RetryPolicy.builder().includes(IOException.class).build(); + + assertThat(policy.shouldRetry(new FileNotFoundException())).isTrue(); + assertThat(policy.shouldRetry(new FileSystemException("fs"))).isTrue(); + + assertThat(policy.shouldRetry(new Throwable())).isFalse(); + assertThat(policy.shouldRetry(new AssertionError())).isFalse(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()); + + assertToString(policy, "includes=[java.io.IOException], ", 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void excludes() { + var policy = RetryPolicy.builder() + .excludes(FileNotFoundException.class, IllegalArgumentException.class) + .excludes(List.of(NumberFormatException.class, AssertionError.class)) + .build(); + + assertThat(policy.shouldRetry(new FileNotFoundException())).isFalse(); + assertThat(policy.shouldRetry(new IllegalArgumentException())).isFalse(); + assertThat(policy.shouldRetry(new NumberFormatException())).isFalse(); + assertThat(policy.shouldRetry(new AssertionError())).isFalse(); + + assertThat(policy.shouldRetry(new Throwable())).isTrue(); + assertThat(policy.shouldRetry(new FileSystemException("fs"))).isTrue(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()); + + String filters = "excludes=" + names(FileNotFoundException.class, IllegalArgumentException.class, + NumberFormatException.class, AssertionError.class) + ", "; + assertToString(policy, filters, 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void excludesSubtypeMatching() { + var policy = RetryPolicy.builder().excludes(IOException.class).build(); + + assertThat(policy.shouldRetry(new IOException("fs"))).isFalse(); + assertThat(policy.shouldRetry(new FileNotFoundException())).isFalse(); + assertThat(policy.shouldRetry(new FileSystemException("fs"))).isFalse(); + + assertThat(policy.shouldRetry(new Throwable())).isTrue(); + assertThat(policy.shouldRetry(new AssertionError())).isTrue(); + + assertToString(policy, "excludes=[java.io.IOException], ", 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void predicate() { + var policy = RetryPolicy.builder() + .predicate(new NumberFormatExceptionMatcher()) + .build(); + + assertThat(policy.shouldRetry(new NumberFormatException())).isTrue(); + assertThat(policy.shouldRetry(new CustomNumberFormatException())).isTrue(); + + assertThat(policy.shouldRetry(new Throwable())).isFalse(); + assertThat(policy.shouldRetry(new Exception())).isFalse(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()); + + assertToString(policy, "predicate=NumberFormatExceptionMatcher, ", + 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3); + } + + @Test + void predicatesCombined() { + var BOOM = "Boom!"; + var policy = RetryPolicy.builder() + .predicate(new NumberFormatExceptionMatcher()) + .predicate(throwable -> BOOM.equals(throwable.getMessage())) + .build(); + + assertThat(policy.shouldRetry(new NumberFormatException(BOOM))).isTrue(); + assertThat(policy.shouldRetry(new CustomNumberFormatException(BOOM))).isTrue(); + + assertThat(policy.shouldRetry(new NumberFormatException())).isFalse(); + assertThat(policy.shouldRetry(new CustomNumberFormatException())).isFalse(); + assertThat(policy.shouldRetry(new Throwable())).isFalse(); + assertThat(policy.shouldRetry(new Exception())).isFalse(); + + assertThat(policy.getBackOff()) + .asInstanceOf(type(ExponentialBackOff.class)) + .satisfies(hasDefaultMaxAttemptsAndDelay()); + + assertThat(policy).asString() + .matches("DefaultRetryPolicy\\[predicate=Predicate.+?Lambda.+?, backOff=ExponentialBackOff\\[.+?]]"); + } + + private static void assertToString(RetryPolicy policy, long initialInterval, long jitter, + double multiplier, long maxInterval, long maxElapsedTime, int maxAttempts) { + + assertToString(policy, "", initialInterval, jitter, multiplier, maxInterval, maxElapsedTime, maxAttempts); + } + + private static void assertToString(RetryPolicy policy, String filters, long initialInterval, long jitter, + double multiplier, long maxInterval, long maxElapsedTime, int maxAttempts) { + + assertThat(policy).asString() + .isEqualTo(""" + DefaultRetryPolicy[%sbackOff=ExponentialBackOff[\ + initialInterval=%d, \ + jitter=%d, \ + multiplier=%s, \ + maxInterval=%d, \ + maxElapsedTime=%d, \ + maxAttempts=%d\ + ]]""", + filters, initialInterval, jitter, multiplier, maxInterval, maxElapsedTime, maxAttempts); + } + + @SafeVarargs + @SuppressWarnings("unchecked") + private static String names(Class... types) { + StringJoiner result = new StringJoiner(", ", "[", "]"); + for (Class type : types) { + String name = type.getCanonicalName(); + result.add(name != null? name : type.getName()); + } + return result.toString(); + } + + } + + private static ThrowingConsumer hasDefaultMaxAttemptsAndDelay() { + return backOff -> { + assertThat(backOff.getMaxAttempts()).isEqualTo(3); + assertThat(backOff.getInitialInterval()).isEqualTo(1000); + }; + } + + @SuppressWarnings("serial") + private static class CustomNumberFormatException extends NumberFormatException { + + CustomNumberFormatException() { + } + + CustomNumberFormatException(String s) { + super(s); + } + } + +} diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java index 4772626efb..27995a45d7 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java @@ -30,18 +30,17 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments.ArgumentSet; import org.junit.jupiter.params.provider.FieldSource; -import org.springframework.util.backoff.FixedBackOff; - import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.jupiter.params.provider.Arguments.argumentSet; /** - * Tests for {@link RetryTemplate}. + * Integration tests for {@link RetryTemplate} and {@link RetryPolicy}. * * @author Mahmoud Ben Hassine * @author Sam Brannen * @since 7.0 + * @see RetryPolicyTests */ class RetryTemplateTests { @@ -49,8 +48,13 @@ class RetryTemplateTests { @BeforeEach - void configureTemplate() { - this.retryTemplate.setBackOffPolicy(new FixedBackOff(Duration.ofMillis(10))); + void configureRetryTemplate() { + var retryPolicy = RetryPolicy.builder() + .maxAttempts(3) + .delay(Duration.ofMillis(1)) + .build(); + + retryTemplate.setRetryPolicy(retryPolicy); } @Test @@ -109,7 +113,7 @@ class RetryTemplateTests { } @Test - void retryWithFailingRetryableAndCustomRetryPolicyWithMultiplePredicates() { + void retryWithFailingRetryableAndMultiplePredicates() { var invocationCount = new AtomicInteger(); var exception = new NumberFormatException("Boom!"); @@ -128,7 +132,7 @@ class RetryTemplateTests { var retryPolicy = RetryPolicy.builder() .maxAttempts(5) - .maxDuration(Duration.ofMillis(100)) + .delay(Duration.ofMillis(1)) .predicate(NumberFormatException.class::isInstance) .predicate(t -> t.getMessage().equals("Boom!")) .build(); @@ -167,6 +171,7 @@ class RetryTemplateTests { var retryPolicy = RetryPolicy.builder() .maxAttempts(Integer.MAX_VALUE) + .delay(Duration.ofMillis(1)) .includes(IOException.class) .build(); @@ -189,11 +194,13 @@ class RetryTemplateTests { argumentSet("Excludes", RetryPolicy.builder() .maxAttempts(Integer.MAX_VALUE) + .delay(Duration.ofMillis(1)) .excludes(FileNotFoundException.class) .build()), argumentSet("Includes & Excludes", RetryPolicy.builder() .maxAttempts(Integer.MAX_VALUE) + .delay(Duration.ofMillis(1)) .includes(IOException.class) .excludes(FileNotFoundException.class) .build()) @@ -201,7 +208,7 @@ class RetryTemplateTests { @ParameterizedTest @FieldSource("includesAndExcludesRetryPolicies") - void retryWithIncludesAndExcludesRetryPolicies(RetryPolicy retryPolicy) { + void retryWithExceptionIncludesAndExcludes(RetryPolicy retryPolicy) { retryTemplate.setRetryPolicy(retryPolicy); var invocationCount = new AtomicInteger(); diff --git a/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java b/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java index 4adfdee2cc..8fac26872f 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java @@ -21,8 +21,9 @@ import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.core.retry.RetryExecution; import org.springframework.core.retry.RetryListener; +import org.springframework.core.retry.RetryPolicy; +import org.springframework.core.retry.Retryable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -39,7 +40,8 @@ class CompositeRetryListenerTests { private final RetryListener listener1 = mock(); private final RetryListener listener2 = mock(); private final RetryListener listener3 = mock(); - private final RetryExecution retryExecution = mock(); + private final RetryPolicy retryPolicy = mock(); + private final Retryable retryable = mock(); private final CompositeRetryListener compositeRetryListener = new CompositeRetryListener(List.of(listener1, listener2)); @@ -52,41 +54,41 @@ class CompositeRetryListenerTests { @Test void beforeRetry() { - compositeRetryListener.beforeRetry(retryExecution); + compositeRetryListener.beforeRetry(retryPolicy, retryable); - verify(listener1).beforeRetry(retryExecution); - verify(listener2).beforeRetry(retryExecution); - verify(listener3).beforeRetry(retryExecution); + verify(listener1).beforeRetry(retryPolicy, retryable); + verify(listener2).beforeRetry(retryPolicy, retryable); + verify(listener3).beforeRetry(retryPolicy, retryable); } @Test void onRetrySuccess() { Object result = new Object(); - compositeRetryListener.onRetrySuccess(retryExecution, result); + compositeRetryListener.onRetrySuccess(retryPolicy, retryable, result); - verify(listener1).onRetrySuccess(retryExecution, result); - verify(listener2).onRetrySuccess(retryExecution, result); - verify(listener3).onRetrySuccess(retryExecution, result); + verify(listener1).onRetrySuccess(retryPolicy, retryable, result); + verify(listener2).onRetrySuccess(retryPolicy, retryable, result); + verify(listener3).onRetrySuccess(retryPolicy, retryable, result); } @Test void onRetryFailure() { Exception exception = new Exception(); - compositeRetryListener.onRetryFailure(retryExecution, exception); + compositeRetryListener.onRetryFailure(retryPolicy, retryable, exception); - verify(listener1).onRetryFailure(retryExecution, exception); - verify(listener2).onRetryFailure(retryExecution, exception); - verify(listener3).onRetryFailure(retryExecution, exception); + verify(listener1).onRetryFailure(retryPolicy, retryable, exception); + verify(listener2).onRetryFailure(retryPolicy, retryable, exception); + verify(listener3).onRetryFailure(retryPolicy, retryable, exception); } @Test void onRetryPolicyExhaustion() { Exception exception = new Exception(); - compositeRetryListener.onRetryPolicyExhaustion(retryExecution, exception); + compositeRetryListener.onRetryPolicyExhaustion(retryPolicy, retryable, exception); - verify(listener1).onRetryPolicyExhaustion(retryExecution, exception); - verify(listener2).onRetryPolicyExhaustion(retryExecution, exception); - verify(listener3).onRetryPolicyExhaustion(retryExecution, exception); + verify(listener1).onRetryPolicyExhaustion(retryPolicy, retryable, exception); + verify(listener2).onRetryPolicyExhaustion(retryPolicy, retryable, exception); + verify(listener3).onRetryPolicyExhaustion(retryPolicy, retryable, exception); } }