From d97288a74e5a59f19055cc029e5c063ecf9309c4 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 27 Jun 2025 16:04:32 +0200 Subject: [PATCH] Improve Javadoc and tests for BackOff strategies --- .../springframework/util/backoff/BackOff.java | 14 ++++++----- .../util/backoff/BackOffExecution.java | 7 +++--- .../util/backoff/ExponentialBackOff.java | 24 +++++++++++-------- .../util/ExponentialBackOffTests.java | 15 ++++++------ .../util/FixedBackOffTests.java | 13 ++++++---- 5 files changed, 43 insertions(+), 30 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/backoff/BackOff.java b/spring-core/src/main/java/org/springframework/util/backoff/BackOff.java index 6502bee46b..be21a7f7ca 100644 --- a/spring-core/src/main/java/org/springframework/util/backoff/BackOff.java +++ b/spring-core/src/main/java/org/springframework/util/backoff/BackOff.java @@ -17,16 +17,16 @@ package org.springframework.util.backoff; /** - * Provide a {@link BackOffExecution} that indicates the rate at which - * an operation should be retried. + * Strategy interface for providing a {@link BackOffExecution} that indicates the + * rate at which an operation should be retried. * *

Users of this interface are expected to use it like this: * *

- * BackOffExecution exec = backOff.start();
+ * BackOffExecution execution = backOff.start();
  *
  * // In the operation recovery/retry loop:
- * long waitInterval = exec.nextBackOff();
+ * long waitInterval = execution.nextBackOff();
  * if (waitInterval == BackOffExecution.STOP) {
  *     // do not retry operation
  * }
@@ -35,12 +35,14 @@ package org.springframework.util.backoff;
  *     // retry operation
  * }
* - * Once the underlying operation has completed successfully, - * the execution instance can be discarded. + *

Once the underlying operation has completed successfully, the execution + * instance can be discarded. * * @author Stephane Nicoll * @since 4.1 * @see BackOffExecution + * @see FixedBackOff + * @see ExponentialBackOff */ @FunctionalInterface public interface BackOff { diff --git a/spring-core/src/main/java/org/springframework/util/backoff/BackOffExecution.java b/spring-core/src/main/java/org/springframework/util/backoff/BackOffExecution.java index f6efb91441..c70a6029f0 100644 --- a/spring-core/src/main/java/org/springframework/util/backoff/BackOffExecution.java +++ b/spring-core/src/main/java/org/springframework/util/backoff/BackOffExecution.java @@ -17,9 +17,10 @@ package org.springframework.util.backoff; /** - * Represent a particular back-off execution. + *

A {@code BackOffExecution} is effectively an executable instance of a given + * {@link BackOff} strategy. * - *

Implementations do not need to be thread safe. + *

Implementations may be stateful but do not need to be thread-safe. * * @author Stephane Nicoll * @since 4.1 @@ -29,7 +30,7 @@ package org.springframework.util.backoff; public interface BackOffExecution { /** - * Return value of {@link #nextBackOff()} that indicates that the operation + * Return value of {@link #nextBackOff()} which indicates that the operation * should not be retried. */ long STOP = -1; diff --git a/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java b/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java index 72ab432700..6532f5c1f0 100644 --- a/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java +++ b/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java @@ -66,7 +66,7 @@ public class ExponentialBackOff implements BackOff { public static final long DEFAULT_INITIAL_INTERVAL = 2000L; /** - * The default jitter range for each interval: {@value} ms. + * The default jitter value for each interval: {@value} ms. * @since 7.0 */ public static final long DEFAULT_JITTER = 0; @@ -121,7 +121,7 @@ public class ExponentialBackOff implements BackOff { /** * Create an instance with the supplied settings. * @param initialInterval the initial interval in milliseconds - * @param multiplier the multiplier (should be greater than or equal to 1) + * @param multiplier the multiplier (must be greater than or equal to 1) */ public ExponentialBackOff(long initialInterval, double multiplier) { checkMultiplier(multiplier); @@ -131,7 +131,8 @@ public class ExponentialBackOff implements BackOff { /** - * Set the initial interval in milliseconds. + * Set the initial interval. + * @param initialInterval the initial interval in milliseconds */ public void setInitialInterval(long initialInterval) { this.initialInterval = initialInterval; @@ -145,20 +146,22 @@ public class ExponentialBackOff implements BackOff { } /** - * Set the jitter range (ms) to apply for each interval, leading to random - * milliseconds within the range to be subtracted or added, resulting in a - * value between {@code interval - jitter} and {@code interval + jitter} - * but never below {@code initialInterval} or above {@code maxInterval}. - * If a multiplier is specified, it applies to the jitter range as well. + * Set the jitter value to apply for each interval, leading to random + * milliseconds to be subtracted or added and resulting in a value between + * {@code interval - jitter} and {@code interval + jitter} but never below + * {@code initialInterval} or above {@code maxInterval}. + *

If a {@code multiplier} is specified, it is applied to the jitter value + * as well. + * @param jitter the jitter value in milliseconds * @since 7.0 */ public void setJitter(long jitter) { - Assert.isTrue(jitter >= 0, () -> "Invalid jitter '" + jitter + "': Must be >=0."); + Assert.isTrue(jitter >= 0, () -> "Invalid jitter '" + jitter + "': must be >= 0."); this.jitter = jitter; } /** - * Return the jitter range to apply for each interval. + * Return the jitter value to apply for each interval in milliseconds. * @since 7.0 */ public long getJitter() { @@ -169,6 +172,7 @@ public class ExponentialBackOff implements BackOff { * Set the value to multiply the current interval by for each attempt. *

This applies to the {@linkplain #setInitialInterval initial interval} * as well as the {@linkplain #setJitter jitter range}. + * @param multiplier the multiplier (must be greater than or equal to 1) */ public void setMultiplier(double multiplier) { checkMultiplier(multiplier); diff --git a/spring-core/src/test/java/org/springframework/util/ExponentialBackOffTests.java b/spring-core/src/test/java/org/springframework/util/ExponentialBackOffTests.java index cbc661141a..12d12480c3 100644 --- a/spring-core/src/test/java/org/springframework/util/ExponentialBackOffTests.java +++ b/spring-core/src/test/java/org/springframework/util/ExponentialBackOffTests.java @@ -92,28 +92,29 @@ class ExponentialBackOffTests { } @Test - void startReturnDifferentInstances() { + void startReturnsDifferentInstances() { ExponentialBackOff backOff = new ExponentialBackOff(); backOff.setInitialInterval(2000L); backOff.setMultiplier(2.0); backOff.setMaxElapsedTime(4000L); - BackOffExecution execution = backOff.start(); + BackOffExecution execution1 = backOff.start(); BackOffExecution execution2 = backOff.start(); - assertThat(execution.nextBackOff()).isEqualTo(2000L); + assertThat(execution1).isNotSameAs(execution2); + + assertThat(execution1.nextBackOff()).isEqualTo(2000L); assertThat(execution2.nextBackOff()).isEqualTo(2000L); - assertThat(execution.nextBackOff()).isEqualTo(4000L); + assertThat(execution1.nextBackOff()).isEqualTo(4000L); assertThat(execution2.nextBackOff()).isEqualTo(4000L); - assertThat(execution.nextBackOff()).isEqualTo(BackOffExecution.STOP); + assertThat(execution1.nextBackOff()).isEqualTo(BackOffExecution.STOP); assertThat(execution2.nextBackOff()).isEqualTo(BackOffExecution.STOP); } @Test void invalidInterval() { ExponentialBackOff backOff = new ExponentialBackOff(); - assertThatIllegalArgumentException().isThrownBy(() -> - backOff.setMultiplier(0.9)); + assertThatIllegalArgumentException().isThrownBy(() -> backOff.setMultiplier(0.9)); } @Test diff --git a/spring-core/src/test/java/org/springframework/util/FixedBackOffTests.java b/spring-core/src/test/java/org/springframework/util/FixedBackOffTests.java index cc0e8fedbe..5e7c5b8751 100644 --- a/spring-core/src/test/java/org/springframework/util/FixedBackOffTests.java +++ b/spring-core/src/test/java/org/springframework/util/FixedBackOffTests.java @@ -24,6 +24,8 @@ import org.springframework.util.backoff.FixedBackOff; import static org.assertj.core.api.Assertions.assertThat; /** + * Tests for {@link FixedBackOff}. + * * @author Stephane Nicoll */ class FixedBackOffTests { @@ -54,14 +56,17 @@ class FixedBackOffTests { } @Test - void startReturnDifferentInstances() { + void startReturnsDifferentInstances() { FixedBackOff backOff = new FixedBackOff(100L, 1); - BackOffExecution execution = backOff.start(); + + BackOffExecution execution1 = backOff.start(); BackOffExecution execution2 = backOff.start(); - assertThat(execution.nextBackOff()).isEqualTo(100L); + assertThat(execution1).isNotSameAs(execution2); + + assertThat(execution1.nextBackOff()).isEqualTo(100L); assertThat(execution2.nextBackOff()).isEqualTo(100L); - assertThat(execution.nextBackOff()).isEqualTo(BackOffExecution.STOP); + assertThat(execution1.nextBackOff()).isEqualTo(BackOffExecution.STOP); assertThat(execution2.nextBackOff()).isEqualTo(BackOffExecution.STOP); }