diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java index 5de9100019..47ebcd6314 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java @@ -34,17 +34,18 @@ import org.springframework.stereotype.Component; * *

Classes annotated with {@code @ControllerAdvice} can be declared explicitly * as Spring beans or auto-detected via classpath scanning. All such beans are - * sorted based on {@link org.springframework.core.Ordered Ordered} / - * {@link org.springframework.core.PriorityOrdered PriorityOrdered} semantics or + * sorted based on {@link org.springframework.core.Ordered Ordered} semantics or * {@link org.springframework.core.annotation.Order @Order} / - * {@link javax.annotation.Priority @Priority} declarations, with {@code Ordered} / - * {@code PriorityOrdered} semantics taking precedence over {@code @Order} / - * {@code @Priority} declarations. {@code @ControllerAdvice} beans are then - * applied in that order at runtime. For handling exceptions, an - * {@code @ExceptionHandler} will be picked on the first advice with a matching - * exception handler method. For model attributes and {@code InitBinder} - * initialization, {@code @ModelAttribute} and {@code @InitBinder} methods will - * also follow {@code @ControllerAdvice} order. + * {@link javax.annotation.Priority @Priority} declarations, with {@code Ordered} + * semantics taking precedence over {@code @Order} / {@code @Priority} declarations. + * {@code @ControllerAdvice} beans are then applied in that order at runtime. + * Note, however, that {@code @ControllerAdvice} beans that implement + * {@link org.springframework.core.PriorityOrdered PriorityOrdered} are not + * given priority over {@code @ControllerAdvice} beans that implement {@code Ordered}. + * For handling exceptions, an {@code @ExceptionHandler} will be picked on the + * first advice with a matching exception handler method. For model attributes + * and {@code InitBinder} initialization, {@code @ModelAttribute} and + * {@code @InitBinder} methods will also follow {@code @ControllerAdvice} order. * *

Note: For {@code @ExceptionHandler} methods, a root exception match will be * preferred to just matching a cause of the current exception, among the handler diff --git a/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java b/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java index 0c54d039fa..6f7349c9b3 100644 --- a/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java +++ b/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java @@ -22,6 +22,7 @@ import java.util.List; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.context.ApplicationContext; +import org.springframework.core.OrderComparator; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.OrderUtils; @@ -215,6 +216,11 @@ public class ControllerAdviceBean implements Ordered { * Find beans annotated with {@link ControllerAdvice @ControllerAdvice} in the * given {@link ApplicationContext} and wrap them as {@code ControllerAdviceBean} * instances. + *

As of Spring Framework 5.2, the {@code ControllerAdviceBean} instances + * in the returned list are sorted using {@link OrderComparator#sort(List)}. + * @see #getOrder() + * @see OrderComparator + * @see Ordered */ public static List findAnnotatedBeans(ApplicationContext context) { List adviceBeans = new ArrayList<>(); @@ -226,6 +232,7 @@ public class ControllerAdviceBean implements Ordered { adviceBeans.add(new ControllerAdviceBean(name, context, controllerAdvice)); } } + OrderComparator.sort(adviceBeans); return adviceBeans; } diff --git a/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java b/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java index 4236c8256d..d1bd28a9a0 100644 --- a/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java @@ -18,13 +18,19 @@ package org.springframework.web.method; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.List; + import javax.annotation.Priority; import org.junit.Test; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactory; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; import org.springframework.core.annotation.Order; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.RestController; @@ -36,7 +42,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; /** - * Unit tests for {@link ControllerAdviceBean}. + * Unit and integration tests for {@link ControllerAdviceBean}. * * @author Brian Clozel * @author Sam Brannen @@ -113,14 +119,14 @@ public class ControllerAdviceBeanTests { @Test public void orderedViaAnnotationForBeanName() { - assertOrder(OrderAnnotationControllerAdvice.class, 42); - assertOrder(PriorityAnnotationControllerAdvice.class, 42); + assertOrder(OrderAnnotationControllerAdvice.class, 100); + assertOrder(PriorityAnnotationControllerAdvice.class, 200); } @Test public void orderedViaAnnotationForBeanInstance() { - assertOrder(new OrderAnnotationControllerAdvice(), 42); - assertOrder(new PriorityAnnotationControllerAdvice(), 42); + assertOrder(new OrderAnnotationControllerAdvice(), 100); + assertOrder(new PriorityAnnotationControllerAdvice(), 200); } @Test @@ -192,6 +198,25 @@ public class ControllerAdviceBeanTests { assertNotApplicable("should not match", bean, InheritanceController.class); } + @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) + public void findAnnotatedBeansSortsBeans() { + Class[] expectedTypes = { + // Since ControllerAdviceBean currently treats PriorityOrdered the same as Ordered, + // OrderedControllerAdvice is sorted before PriorityOrderedControllerAdvice. + OrderedControllerAdvice.class, + PriorityOrderedControllerAdvice.class, + OrderAnnotationControllerAdvice.class, + PriorityAnnotationControllerAdvice.class, + SimpleControllerAdvice.class, + }; + + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Config.class); + List adviceBeans = ControllerAdviceBean.findAnnotatedBeans(context); + + assertThat(adviceBeans).extracting(ControllerAdviceBean::getBeanType).containsExactly(expectedTypes); + } + private void assertEqualsHashCodeAndToString(ControllerAdviceBean bean1, ControllerAdviceBean bean2, String toString) { assertThat(bean1).isEqualTo(bean2); assertThat(bean2).isEqualTo(bean1); @@ -237,14 +262,17 @@ public class ControllerAdviceBeanTests { static class SimpleControllerAdvice {} @ControllerAdvice - @Order(42) + @Order(100) static class OrderAnnotationControllerAdvice {} @ControllerAdvice - @Priority(42) + @Priority(200) static class PriorityAnnotationControllerAdvice {} @ControllerAdvice + // @Order and @Priority should be ignored due to implementation of Ordered. + @Order(100) + @Priority(200) static class OrderedControllerAdvice implements Ordered { @Override @@ -253,6 +281,18 @@ public class ControllerAdviceBeanTests { } } + @ControllerAdvice + // @Order and @Priority should be ignored due to implementation of PriorityOrdered. + @Order(100) + @Priority(200) + static class PriorityOrderedControllerAdvice implements PriorityOrdered { + + @Override + public int getOrder() { + return 55; + } + } + @ControllerAdvice(annotations = ControllerAnnotation.class) static class AnnotationSupport {} @@ -294,4 +334,33 @@ public class ControllerAdviceBeanTests { static class InheritanceController extends AbstractController {} + @Configuration(proxyBeanMethods = false) + static class Config { + + @Bean + SimpleControllerAdvice simpleControllerAdvice() { + return new SimpleControllerAdvice(); + } + + @Bean + OrderAnnotationControllerAdvice orderAnnotationControllerAdvice() { + return new OrderAnnotationControllerAdvice(); + } + + @Bean + PriorityAnnotationControllerAdvice priorityAnnotationControllerAdvice() { + return new PriorityAnnotationControllerAdvice(); + } + + @Bean + OrderedControllerAdvice orderedControllerAdvice() { + return new OrderedControllerAdvice(); + } + + @Bean + PriorityOrderedControllerAdvice priorityOrderedControllerAdvice() { + return new PriorityOrderedControllerAdvice(); + } + } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java index 0c5c30a915..961b922074 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java @@ -36,7 +36,6 @@ import org.springframework.core.KotlinDetector; import org.springframework.core.MethodIntrospector; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.codec.HttpMessageReader; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -223,8 +222,6 @@ class ControllerMethodResolver { private void initControllerAdviceCaches(ApplicationContext applicationContext) { List beans = ControllerAdviceBean.findAnnotatedBeans(applicationContext); - AnnotationAwareOrderComparator.sort(beans); - for (ControllerAdviceBean bean : beans) { Class beanType = bean.getBeanType(); if (beanType != null) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java index 74d29db10a..f3c403e7bb 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java @@ -31,7 +31,6 @@ import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.HttpStatus; import org.springframework.http.converter.ByteArrayHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; @@ -273,8 +272,6 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce } List adviceBeans = ControllerAdviceBean.findAnnotatedBeans(getApplicationContext()); - AnnotationAwareOrderComparator.sort(adviceBeans); - for (ControllerAdviceBean adviceBean : adviceBeans) { Class beanType = adviceBean.getBeanType(); if (beanType == null) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 4bb3490261..0b1efe7d28 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -37,7 +37,6 @@ import org.springframework.core.MethodIntrospector; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.log.LogFormatUtils; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.core.task.SimpleAsyncTaskExecutor; @@ -578,7 +577,6 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter } List adviceBeans = ControllerAdviceBean.findAnnotatedBeans(getApplicationContext()); - AnnotationAwareOrderComparator.sort(adviceBeans); List requestResponseBodyAdviceBeans = new ArrayList<>(); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketAnnotationMethodMessageHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketAnnotationMethodMessageHandler.java index bb27b60fce..847741b2c1 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketAnnotationMethodMessageHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketAnnotationMethodMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.List; import org.springframework.context.ApplicationContext; -import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.lang.Nullable; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.SubscribableChannel; @@ -62,14 +61,10 @@ public class WebSocketAnnotationMethodMessageHandler extends SimpAnnotationMetho logger.trace("Looking for @MessageExceptionHandler mappings: " + context); } List beans = ControllerAdviceBean.findAnnotatedBeans(context); - AnnotationAwareOrderComparator.sort(beans); initMessagingAdviceCache(MessagingControllerAdviceBean.createFromList(beans)); } - private void initMessagingAdviceCache(@Nullable List beans) { - if (beans == null) { - return; - } + private void initMessagingAdviceCache(List beans) { for (MessagingAdviceBean bean : beans) { Class type = bean.getBeanType(); if (type != null) {