From e646fc9f16be790ba3900ec3fbedf5536f265a67 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 17 Jan 2014 12:45:10 +0100 Subject: [PATCH] DATACMNS-418 - Improvements to template variables in HandlerMethodArgumentResolvers. The Hateoas(Pageable|Sort)HandlerMethodArgumentResolver now receive a UriComponents instance to inspect for existing query parameters. They then refrain from adding template variables for already present parameters. Slightly modified the way the sort parameters is appended to the pagination properties (although semantically equivalent). --- ...PageableHandlerMethodArgumentResolver.java | 40 +++++++++++++++++-- ...eoasSortHandlerMethodArgumentResolver.java | 10 ++++- .../data/web/PagedResourcesAssembler.java | 5 ++- ...andlerMethodArgumentResolverUnitTests.java | 32 +++++++++++++-- ...andlerMethodArgumentResolverUnitTests.java | 5 ++- ...bleResourcesAssemblerIntegrationTests.java | 1 + 6 files changed, 81 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java index e107e2e00..7927a3082 100644 --- a/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java @@ -15,10 +15,17 @@ */ package org.springframework.data.web; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.springframework.core.MethodParameter; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.hateoas.mvc.UriComponentsContributor; +import org.springframework.util.MultiValueMap; +import org.springframework.util.StringUtils; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; /** @@ -83,13 +90,40 @@ public class HateoasPageableHandlerMethodArgumentResolver extends PageableHandle * @return * @since 1.7 */ - public String getPaginationTemplateVariables(MethodParameter parameter) { + public String getPaginationTemplateVariables(MethodParameter parameter, UriComponents template) { String pagePropertyName = getParameterNameToUse(getPageParameterName(), parameter); String sizePropertyName = getParameterNameToUse(getSizeParameterName(), parameter); - String sortTemplateVariables = sortResolver.getSortTemplateVariables(parameter); - return String.format("{?%s,%s}{&%s", pagePropertyName, sizePropertyName, sortTemplateVariables.substring(2)); + List names = new ArrayList(); + MultiValueMap queryParameters = template.getQueryParams(); + boolean existingFound = false; + + for (String propertyName : Arrays.asList(pagePropertyName, sizePropertyName)) { + + if (!queryParameters.containsKey(propertyName)) { + names.add(propertyName); + } else { + existingFound = true; + } + } + + String sortTemplateVariables = sortResolver.getSortTemplateVariables(parameter, template); + + if (StringUtils.hasText(sortTemplateVariables)) { + names.add(sortTemplateVariables.substring(2, sortTemplateVariables.length() - 1)); + } + + if (names.isEmpty()) { + return ""; + } + + StringBuilder builder = new StringBuilder(); + builder.append(existingFound ? "{&" : "{?"); + builder.append(StringUtils.collectionToCommaDelimitedString(names)); + builder.append("}"); + + return builder.toString(); } /* diff --git a/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java index f27939bd0..7b02b2960 100644 --- a/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java @@ -21,6 +21,8 @@ import org.springframework.core.MethodParameter; import org.springframework.data.domain.Sort; import org.springframework.hateoas.mvc.UriComponentsContributor; import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; /** @@ -42,8 +44,12 @@ public class HateoasSortHandlerMethodArgumentResolver extends SortHandlerMethodA * @return * @since 1.7 */ - public String getSortTemplateVariables(MethodParameter parameter) { - return String.format("{?%s}", getSortParameter(parameter)); + public String getSortTemplateVariables(MethodParameter parameter, UriComponents template) { + + String sortParameter = getSortParameter(parameter); + MultiValueMap queryParameters = template.getQueryParams(); + + return queryParameters.containsKey(sortParameter) ? "" : String.format("{?%s}", sortParameter); } /* diff --git a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java index f58629116..72d0cbad8 100644 --- a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java +++ b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java @@ -143,8 +143,9 @@ public class PagedResourcesAssembler implements ResourceAssembler, Pa foo(resources, page.previousPageable(), uri, Link.REL_PREVIOUS); } - resources.add(new LinkTemplate(uri + pageableResolver.getPaginationTemplateVariables(getMethodParameter()), - Link.REL_SELF)); + UriComponents uriComponents = UriComponentsBuilder.fromUriString(uri).build(); + resources.add(new LinkTemplate(uri + + pageableResolver.getPaginationTemplateVariables(getMethodParameter(), uriComponents), Link.REL_SELF)); return resources; } diff --git a/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java index e9d8695a5..65e21d4e8 100644 --- a/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java @@ -26,6 +26,7 @@ import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.hateoas.mvc.UriComponentsContributor; import org.springframework.util.MultiValueMap; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; /** @@ -80,13 +81,28 @@ public class HateoasPageableHandlerMethodArgumentResolverUnitTests extends * @see DATACMNS-418 */ @Test - public void returnCorrectTemplateVariables() { + public void appendsTemplateVariablesCorrectly() { + + assertTemplateEnrichment("/foo", "{?page,size,sort}"); + assertTemplateEnrichment("/foo?page=1", "{&size,sort}"); + assertTemplateEnrichment("/foo?page=1&size=10", "{&sort}"); + assertTemplateEnrichment("/foo?page=1&sort=foo,asc", "{&size}"); + assertTemplateEnrichment("/foo?page=1&size=10&sort=foo,asc", ""); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void returnsCustomizedTemplateVariables() { + + UriComponents uriComponents = UriComponentsBuilder.fromPath("/foo").build(); HateoasPageableHandlerMethodArgumentResolver resolver = getResolver(); - assertThat(resolver.getPaginationTemplateVariables(null), is("{?page,size}{&sort}")); - resolver.setPageParameterName("foo"); - assertThat(resolver.getPaginationTemplateVariables(null), is("{?foo,size}{&sort}")); + String variables = resolver.getPaginationTemplateVariables(null, uriComponents); + + assertThat(variables, is("{?foo,size,sort}")); } @Override @@ -106,4 +122,12 @@ public class HateoasPageableHandlerMethodArgumentResolverUnitTests extends assertThat(builder.build().toUriString(), endsWith(expected)); } + + private void assertTemplateEnrichment(String baseUri, String expected) { + + UriComponents uriComponents = UriComponentsBuilder.fromUriString(baseUri).build(); + + HateoasPageableHandlerMethodArgumentResolver resolver = getResolver(); + assertThat(resolver.getPaginationTemplateVariables(null, uriComponents), is(expected)); + } } diff --git a/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java index 99727e6d7..7b7821373 100644 --- a/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java @@ -24,6 +24,7 @@ import java.net.URI; import org.junit.Test; import org.springframework.core.MethodParameter; import org.springframework.data.domain.Sort; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; /** @@ -57,8 +58,10 @@ public class HateoasSortHandlerMethodArgumentResolverUnitTests extends SortHandl @Test public void returnCorrectTemplateVariables() { + UriComponents uriComponents = UriComponentsBuilder.fromPath("/").build(); + HateoasSortHandlerMethodArgumentResolver resolver = new HateoasSortHandlerMethodArgumentResolver(); - assertThat(resolver.getSortTemplateVariables(null), is("{?sort}")); + assertThat(resolver.getSortTemplateVariables(null, uriComponents), is("{?sort}")); } private void assertUriStringFor(Sort sort, String expected) throws Exception { diff --git a/src/test/java/org/springframework/data/web/config/PageableResourcesAssemblerIntegrationTests.java b/src/test/java/org/springframework/data/web/config/PageableResourcesAssemblerIntegrationTests.java index 466313b25..1f6083e36 100644 --- a/src/test/java/org/springframework/data/web/config/PageableResourcesAssemblerIntegrationTests.java +++ b/src/test/java/org/springframework/data/web/config/PageableResourcesAssemblerIntegrationTests.java @@ -76,6 +76,7 @@ public class PageableResourcesAssemblerIntegrationTests { assertThat(resources.getLink(Link.REL_PREVIOUS), is(notNullValue())); assertThat(resources.getLink(Link.REL_NEXT), is(notNullValue())); + assertThat(resources.getLink(Link.REL_SELF), is(notNullValue())); } @Controller