Skip to content

only use specified listeners in @Retryable #493

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,16 @@ public class Application {

@Service
class Service {
@Retryable(RemoteAccessException.class)
@Retryable(retryFor = RemoteAccessException.class, listeners = { "retryListener1", "retryListener2" })
public service() {
// ... do something
}
}
```

Starting with version 2.0.13, any bean instance of `RetryListener` are not considered as global listeners.
You always have to explicitly specify the `listeners` attribute in `@Retryable` if needed.
Copy link
Member

Choose a reason for hiding this comment

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

And this is exactly why it has not been fixed yet.
This is a breaking change which we don't do according to our backward compatibility support.
This makes sense for a major version and we are not there yet.
We can keep your PR for time being though!

Copy link
Author

Choose a reason for hiding this comment

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

got it. thank you


You can use the attributes of `@Retryable` to control the `RetryPolicy` and `BackoffPolicy`, as follows:

```java
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand Down Expand Up @@ -77,6 +78,7 @@
* @author Gary Russell
* @author Roman Akentev
* @author Aftab Shaikh
* @author Jiandong Ma
* @since 1.1
*/
public class AnnotationAwareRetryOperationsInterceptor implements IntroductionInterceptor, BeanFactoryAware {
Expand Down Expand Up @@ -134,16 +136,6 @@ public void setNewItemIdentifier(NewMethodArgumentsIdentifier newMethodArguments
this.newMethodArgumentsIdentifier = newMethodArgumentsIdentifier;
}

/**
* Default retry listeners to apply to all operations.
* @param globalListeners the default listeners
*/
public void setListeners(Collection<RetryListener> globalListeners) {
ArrayList<RetryListener> retryListeners = new ArrayList<>(globalListeners);
AnnotationAwareOrderComparator.sort(retryListeners);
this.globalListeners = retryListeners.toArray(new RetryListener[0]);
}

@Override
public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
this.beanFactory = beanFactory;
Expand Down Expand Up @@ -319,21 +311,17 @@ private RetryTemplate createTemplate(String[] listenersBeanNames) {
if (listenersBeanNames.length > 0) {
template.setListeners(getListenersBeans(listenersBeanNames));
}
else if (this.globalListeners != null) {
template.setListeners(this.globalListeners);
}
return template;
}

private RetryListener[] getListenersBeans(String[] listenersBeanNames) {
if (listenersBeanNames.length == 1 && "".equals(listenersBeanNames[0].trim())) {
return new RetryListener[0];
}
RetryListener[] listeners = new RetryListener[listenersBeanNames.length];
for (int i = 0; i < listeners.length; i++) {
listeners[i] = this.beanFactory.getBean(listenersBeanNames[i], RetryListener.class);
List<RetryListener> retryListeners = new ArrayList<>(listenersBeanNames.length);
for (String beanName : listenersBeanNames) {
if (StringUtils.hasText(beanName)) {
retryListeners.add(this.beanFactory.getBean(beanName, RetryListener.class));
}
}
return listeners;
return retryListeners.toArray(new RetryListener[0]);
}

private MethodInvocationRecoverer<?> getRecoverer(Object target, Method method) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* @author Dave Syer
* @author Artem Bilan
* @author Gary Russell
* @author Jiandong Ma
* @since 1.2
*
*/
Expand Down Expand Up @@ -199,4 +200,12 @@
@AliasFor(annotation = Retryable.class)
String recover() default "";

/**
* Bean names of retry listeners.
* @return retry listeners bean names
* @since 2.0.13
*/
@AliasFor(annotation = Retryable.class)
String[] listeners() default {};

}
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,15 @@
* @author Gary Russell
* @author Yanming Zhou
* @author Evgeny Lazarev
* @author Jiandong Ma
* @since 1.1
*
*/
@SuppressWarnings("serial")
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
@Component
public class RetryConfiguration extends AbstractPointcutAdvisor
implements IntroductionAdvisor, BeanFactoryAware, InitializingBean, SmartInitializingSingleton, ImportAware {
implements IntroductionAdvisor, BeanFactoryAware, InitializingBean, ImportAware {

@Nullable
protected AnnotationAttributes enableRetry;
Expand All @@ -88,8 +89,6 @@ public class RetryConfiguration extends AbstractPointcutAdvisor

private RetryContextCache retryContextCache;

private List<RetryListener> retryListeners;

private MethodArgumentsKeyGenerator methodArgumentsKeyGenerator;

private NewMethodArgumentsIdentifier newMethodArgumentsIdentifier;
Expand Down Expand Up @@ -120,17 +119,8 @@ public void afterPropertiesSet() throws Exception {
}
}

@Override
public void afterSingletonsInstantiated() {
this.retryListeners = findBeans(RetryListener.class);
if (this.retryListeners != null) {
this.advice.setListeners(this.retryListeners);
}
}

private <T> List<T> findBeans(Class<? extends T> type) {
if (this.beanFactory instanceof ListableBeanFactory) {
ListableBeanFactory listable = (ListableBeanFactory) this.beanFactory;
if (this.beanFactory instanceof ListableBeanFactory listable) {
if (listable.getBeanNamesForType(type).length > 0) {
ArrayList<T> list = new ArrayList<>(listable.getBeansOfType(type, false, false).values());
OrderComparator.sort(list);
Expand All @@ -141,8 +131,7 @@ private <T> List<T> findBeans(Class<? extends T> type) {
}

private <T> T findBean(Class<? extends T> type) {
if (this.beanFactory instanceof ListableBeanFactory) {
ListableBeanFactory listable = (ListableBeanFactory) this.beanFactory;
if (this.beanFactory instanceof ListableBeanFactory listable) {
if (listable.getBeanNamesForType(type, false, false).length == 1) {
return listable.getBean(type);
}
Expand Down Expand Up @@ -237,10 +226,9 @@ public boolean equals(Object other) {
if (this == other) {
return true;
}
if (!(other instanceof AnnotationClassOrMethodPointcut)) {
if (!(other instanceof AnnotationClassOrMethodPointcut otherAdvisor)) {
return false;
}
AnnotationClassOrMethodPointcut otherAdvisor = (AnnotationClassOrMethodPointcut) other;
return ObjectUtils.nullSafeEquals(this.methodResolver, otherAdvisor.methodResolver);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
* @author Gary Russell
* @author Maksim Kita
* @author Roman Akentev
* @author Jiandong Ma
* @since 1.1
*
*/
Expand Down Expand Up @@ -165,10 +166,7 @@
String exceptionExpression() default "";

/**
* Bean names of retry listeners to use instead of default ones defined in Spring
* context. If this attribute is set to an empty string {@code ""}, it will
* effectively exclude all retry listeners, including with the default listener beans,
* from being used.
* Bean names of retry listeners.
* @return retry listeners bean names
*/
String[] listeners() default {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
* @author Yanming Zhou
* @author Anton Aharkau
* @author Emanuele Ivaldi
* @author Jiandong Ma
* @since 1.1
*/
public class EnableRetryTests {
Expand All @@ -77,7 +78,7 @@ public void vanilla() {
TestConfiguration config = context.getBean(TestConfiguration.class);
assertThat(config.listener1).isTrue();
assertThat(config.listener2).isTrue();
assertThat(config.twoFirst).isTrue();
assertThat(config.twoFirst).isFalse();
context.close();
}

Expand Down Expand Up @@ -590,7 +591,7 @@ protected static class Service {

private int count = 0;

@Retryable(RuntimeException.class)
@Retryable(retryFor = RuntimeException.class, listeners = { "listener1", "listener2" })
public void service() {
if (this.count++ < 2) {
throw new RuntimeException("Planned");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ protected static class Service {

private int count = 0;

@Retryable(backoff = @Backoff(delay = 1000))
@Retryable(backoff = @Backoff(delay = 1000), listeners = "listener")
public void service() {
if (count++ < 2) {
throw new RuntimeException("Planned");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
/**
* @author Dave Syer
* @author Artem Bilan
* @author Jiandong Ma
*
*/
public class CircuitBreakerInterceptorStatisticsTests {
Expand Down Expand Up @@ -107,7 +108,7 @@ protected static class Service {

private RetryContext status;

@CircuitBreaker(label = "test", maxAttempts = 1, recover = "recover")
@CircuitBreaker(label = "test", maxAttempts = 1, recover = "recover", listeners = "listener")
public Object service(String input) throws Exception {
this.status = RetrySynchronizationManager.getContext();
Integer attempts = (Integer) status.getAttribute("attempts");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,19 @@ protected static class Service {

private int count = 0;

@Retryable
@Retryable(listeners = "metricsRetryListener")
public void service1() {

}

@Retryable
@Retryable(listeners = "metricsRetryListener")
public void service2() {
if (count++ < 2) {
throw new RuntimeException("Planned");
}
}

@Retryable
@Retryable(listeners = "metricsRetryListener")
public void service3() {
throw new RetryException("Planned");
}
Expand Down