From 382002e85530b700a7efc95d4fcc66820b59fed3 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 30 May 2022 18:09:35 +0200 Subject: [PATCH] =?UTF-8?q?#1795=20-=20Fix=20handling=20of=20{*=E2=80=A6}?= =?UTF-8?q?=20pattern=20variables=20in=20URI=20mappings.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../hateoas/server/core/WebHandler.java | 213 ++++++++++++++++-- .../server/mvc/WebMvcLinkBuilderUnitTest.java | 30 +++ 2 files changed, 230 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java index 2de64943..dd14d117 100644 --- a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java +++ b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java @@ -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 values = new HashMap<>(); - List variableNames = template.getVariableNames(); - Iterator names = variableNames.iterator(); + UriComponentsBuilder builder = finisher.apply(mapping); + Iterator variablesIterator = mappingVariables.iterator(); Iterator 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 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 optionalEmptyParameters = new ArrayList<>(); + if (mappingVariable.isCapturing()) { + + List 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 { + + 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 { + + private final List 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 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; + } + } } diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java index 6b1a4169..121178b7 100644 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java @@ -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 bindsCatchAllPathVariableCorrectly() { + + Stream> 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; + } + } }