From ae76d0840369fdd1eae1ab98a1478212bd96e0dc Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Sun, 18 Jun 2023 16:09:27 +0800 Subject: [PATCH] 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: GH-3094 Original pull request: GH-2865 --- ...PageableHandlerMethodArgumentResolver.java | 8 ++++- ...PageableHandlerMethodArgumentResolver.java | 13 ++++++-- ...andlerMethodArgumentResolverUnitTests.java | 33 +++++++++++++++++-- ...andlerMethodArgumentResolverUnitTests.java | 24 ++++++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java index b07733032..d9d0ecba5 100644 --- a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java @@ -33,6 +33,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; * @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 class PageableHandlerMethodArgumentResolver extends PageableHandlerMethod 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 --git a/src/main/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolver.java index fa800dbc0..9b8f710cc 100644 --- a/src/main/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolver.java @@ -33,6 +33,7 @@ import org.springframework.web.server.ServerWebExchange; * * @since 2.2 * @author Mark Paluch + * @author Yanming Zhou */ public class ReactivePageableHandlerMethodArgumentResolver extends PageableHandlerMethodArgumentResolverSupport implements SyncHandlerMethodArgumentResolver { @@ -75,9 +76,17 @@ public class ReactivePageableHandlerMethodArgumentResolver extends PageableHandl 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 --git a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java index a364b74d9..8577fed4b 100755 --- a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java @@ -24,6 +24,7 @@ import org.springframework.beans.factory.annotation.Qualifier; 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 @@ import org.springframework.web.context.request.ServletWebRequest; * @author Nick Williams * @author Vedran Pavic * @author Mark Paluch + * @author Yanming Zhou */ class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefaultUnitTests { @@ -159,7 +161,7 @@ class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefaultUnit } @Test // DATACMNS-477 - void returnsFallbackIfOnlyPageIsGiven() throws Exception { + void returnsFallbackIfOnlyPageIsGiven() { var resolver = getResolver(); resolver.setFallbackPageable(Pageable.unpaged()); @@ -171,8 +173,35 @@ class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefaultUnit .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 --git a/src/test/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolverUnitTests.java index ced6a7df8..595519c17 100755 --- a/src/test/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/ReactivePageableHandlerMethodArgumentResolverUnitTests.java @@ -25,6 +25,7 @@ import org.springframework.beans.factory.annotation.Qualifier; 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 @@ import org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentR * Unit tests for {@link ReactivePageableHandlerMethodArgumentResolver}. * * @author Mark Paluch + * @author Yanming Zhou */ class ReactivePageableHandlerMethodArgumentResolverUnitTests { @@ -150,6 +152,28 @@ class ReactivePageableHandlerMethodArgumentResolverUnitTests { 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() {