From 6ebb05e60621fbb9a2d194afe935384fe6e84983 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 14 May 2015 14:06:35 +0200 Subject: [PATCH] DATACMNS-515 - Links for PagedResources should be canonical. PagedResourcesAssembler now creates the self, prev and next links for a PagedResource to be canonical and not contain any template parameters. This is to make sure a client can not actually tweak the links and follow a link that's not strictly conforming to semantics of the rel. --- .../data/web/PagedResourcesAssembler.java | 17 +++---- .../web/PagedResourcesAssemblerUnitTests.java | 46 +++++++------------ 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java index 2d3f1a090..8beda1b78 100644 --- a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java +++ b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java @@ -29,7 +29,6 @@ import org.springframework.hateoas.PagedResources.PageMetadata; import org.springframework.hateoas.Resource; import org.springframework.hateoas.ResourceAssembler; import org.springframework.hateoas.ResourceSupport; -import org.springframework.hateoas.TemplateVariables; import org.springframework.hateoas.UriTemplate; import org.springframework.util.Assert; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; @@ -118,7 +117,10 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa * * @param link must not be {@literal null}. * @return + * @deprecated this method will be removed in 1.11 as no Spring Data module actually calls it. Other clients calling + * it should stop doing so as {@link Link}s used for pagination shouldn't be templated in the first place. */ + @Deprecated public Link appendPaginationParameterTemplates(Link link) { Assert.notNull(link, "Link must not be null!"); @@ -138,9 +140,9 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa } UriTemplate base = new UriTemplate(link == null ? getDefaultUriString() : link.getHref()); + Link selfLink = createLink(base, null, Link.REL_SELF); - PagedResources pagedResources = new PagedResources(resources, asPageMetadata(page)); - pagedResources.add(createLink(base, null, Link.REL_SELF)); + PagedResources pagedResources = new PagedResources(resources, asPageMetadata(page), selfLink); if (page.hasNext()) { pagedResources.add(createLink(base, page.nextPageable(), Link.REL_NEXT)); @@ -165,8 +167,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa /** * Creates a {@link Link} with the given rel that will be based on the given {@link UriTemplate} but enriched with the - * values of the given {@link Pageable} (if not {@literal null}) and the missing parameters added as template - * variables. + * values of the given {@link Pageable} (if not {@literal null}). * * @param base must not be {@literal null}. * @param pageable can be {@literal null} @@ -178,11 +179,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa UriComponentsBuilder builder = fromUri(base.expand()); pageableResolver.enhance(builder, getMethodParameter(), pageable); - UriComponents components = builder.build(); - TemplateVariables variables = new TemplateVariables(base.getVariables()); - variables = variables.concat(pageableResolver.getPaginationTemplateVariables(getMethodParameter(), components)); - - return new Link(new UriTemplate(components.toString()).with(variables), rel); + return new Link(new UriTemplate(builder.build().toString()), rel); } /** diff --git a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java index 4c56397af..758d1a40a 100644 --- a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java +++ b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java @@ -129,15 +129,16 @@ public class PagedResourcesAssemblerUnitTests { /** * @see DATACMNS-418 + * @see DATACMNS-515 */ @Test - public void addsSelfLinkWithPaginationTemplateVariables() { + public void createsACanonicalLinkWithoutTemplateParameters() { PagedResourcesAssembler assembler = new PagedResourcesAssembler(resolver, null); PagedResources> resources = assembler.toResource(createPage(1)); Link selfLink = resources.getLink(Link.REL_SELF); - assertThat(selfLink.getHref(), endsWith("{?page,size,sort}")); + assertThat(selfLink.getHref(), endsWith("localhost")); } /** @@ -158,33 +159,6 @@ public class PagedResourcesAssemblerUnitTests { assertThat(content.iterator().next().name, is("Dave")); } - /** - * @see DATACMNS-418 - */ - @Test - public void appendsMissingTemplateParametersToLink() { - - PagedResourcesAssembler assembler = new PagedResourcesAssembler(resolver, null); - - Link link = new Link("/foo?page=0"); - assertThat(assembler.appendPaginationParameterTemplates(link), is(new Link("/foo?page=0{&size,sort}"))); - } - - /** - * @see DATACMNS-519 - */ - @Test - public void keepsExistingTemplateVariablesFromBaseLink() { - - PagedResourcesAssembler assembler = new PagedResourcesAssembler(resolver, null); - - Link link = new Link("/foo?page=0{&projection}"); - Link result = assembler.appendPaginationParameterTemplates(link); - - assertThat(result.getVariableNames(), hasSize(3)); - assertThat(result.getVariableNames(), hasItems("projection", "size", "sort")); - } - /** * @see DATAMCNS-563 */ @@ -204,6 +178,20 @@ public class PagedResourcesAssemblerUnitTests { assertThat(getQueryParameters(resource.getLink("next")), hasEntry("page", "3")); } + /** + * @see DATACMNS-515 + */ + @Test + public void generatedLinksShouldNotBeTemplated() { + + PagedResourcesAssembler assembler = new PagedResourcesAssembler(resolver, null); + PagedResources> resources = assembler.toResource(createPage(1)); + + assertThat(resources.getLink(Link.REL_SELF).getHref(), endsWith("localhost")); + assertThat(resources.getLink(Link.REL_NEXT).getHref(), endsWith("?page=2&size=1")); + assertThat(resources.getLink(Link.REL_PREVIOUS).getHref(), endsWith("?page=0&size=1")); + } + private static Page createPage(int index) { AbstractPageRequest request = new PageRequest(index, 1);