From f54db742f54ec1d15edfe5bc9bd4e879289f806b Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 23 Oct 2018 12:01:39 -0400 Subject: [PATCH] Find @CircuitBreaker on target class w/ JDK proxy --- ...tationAwareRetryOperationsInterceptor.java | 24 ++++--- .../RecoverAnnotationRecoveryHandler.java | 72 ++++++++----------- .../retry/annotation/CircuitBreakerTests.java | 20 +++++- 3 files changed, 62 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index a39d543..9d6d2a1 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -30,6 +30,7 @@ import javax.naming.OperationNotSupportedException; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; + import org.springframework.aop.IntroductionInterceptor; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; @@ -167,7 +168,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn } private MethodInterceptor getDelegate(Object target, Method method) { - ConcurrentMap cachedMethods = delegates.get(target); + ConcurrentMap cachedMethods = this.delegates.get(target); if (cachedMethods == null) { cachedMethods = new ConcurrentHashMap(); } @@ -179,7 +180,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn retryable = AnnotatedElementUtils.findMergedAnnotation(method.getDeclaringClass(), Retryable.class); } if (retryable == null) { - retryable = findAnnotationOnTarget(target, method); + retryable = findAnnotationOnTarget(target, method, Retryable.class); } if (retryable != null) { if (StringUtils.hasText(retryable.interceptor())) { @@ -195,17 +196,17 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn cachedMethods.putIfAbsent(method, interceptor); delegate = cachedMethods.get(method); } - delegates.putIfAbsent(target, cachedMethods); + this.delegates.putIfAbsent(target, cachedMethods); return delegate == NULL_INTERCEPTOR ? null : delegate; } - private Retryable findAnnotationOnTarget(Object target, Method method) { + private A findAnnotationOnTarget(Object target, Method method, Class annotation) { + try { Method targetMethod = target.getClass().getMethod(method.getName(), method.getParameterTypes()); - Retryable retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod, Retryable.class); + A retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod, annotation); if (retryable == null) { - retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod.getDeclaringClass(), - Retryable.class); + retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod.getDeclaringClass(), annotation); } return retryable; @@ -228,6 +229,9 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn template.setRetryContextCache(this.retryContextCache); CircuitBreaker circuit = AnnotatedElementUtils.findMergedAnnotation(method, CircuitBreaker.class); + if (circuit == null) { + circuit = findAnnotationOnTarget(target, method, CircuitBreaker.class); + } if (circuit != null) { RetryPolicy policy = getRetryPolicy(circuit); CircuitBreakerRetryPolicy breaker = new CircuitBreakerRetryPolicy(policy); @@ -278,8 +282,8 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn if (listenersBeanNames.length > 0) { template.setListeners(getListenersBeans(listenersBeanNames)); } - else if (globalListeners != null) { - template.setListeners(globalListeners); + else if (this.globalListeners != null) { + template.setListeners(this.globalListeners); } return template; } @@ -287,7 +291,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn private RetryListener[] getListenersBeans(String[] listenersBeanNames) { RetryListener[] listeners = new RetryListener[listenersBeanNames.length]; for (int i = 0; i < listeners.length; i++) { - listeners[i] = beanFactory.getBean(listenersBeanNames[i], RetryListener.class); + listeners[i] = this.beanFactory.getBean(listenersBeanNames[i], RetryListener.class); } return listeners; } diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index 8cdc82f..21e67c2 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -104,8 +104,8 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco result = method; } else if (distance == min) { - boolean parametersMatch = compareParameters(args, - meta.getArgCount(), method.getParameterTypes()); + boolean parametersMatch = compareParameters(args, meta.getArgCount(), + method.getParameterTypes()); if (parametersMatch) { result = method; } @@ -118,8 +118,8 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco Method method = entry.getKey(); if (method.getName().equals(this.recoverMethodName)) { SimpleMetadata meta = entry.getValue(); - if (meta.type.isAssignableFrom(cause) && compareParameters(args, - meta.getArgCount(), method.getParameterTypes())) { + if (meta.type.isAssignableFrom(cause) + && compareParameters(args, meta.getArgCount(), method.getParameterTypes())) { result = method; break; } @@ -129,8 +129,7 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco return result; } - private int calculateDistance(Class cause, - Class type) { + private int calculateDistance(Class cause, Class type) { int result = 0; Class current = cause; while (current != type && current != Throwable.class) { @@ -140,17 +139,14 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco return result; } - private boolean compareParameters(Object[] args, int argCount, - Class[] parameterTypes) { + private boolean compareParameters(Object[] args, int argCount, Class[] parameterTypes) { if (argCount == (args.length + 1)) { int startingIndex = 0; - if (parameterTypes.length > 0 - && Throwable.class.isAssignableFrom(parameterTypes[0])) { + if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) { startingIndex = 1; } for (int i = startingIndex; i < parameterTypes.length; i++) { - final Object argument = i - startingIndex < args.length - ? args[i - startingIndex] : null; + final Object argument = i - startingIndex < args.length ? args[i - startingIndex] : null; if (argument == null) { continue; } @@ -170,41 +166,34 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco if (retryable != null) { this.recoverMethodName = retryable.recover(); } - ReflectionUtils.doWithMethods(failingMethod.getDeclaringClass(), - new MethodCallback() { - @Override - public void doWith(Method method) - throws IllegalArgumentException, IllegalAccessException { - Recover recover = AnnotationUtils.findAnnotation(method, - Recover.class); - if (recover != null && method.getReturnType() - .isAssignableFrom(failingMethod.getReturnType())) { - Class[] parameterTypes = method.getParameterTypes(); - if (parameterTypes.length > 0 && Throwable.class - .isAssignableFrom(parameterTypes[0])) { - @SuppressWarnings("unchecked") - Class type = (Class) parameterTypes[0]; - types.put(type, method); - RecoverAnnotationRecoveryHandler.this.methods.put(method, - new SimpleMetadata(parameterTypes.length, type)); - } - else { - RecoverAnnotationRecoveryHandler.this.classifier - .setDefaultValue(method); - RecoverAnnotationRecoveryHandler.this.methods.put(method, - new SimpleMetadata(parameterTypes.length, null)); - } - } + ReflectionUtils.doWithMethods(failingMethod.getDeclaringClass(), new MethodCallback() { + @Override + public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { + Recover recover = AnnotationUtils.findAnnotation(method, Recover.class); + if (recover != null && method.getReturnType().isAssignableFrom(failingMethod.getReturnType())) { + Class[] parameterTypes = method.getParameterTypes(); + if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) { + @SuppressWarnings("unchecked") + Class type = (Class) parameterTypes[0]; + types.put(type, method); + RecoverAnnotationRecoveryHandler.this.methods.put(method, + new SimpleMetadata(parameterTypes.length, type)); } - }); + else { + RecoverAnnotationRecoveryHandler.this.classifier.setDefaultValue(method); + RecoverAnnotationRecoveryHandler.this.methods.put(method, + new SimpleMetadata(parameterTypes.length, null)); + } + } + } + }); this.classifier.setTypeMap(types); optionallyFilterMethodsBy(failingMethod.getReturnType()); } private Recover findAnnotationOnTarget(Object target, Method method) { try { - Method targetMethod = target.getClass().getMethod(method.getName(), - method.getParameterTypes()); + Method targetMethod = target.getClass().getMethod(method.getName(), method.getParameterTypes()); return AnnotationUtils.findAnnotation(targetMethod, Recover.class); } catch (Exception e) { @@ -251,8 +240,7 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco result[0] = t; startArgs = 1; } - int length = result.length - startArgs > args.length ? args.length - : result.length - startArgs; + int length = result.length - startArgs > args.length ? args.length : result.length - startArgs; if (length == 0) { return result; } diff --git a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java index cbbd667..6f07d55 100644 --- a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java +++ b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java @@ -103,17 +103,30 @@ public class CircuitBreakerTests { @Bean public Service service() { - return new Service(); + return new ServiceImpl(); } } - protected static class Service { + interface Service { + + void service(); + + void expressionService(); + + int getCount(); + + RetryContext getContext(); + + } + + protected static class ServiceImpl implements Service { private int count = 0; private RetryContext context; + @Override @CircuitBreaker(RuntimeException.class) public void service() { this.context = RetrySynchronizationManager.getContext(); @@ -122,6 +135,7 @@ public class CircuitBreakerTests { } } + @Override @CircuitBreaker(maxAttemptsExpression = "#{2 * ${foo:4}}", openTimeoutExpression = "#{${bar:19}000}", resetTimeoutExpression = "#{${baz:20}000}", exceptionExpression = "#{#root instanceof RuntimeExpression}") @@ -129,10 +143,12 @@ public class CircuitBreakerTests { this.count++; } + @Override public RetryContext getContext() { return this.context; } + @Override public int getCount() { return this.count; }