Skip to content

Commit

Permalink
feat: setter for Log of RetryTemplate (#471)
Browse files Browse the repository at this point in the history
GH-470: Add `RetryTemplate.setLogger()` to avoid reflection in other places

Fixes: #470
Issue link: #470

Spring Cloud Config does mutation in the `RetryTemplate` for its system loading logger via `RetryTemplateFactory`.

* Expose setter for `logger` property to avoid reflection.
* Add `RetryTemplateBuilder.withLogger()` for convenience
  • Loading branch information
klopfdreh authored Sep 19, 2024
1 parent a34a627 commit 4886b75
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
* @author Josh Long
* @author Aleksandr Shamukov
* @author Emanuele Ivaldi
* @author Tobias Soloschenko
*/
public class RetryTemplate implements RetryOperations {

Expand All @@ -87,7 +88,7 @@ public class RetryTemplate implements RetryOperations {
*/
private static final String GLOBAL_STATE = "state.global";

protected final Log logger = LogFactory.getLog(getClass());
protected Log logger = LogFactory.getLog(getClass());

private volatile BackOffPolicy backOffPolicy = new NoBackOffPolicy();

Expand Down Expand Up @@ -186,6 +187,18 @@ public boolean hasListeners() {
return this.listeners.length > 0;
}

/**
* Setter for {@link Log}. If not applied the following is used:
* <p>
* {@code LogFactory.getLog(getClass())}
* </p>
* @param logger the logger the retry template uses for logging
* @since 2.0.10
*/
public void setLogger(Log logger) {
this.logger = logger;
}

/**
* Setter for {@link BackOffPolicy}.
* @param backOffPolicy the {@link BackOffPolicy}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.List;
import java.util.function.Predicate;

import org.apache.commons.logging.Log;
import org.springframework.classify.BinaryExceptionClassifier;
import org.springframework.classify.BinaryExceptionClassifierBuilder;
import org.springframework.retry.RetryListener;
Expand Down Expand Up @@ -79,12 +80,15 @@
* @author Kim In Hoi
* @author Andreas Ahlenstorf
* @author Morulai Planinski
* @author Tobias Soloschenko
* @since 1.3
*/
public class RetryTemplateBuilder {

private RetryPolicy baseRetryPolicy;

private Log logger;

private BackOffPolicy backOffPolicy;

private List<RetryListener> listeners;
Expand Down Expand Up @@ -288,6 +292,23 @@ public RetryTemplateBuilder exponentialBackoff(Duration initialInterval, double
return this.exponentialBackoff(initialInterval.toMillis(), multiplier, maxInterval.toMillis(), withRandom);
}

/**
* Applies a dedicated logger to the {@link RetryTemplate}. If not applied the
* following is used:
* <p>
* {@code LogFactory.getLog(getClass())}
* </p>
* @param logger the logger which should be used for logging
* @return this
* @since 2.0.10
*/
public RetryTemplateBuilder withLogger(Log logger) {
Assert.isNull(this.logger, "You have already applied a logger");
Assert.notNull(logger, "The given logger should not be null");
this.logger = logger;
return this;
}

/**
* Perform each retry after a fixed amount of time.
* @param interval fixed interval in milliseconds
Expand Down Expand Up @@ -584,6 +605,12 @@ public RetryTemplate build() {
finalPolicy.setPolicies(new RetryPolicy[] { this.baseRetryPolicy, exceptionRetryPolicy });
retryTemplate.setRetryPolicy(finalPolicy);

// Logger

if (this.logger != null) {
retryTemplate.setLogger(this.logger);
}

// Backoff policy

if (this.backOffPolicy == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.List;
import java.util.function.Predicate;

import org.apache.commons.logging.Log;
import org.junit.jupiter.api.Test;
import org.springframework.classify.BinaryExceptionClassifier;
import org.springframework.retry.RetryListener;
Expand Down Expand Up @@ -58,6 +59,7 @@
* @author Gary Russell
* @author Andreas Ahlenstorf
* @author Morulai Planinski
* @author Tobias Soloschenko
*/
public class RetryTemplateBuilderTests {

Expand Down Expand Up @@ -346,6 +348,21 @@ public void testValidateZeroInitInterval() {
.isThrownBy(() -> RetryTemplate.builder().exponentialBackoff(0, 2, 200).build());
}

@Test
public void testBuilderWithLogger() {
Log logMock = mock(Log.class);
RetryTemplate retryTemplate = RetryTemplate.builder().withLogger(logMock).build();
Log logger = getPropertyValue(retryTemplate, "logger", Log.class);
assertThat(logger).isEqualTo(logMock);
}

@Test
public void testBuilderWithDefaultLogger() {
RetryTemplate retryTemplate = RetryTemplate.builder().build();
Log logger = getPropertyValue(retryTemplate, "logger", Log.class);
assertThat(logger).isNotNull();
}

/* ---------------- Utils -------------- */

private static class PolicyTuple {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

import org.apache.commons.logging.Log;
import org.junit.jupiter.api.Test;

import org.mockito.ArgumentCaptor;
import org.springframework.classify.BinaryExceptionClassifier;
import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryContext;
Expand All @@ -45,6 +47,7 @@
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* @author Rob Harrop
Expand All @@ -53,6 +56,7 @@
* @author Henning Pöttker
* @author Emanuele Ivaldi
* @author Morulai Planinski
* @author Tobias Soloschenko
*/
public class RetryTemplateTests {

Expand Down Expand Up @@ -286,7 +290,6 @@ public void testRethrowError() {
}
}

@SuppressWarnings("serial")
@Test
public void testFailedPolicy() {
RetryTemplate retryTemplate = new RetryTemplate();
Expand Down Expand Up @@ -325,7 +328,6 @@ public void testNoBackOffForRethrownException() {
tested.setRetryPolicy(new SimpleRetryPolicy(1));

BackOffPolicy bop = mock(BackOffPolicy.class);
@SuppressWarnings("serial")
BackOffContext backOffContext = new BackOffContext() {
};
tested.setBackOffPolicy(bop);
Expand Down Expand Up @@ -439,4 +441,17 @@ public void backOff(BackOffContext backOffContext) throws BackOffInterruptedExce

}

@Test
public void testLoggingAppliedCorrectly() throws Exception {
ArgumentCaptor<String> logOutputCaptor = ArgumentCaptor.forClass(String.class);
RetryTemplate retryTemplate = new RetryTemplate();
Log logMock = mock(Log.class);
when(logMock.isTraceEnabled()).thenReturn(false);
when(logMock.isDebugEnabled()).thenReturn(true);
retryTemplate.setLogger(logMock);
retryTemplate.execute(new MockRetryCallback());
verify(logMock).debug(logOutputCaptor.capture());
assertThat(logOutputCaptor.getValue()).contains("Retry: count=0");
}

}

0 comments on commit 4886b75

Please sign in to comment.