From 7b41f6ae14db0dfd97c7efa41dac6474384934ae Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 13 Nov 2023 15:01:14 +0100 Subject: [PATCH] #2036 - Template variables of types requiring order are now inserted at the right place. Support added for path segments, path style parameter and request parameters. --- .../hateoas/TemplateVariable.java | 4 +++ .../springframework/hateoas/UriTemplate.java | 29 +++++++++++++++++-- .../hateoas/TemplateVariablesUnitTest.java | 14 ++++----- .../hateoas/UriTemplateUnitTest.java | 15 ++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/TemplateVariable.java b/src/main/java/org/springframework/hateoas/TemplateVariable.java index e7a0576b..9c9eb3f9 100644 --- a/src/main/java/org/springframework/hateoas/TemplateVariable.java +++ b/src/main/java/org/springframework/hateoas/TemplateVariable.java @@ -554,6 +554,10 @@ public final class TemplateVariable implements Serializable, UriTemplate.Expanda return this.equals(type) || COMBINABLE_TYPES.contains(this) && COMBINABLE_TYPES.contains(type); } + int findIndexWithin(String template) { + return template.indexOf(key); + } + /** * Returns the {@link VariableType} for the given variable key. * diff --git a/src/main/java/org/springframework/hateoas/UriTemplate.java b/src/main/java/org/springframework/hateoas/UriTemplate.java index 3a5453e5..e1c1f759 100644 --- a/src/main/java/org/springframework/hateoas/UriTemplate.java +++ b/src/main/java/org/springframework/hateoas/UriTemplate.java @@ -15,6 +15,8 @@ */ package org.springframework.hateoas; +import static org.springframework.hateoas.TemplateVariable.VariableType.*; + import java.io.Serializable; import java.net.URI; import java.nio.charset.StandardCharsets; @@ -28,6 +30,7 @@ import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.springframework.hateoas.TemplateVariable.VariableType; import org.springframework.lang.Nullable; @@ -35,7 +38,6 @@ import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; -import org.springframework.web.util.UriBuilderFactory; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UriUtils; @@ -178,6 +180,7 @@ public class UriTemplate implements Iterable, Serializable { List result = new ArrayList<>(); for (TemplateVariable variable : variables) { + boolean isRequestParam = variable.isRequestParameterVariable(); boolean alreadyPresent = parameters.containsKey(variable.getName()); @@ -215,7 +218,7 @@ public class UriTemplate implements Iterable, Serializable { group = existing.merge(group); newOriginal = newOriginal.replace(existing.asString(), group.asString()); } else { - newOriginal = newOriginal.concat(group.asString()); + newOriginal = group.insertInto(newOriginal); } groups = groups.addOrAugment(group); @@ -470,6 +473,28 @@ public class UriTemplate implements Iterable, Serializable { return this.type.canBeCombinedWith(type); } + /** + * Inserts the current {@link ExpandGroup} into the given URI template. + * + * @param template must not be {@literal null} or empty. + * @return will never be {@literal null}. + */ + String insertInto(String template) { + + var followingTypes = switch (type) { + case PATH_SEGMENT -> Stream.of(PATH_STYLE_PARAMETER, REQUEST_PARAM, FRAGMENT); + case PATH_STYLE_PARAMETER -> Stream.of(REQUEST_PARAM, FRAGMENT); + case REQUEST_PARAM, REQUEST_PARAM_CONTINUED -> Stream.of(FRAGMENT); + default -> Stream. empty(); + }; + + return followingTypes.map(it -> it.findIndexWithin(template)) + .filter(it -> it != -1) + .findFirst() + .map(it -> template.substring(0, it) + toString() + template.substring(it)) + .orElseGet(() -> template.concat(toString())); + } + /* * (non-Javadoc) * @see org.springframework.hateoas.UriTemplate.Expandable#asString() diff --git a/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java b/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java index 6cb97748..00d74539 100755 --- a/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java +++ b/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java @@ -45,7 +45,7 @@ class TemplateVariablesUnitTest { @Test void rendersSingleVariableCorrectly() { - TemplateVariables variables = new TemplateVariables(new TemplateVariable("foo", SEGMENT)); + TemplateVariables variables = new TemplateVariables(new TemplateVariable("foo", PATH_SEGMENT)); assertThat(variables.toString()).isEqualTo("{/foo}"); } @@ -69,7 +69,7 @@ class TemplateVariablesUnitTest { @Test void combinesMultipleVariablesOfTheDifferentType() { - TemplateVariable first = new TemplateVariable("foo", SEGMENT); + TemplateVariable first = new TemplateVariable("foo", PATH_SEGMENT); TemplateVariable second = new TemplateVariable("bar", REQUEST_PARAM); TemplateVariables variables = new TemplateVariables(first, second); @@ -83,7 +83,7 @@ class TemplateVariablesUnitTest { @Test void concatsVariables() { - TemplateVariables variables = new TemplateVariables(new TemplateVariable("foo", SEGMENT)); + TemplateVariables variables = new TemplateVariables(new TemplateVariable("foo", PATH_SEGMENT)); variables = variables.concat(new TemplateVariable("bar", REQUEST_PARAM)); assertThat(variables.toString()).isEqualTo("{/foo}{?bar}"); @@ -179,7 +179,7 @@ class TemplateVariablesUnitTest { void variableRejectsEmptyName() { assertThatIllegalArgumentException().isThrownBy(() -> { - new TemplateVariable("", PATH_VARIABLE); + new TemplateVariable("", SIMPLE); }); } @@ -190,7 +190,7 @@ class TemplateVariablesUnitTest { void variableRejectsNullName() { assertThatIllegalArgumentException().isThrownBy(() -> { - new TemplateVariable(null, PATH_VARIABLE); + new TemplateVariable(null, SIMPLE); }); } @@ -212,7 +212,7 @@ class TemplateVariablesUnitTest { void variableRejectsNullDescription() { assertThatIllegalArgumentException().isThrownBy(() -> { - new TemplateVariable("foo", PATH_VARIABLE, null, Cardinality.SINGULAR); + new TemplateVariable("foo", SIMPLE, null, Cardinality.SINGULAR); }); } @@ -220,7 +220,7 @@ class TemplateVariablesUnitTest { void variableRejectsNullCardinality() { assertThatIllegalArgumentException().isThrownBy(() -> { - new TemplateVariable("foo", PATH_VARIABLE, "description", null); + new TemplateVariable("foo", SIMPLE, "description", null); }); } } diff --git a/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java b/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java index 67f5f533..c17d4b5e 100755 --- a/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java +++ b/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java @@ -387,6 +387,21 @@ class UriTemplateUnitTest { assertThat(UriTemplate.of("/path/{foo.bar}").getVariableNames()).contains("foo.bar"); } + @Test // #2036 + void addsPathSegmentAtTheRightPositionWithinTheUri() { + + var template = UriTemplate.of("/api?foo=bar#baz"); + + assertThat(template.with("p", VariableType.REQUEST_PARAM).toString()) + .isEqualTo("/api?foo=bar{&p}#baz"); + + assertThat(template.with("p", VariableType.PATH_SEGMENT).toString()) + .isEqualTo("/api{/p}?foo=bar#baz"); + + assertThat(template.with("p", VariableType.PATH_STYLE_PARAMETER).toString()) + .isEqualTo("/api{;p}?foo=bar#baz"); + } + private static void assertVariables(UriTemplate template, TemplateVariable... variables) { assertVariables(template, Arrays.asList(variables)); }