From 46d7e47239acf3553c3d588ef854c4b37c364d10 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 18 Apr 2017 09:38:37 +0200 Subject: [PATCH] DATACMNS-1042 - PagedResourceAssembler now keeps pagination information for self link. We now hand the Pageable contained in the Page to the code that constructs the self link. Added toEmptyResource(Page, Class) to force clients to resolve an optional base link before calling the method. Removed deprecated method to append pagination template parameters. --- .../data/web/PagedResourcesAssembler.java | 75 ++++++++++--------- ...cesAssemblerArgumentResolverUnitTests.java | 8 +- .../web/PagedResourcesAssemblerUnitTests.java | 22 ++++-- 3 files changed, 59 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java index 25c4642fe..3ac0b833b 100644 --- a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java +++ b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java @@ -20,6 +20,7 @@ import static org.springframework.web.util.UriComponentsBuilder.*; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import org.springframework.core.MethodParameter; import org.springframework.data.domain.Page; @@ -49,7 +50,7 @@ import org.springframework.web.util.UriComponentsBuilder; public class PagedResourcesAssembler implements ResourceAssembler, PagedResources>> { private final HateoasPageableHandlerMethodArgumentResolver pageableResolver; - private final UriComponents baseUri; + private final Optional baseUri; private final EmbeddedWrappers wrappers = new EmbeddedWrappers(false); private boolean forceFirstAndLastRels = false; @@ -65,7 +66,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa public PagedResourcesAssembler(HateoasPageableHandlerMethodArgumentResolver resolver, UriComponents baseUri) { this.pageableResolver = resolver == null ? new HateoasPageableHandlerMethodArgumentResolver() : resolver; - this.baseUri = baseUri; + this.baseUri = Optional.ofNullable(baseUri); } /** @@ -87,7 +88,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa */ @Override public PagedResources> toResource(Page entity) { - return toResource(entity, new SimplePagedResourceAssembler<>()); + return toResource(entity, it -> new Resource<>(it)); } /** @@ -100,7 +101,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa * @return */ public PagedResources> toResource(Page page, Link selfLink) { - return toResource(page, new SimplePagedResourceAssembler<>(), selfLink); + return toResource(page, it -> new Resource<>(it), selfLink); } /** @@ -112,7 +113,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa * @return */ public PagedResources toResource(Page page, ResourceAssembler assembler) { - return createResource(page, assembler, null); + return createResource(page, assembler, Optional.empty()); } /** @@ -129,7 +130,8 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa Link link) { Assert.notNull(link, "Link must not be null!"); - return createResource(page, assembler, link); + + return createResource(page, assembler, Optional.of(link)); } /** @@ -137,15 +139,32 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa * * @param page must not be {@literal null}, content must be empty. * @param type must not be {@literal null}. - * @param link can be {@literal null}. + * @return + * @since 2.0 + */ + public PagedResources toEmptyResource(Page page, Class type) { + return toEmptyResource(page, type, Optional.empty()); + } + + /** + * Creates a {@link PagedResources} with an empt collection {@link EmbeddedWrapper} for the given domain type. + * + * @param page must not be {@literal null}, content must be empty. + * @param type must not be {@literal null}. + * @param link must not be {@literal null}. * @return * @since 1.11 */ public PagedResources toEmptyResource(Page page, Class type, Link link) { + return toEmptyResource(page, type, Optional.of(link)); + } + + private PagedResources toEmptyResource(Page page, Class type, Optional link) { Assert.notNull(page, "Page must must not be null!"); Assert.isTrue(!page.hasContent(), "Page must not have any content!"); Assert.notNull(type, "Type must not be null!"); + Assert.notNull(link, "Link must not be null!"); PageMetadata metadata = asPageMetadata(page); @@ -155,21 +174,6 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa return addPaginationLinks(new PagedResources<>(embedded, metadata), page, link); } - /** - * Adds the pagination parameters for all parameters not already present in the given {@link Link}. - * - * @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!"); - return createLink(new UriTemplate(link.getHref()), null, link.getRel()); - } - /** * Creates the {@link PagedResources} to be equipped with pagination links downstream. * @@ -189,7 +193,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa } private PagedResources createResource(Page page, - ResourceAssembler assembler, Link link) { + ResourceAssembler assembler, Optional link) { Assert.notNull(page, "Page must not be null!"); Assert.notNull(assembler, "ResourceAssembler must not be null!"); @@ -205,7 +209,7 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa return addPaginationLinks(resource, page, link); } - private PagedResources addPaginationLinks(PagedResources resources, Page page, Link link) { + private PagedResources addPaginationLinks(PagedResources resources, Page page, Optional link) { UriTemplate base = getUriTemplate(link); @@ -219,7 +223,10 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa resources.add(createLink(base, page.previousPageable(), Link.REL_PREVIOUS)); } - resources.add(createLink(base, null, Link.REL_SELF)); + Link selfLink = link.map(it -> it.withSelfRel())// + .orElseGet(() -> createLink(base, page.getPageable(), Link.REL_SELF)); + + resources.add(selfLink); if (page.hasNext()) { resources.add(createLink(base, page.nextPageable(), Link.REL_NEXT)); @@ -241,12 +248,8 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa * * @return */ - private UriTemplate getUriTemplate(Link baseLink) { - - String href = baseLink != null ? baseLink.getHref() - : baseUri == null ? ServletUriComponentsBuilder.fromCurrentRequest().build().toString() : baseUri.toString(); - - return new UriTemplate(href); + private UriTemplate getUriTemplate(Optional baseLink) { + return new UriTemplate(baseLink.map(Link::getHref).orElseGet(this::baseUriOrCurrentRequest)); } /** @@ -289,11 +292,11 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa return new PageMetadata(page.getSize(), page.getNumber(), page.getTotalElements(), page.getTotalPages()); } - private static class SimplePagedResourceAssembler implements ResourceAssembler> { + private String baseUriOrCurrentRequest() { + return baseUri.map(Object::toString).orElseGet(PagedResourcesAssembler::currentRequest); + } - @Override - public Resource toResource(T entity) { - return new Resource<>(entity); - } + private static String currentRequest() { + return ServletUriComponentsBuilder.fromCurrentRequest().build().toString(); } } diff --git a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java index b443d22a6..65d26c489 100755 --- a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java @@ -18,6 +18,7 @@ package org.springframework.data.web; import static org.assertj.core.api.Assertions.*; import java.lang.reflect.Method; +import java.util.Optional; import org.junit.Before; import org.junit.Rule; @@ -130,9 +131,12 @@ public class PagedResourcesAssemblerArgumentResolverUnitTests { Object result = resolver.resolveArgument(methodParameter, null, null, null); assertThat(result).isInstanceOf(PagedResourcesAssembler.class); - UriComponents uriComponents = (UriComponents) ReflectionTestUtils.getField(result, "baseUri"); - assertThat(uriComponents.getPath()).isEqualTo("/foo/mapping"); + Optional uriComponents = (Optional) ReflectionTestUtils.getField(result, "baseUri"); + + assertThat(uriComponents).hasValueSatisfying(it -> { + assertThat(it.getPath()).isEqualTo("/foo/mapping"); + }); } private void assertSelectsParameter(Method method, int expectedIndex) throws Exception { diff --git a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java index 9274ce45b..b91a1349c 100755 --- a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java +++ b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java @@ -18,7 +18,6 @@ package org.springframework.data.web; import static org.assertj.core.api.Assertions.*; import java.net.URI; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -111,7 +110,7 @@ public class PagedResourcesAssemblerUnitTests { PagedResources> resources = assembler.toResource(createPage(1), link); assertThat(resources.getLink(Link.REL_PREVIOUS).getHref()).startsWith(link.getHref()); - assertThat(resources.getLink(Link.REL_SELF)).isNotNull(); + assertThat(resources.getLink(Link.REL_SELF)).isEqualTo(link.withSelfRel()); assertThat(resources.getLink(Link.REL_NEXT).getHref()).startsWith(link.getHref()); } @@ -131,8 +130,7 @@ public class PagedResourcesAssemblerUnitTests { PagedResources> resources = assembler.toResource(createPage(1)); - Link selfLink = resources.getLink(Link.REL_SELF); - assertThat(selfLink.getHref()).endsWith("localhost"); + assertThat(resources.getLink(Link.REL_SELF).getHref()).doesNotContain("{").doesNotContain("}"); } @Test // DATACMNS-418 @@ -170,7 +168,7 @@ public class PagedResourcesAssemblerUnitTests { PagedResources> resources = assembler.toResource(createPage(1)); - assertThat(resources.getLink(Link.REL_SELF).getHref()).endsWith("localhost"); + assertThat(resources.getLink(Link.REL_SELF).getHref()).doesNotContain("{").doesNotContain("}"); assertThat(resources.getLink(Link.REL_NEXT).getHref()).endsWith("?page=2&size=1"); assertThat(resources.getLink(Link.REL_PREVIOUS).getHref()).endsWith("?page=0&size=1"); } @@ -178,7 +176,7 @@ public class PagedResourcesAssemblerUnitTests { @Test // DATACMNS-699 public void generatesEmptyPagedResourceWithEmbeddedWrapper() { - PagedResources result = assembler.toEmptyResource(EMPTY_PAGE, Person.class, null); + PagedResources result = assembler.toEmptyResource(EMPTY_PAGE, Person.class); Collection content = result.getContent(); assertThat(content).hasSize(1); @@ -190,12 +188,12 @@ public class PagedResourcesAssemblerUnitTests { @Test(expected = IllegalArgumentException.class) // DATACMNS-699 public void emptyPageCreatorRejectsPageWithContent() { - assembler.toEmptyResource(createPage(1), Person.class, null); + assembler.toEmptyResource(createPage(1), Person.class); } @Test(expected = IllegalArgumentException.class) // DATACMNS-699 public void emptyPageCreatorRejectsNullType() { - assembler.toEmptyResource(EMPTY_PAGE, null, null); + assembler.toEmptyResource(EMPTY_PAGE, null); } @Test // DATACMNS-701 @@ -246,6 +244,14 @@ public class PagedResourcesAssemblerUnitTests { assertThat(assembler.toResource(EMPTY_PAGE)).isInstanceOf(CustomPagedResources.class); } + @Test // DATACMNS-1042 + public void selfLinkContainsCoordinatesForCurrentPage() { + + PagedResources> resource = assembler.toResource(createPage(0)); + + assertThat(resource.getLink(Link.REL_SELF).getHref()).endsWith("?page=0&size=1"); + } + private static Page createPage(int index) { Pageable request = PageRequest.of(index, 1);