Skip to content

Commit

Permalink
Improve PathMatcher/PatternParser XML configuration
Browse files Browse the repository at this point in the history
Prior to this commit, the MVC namespace for the XML Spring configuration
model would use the `PathMatcher` bean instance when provided like this:

```
<bean id="pathMatcher" class="org.springframework.util.AntPathMatcher"/>
<mvc:annotation-driven>
  <mvc:path-matching path-matcher="pathMatcher"/>
</mvc:annotation-driven>
<mvc:resources mapping="/resources/**" location="classpath:/static/"/>
```

With this configuration, the handler mapping for annotated controller
would use the given `AntPathMatcher` instance but the handler mapping
for resources would still use the default, which is `PathPatternParser`
since 6.0.

This commit ensures that when a custom `path-matcher` is defined, it's
consistently used for all MVC handler mappings as an alias to the
well-known bean name. This allows to use `AntPathMatcher` consistently
while working on a migration path to `PathPatternParser`

This commit also adds a new XML attribute to the path matching
configuration that makes it possible to use a custom `PathPatternParser`
instance:

```
<bean id="patternParser" class="org.springframework.web.util.pattern.PathPatternParser"/>
<mvc:annotation-driven>
  <mvc:path-matching pattern-parser="patternParser"/>
</mvc:annotation-driven>
```

Closes spring-projectsgh-34102
See spring-projectsgh-34064
  • Loading branch information
bclozel committed Dec 17, 2024
1 parent 95003e3 commit 4b45338
Show file tree
Hide file tree
Showing 9 changed files with 176 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,13 @@ private void configurePathMatchingProperties(
RootBeanDefinition handlerMappingDef, Element element, ParserContext context) {

Element pathMatchingElement = DomUtils.getChildElementByTagName(element, "path-matching");
Object source = context.extractSource(element);
if (pathMatchingElement != null) {
Object source = context.extractSource(element);

if (pathMatchingElement.hasAttribute("trailing-slash")) {
boolean useTrailingSlashMatch = Boolean.parseBoolean(pathMatchingElement.getAttribute("trailing-slash"));
handlerMappingDef.getPropertyValues().add("useTrailingSlashMatch", useTrailingSlashMatch);
}

boolean preferPathMatcher = false;

if (pathMatchingElement.hasAttribute("suffix-pattern")) {
boolean useSuffixPatternMatch = Boolean.parseBoolean(pathMatchingElement.getAttribute("suffix-pattern"));
handlerMappingDef.getPropertyValues().add("useSuffixPatternMatch", useSuffixPatternMatch);
Expand All @@ -435,12 +432,20 @@ private void configurePathMatchingProperties(
pathMatcherRef = new RuntimeBeanReference(pathMatchingElement.getAttribute("path-matcher"));
preferPathMatcher = true;
}
pathMatcherRef = MvcNamespaceUtils.registerPathMatcher(pathMatcherRef, context, source);
handlerMappingDef.getPropertyValues().add("pathMatcher", pathMatcherRef);

if (preferPathMatcher) {
pathMatcherRef = MvcNamespaceUtils.registerPathMatcher(pathMatcherRef, context, source);
handlerMappingDef.getPropertyValues().add("pathMatcher", pathMatcherRef);
handlerMappingDef.getPropertyValues().add("patternParser", null);
}
else if (pathMatchingElement.hasAttribute("pattern-parser")) {
RuntimeBeanReference patternParserRef = new RuntimeBeanReference(pathMatchingElement.getAttribute("pattern-parser"));
patternParserRef = MvcNamespaceUtils.registerPatternParser(patternParserRef, context, source);
handlerMappingDef.getPropertyValues().add("patternParser", patternParserRef);
}
}
else {
RuntimeBeanReference pathMatcherRef = MvcNamespaceUtils.registerPathMatcher(null, context, source);
handlerMappingDef.getPropertyValues().add("pathMatcher", pathMatcherRef);
}

}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -28,9 +28,11 @@
import org.springframework.beans.factory.xml.ParserContext;
import org.springframework.lang.Nullable;
import org.springframework.util.AntPathMatcher;
import org.springframework.util.Assert;
import org.springframework.util.PathMatcher;
import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.servlet.DispatcherServlet;
import org.springframework.web.servlet.handler.AbstractHandlerMapping;
import org.springframework.web.servlet.handler.BeanNameUrlHandlerMapping;
import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
import org.springframework.web.servlet.i18n.AcceptHeaderLocaleResolver;
Expand All @@ -39,6 +41,7 @@
import org.springframework.web.servlet.support.SessionFlashMapManager;
import org.springframework.web.servlet.view.DefaultRequestToViewNameTranslator;
import org.springframework.web.util.UrlPathHelper;
import org.springframework.web.util.pattern.PathPatternParser;

/**
* Convenience methods for use in MVC namespace BeanDefinitionParsers.
Expand All @@ -64,6 +67,8 @@ public abstract class MvcNamespaceUtils {

private static final String PATH_MATCHER_BEAN_NAME = "mvcPathMatcher";

private static final String PATTERN_PARSER_BEAN_NAME = "mvcPatternParser";

private static final String CORS_CONFIGURATION_BEAN_NAME = "mvcCorsConfigurations";

private static final String HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME = "mvcHandlerMappingIntrospector";
Expand Down Expand Up @@ -105,6 +110,18 @@ else if (!context.getRegistry().isAlias(URL_PATH_HELPER_BEAN_NAME) &&
return new RuntimeBeanReference(URL_PATH_HELPER_BEAN_NAME);
}

/**
* Return the {@link PathMatcher} bean definition if it has been registered
* in the context as an alias with its well-known name, or {@code null}.
*/
@Nullable
static RuntimeBeanReference getCustomPathMatcher(ParserContext context) {
if(context.getRegistry().isAlias(PATH_MATCHER_BEAN_NAME)) {
return new RuntimeBeanReference(PATH_MATCHER_BEAN_NAME);
}
return null;
}

/**
* Adds an alias to an existing well-known name or registers a new instance of a {@link PathMatcher}
* under that well-known name, unless already registered.
Expand All @@ -117,6 +134,9 @@ public static RuntimeBeanReference registerPathMatcher(@Nullable RuntimeBeanRefe
if (context.getRegistry().isAlias(PATH_MATCHER_BEAN_NAME)) {
context.getRegistry().removeAlias(PATH_MATCHER_BEAN_NAME);
}
if (context.getRegistry().containsBeanDefinition(PATH_MATCHER_BEAN_NAME)) {
context.getRegistry().removeBeanDefinition(PATH_MATCHER_BEAN_NAME);
}
context.getRegistry().registerAlias(pathMatcherRef.getBeanName(), PATH_MATCHER_BEAN_NAME);
}
else if (!context.getRegistry().isAlias(PATH_MATCHER_BEAN_NAME) &&
Expand All @@ -130,6 +150,60 @@ else if (!context.getRegistry().isAlias(PATH_MATCHER_BEAN_NAME) &&
return new RuntimeBeanReference(PATH_MATCHER_BEAN_NAME);
}

/**
* Return the {@link PathPatternParser} bean definition if it has been registered
* in the context as an alias with its well-known name, or {@code null}.
*/
@Nullable
static RuntimeBeanReference getCustomPatternParser(ParserContext context) {
if (context.getRegistry().isAlias(PATTERN_PARSER_BEAN_NAME)) {
return new RuntimeBeanReference(PATTERN_PARSER_BEAN_NAME);
}
return null;
}

/**
* Adds an alias to an existing well-known name or registers a new instance of a {@link PathPatternParser}
* under that well-known name, unless already registered.
* @return a RuntimeBeanReference to this {@link PathPatternParser} instance
*/
public static RuntimeBeanReference registerPatternParser(@Nullable RuntimeBeanReference patternParserRef,
ParserContext context, @Nullable Object source) {
if (patternParserRef != null) {
if (context.getRegistry().isAlias(PATTERN_PARSER_BEAN_NAME)) {
context.getRegistry().removeAlias(PATTERN_PARSER_BEAN_NAME);
}
context.getRegistry().registerAlias(patternParserRef.getBeanName(), PATTERN_PARSER_BEAN_NAME);
}
else if (!context.getRegistry().isAlias(PATTERN_PARSER_BEAN_NAME) &&
!context.getRegistry().containsBeanDefinition(PATTERN_PARSER_BEAN_NAME)) {
RootBeanDefinition pathMatcherDef = new RootBeanDefinition(PathPatternParser.class);
pathMatcherDef.setSource(source);
pathMatcherDef.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
context.getRegistry().registerBeanDefinition(PATTERN_PARSER_BEAN_NAME, pathMatcherDef);
context.registerComponent(new BeanComponentDefinition(pathMatcherDef, PATTERN_PARSER_BEAN_NAME));
}
return new RuntimeBeanReference(PATTERN_PARSER_BEAN_NAME);
}

static void configurePathMatching(RootBeanDefinition handlerMappingDef, ParserContext context, @Nullable Object source) {
Assert.isTrue(AbstractHandlerMapping.class.isAssignableFrom(handlerMappingDef.getBeanClass()),
() -> "Handler mapping type [" + handlerMappingDef.getTargetType() + "] not supported");
RuntimeBeanReference customPathMatcherRef = MvcNamespaceUtils.getCustomPathMatcher(context);
RuntimeBeanReference customPatternParserRef = MvcNamespaceUtils.getCustomPatternParser(context);
if (customPathMatcherRef != null) {
handlerMappingDef.getPropertyValues().add("pathMatcher", customPathMatcherRef)
.add("patternParser", null);
}
else if (customPatternParserRef != null) {
handlerMappingDef.getPropertyValues().add("patternParser", customPatternParserRef);
}
else {
handlerMappingDef.getPropertyValues().add("pathMatcher",
MvcNamespaceUtils.registerPathMatcher(null, context, source));
}
}

/**
* Registers an {@link HttpRequestHandlerAdapter} under a well-known
* name unless already registered.
Expand All @@ -142,6 +216,7 @@ private static void registerBeanNameUrlHandlerMapping(ParserContext context, @Nu
mappingDef.getPropertyValues().add("order", 2); // consistent with WebMvcConfigurationSupport
RuntimeBeanReference corsRef = MvcNamespaceUtils.registerCorsConfigurations(null, context, source);
mappingDef.getPropertyValues().add("corsConfigurations", corsRef);
configurePathMatching(mappingDef, context, source);
context.getRegistry().registerBeanDefinition(BEAN_NAME_URL_HANDLER_MAPPING_BEAN_NAME, mappingDef);
context.registerComponent(new BeanComponentDefinition(mappingDef, BEAN_NAME_URL_HANDLER_MAPPING_BEAN_NAME));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -92,7 +92,6 @@ public BeanDefinition parse(Element element, ParserContext context) {

registerUrlProvider(context, source);

RuntimeBeanReference pathMatcherRef = MvcNamespaceUtils.registerPathMatcher(null, context, source);
RuntimeBeanReference pathHelperRef = MvcNamespaceUtils.registerUrlPathHelper(null, context, source);

String resourceHandlerName = registerResourceHandler(context, element, pathHelperRef, source);
Expand All @@ -111,8 +110,8 @@ public BeanDefinition parse(Element element, ParserContext context) {
RootBeanDefinition handlerMappingDef = new RootBeanDefinition(SimpleUrlHandlerMapping.class);
handlerMappingDef.setSource(source);
handlerMappingDef.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
handlerMappingDef.getPropertyValues().add("urlMap", urlMap);
handlerMappingDef.getPropertyValues().add("pathMatcher", pathMatcherRef).add("urlPathHelper", pathHelperRef);
handlerMappingDef.getPropertyValues().add("urlMap", urlMap).add("urlPathHelper", pathHelperRef);
MvcNamespaceUtils.configurePathMatching(handlerMappingDef, context, source);

String orderValue = element.getAttribute("order");
// Use a default of near-lowest precedence, still allowing for even lower precedence in other mappings
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 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.
Expand Down Expand Up @@ -123,7 +123,7 @@ private BeanDefinition registerHandlerMapping(ParserContext context, @Nullable O

beanDef.setSource(source);
beanDef.getPropertyValues().add("order", "1");
beanDef.getPropertyValues().add("pathMatcher", MvcNamespaceUtils.registerPathMatcher(null, context, source));
MvcNamespaceUtils.configurePathMatching(beanDef, context, source);
beanDef.getPropertyValues().add("urlPathHelper", MvcNamespaceUtils.registerUrlPathHelper(null, context, source));
RuntimeBeanReference corsConfigurationsRef = MvcNamespaceUtils.registerCorsConfigurations(null, context, source);
beanDef.getPropertyValues().add("corsConfigurations", corsConfigurationsRef);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,14 @@
<xsd:annotation>
<xsd:documentation><![CDATA[
The bean name of the PathMatcher implementation to use for matching URL paths against registered URL patterns.
Default is AntPathMatcher.
If no bean is provided, the PathPatternParser will be used instead.
]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="pattern-parser" type="xsd:string">
<xsd:annotation>
<xsd:documentation><![CDATA[
The bean name of the PathPatternParser instance to use for matching URL paths against registered URL patterns.
]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public void testPathMatchingConfiguration() {
assertThat(hm.useRegisteredSuffixPatternMatch()).isTrue();
assertThat(hm.getUrlPathHelper()).isInstanceOf(TestPathHelper.class);
assertThat(hm.getPathMatcher()).isInstanceOf(TestPathMatcher.class);
assertThat(hm.getPatternParser()).isNull();
List<String> fileExtensions = hm.getContentNegotiationManager().getAllFileExtensions();
assertThat(fileExtensions).containsExactly("xml");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import org.springframework.lang.Nullable;
import org.springframework.scheduling.concurrent.ConcurrentTaskExecutor;
import org.springframework.stereotype.Controller;
import org.springframework.util.AntPathMatcher;
import org.springframework.util.PathMatcher;
import org.springframework.validation.BindingResult;
import org.springframework.validation.Errors;
Expand Down Expand Up @@ -145,6 +146,7 @@
import org.springframework.web.testfixture.servlet.MockRequestDispatcher;
import org.springframework.web.testfixture.servlet.MockServletContext;
import org.springframework.web.util.UrlPathHelper;
import org.springframework.web.util.pattern.PathPatternParser;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -202,6 +204,8 @@ void testDefaultConfig() throws Exception {
assertThat(mapping).isNotNull();
assertThat(mapping.getOrder()).isEqualTo(0);
assertThat(mapping.getUrlPathHelper().shouldRemoveSemicolonContent()).isTrue();
assertThat(mapping.getPathMatcher()).isEqualTo(appContext.getBean("mvcPathMatcher"));
assertThat(mapping.getPatternParser()).isNotNull();
mapping.setDefaultHandler(handlerMethod);

MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.json");
Expand Down Expand Up @@ -392,6 +396,8 @@ void testResources() throws Exception {
SimpleUrlHandlerMapping resourceMapping = appContext.getBean(SimpleUrlHandlerMapping.class);
assertThat(resourceMapping).isNotNull();
assertThat(resourceMapping.getOrder()).isEqualTo(Ordered.LOWEST_PRECEDENCE - 1);
assertThat(resourceMapping.getPathMatcher()).isNotNull();
assertThat(resourceMapping.getPatternParser()).isNotNull();

BeanNameUrlHandlerMapping beanNameMapping = appContext.getBean(BeanNameUrlHandlerMapping.class);
assertThat(beanNameMapping).isNotNull();
Expand Down Expand Up @@ -423,6 +429,31 @@ void testResources() throws Exception {
.isInstanceOf(NoResourceFoundException.class);
}

@Test
void testUseDeprecatedPathMatcher() throws Exception {
loadBeanDefinitions("mvc-config-deprecated-path-matcher.xml");
Map<String, AbstractHandlerMapping> handlerMappings = appContext.getBeansOfType(AbstractHandlerMapping.class);
AntPathMatcher mvcPathMatcher = appContext.getBean("pathMatcher", AntPathMatcher.class);
assertThat(handlerMappings).hasSize(4);
handlerMappings.forEach((name, hm) -> {
assertThat(hm.getPathMatcher()).as("path matcher for %s", name).isEqualTo(mvcPathMatcher);
assertThat(hm.getPatternParser()).as("pattern parser for %s", name).isNull();
});
}

@Test
void testUsePathPatternParser() throws Exception {
loadBeanDefinitions("mvc-config-custom-pattern-parser.xml");

PathPatternParser patternParser = appContext.getBean("patternParser", PathPatternParser.class);
Map<String, AbstractHandlerMapping> handlerMappings = appContext.getBeansOfType(AbstractHandlerMapping.class);
assertThat(handlerMappings).hasSize(4);
handlerMappings.forEach((name, hm) -> {
assertThat(hm.getPathMatcher()).as("path matcher for %s", name).isNotNull();
assertThat(hm.getPatternParser()).as("pattern parser for %s", name).isEqualTo(patternParser);
});
}

@Test
void testResourcesWithOptionalAttributes() {
loadBeanDefinitions("mvc-config-resources-optional-attrs.xml");
Expand Down Expand Up @@ -600,6 +631,9 @@ void testViewControllers() throws Exception {
assertThat(beanNameMapping).isNotNull();
assertThat(beanNameMapping.getOrder()).isEqualTo(2);

assertThat(beanNameMapping.getPathMatcher()).isNotNull();
assertThat(beanNameMapping.getPatternParser()).isNotNull();

MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("GET");

Expand Down Expand Up @@ -895,11 +929,11 @@ void testPathMatchingHandlerMappings() {
assertThat(viewController.getUrlPathHelper().getClass()).isEqualTo(TestPathHelper.class);
assertThat(viewController.getPathMatcher().getClass()).isEqualTo(TestPathMatcher.class);

for (SimpleUrlHandlerMapping handlerMapping : appContext.getBeansOfType(SimpleUrlHandlerMapping.class).values()) {
appContext.getBeansOfType(SimpleUrlHandlerMapping.class).forEach((name, handlerMapping) -> {
assertThat(handlerMapping).isNotNull();
assertThat(handlerMapping.getUrlPathHelper().getClass()).isEqualTo(TestPathHelper.class);
assertThat(handlerMapping.getPathMatcher().getClass()).isEqualTo(TestPathMatcher.class);
}
assertThat(handlerMapping.getUrlPathHelper().getClass()).as("path helper for %s", name).isEqualTo(TestPathHelper.class);
assertThat(handlerMapping.getPathMatcher().getClass()).as("path matcher for %s", name).isEqualTo(TestPathMatcher.class);
});
}

@Test
Expand All @@ -909,7 +943,7 @@ void testCorsMinimal() {
String[] beanNames = appContext.getBeanNamesForType(AbstractHandlerMapping.class);
assertThat(beanNames).hasSize(2);
for (String beanName : beanNames) {
AbstractHandlerMapping handlerMapping = (AbstractHandlerMapping)appContext.getBean(beanName);
AbstractHandlerMapping handlerMapping = (AbstractHandlerMapping) appContext.getBean(beanName);
assertThat(handlerMapping).isNotNull();
DirectFieldAccessor accessor = new DirectFieldAccessor(handlerMapping);
Map<String, CorsConfiguration> configs = ((UrlBasedCorsConfigurationSource) accessor
Expand All @@ -934,7 +968,7 @@ void testCors() {
String[] beanNames = appContext.getBeanNamesForType(AbstractHandlerMapping.class);
assertThat(beanNames).hasSize(2);
for (String beanName : beanNames) {
AbstractHandlerMapping handlerMapping = (AbstractHandlerMapping)appContext.getBean(beanName);
AbstractHandlerMapping handlerMapping = (AbstractHandlerMapping) appContext.getBean(beanName);
assertThat(handlerMapping).isNotNull();
DirectFieldAccessor accessor = new DirectFieldAccessor(handlerMapping);
Map<String, CorsConfiguration> configs = ((UrlBasedCorsConfigurationSource) accessor
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:mvc="http://www.springframework.org/schema/mvc"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/mvc https://www.springframework.org/schema/mvc/spring-mvc.xsd">

<bean id="patternParser" class="org.springframework.web.util.pattern.PathPatternParser"/>

<mvc:annotation-driven>
<mvc:path-matching pattern-parser="patternParser"/>
</mvc:annotation-driven>

<mvc:view-controller path="/foo"/>
<mvc:resources mapping="/resources/**" location="/, classpath:/META-INF/"/>

</beans>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:mvc="http://www.springframework.org/schema/mvc"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/mvc https://www.springframework.org/schema/mvc/spring-mvc.xsd">

<bean id="pathMatcher" class="org.springframework.util.AntPathMatcher"/>

<mvc:annotation-driven>
<mvc:path-matching path-matcher="pathMatcher"/>
</mvc:annotation-driven>

<mvc:view-controller path="/foo"/>
<mvc:resources mapping="/resources/**" location="/, classpath:/META-INF/"/>

</beans>

0 comments on commit 4b45338

Please sign in to comment.