Skip to content

Commit 4370030

Browse files
rstoyanchevrstoyanchev
committed
RequestMappingInfo defaults to PathPatternParser
This changes ensures RequestMappingInfo uses PathPatternParser by default as all AbstractHandlerMapping implementations do as of 6.0. RequestMappingInfo instances are typically created internally and aligned with the RequestMappingHandlerMapping in terms of path mapping options. If a RequestMappingInfo is registered programmatically, the caller needs to also ensure they are aligned. However, if the two should be aligned by default. Closes gh-31662
1 parent 54ecbb2 commit 4370030

File tree

4 files changed

+61
-35
lines changed

4 files changed

+61
-35
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -705,27 +705,28 @@ public Builder options(BuilderConfiguration options) {
705705
@SuppressWarnings("deprecation")
706706
public RequestMappingInfo build() {
707707

708-
PathPatternsRequestCondition pathPatterns = null;
709-
PatternsRequestCondition patterns = null;
708+
PathPatternsRequestCondition pathPatternsCondition = null;
709+
PatternsRequestCondition patternsCondition = null;
710710

711-
if (this.options.patternParser != null) {
712-
pathPatterns = (ObjectUtils.isEmpty(this.paths) ?
713-
EMPTY_PATH_PATTERNS :
714-
new PathPatternsRequestCondition(this.options.patternParser, this.paths));
715-
}
716-
else {
717-
patterns = (ObjectUtils.isEmpty(this.paths) ?
711+
if (this.options.getPathMatcher() != null) {
712+
patternsCondition = (ObjectUtils.isEmpty(this.paths) ?
718713
EMPTY_PATTERNS :
719714
new PatternsRequestCondition(
720715
this.paths, null, this.options.getPathMatcher(),
721716
this.options.useSuffixPatternMatch(), this.options.useTrailingSlashMatch(),
722717
this.options.getFileExtensions()));
723718
}
719+
else {
720+
PathPatternParser parser = (this.options.getPatternParser() != null ?
721+
this.options.getPatternParser() : new PathPatternParser());
722+
pathPatternsCondition = (ObjectUtils.isEmpty(this.paths) ?
723+
EMPTY_PATH_PATTERNS : new PathPatternsRequestCondition(parser, this.paths));
724+
}
724725

725726
ContentNegotiationManager manager = this.options.getContentNegotiationManager();
726727

727728
return new RequestMappingInfo(
728-
this.mappingName, pathPatterns, patterns,
729+
this.mappingName, pathPatternsCondition, patternsCondition,
729730
ObjectUtils.isEmpty(this.methods) ?
730731
EMPTY_REQUEST_METHODS : new RequestMethodsRequestCondition(this.methods),
731732
ObjectUtils.isEmpty(this.params) ?
@@ -737,8 +738,7 @@ public RequestMappingInfo build() {
737738
ObjectUtils.isEmpty(this.produces) && !this.hasAccept ?
738739
EMPTY_PRODUCES : new ProducesRequestCondition(this.produces, this.headers, manager),
739740
this.customCondition != null ?
740-
new RequestConditionHolder(this.customCondition) : EMPTY_CUSTOM,
741-
this.options);
741+
new RequestConditionHolder(this.customCondition) : EMPTY_CUSTOM, this.options);
742742
}
743743
}
744744

@@ -894,7 +894,9 @@ public static class BuilderConfiguration {
894894
* {@link AbstractHandlerMapping#setPatternParser(PathPatternParser)}.
895895
* <p><strong>Note:</strong> This property is mutually exclusive with
896896
* {@link #setPathMatcher(PathMatcher)}.
897-
* <p>By default this is not enabled.
897+
* <p>By default this is not set, but {@link RequestMappingInfo.Builder}
898+
* defaults to using {@link PathPatternParser} unless
899+
* {@link #setPathMatcher(PathMatcher)} is explicitly set.
898900
* @since 5.3
899901
*/
900902
public void setPatternParser(@Nullable PathPatternParser patternParser) {
@@ -936,7 +938,9 @@ public UrlPathHelper getUrlPathHelper() {
936938

937939
/**
938940
* Set a custom PathMatcher to use for the PatternsRequestCondition.
939-
* <p>By default this is not set.
941+
* <p>By default this is not set. You must set it explicitly if you want
942+
* {@link PathMatcher} to be used, or otherwise {@link RequestMappingInfo}
943+
* defaults to using {@link PathPatternParser}.
940944
*/
941945
public void setPathMatcher(@Nullable PathMatcher pathMatcher) {
942946
this.pathMatcher = pathMatcher;

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.springframework.http.server.observation.ServerRequestObservationContext;
3939
import org.springframework.lang.Nullable;
4040
import org.springframework.stereotype.Controller;
41+
import org.springframework.util.AntPathMatcher;
4142
import org.springframework.util.MultiValueMap;
4243
import org.springframework.web.HttpMediaTypeNotAcceptableException;
4344
import org.springframework.web.HttpMediaTypeNotSupportedException;
@@ -258,10 +259,13 @@ void getHandlerMappedInterceptors() throws Exception {
258259
@SuppressWarnings("unchecked")
259260
@PathPatternsParameterizedTest
260261
void handleMatchUriTemplateVariables(TestRequestMappingInfoHandlerMapping mapping) {
261-
RequestMappingInfo key = RequestMappingInfo.paths("/{path1}/{path2}").build();
262+
RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration();
263+
config.setPathMatcher(new AntPathMatcher());
264+
265+
RequestMappingInfo info = RequestMappingInfo.paths("/{path1}/{path2}").options(config).build();
262266
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2");
263267
String lookupPath = new UrlPathHelper().getLookupPathForRequest(request);
264-
mapping.handleMatch(key, lookupPath, request);
268+
mapping.handleMatch(info, lookupPath, request);
265269

266270
String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
267271
Map<String, String> uriVariables = (Map<String, String>) request.getAttribute(name);
@@ -274,15 +278,18 @@ void handleMatchUriTemplateVariables(TestRequestMappingInfoHandlerMapping mappin
274278
@SuppressWarnings("unchecked")
275279
@PathPatternsParameterizedTest // SPR-9098
276280
void handleMatchUriTemplateVariablesDecode(TestRequestMappingInfoHandlerMapping mapping) {
277-
RequestMappingInfo key = RequestMappingInfo.paths("/{group}/{identifier}").build();
281+
RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration();
282+
config.setPathMatcher(new AntPathMatcher());
283+
284+
RequestMappingInfo info = RequestMappingInfo.paths("/{group}/{identifier}").options(config).build();
278285
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/group/a%2Fb");
279286

280287
UrlPathHelper pathHelper = new UrlPathHelper();
281288
pathHelper.setUrlDecode(false);
282289
String lookupPath = pathHelper.getLookupPathForRequest(request);
283290

284291
mapping.setUrlPathHelper(pathHelper);
285-
mapping.handleMatch(key, lookupPath, request);
292+
mapping.handleMatch(info, lookupPath, request);
286293

287294
String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
288295
Map<String, String> uriVariables = (Map<String, String>) request.getAttribute(name);
@@ -294,20 +301,26 @@ void handleMatchUriTemplateVariablesDecode(TestRequestMappingInfoHandlerMapping
294301

295302
@PathPatternsParameterizedTest
296303
void handleMatchBestMatchingPatternAttribute(TestRequestMappingInfoHandlerMapping mapping) {
297-
RequestMappingInfo key = RequestMappingInfo.paths("/{path1}/2", "/**").build();
304+
RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration();
305+
config.setPathMatcher(new AntPathMatcher());
306+
307+
RequestMappingInfo info = RequestMappingInfo.paths("/{path1}/2", "/**").options(config).build();
298308
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2");
299-
mapping.handleMatch(key, "/1/2", request);
309+
mapping.handleMatch(info, "/1/2", request);
300310

301311
assertThat(request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)).isEqualTo("/{path1}/2");
302312
}
303313

304314
@PathPatternsParameterizedTest
305315
void handleMatchBestMatchingPatternAttributeInObservationContext(TestRequestMappingInfoHandlerMapping mapping) {
306-
RequestMappingInfo key = RequestMappingInfo.paths("/{path1}/2", "/**").build();
316+
RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration();
317+
config.setPathMatcher(new AntPathMatcher());
318+
319+
RequestMappingInfo info = RequestMappingInfo.paths("/{path1}/2", "/**").options(config).build();
307320
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2");
308321
ServerRequestObservationContext observationContext = new ServerRequestObservationContext(request, new MockHttpServletResponse());
309322
request.setAttribute(ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext);
310-
mapping.handleMatch(key, "/1/2", request);
323+
mapping.handleMatch(info, "/1/2", request);
311324

312325
assertThat(request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)).isEqualTo("/{path1}/2");
313326
assertThat(observationContext.getPathPattern()).isEqualTo("/{path1}/2");

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.springframework.http.HttpHeaders;
2828
import org.springframework.http.MediaType;
29+
import org.springframework.util.AntPathMatcher;
2930
import org.springframework.web.bind.annotation.RequestMethod;
3031
import org.springframework.web.servlet.handler.PathPatternsParameterizedTest;
3132
import org.springframework.web.servlet.handler.PathPatternsTestUtils;
@@ -48,8 +49,8 @@ class RequestMappingInfoTests {
4849
@SuppressWarnings("unused")
4950
static Stream<RequestMappingInfo.Builder> pathPatternsArguments() {
5051
RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration();
51-
config.setPatternParser(new PathPatternParser());
52-
return Stream.of(RequestMappingInfo.paths().options(config), RequestMappingInfo.paths());
52+
config.setPathMatcher(new AntPathMatcher());
53+
return Stream.of(RequestMappingInfo.paths(), RequestMappingInfo.paths().options(config));
5354
}
5455

5556

@@ -86,6 +87,13 @@ void createEmpty(RequestMappingInfo.Builder infoBuilder) {
8687
assertThat(info.getCustomCondition()).isSameAs(result.getCustomCondition());
8788
}
8889

90+
@Test // gh-31662
91+
void pathPatternByDefault() {
92+
RequestMappingInfo info = RequestMappingInfo.paths().build();
93+
assertThat(info.getPathPatternsCondition()).isNotNull();
94+
assertThat(info.getPatternsCondition()).isNull();
95+
}
96+
8997
@PathPatternsParameterizedTest
9098
void matchPatternsCondition(RequestMappingInfo.Builder builder) {
9199

@@ -105,7 +113,7 @@ void matchPatternsCondition(RequestMappingInfo.Builder builder) {
105113

106114
@Test
107115
void matchParamsCondition() {
108-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false);
116+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true);
109117
request.setParameter("foo", "bar");
110118

111119
RequestMappingInfo info = RequestMappingInfo.paths("/foo").params("foo=bar").build();
@@ -121,7 +129,7 @@ void matchParamsCondition() {
121129

122130
@Test
123131
void matchHeadersCondition() {
124-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false);
132+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true);
125133
request.addHeader("foo", "bar");
126134

127135
RequestMappingInfo info = RequestMappingInfo.paths("/foo").headers("foo=bar").build();
@@ -137,7 +145,7 @@ void matchHeadersCondition() {
137145

138146
@Test
139147
void matchConsumesCondition() {
140-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false);
148+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true);
141149
request.setContentType("text/plain");
142150

143151
RequestMappingInfo info = RequestMappingInfo.paths("/foo").consumes("text/plain").build();
@@ -153,7 +161,7 @@ void matchConsumesCondition() {
153161

154162
@Test
155163
void matchProducesCondition() {
156-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false);
164+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true);
157165
request.addHeader("Accept", "text/plain");
158166

159167
RequestMappingInfo info = RequestMappingInfo.paths("/foo").produces("text/plain").build();
@@ -169,7 +177,7 @@ void matchProducesCondition() {
169177

170178
@Test
171179
void matchCustomCondition() {
172-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false);
180+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true);
173181
request.setParameter("foo", "bar");
174182

175183
RequestMappingInfo info = RequestMappingInfo.paths("/foo").params("foo=bar").build();
@@ -189,7 +197,7 @@ void compareToWithImplicitVsExplicitHttpMethodDeclaration() {
189197
RequestMappingInfo oneMethod = RequestMappingInfo.paths().methods(GET).build();
190198
RequestMappingInfo oneMethodOneParam = RequestMappingInfo.paths().methods(GET).params("foo").build();
191199

192-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", false);
200+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", true);
193201
Comparator<RequestMappingInfo> comparator = (info, otherInfo) -> info.compareTo(otherInfo, request);
194202

195203
List<RequestMappingInfo> list = asList(noMethods, oneMethod, oneMethodOneParam);
@@ -204,7 +212,7 @@ void compareToWithImplicitVsExplicitHttpMethodDeclaration() {
204212
@Test
205213
// SPR-14383
206214
void compareToWithHttpHeadMapping() {
207-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", false);
215+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", true);
208216
request.setMethod("HEAD");
209217
request.addHeader("Accept", "application/json");
210218

@@ -297,7 +305,7 @@ void equalsMethod(RequestMappingInfo.Builder infoBuilder) {
297305

298306
@Test
299307
void preFlightRequest() {
300-
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("OPTIONS", "/foo", false);
308+
MockHttpServletRequest request = PathPatternsTestUtils.initRequest("OPTIONS", "/foo", true);
301309
request.addHeader(HttpHeaders.ORIGIN, "https://domain.com");
302310
request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "POST");
303311

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,6 +38,7 @@
3838
import org.springframework.http.server.RequestPath;
3939
import org.springframework.lang.Nullable;
4040
import org.springframework.stereotype.Controller;
41+
import org.springframework.util.AntPathMatcher;
4142
import org.springframework.util.CollectionUtils;
4243
import org.springframework.web.bind.annotation.CrossOrigin;
4344
import org.springframework.web.bind.annotation.GetMapping;
@@ -595,8 +596,8 @@ protected RequestMappingInfo getMappingForMethod(Method method, Class<?> handler
595596
RequestMapping annotation = AnnotatedElementUtils.findMergedAnnotation(method, RequestMapping.class);
596597
if (annotation != null) {
597598
RequestMappingInfo.BuilderConfiguration options = new RequestMappingInfo.BuilderConfiguration();
598-
if (getPatternParser() != null) {
599-
options.setPatternParser(getPatternParser());
599+
if (getPatternParser() == null) {
600+
options.setPathMatcher(new AntPathMatcher());
600601
}
601602
return RequestMappingInfo.paths(annotation.value())
602603
.methods(annotation.method())

0 commit comments

Comments
 (0)