From 614f826cedec982f138b1f5913334c115ef2c648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Skowro=C5=84ski?= <5780288+hexmind@users.noreply.github.com> Date: Fri, 7 Feb 2020 14:02:49 +0100 Subject: [PATCH] Improved TimeLimiterAspect message for not supported types (#849) (#852) --- .../fallback/DefaultFallbackDecorator.java | 3 +++ .../configure/IllegalReturnTypeException.java | 9 +++++++++ .../configure/ReactorTimeLimiterAspectExt.java | 10 ++-------- .../configure/RxJava2TimeLimiterAspectExt.java | 13 +++++-------- .../timelimiter/configure/TimeLimiterAspect.java | 3 ++- .../configure/ReactorTimeLimiterAspectExtTest.java | 13 +++++++++++-- .../configure/RxJava2TimeLimiterAspectExtTest.java | 13 +++++++++++-- .../configure/TimeLimiterRecoveryTest.java | 7 ++++--- 8 files changed, 47 insertions(+), 24 deletions(-) create mode 100644 resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/IllegalReturnTypeException.java diff --git a/resilience4j-spring/src/main/java/io/github/resilience4j/fallback/DefaultFallbackDecorator.java b/resilience4j-spring/src/main/java/io/github/resilience4j/fallback/DefaultFallbackDecorator.java index 0417a0fea7..1c2ad3b598 100644 --- a/resilience4j-spring/src/main/java/io/github/resilience4j/fallback/DefaultFallbackDecorator.java +++ b/resilience4j-spring/src/main/java/io/github/resilience4j/fallback/DefaultFallbackDecorator.java @@ -15,6 +15,7 @@ */ package io.github.resilience4j.fallback; +import io.github.resilience4j.timelimiter.configure.IllegalReturnTypeException; import io.vavr.CheckedFunction0; /** @@ -33,6 +34,8 @@ public CheckedFunction0 decorate(FallbackMethod fallbackMethod, return () -> { try { return supplier.apply(); + } catch (IllegalReturnTypeException e) { + throw e; } catch (Throwable throwable) { return fallbackMethod.fallback(throwable); } diff --git a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/IllegalReturnTypeException.java b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/IllegalReturnTypeException.java new file mode 100644 index 0000000000..dfc58773d1 --- /dev/null +++ b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/IllegalReturnTypeException.java @@ -0,0 +1,9 @@ +package io.github.resilience4j.timelimiter.configure; + +public class IllegalReturnTypeException extends IllegalArgumentException { + + public IllegalReturnTypeException(Class returnType, String methodName, String explanation) { + super(String.join(" ", returnType.getName(), methodName, + "has unsupported by @TimeLimiter return type.", explanation)); + } +} diff --git a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExt.java b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExt.java index 79a0ef9bf4..307f6a03ca 100644 --- a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExt.java +++ b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExt.java @@ -19,15 +19,11 @@ import io.github.resilience4j.reactor.timelimiter.TimeLimiterOperator; import io.github.resilience4j.timelimiter.TimeLimiter; import org.aspectj.lang.ProceedingJoinPoint; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; public class ReactorTimeLimiterAspectExt implements TimeLimiterAspectExt{ - private static final Logger logger = LoggerFactory.getLogger(ReactorTimeLimiterAspectExt.class); - /** * @param returnType the AOP method return type class * @return boolean if the method has Reactor return type @@ -58,10 +54,8 @@ public Object handle(ProceedingJoinPoint proceedingJoinPoint, Mono monoReturnValue = (Mono) returnValue; return monoReturnValue.compose(TimeLimiterOperator.of(timeLimiter)); } else { - logger.error("Unsupported type for Reactor timeLimiter {}", returnValue.getClass().getTypeName()); - throw new IllegalArgumentException( - "Not Supported type for the timeLimiter in Reactor :" + returnValue.getClass().getName()); - + throw new IllegalReturnTypeException(returnValue.getClass(), methodName, + "Reactor expects Mono/Flux."); } } diff --git a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExt.java b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExt.java index abadf12b8e..8a61a4bc1b 100644 --- a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExt.java +++ b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExt.java @@ -20,8 +20,6 @@ import io.github.resilience4j.timelimiter.transformer.TimeLimiterTransformer; import io.reactivex.*; import org.aspectj.lang.ProceedingJoinPoint; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.Set; @@ -29,7 +27,6 @@ public class RxJava2TimeLimiterAspectExt implements TimeLimiterAspectExt { - private static final Logger logger = LoggerFactory.getLogger(RxJava2TimeLimiterAspectExt.class); private final Set> rxSupportedTypes = newHashSet(ObservableSource.class, SingleSource.class, CompletableSource.class, MaybeSource.class, Flowable.class); @@ -54,11 +51,12 @@ public Object handle(ProceedingJoinPoint proceedingJoinPoint, TimeLimiter timeLi throws Throwable { TimeLimiterTransformer timeLimiterTransformer = TimeLimiterTransformer.of(timeLimiter); Object returnValue = proceedingJoinPoint.proceed(); - return executeRxJava2Aspect(timeLimiterTransformer, returnValue); + return executeRxJava2Aspect(timeLimiterTransformer, returnValue, methodName); } @SuppressWarnings("unchecked") - private static Object executeRxJava2Aspect(TimeLimiterTransformer timeLimiterTransformer, Object returnValue) { + private static Object executeRxJava2Aspect(TimeLimiterTransformer timeLimiterTransformer, + Object returnValue, String methodName) { if (returnValue instanceof ObservableSource) { Observable observable = (Observable) returnValue; return observable.compose(timeLimiterTransformer); @@ -75,9 +73,8 @@ private static Object executeRxJava2Aspect(TimeLimiterTransformer timeLimiterTra Flowable flowable = (Flowable) returnValue; return flowable.compose(timeLimiterTransformer); } else { - logger.error("Unsupported type for TimeLimiter RxJava2 {}", returnValue.getClass().getTypeName()); - throw new IllegalArgumentException( - "Not Supported type for the TimeLimiter in RxJava2 :" + returnValue.getClass().getName()); + throw new IllegalReturnTypeException(returnValue.getClass(), methodName, + "RxJava2 expects Flowable/Single/..."); } } } diff --git a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/TimeLimiterAspect.java b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/TimeLimiterAspect.java index a30d1f5ac2..b3e9755181 100644 --- a/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/TimeLimiterAspect.java +++ b/resilience4j-spring/src/main/java/io/github/resilience4j/timelimiter/configure/TimeLimiterAspect.java @@ -109,7 +109,8 @@ private Object proceed(ProceedingJoinPoint proceedingJoinPoint, String methodNam } if (!CompletionStage.class.isAssignableFrom(returnType)) { - throw new IllegalStateException("Not supported type by TimeLimiterAspect"); + throw new IllegalReturnTypeException(returnType, methodName, + "CompletionStage expected."); } return handleJoinPointCompletableFuture(proceedingJoinPoint, timeLimiter); diff --git a/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExtTest.java b/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExtTest.java index 1b47aa3f12..99a25671b8 100644 --- a/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExtTest.java +++ b/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/ReactorTimeLimiterAspectExtTest.java @@ -11,6 +11,7 @@ import reactor.core.publisher.Mono; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -40,11 +41,19 @@ public void testReactorTypes() throws Throwable { assertThat(reactorTimeLimiterAspectExt.handle(proceedingJoinPoint, timeLimiter, "testMethod")).isNotNull(); } - @Test(expected = IllegalArgumentException.class) + @Test public void shouldThrowIllegalArgumentExceptionWithNotReactorType() throws Throwable{ TimeLimiter timeLimiter = TimeLimiter.ofDefaults("test"); when(proceedingJoinPoint.proceed()).thenReturn("NOT REACTOR TYPE"); - reactorTimeLimiterAspectExt.handle(proceedingJoinPoint, timeLimiter, "testMethod"); + + try { + reactorTimeLimiterAspectExt.handle(proceedingJoinPoint, timeLimiter, "testMethod"); + fail("exception missed"); + } catch (Throwable e) { + assertThat(e).isInstanceOf(IllegalReturnTypeException.class) + .hasMessage( + "java.lang.String testMethod has unsupported by @TimeLimiter return type. Reactor expects Mono/Flux."); + } } } \ No newline at end of file diff --git a/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExtTest.java b/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExtTest.java index 34a6ba5f06..793fef512b 100644 --- a/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExtTest.java +++ b/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/RxJava2TimeLimiterAspectExtTest.java @@ -10,6 +10,7 @@ import org.mockito.junit.MockitoJUnitRunner; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -49,11 +50,19 @@ public void testRxJava2Types() throws Throwable { assertThat(rxJava2TimeLimiterAspectExt.handle(proceedingJoinPoint, timeLimiter, "testMethod")).isNotNull(); } - @Test(expected = IllegalArgumentException.class) + @Test public void shouldThrowIllegalArgumentExceptionWithNotRxJava2Type() throws Throwable{ TimeLimiter timeLimiter = TimeLimiter.ofDefaults("test"); when(proceedingJoinPoint.proceed()).thenReturn("NOT RXJAVA2 TYPE"); - rxJava2TimeLimiterAspectExt.handle(proceedingJoinPoint, timeLimiter, "testMethod"); + + try { + rxJava2TimeLimiterAspectExt.handle(proceedingJoinPoint, timeLimiter, "testMethod"); + fail("exception missed"); + } catch (Throwable e) { + assertThat(e).isInstanceOf(IllegalReturnTypeException.class) + .hasMessage( + "java.lang.String testMethod has unsupported by @TimeLimiter return type. RxJava2 expects Flowable/Single/..."); + } } } \ No newline at end of file diff --git a/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/TimeLimiterRecoveryTest.java b/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/TimeLimiterRecoveryTest.java index 10ed372b55..c9947568f1 100644 --- a/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/TimeLimiterRecoveryTest.java +++ b/resilience4j-spring/src/test/java/io/github/resilience4j/timelimiter/configure/TimeLimiterRecoveryTest.java @@ -22,7 +22,8 @@ public class TimeLimiterRecoveryTest { @Test public void testAsyncRecovery() throws Exception { - assertThat(testDummyService.async().toCompletableFuture().get(5, TimeUnit.SECONDS)).isEqualTo("recovered"); + String result = testDummyService.async().toCompletableFuture().get(5, TimeUnit.SECONDS); + assertThat(result).isEqualTo("recovered"); } @Test @@ -60,9 +61,9 @@ public void testFlowableRecovery() { assertThat(testDummyService.flowable().blockingFirst()).isEqualTo("recovered"); } - @Test + @Test(expected = IllegalReturnTypeException.class) public void testSpelRecovery() { - assertThat(testDummyService.spelSync()).isEqualTo("recovered"); + testDummyService.spelSync(); } }