From dd819b4eec264f6a726be9e336c05c8f613a5bb2 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Mon, 24 Jan 2022 12:50:42 -0500 Subject: [PATCH] Annotation Expression Improvements - remove unnecessary template delimiters from tests - make expression handling consistent - template delimiters should not be needed for any - fix bean resolution in `@BackOff` --- ...tationAwareRetryOperationsInterceptor.java | 48 ++++++++++++++----- .../retry/annotation/Backoff.java | 4 +- .../retry/policy/ExpressionRetryPolicy.java | 3 +- .../retry/support/RetryTemplateBuilder.java | 2 +- .../retry/annotation/EnableRetryTests.java | 10 ++-- .../support/RetryTemplateBuilderTest.java | 6 +-- 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index 9d6d2a1..d2b46e7 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -332,8 +332,14 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn Integer maxAttempts = (Integer) attrs.get("maxAttempts"); String maxAttemptsExpression = (String) attrs.get("maxAttemptsExpression"); if (StringUtils.hasText(maxAttemptsExpression)) { - maxAttempts = PARSER.parseExpression(resolve(maxAttemptsExpression), PARSER_CONTEXT) - .getValue(this.evaluationContext, Integer.class); + if (ExpressionRetryPolicy.isTemplate(maxAttemptsExpression)) { + maxAttempts = PARSER.parseExpression(resolve(maxAttemptsExpression), PARSER_CONTEXT) + .getValue(this.evaluationContext, Integer.class); + } + else { + maxAttempts = PARSER.parseExpression(resolve(maxAttemptsExpression)).getValue(this.evaluationContext, + Integer.class); + } } if (includes.length == 0 && excludes.length == 0) { SimpleRetryPolicy simple = hasExpression @@ -360,20 +366,40 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn } private BackOffPolicy getBackoffPolicy(Backoff backoff) { + Map attrs = AnnotationUtils.getAnnotationAttributes(backoff); long min = backoff.delay() == 0 ? backoff.value() : backoff.delay(); - if (StringUtils.hasText(backoff.delayExpression())) { - min = PARSER.parseExpression(resolve(backoff.delayExpression()), PARSER_CONTEXT) - .getValue(this.evaluationContext, Long.class); + String delayExpression = (String) attrs.get("delayExpression"); + if (StringUtils.hasText(delayExpression)) { + if (ExpressionRetryPolicy.isTemplate(delayExpression)) { + min = PARSER.parseExpression(resolve(delayExpression), PARSER_CONTEXT).getValue(this.evaluationContext, + Long.class); + } + else { + min = PARSER.parseExpression(resolve(delayExpression)).getValue(this.evaluationContext, Long.class); + } } long max = backoff.maxDelay(); - if (StringUtils.hasText(backoff.maxDelayExpression())) { - max = PARSER.parseExpression(resolve(backoff.maxDelayExpression()), PARSER_CONTEXT) - .getValue(this.evaluationContext, Long.class); + String maxDelayExpression = (String) attrs.get("maxDelayExpression"); + if (StringUtils.hasText(maxDelayExpression)) { + if (ExpressionRetryPolicy.isTemplate(delayExpression)) { + max = PARSER.parseExpression(resolve(maxDelayExpression), PARSER_CONTEXT) + .getValue(this.evaluationContext, Long.class); + } + else { + max = PARSER.parseExpression(resolve(maxDelayExpression)).getValue(this.evaluationContext, Long.class); + } } double multiplier = backoff.multiplier(); - if (StringUtils.hasText(backoff.multiplierExpression())) { - multiplier = PARSER.parseExpression(resolve(backoff.multiplierExpression()), PARSER_CONTEXT) - .getValue(this.evaluationContext, Double.class); + String multiplierExpression = (String) attrs.get("multiplierExpression"); + if (StringUtils.hasText(multiplierExpression)) { + if (ExpressionRetryPolicy.isTemplate(delayExpression)) { + multiplier = PARSER.parseExpression(resolve(multiplierExpression), PARSER_CONTEXT) + .getValue(this.evaluationContext, Double.class); + } + else { + multiplier = PARSER.parseExpression(resolve(multiplierExpression)).getValue(this.evaluationContext, + Double.class); + } } if (multiplier > 0) { ExponentialBackOffPolicy policy = new ExponentialBackOffPolicy(); diff --git a/src/main/java/org/springframework/retry/annotation/Backoff.java b/src/main/java/org/springframework/retry/annotation/Backoff.java index 99fddd1..8e1337b 100644 --- a/src/main/java/org/springframework/retry/annotation/Backoff.java +++ b/src/main/java/org/springframework/retry/annotation/Backoff.java @@ -90,8 +90,8 @@ public @interface Backoff { String delayExpression() default ""; /** - * An expression evaluating to the maximum wait (in milliseconds) between retries. - * If less than the {@link #delay()} then the default of + * An expression evaluating to the maximum wait (in milliseconds) between retries. If + * less than the {@link #delay()} then the default of * {@value org.springframework.retry.backoff.ExponentialBackOffPolicy#DEFAULT_MAX_INTERVAL} * is applied. Overrides {@link #maxDelay()} * @return the maximum delay between retries (default 0 = ignored) diff --git a/src/main/java/org/springframework/retry/policy/ExpressionRetryPolicy.java b/src/main/java/org/springframework/retry/policy/ExpressionRetryPolicy.java index 810104c..a7e5235 100644 --- a/src/main/java/org/springframework/retry/policy/ExpressionRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/ExpressionRetryPolicy.java @@ -19,6 +19,7 @@ import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; @@ -139,7 +140,7 @@ public class ExpressionRetryPolicy extends SimpleRetryPolicy implements BeanFact * @param expression the expression string * @return true if the expression string is a template */ - private static boolean isTemplate(String expression) { + public static boolean isTemplate(String expression) { return expression.contains(PARSER_CONTEXT.getExpressionPrefix()) && expression.contains(PARSER_CONTEXT.getExpressionSuffix()); } diff --git a/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java b/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java index 62ee2e4..87258f4 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java @@ -310,7 +310,7 @@ public class RetryTemplateBuilder { /** * Add all throwables to the while list of retryable exceptions. *

- * Warn: touching this method drops default {@code retryOn(Exception.class)} and you + * Warn: touching this method drops default {@code retryOn(Exception.class)} and you * should configure whole classifier from scratch. *

* You should select the way you want to configure exception classifier: white list or diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index 8e3610d..7bff6ae 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -654,22 +654,22 @@ public class EnableRetryTests { private int count = 0; - @Retryable(exceptionExpression = "#{message.contains('this can be retried')}") + @Retryable(exceptionExpression = "message.contains('this can be retried')") public void service1() { if (this.count++ < 2) { throw new RuntimeException("this can be retried"); } } - @Retryable(exceptionExpression = "#{message.contains('this can be retried')}") + @Retryable(exceptionExpression = "message.contains('this can be retried')") public void service2() { this.count++; throw new RuntimeException("this cannot be retried"); } - @Retryable(exceptionExpression = "#{@exceptionChecker.${retryMethod}(#root)}", - maxAttemptsExpression = "#{@integerFiveBean}", backoff = @Backoff(delayExpression = "#{${one}}", - maxDelayExpression = "#{${five}}", multiplierExpression = "#{${onePointOne}}")) + @Retryable(exceptionExpression = "@exceptionChecker.${retryMethod}(#root)", + maxAttemptsExpression = "@integerFiveBean", backoff = @Backoff(delayExpression = "${one}", + maxDelayExpression = "@integerFiveBean", multiplierExpression = "${onePointOne}")) public void service3() { if (this.count++ < 8) { throw new RuntimeException(); diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTest.java b/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTest.java index 1570309..13019e4 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTest.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTest.java @@ -78,9 +78,9 @@ public class RetryTemplateBuilderTest { RetryListener listener2 = mock(RetryListener.class); RetryTemplate template = RetryTemplate.builder().maxAttempts(10).exponentialBackoff(99, 1.5, 1717) - .retryOn(IOException.class).retryOn(Collections.>singletonList(IllegalArgumentException.class)) - .traversingCauses().withListener(listener1) - .withListeners(Collections.singletonList(listener2)).build(); + .retryOn(IOException.class) + .retryOn(Collections.>singletonList(IllegalArgumentException.class)) + .traversingCauses().withListener(listener1).withListeners(Collections.singletonList(listener2)).build(); PolicyTuple policyTuple = PolicyTuple.extractWithAsserts(template);