Skip to content

Commit 0e6f529

Browse files
committed
Add warnings for invalid setter values in ExponentialBackoffPolicy
- Add warning logs when setter values don't meet expected constraints - Maintain backward compatibility by not changing behavior Fixes #352 Signed-off-by: kssumin <[email protected]>
1 parent 47a2359 commit 0e6f529

File tree

2 files changed

+73
-1
lines changed

2 files changed

+73
-1
lines changed

src/main/java/org/springframework/retry/backoff/ExponentialBackOffPolicy.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,12 @@
4343
* @author Artem Bilan
4444
* @author Marius Lichtblau
4545
* @author Anton Aharkau
46+
* @author Kim Sumin
4647
*/
4748
@SuppressWarnings("serial")
4849
public class ExponentialBackOffPolicy implements SleepingBackOffPolicy<ExponentialBackOffPolicy> {
4950

50-
protected final Log logger = LogFactory.getLog(this.getClass());
51+
protected Log logger = LogFactory.getLog(this.getClass());
5152

5253
/**
5354
* The default 'initialInterval' value - 100 millisecs. Coupled with the default
@@ -130,6 +131,11 @@ protected void cloneValues(ExponentialBackOffPolicy target) {
130131
* @param initialInterval the initial interval
131132
*/
132133
public void setInitialInterval(long initialInterval) {
134+
if (initialInterval < 1) {
135+
logger.warn("The initial interval value " + initialInterval +
136+
" is less than 1. " +
137+
"The initial interval should be at least 1. ");
138+
}
133139
this.initialInterval = initialInterval > 1 ? initialInterval : 1;
134140
}
135141

@@ -139,6 +145,12 @@ public void setInitialInterval(long initialInterval) {
139145
* @param multiplier the multiplier
140146
*/
141147
public void setMultiplier(double multiplier) {
148+
if (multiplier <= 1.0) {
149+
logger.warn("The multiplier value " + multiplier +
150+
" is less than or equal to 1.0. " +
151+
"For exponential backoff to work effectively, " +
152+
"the multiplier should be greater than 1.0. ");
153+
}
142154
this.multiplier = multiplier > 1.0 ? multiplier : 1.0;
143155
}
144156

@@ -150,6 +162,11 @@ public void setMultiplier(double multiplier) {
150162
* @param maxInterval in milliseconds.
151163
*/
152164
public void setMaxInterval(long maxInterval) {
165+
if (maxInterval < 1) {
166+
logger.warn("The max interval value " + maxInterval +
167+
" is less than or equal to 0. " +
168+
"The max interval should be positive.");
169+
}
153170
this.maxInterval = maxInterval > 0 ? maxInterval : 1;
154171
}
155172

@@ -252,6 +269,18 @@ public void backOff(BackOffContext backOffContext) throws BackOffInterruptedExce
252269
}
253270
}
254271

272+
/**
273+
* Setter for {@link Log}. If not applied the following is used:
274+
* <p>
275+
* {@code LogFactory.getLog(getClass())}
276+
* </p>
277+
* @param logger the logger the exponential backoff policy uses for logging
278+
* @since 2.0.x
279+
*/
280+
public void setLogger(Log logger) {
281+
this.logger = logger;
282+
}
283+
255284
static class ExponentialBackOffContext implements BackOffContext {
256285

257286
private final double multiplier;

src/test/java/org/springframework/retry/backoff/ExponentialBackOffPolicyTests.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,21 @@
1616

1717
package org.springframework.retry.backoff;
1818

19+
import org.apache.commons.logging.Log;
1920
import org.junit.jupiter.api.Test;
21+
import org.mockito.ArgumentCaptor;
2022

2123
import static org.assertj.core.api.Assertions.assertThat;
2224
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
25+
import static org.mockito.Mockito.*;
2326

2427
/**
2528
* @author Rob Harrop
2629
* @author Dave Syer
2730
* @author Gary Russell
2831
* @author Marius Lichtblau
2932
* @author Anton Aharkau
33+
* @author Kim Sumin
3034
*/
3135
public class ExponentialBackOffPolicyTests {
3236

@@ -125,4 +129,43 @@ public void sleep(long backOffPeriod) throws InterruptedException {
125129
assertThat(Thread.interrupted()).isTrue();
126130
}
127131

132+
@Test
133+
public void testSetMultiplierWithWarning() {
134+
Log logMock = mock(Log.class);
135+
ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
136+
strategy.setLogger(logMock);
137+
138+
strategy.setMultiplier(1.0);
139+
140+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
141+
verify(logMock).warn(captor.capture());
142+
assertThat(captor.getValue()).contains("The multiplier value 1.0 is less than or equal to 1.0");
143+
}
144+
145+
146+
@Test
147+
public void testSetInitialIntervalWithWarning() {
148+
Log logMock = mock(Log.class);
149+
ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
150+
strategy.setLogger(logMock);
151+
152+
strategy.setInitialInterval(0);
153+
154+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
155+
verify(logMock).warn(captor.capture());
156+
assertThat(captor.getValue()).contains("The initial interval value 0 is less than 1. The initial interval should be at least 1.");
157+
}
158+
159+
@Test
160+
public void testSetMaxIntervalWithWarning() {
161+
Log logMock = mock(Log.class);
162+
ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
163+
strategy.setLogger(logMock);
164+
165+
strategy.setMaxInterval(0);
166+
167+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
168+
verify(logMock).warn(captor.capture());
169+
assertThat(captor.getValue()).contains("The max interval value 0 is less than or equal to 0. The max interval should be positive.");
170+
}
128171
}

0 commit comments

Comments
 (0)