Browse Source

Switch to ContainerPath variants in HandlerMapping's

pull/1466/merge
Rossen Stoyanchev 8 years ago
parent
commit
15cf9c1d78
  1. 7
      spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java
  2. 9
      spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java
  3. 3
      spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java
  4. 5
      spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java
  5. 33
      spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java
  6. 11
      spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java
  7. 4
      spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java
  8. 5
      spring-webflux/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingTests.java
  9. 12
      spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java
  10. 15
      spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java

7
spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java

@ -262,6 +262,13 @@ public class PathPattern implements Comparable<PathPattern> { @@ -262,6 +262,13 @@ public class PathPattern implements Comparable<PathPattern> {
}
}
// TODO: implement extractPathWithinPattern natively for PathContainer
public PathContainer extractPathWithinPattern(PathContainer path) {
String result = extractPathWithinPattern(path.value());
return PathContainer.parse(result, StandardCharsets.UTF_8);
}
/**
* Given a full path, determine the pattern-mapped part. <p>For example: <ul>
* <li>'{@code /docs/cvs/commit.html}' and '{@code /docs/cvs/commit.html} -> ''</li>

9
spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java

@ -22,6 +22,7 @@ import java.util.Map; @@ -22,6 +22,7 @@ import java.util.Map;
import reactor.core.publisher.Mono;
import org.springframework.beans.BeansException;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.web.server.ServerWebExchange;
@ -80,7 +81,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { @@ -80,7 +81,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping {
@Override
public Mono<Object> getHandlerInternal(ServerWebExchange exchange) {
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication();
Object handler;
try {
handler = lookupHandler(lookupPath, exchange);
@ -110,14 +111,14 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { @@ -110,14 +111,14 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping {
* @see org.springframework.web.util.pattern.PathPattern
*/
@Nullable
protected Object lookupHandler(String lookupPath, ServerWebExchange exchange) throws Exception {
protected Object lookupHandler(PathContainer lookupPath, ServerWebExchange exchange) throws Exception {
if (this.patternRegistry != null) {
PathMatchResult<Object> bestMatch = this.patternRegistry.findFirstMatch(lookupPath);
if (bestMatch != null) {
if (logger.isDebugEnabled()) {
logger.debug("Matching patterns for request [" + lookupPath + "] are " + bestMatch);
}
String pathWithinMapping = bestMatch.getPattern().extractPathWithinPattern(lookupPath);
PathContainer pathWithinMapping = bestMatch.getPattern().extractPathWithinPattern(lookupPath);
Object handler = bestMatch.getHandler();
return handleMatch(handler, bestMatch.getPattern(), pathWithinMapping, exchange);
}
@ -127,7 +128,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { @@ -127,7 +128,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping {
return null;
}
private Object handleMatch(Object handler, PathPattern bestMatch, String pathWithinMapping,
private Object handleMatch(Object handler, PathPattern bestMatch, PathContainer pathWithinMapping,
ServerWebExchange exchange) throws Exception {
// Bean name or resolved handler?

3
spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java

@ -24,6 +24,7 @@ import java.util.SortedSet; @@ -24,6 +24,7 @@ import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.Collectors;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.lang.Nullable;
import org.springframework.util.StringUtils;
import org.springframework.web.util.pattern.PathPattern;
@ -121,7 +122,7 @@ public class PathPatternRegistry<T> { @@ -121,7 +122,7 @@ public class PathPatternRegistry<T> {
* @param lookupPath the URL lookup path to be matched against
*/
@Nullable
public PathMatchResult<T> findFirstMatch(String lookupPath) {
public PathMatchResult<T> findFirstMatch(PathContainer lookupPath) {
return this.patternsMap.entrySet().stream()
.filter(entry -> entry.getKey().matches(lookupPath))
.sorted(Comparator.comparing(Map.Entry::getKey))

5
spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java

@ -41,6 +41,7 @@ import org.springframework.http.HttpStatus; @@ -41,6 +41,7 @@ import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.MediaTypeFactory;
import org.springframework.http.codec.ResourceHttpMessageWriter;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
@ -311,8 +312,8 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { @@ -311,8 +312,8 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
protected Mono<Resource> getResource(ServerWebExchange exchange) {
String name = HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE;
String pathWithinHandler = exchange.getRequiredAttribute(name);
String path = processPath(pathWithinHandler);
PathContainer pathWithinHandler = exchange.getRequiredAttribute(name);
String path = processPath(pathWithinHandler.value());
if (!StringUtils.hasText(path) || isInvalidPath(path)) {
if (logger.isTraceEnabled()) {
logger.trace("Ignoring invalid resource path [" + path + "]");

33
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java

@ -255,15 +255,15 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap @@ -255,15 +255,15 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
*/
@Override
public Mono<HandlerMethod> getHandlerInternal(ServerWebExchange exchange) {
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
if (logger.isDebugEnabled()) {
logger.debug("Looking up handler method for path " + lookupPath);
logger.debug("Looking up handler method for path " +
exchange.getRequest().getPath().value());
}
this.mappingRegistry.acquireReadLock();
try {
HandlerMethod handlerMethod;
try {
handlerMethod = lookupHandlerMethod(lookupPath, exchange);
handlerMethod = lookupHandlerMethod(exchange);
}
catch (Exception ex) {
return Mono.error(ex);
@ -273,7 +273,8 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap @@ -273,7 +273,8 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
logger.debug("Returning handler method [" + handlerMethod + "]");
}
else {
logger.debug("Did not find handler method for [" + lookupPath + "]");
logger.debug("Did not find handler method for " +
"[" + exchange.getRequest().getPath().value() + "]");
}
}
if (handlerMethod != null) {
@ -289,14 +290,13 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap @@ -289,14 +290,13 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
/**
* Look up the best-matching handler method for the current request.
* If multiple matches are found, the best match is selected.
* @param lookupPath the lookup path within the current mapping
* @param exchange the current exchange
* @return the best-matching handler method, or {@code null} if no match
* @see #handleMatch(Object, HandlerMethod, String, ServerWebExchange)
* @see #handleNoMatch(Set, String, ServerWebExchange)
* @see #handleMatch(T, HandlerMethod, ServerWebExchange)
* @see #handleNoMatch(Set, ServerWebExchange)
*/
@Nullable
protected HandlerMethod lookupHandlerMethod(String lookupPath, ServerWebExchange exchange)
protected HandlerMethod lookupHandlerMethod(ServerWebExchange exchange)
throws Exception {
List<Match> matches = new ArrayList<>();
@ -307,7 +307,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap @@ -307,7 +307,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
Collections.sort(matches, comparator);
if (logger.isTraceEnabled()) {
logger.trace("Found " + matches.size() + " matching mapping(s) for [" +
lookupPath + "] : " + matches);
exchange.getRequest().getPath() + "] : " + matches);
}
Match bestMatch = matches.get(0);
if (matches.size() > 1) {
@ -319,14 +319,14 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap @@ -319,14 +319,14 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
Method m1 = bestMatch.handlerMethod.getMethod();
Method m2 = secondBestMatch.handlerMethod.getMethod();
throw new IllegalStateException("Ambiguous handler methods mapped for HTTP path '" +
lookupPath + "': {" + m1 + ", " + m2 + "}");
exchange.getRequest().getPath() + "': {" + m1 + ", " + m2 + "}");
}
}
handleMatch(bestMatch.mapping, bestMatch.handlerMethod, lookupPath, exchange);
handleMatch(bestMatch.mapping, bestMatch.handlerMethod, exchange);
return bestMatch.handlerMethod;
}
else {
return handleNoMatch(this.mappingRegistry.getMappings().keySet(), lookupPath, exchange);
return handleNoMatch(this.mappingRegistry.getMappings().keySet(), exchange);
}
}
@ -343,25 +343,20 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap @@ -343,25 +343,20 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
* Invoked when a matching mapping is found.
* @param mapping the matching mapping
* @param handlerMethod the matching method
* @param lookupPath the lookup path within the current mapping
* @param exchange the current exchange
*/
protected void handleMatch(T mapping, HandlerMethod handlerMethod, String lookupPath,
ServerWebExchange exchange) {
protected void handleMatch(T mapping, HandlerMethod handlerMethod, ServerWebExchange exchange) {
}
/**
* Invoked when no matching mapping is not found.
* @param mappings all registered mappings
* @param lookupPath the lookup path within the current mapping
* @param exchange the current exchange
* @return an alternative HandlerMethod or {@code null}
* @throws Exception provides details that can be translated into an error status code
*/
@Nullable
protected HandlerMethod handleNoMatch(Set<T> mappings, String lookupPath, ServerWebExchange exchange)
throws Exception {
protected HandlerMethod handleNoMatch(Set<T> mappings, ServerWebExchange exchange) throws Exception {
return null;
}

11
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java

@ -31,6 +31,7 @@ import org.springframework.http.HttpHeaders; @@ -31,6 +31,7 @@ import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.InvalidMediaTypeException;
import org.springframework.http.MediaType;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.util.MultiValueMap;
import org.springframework.web.method.HandlerMethod;
@ -92,10 +93,12 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe @@ -92,10 +93,12 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
* @see HandlerMapping#PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE
*/
@Override
protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod, String lookupPath,
protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod,
ServerWebExchange exchange) {
super.handleMatch(info, handlerMethod, lookupPath, exchange);
super.handleMatch(info, handlerMethod, exchange);
PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication();
PathPattern bestPattern;
Map<String, String> uriVariables;
@ -103,7 +106,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe @@ -103,7 +106,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
Set<PathPattern> patterns = info.getPatternsCondition().getPatterns();
if (patterns.isEmpty()) {
bestPattern = getPathPatternParser().parse(lookupPath);
bestPattern = getPathPatternParser().parse(lookupPath.value());
uriVariables = Collections.emptyMap();
matrixVariables = Collections.emptyMap();
}
@ -137,7 +140,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe @@ -137,7 +140,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
* method but not by query parameter conditions
*/
@Override
protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> infos, String lookupPath,
protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> infos,
ServerWebExchange exchange) throws Exception {
PartialMatchHelper helper = new PartialMatchHelper(infos, exchange);

4
spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java

@ -31,6 +31,7 @@ import org.springframework.cache.concurrent.ConcurrentMapCache; @@ -31,6 +31,7 @@ import org.springframework.cache.concurrent.ConcurrentMapCache;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.io.buffer.support.DataBufferTestUtils;
import org.springframework.http.CacheControl;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
import org.springframework.mock.http.server.reactive.test.MockServerWebExchange;
import org.springframework.web.reactive.HandlerMapping;
@ -81,7 +82,8 @@ public class ResourceHandlerRegistryTests { @@ -81,7 +82,8 @@ public class ResourceHandlerRegistryTests {
@Test
public void mapPathToLocation() throws Exception {
MockServerWebExchange exchange = MockServerHttpRequest.get("").toExchange();
exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "/testStylesheet.css");
exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE,
PathContainer.parse("/testStylesheet.css", StandardCharsets.UTF_8));
ResourceWebHandler handler = getHandler("/resources/**");
handler.handle(exchange).block(Duration.ofSeconds(5));

5
spring-webflux/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingTests.java

@ -24,6 +24,7 @@ import org.springframework.context.annotation.Bean; @@ -24,6 +24,7 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.support.ClassPathXmlApplicationContext;
import org.springframework.http.HttpMethod;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
import org.springframework.web.reactive.HandlerMapping;
import org.springframework.web.server.ServerWebExchange;
@ -103,7 +104,9 @@ public class SimpleUrlHandlerMappingTests { @@ -103,7 +104,9 @@ public class SimpleUrlHandlerMappingTests {
assertNotNull(actual);
assertSame(bean, actual);
//noinspection OptionalGetWithoutIsPresent
assertEquals(pathWithinMapping, exchange.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE));
PathContainer path = exchange.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE);
assertNotNull(path);
assertEquals(pathWithinMapping, path.value());
}
else {
assertNull(actual);

12
spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java

@ -44,6 +44,7 @@ import org.springframework.http.HttpHeaders; @@ -44,6 +44,7 @@ import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.server.reactive.PathContainer;
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
import org.springframework.mock.http.server.reactive.test.MockServerWebExchange;
@ -276,14 +277,6 @@ public class ResourceWebHandlerTests { @@ -276,14 +277,6 @@ public class ResourceWebHandlerTests {
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
}
@Test
public void ignoreInvalidEscapeSequence() throws Exception {
ServerWebExchange exchange = MockServerHttpRequest.get("").toExchange();
setPathWithinHandlerMapping(exchange, "/%foo%/bar.txt");
this.handler.handle(exchange).block(TIMEOUT);
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
}
@Test
public void processPath() throws Exception {
assertSame("/foo/bar", this.handler.processPath("/foo/bar"));
@ -559,7 +552,8 @@ public class ResourceWebHandlerTests { @@ -559,7 +552,8 @@ public class ResourceWebHandlerTests {
private void setPathWithinHandlerMapping(ServerWebExchange exchange, String path) {
exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, path);
exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE,
PathContainer.parse(path, StandardCharsets.UTF_8));
}
private long resourceLastModified(String resourceName) throws IOException {

15
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java

@ -213,10 +213,9 @@ public class RequestMappingInfoHandlerMappingTests { @@ -213,10 +213,9 @@ public class RequestMappingInfoHandlerMappingTests {
@SuppressWarnings("unchecked")
public void handleMatchUriTemplateVariables() throws Exception {
ServerWebExchange exchange = get("/1/2").toExchange();
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
RequestMappingInfo key = paths("/{path1}/{path2}").build();
this.handlerMapping.handleMatch(key, handlerMethod, lookupPath, exchange);
this.handlerMapping.handleMatch(key, handlerMethod, exchange);
String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
Map<String, String> uriVariables = (Map<String, String>) exchange.getAttributes().get(name);
@ -232,8 +231,7 @@ public class RequestMappingInfoHandlerMappingTests { @@ -232,8 +231,7 @@ public class RequestMappingInfoHandlerMappingTests {
URI url = URI.create("/group/a%2Fb");
ServerWebExchange exchange = method(HttpMethod.GET, url).toExchange();
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
this.handlerMapping.handleMatch(key, handlerMethod, lookupPath, exchange);
this.handlerMapping.handleMatch(key, handlerMethod, exchange);
String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE;
@SuppressWarnings("unchecked")
@ -248,8 +246,7 @@ public class RequestMappingInfoHandlerMappingTests { @@ -248,8 +246,7 @@ public class RequestMappingInfoHandlerMappingTests {
public void handleMatchBestMatchingPatternAttribute() throws Exception {
RequestMappingInfo key = paths("/{path1}/2", "/**").build();
ServerWebExchange exchange = get("/1/2").toExchange();
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
this.handlerMapping.handleMatch(key, handlerMethod, lookupPath, exchange);
this.handlerMapping.handleMatch(key, handlerMethod, exchange);
PathPattern bestMatch = (PathPattern) exchange.getAttributes().get(BEST_MATCHING_PATTERN_ATTRIBUTE);
assertEquals("/{path1}/2", bestMatch.getPatternString());
@ -262,8 +259,7 @@ public class RequestMappingInfoHandlerMappingTests { @@ -262,8 +259,7 @@ public class RequestMappingInfoHandlerMappingTests {
public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() throws Exception {
RequestMappingInfo key = paths().build();
ServerWebExchange exchange = get("/1/2").toExchange();
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
this.handlerMapping.handleMatch(key, handlerMethod, lookupPath, exchange);
this.handlerMapping.handleMatch(key, handlerMethod, exchange);
PathPattern bestMatch = (PathPattern) exchange.getAttributes().get(BEST_MATCHING_PATTERN_ATTRIBUTE);
assertEquals("/1/2", bestMatch.getPatternString());
@ -349,8 +345,7 @@ public class RequestMappingInfoHandlerMappingTests { @@ -349,8 +345,7 @@ public class RequestMappingInfoHandlerMappingTests {
private void handleMatch(ServerWebExchange exchange, String pattern) {
RequestMappingInfo info = paths(pattern).build();
String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value();
this.handlerMapping.handleMatch(info, handlerMethod, lookupPath, exchange);
this.handlerMapping.handleMatch(info, handlerMethod, exchange);
}
@SuppressWarnings("unchecked")

Loading…
Cancel
Save