Skip to content

Commit 4fd7645

Browse files
rstoyanchevrstoyanchev
committed
Enable smart suffix pattern match for request mapping
Following the introduction of ContentNegotiationManager that allows, among other things, to configure the file extensions to use for content negotiation, this change adds "smart" suffix pattern match that matches against the configured file extensions only rather than against any extension. Given the request mapping "/jobs/{jobName}" and one configured file extension ("json"), a request for "/jobs/my.job" will select the pattern "/jobs/{jobName}" while a request for "/jobs/my.job.json" will select the pattern "/jobs/{jobName}.json". Previously, both requests would have resulted in the pattern "/jobs/{jobName}.*". Issue: SPR-7632, SPR-8474
1 parent f94aed8 commit 4fd7645

File tree

7 files changed

+182
-77
lines changed

7 files changed

+182
-77
lines changed

‎spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,23 @@ public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws Htt
9797
* the list of all file extensions found.
9898
*/
9999
public List<String> resolveFileExtensions(MediaType mediaType) {
100-
Set<String> extensions = new LinkedHashSet<String>();
100+
Set<String> result = new LinkedHashSet<String>();
101101
for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) {
102-
extensions.addAll(resolver.resolveFileExtensions(mediaType));
102+
result.addAll(resolver.resolveFileExtensions(mediaType));
103103
}
104-
return new ArrayList<String>(extensions);
104+
return new ArrayList<String>(result);
105+
}
106+
107+
/**
108+
* Delegate to all configured MediaTypeFileExtensionResolver instances and aggregate
109+
* the list of all known file extensions.
110+
*/
111+
public List<String> getAllFileExtensions() {
112+
Set<String> result = new LinkedHashSet<String>();
113+
for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) {
114+
result.addAll(resolver.getAllFileExtensions());
115+
}
116+
return new ArrayList<String>(result);
105117
}
106118

107119
}

‎spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.springframework.web.accept;
1717

18+
import java.util.ArrayList;
1819
import java.util.Collections;
1920
import java.util.List;
2021
import java.util.Locale;
@@ -40,6 +41,8 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten
4041

4142
private final MultiValueMap<MediaType, String> fileExtensions = new LinkedMultiValueMap<MediaType, String>();
4243

44+
private final List<String> allFileExtensions = new ArrayList<String>();
45+
4346
/**
4447
* Create an instance with the given mappings between extensions and media types.
4548
* @throws IllegalArgumentException if a media type string cannot be parsed
@@ -55,19 +58,23 @@ public MappingMediaTypeFileExtensionResolver(Map<String, MediaType> mediaTypes)
5558
}
5659

5760
/**
58-
* Find the extensions applicable to the given MediaType.
61+
* Find the file extensions mapped to the given MediaType.
5962
* @return 0 or more extensions, never {@code null}
6063
*/
6164
public List<String> resolveFileExtensions(MediaType mediaType) {
6265
List<String> fileExtensions = this.fileExtensions.get(mediaType);
6366
return (fileExtensions != null) ? fileExtensions : Collections.<String>emptyList();
6467
}
6568

69+
public List<String> getAllFileExtensions() {
70+
return Collections.unmodifiableList(this.allFileExtensions);
71+
}
72+
6673
/**
6774
* Return the MediaType mapped to the given extension.
6875
* @return a MediaType for the key or {@code null}
6976
*/
70-
public MediaType lookupMediaType(String extension) {
77+
protected MediaType lookupMediaType(String extension) {
7178
return this.mediaTypes.get(extension);
7279
}
7380

@@ -78,6 +85,7 @@ protected void addMapping(String extension, MediaType mediaType) {
7885
MediaType previous = this.mediaTypes.putIfAbsent(extension, mediaType);
7986
if (previous == null) {
8087
this.fileExtensions.add(mediaType, extension);
88+
this.allFileExtensions.add(extension);
8189
}
8290
}
8391

‎spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,10 @@ public interface MediaTypeFileExtensionResolver {
3737
*/
3838
List<String> resolveFileExtensions(MediaType mediaType);
3939

40+
/**
41+
* Return all known file extensions.
42+
* @return a list of extensions or an empty list, never {@code null}
43+
*/
44+
List<String> getAllFileExtensions();
45+
4046
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java

Lines changed: 88 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -34,63 +34,86 @@
3434
import org.springframework.web.util.UrlPathHelper;
3535

3636
/**
37-
* A logical disjunction (' || ') request condition that matches a request
38-
* against a set of URL path patterns.
39-
*
37+
* A logical disjunction (' || ') request condition that matches a request
38+
* against a set of URL path patterns.
39+
*
4040
* @author Rossen Stoyanchev
4141
* @since 3.1
4242
*/
4343
public final class PatternsRequestCondition extends AbstractRequestCondition<PatternsRequestCondition> {
44-
45-
private final Set<String> patterns;
44+
45+
private final Set<String> patterns;
4646

4747
private final UrlPathHelper urlPathHelper;
48-
48+
4949
private final PathMatcher pathMatcher;
5050

5151
private final boolean useSuffixPatternMatch;
5252

5353
private final boolean useTrailingSlashMatch;
54-
54+
55+
private final List<String> fileExtensions = new ArrayList<String>();
56+
5557
/**
5658
* Creates a new instance with the given URL patterns.
57-
* Each pattern that is not empty and does not start with "/" is pre-pended with "/".
58-
* @param patterns 0 or more URL patterns; if 0 the condition will match to every request.
59+
* Each pattern that is not empty and does not start with "/" is prepended with "/".
60+
* @param patterns 0 or more URL patterns; if 0 the condition will match to every request.
5961
*/
6062
public PatternsRequestCondition(String... patterns) {
61-
this(asList(patterns), null, null, true, true);
63+
this(asList(patterns), null, null, true, true, null);
64+
}
65+
66+
/**
67+
* Additional constructor with flags for using suffix pattern (.*) and
68+
* trailing slash matches.
69+
*
70+
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
71+
* @param urlPathHelper for determining the lookup path of a request
72+
* @param pathMatcher for path matching with patterns
73+
* @param useSuffixPatternMatch whether to enable matching by suffix (".*")
74+
* @param useTrailingSlashMatch whether to match irrespective of a trailing slash
75+
*/
76+
public PatternsRequestCondition(String[] patterns, UrlPathHelper urlPathHelper, PathMatcher pathMatcher,
77+
boolean useSuffixPatternMatch, boolean useTrailingSlashMatch) {
78+
79+
this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, null);
6280
}
6381

6482
/**
6583
* Creates a new instance with the given URL patterns.
6684
* Each pattern that is not empty and does not start with "/" is pre-pended with "/".
67-
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
85+
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
6886
* @param urlPathHelper a {@link UrlPathHelper} for determining the lookup path for a request
6987
* @param pathMatcher a {@link PathMatcher} for pattern path matching
7088
* @param useSuffixPatternMatch whether to enable matching by suffix (".*")
7189
* @param useTrailingSlashMatch whether to match irrespective of a trailing slash
90+
* @param fileExtensions a list of file extensions to consider for path matching
7291
*/
73-
public PatternsRequestCondition(String[] patterns,
74-
UrlPathHelper urlPathHelper,
75-
PathMatcher pathMatcher,
76-
boolean useSuffixPatternMatch,
77-
boolean useTrailingSlashMatch) {
78-
this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch);
92+
public PatternsRequestCondition(String[] patterns, UrlPathHelper urlPathHelper,
93+
PathMatcher pathMatcher, boolean useSuffixPatternMatch, boolean useTrailingSlashMatch,
94+
List<String> fileExtensions) {
95+
96+
this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, fileExtensions);
7997
}
8098

8199
/**
82100
* Private constructor accepting a collection of patterns.
101+
* @param fileExtensionResolver
83102
*/
84-
private PatternsRequestCondition(Collection<String> patterns,
85-
UrlPathHelper urlPathHelper,
86-
PathMatcher pathMatcher,
87-
boolean useSuffixPatternMatch,
88-
boolean useTrailingSlashMatch) {
103+
private PatternsRequestCondition(Collection<String> patterns, UrlPathHelper urlPathHelper,
104+
PathMatcher pathMatcher, boolean useSuffixPatternMatch, boolean useTrailingSlashMatch,
105+
List<String> fileExtensions) {
106+
89107
this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns));
90108
this.urlPathHelper = urlPathHelper != null ? urlPathHelper : new UrlPathHelper();
91109
this.pathMatcher = pathMatcher != null ? pathMatcher : new AntPathMatcher();
92110
this.useSuffixPatternMatch = useSuffixPatternMatch;
93111
this.useTrailingSlashMatch = useTrailingSlashMatch;
112+
if (fileExtensions != null) {
113+
for (String fileExtension : fileExtensions) {
114+
this.fileExtensions.add("." + fileExtension);
115+
}
116+
}
94117
}
95118

96119
private static List<String> asList(String... patterns) {
@@ -126,15 +149,15 @@ protected String getToStringInfix() {
126149
}
127150

128151
/**
129-
* Returns a new instance with URL patterns from the current instance ("this") and
130-
* the "other" instance as follows:
152+
* Returns a new instance with URL patterns from the current instance ("this") and
153+
* the "other" instance as follows:
131154
* <ul>
132-
* <li>If there are patterns in both instances, combine the patterns in "this" with
155+
* <li>If there are patterns in both instances, combine the patterns in "this" with
133156
* the patterns in "other" using {@link PathMatcher#combine(String, String)}.
134157
* <li>If only one instance has patterns, use them.
135158
* <li>If neither instance has patterns, use an empty String (i.e. "").
136159
* </ul>
137-
*/
160+
*/
138161
public PatternsRequestCondition combine(PatternsRequestCondition other) {
139162
Set<String> result = new LinkedHashSet<String>();
140163
if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) {
@@ -154,26 +177,26 @@ else if (!other.patterns.isEmpty()) {
154177
result.add("");
155178
}
156179
return new PatternsRequestCondition(result, this.urlPathHelper, this.pathMatcher, this.useSuffixPatternMatch,
157-
this.useTrailingSlashMatch);
180+
this.useTrailingSlashMatch, this.fileExtensions);
158181
}
159182

160183
/**
161-
* Checks if any of the patterns match the given request and returns an instance
162-
* that is guaranteed to contain matching patterns, sorted via
163-
* {@link PathMatcher#getPatternComparator(String)}.
164-
*
184+
* Checks if any of the patterns match the given request and returns an instance
185+
* that is guaranteed to contain matching patterns, sorted via
186+
* {@link PathMatcher#getPatternComparator(String)}.
187+
*
165188
* <p>A matching pattern is obtained by making checks in the following order:
166189
* <ul>
167190
* <li>Direct match
168191
* <li>Pattern match with ".*" appended if the pattern doesn't already contain a "."
169192
* <li>Pattern match
170193
* <li>Pattern match with "/" appended if the pattern doesn't already end in "/"
171194
* </ul>
172-
*
195+
*
173196
* @param request the current request
174-
*
175-
* @return the same instance if the condition contains no patterns;
176-
* or a new condition with sorted matching patterns;
197+
*
198+
* @return the same instance if the condition contains no patterns;
199+
* or a new condition with sorted matching patterns;
177200
* or {@code null} if no patterns match.
178201
*/
179202
public PatternsRequestCondition getMatchingCondition(HttpServletRequest request) {
@@ -189,19 +212,28 @@ public PatternsRequestCondition getMatchingCondition(HttpServletRequest request)
189212
}
190213
}
191214
Collections.sort(matches, this.pathMatcher.getPatternComparator(lookupPath));
192-
return matches.isEmpty() ? null :
215+
return matches.isEmpty() ? null :
193216
new PatternsRequestCondition(matches, this.urlPathHelper, this.pathMatcher, this.useSuffixPatternMatch,
194-
this.useTrailingSlashMatch);
217+
this.useTrailingSlashMatch, this.fileExtensions);
195218
}
196219

197220
private String getMatchingPattern(String pattern, String lookupPath) {
198221
if (pattern.equals(lookupPath)) {
199222
return pattern;
200223
}
201224
if (this.useSuffixPatternMatch) {
202-
boolean hasSuffix = pattern.indexOf('.') != -1;
203-
if (!hasSuffix && this.pathMatcher.match(pattern + ".*", lookupPath)) {
204-
return pattern + ".*";
225+
if (useSmartSuffixPatternMatch(pattern, lookupPath)) {
226+
for (String extension : this.fileExtensions) {
227+
if (this.pathMatcher.match(pattern + extension, lookupPath)) {
228+
return pattern + extension;
229+
}
230+
}
231+
}
232+
else {
233+
boolean hasSuffix = pattern.indexOf('.') != -1;
234+
if (!hasSuffix && this.pathMatcher.match(pattern + ".*", lookupPath)) {
235+
return pattern + ".*";
236+
}
205237
}
206238
}
207239
if (this.pathMatcher.match(pattern, lookupPath)) {
@@ -217,15 +249,23 @@ private String getMatchingPattern(String pattern, String lookupPath) {
217249
}
218250

219251
/**
220-
* Compare the two conditions based on the URL patterns they contain.
221-
* Patterns are compared one at a time, from top to bottom via
222-
* {@link PathMatcher#getPatternComparator(String)}. If all compared
223-
* patterns match equally, but one instance has more patterns, it is
252+
* Whether to match by known file extensions. Return "true" if file extensions
253+
* are configured, and the lookup path has a suffix.
254+
*/
255+
private boolean useSmartSuffixPatternMatch(String pattern, String lookupPath) {
256+
return (!this.fileExtensions.isEmpty() && lookupPath.indexOf('.') != -1) ;
257+
}
258+
259+
/**
260+
* Compare the two conditions based on the URL patterns they contain.
261+
* Patterns are compared one at a time, from top to bottom via
262+
* {@link PathMatcher#getPatternComparator(String)}. If all compared
263+
* patterns match equally, but one instance has more patterns, it is
224264
* considered a closer match.
225-
*
226-
* <p>It is assumed that both instances have been obtained via
227-
* {@link #getMatchingCondition(HttpServletRequest)} to ensure they
228-
* contain only patterns that match the request and are sorted with
265+
*
266+
* <p>It is assumed that both instances have been obtained via
267+
* {@link #getMatchingCondition(HttpServletRequest)} to ensure they
268+
* contain only patterns that match the request and are sorted with
229269
* the best matches on top.
230270
*/
231271
public int compareTo(PatternsRequestCondition other, HttpServletRequest request) {

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

1919
import java.lang.reflect.Method;
20+
import java.util.ArrayList;
21+
import java.util.List;
2022

2123
import org.springframework.core.annotation.AnnotationUtils;
2224
import org.springframework.stereotype.Controller;
25+
import org.springframework.util.Assert;
2326
import org.springframework.web.accept.ContentNegotiationManager;
24-
import org.springframework.web.accept.HeaderContentNegotiationStrategy;
2527
import org.springframework.web.bind.annotation.RequestMapping;
2628
import org.springframework.web.servlet.mvc.condition.AbstractRequestCondition;
2729
import org.springframework.web.servlet.mvc.condition.CompositeRequestCondition;
@@ -52,6 +54,8 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
5254

5355
private ContentNegotiationManager contentNegotiationManager = new ContentNegotiationManager();
5456

57+
private final List<String> contentNegotiationFileExtensions = new ArrayList<String>();
58+
5559
/**
5660
* Whether to use suffix pattern match (".*") when matching patterns to
5761
* requests. If enabled a method mapped to "/users" also matches to "/users.*".
@@ -75,7 +79,9 @@ public void setUseTrailingSlashMatch(boolean useTrailingSlashMatch) {
7579
* If not set, the default constructor is used.
7680
*/
7781
public void setContentNegotiationManager(ContentNegotiationManager contentNegotiationManager) {
82+
Assert.notNull(contentNegotiationManager);
7883
this.contentNegotiationManager = contentNegotiationManager;
84+
this.contentNegotiationFileExtensions.addAll(contentNegotiationManager.getAllFileExtensions());
7985
}
8086

8187
/**
@@ -95,7 +101,14 @@ public boolean useTrailingSlashMatch() {
95101
* Return the configured {@link ContentNegotiationManager}.
96102
*/
97103
public ContentNegotiationManager getContentNegotiationManager() {
98-
return contentNegotiationManager;
104+
return this.contentNegotiationManager;
105+
}
106+
107+
/**
108+
* Return the known file extensions for content negotiation.
109+
*/
110+
public List<String> getContentNegotiationFileExtensions() {
111+
return this.contentNegotiationFileExtensions;
99112
}
100113

101114
/**
@@ -173,8 +186,8 @@ protected RequestCondition<?> getCustomMethodCondition(Method method) {
173186
*/
174187
private RequestMappingInfo createRequestMappingInfo(RequestMapping annotation, RequestCondition<?> customCondition) {
175188
return new RequestMappingInfo(
176-
new PatternsRequestCondition(annotation.value(),
177-
getUrlPathHelper(), getPathMatcher(), this.useSuffixPatternMatch, this.useTrailingSlashMatch),
189+
new PatternsRequestCondition(annotation.value(), getUrlPathHelper(), getPathMatcher(),
190+
this.useSuffixPatternMatch, this.useTrailingSlashMatch, this.contentNegotiationFileExtensions),
178191
new RequestMethodsRequestCondition(annotation.method()),
179192
new ParamsRequestCondition(annotation.params()),
180193
new HeadersRequestCondition(annotation.headers()),

0 commit comments

Comments
 (0)