Reduce API Surface in PathPatternRegistry

This commit reduces the exposition of `PathPattern` instances throughout
the `HandlerMapping` API and removes some methods from its public API.

Issue: SPR-14544
This commit is contained in:
Brian Clozel
2017-02-09 22:09:46 +01:00
parent 4999898617
commit 8d43f45515
13 changed files with 135 additions and 127 deletions

View File

@@ -34,7 +34,6 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse
import org.springframework.util.FileCopyUtils;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.adapter.DefaultServerWebExchange;
import org.springframework.web.util.patterns.PathPatternParser;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
@@ -58,8 +57,7 @@ public class AppCacheManifestTransformerTests {
ClassPathResource allowedLocation = new ClassPathResource("test/", getClass());
ResourceWebHandler resourceHandler = new ResourceWebHandler();
ResourceUrlProvider resourceUrlProvider = new ResourceUrlProvider();
PathPatternParser parser = new PathPatternParser();
resourceUrlProvider.setHandlerMap(Collections.singletonMap(parser.parse("/static/**"), resourceHandler));
resourceUrlProvider.setHandlerMap(Collections.singletonMap("/static/**", resourceHandler));
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));

View File

@@ -59,8 +59,7 @@ public class CssLinkResourceTransformerTests {
ResourceWebHandler resourceHandler = new ResourceWebHandler();
ResourceUrlProvider resourceUrlProvider = new ResourceUrlProvider();
PathPatternParser parser = new PathPatternParser();
resourceUrlProvider.setHandlerMap(Collections.singletonMap(parser.parse("/static/**"), resourceHandler));
resourceUrlProvider.setHandlerMap(Collections.singletonMap("/static/**", resourceHandler));
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));

View File

@@ -30,7 +30,6 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.adapter.DefaultServerWebExchange;
import org.springframework.web.util.patterns.PathPatternParser;
import static org.junit.Assert.assertEquals;
@@ -70,8 +69,7 @@ public class ResourceTransformerSupportTests {
handler.setLocations(Collections.singletonList(new ClassPathResource("test/", getClass())));
handler.setResourceResolvers(resolvers);
ResourceUrlProvider urlProvider = new ResourceUrlProvider();
PathPatternParser parser = new PathPatternParser();
urlProvider.setHandlerMap(Collections.singletonMap(parser.parse("/resources/**"), handler));
urlProvider.setHandlerMap(Collections.singletonMap("/resources/**", handler));
return urlProvider;
}

View File

@@ -36,9 +36,6 @@ import org.springframework.web.context.support.AnnotationConfigWebApplicationCon
import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.adapter.DefaultServerWebExchange;
import org.springframework.web.server.session.DefaultWebSessionManager;
import org.springframework.web.server.session.WebSessionManager;
import org.springframework.web.util.patterns.PathPattern;
import org.springframework.web.util.patterns.PathPatternParser;
import static org.junit.Assert.assertEquals;
@@ -57,7 +54,7 @@ public class ResourceUrlProviderTests {
private final ResourceWebHandler handler = new ResourceWebHandler();
private final Map<PathPattern, ResourceWebHandler> handlerMap = new HashMap<>();
private final Map<String, ResourceWebHandler> handlerMap = new HashMap<>();
private final ResourceUrlProvider urlProvider = new ResourceUrlProvider();
@@ -68,8 +65,7 @@ public class ResourceUrlProviderTests {
this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
this.handler.setLocations(locations);
this.handler.afterPropertiesSet();
PathPattern staticResources = new PathPatternParser().parse("/resources/**");
this.handlerMap.put(staticResources, this.handler);
this.handlerMap.put("/resources/**", this.handler);
this.urlProvider.setHandlerMap(this.handlerMap);
}
@@ -125,8 +121,7 @@ public class ResourceUrlProviderTests {
resolvers.add(new PathResourceResolver());
otherHandler.setResourceResolvers(resolvers);
PathPattern staticResources = new PathPatternParser().parse("/resources/*.css");
this.handlerMap.put(staticResources, otherHandler);
this.handlerMap.put("/resources/*.css", otherHandler);
this.urlProvider.setHandlerMap(this.handlerMap);
String url = this.urlProvider.getForLookupPath("/resources/foo.css").blockMillis(5000);

View File

@@ -18,7 +18,9 @@ package org.springframework.web.reactive.result.method;
import java.lang.reflect.Method;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.Comparator;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
@@ -39,6 +41,7 @@ import org.springframework.web.server.adapter.DefaultServerWebExchange;
import org.springframework.web.server.session.MockWebSessionManager;
import org.springframework.web.server.session.WebSessionManager;
import org.springframework.web.util.patterns.PathPattern;
import org.springframework.web.util.patterns.PathPatternComparator;
import org.springframework.web.util.patterns.PathPatternParser;
import org.springframework.web.util.patterns.PatternComparatorConsideringPath;
@@ -54,7 +57,7 @@ public class HandlerMethodMappingTests {
private PathPatternParser patternParser = new PathPatternParser();
private AbstractHandlerMethodMapping<PathPattern> mapping;
private AbstractHandlerMethodMapping<String> mapping;
private MyHandler handler;
@@ -74,14 +77,14 @@ public class HandlerMethodMappingTests {
@Test(expected = IllegalStateException.class)
public void registerDuplicates() {
this.mapping.registerMapping(this.patternParser.parse("/foo"), this.handler, this.method1);
this.mapping.registerMapping(this.patternParser.parse("/foo"), this.handler, this.method2);
this.mapping.registerMapping("/foo", this.handler, this.method1);
this.mapping.registerMapping("/foo", this.handler, this.method2);
}
@Test
public void directMatch() throws Exception {
String key = "foo";
this.mapping.registerMapping(this.patternParser.parse(key), this.handler, this.method1);
String key = "/foo";
this.mapping.registerMapping(key, this.handler, this.method1);
Mono<Object> result = this.mapping.getHandler(createExchange(HttpMethod.GET, key));
assertEquals(this.method1, ((HandlerMethod) result.block()).getMethod());
@@ -89,8 +92,8 @@ public class HandlerMethodMappingTests {
@Test
public void patternMatch() throws Exception {
this.mapping.registerMapping(this.patternParser.parse("/fo*"), this.handler, this.method1);
this.mapping.registerMapping(this.patternParser.parse("/f*"), this.handler, this.method2);
this.mapping.registerMapping("/fo*", this.handler, this.method1);
this.mapping.registerMapping("/f*", this.handler, this.method2);
Mono<Object> result = this.mapping.getHandler(createExchange(HttpMethod.GET, "/foo"));
assertEquals(this.method1, ((HandlerMethod) result.block()).getMethod());
@@ -98,8 +101,8 @@ public class HandlerMethodMappingTests {
@Test
public void ambiguousMatch() throws Exception {
this.mapping.registerMapping(this.patternParser.parse("/f?o"), this.handler, this.method1);
this.mapping.registerMapping(this.patternParser.parse("/fo?"), this.handler, this.method2);
this.mapping.registerMapping("/f?o", this.handler, this.method1);
this.mapping.registerMapping("/fo?", this.handler, this.method2);
Mono<Object> result = this.mapping.getHandler(createExchange(HttpMethod.GET, "/foo"));
StepVerifier.create(result).expectError(IllegalStateException.class).verify();
@@ -107,8 +110,8 @@ public class HandlerMethodMappingTests {
@Test
public void registerMapping() throws Exception {
PathPattern key1 = this.patternParser.parse("/foo");
PathPattern key2 = this.patternParser.parse("/foo*");
String key1 = "/foo";
String key2 = "/foo*";
this.mapping.registerMapping(key1, this.handler, this.method1);
this.mapping.registerMapping(key2, this.handler, this.method2);
@@ -118,8 +121,8 @@ public class HandlerMethodMappingTests {
@Test
public void registerMappingWithSameMethodAndTwoHandlerInstances() throws Exception {
PathPattern key1 = this.patternParser.parse("/foo");
PathPattern key2 = this.patternParser.parse("/bar");
String key1 = "/foo";
String key2 = "/bar";
MyHandler handler1 = new MyHandler();
MyHandler handler2 = new MyHandler();
this.mapping.registerMapping(key1, handler1, this.method1);
@@ -131,13 +134,13 @@ public class HandlerMethodMappingTests {
@Test
public void unregisterMapping() throws Exception {
String key = "foo";
this.mapping.registerMapping(this.patternParser.parse(key), this.handler, this.method1);
String key = "/foo";
this.mapping.registerMapping(key, this.handler, this.method1);
Mono<Object> result = this.mapping.getHandler(createExchange(HttpMethod.GET, key));
assertNotNull(result.block());
this.mapping.unregisterMapping(this.patternParser.parse(key));
this.mapping.unregisterMapping(key);
result = this.mapping.getHandler(createExchange(HttpMethod.GET, key));
assertNull(result.block());
@@ -152,7 +155,7 @@ public class HandlerMethodMappingTests {
}
private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping<PathPattern> {
private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping<String> {
private PathPatternParser patternParser = new PathPatternParser();
@@ -162,28 +165,34 @@ public class HandlerMethodMappingTests {
}
@Override
protected PathPattern getMappingForMethod(Method method, Class<?> handlerType) {
protected String getMappingForMethod(Method method, Class<?> handlerType) {
String methodName = method.getName();
return methodName.startsWith("handler") ? this.patternParser.parse(methodName) : null;
return methodName.startsWith("handler") ? methodName : null;
}
@Override
protected SortedSet<PathPattern> getMappingPathPatterns(PathPattern key) {
TreeSet<PathPattern> patterns = new TreeSet<>();
patterns.add(key);
return patterns;
protected Set<String> getMappingPathPatterns(String key) {
return Collections.singleton(key);
}
@Override
protected PathPattern getMatchingMapping(PathPattern pattern, ServerWebExchange exchange) {
protected String getMatchingMapping(String pattern, ServerWebExchange exchange) {
String lookupPath = exchange.getRequest().getURI().getPath();
return (pattern.matches(lookupPath) ? pattern : null);
PathPattern pathPattern = this.patternParser.parse(pattern);
return (pathPattern.matches(lookupPath) ? pattern : null);
}
@Override
protected Comparator<PathPattern> getMappingComparator(ServerWebExchange exchange) {
protected Comparator<String> getMappingComparator(ServerWebExchange exchange) {
String lookupPath = exchange.getRequest().getURI().getPath();
return new PatternComparatorConsideringPath(lookupPath);
PatternComparatorConsideringPath comparator = new PatternComparatorConsideringPath(lookupPath);
return new Comparator<String>() {
@Override
public int compare(String o1, String o2) {
return comparator.compare(patternParser.parse(o1), patternParser.parse(o2));
}
};
}
}

View File

@@ -24,9 +24,7 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.SortedSet;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import org.hamcrest.Matchers;
import org.junit.Before;
@@ -98,11 +96,10 @@ public class RequestMappingInfoHandlerMappingTests {
public void getMappingPathPatterns() throws Exception {
String[] patterns = {"/foo/*", "/foo", "/bar/*", "/bar"};
RequestMappingInfo info = paths(patterns).build();
Set<PathPattern> actual = this.handlerMapping.getMappingPathPatterns(info);
Set<String> actual = this.handlerMapping.getMappingPathPatterns(info);
assertThat(actual.stream().map(PathPattern::getPatternString).collect(Collectors.toList()),
Matchers.containsInAnyOrder(new String[]{"/foo/*", "/foo", "/bar/*", "/bar",
"/foo/*/", "/foo/", "/bar/*/", "/bar/"}));
assertThat(actual, Matchers.containsInAnyOrder("/foo/*", "/foo", "/bar/*", "/bar",
"/foo/*/", "/foo/", "/bar/*/", "/bar/"));
}
@Test
@@ -192,7 +189,7 @@ public class RequestMappingInfoHandlerMappingTests {
assertError(mono, UnsupportedMediaTypeStatusException.class,
ex -> assertEquals("Request failure [status: 415, " +
"reason: \"Invalid mime type \"bogus\": does not contain '/'\"]",
"reason: \"Invalid mime type \"bogus\": does not contain '/'\"]",
ex.getMessage()));
}
@@ -238,7 +235,8 @@ public class RequestMappingInfoHandlerMappingTests {
exchange.getAttributes().get(name));
}
@Test @SuppressWarnings("unchecked")
@Test
@SuppressWarnings("unchecked")
public void handleMatchUriTemplateVariables() throws Exception {
String lookupPath = "/1/2";
this.request = MockServerHttpRequest.get(lookupPath).build();
@@ -370,7 +368,7 @@ public class RequestMappingInfoHandlerMappingTests {
}
@SuppressWarnings("unchecked")
private <T> void assertError(Mono<Object> mono, final Class<T> exceptionClass, final Consumer<T> consumer) {
private <T> void assertError(Mono<Object> mono, final Class<T> exceptionClass, final Consumer<T> consumer) {
StepVerifier.create(mono)
.consumeErrorWith(error -> {
@@ -469,11 +467,11 @@ public class RequestMappingInfoHandlerMappingTests {
public void foo() {
}
@GetMapping(path = "/foo", params="p")
@GetMapping(path = "/foo", params = "p")
public void fooParam() {
}
@RequestMapping(path = "/ba*", method = { GET, HEAD })
@RequestMapping(path = "/ba*", method = {GET, HEAD})
public void bar() {
}
@@ -481,31 +479,31 @@ public class RequestMappingInfoHandlerMappingTests {
public void empty() {
}
@PutMapping(path = "/person/{id}", consumes="application/xml")
@PutMapping(path = "/person/{id}", consumes = "application/xml")
public void consumes(@RequestBody String text) {
}
@RequestMapping(path = "/persons", produces="application/xml")
@RequestMapping(path = "/persons", produces = "application/xml")
public String produces() {
return "";
}
@RequestMapping(path = "/params", params="foo=bar")
@RequestMapping(path = "/params", params = "foo=bar")
public String param() {
return "";
}
@RequestMapping(path = "/params", params="bar=baz")
@RequestMapping(path = "/params", params = "bar=baz")
public String param2() {
return "";
}
@RequestMapping(path = "/content", produces="application/xml")
@RequestMapping(path = "/content", produces = "application/xml")
public String xmlContent() {
return "";
}
@RequestMapping(path = "/content", produces="!application/xml")
@RequestMapping(path = "/content", produces = "!application/xml")
public String nonXmlContent() {
return "";
}