From 0f5197a21cbb769b9c96d425ad835594f6c04bba Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Wed, 20 Apr 2022 13:35:28 -0400 Subject: [PATCH] GH-293: Fix Bean Resolver for CBreaker Expressions Resolves https://github.com/spring-projects/spring-retry/issues/293 **cherry-pick to 1.3.x** --- ...tationAwareRetryOperationsInterceptor.java | 28 +++++++++++++--- .../retry/annotation/CircuitBreakerTests.java | 32 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index 3782f86..74585b0 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -62,7 +62,6 @@ import org.springframework.retry.policy.SimpleRetryPolicy; import org.springframework.retry.support.RetryTemplate; import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.ReflectionUtils; -import org.springframework.util.ReflectionUtils.MethodCallback; import org.springframework.util.StringUtils; /** @@ -251,8 +250,15 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn private long getOpenTimeout(CircuitBreaker circuit) { if (StringUtils.hasText(circuit.openTimeoutExpression())) { - Long value = PARSER.parseExpression(resolve(circuit.openTimeoutExpression()), PARSER_CONTEXT) - .getValue(Long.class); + Long value = null; + if (isTemplate(circuit.openTimeoutExpression())) { + value = PARSER.parseExpression(resolve(circuit.openTimeoutExpression()), PARSER_CONTEXT) + .getValue(this.evaluationContext, Long.class); + } + else { + value = PARSER.parseExpression(resolve(circuit.openTimeoutExpression())) + .getValue(this.evaluationContext, Long.class); + } if (value != null) { return value; } @@ -262,8 +268,15 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn private long getResetTimeout(CircuitBreaker circuit) { if (StringUtils.hasText(circuit.resetTimeoutExpression())) { - Long value = PARSER.parseExpression(resolve(circuit.resetTimeoutExpression()), PARSER_CONTEXT) - .getValue(Long.class); + Long value = null; + if (isTemplate(circuit.openTimeoutExpression())) { + value = PARSER.parseExpression(resolve(circuit.resetTimeoutExpression()), PARSER_CONTEXT) + .getValue(this.evaluationContext, Long.class); + } + else { + value = PARSER.parseExpression(resolve(circuit.resetTimeoutExpression())) + .getValue(this.evaluationContext, Long.class); + } if (value != null) { return value; } @@ -271,6 +284,11 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn return circuit.resetTimeout(); } + private boolean isTemplate(String expression) { + return expression.contains(PARSER_CONTEXT.getExpressionPrefix()) + && expression.contains(PARSER_CONTEXT.getExpressionSuffix()); + } + private RetryTemplate createTemplate(String[] listenersBeanNames) { RetryTemplate template = new RetryTemplate(); if (listenersBeanNames.length > 0) { diff --git a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java index 0bc1104..6547fb8 100644 --- a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java +++ b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java @@ -59,6 +59,8 @@ public class CircuitBreakerTests { assertThat(service.getCount()).isEqualTo(3); service.expressionService(); assertThat(service.getCount()).isEqualTo(4); + service.expressionService2(); + assertThat(service.getCount()).isEqualTo(5); Advised advised = (Advised) service; Advisor advisor = advised.getAdvisors()[0]; Map delegates = (Map) new DirectFieldAccessor(advisor).getPropertyValue("advice.delegates"); @@ -72,6 +74,12 @@ public class CircuitBreakerTests { assertThat(accessor.getPropertyValue("retryOperations.retryPolicy.resetTimeout")).isEqualTo(20000L); assertThat(accessor.getPropertyValue("retryOperations.retryPolicy.delegate.expression.expression")) .isEqualTo("#root instanceof RuntimeExpression"); + + interceptor = (MethodInterceptor) methodMap.get(Service.class.getDeclaredMethod("expressionService2")); + accessor = new DirectFieldAccessor(interceptor); + assertThat(accessor.getPropertyValue("retryOperations.retryPolicy.delegate.maxAttempts")).isEqualTo(10); + assertThat(accessor.getPropertyValue("retryOperations.retryPolicy.openTimeout")).isEqualTo(10000L); + assertThat(accessor.getPropertyValue("retryOperations.retryPolicy.resetTimeout")).isEqualTo(20000L); context.close(); } @@ -84,6 +92,21 @@ public class CircuitBreakerTests { return new ServiceImpl(); } + @Bean + Configs configs() { + return new Configs(); + } + + } + + public static class Configs { + + public int maxAttempts = 10; + + public long openTimeout = 10000; + + public long resetTimeout = 20000; + } interface Service { @@ -92,6 +115,8 @@ public class CircuitBreakerTests { void expressionService(); + void expressionService2(); + int getCount(); RetryContext getContext(); @@ -121,6 +146,13 @@ public class CircuitBreakerTests { this.count++; } + @Override + @CircuitBreaker(maxAttemptsExpression = "@configs.maxAttempts", openTimeoutExpression = "@configs.openTimeout", + resetTimeoutExpression = "@configs.resetTimeout") + public void expressionService2() { + this.count++; + } + @Override public RetryContext getContext() { return this.context;