Clean up path variables after non match

This commit makes sure the nested path variables are only commited to
the attributes when all predicates match.

Issue: SPR-16692
This commit is contained in:
Arjen Poutsma 2018-05-03 09:41:14 +02:00
parent 03af55a363
commit 51325afbcc
2 changed files with 33 additions and 17 deletions

View File

@ -362,10 +362,7 @@ public abstract class RequestPredicates {
@Override @Override
public Optional<ServerRequest> nest(ServerRequest request) { public Optional<ServerRequest> nest(ServerRequest request) {
return Optional.ofNullable(this.pattern.matchStartOfPath(request.pathContainer())) return Optional.ofNullable(this.pattern.matchStartOfPath(request.pathContainer()))
.map(info -> { .map(info -> new SubPathServerRequestWrapper(request, info));
mergeTemplateVariables(request, info.getUriVariables());
return new SubPathServerRequestWrapper(request, info);
});
} }
private void mergeTemplateVariables(ServerRequest request, Map<String, String> variables) { private void mergeTemplateVariables(ServerRequest request, Map<String, String> variables) {
@ -475,10 +472,21 @@ public abstract class RequestPredicates {
private final PathContainer subPathContainer; private final PathContainer subPathContainer;
private final Map<String, String> pathVariables;
public SubPathServerRequestWrapper(ServerRequest request, PathPattern.PathRemainingMatchInfo info) { public SubPathServerRequestWrapper(ServerRequest request, PathPattern.PathRemainingMatchInfo info) {
this.request = request; this.request = request;
this.subPathContainer = new SubPathContainer(info.getPathRemaining()); this.subPathContainer = new SubPathContainer(info.getPathRemaining());
this.pathVariables = mergePathVariables(request, info.getUriVariables());
}
private static Map<String, String> mergePathVariables(ServerRequest request,
Map<String, String> pathVariables) {
Map<String, String> result = new LinkedHashMap<>(request.pathVariables());
result.putAll(pathVariables);
return Collections.unmodifiableMap(result);
} }
@Override @Override
@ -581,14 +589,9 @@ public abstract class RequestPredicates {
return this.request.queryParams(); return this.request.queryParams();
} }
@Override
public String pathVariable(String name) {
return this.request.pathVariable(name);
}
@Override @Override
public Map<String, String> pathVariables() { public Map<String, String> pathVariables() {
return this.request.pathVariables(); return this.pathVariables;
} }
@Override @Override

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2017 the original author or authors. * Copyright 2002-2018 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -17,7 +17,6 @@
package org.springframework.web.reactive.function.server; package org.springframework.web.reactive.function.server;
import org.junit.Test; import org.junit.Test;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
@ -46,7 +45,8 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati
.andRoute(GET("/baz"), nestedHandler::baz)) .andRoute(GET("/baz"), nestedHandler::baz))
.andNest(GET("/{foo}"), .andNest(GET("/{foo}"),
nest(GET("/{bar}"), nest(GET("/{bar}"),
route(GET("/{baz}"), nestedHandler::variables))); route(GET("/{baz}"), nestedHandler::variables)))
.andRoute(GET("/{qux}/quux"), nestedHandler::variables);
} }
@ -74,7 +74,18 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati
restTemplate.getForEntity("http://localhost:" + port + "/1/2/3", String.class); restTemplate.getForEntity("http://localhost:" + port + "/1/2/3", String.class);
assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals("1-2-3", result.getBody()); assertEquals("{foo=1, bar=2, baz=3}", result.getBody());
}
// SPR 16692
@Test
public void removeFailedPathVariables() throws Exception {
ResponseEntity<String> result =
restTemplate.getForEntity("http://localhost:" + port + "/qux/quux", String.class);
assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals("{qux=qux}", result.getBody());
} }
@ -89,11 +100,13 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati
} }
public Mono<ServerResponse> variables(ServerRequest request) { public Mono<ServerResponse> variables(ServerRequest request) {
Flux<String> responseBody = assertEquals(request.pathVariables(),
Flux.just(request.pathVariable("foo"), "-", request.pathVariable("bar"), "-", request.attributes().get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE));
request.pathVariable("baz"));
Mono<String> responseBody = Mono.just(request.pathVariables().toString());
return ServerResponse.ok().body(responseBody, String.class); return ServerResponse.ok().body(responseBody, String.class);
} }
} }
} }