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 c1f916d4..006ac950 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,9 +21,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import graphql.GraphQLError; @@ -71,6 +71,7 @@ import org.springframework.web.method.ControllerAdviceBean; * non-controller {@link graphql.schema.DataFetcher}s. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 1.2.0 */ final class AnnotatedControllerExceptionResolver implements HandlerDataFetcherExceptionResolver { @@ -82,7 +83,7 @@ final class AnnotatedControllerExceptionResolver implements HandlerDataFetcherEx private final Map, MethodResolver> controllerCache = new ConcurrentHashMap<>(64); - private final Map controllerAdviceCache = new TreeMap<>(OrderComparator.INSTANCE); + private final Map controllerAdviceCache = new LinkedHashMap<>(); AnnotatedControllerExceptionResolver(HandlerMethodArgumentResolverComposite resolvers) { @@ -110,15 +111,19 @@ final class AnnotatedControllerExceptionResolver implements HandlerDataFetcherEx * @param context the context to look into */ public void registerControllerAdvice(ApplicationContext context) { + Map detectedControllerAdvice = new HashMap<>(); for (ControllerAdviceBean bean : ControllerAdviceBean.findAnnotatedBeans(context)) { Class beanType = bean.getBeanType(); if (beanType != null) { Map, Method> methods = findExceptionHandlers(beanType); if (!methods.isEmpty()) { - this.controllerAdviceCache.put(bean, new MethodResolver(methods)); + detectedControllerAdvice.put(bean, new MethodResolver(methods)); } } } + detectedControllerAdvice.keySet().stream().sorted(OrderComparator.INSTANCE).forEach(bean -> { + this.controllerAdviceCache.put(bean, detectedControllerAdvice.get(bean)); + }); if (logger.isDebugEnabled()) { logger.debug("@GraphQlException methods in ControllerAdvice beans: " + (this.controllerAdviceCache.isEmpty() ? "none" : this.controllerAdviceCache.size())); 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 11a9b242..76160f3d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -47,7 +47,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; * Unit tests for {@link AnnotatedControllerExceptionResolver}. * * @author Rossen Stoyanchev - * @since 1.2.0 + * @author Brian Clozel */ public class AnnotatedControllerExceptionResolverTests { @@ -137,6 +137,25 @@ public class AnnotatedControllerExceptionResolverTests { assertThat(actual.get(0).getMessage()).isEqualTo("ordered handle: Bad input"); } + @Test + void resolveWithMultipleControllerAdviceAtSameOrder() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(TestControllerAdvice.class); + context.register(OtherTestControllerAdvice.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("handle: Bad input"); + + ex = new IllegalStateException("Bad state"); + actual = exceptionResolver(context).resolveException(ex, this.environment, null).block(); + assertThat(actual).hasSize(1); + assertThat(actual.get(0).getMessage()).isEqualTo("handle: Bad state"); + } + + @Test void invalidReturnType() { assertThatIllegalStateException().isThrownBy(() -> @@ -250,6 +269,17 @@ public class AnnotatedControllerExceptionResolverTests { } + @SuppressWarnings("unused") + @ControllerAdvice + private static class OtherTestControllerAdvice { + + @GraphQlExceptionHandler + GraphQLError handle(IllegalStateException ex) { + return GraphQLError.newError().message("handle: " + ex.getMessage()).build(); + } + + } + @SuppressWarnings("unused") @ControllerAdvice