Spring/spring-retry
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GH-244: Invoke Proxy for @Recover Method

Browse Source 
Resolves https://github.com/spring-projects/spring-retry/issues/244

The `RecoverAnnotationRecoveryHandler` only has access to the undecorated
raw bean.

Use the `RetryContext` to make the proxy (if CGLIB) available to the recoverer.

* Move cleanup to a finally block.

* Support JDK proxies too.

* Add CGLib proxy test; JDK proxies are covered by existing tests.
This commit is contained in:
Gary Russell
2021-06-07 13:34:33 -04:00
committed by GitHub
parent e637911a8a
commit 83a73d0cc8
3 changed files with 119 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java
View File

@@ -25,7 +25,9 @@ import java.util.Map;
import org.springframework.classify.SubclassClassifier;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.retry.ExhaustedRetryException;
import org.springframework.retry.RetryContext;
import org.springframework.retry.interceptor.MethodInvocationRecoverer;
import org.springframework.retry.support.RetrySynchronizationManager;
import org.springframework.util.ReflectionUtils;
import org.springframework.util.ReflectionUtils.MethodCallback;
import org.springframework.util.StringUtils;
@@ -76,8 +78,25 @@ public class RecoverAnnotationRecoveryHandler<T> implements MethodInvocationReco
boolean methodAccessible = method.isAccessible();
try {
ReflectionUtils.makeAccessible(method);
RetryContext context = RetrySynchronizationManager.getContext();
Object proxy = null;
if (context != null) {
proxy = context.getAttribute("___proxy___");
if (proxy != null) {
Method proxyMethod = findMethodOnProxy(method, proxy);
if (proxyMethod == null) {
proxy = null;
}
else {
method = proxyMethod;
}
}
}
if (proxy == null) {
proxy = this.target;
}
@SuppressWarnings("unchecked")
T result = (T) ReflectionUtils.invokeMethod(method, this.target, argsToUse);
T result = (T) ReflectionUtils.invokeMethod(method, proxy, argsToUse);
return result;
}
finally {
@@ -87,6 +106,18 @@ public class RecoverAnnotationRecoveryHandler<T> implements MethodInvocationReco
}
}
private Method findMethodOnProxy(Method method, Object proxy) {
try {
return proxy.getClass().getMethod(method.getName(), method.getParameterTypes());
}
catch (NoSuchMethodException e) {
return null;
}
catch (SecurityException e) {
return null;
}
}
private Method findClosestMatch(Object[] args, Class<? extends Throwable> cause) {
Method result = null;

33
src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java
View File

@@ -23,6 +23,7 @@ import org.springframework.retry.RecoveryCallback;
import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryContext;
import org.springframework.retry.RetryOperations;
import org.springframework.retry.support.RetrySynchronizationManager;
import org.springframework.retry.support.RetryTemplate;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
@@ -64,11 +65,12 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
this.recoverer = recoverer;
}
@Override
public Object invoke(final MethodInvocation invocation) throws Throwable {
String name;
if (StringUtils.hasText(label)) {
name = label;
if (StringUtils.hasText(this.label)) {
name = this.label;
}
else {
name = invocation.getMethod().toGenericString();
@@ -78,9 +80,10 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
RetryCallback<Object, Throwable> retryCallback = new MethodInvocationRetryCallback<Object, Throwable>(
invocation, label) {
@Override
public Object doWithRetry(RetryContext context) throws Exception {
context.setAttribute(RetryContext.NAME, label);
context.setAttribute(RetryContext.NAME, this.label);
/*
* If we don't copy the invocation carefully it won't keep a reference to
@@ -88,9 +91,10 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
* specialise to ReflectiveMethodInvocation (but how often would another
* implementation come along?).
*/
if (invocation instanceof ProxyMethodInvocation) {
if (this.invocation instanceof ProxyMethodInvocation) {
context.setAttribute("___proxy___", ((ProxyMethodInvocation) this.invocation).getProxy());
try {
return ((ProxyMethodInvocation) invocation).invocableClone().proceed();
return ((ProxyMethodInvocation) this.invocation).invocableClone().proceed();
}
catch (Exception e) {
throw e;
@@ -111,9 +115,19 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
};
if (recoverer != null) {
ItemRecovererCallback recoveryCallback = new ItemRecovererCallback(invocation.getArguments(), recoverer);
return this.retryOperations.execute(retryCallback, recoveryCallback);
if (this.recoverer != null) {
ItemRecovererCallback recoveryCallback = new ItemRecovererCallback(invocation.getArguments(),
this.recoverer);
try {
Object recovered = this.retryOperations.execute(retryCallback, recoveryCallback);
return recovered;
}
finally {
RetryContext context = RetrySynchronizationManager.getContext();
if (context != null) {
context.removeAttribute("__proxy__");
}
}
}
return this.retryOperations.execute(retryCallback);
@@ -138,8 +152,9 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
this.recoverer = recoverer;
}
@Override
public Object recover(RetryContext context) {
return recoverer.recover(args, context.getLastThrowable());
return this.recoverer.recover(this.args, context.getLastThrowable());
}
}

63
src/test/java/org/springframework/retry/annotation/EnableRetryTests.java
View File

@@ -21,11 +21,14 @@ import java.util.Map;
import java.util.Properties;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.junit.Test;
import org.springframework.aop.framework.Advised;
import org.springframework.aop.support.AopUtils;
import org.springframework.beans.BeansException;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -89,6 +92,9 @@ public class EnableRetryTests {
TestProxyConfiguration.class);
Service service = context.getBean(Service.class);
assertTrue(AopUtils.isCglibProxy(service));
RecoverableService recoverable = context.getBean(RecoverableService.class);
recoverable.service();
assertTrue(recoverable.isOtherAdviceCalled());
context.close();
}
@@ -270,6 +276,53 @@ public class EnableRetryTests {
return new Service();
}
@Bean
public RecoverableService recoverable() {
return new RecoverableService();
}
@Bean
public static AdviceBPP bpp() {
return new AdviceBPP();
}
static class AdviceBPP implements BeanPostProcessor, Ordered {
@Override
public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
return bean;
}
@Override
public Object postProcessAfterInitialization(final Object bean, String beanName) throws BeansException {
if (bean instanceof RecoverableService) {
Advised advised = (Advised) bean;
advised.addAdvice(new MethodInterceptor() {
@Override
public Object invoke(MethodInvocation invocation) throws Throwable {
if (invocation.getMethod().getName().equals("recover")) {
((RecoverableService) bean).setOtherAdviceCalled();
}
return invocation.proceed();
}
});
return bean;
}
return bean;
}
@Override
public int getOrder() {
return Integer.MAX_VALUE;
}
}
}
@Configuration
@@ -475,6 +528,8 @@ public class EnableRetryTests {
private Throwable cause;
boolean otherAdviceCalled;
@Retryable(RuntimeException.class)
public void service() {
this.count++;
@@ -494,6 +549,14 @@ public class EnableRetryTests {
return this.count;
}
public void setOtherAdviceCalled() {
this.otherAdviceCalled = true;
}
public boolean isOtherAdviceCalled() {
return this.otherAdviceCalled;
}
}
@Retryable(RuntimeException.class)