Fix potentially loses precision and jitter is not well capped with unit tests

This commit is contained in:
NeatGuyCoding 2025-07-01 12:53:28 +08:00
parent 60b19278c0
commit 42a11da031
No known key found for this signature in database
GPG Key ID: 986432EB84C921B3
2 changed files with 162 additions and 1 deletions

View File

@ -136,7 +136,10 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor {
Publisher<?> publisher = adapter.toPublisher(result);
Retry retry = Retry.backoff(spec.maxAttempts(), spec.delay())
.jitter((double) spec.jitter().toMillis() / spec.delay().toMillis())
.jitter(
spec.delay().isZero() ? 0.0 :
Math.max(0.0, Math.min(1.0, spec.jitter().toNanos() / (double) spec.delay().toNanos()))
)
.multiplier(spec.multiplier())
.maxBackoff(spec.maxDelay())
.filter(spec.combinedPredicate().forMethod(method));

View File

@ -126,6 +126,97 @@ public class ReactiveRetryInterceptorTests {
assertThat(target.counter.get()).isEqualTo(6);
}
@Test
void adaptReactiveResultWithMinimalRetrySpec() {
// 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, 1, Duration.ZERO, Duration.ZERO, 1.0, Duration.ZERO)));
MinimalRetryBean proxy = (MinimalRetryBean) pf.getProxy();
// Should execute only 2 times, because maxAttempts=1 means 1 call + 1 retry
assertThatIllegalStateException().isThrownBy(() -> proxy.retryOperation().block())
.withCauseInstanceOf(IOException.class).havingCause().withMessage("2");
assertThat(target.counter.get()).isEqualTo(2);
}
@Test
void adaptReactiveResultWithZeroDelayAndJitter() {
// Test case where delay=0 and jitter>0
ZeroDelayJitterBean target = new ZeroDelayJitterBean();
ProxyFactory pf = new ProxyFactory();
pf.setTarget(target);
pf.addAdvice(new SimpleRetryInterceptor(
new MethodRetrySpec((m, t) -> true, 3, Duration.ZERO, Duration.ofMillis(10), 2.0, Duration.ofMillis(100))));
ZeroDelayJitterBean proxy = (ZeroDelayJitterBean) pf.getProxy();
assertThatIllegalStateException().isThrownBy(() -> proxy.retryOperation().block())
.withCauseInstanceOf(IOException.class).havingCause().withMessage("4");
assertThat(target.counter.get()).isEqualTo(4);
}
@Test
void adaptReactiveResultWithJitterGreaterThanDelay() {
// Test case where jitter > delay
JitterGreaterThanDelayBean target = new JitterGreaterThanDelayBean();
ProxyFactory pf = new ProxyFactory();
pf.setTarget(target);
pf.addAdvice(new SimpleRetryInterceptor(
new MethodRetrySpec((m, t) -> true, 3, Duration.ofMillis(5), Duration.ofMillis(20), 1.5, Duration.ofMillis(50))));
JitterGreaterThanDelayBean proxy = (JitterGreaterThanDelayBean) pf.getProxy();
assertThatIllegalStateException().isThrownBy(() -> proxy.retryOperation().block())
.withCauseInstanceOf(IOException.class).havingCause().withMessage("4");
assertThat(target.counter.get()).isEqualTo(4);
}
@Test
void adaptReactiveResultWithFluxMultiValue() {
// Test Flux multi-value stream case
FluxMultiValueBean target = new FluxMultiValueBean();
ProxyFactory pf = new ProxyFactory();
pf.setTarget(target);
pf.addAdvice(new SimpleRetryInterceptor(
new MethodRetrySpec((m, t) -> true, 3, Duration.ofMillis(10), Duration.ofMillis(5), 2.0, Duration.ofMillis(100))));
FluxMultiValueBean proxy = (FluxMultiValueBean) pf.getProxy();
assertThatIllegalStateException().isThrownBy(() -> proxy.retryOperation().blockFirst())
.withCauseInstanceOf(IOException.class).havingCause().withMessage("4");
assertThat(target.counter.get()).isEqualTo(4);
}
@Test
void adaptReactiveResultWithSuccessfulOperation() {
// Test successful return case, ensuring retry mechanism doesn't activate
SuccessfulOperationBean target = new SuccessfulOperationBean();
ProxyFactory pf = new ProxyFactory();
pf.setTarget(target);
pf.addAdvice(new SimpleRetryInterceptor(
new MethodRetrySpec((m, t) -> true, 5, Duration.ofMillis(10), Duration.ofMillis(5), 2.0, Duration.ofMillis(100))));
SuccessfulOperationBean proxy = (SuccessfulOperationBean) pf.getProxy();
String result = proxy.retryOperation().block();
assertThat(result).isEqualTo("success");
// Should execute only once because of successful return
assertThat(target.counter.get()).isEqualTo(1);
}
@Test
void adaptReactiveResultWithImmediateFailure() {
// Test immediate failure case
ImmediateFailureBean target = new ImmediateFailureBean();
ProxyFactory pf = new ProxyFactory();
pf.setTarget(target);
pf.addAdvice(new SimpleRetryInterceptor(
new MethodRetrySpec((m, t) -> true, 3, Duration.ofMillis(10), Duration.ofMillis(5), 1.5, Duration.ofMillis(50))));
ImmediateFailureBean proxy = (ImmediateFailureBean) pf.getProxy();
assertThatIllegalStateException().isThrownBy(() -> proxy.retryOperation().block())
.withCauseInstanceOf(RuntimeException.class).havingCause().withMessage("immediate failure");
assertThat(target.counter.get()).isEqualTo(4);
}
public static class NonAnnotatedBean {
@ -193,4 +284,71 @@ public class ReactiveRetryInterceptorTests {
}
}
// Bean classes for boundary testing
public static class MinimalRetryBean {
AtomicInteger counter = new AtomicInteger();
public Mono<Object> retryOperation() {
return Mono.fromCallable(() -> {
counter.incrementAndGet();
throw new IOException(counter.toString());
});
}
}
public static class ZeroDelayJitterBean {
AtomicInteger counter = new AtomicInteger();
public Mono<Object> retryOperation() {
return Mono.fromCallable(() -> {
counter.incrementAndGet();
throw new IOException(counter.toString());
});
}
}
public static class JitterGreaterThanDelayBean {
AtomicInteger counter = new AtomicInteger();
public Mono<Object> retryOperation() {
return Mono.fromCallable(() -> {
counter.incrementAndGet();
throw new IOException(counter.toString());
});
}
}
public static class FluxMultiValueBean {
AtomicInteger counter = new AtomicInteger();
public Flux<Object> retryOperation() {
return Flux.from(Mono.fromCallable(() -> {
counter.incrementAndGet();
throw new IOException(counter.toString());
}));
}
}
public static class SuccessfulOperationBean {
AtomicInteger counter = new AtomicInteger();
public Mono<String> retryOperation() {
return Mono.fromCallable(() -> {
counter.incrementAndGet();
return "success";
});
}
}
public static class ImmediateFailureBean {
AtomicInteger counter = new AtomicInteger();
public Mono<Object> retryOperation() {
return Mono.fromCallable(() -> {
counter.incrementAndGet();
throw new RuntimeException("immediate failure");
});
}
}
}