From 6c15e0931a8af76f1e2caf8401f9a354e72321a9 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 2 Oct 2023 10:16:25 +0200 Subject: [PATCH] Order ControllerAdvice beans in controller support Prior to this commit, the `AnnotatedControllerExceptionResolver` would scan the application context for `@ControllerAdvice` beans and cache the results in an unordered Map. It's later iterating over the entries to resolve an exception handler at runtime. This commit ensures that such entries are now stored in an ordered map, using the `OrderComparator.INSTANCE` comparator. `ControllerAdvice` beans will be iterated over the order specified by the `Ordered` contract or the `@Order` annotation. Fixes gh-830 --- .../AnnotatedControllerExceptionResolver.java | 4 ++- ...tatedControllerExceptionResolverTests.java | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java index 307cf013..87678597 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolver.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import graphql.GraphQLError; @@ -36,6 +37,7 @@ import org.springframework.core.ExceptionDepthComparator; import org.springframework.core.KotlinDetector; import org.springframework.core.MethodIntrospector; import org.springframework.core.MethodParameter; +import org.springframework.core.OrderComparator; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.graphql.data.method.HandlerMethod; import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite; @@ -80,7 +82,7 @@ final class AnnotatedControllerExceptionResolver { private final Map, MethodResolver> controllerCache = new ConcurrentHashMap<>(64); - private final Map controllerAdviceCache = new ConcurrentHashMap<>(64); + private final Map controllerAdviceCache = new TreeMap<>(OrderComparator.INSTANCE); AnnotatedControllerExceptionResolver(HandlerMethodArgumentResolverComposite resolvers) { diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolverTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolverTests.java index a19a824d..11a9b242 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolverTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerExceptionResolverTests.java @@ -31,6 +31,8 @@ import org.springframework.aop.target.SingletonTargetSource; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.support.StaticApplicationContext; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite; import org.springframework.graphql.data.method.annotation.GraphQlExceptionHandler; import org.springframework.lang.Nullable; @@ -121,6 +123,20 @@ public class AnnotatedControllerExceptionResolverTests { assertThat(actual.get(0).getMessage()).isEqualTo("handle: Bad input"); } + @Test + void resolveWithOrderedControllerAdvice() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(TestControllerAdvice.class); + context.register(OrderedTestControllerAdvice.class); + context.refresh(); + + Exception ex = new IllegalArgumentException("Bad input"); + List actual = exceptionResolver(context).resolveException(ex, this.environment, null).block(); + + assertThat(actual).hasSize(1); + assertThat(actual.get(0).getMessage()).isEqualTo("ordered handle: Bad input"); + } + @Test void invalidReturnType() { assertThatIllegalStateException().isThrownBy(() -> @@ -235,6 +251,19 @@ public class AnnotatedControllerExceptionResolverTests { } + @SuppressWarnings("unused") + @ControllerAdvice + @Order(Ordered.HIGHEST_PRECEDENCE) + private static class OrderedTestControllerAdvice { + + @GraphQlExceptionHandler + GraphQLError handle(IllegalArgumentException ex) { + return GraphQLError.newError().message("ordered handle: " + ex.getMessage()).build(); + } + + } + + private static class InvalidReturnTypeController { @GraphQlExceptionHandler