Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support of Duration in RetryTemplateBuilder #344

Merged
merged 2 commits into from
May 31, 2023
Merged

Add support of Duration in RetryTemplateBuilder #344

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cba48e
Introduce java.time.Duration
aahlenst May 25, 2023
ae51a22
Fix non-null assertion messages
aahlenst May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 114 additions & 4 deletions src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.retry.support;

import java.time.Duration;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -83,6 +84,7 @@
* @author Aleksandr Shamukov
* @author Artem Bilan
* @author Kim In Hoi
* @author Andreas Ahlenstorf
* @since 1.3
*/
public class RetryTemplateBuilder {
Expand Down Expand Up @@ -122,16 +124,43 @@ public RetryTemplateBuilder maxAttempts(int maxAttempts) {
* @param timeout whole execution timeout in milliseconds
* @return this
* @see TimeoutRetryPolicy
* @deprecated Use {@link #withTimeout(long)} instead.
*/
@Deprecated(since = "2.0.2", forRemoval = true)
public RetryTemplateBuilder withinMillis(long timeout) {
Assert.isTrue(timeout > 0, "Timeout should be positive");
return withTimeout(timeout);
}

/**
* Retry until {@code timeout} has passed since the initial attempt.
* @param timeoutMillis timeout in milliseconds
* @return this
* @see TimeoutRetryPolicy
* @throws IllegalArgumentException if timeout is {@literal <=} 0 or if another retry
* policy has already been configured.
* @since 2.0.2
*/
public RetryTemplateBuilder withTimeout(long timeoutMillis) {
Assert.isTrue(timeoutMillis > 0, "timeoutMillis should be greater than 0");
Assert.isNull(this.baseRetryPolicy, "You have already selected another retry policy");
TimeoutRetryPolicy timeoutRetryPolicy = new TimeoutRetryPolicy();
timeoutRetryPolicy.setTimeout(timeout);
this.baseRetryPolicy = timeoutRetryPolicy;
this.baseRetryPolicy = new TimeoutRetryPolicy(timeoutMillis);
return this;
}

/**
* Retry until {@code timeout} has passed since the initial attempt.
* @param timeout duration for how long retries should be attempted
* @return this
* @see TimeoutRetryPolicy
* @throws IllegalArgumentException if timeout is {@code null} or 0, or if another
* retry policy has already been configured.
* @since 2.0.2
*/
public RetryTemplateBuilder withTimeout(Duration timeout) {
artembilan marked this conversation as resolved.
Show resolved Hide resolved
Assert.notNull(timeout, "timeout is null");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure where you took this pattern from, but it would be better to give an instruction in this message.
When exception is thrown from that Assert.notNull is is already clear that something is wrong: we only just need to say how to fix: "'timeout' must not be null".

And fix the rest of similar messages in this class, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better?

return withTimeout(timeout.toMillis());
}

/**
* Allows infinite retry, do not limit attempts by number or time.
* <p>
Expand Down Expand Up @@ -180,6 +209,27 @@ public RetryTemplateBuilder exponentialBackoff(long initialInterval, double mult
return exponentialBackoff(initialInterval, multiplier, maxInterval, false);
}

/**
* Use exponential backoff policy. The formula of backoff period:
* <p>
* {@code currentInterval = Math.min(initialInterval * Math.pow(multiplier, retryNum), maxInterval)}
* <p>
* (for first attempt retryNum = 0)
* @param initialInterval initial sleep duration
* @param multiplier backoff interval multiplier
* @param maxInterval maximum backoff duration
* @return this
* @see ExponentialBackOffPolicy
* @throws IllegalArgumentException if initialInterval is {@code null}, multiplier is
* {@literal <=} 1, or if maxInterval is {@code null}
* @since 2.0.2
*/
public RetryTemplateBuilder exponentialBackoff(Duration initialInterval, double multiplier, Duration maxInterval) {
Assert.notNull(initialInterval, "initialInterval is null");
Assert.notNull(maxInterval, "maxInterval is null");
return exponentialBackoff(initialInterval.toMillis(), multiplier, maxInterval.toMillis(), false);
}

/**
* Use exponential backoff policy. The formula of backoff period (without randomness):
* <p>
Expand Down Expand Up @@ -210,6 +260,31 @@ public RetryTemplateBuilder exponentialBackoff(long initialInterval, double mult
return this;
}

/**
* Use exponential backoff policy. The formula of backoff period (without randomness):
* <p>
* {@code currentInterval = Math.min(initialInterval * Math.pow(multiplier, retryNum), maxInterval)}
* <p>
* (for first attempt retryNum = 0)
* @param initialInterval initial sleep duration
* @param multiplier backoff interval multiplier
* @param maxInterval maximum backoff duration
* @param withRandom adds some randomness to backoff intervals. For details, see
* {@link ExponentialRandomBackOffPolicy}
* @return this
* @see ExponentialBackOffPolicy
* @see ExponentialRandomBackOffPolicy
* @throws IllegalArgumentException if initialInterval is {@code null}, multiplier is
* {@literal <=} 1, or maxInterval is {@code null}
* @since 2.0.2
*/
public RetryTemplateBuilder exponentialBackoff(Duration initialInterval, double multiplier, Duration maxInterval,
boolean withRandom) {
Assert.notNull(initialInterval, "initialInterval is null");
Assert.notNull(maxInterval, "maxInterval is null");
return this.exponentialBackoff(initialInterval.toMillis(), multiplier, maxInterval.toMillis(), withRandom);
}

/**
* Perform each retry after fixed amount of time.
* @param interval fixed interval in milliseconds
Expand All @@ -225,6 +300,24 @@ public RetryTemplateBuilder fixedBackoff(long interval) {
return this;
}

/**
* Perform each retry after fixed amount of time.
* @param interval fixed backoff duration
* @return this
* @see FixedBackOffPolicy
* @throws IllegalArgumentException if another backoff policy has already been
* configured, interval is {@code null} or less than 1 millisecond
* @since 2.0.2
*/
public RetryTemplateBuilder fixedBackoff(Duration interval) {
Assert.notNull(interval, "interval is null");

long millis = interval.toMillis();
Assert.isTrue(millis >= 1, "interval is less than 1 millisecond");

return this.fixedBackoff(millis);
}

/**
* Use {@link UniformRandomBackOffPolicy}, see it's doc for details.
* @param minInterval in milliseconds
Expand All @@ -244,6 +337,23 @@ public RetryTemplateBuilder uniformRandomBackoff(long minInterval, long maxInter
return this;
}

/**
* Use {@link UniformRandomBackOffPolicy}.
* @param minInterval minimum backoff duration
* @param maxInterval maximum backoff duration
* @return this
* @see UniformRandomBackOffPolicy
* @throws IllegalArgumentException if minInterval is {@code null} or {@literal <} 1,
* maxInterval is {@code null} or {@literal <} 1, maxInterval {@literal >=}
* minInterval or if another backoff policy has already been configured.
* @since 2.0.2
*/
public RetryTemplateBuilder uniformRandomBackoff(Duration minInterval, Duration maxInterval) {
Assert.notNull(minInterval, "minInterval is null");
Assert.notNull(maxInterval, "maxInterval is null");
return this.uniformRandomBackoff(minInterval.toMillis(), maxInterval.toMillis());
}

/**
* Do not pause between attempts, retry immediately.
* @return this
Expand Down
95 changes: 94 additions & 1 deletion src/test/java/org/springframework/retry/support/RetryTemplateBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.FileNotFoundException;
import java.io.IOException;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand All @@ -29,6 +30,7 @@
import org.springframework.retry.RetryPolicy;
import org.springframework.retry.backoff.ExponentialBackOffPolicy;
import org.springframework.retry.backoff.ExponentialRandomBackOffPolicy;
import org.springframework.retry.backoff.FixedBackOffPolicy;
import org.springframework.retry.backoff.NoBackOffPolicy;
import org.springframework.retry.backoff.UniformRandomBackOffPolicy;
import org.springframework.retry.policy.AlwaysRetryPolicy;
Expand All @@ -53,6 +55,7 @@
* @author Aleksandr Shamukov
* @author Kim In Hoi
* @author Gary Russell
* @author Andreas Ahlenstorf
*/
public class RetryTemplateBuilderTests {

Expand Down Expand Up @@ -113,10 +116,11 @@ public void testBasicCustomization() {
@Test
public void testFailOnRetryPoliciesConflict() {
assertThatIllegalArgumentException()
.isThrownBy(() -> RetryTemplate.builder().maxAttempts(3).withinMillis(1000).build());
.isThrownBy(() -> RetryTemplate.builder().maxAttempts(3).withTimeout(1000).build());
}

@Test
@SuppressWarnings("removal")
public void testTimeoutPolicy() {
RetryTemplate template = RetryTemplate.builder().withinMillis(10000).build();

Expand All @@ -127,6 +131,28 @@ public void testTimeoutPolicy() {
assertThat(((TimeoutRetryPolicy) policyTuple.baseRetryPolicy).getTimeout()).isEqualTo(10000);
}

@Test
public void testTimeoutMillis() {
RetryTemplate template = RetryTemplate.builder().withTimeout(10000).build();

PolicyTuple policyTuple = PolicyTuple.extractWithAsserts(template);
assertDefaultClassifier(policyTuple);

assertThat(policyTuple.baseRetryPolicy).isInstanceOf(TimeoutRetryPolicy.class);
assertThat(((TimeoutRetryPolicy) policyTuple.baseRetryPolicy).getTimeout()).isEqualTo(10000);
}

@Test
public void testTimeoutDuration() {
RetryTemplate template = RetryTemplate.builder().withTimeout(Duration.ofSeconds(3)).build();

PolicyTuple policyTuple = PolicyTuple.extractWithAsserts(template);
assertDefaultClassifier(policyTuple);

assertThat(policyTuple.baseRetryPolicy).isInstanceOf(TimeoutRetryPolicy.class);
assertThat(((TimeoutRetryPolicy) policyTuple.baseRetryPolicy).getTimeout()).isEqualTo(3000);
}

@Test
public void testInfiniteRetry() {
RetryTemplate template = RetryTemplate.builder().infiniteRetry().build();
Expand Down Expand Up @@ -190,24 +216,91 @@ public void testFailOnBackOffPolicyConflict() {
.isThrownBy(() -> RetryTemplate.builder().noBackoff().fixedBackoff(1000).build());
}

@Test
public void testFixedBackoff() {
RetryTemplate template = RetryTemplate.builder().fixedBackoff(200).build();
FixedBackOffPolicy policy = getPropertyValue(template, "backOffPolicy", FixedBackOffPolicy.class);

assertThat(policy.getBackOffPeriod()).isEqualTo(200);
}

@Test
public void testFixedBackoffDuration() {
RetryTemplate template = RetryTemplate.builder().fixedBackoff(Duration.ofSeconds(1)).build();
FixedBackOffPolicy policy = getPropertyValue(template, "backOffPolicy", FixedBackOffPolicy.class);

assertThat(policy.getBackOffPeriod()).isEqualTo(1000);
}

@Test
public void testUniformRandomBackOff() {
RetryTemplate template = RetryTemplate.builder().uniformRandomBackoff(10, 100).build();
assertThat(getPropertyValue(template, "backOffPolicy")).isInstanceOf(UniformRandomBackOffPolicy.class);
}

@Test
public void testUniformRandomBackOffDuration() {
RetryTemplate template = RetryTemplate.builder()
.uniformRandomBackoff(Duration.ofSeconds(1), Duration.ofSeconds(2))
.build();

UniformRandomBackOffPolicy policy = getPropertyValue(template, "backOffPolicy",
UniformRandomBackOffPolicy.class);

assertThat(policy.getMinBackOffPeriod()).isEqualTo(1000);
assertThat(policy.getMaxBackOffPeriod()).isEqualTo(2000);
}

@Test
public void testNoBackOff() {
RetryTemplate template = RetryTemplate.builder().noBackoff().build();
assertThat(getPropertyValue(template, "backOffPolicy")).isInstanceOf(NoBackOffPolicy.class);
}

@Test
public void testExponentialBackoff() {
RetryTemplate template = RetryTemplate.builder().exponentialBackoff(10, 2, 500).build();
ExponentialBackOffPolicy policy = getPropertyValue(template, "backOffPolicy", ExponentialBackOffPolicy.class);

assertThat(policy.getInitialInterval()).isEqualTo(10);
assertThat(policy.getMultiplier()).isEqualTo(2);
assertThat(policy.getMaxInterval()).isEqualTo(500);
}

@Test
public void testExponentialBackoffDuration() {
RetryTemplate template = RetryTemplate.builder()
.exponentialBackoff(Duration.ofSeconds(2), 2, Duration.ofSeconds(3))
.build();

ExponentialBackOffPolicy policy = getPropertyValue(template, "backOffPolicy", ExponentialBackOffPolicy.class);

assertThat(policy.getInitialInterval()).isEqualTo(2000);
assertThat(policy.getMultiplier()).isEqualTo(2);
assertThat(policy.getMaxInterval()).isEqualTo(3000);
assertThat(policy.getMaxInterval()).isEqualTo(3000);
}

@Test
public void testExpBackOffWithRandom() {
RetryTemplate template = RetryTemplate.builder().exponentialBackoff(10, 2, 500, true).build();
assertThat(getPropertyValue(template, "backOffPolicy")).isInstanceOf(ExponentialRandomBackOffPolicy.class);
}

@Test
public void testExponentialRandomBackoffDuration() {
RetryTemplate template = RetryTemplate.builder()
.exponentialBackoff(Duration.ofSeconds(2), 2, Duration.ofSeconds(3), true)
.build();

ExponentialRandomBackOffPolicy policy = getPropertyValue(template, "backOffPolicy",
ExponentialRandomBackOffPolicy.class);

assertThat(policy.getInitialInterval()).isEqualTo(2000);
assertThat(policy.getMultiplier()).isEqualTo(2);
assertThat(policy.getMaxInterval()).isEqualTo(3000);
}

@Test
public void testValidateInitAndMax() {
assertThatIllegalArgumentException()
Expand Down