From 32ebd45d85aa66c25dd16c7b4872246da5050bcb Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Mon, 6 Jun 2022 14:11:01 -0500 Subject: [PATCH 1/8] added startFromPageOne config value --- .../data/runtime/config/DataConfiguration.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java b/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java index 2202190711..6253064fdc 100644 --- a/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java +++ b/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java @@ -36,6 +36,7 @@ public class DataConfiguration implements DataSettings { public static class PageableConfiguration { public static final int DEFAULT_MAX_PAGE_SIZE = 100; public static final boolean DEFAULT_SORT_IGNORE_CASE = false; + public static final boolean DEFAULT_START_FROM_PAGE_ONE = false; public static final String DEFAULT_SORT_PARAMETER = "sort"; public static final String DEFAULT_SIZE_PARAMETER = "size"; public static final String DEFAULT_PAGE_PARAMETER = "page"; @@ -43,6 +44,7 @@ public static class PageableConfiguration { private int maxPageSize = DEFAULT_MAX_PAGE_SIZE; private Integer defaultPageSize = null; // When is not specified the maxPageSize should be used private boolean sortIgnoreCase = DEFAULT_SORT_IGNORE_CASE; + private boolean startFromPageOne = DEFAULT_START_FROM_PAGE_ONE; private String sortParameterName = DEFAULT_SORT_PARAMETER; private String sizeParameterName = DEFAULT_SIZE_PARAMETER; private String pageParameterName = DEFAULT_PAGE_PARAMETER; @@ -78,6 +80,21 @@ public void setSortDelimiter(String sortDelimiter) { } } + /** + * @return Whether the first page is 1 + */ + public boolean isStartFromPageOne() { + return startFromPageOne; + } + + /** + * This parameter is used to shift the provided page number back one when this is true. + * @param startFromPageOne Whether the first page is 1 + */ + public void setStartFromPageOne(boolean startFromPageOne) { + this.startFromPageOne = startFromPageOne; + } + /** * @return The maximum page size when binding {@link io.micronaut.data.model.Pageable} objects. */ From 7ad81f9244dcbf09405474135a5be712d6130ca5 Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Mon, 6 Jun 2022 14:22:37 -0500 Subject: [PATCH 2/8] added page shift when config value is true --- .../data/runtime/http/PageableRequestArgumentBinder.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java b/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java index 8660e6c4af..c9ebf1ea5d 100644 --- a/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java +++ b/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java @@ -96,6 +96,10 @@ public BindingResult bind(ArgumentConversionContext context, sort = Sort.of(orders); } + if (configuration.isStartFromPageOne() && page > 0) { + page--; + } + if (size < 1) { if (page == 0 && configuredMaxSize < 1 && sort == null) { pageable = Pageable.UNPAGED; From 60054034cf5163a3dc748bf4728e62f51e193bcb Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Mon, 6 Jun 2022 14:27:02 -0500 Subject: [PATCH 3/8] added page shift for spring --- .../runtime/http/SpringPageableRequestArgumentBinder.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java b/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java index e4008c9a28..c4fe069860 100644 --- a/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java +++ b/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java @@ -100,6 +100,10 @@ public BindingResult bind(ArgumentConversionContext context, sort = Sort.unsorted(); } + if (configuration.isStartFromPageOne() && page > 0) { + page--; + } + if (size < 1) { if (page == 0 && configuredMaxSize < 1 && sort.isUnsorted()) { pageable = Pageable.unpaged(); From 20fe2f0d09d845eb0b265e40713e07fda7953e3b Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Tue, 7 Jun 2022 10:45:48 -0500 Subject: [PATCH 4/8] added unit test for configuration --- .../io/micronaut/data/runtime/http/PageableConfigSpec.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableConfigSpec.groovy b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableConfigSpec.groovy index 622632684a..255979c98a 100644 --- a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableConfigSpec.groovy +++ b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableConfigSpec.groovy @@ -28,7 +28,8 @@ class PageableConfigSpec extends Specification { 'micronaut.data.pageable.default-page-size': 10, 'micronaut.data.pageable.sort-parameter-name': 's', 'micronaut.data.pageable.page-parameter-name': 'index', - 'micronaut.data.pageable.size-parameter-name': 'max' + 'micronaut.data.pageable.size-parameter-name': 'max', + 'micronaut.data.pageable.start-from-page-one': true ) DataConfiguration.PageableConfiguration configuration = context.getBean(DataConfiguration.PageableConfiguration) @@ -38,6 +39,7 @@ class PageableConfigSpec extends Specification { configuration.sortParameterName == 's' configuration.pageParameterName == 'index' configuration.sizeParameterName == 'max' + configuration.startFromPageOne cleanup: context.close() From 15b9eae4d49e68a3fda0d11d12c31924f8675adf Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Tue, 7 Jun 2022 11:07:56 -0500 Subject: [PATCH 5/8] added test for page shifting when a config is set --- .../PageableRequestArgumentBinderSpec.groovy | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy index f78df6c814..82bede132e 100644 --- a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy +++ b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy @@ -72,10 +72,10 @@ class PageableRequestArgumentBinderSpec extends Specification { void 'test bind size #size and page #page with custom configuration'() { given: def configuration = new DataConfiguration.PageableConfiguration() - configuration.defaultPageSize=40 - configuration.maxPageSize=200 - configuration.sizeParameterName="perPage" - configuration.pageParameterName="myPage" + configuration.defaultPageSize = 40 + configuration.maxPageSize = 200 + configuration.sizeParameterName = "perPage" + configuration.pageParameterName = "myPage" PageableRequestArgumentBinder binder = new PageableRequestArgumentBinder(configuration) def get = HttpRequest.GET('/') get.parameters.add("perPage", size) @@ -94,4 +94,27 @@ class PageableRequestArgumentBinderSpec extends Specification { "-1" | "0" | 40 | 0 // negative => uses default != max "junk" | "0" | 40 | 0 // can't be parsed } + + @Unroll + void 'test page #page is or is not shifted with custom configuration for startFromPageOne #startFromPageOne'() { + given: + def configuration = new DataConfiguration.PageableConfiguration() + configuration.startFromPageOne = startFromPageOne + PageableRequestArgumentBinder binder = new PageableRequestArgumentBinder(configuration) + def get = HttpRequest.GET('/') + get.parameters.add("page", page) + Pageable p = binder.bind(ConversionContext.of(Pageable), get).get() + + expect: + p.number == pageNumber + + where: + page | pageNumber | startFromPageOne + "1" | 0 | true // first page is shifted to 0 + "5" | 4 | true // fifth page is shifted to 4 + "0" | 0 | true // 0 page is still 0 + "1" | 1 | false // 1 is 1 + "5" | 5 | false // 5 is 5 + "0" | 0 | false // 0 is 0 + } } From b2d373b932e8cdbcc9061bdf6d4c4b777b03218b Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Tue, 7 Jun 2022 11:08:18 -0500 Subject: [PATCH 6/8] removed redundant test cases --- .../data/runtime/http/PageableRequestArgumentBinderSpec.groovy | 3 --- 1 file changed, 3 deletions(-) diff --git a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy index 82bede132e..a5c0a80456 100644 --- a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy +++ b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy @@ -113,8 +113,5 @@ class PageableRequestArgumentBinderSpec extends Specification { "1" | 0 | true // first page is shifted to 0 "5" | 4 | true // fifth page is shifted to 4 "0" | 0 | true // 0 page is still 0 - "1" | 1 | false // 1 is 1 - "5" | 5 | false // 5 is 5 - "0" | 0 | false // 0 is 0 } } From 7f7ae33e3276b403b48cdac9a43e071815465055 Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Tue, 7 Jun 2022 11:47:51 -0500 Subject: [PATCH 7/8] updated document comment --- .../io/micronaut/data/runtime/config/DataConfiguration.java | 6 ++++-- .../runtime/http/PageableRequestArgumentBinderSpec.groovy | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java b/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java index 6253064fdc..2b167ff755 100644 --- a/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java +++ b/data-runtime/src/main/java/io/micronaut/data/runtime/config/DataConfiguration.java @@ -81,7 +81,7 @@ public void setSortDelimiter(String sortDelimiter) { } /** - * @return Whether the first page is 1 + * @return Whether the first page parameter starts at one */ public boolean isStartFromPageOne() { return startFromPageOne; @@ -89,6 +89,8 @@ public boolean isStartFromPageOne() { /** * This parameter is used to shift the provided page number back one when this is true. + * So when the page parameter is entered as one, the first page (0) is provided. + * * @param startFromPageOne Whether the first page is 1 */ public void setStartFromPageOne(boolean startFromPageOne) { @@ -112,7 +114,7 @@ public void setMaxPageSize(int maxPageSize) { /** * @return the page size to use when binding {@link io.micronaut.data.model.Pageable} - * objects and no size parameter is used. By default is set to the same vale as {@link #maxPageSize} + * objects and no size parameter is used. By default is set to the same value as {@link #maxPageSize} */ public int getDefaultPageSize() { return defaultPageSize == null ? maxPageSize : defaultPageSize; diff --git a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy index a5c0a80456..e907a48ce3 100644 --- a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy +++ b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy @@ -110,8 +110,8 @@ class PageableRequestArgumentBinderSpec extends Specification { where: page | pageNumber | startFromPageOne - "1" | 0 | true // first page is shifted to 0 - "5" | 4 | true // fifth page is shifted to 4 - "0" | 0 | true // 0 page is still 0 + "1" | 0 | true // first page is shifted to 0 + "5" | 4 | true // fifth page is shifted to 4 + "0" | 0 | true // 0 page is still 0 } } From 52b0429511ad75aed31a27b3a58f86393b7772d8 Mon Sep 17 00:00:00 2001 From: alexsimpson440dev Date: Wed, 8 Jun 2022 15:01:17 -0500 Subject: [PATCH 8/8] added exception when page is less than 1 and supporting test --- .../http/PageableRequestArgumentBinder.java | 6 +++++- .../PageableRequestArgumentBinderSpec.groovy | 16 +++++++++++++++- .../SpringPageableRequestArgumentBinder.java | 5 ++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java b/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java index c9ebf1ea5d..10f7a0425c 100644 --- a/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java +++ b/data-runtime/src/main/java/io/micronaut/data/runtime/http/PageableRequestArgumentBinder.java @@ -26,6 +26,7 @@ import io.micronaut.http.bind.binders.RequestArgumentBinder; import io.micronaut.http.bind.binders.TypedRequestArgumentBinder; +import io.micronaut.http.exceptions.HttpStatusException; import jakarta.inject.Singleton; import java.util.List; import java.util.Locale; @@ -96,7 +97,10 @@ public BindingResult bind(ArgumentConversionContext context, sort = Sort.of(orders); } - if (configuration.isStartFromPageOne() && page > 0) { + if (configuration.isStartFromPageOne()) { + if (page < 1) { + throw new IllegalArgumentException(String.format("%s parameter starts at 1", configuration.getPageParameterName())); + } page--; } diff --git a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy index e907a48ce3..401bde1068 100644 --- a/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy +++ b/data-runtime/src/test/groovy/io/micronaut/data/runtime/http/PageableRequestArgumentBinderSpec.groovy @@ -112,6 +112,20 @@ class PageableRequestArgumentBinderSpec extends Specification { page | pageNumber | startFromPageOne "1" | 0 | true // first page is shifted to 0 "5" | 4 | true // fifth page is shifted to 4 - "0" | 0 | true // 0 page is still 0 + } + + void 'test IllegalArgumentException is thrown when startFromPageOne is true and page provided is 0'() { + given: + def configuration = new DataConfiguration.PageableConfiguration() + configuration.startFromPageOne = true + PageableRequestArgumentBinder binder = new PageableRequestArgumentBinder(configuration) + def get = HttpRequest.GET('/') + get.parameters.add("page", "0") + + when: + binder.bind(ConversionContext.of(Pageable), get).get() + + then: + thrown(IllegalArgumentException) } } diff --git a/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java b/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java index c4fe069860..6cfde2478a 100644 --- a/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java +++ b/data-spring/src/main/java/io/micronaut/data/spring/runtime/http/SpringPageableRequestArgumentBinder.java @@ -100,7 +100,10 @@ public BindingResult bind(ArgumentConversionContext context, sort = Sort.unsorted(); } - if (configuration.isStartFromPageOne() && page > 0) { + if (configuration.isStartFromPageOne()) { + if (page < 1) { + throw new IllegalArgumentException(String.format("%s parameter starts at 1", configuration.getPageParameterName())); + } page--; }