diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index 23550a46e04..53e4d165933 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -17,12 +17,14 @@ package org.springframework.web.servlet; import java.io.IOException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -42,9 +44,14 @@ import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.i18n.LocaleContext; +import org.springframework.core.OrderComparator; +import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.support.PropertiesLoaderUtils; @@ -513,7 +520,7 @@ private void initHandlerMappings(ApplicationContext context) { if (!matchingBeans.isEmpty()) { this.handlerMappings = new ArrayList<>(matchingBeans.values()); // We keep HandlerMappings in sorted order. - AnnotationAwareOrderComparator.sort(this.handlerMappings); + sortStrategyBeans(context, matchingBeans, this.handlerMappings); } } else { @@ -559,7 +566,7 @@ private void initHandlerAdapters(ApplicationContext context) { if (!matchingBeans.isEmpty()) { this.handlerAdapters = new ArrayList<>(matchingBeans.values()); // We keep HandlerAdapters in sorted order. - AnnotationAwareOrderComparator.sort(this.handlerAdapters); + sortStrategyBeans(context, matchingBeans, this.handlerAdapters); } } else { @@ -598,7 +605,7 @@ private void initHandlerExceptionResolvers(ApplicationContext context) { if (!matchingBeans.isEmpty()) { this.handlerExceptionResolvers = new ArrayList<>(matchingBeans.values()); // We keep HandlerExceptionResolvers in sorted order. - AnnotationAwareOrderComparator.sort(this.handlerExceptionResolvers); + sortStrategyBeans(context, matchingBeans, this.handlerExceptionResolvers); } } else { @@ -663,7 +670,7 @@ private void initViewResolvers(ApplicationContext context) { if (!matchingBeans.isEmpty()) { this.viewResolvers = new ArrayList<>(matchingBeans.values()); // We keep ViewResolvers in sorted order. - AnnotationAwareOrderComparator.sort(this.viewResolvers); + sortStrategyBeans(context, matchingBeans, this.viewResolvers); } } else { @@ -712,6 +719,24 @@ else if (logger.isDebugEnabled()) { } } + /** + * Sort the given strategy beans using {@link AnnotationAwareOrderComparator}, additionally + * consulting each bean's merged {@link BeanDefinition} for an + * {@link AbstractBeanDefinition#ORDER_ATTRIBUTE order attribute}, factory method or + * declared target type. This mirrors the ordering behavior the bean factory uses when + * resolving sorted dependency injections (see + * {@code DefaultListableBeanFactory.FactoryAwareOrderSourceProvider}), so programmatic + * ordering via {@code BeanRegistrar}, {@code GenericApplicationContext.registerBean(..., order)}, + * or a direct {@code ORDER_ATTRIBUTE} on a bean definition is reflected here. + */ + private static void sortStrategyBeans(ApplicationContext context, Map matchingBeans, List beans) { + if (beans.size() <= 1) { + return; + } + beans.sort(AnnotationAwareOrderComparator.INSTANCE.withSourceProvider( + new BeanDefinitionOrderSourceProvider(context, matchingBeans))); + } + /** * Obtain this servlet's MultipartResolver, if any. * @return the MultipartResolver used by this servlet, or {@code null} if none @@ -1405,4 +1430,63 @@ private static String getRequestUri(HttpServletRequest request) { return uri; } + /** + * {@link OrderComparator.OrderSourceProvider} that resolves order metadata for a given + * bean instance from its merged {@link BeanDefinition}: an + * {@link AbstractBeanDefinition#ORDER_ATTRIBUTE order attribute}, a factory method, and + * a declared target type when distinct from the bean's runtime class. Mirrors + * {@code DefaultListableBeanFactory.FactoryAwareOrderSourceProvider} so that + * DispatcherServlet's strategy detection sees the same ordering inputs the bean factory + * uses for sorted dependency injection. Standard {@code @Order} / {@link Ordered} + * fallback handling is left to the comparator. + */ + private static class BeanDefinitionOrderSourceProvider implements OrderComparator.OrderSourceProvider { + + private final @Nullable ApplicationContext context; + + private final Map instancesToBeanNames; + + BeanDefinitionOrderSourceProvider(@Nullable ApplicationContext context, Map matchingBeans) { + this.context = context; + this.instancesToBeanNames = new IdentityHashMap<>(matchingBeans.size()); + matchingBeans.forEach((name, instance) -> this.instancesToBeanNames.put(instance, name)); + } + + @Override + public @Nullable Object getOrderSource(Object obj) { + String beanName = this.instancesToBeanNames.get(obj); + if (beanName == null || !(this.context instanceof ConfigurableApplicationContext cac)) { + return null; + } + try { + BeanDefinition beanDefinition = cac.getBeanFactory().getMergedBeanDefinition(beanName); + List sources = new ArrayList<>(3); + Object orderAttribute = beanDefinition.getAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE); + if (orderAttribute != null) { + if (orderAttribute instanceof Integer order) { + sources.add((Ordered) () -> order); + } + else { + throw new IllegalStateException("Invalid value type for attribute '" + + AbstractBeanDefinition.ORDER_ATTRIBUTE + "': " + orderAttribute.getClass().getName()); + } + } + if (beanDefinition instanceof RootBeanDefinition rootBeanDefinition) { + Method factoryMethod = rootBeanDefinition.getResolvedFactoryMethod(); + if (factoryMethod != null) { + sources.add(factoryMethod); + } + Class targetType = rootBeanDefinition.getTargetType(); + if (targetType != null && targetType != obj.getClass()) { + sources.add(targetType); + } + } + return sources.toArray(); + } + catch (NoSuchBeanDefinitionException ex) { + return null; + } + } + } + } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java index 1bb17460bac..818cdfd2b27 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -30,15 +31,19 @@ import jakarta.servlet.http.HttpServletRequestWrapper; import jakarta.servlet.http.HttpServletResponse; import org.assertj.core.api.InstanceOfAssertFactories; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.PropertyValue; +import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.core.annotation.Order; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; import org.springframework.http.HttpHeaders; @@ -920,6 +925,96 @@ void shouldResetContentHeadersIfNotCommitted() throws Exception { assertThat(response.getHeaderNames()).doesNotContain(HttpHeaders.CONTENT_DISPOSITION); } + @Test // gh-36637 + void detectsHandlerMappingsOrderedByBeanDefinitionOrderAttribute() throws Exception { + StaticWebApplicationContext context = new StaticWebApplicationContext(); + context.setServletContext(getServletContext()); + + RootBeanDefinition first = new RootBeanDefinition(StubHandlerMapping.class); + first.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 1); + context.registerBeanDefinition("first", first); + + RootBeanDefinition second = new RootBeanDefinition(StubHandlerMapping.class); + second.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 2); + context.registerBeanDefinition("second", second); + + DispatcherServlet servlet = new DispatcherServlet(context); + servlet.init(servletConfig); + + List mappings = servlet.getHandlerMappings(); + assertThat(mappings).isNotNull().hasSize(2); + assertThat(mappings.get(0)).isSameAs(context.getBean("first")); + assertThat(mappings.get(1)).isSameAs(context.getBean("second")); + } + + @Test // gh-36637 + void beanDefinitionOrderAttributeOverridesAnnotationOrderForHandlerMappings() throws Exception { + StaticWebApplicationContext context = new StaticWebApplicationContext(); + context.setServletContext(getServletContext()); + + // Without an ORDER_ATTRIBUTE override this bean's @Order(1) would put it first. + RootBeanDefinition annotated = new RootBeanDefinition(LowOrderAnnotatedHandlerMapping.class); + annotated.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 100); + context.registerBeanDefinition("annotatedButOverridden", annotated); + + RootBeanDefinition viaAttribute = new RootBeanDefinition(StubHandlerMapping.class); + viaAttribute.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 1); + context.registerBeanDefinition("orderedByAttribute", viaAttribute); + + DispatcherServlet servlet = new DispatcherServlet(context); + servlet.init(servletConfig); + + List mappings = servlet.getHandlerMappings(); + assertThat(mappings).isNotNull().hasSize(2); + assertThat(mappings.get(0)).isSameAs(context.getBean("orderedByAttribute")); + assertThat(mappings.get(1)).isSameAs(context.getBean("annotatedButOverridden")); + } + + @Test // gh-36637 + void nonIntegerOrderAttributeIsRejected() { + StaticWebApplicationContext context = new StaticWebApplicationContext(); + context.setServletContext(getServletContext()); + + RootBeanDefinition invalid = new RootBeanDefinition(StubHandlerMapping.class); + invalid.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, "not-an-integer"); + context.registerBeanDefinition("invalid", invalid); + + // A second bean is required to actually trigger sorting. + RootBeanDefinition valid = new RootBeanDefinition(StubHandlerMapping.class); + valid.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 1); + context.registerBeanDefinition("valid", valid); + + DispatcherServlet servlet = new DispatcherServlet(context); + assertThatExceptionOfType(IllegalStateException.class) + .isThrownBy(() -> servlet.init(servletConfig)) + .withMessageContaining("Invalid value type for attribute 'order'"); + } + + @Test // gh-36637 + void handlerMappingOrderFromBeanDefinitionInheritsAcrossParentContext() throws Exception { + StaticWebApplicationContext parent = new StaticWebApplicationContext(); + parent.setServletContext(getServletContext()); + RootBeanDefinition fromParent = new RootBeanDefinition(StubHandlerMapping.class); + fromParent.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 5); + parent.registerBeanDefinition("fromParent", fromParent); + parent.refresh(); + + StaticWebApplicationContext child = new StaticWebApplicationContext(); + child.setServletContext(getServletContext()); + child.setParent(parent); + RootBeanDefinition fromChild = new RootBeanDefinition(StubHandlerMapping.class); + fromChild.setAttribute(AbstractBeanDefinition.ORDER_ATTRIBUTE, 1); + child.registerBeanDefinition("fromChild", fromChild); + + DispatcherServlet servlet = new DispatcherServlet(child); + servlet.init(servletConfig); + + List mappings = servlet.getHandlerMappings(); + assertThat(mappings).isNotNull().hasSize(2); + assertThat(mappings.get(0)).isSameAs(child.getBean("fromChild")); + assertThat(mappings.get(1)).isSameAs(parent.getBean("fromParent")); + } + public static class ControllerFromParent implements Controller { @@ -982,4 +1077,15 @@ public ModelAndView handleRequest(HttpServletRequest request, HttpServletRespons } } + private static class StubHandlerMapping implements HandlerMapping { + @Override + public @Nullable HandlerExecutionChain getHandler(HttpServletRequest request) { + return null; + } + } + + @Order(1) + private static class LowOrderAnnotatedHandlerMapping extends StubHandlerMapping { + } + }