#1795 - Fix handling of {*…} pattern variables in URI mappings.
We now properly handle path segment capture variables in URI mappings. Values handed into the dummy method invocations are expanded properly and a given null results in the composite segment template variable {/…*} being rendered to advertise the ability to add further segments.
This commit is contained in:
@@ -27,6 +27,8 @@ import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.function.BiFunction;
|
||||
import java.util.function.Function;
|
||||
import java.util.function.Supplier;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import org.springframework.core.MethodParameter;
|
||||
@@ -103,39 +105,56 @@ public class WebHandler {
|
||||
|
||||
FormatterFactory factory = new FormatterFactory(conversionService);
|
||||
|
||||
UriComponentsBuilder builder = finisher.apply(mapping);
|
||||
UriTemplate template = UriTemplateFactory.templateFor(mapping == null ? "/" : mapping);
|
||||
MappingVariables mappingVariables = new MappingVariables(template);
|
||||
|
||||
if (mapping != null && mapping.contains("{*")) {
|
||||
finisher = new PathCapturingMappingPreparer(mappingVariables).andThen(finisher);
|
||||
}
|
||||
|
||||
Map<String, Object> values = new HashMap<>();
|
||||
|
||||
List<String> variableNames = template.getVariableNames();
|
||||
Iterator<String> names = variableNames.iterator();
|
||||
UriComponentsBuilder builder = finisher.apply(mapping);
|
||||
Iterator<MappingVariable> variablesIterator = mappingVariables.iterator();
|
||||
Iterator<Object> classMappingParameters = invocations.getObjectParameters();
|
||||
|
||||
while (classMappingParameters.hasNext()) {
|
||||
|
||||
String name = names.next();
|
||||
TemplateVariable variable = TemplateVariable.segment(name);
|
||||
MappingVariable name = variablesIterator.next();
|
||||
Object source = classMappingParameters.next();
|
||||
|
||||
values.put(name, variable.prepareAndEncode(
|
||||
values.put(name.getKey(), name.toSegment().prepareAndEncode(
|
||||
HandlerMethodParameter.prepareValue(source, factory, TypeDescriptor.forObject(source))));
|
||||
}
|
||||
|
||||
Method method = invocation.getMethod();
|
||||
HandlerMethodParameters parameters = HandlerMethodParameters.of(method);
|
||||
Object[] arguments = invocation.getArguments();
|
||||
List<String> optionalEmptyParameters = new ArrayList<>();
|
||||
|
||||
for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(PathVariable.class, arguments)) {
|
||||
|
||||
TemplateVariable variable = TemplateVariable.segment(parameter.getVariableName());
|
||||
MappingVariable mappingVariable = mappingVariables.getVariable(parameter.getVariableName());
|
||||
Object verifiedValue = parameter.getVerifiedValue(arguments);
|
||||
Object preparedValue = verifiedValue == null ? verifiedValue
|
||||
: parameter.prepareValue(verifiedValue, factory);
|
||||
|
||||
values.put(variable.getName(), variable.prepareAndEncode(preparedValue));
|
||||
}
|
||||
// Handling for special catch-all path segments syntax in mappings {*…}.
|
||||
TemplateVariable segment = mappingVariable.toSegment();
|
||||
String key = mappingVariable.getKey();
|
||||
|
||||
List<String> optionalEmptyParameters = new ArrayList<>();
|
||||
if (mappingVariable.isCapturing()) {
|
||||
|
||||
List<String> segments = Arrays.asList(((String) preparedValue).split("/"));
|
||||
Object value = segments.size() != 0 ? "/" + segment.composite().prepareAndEncode(segments) : "";
|
||||
|
||||
values.put(key, value);
|
||||
|
||||
} else {
|
||||
|
||||
values.put(key, segment.prepareAndEncode(preparedValue));
|
||||
}
|
||||
}
|
||||
|
||||
for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(RequestParam.class, arguments)) {
|
||||
|
||||
@@ -154,9 +173,9 @@ public class WebHandler {
|
||||
}
|
||||
}
|
||||
|
||||
for (String variable : variableNames) {
|
||||
if (!values.containsKey(variable)) {
|
||||
values.put(variable, SKIP_VALUE);
|
||||
for (MappingVariable variable : mappingVariables) {
|
||||
if (!values.containsKey(variable.getKey())) {
|
||||
values.put(variable.getKey(), variable.getAbsentValue());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -630,4 +649,172 @@ public class WebHandler {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
private static class PathCapturingMappingPreparer implements Function<String, String> {
|
||||
|
||||
private static final Pattern PATH_CAPTURE = Pattern.compile("\\/\\{\\*(\\w+)\\}");
|
||||
|
||||
private final MappingVariables variables;
|
||||
|
||||
public PathCapturingMappingPreparer(MappingVariables variables) {
|
||||
this.variables = variables;
|
||||
}
|
||||
|
||||
/*
|
||||
* (non-Javadoc)
|
||||
* @see java.util.function.Function#apply(java.lang.Object)
|
||||
*/
|
||||
@Nullable
|
||||
@Override
|
||||
public String apply(@Nullable String source) {
|
||||
|
||||
if (source == null) {
|
||||
return source;
|
||||
}
|
||||
|
||||
Matcher matcher = PATH_CAPTURE.matcher(source);
|
||||
|
||||
while (matcher.find()) {
|
||||
|
||||
MappingVariable variable = variables.getVariable(matcher.group(1));
|
||||
|
||||
source = source.replace(matcher.group(0), variable.getPlaceholder());
|
||||
}
|
||||
|
||||
return source;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* All {@link MappingVariable}s contained in a {@link UriTemplate}.
|
||||
*
|
||||
* @author Oliver Drotbohm
|
||||
*/
|
||||
static class MappingVariables implements Iterable<MappingVariable> {
|
||||
|
||||
private final List<MappingVariable> variables;
|
||||
|
||||
public MappingVariables(UriTemplate template) {
|
||||
this.variables = template.getVariableNames().stream().map(MappingVariable::of).collect(Collectors.toList());
|
||||
}
|
||||
|
||||
public boolean hasCapturingVariable() {
|
||||
return variables.stream().anyMatch(it -> it.isCapturing());
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the {@link MappingVariable} with the given name.
|
||||
*
|
||||
* @param name must not be {@literal null} or empty.
|
||||
* @return
|
||||
* @throws IllegalArgumentException if no {@link MappingVariable} with the given name can be found.
|
||||
*/
|
||||
public MappingVariable getVariable(String name) {
|
||||
|
||||
Assert.hasText(name, "Variable must not be null or empty!");
|
||||
|
||||
return variables.stream()
|
||||
.filter(it -> it.hasName(name))
|
||||
.findFirst()
|
||||
.orElseThrow(() -> new IllegalArgumentException("No variable named " + name + " found!"));
|
||||
}
|
||||
|
||||
/*
|
||||
* (non-Javadoc)
|
||||
* @see java.lang.Iterable#iterator()
|
||||
*/
|
||||
@Override
|
||||
public Iterator<MappingVariable> iterator() {
|
||||
return variables.iterator();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A variable present in a Spring MVC controller mapping. These variables follow slightly different semantics than URI
|
||||
* template variables. For example, a capturing pattern {@code {*…}} indicates all trailing path segments to be
|
||||
* mapped.
|
||||
*
|
||||
* @author Oliver Drotbohm
|
||||
*/
|
||||
static class MappingVariable {
|
||||
|
||||
private final String name;
|
||||
private final boolean composite;
|
||||
|
||||
/**
|
||||
* Creates a new {@link MappingVariable} from the original source name as used in the mapping.
|
||||
*
|
||||
* @param source must not be {@literal null} or empty.
|
||||
* @return
|
||||
*/
|
||||
public static MappingVariable of(String source) {
|
||||
|
||||
Assert.hasText(source, "Variable source must not be null or empty!");
|
||||
|
||||
return source.startsWith("*") //
|
||||
? new MappingVariable(source.substring(1), true) //
|
||||
: new MappingVariable(source, false);
|
||||
}
|
||||
|
||||
private MappingVariable(String name, boolean composite) {
|
||||
|
||||
this.name = name;
|
||||
this.composite = composite;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns whether the variable has the given name.
|
||||
*
|
||||
* @param candidate must not be {@literal null} or empty.
|
||||
* @return
|
||||
*/
|
||||
public boolean hasName(String candidate) {
|
||||
return name.equals(candidate);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns whether the variable is capturing one.
|
||||
*
|
||||
* @return
|
||||
*/
|
||||
public boolean isCapturing() {
|
||||
return composite;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the key to be used for variable expansion.
|
||||
*
|
||||
* @return will never be {@literal null}.
|
||||
*/
|
||||
public String getKey() {
|
||||
return composite ? "__composite-" + name + "__" : name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the placeholder to be used when preparing the original mapping for capturing variables.
|
||||
*
|
||||
* @return will never be {@literal null}.
|
||||
*/
|
||||
public String getPlaceholder() {
|
||||
return "{" + getKey() + "}";
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a segment {@link TemplateVariable} for the current variable.
|
||||
*
|
||||
* @return will never be {@literal null}.
|
||||
*/
|
||||
public TemplateVariable toSegment() {
|
||||
return TemplateVariable.segment(name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the value to be used for expansion if the original value for it was absent.
|
||||
*
|
||||
* @return
|
||||
*/
|
||||
public Object getAbsentValue() {
|
||||
return composite ? TemplateVariable.segment(name).composite().toString() : SKIP_VALUE;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,7 +33,10 @@ import java.util.Optional;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
|
||||
import org.junit.jupiter.api.DynamicTest;
|
||||
import org.junit.jupiter.api.Named;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.TestFactory;
|
||||
import org.springframework.core.convert.converter.Converter;
|
||||
import org.springframework.core.convert.support.ConfigurableConversionService;
|
||||
import org.springframework.format.annotation.DateTimeFormat;
|
||||
@@ -730,6 +733,24 @@ class WebMvcLinkBuilderUnitTest extends TestUtils {
|
||||
.endsWith(UriUtils.encode("I+will:be+double+encoded", Charset.defaultCharset()));
|
||||
}
|
||||
|
||||
@TestFactory // #1795
|
||||
Stream<DynamicTest> bindsCatchAllPathVariableCorrectly() {
|
||||
|
||||
Stream<Named<String[]>> tests = Stream.of(//
|
||||
Named.of("Appends single", new String[] { "second", "/second" }),
|
||||
Named.of("Appends multiple", new String[] { "second/second", "/second/second" }),
|
||||
Named.of("Appends empty", new String[] { "", "/" }),
|
||||
Named.of("Appends null", new String[] { null, "/first{/second*}" }));
|
||||
|
||||
return DynamicTest.stream(tests, it -> {
|
||||
|
||||
assertThat(
|
||||
linkTo(methodOn(ControllerWithPathVariableCatchAll.class).test("first", it[0])).withSelfRel().getHref())
|
||||
.endsWith(it[1]);
|
||||
});
|
||||
|
||||
}
|
||||
|
||||
private static UriComponents toComponents(Link link) {
|
||||
return UriComponentsBuilder.fromUriString(link.expand().getHref()).build();
|
||||
}
|
||||
@@ -933,4 +954,13 @@ class WebMvcLinkBuilderUnitTest extends TestUtils {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// #1795
|
||||
static class ControllerWithPathVariableCatchAll {
|
||||
|
||||
@RequestMapping("/{first}/{*second}")
|
||||
HttpEntity<?> test(@PathVariable String first, @PathVariable String second) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user