Use Pageable.unpaged(sort) for sorted unpaged pageable.

The (Reactive)PageableHandlerMethodArgumentResolver now falls back to a unpaged Pageable instance with a resolved sort if the the resolved Pageable is unpaged.

Fixes: spring-projectsGH-3094
Original pull request: spring-projectsGH-2865

Author: quaff

Diff for: src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java

@@ -33,6 +33,7 @@
  * @author Nick Williams
  * @author Mark Paluch
  * @author Christoph Strobl
+ * @author Yanming Zhou
  * @since 1.6
  */
 public class PageableHandlerMethodArgumentResolver extends PageableHandlerMethodArgumentResolverSupport
@@ -83,7 +84,12 @@ public Pageable resolveArgument(MethodParameter methodParameter, @Nullable Model
 		Pageable pageable = getPageable(methodParameter, page, pageSize);
 
 		if (sort.isSorted()) {
-			return PageRequest.of(pageable.getPageNumber(), pageable.getPageSize(), sort);
+			if (pageable.isPaged()) {
+				pageable = PageRequest.of(pageable.getPageNumber(), pageable.getPageSize(), sort);
+			}
+			else {
+				pageable = Pageable.unpaged(sort);
+			}
 		}
 
 		return pageable;

Diff for: src/main/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolver.java

@@ -33,6 +33,7 @@
  *
  * @since 2.2
  * @author Mark Paluch
+ * @author Yanming Zhou
  */
 public class ReactivePageableHandlerMethodArgumentResolver extends PageableHandlerMethodArgumentResolverSupport
 		implements SyncHandlerMethodArgumentResolver {
@@ -75,9 +76,17 @@ public Pageable resolveArgumentValue(MethodParameter parameter, BindingContext b
 		String pageSize = queryParams.getFirst(getParameterNameToUse(getSizeParameterName(), parameter));
 
 		Sort sort = sortResolver.resolveArgumentValue(parameter, bindingContext, exchange);
-
 		Pageable pageable = getPageable(parameter, page, pageSize);
 
-		return sort.isSorted() ? PageRequest.of(pageable.getPageNumber(), pageable.getPageSize(), sort) : pageable;
+		if (sort.isSorted()) {
+			if (pageable.isPaged()) {
+				pageable = PageRequest.of(pageable.getPageNumber(), pageable.getPageSize(), sort);
+			}
+			else {
+				pageable = Pageable.unpaged(sort);
+			}
+		}
+
+		return pageable;
 	}
 }

Diff for: src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java

@@ -24,6 +24,7 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.web.SortDefault.SortDefaults;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -37,6 +38,7 @@
  * @author Nick Williams
  * @author Vedran Pavic
  * @author Mark Paluch
+ * @author Yanming Zhou
  */
 class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefaultUnitTests {
 
@@ -159,7 +161,7 @@ void returnsUnpagedIfFallbackIsUnpagedAndNoParametersGiven() throws Exception {
 	}
 
 	@Test // DATACMNS-477
-	void returnsFallbackIfOnlyPageIsGiven() throws Exception {
+	void returnsFallbackIfOnlyPageIsGiven() {
 
 		var resolver = getResolver();
 		resolver.setFallbackPageable(Pageable.unpaged());
@@ -171,8 +173,35 @@ void returnsFallbackIfOnlyPageIsGiven() throws Exception {
 				.isEqualTo(Pageable.unpaged());
 	}
 
+	@Test
+	void returnsSortedUnpagedIfOnlyPageAndSortIsGiven() {
+
+		var resolver = getResolver();
+		resolver.setFallbackPageable(Pageable.unpaged());
+
+		var request = new MockHttpServletRequest();
+		request.addParameter("page", "20");
+		request.addParameter("sort", "foo");
+
+		assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null))
+				.isEqualTo(Pageable.unpaged(Sort.by("foo")));
+	}
+
+	@Test
+	void returnsSortedUnpagedIfOnlySortIsGiven() {
+
+		var resolver = getResolver();
+		resolver.setFallbackPageable(Pageable.unpaged());
+
+		var request = new MockHttpServletRequest();
+		request.addParameter("sort", "foo");
+
+		assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null))
+				.isEqualTo(Pageable.unpaged(Sort.by("foo")));
+	}
+
 	@Test // DATACMNS-477
-	void returnsFallbackIfFallbackIsUnpagedAndOnlySizeIsGiven() throws Exception {
+	void returnsFallbackIfFallbackIsUnpagedAndOnlySizeIsGiven() {
 
 		var resolver = getResolver();
 		resolver.setFallbackPageable(Pageable.unpaged());

Diff for: src/test/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolverUnitTests.java

@@ -25,6 +25,7 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.web.SortDefault.SortDefaults;
 import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
@@ -35,6 +36,7 @@
  * Unit tests for {@link ReactivePageableHandlerMethodArgumentResolver}.
  *
  * @author Mark Paluch
+ * @author Yanming Zhou
  */
 class ReactivePageableHandlerMethodArgumentResolverUnitTests {
 
@@ -150,6 +152,28 @@ void returnsFallbackIfOnlyPageIsGiven() {
 		assertThat(resolve(resolver, request)).isEqualTo(Pageable.unpaged());
 	}
 
+	@Test
+	void returnsSortedUnpagedIfOnlyPageAndSortIsGiven() {
+
+		var resolver = getReactiveResolver();
+		resolver.setFallbackPageable(Pageable.unpaged());
+
+		var request = MockServerHttpRequest.get("foo?page=20&sort=test").build();
+
+		assertThat(resolve(resolver, request)).isEqualTo(Pageable.unpaged(Sort.by("test")));
+	}
+
+	@Test
+	void returnsSortedUnpagedIfOnlySortIsGiven() {
+
+		var resolver = getReactiveResolver();
+		resolver.setFallbackPageable(Pageable.unpaged());
+
+		var request = MockServerHttpRequest.get("foo?sort=test").build();
+
+		assertThat(resolve(resolver, request)).isEqualTo(Pageable.unpaged(Sort.by("test")));
+	}
+
 	@Test // DATACMNS-1211
 	void returnsFallbackIfFallbackIsUnpagedAndOnlySizeIsGiven() {