Avoid pre-computing base URI in PagedResourceAssemblerArgumentResolver.

We now avoid the pre-computation of the base URI in PagedResourceAssemblerArgumentResolver as the actual assembler will fall back to using the URI of the current request by default anyway. The latter makes sure that request parameters, that are contained in the original requests appear in links created. If a controller wants to deviate from that behavior, they can create a dedicated link themselves and hand that to the assembler explicitly.

Fixes GH-2173, GH-452.

Author: odrotbohm

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

@@ -26,7 +26,6 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.core.log.LogMessage;
 import org.springframework.data.domain.Pageable;
-import org.springframework.hateoas.Link;
 import org.springframework.hateoas.server.MethodLinkBuilderFactory;
 import org.springframework.hateoas.server.core.MethodParameters;
 import org.springframework.hateoas.server.mvc.WebMvcLinkBuilderFactory;
@@ -35,8 +34,6 @@
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.method.support.HandlerMethodArgumentResolver;
 import org.springframework.web.method.support.ModelAndViewContainer;
-import org.springframework.web.util.UriComponents;
-import org.springframework.web.util.UriComponentsBuilder;
 
 /**
  * {@link HandlerMethodArgumentResolver} to allow injection of {@link PagedResourcesAssembler} into Spring MVC
@@ -55,20 +52,30 @@ public class PagedResourcesAssemblerArgumentResolver implements HandlerMethodArg
 	private static final String PARAMETER_AMBIGUITY = "Discovered multiple parameters of type Pageable but no qualifier annotations to disambiguate!";
 
 	private final HateoasPageableHandlerMethodArgumentResolver resolver;
-	private final MethodLinkBuilderFactory<?> linkBuilderFactory;
 
 	/**
 	 * Creates a new {@link PagedResourcesAssemblerArgumentResolver} using the given
 	 * {@link PageableHandlerMethodArgumentResolver} and {@link MethodLinkBuilderFactory}.
 	 *
 	 * @param resolver can be {@literal null}.
 	 * @param linkBuilderFactory can be {@literal null}, will be defaulted to a {@link WebMvcLinkBuilderFactory}.
+	 * @deprecated since 2.5, 2.4.4, 2.3.7, to be removed in 3.0
+	 * @use {@link #PagedResourcesAssemblerArgumentResolver(HateoasPageableHandlerMethodArgumentResolver)} instead.
 	 */
+	@Deprecated
 	public PagedResourcesAssemblerArgumentResolver(HateoasPageableHandlerMethodArgumentResolver resolver,
 			@Nullable MethodLinkBuilderFactory<?> linkBuilderFactory) {
+		this(resolver);
+	}
 
+	/**
+	 * Creates a new {@link PagedResourcesAssemblerArgumentResolver} using the given
+	 * {@link PageableHandlerMethodArgumentResolver}.
+	 *
+	 * @param resolver can be {@literal null}.
+	 */
+	public PagedResourcesAssemblerArgumentResolver(HateoasPageableHandlerMethodArgumentResolver resolver) {
 		this.resolver = resolver;
-		this.linkBuilderFactory = linkBuilderFactory == null ? new WebMvcLinkBuilderFactory() : linkBuilderFactory;
 	}
 
 	/*
@@ -89,36 +96,12 @@ public boolean supportsParameter(MethodParameter parameter) {
 	public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewContainer mavContainer,
 			NativeWebRequest webRequest, @Nullable WebDataBinderFactory binderFactory) {
 
-		UriComponents fromUriString = resolveBaseUri(parameter);
 		MethodParameter pageableParameter = findMatchingPageableParameter(parameter);
 
 		if (pageableParameter != null) {
-			return new MethodParameterAwarePagedResourcesAssembler<>(pageableParameter, resolver, fromUriString);
+			return new MethodParameterAwarePagedResourcesAssembler<>(pageableParameter, resolver, null);
 		} else {
-			return new PagedResourcesAssembler<>(resolver, fromUriString);
-		}
-	}
-
-	/**
-	 * Eagerly resolve a base URI for the given {@link MethodParameter} to be handed to the assembler.
-	 *
-	 * @param parameter must not be {@literal null}.
-	 * @return the {@link UriComponents} representing the base URI, or {@literal null} if it can't be resolved eagerly.
-	 */
-	@Nullable
-	private UriComponents resolveBaseUri(MethodParameter parameter) {
-
-		Method method = parameter.getMethod();
-
-		if (method == null) {
-			throw new IllegalArgumentException(String.format("Could not obtain method from parameter %s!", parameter));
-		}
-
-		try {
-			Link linkToMethod = linkBuilderFactory.linkTo(parameter.getDeclaringClass(), method).withSelfRel();
-			return UriComponentsBuilder.fromUriString(linkToMethod.getHref()).build();
-		} catch (IllegalArgumentException o_O) {
-			return null;
+			return new PagedResourcesAssembler<>(resolver, null);
 		}
 	}
 

src/main/java/org/springframework/data/web/config/HateoasAwareSpringDataWebConfiguration.java

@@ -99,7 +99,7 @@ public PagedResourcesAssembler<?> pagedResourcesAssembler() {
 
 	@Bean
 	public PagedResourcesAssemblerArgumentResolver pagedResourcesAssemblerArgumentResolver() {
-		return new PagedResourcesAssemblerArgumentResolver(pageableResolver.get(), null);
+		return new PagedResourcesAssemblerArgumentResolver(pageableResolver.get());
 	}
 
 	/*

src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java

@@ -18,17 +18,13 @@
 import static org.assertj.core.api.Assertions.*;
 
 import java.lang.reflect.Method;
-import java.util.Optional;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.core.MethodParameter;
 import org.springframework.data.domain.Pageable;
-import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.util.UriComponents;
 
 /**
  * Unit tests for {@link PagedResourcesAssemblerArgumentResolver}.
@@ -46,7 +42,7 @@ void setUp() {
 		WebTestUtils.initWebTest();
 
 		HateoasPageableHandlerMethodArgumentResolver hateoasPageableHandlerMethodArgumentResolver = new HateoasPageableHandlerMethodArgumentResolver();
-		this.resolver = new PagedResourcesAssemblerArgumentResolver(hateoasPageableHandlerMethodArgumentResolver, null);
+		this.resolver = new PagedResourcesAssemblerArgumentResolver(hateoasPageableHandlerMethodArgumentResolver);
 	}
 
 	@Test // DATACMNS-418
@@ -112,30 +108,6 @@ void doesNotFailForTemplatedMethodMapping() throws Exception {
 		assertThat(result).isNotNull();
 	}
 
-	@Test // DATACMNS-513
-	void detectsMappingOfInvokedSubType() throws Exception {
-
-		Method method = Controller.class.getMethod("methodWithMapping", PagedResourcesAssembler.class);
-
-		// Simulate HandlerMethod.HandlerMethodParameter.getDeclaringClass()
-		// as it's returning the invoked class as the declared one
-		MethodParameter methodParameter = new MethodParameter(method, 0) {
-			public java.lang.Class<?> getDeclaringClass() {
-				return SubController.class;
-			}
-		};
-
-		Object result = resolver.resolveArgument(methodParameter, null, null, null);
-
-		assertThat(result).isInstanceOf(PagedResourcesAssembler.class);
-
-		Optional<UriComponents> uriComponents = (Optional<UriComponents>) ReflectionTestUtils.getField(result, "baseUri");
-
-		assertThat(uriComponents).hasValueSatisfying(it -> {
-			assertThat(it.getPath()).isEqualTo("/foo/mapping");
-		});
-	}
-
 	private void assertSelectsParameter(Method method, int expectedIndex) {
 
 		MethodParameter parameter = new MethodParameter(method, 0);
@@ -190,9 +162,4 @@ void noMatchingQualifiers(@Qualifier("qualified") PagedResourcesAssembler<Object
 		@RequestMapping("/mapping")
 		Object methodWithMapping(PagedResourcesAssembler<Object> pageable);
 	}
-
-	@RequestMapping("/foo")
-	interface SubController extends Controller {
-
-	}
 }

src/test/java/org/springframework/data/web/PagedResourcesAssemblerUnitTests.java

@@ -38,6 +38,7 @@
 import org.springframework.hateoas.RepresentationModel;
 import org.springframework.hateoas.server.RepresentationModelAssembler;
 import org.springframework.hateoas.server.core.EmbeddedWrapper;
+import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.web.util.UriComponents;
 import org.springframework.web.util.UriComponentsBuilder;
 
@@ -257,6 +258,17 @@ void selfLinkContainsCoordinatesForCurrentPage() {
 		assertThat(resource.getRequiredLink(IanaLinkRelations.SELF).getHref()).endsWith("?page=0&size=1");
 	}
 
+	@Test // #2173
+	void keepsRequestParametersOfOriginalRequestUri() {
+
+		WebTestUtils.initWebTest(new MockHttpServletRequest("GET", "/sample?foo=bar"));
+
+		PagedModel<EntityModel<Person>> model = assembler.toModel(createPage(1));
+
+		assertThat(model.getRequiredLink(IanaLinkRelations.FIRST).getHref())
+				.isEqualTo("http://localhost/sample?foo=bar&page=0&size=1");
+	}
+
 	private static Page<Person> createPage(int index) {
 
 		Pageable request = PageRequest.of(index, 1);

src/test/java/org/springframework/data/web/WebTestUtils.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.web;
 
+import javax.servlet.http.HttpServletRequest;
+
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockServletContext;
 import org.springframework.web.context.WebApplicationContext;
@@ -33,8 +35,14 @@ public class WebTestUtils {
 	 * Initializes web tests. Will register a {@link MockHttpServletRequest} for the current thread.
 	 */
 	public static void initWebTest() {
+		initWebTest(new MockHttpServletRequest());
+	}
+
+	/**
+	 * Initializes web tests for the given {@link HttpServletRequest} which will registered for the current thread.
+	 */
+	public static void initWebTest(HttpServletRequest request) {
 
-		MockHttpServletRequest request = new MockHttpServletRequest();
 		ServletRequestAttributes requestAttributes = new ServletRequestAttributes(request);
 		RequestContextHolder.setRequestAttributes(requestAttributes);
 	}