From 6ab7017752c56c04de9160f631e77db4bf358924 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Wed, 21 Jun 2023 12:43:30 +0800 Subject: [PATCH] Throwable as first argument of explicit recover method should be optional According to Javadoc of @Recover, The Throwable first argument is optional. Before this commit, ``` java.lang.NullPointerException: Cannot invoke "java.lang.Class.isAssignableFrom(java.lang.Class)" because "meta.type" is null at org.springframework.retry.annotation.RecoverAnnotationRecoveryHandler.findClosestMatch(RecoverAnnotationRecoveryHandler.java:154) at org.springframework.retry.annotation.RecoverAnnotationRecoveryHandler.recover(RecoverAnnotationRecoveryHandler.java:75) at org.springframework.retry.interceptor.RetryOperationsInterceptor$ItemRecovererCallback.recover(RetryOperationsInterceptor.java:159) at org.springframework.retry.support.RetryTemplate.handleRetryExhausted(RetryTemplate.java:543) at org.springframework.retry.support.RetryTemplate.doExecute(RetryTemplate.java:389) at org.springframework.retry.support.RetryTemplate.execute(RetryTemplate.java:225) at org.springframework.retry.interceptor.RetryOperationsInterceptor.invoke(RetryOperationsInterceptor.java:124) at org.springframework.retry.annotation.AnnotationAwareRetryOperationsInterceptor.invoke(AnnotationAwareRetryOperationsInterceptor.java:160) at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:750) ``` is raised for ```java @Retryable(retryFor = RuntimeException.class, recover = "recover") public String service(String param) { throw new RuntimeException("Planned"); } @Recover public String recover(String param) { return param; } ``` --- .../RecoverAnnotationRecoveryHandler.java | 12 +++--- .../retry/annotation/EnableRetryTests.java | 38 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index 454655bb..d798a863 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -54,6 +54,7 @@ * @author Artem Bilan * @author Gianluca Medici * @author Lijinliang + * @author Yanming Zhou */ public class RecoverAnnotationRecoveryHandler implements MethodInvocationRecoverer { @@ -130,7 +131,7 @@ private Method findClosestMatch(Object[] args, Class cause) } else if (distance == min) { boolean parametersMatch = compareParameters(args, meta.getArgCount(), - method.getParameterTypes()); + method.getParameterTypes(), false); if (parametersMatch) { result = method; } @@ -143,8 +144,8 @@ else if (distance == min) { 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 == null || meta.type.isAssignableFrom(cause)) + && compareParameters(args, meta.getArgCount(), method.getParameterTypes(), true)) { result = method; break; } @@ -164,8 +165,9 @@ private int calculateDistance(Class cause, Class[] parameterTypes) { - if (argCount == (args.length + 1)) { + private boolean compareParameters(Object[] args, int argCount, Class[] parameterTypes, + boolean withRecoverMethodName) { + if ((withRecoverMethodName && argCount == args.length) || argCount == (args.length + 1)) { int startingIndex = 0; if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) { startingIndex = 1; diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index 2be01c45..63b0c2e9 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -141,6 +141,24 @@ public void recovery() { context.close(); } + @Test + public void recoveryWithoutParam() { + try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + TestConfiguration.class)) { + RecoverableService service = context.getBean(RecoverableService.class); + assertThat(service.serviceWithoutParam()).isEqualTo("test"); + } + } + + @Test + public void recoveryWithParam() { + try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + TestConfiguration.class)) { + RecoverableService service = context.getBean(RecoverableService.class); + assertThat(service.serviceWithParam("test")).isEqualTo("test"); + } + } + @Test public void type() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class); @@ -637,6 +655,26 @@ public void recover(Throwable cause) { this.cause = cause; } + @Retryable(retryFor = RuntimeException.class, recover = "recoverWithoutParam") + public String serviceWithoutParam() { + throw new RuntimeException("Planned"); + } + + @Recover + public String recoverWithoutParam() { + return "test"; + } + + @Retryable(retryFor = RuntimeException.class, recover = "recoverWithParam") + public String serviceWithParam(String param) { + throw new RuntimeException("Planned"); + } + + @Recover + public String recoverWithParam(String param) { + return param; + } + public Throwable getCause() { return this.cause; }