From 94b94c58e42bcc6508424fef09a0211a639039a4 Mon Sep 17 00:00:00 2001 From: Patrick Wels Date: Mon, 17 Sep 2018 07:29:13 +0200 Subject: [PATCH 1/2] 559: fix multiple encodings of params --- .../org/springframework/hateoas/Link.java | 18 ++++++---- .../springframework/hateoas/UriTemplate.java | 22 +++++++++++++ .../mvc/ControllerLinkBuilderUnitTest.java | 33 +++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/Link.java b/src/main/java/org/springframework/hateoas/Link.java index e55313897..96cfc7c11 100755 --- a/src/main/java/org/springframework/hateoas/Link.java +++ b/src/main/java/org/springframework/hateoas/Link.java @@ -22,6 +22,7 @@ import lombok.experimental.Wither; import java.io.Serializable; +import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -49,7 +50,7 @@ */ @XmlType(name = "link", namespace = Link.ATOM_NAMESPACE) @JsonInclude(JsonInclude.Include.NON_NULL) -@JsonIgnoreProperties(value = "templated", ignoreUnknown = true) +@JsonIgnoreProperties(value = "templated", ignoreUnknown = true) @AllArgsConstructor(access = AccessLevel.PACKAGE) @Getter @EqualsAndHashCode(of = { "rel", "href", "hreflang", "media", "title", "deprecation", "affordances" }) @@ -131,7 +132,7 @@ protected Link() { /** * Returns safe copy of {@link Affordance}s. - * + * * @return */ public List getAffordances() { @@ -166,7 +167,7 @@ public Link andAffordance(Affordance affordance) { /** * Create new {@link Link} with additional {@link Affordance}s. - * + * * @param affordances must not be {@literal null}. * @return */ @@ -181,7 +182,7 @@ public Link andAffordances(List affordances) { /** * Creats a new {@link Link} with the given {@link Affordance}s. - * + * * @param affordances must not be {@literal null}. * @return */ @@ -227,7 +228,12 @@ public boolean isTemplated() { * @return */ public Link expand(Object... arguments) { - return new Link(getUriTemplate().expand(arguments).toString(), getRel()); + + URI uri = (arguments == null || arguments.length == 0) ? + getUriTemplate().checkParams(arguments) : + getUriTemplate().expand(arguments); + + return new Link(uri.toString(), getRel()); } /** @@ -242,7 +248,7 @@ public Link expand(Map arguments) { /** * Returns whether the current {@link Link} has the given link relation. - * + * * @param rel must not be {@literal null} or empty. * @return */ diff --git a/src/main/java/org/springframework/hateoas/UriTemplate.java b/src/main/java/org/springframework/hateoas/UriTemplate.java index e1a651019..c1d612543 100644 --- a/src/main/java/org/springframework/hateoas/UriTemplate.java +++ b/src/main/java/org/springframework/hateoas/UriTemplate.java @@ -203,6 +203,28 @@ public URI expand(Object... parameters) { return builder.build().toUri(); } + /** + * Expands the {@link UriTemplate} using the given parameters. The values will be applied in the order of the + * variables discovered. + * + * @param parameters + * @return + * @see #expand(Map) + */ + public URI checkParams(Object... parameters) { + org.springframework.web.util.UriTemplate baseTemplate = new org.springframework.web.util.UriTemplate(baseUri); + UriComponentsBuilder builder = UriComponentsBuilder.fromUri(baseTemplate.expand(parameters)); + Iterator iterator = Arrays.asList(parameters).iterator(); + + for (TemplateVariable variable : getOptionalVariables()) { + + Object value = iterator.hasNext() ? iterator.next() : null; + appendToBuilder(builder, variable, value); + } + + return URI.create(baseUri); + } + /** * Expands the {@link UriTemplate} using the given parameters. * diff --git a/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java index 25e316190..0fca9551e 100755 --- a/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java @@ -384,6 +384,27 @@ public void linksToMethodWithPathVariableContainingBlank() { assertThat(link.getHref()).endsWith("/something/with%20blank/foo"); } + @Test + public void uriComponentsToMethodWithPathVariableContainingBlankAnsOptionalQueryParams() { + + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("with blank", 0, 10)).withSelfRel(); + + UriComponents components = toComponents(link); + assertThat(components.toUriString(), containsString("/something/with%20blank/foo")); + assertThat(components.toUriString(), containsString("offset=0")); + assertThat(components.toUriString(), containsString("limit=10")); + } + + @Test + public void uriComponentsToMethodWithPathVariableContainingBlankAnsOptionalQueryParamNull() { + + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("with blank", null, 10)).withSelfRel(); + + UriComponents components = toComponents(link); + assertThat(components.toUriString(), containsString("/something/with%20blank/foo")); + assertThat(components.toUriString(), containsString("limit=10")); + } + /** * @see #192 */ @@ -590,6 +611,18 @@ public void alternativePathVariableParameter() { assertThat(link.getHref()).isEqualTo("http://localhost/something/bar/foo"); } + @Test + public void uriComponentsToMethodWithPathVariableAndRequestParamsContainingNull() + { + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("1", null, 10)).withSelfRel(); + + UriComponents components = toComponents(link); + assertThat(components.getPath(), is(equalTo("/something/1/foo"))); + + MultiValueMap queryParams = components.getQueryParams(); + assertThat(queryParams.get("limit"), contains("10")); + } + private static UriComponents toComponents(Link link) { return UriComponentsBuilder.fromUriString(link.expand().getHref()).build(); } From b64aa2e51b17d92c86eb1d77647fb0ee73d30794 Mon Sep 17 00:00:00 2001 From: Patrick Wels Date: Tue, 18 Sep 2018 09:47:35 +0200 Subject: [PATCH 2/2] 559: Optimized check method and tests --- .../org/springframework/hateoas/Link.java | 2 +- .../springframework/hateoas/UriTemplate.java | 18 ++--- .../mvc/ControllerLinkBuilderUnitTest.java | 78 +++++++++++-------- 3 files changed, 52 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/Link.java b/src/main/java/org/springframework/hateoas/Link.java index 96cfc7c11..ba150f52d 100755 --- a/src/main/java/org/springframework/hateoas/Link.java +++ b/src/main/java/org/springframework/hateoas/Link.java @@ -230,7 +230,7 @@ public boolean isTemplated() { public Link expand(Object... arguments) { URI uri = (arguments == null || arguments.length == 0) ? - getUriTemplate().checkParams(arguments) : + getUriTemplate().checkRequiredParams() : getUriTemplate().expand(arguments); return new Link(uri.toString(), getRel()); diff --git a/src/main/java/org/springframework/hateoas/UriTemplate.java b/src/main/java/org/springframework/hateoas/UriTemplate.java index c1d612543..614545175 100644 --- a/src/main/java/org/springframework/hateoas/UriTemplate.java +++ b/src/main/java/org/springframework/hateoas/UriTemplate.java @@ -204,22 +204,16 @@ public URI expand(Object... parameters) { } /** - * Expands the {@link UriTemplate} using the given parameters. The values will be applied in the order of the - * variables discovered. + * Checks the {@link UriTemplate} for unset but required params. * - * @param parameters * @return - * @see #expand(Map) */ - public URI checkParams(Object... parameters) { - org.springframework.web.util.UriTemplate baseTemplate = new org.springframework.web.util.UriTemplate(baseUri); - UriComponentsBuilder builder = UriComponentsBuilder.fromUri(baseTemplate.expand(parameters)); - Iterator iterator = Arrays.asList(parameters).iterator(); - + public URI checkRequiredParams() { for (TemplateVariable variable : getOptionalVariables()) { - - Object value = iterator.hasNext() ? iterator.next() : null; - appendToBuilder(builder, variable, value); + if (variable.isRequired()) { + throw new IllegalArgumentException( + String.format("Template variable %s is required but no value was given!", variable.getName())); + } } return URI.create(baseUri); diff --git a/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java index 0fca9551e..2d9f75b7e 100755 --- a/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java @@ -231,6 +231,42 @@ public void linksToMethodWithPathVariableAndRequestParams() { assertThat(queryParams.get("offset"), contains("10")); } + @Test + public void linksToMethodWithPathVariableAndRequestParamsWithNull() { + + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("1", null, 5)).withSelfRel(); + + UriComponents components = toComponents(link); + assertThat(components.getPath()).isEqualTo("/something/1/foo"); + + MultiValueMap queryParams = components.getQueryParams(); + assertThat(queryParams.get("limit"), contains("5")); + } + + @Test + public void linksToMethodWithPathVariableWithBlankAndRequestParamsWithNull() { + + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("with blank", null, 5)).withSelfRel(); + + UriComponents components = toComponents(link); + assertThat(components.getPath()).isEqualTo("/something/with%20blank/foo"); + + MultiValueMap queryParams = components.getQueryParams(); + assertThat(queryParams.get("limit"), contains("5")); + } + + @Test + public void linksToMethodWithRequestParam() { + + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("with blank", null, 5)).withSelfRel(); + + UriComponents components = toComponents(link); + assertThat(components.getPath()).isEqualTo("/something/with%20blank/foo"); + + MultiValueMap queryParams = components.getQueryParams(); + assertThat(queryParams.get("limit"), contains("5")); + } + /** * @see #26, #39 */ @@ -384,27 +420,6 @@ public void linksToMethodWithPathVariableContainingBlank() { assertThat(link.getHref()).endsWith("/something/with%20blank/foo"); } - @Test - public void uriComponentsToMethodWithPathVariableContainingBlankAnsOptionalQueryParams() { - - Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("with blank", 0, 10)).withSelfRel(); - - UriComponents components = toComponents(link); - assertThat(components.toUriString(), containsString("/something/with%20blank/foo")); - assertThat(components.toUriString(), containsString("offset=0")); - assertThat(components.toUriString(), containsString("limit=10")); - } - - @Test - public void uriComponentsToMethodWithPathVariableContainingBlankAnsOptionalQueryParamNull() { - - Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("with blank", null, 10)).withSelfRel(); - - UriComponents components = toComponents(link); - assertThat(components.toUriString(), containsString("/something/with%20blank/foo")); - assertThat(components.toUriString(), containsString("limit=10")); - } - /** * @see #192 */ @@ -495,6 +510,15 @@ public void encodesRequestParameterWithSpecialValue() { assertThat(link.getHref()).endsWith("/something/foo?id=Spring%23%0A"); } + @Test + public void encodesAndExpandsPathvariableWithSpecialValueAndRequestParameterWithNull() { + + Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("Spring#\n", null, 10)).withSelfRel().expand(); + + assertThat(link.getRel()).isEqualTo(Link.REL_SELF); + assertThat(link.getHref()).endsWith("/something/Spring%23%0A/foo?limit=10"); + } + /** * @see #169 */ @@ -611,18 +635,6 @@ public void alternativePathVariableParameter() { assertThat(link.getHref()).isEqualTo("http://localhost/something/bar/foo"); } - @Test - public void uriComponentsToMethodWithPathVariableAndRequestParamsContainingNull() - { - Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("1", null, 10)).withSelfRel(); - - UriComponents components = toComponents(link); - assertThat(components.getPath(), is(equalTo("/something/1/foo"))); - - MultiValueMap queryParams = components.getQueryParams(); - assertThat(queryParams.get("limit"), contains("10")); - } - private static UriComponents toComponents(Link link) { return UriComponentsBuilder.fromUriString(link.expand().getHref()).build(); }