mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 16:39:29 +00:00
Expose RetryException#getRetryCount() and accept maxAttempts(0)
Closes gh-35351 Closes gh-35362
This commit is contained in:
+20
@@ -189,6 +189,26 @@ class ReactiveRetryInterceptorTests {
|
||||
assertThat(target.counter.get()).isEqualTo(2);
|
||||
}
|
||||
|
||||
@Test
|
||||
void adaptReactiveResultWithZeroAttempts() {
|
||||
// Test minimal retry configuration: maxAttempts=1, delay=0, jitter=0, multiplier=1.0, maxDelay=0
|
||||
MinimalRetryBean target = new MinimalRetryBean();
|
||||
ProxyFactory pf = new ProxyFactory();
|
||||
pf.setTarget(target);
|
||||
pf.addAdvice(new SimpleRetryInterceptor(
|
||||
new MethodRetrySpec((m, t) -> true, 0, Duration.ZERO, Duration.ZERO, 1.0, Duration.ZERO)));
|
||||
MinimalRetryBean proxy = (MinimalRetryBean) pf.getProxy();
|
||||
|
||||
// Should execute only 1 time, because maxAttempts=0 means initial call only
|
||||
assertThatIllegalStateException()
|
||||
.isThrownBy(() -> proxy.retryOperation().block())
|
||||
.satisfies(isRetryExhaustedException())
|
||||
.havingCause()
|
||||
.isInstanceOf(IOException.class)
|
||||
.withMessage("1");
|
||||
assertThat(target.counter.get()).isEqualTo(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
void adaptReactiveResultWithZeroDelayAndJitter() {
|
||||
// Test case where delay=0 and jitter>0
|
||||
|
||||
+25
@@ -194,6 +194,31 @@ class RetryInterceptorTests {
|
||||
assertThat(target.counter).isEqualTo(6);
|
||||
}
|
||||
|
||||
@Test
|
||||
void withPostProcessorForClassWithZeroAttempts() {
|
||||
Properties props = new Properties();
|
||||
props.setProperty("delay", "10");
|
||||
props.setProperty("jitter", "5");
|
||||
props.setProperty("multiplier", "2.0");
|
||||
props.setProperty("maxDelay", "40");
|
||||
props.setProperty("limitedAttempts", "0");
|
||||
|
||||
GenericApplicationContext ctx = new GenericApplicationContext();
|
||||
ctx.getEnvironment().getPropertySources().addFirst(new PropertiesPropertySource("props", props));
|
||||
ctx.registerBeanDefinition("bean", new RootBeanDefinition(AnnotatedClassBeanWithStrings.class));
|
||||
ctx.registerBeanDefinition("bpp", new RootBeanDefinition(RetryAnnotationBeanPostProcessor.class));
|
||||
ctx.refresh();
|
||||
AnnotatedClassBeanWithStrings proxy = ctx.getBean(AnnotatedClassBeanWithStrings.class);
|
||||
AnnotatedClassBeanWithStrings target = (AnnotatedClassBeanWithStrings) AopProxyUtils.getSingletonTarget(proxy);
|
||||
|
||||
assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("3");
|
||||
assertThat(target.counter).isEqualTo(3);
|
||||
assertThatIOException().isThrownBy(proxy::otherOperation);
|
||||
assertThat(target.counter).isEqualTo(4);
|
||||
assertThatIOException().isThrownBy(proxy::overrideOperation);
|
||||
assertThat(target.counter).isEqualTo(5);
|
||||
}
|
||||
|
||||
@Test
|
||||
void withEnableAnnotation() throws Exception {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
|
||||
@@ -45,7 +45,6 @@ class DefaultRetryPolicy implements RetryPolicy {
|
||||
private final BackOff backOff;
|
||||
|
||||
|
||||
|
||||
DefaultRetryPolicy(Set<Class<? extends Throwable>> includes, Set<Class<? extends Throwable>> excludes,
|
||||
@Nullable Predicate<Throwable> predicate, BackOff backOff) {
|
||||
|
||||
@@ -59,8 +58,8 @@ class DefaultRetryPolicy implements RetryPolicy {
|
||||
|
||||
@Override
|
||||
public boolean shouldRetry(Throwable throwable) {
|
||||
return this.exceptionFilter.match(throwable) &&
|
||||
(this.predicate == null || this.predicate.test(throwable));
|
||||
return (this.exceptionFilter.match(throwable) &&
|
||||
(this.predicate == null || this.predicate.test(throwable)));
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -28,6 +28,7 @@ import java.util.Objects;
|
||||
* exceptions}.
|
||||
*
|
||||
* @author Mahmoud Ben Hassine
|
||||
* @author Juergen Hoeller
|
||||
* @since 7.0
|
||||
* @see RetryOperations
|
||||
*/
|
||||
@@ -51,8 +52,16 @@ public class RetryException extends Exception {
|
||||
* Get the last exception thrown by the {@link Retryable} operation.
|
||||
*/
|
||||
@Override
|
||||
public final synchronized Throwable getCause() {
|
||||
public final Throwable getCause() {
|
||||
return Objects.requireNonNull(super.getCause());
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the number of retry attempts, or 0 if no retry has been attempted
|
||||
* after the initial invocation at all.
|
||||
*/
|
||||
public int getRetryCount() {
|
||||
return getSuppressed().length;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -82,12 +82,13 @@ public interface RetryPolicy {
|
||||
* Create a {@link RetryPolicy} configured with a maximum number of retry attempts.
|
||||
* <p>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
|
||||
* @param maxAttempts the maximum number of retry attempts;
|
||||
* must be positive (or zero for no retry)
|
||||
* @see Builder#maxAttempts(long)
|
||||
* @see FixedBackOff
|
||||
*/
|
||||
static RetryPolicy withMaxAttempts(long maxAttempts) {
|
||||
assertMaxAttemptsIsPositive(maxAttempts);
|
||||
assertMaxAttemptsIsNotNegative(maxAttempts);
|
||||
return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build();
|
||||
}
|
||||
|
||||
@@ -100,14 +101,9 @@ public interface RetryPolicy {
|
||||
}
|
||||
|
||||
|
||||
private static void assertMaxAttemptsIsPositive(long maxAttempts) {
|
||||
Assert.isTrue(maxAttempts > 0,
|
||||
() -> "Invalid maxAttempts (%d): must be greater than zero.".formatted(maxAttempts));
|
||||
}
|
||||
|
||||
private static void assertIsPositive(String name, Duration duration) {
|
||||
Assert.isTrue((!duration.isNegative() && !duration.isZero()),
|
||||
() -> "Invalid %s (%dms): must be greater than zero.".formatted(name, duration.toMillis()));
|
||||
private static void assertMaxAttemptsIsNotNegative(long maxAttempts) {
|
||||
Assert.isTrue(maxAttempts >= 0,
|
||||
() -> "Invalid maxAttempts (%d): must be positive or zero for no retry.".formatted(maxAttempts));
|
||||
}
|
||||
|
||||
private static void assertIsNotNegative(String name, Duration duration) {
|
||||
@@ -115,6 +111,11 @@ public interface RetryPolicy {
|
||||
() -> "Invalid %s (%dms): must be greater than or equal to zero.".formatted(name, duration.toMillis()));
|
||||
}
|
||||
|
||||
private static void assertIsPositive(String name, Duration duration) {
|
||||
Assert.isTrue((!duration.isNegative() && !duration.isZero()),
|
||||
() -> "Invalid %s (%dms): must be greater than zero.".formatted(name, duration.toMillis()));
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Fluent API for configuring a {@link RetryPolicy} with common configuration
|
||||
@@ -146,13 +147,13 @@ public interface RetryPolicy {
|
||||
|
||||
private @Nullable BackOff backOff;
|
||||
|
||||
private long maxAttempts;
|
||||
private @Nullable Long maxAttempts;
|
||||
|
||||
private @Nullable Duration delay;
|
||||
|
||||
private @Nullable Duration jitter;
|
||||
|
||||
private double multiplier;
|
||||
private @Nullable Double multiplier;
|
||||
|
||||
private @Nullable Duration maxDelay;
|
||||
|
||||
@@ -191,12 +192,12 @@ public interface RetryPolicy {
|
||||
* <p>The supplied value will override any previously configured value.
|
||||
* <p>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
|
||||
* @param maxAttempts the maximum number of retry attempts;
|
||||
* must be positive (or zero for no retry)
|
||||
* @return this {@code Builder} instance for chained method invocations
|
||||
*/
|
||||
public Builder maxAttempts(long maxAttempts) {
|
||||
assertMaxAttemptsIsPositive(maxAttempts);
|
||||
assertMaxAttemptsIsNotNegative(maxAttempts);
|
||||
this.maxAttempts = maxAttempts;
|
||||
return this;
|
||||
}
|
||||
@@ -399,18 +400,18 @@ public interface RetryPolicy {
|
||||
public RetryPolicy build() {
|
||||
BackOff backOff = this.backOff;
|
||||
if (backOff != null) {
|
||||
boolean misconfigured = (this.maxAttempts != 0) || (this.delay != null) || (this.jitter != null) ||
|
||||
(this.multiplier != 0) || (this.maxDelay != null);
|
||||
boolean misconfigured = (this.maxAttempts != null || this.delay != null || this.jitter != null ||
|
||||
this.multiplier != null || this.maxDelay != null);
|
||||
Assert.state(!misconfigured, """
|
||||
The following configuration options are not supported with a custom BackOff strategy: \
|
||||
maxAttempts, delay, jitter, multiplier, or maxDelay.""");
|
||||
}
|
||||
else {
|
||||
ExponentialBackOff exponentialBackOff = new ExponentialBackOff();
|
||||
exponentialBackOff.setMaxAttempts(this.maxAttempts > 0 ? this.maxAttempts : DEFAULT_MAX_ATTEMPTS);
|
||||
exponentialBackOff.setMaxAttempts(this.maxAttempts != null ? 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);
|
||||
exponentialBackOff.setMultiplier(this.multiplier != null ? this.multiplier : DEFAULT_MULTIPLIER);
|
||||
if (this.jitter != null) {
|
||||
exponentialBackOff.setJitter(this.jitter.toMillis());
|
||||
}
|
||||
|
||||
+13
@@ -54,6 +54,18 @@ class MaxAttemptsRetryPolicyTests {
|
||||
assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP);
|
||||
}
|
||||
|
||||
@Test
|
||||
void maxAttemptsZero() {
|
||||
var retryPolicy = RetryPolicy.builder().maxAttempts(0).delay(Duration.ZERO).build();
|
||||
var backOffExecution = retryPolicy.getBackOff().start();
|
||||
var throwable = mock(Throwable.class);
|
||||
|
||||
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()
|
||||
@@ -115,6 +127,7 @@ class MaxAttemptsRetryPolicyTests {
|
||||
private static class CustomNumberFormatException extends NumberFormatException {
|
||||
}
|
||||
|
||||
|
||||
@SuppressWarnings("serial")
|
||||
private static class CustomFileSystemException extends FileSystemException {
|
||||
|
||||
|
||||
@@ -65,12 +65,9 @@ class RetryPolicyTests {
|
||||
|
||||
@Test
|
||||
void withMaxAttemptsPreconditions() {
|
||||
assertThatIllegalArgumentException()
|
||||
.isThrownBy(() -> RetryPolicy.withMaxAttempts(0))
|
||||
.withMessage("Invalid maxAttempts (0): must be greater than zero.");
|
||||
assertThatIllegalArgumentException()
|
||||
.isThrownBy(() -> RetryPolicy.withMaxAttempts(-1))
|
||||
.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
|
||||
.withMessageStartingWith("Invalid maxAttempts (-1)");
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -115,12 +112,9 @@ class RetryPolicyTests {
|
||||
|
||||
@Test
|
||||
void maxAttemptsPreconditions() {
|
||||
assertThatIllegalArgumentException()
|
||||
.isThrownBy(() -> RetryPolicy.builder().maxAttempts(0))
|
||||
.withMessage("Invalid maxAttempts (0): must be greater than zero.");
|
||||
assertThatIllegalArgumentException()
|
||||
.isThrownBy(() -> RetryPolicy.builder().maxAttempts(-1))
|
||||
.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
|
||||
.withMessageStartingWith("Invalid maxAttempts (-1)");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -32,9 +32,6 @@ import org.junit.jupiter.params.provider.Arguments.ArgumentSet;
|
||||
import org.junit.jupiter.params.provider.FieldSource;
|
||||
import org.mockito.InOrder;
|
||||
|
||||
import org.springframework.util.backoff.BackOff;
|
||||
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;
|
||||
@@ -51,16 +48,13 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||
*
|
||||
* @author Mahmoud Ben Hassine
|
||||
* @author Sam Brannen
|
||||
* @author Juergen Hoeller
|
||||
* @since 7.0
|
||||
* @see RetryPolicyTests
|
||||
*/
|
||||
class RetryTemplateTests {
|
||||
|
||||
private final RetryPolicy retryPolicy =
|
||||
RetryPolicy.builder()
|
||||
.maxAttempts(3)
|
||||
.delay(Duration.ZERO)
|
||||
.build();
|
||||
private final RetryPolicy retryPolicy = RetryPolicy.builder().maxAttempts(3).delay(Duration.ZERO).build();
|
||||
|
||||
private final RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
|
||||
|
||||
@@ -104,7 +98,8 @@ class RetryTemplateTests {
|
||||
.isThrownBy(() -> retryTemplate.execute(retryable))
|
||||
.withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution")
|
||||
.withCause(exception)
|
||||
.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty());
|
||||
.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty())
|
||||
.satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero());
|
||||
|
||||
// RetryListener interactions:
|
||||
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
|
||||
@@ -112,19 +107,8 @@ class RetryTemplateTests {
|
||||
}
|
||||
|
||||
@Test
|
||||
void retryWithInitialFailureAndZeroRetriesBackOffPolicy() {
|
||||
RetryPolicy retryPolicy = new RetryPolicy() {
|
||||
|
||||
@Override
|
||||
public boolean shouldRetry(Throwable throwable) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@Override
|
||||
public BackOff getBackOff() {
|
||||
return new FixedBackOff(10, 0); // Zero retries
|
||||
}
|
||||
};
|
||||
void retryWithInitialFailureAndZeroRetriesFixedBackOffPolicy() {
|
||||
RetryPolicy retryPolicy = RetryPolicy.withMaxAttempts(0);
|
||||
|
||||
RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
|
||||
retryTemplate.setRetryListener(retryListener);
|
||||
@@ -137,7 +121,31 @@ class RetryTemplateTests {
|
||||
.isThrownBy(() -> retryTemplate.execute(retryable))
|
||||
.withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution")
|
||||
.withCause(exception)
|
||||
.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty());
|
||||
.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty())
|
||||
.satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero());
|
||||
|
||||
// RetryListener interactions:
|
||||
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
|
||||
verifyNoMoreInteractions(retryListener);
|
||||
}
|
||||
|
||||
@Test
|
||||
void retryWithInitialFailureAndZeroRetriesBackOffPolicyFromBuilder() {
|
||||
RetryPolicy retryPolicy = RetryPolicy.builder().maxAttempts(0).build();
|
||||
|
||||
RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
|
||||
retryTemplate.setRetryListener(retryListener);
|
||||
Exception exception = new RuntimeException("Boom!");
|
||||
Retryable<String> retryable = () -> {
|
||||
throw exception;
|
||||
};
|
||||
|
||||
assertThatExceptionOfType(RetryException.class)
|
||||
.isThrownBy(() -> retryTemplate.execute(retryable))
|
||||
.withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution")
|
||||
.withCause(exception)
|
||||
.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty())
|
||||
.satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero());
|
||||
|
||||
// RetryListener interactions:
|
||||
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
|
||||
@@ -282,7 +290,8 @@ class RetryTemplateTests {
|
||||
.satisfies(hasSuppressedExceptionsSatisfyingExactly(
|
||||
suppressed1 -> assertThat(suppressed1).isExactlyInstanceOf(FileNotFoundException.class),
|
||||
suppressed2 -> assertThat(suppressed2).isExactlyInstanceOf(IOException.class)
|
||||
));
|
||||
))
|
||||
.satisfies(throwable -> assertThat(throwable.getRetryCount()).isEqualTo(2));
|
||||
// 3 = 1 initial invocation + 2 retry attempts
|
||||
assertThat(invocationCount).hasValue(3);
|
||||
|
||||
@@ -344,7 +353,8 @@ class RetryTemplateTests {
|
||||
.satisfies(hasSuppressedExceptionsSatisfyingExactly(
|
||||
suppressed1 -> assertThat(suppressed1).isExactlyInstanceOf(IOException.class),
|
||||
suppressed2 -> assertThat(suppressed2).isExactlyInstanceOf(IOException.class)
|
||||
));
|
||||
))
|
||||
.satisfies(throwable -> assertThat(throwable.getRetryCount()).isEqualTo(2));
|
||||
// 3 = 1 initial invocation + 2 retry attempts
|
||||
assertThat(invocationCount).hasValue(3);
|
||||
|
||||
@@ -366,8 +376,9 @@ class RetryTemplateTests {
|
||||
}
|
||||
|
||||
@SafeVarargs
|
||||
private static final Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactly(
|
||||
private static Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactly(
|
||||
ThrowingConsumer<? super Throwable>... requirements) {
|
||||
|
||||
return throwable -> assertThat(throwable.getSuppressed()).satisfiesExactly(requirements);
|
||||
}
|
||||
|
||||
@@ -376,6 +387,7 @@ class RetryTemplateTests {
|
||||
private static class CustomFileNotFoundException extends FileNotFoundException {
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Custom {@link RuntimeException} that implements {@link #equals(Object)}
|
||||
* and {@link #hashCode()} for use in assertions that check for equality.
|
||||
|
||||
Reference in New Issue
Block a user