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).

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<String> names = new ArrayList<String>();
+		MultiValueMap<String, String> 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();
 	}
 
 	/*

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<String, String> queryParameters = template.getQueryParams();
+
+		return queryParameters.containsKey(sortParameter) ? "" : String.format("{?%s}", sortParameter);
 	}
 
 	/*

src/main/java/org/springframework/data/web/PagedResourcesAssembler.java

@@ -143,8 +143,9 @@ public class PagedResourcesAssembler<T> implements ResourceAssembler<Page<T>, 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;
 	}

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));
+	}
 }

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 {

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