From 26ffc6fef1d0a12b5fcb9a2b6d34d15e67197cf9 Mon Sep 17 00:00:00 2001 From: Nathan Rauh Date: Tue, 9 Apr 2024 08:35:42 -0500 Subject: [PATCH] Issue #672 review comment to add builder method with all values specified --- .../java/jakarta/data/page/CursoredPage.java | 13 ++-- .../java/jakarta/data/page/PageRequest.java | 59 +++++++++++++++---- .../java/jakarta/data/page/Pagination.java | 10 ---- .../data/page/impl/CursoredPageRecord.java | 26 ++++---- .../jakarta/data/page/impl/PageRecord.java | 14 ++--- .../data/page/PageRequestCursorTest.java | 41 ++++++------- .../data/standalone/entity/EntityTests.java | 13 ++-- 7 files changed, 98 insertions(+), 78 deletions(-) diff --git a/api/src/main/java/jakarta/data/page/CursoredPage.java b/api/src/main/java/jakarta/data/page/CursoredPage.java index f6b1cfbb6..6bc610003 100644 --- a/api/src/main/java/jakarta/data/page/CursoredPage.java +++ b/api/src/main/java/jakarta/data/page/CursoredPage.java @@ -18,6 +18,7 @@ package jakarta.data.page; import jakarta.data.repository.OrderBy; +import jakarta.data.Order; import jakarta.data.Sort; import java.util.NoSuchElementException; @@ -78,12 +79,12 @@ * in an instance of {@link PageRequest.Cursor Cursor} and identifying the last * result on the current page.

* - *

A {@link PageRequest} based on an explicit key may be constructed by - * calling {@link PageRequest#afterKey(Object...)}. The arguments supplied - * to this method must match the list of sorting criteria specified by - * {@link OrderBy} annotations of the repository method, {@link Sort} - * parameters of the page request, or {@code OrderBy} name pattern of - * the repository method. For example:

+ *

A {@link PageRequest} based on an explicit cursor may be constructed by + * calling {@link PageRequest#afterCursor(PageRequest.Cursor)}. The key component + * values of the cursor supplied to this method must match the list of sorting + * criteria specified by {@link OrderBy} annotations or {@code OrderBy} name + * pattern of the repository method and the {@link Sort} and {@link Order} + * parameters of the repository method. For example:

* * 
  * Employee emp = ...
diff --git a/api/src/main/java/jakarta/data/page/PageRequest.java b/api/src/main/java/jakarta/data/page/PageRequest.java
index d129636f1..40ec4c7c2 100644
--- a/api/src/main/java/jakarta/data/page/PageRequest.java
+++ b/api/src/main/java/jakarta/data/page/PageRequest.java
@@ -18,6 +18,8 @@
 package jakarta.data.page;
 
 import jakarta.data.Limit;
+import jakarta.data.Order;
+import jakarta.data.Sort;
 import jakarta.data.repository.OrderBy;
 
 import java.util.List;
@@ -78,6 +80,21 @@ static PageRequest ofPage(long pageNumber) {
         return new Pagination(pageNumber, 10, Mode.OFFSET, null, true);
     }
 
+    /**
+     * Creates a new page request without a cursor.
+     *
+     * @param pageNumber   The page number.
+     * @param maxPageSize  The number of query results in a full page.
+     * @param requestTotal Indicates whether to retrieve the
+     *                     {@linkplain Page#totalElements() total}
+     *                     number of elements available across all pages.
+     * @return a new instance of {@code PageRequest}. This method never returns {@code null}.
+     * @throws IllegalArgumentException when the page number is negative or zero.
+     */
+    static PageRequest ofPage(long pageNumber, int maxPageSize, boolean requestTotal) {
+        return new Pagination(pageNumber, maxPageSize, Mode.OFFSET, null, requestTotal);
+    }
+
     /**
      * Creates a new page request for requesting pages of the specified size,
      * starting with the first page number, which is 1.
@@ -94,30 +111,46 @@ static PageRequest ofSize(int maxPageSize) {
      * 
Requests {@linkplain CursoredPage cursor-based pagination} in the forward direction,
      * starting after the specified key.

      *
-     * @param key values forming the key, the order and number of which must match the
-     *        {@link OrderBy} annotations, {@link jakarta.data.Sort} parameters, or {@code OrderBy}
-     *        name pattern of the repository method to which this pagination will be
-     *        applied.
+     * @param cursor       cursor with key values, the order and number of
+     *                     which must match the {@link OrderBy} annotations or
+     *                     {@code OrderBy} name pattern and the {@link Order} and
+     *                     {@link Sort} parameters of the repository method to
+     *                     which this page request will be supplied.
+     * @param pageNumber   The page number.
+     * @param maxPageSize  The number of query results in a full page.
+     * @param requestTotal Indicates whether to retrieve the
+     *                     {@linkplain Page#totalElements() total}
+     *                     number of elements available across all pages.
      * @return a new instance of {@code PageRequest} with forward cursor-based pagination.
      *         This method never returns {@code null}.
-     * @throws IllegalArgumentException if no values are provided for the key.
+     * @throws IllegalArgumentException if the cursor is null or has no values.
      */
-    PageRequest afterKey(Object... key);
+    static PageRequest afterCursor(Cursor cursor, long pageNumber, int maxPageSize, boolean requestTotal) {
+        return new Pagination(pageNumber, maxPageSize, Mode.CURSOR_NEXT, cursor, requestTotal);
+    }
 
     /**
      * 
Requests {@linkplain CursoredPage cursor-based pagination} in the previous page
-     * direction relative to the specified key.

+     * direction relative to the specified cursor.

      *
-     * @param key values forming the key, the order and number of which must match the
-     *        {@link OrderBy} annotations, {@link jakarta.data.Sort} parameters, or {@code OrderBy}
-     *        name pattern of the repository method to which this pagination will be
-     *        applied.
+     * @param cursor       cursor with key values, the order and number of
+     *                     which must match the {@link OrderBy} annotations or
+     *                     {@code OrderBy} name pattern and the {@link Order} and
+     *                     {@link Sort} parameters of the repository method to
+     *                     which this page request will be supplied.
+     * @param pageNumber   The page number.
+     * @param maxPageSize  The number of query results in a full page.
+     * @param requestTotal Indicates whether to retrieve the
+     *                     {@linkplain Page#totalElements() total}
+     *                     number of elements available across all pages.
      * @return a new instance of {@code PageRequest} with cursor-based pagination
      *         in the previous page direction.
      *         This method never returns {@code null}.
-     * @throws IllegalArgumentException if no values are provided for the key.
+     * @throws IllegalArgumentException if the cursor is null or has no values.
      */
-    PageRequest beforeKey(Object... key);
+    static PageRequest beforeCursor(Cursor cursor, long pageNumber, int maxPageSize, boolean requestTotal) {
+        return new Pagination(pageNumber, maxPageSize, Mode.CURSOR_PREVIOUS, cursor, requestTotal);
+    }
 
     /**
      * 
Requests {@linkplain CursoredPage cursor-based pagination} in the forward direction,
diff --git a/api/src/main/java/jakarta/data/page/Pagination.java b/api/src/main/java/jakarta/data/page/Pagination.java
index bf5d62d5b..b80f4868d 100644
--- a/api/src/main/java/jakarta/data/page/Pagination.java
+++ b/api/src/main/java/jakarta/data/page/Pagination.java
@@ -46,16 +46,6 @@ public PageRequest withTotal() {
         return new Pagination(page, size, mode, type, true);
     }
 
-    @Override
-    public PageRequest afterKey(Object... key) {
-        return new Pagination(page, size, Mode.CURSOR_NEXT, new PageRequestCursor(key), requestTotal);
-    }
-
-    @Override
-    public PageRequest beforeKey(Object... key) {
-        return new Pagination(page, size, Mode.CURSOR_PREVIOUS, new PageRequestCursor(key), requestTotal);
-    }
-
     @Override
     public PageRequest afterCursor(Cursor cursor) {
         return new Pagination(page, size, Mode.CURSOR_NEXT, cursor, requestTotal);
diff --git a/api/src/main/java/jakarta/data/page/impl/CursoredPageRecord.java b/api/src/main/java/jakarta/data/page/impl/CursoredPageRecord.java
index 7b7a5feb4..728c14fbf 100644
--- a/api/src/main/java/jakarta/data/page/impl/CursoredPageRecord.java
+++ b/api/src/main/java/jakarta/data/page/impl/CursoredPageRecord.java
@@ -46,20 +46,6 @@ public record CursoredPageRecord
          PageRequest nextPageRequest, PageRequest previousPageRequest)
         implements CursoredPage {
 
-    private static final PageRequest after(PageRequest.Cursor newCursor, PageRequest copyFrom) {
-        PageRequest newRequest = PageRequest.ofPage(copyFrom.page() + 1)
-                .size(copyFrom.size())
-                .afterCursor(newCursor);
-        return copyFrom.requestTotal() ? newRequest.withTotal() : newRequest.withoutTotal();
-    }
-
-    private static final PageRequest before(PageRequest.Cursor newCursor, PageRequest copyFrom) {
-        PageRequest newRequest = PageRequest.ofPage(copyFrom.page() == 1 ? 1 : copyFrom.page() - 1)
-                .size(copyFrom.size())
-                .beforeCursor(newCursor);
-        return copyFrom.requestTotal() ? newRequest.withTotal() : newRequest.withoutTotal();
-    }
-
     /**
      * @param content The page content, that is, the query results, in order
      * @param cursors A list of {@link PageRequest.Cursor} instances for result,
@@ -75,8 +61,16 @@ private static final PageRequest before(PageRequest.Cursor newCursor, PageReques
             (List content, List cursors, long totalElements, PageRequest pageRequest,
             boolean firstPage, boolean lastPage) {
         this(content, cursors, totalElements, pageRequest,
-                lastPage ? null : after(cursors.get(cursors.size()-1), pageRequest),
-                firstPage ? null : before(cursors.get(0), pageRequest));
+                lastPage ? null : PageRequest.afterCursor(
+                        cursors.get(cursors.size() - 1),
+                        pageRequest.page() + 1,
+                        pageRequest.size(),
+                        pageRequest.requestTotal()),
+                firstPage ? null : PageRequest.beforeCursor(
+                        cursors.get(0),
+                        pageRequest.page() == 1 ? 1 : pageRequest.page() - 1,
+                        pageRequest.size(),
+                        pageRequest.requestTotal()));
     }
 
     @Override
diff --git a/api/src/main/java/jakarta/data/page/impl/PageRecord.java b/api/src/main/java/jakarta/data/page/impl/PageRecord.java
index ae31dc6c0..04671fed3 100644
--- a/api/src/main/java/jakarta/data/page/impl/PageRecord.java
+++ b/api/src/main/java/jakarta/data/page/impl/PageRecord.java
@@ -85,10 +85,9 @@ public PageRequest nextPageRequest() {
             throw new NoSuchElementException();
         }
 
-        PageRequest newRequest = PageRequest.ofPage(pageRequest.page() + 1)
-                .size(pageRequest.size());
-
-        return pageRequest.requestTotal() ? newRequest.withTotal() : newRequest.withoutTotal();
+        return PageRequest.ofPage(pageRequest.page() + 1,
+                                  pageRequest.size(),
+                                  pageRequest.requestTotal());
     }
 
     @Override
@@ -102,10 +101,9 @@ public PageRequest previousPageRequest() {
             throw new NoSuchElementException();
         }
 
-        PageRequest newRequest = PageRequest.ofPage(pageRequest.page() - 1)
-                .size(pageRequest.size());
-
-        return pageRequest.requestTotal() ? newRequest.withTotal() : newRequest.withoutTotal();
+        return PageRequest.ofPage(pageRequest.page() - 1,
+                pageRequest.size(),
+                pageRequest.requestTotal());
     }
 
     @Override
diff --git a/api/src/test/java/jakarta/data/page/PageRequestCursorTest.java b/api/src/test/java/jakarta/data/page/PageRequestCursorTest.java
index 4a56bc942..66b7ca04a 100644
--- a/api/src/test/java/jakarta/data/page/PageRequestCursorTest.java
+++ b/api/src/test/java/jakarta/data/page/PageRequestCursorTest.java
@@ -32,11 +32,12 @@ class PageRequestCursorTest {
     @Test
     @DisplayName("Should include key values in next PageRequest")
     void shouldCreatePageRequestAfterKeys() {
-        PageRequest pageRequest = PageRequest.ofSize(20).afterKey("First", 2L, 3);
+        PageRequest pageRequest = PageRequest.afterCursor(PageRequest.Cursor.forKey("First", 2L, 3), 1L, 20, false);
 
         assertSoftly(softly -> {
             softly.assertThat(pageRequest.size()).isEqualTo(20);
             softly.assertThat(pageRequest.page()).isEqualTo(1L);
+            softly.assertThat(pageRequest.requestTotal()).isEqualTo(false);
             softly.assertThat(pageRequest.mode()).isEqualTo(PageRequest.Mode.CURSOR_NEXT);
             softly.assertThat(pageRequest.cursor()).get().extracting(PageRequest.Cursor::size).isEqualTo(3);
             softly.assertThat(pageRequest.cursor()).get().extracting(c -> c.get(0)).isEqualTo("First");
@@ -64,11 +65,13 @@ void shouldCreatePageRequestAfterCursor() {
     @Test
     @DisplayName("Should include key values in previous PageRequest")
     void shouldCreatePageRequestBeforeKey() {
-        PageRequest pageRequest = PageRequest.ofPage(10).size(30).beforeKey(1991, "123-45-6789");
+        PageRequest.Cursor cursor = PageRequest.Cursor.forKey(1991, "123-45-6789");
+        PageRequest pageRequest = PageRequest.beforeCursor(cursor, 10L, 30, false);
 
         assertSoftly(softly -> {
             softly.assertThat(pageRequest.size()).isEqualTo(30);
             softly.assertThat(pageRequest.page()).isEqualTo(10L);
+            softly.assertThat(pageRequest.requestTotal()).isEqualTo(false);            
             softly.assertThat(pageRequest.mode()).isEqualTo(PageRequest.Mode.CURSOR_PREVIOUS);
             softly.assertThat(pageRequest.cursor()).get().extracting(PageRequest.Cursor::size).isEqualTo(2);
             softly.assertThat(pageRequest.cursor()).get().extracting(c -> c.get(0)).isEqualTo(1991);
@@ -98,10 +101,10 @@ void shouldCreatePageRequestBeforeKeysetCursor() {
     @Test
     @DisplayName("Should be usable in a hashing structure")
     void shouldHash() {
-        PageRequest pageRequest1 = PageRequest.ofSize(15).afterKey(1, '1', "1");
-        PageRequest pageRequest2A = PageRequest.ofSize(15).afterKey(2, '2', "2");
-        PageRequest pageRequest2B = PageRequest.ofSize(15).beforeKey(2, '2', "2");
-        PageRequest pageRequest2C = PageRequest.ofSize(15).beforeKey(2, '2', "2");
+        PageRequest pageRequest1 = PageRequest.ofSize(15).afterCursor(PageRequest.Cursor.forKey(1, '1', "1"));
+        PageRequest pageRequest2A = PageRequest.afterCursor(PageRequest.Cursor.forKey(2, '2', "2"), 1L, 15, true);
+        PageRequest pageRequest2B = PageRequest.beforeCursor(PageRequest.Cursor.forKey(2, '2', "2"), 1L, 15, true);
+        PageRequest pageRequest2C = PageRequest.ofSize(15).beforeCursor(PageRequest.Cursor.forKey(2, '2', "2"));
         Map map = new HashMap<>();
 
         assertSoftly(softly -> {
@@ -121,8 +124,8 @@ void shouldHash() {
     @Test
     @DisplayName("Should be displayable as String with toString")
     void shouldPageRequestDisplayAsString() {
-        var afterKeySet = PageRequest.ofSize(200).afterKey("value1", 1);
-        var beforeKeySet = PageRequest.ofSize(100).beforeKey("Item1", 3456);
+        var afterKeySet = PageRequest.ofSize(200).afterCursor(PageRequest.Cursor.forKey("value1", 1));
+        var beforeKeySet = PageRequest.ofSize(100).beforeCursor(PageRequest.Cursor.forKey("Item1", 3456));
 
         assertSoftly(softly -> {
 
@@ -138,11 +141,11 @@ void shouldPageRequestDisplayAsString() {
     @Test
     @DisplayName("Should return true from equals if key values and other properties are equal")
     void shouldBeEqualWithSameKeyValues() {
-        PageRequest pageRequest25P1S0A1 = PageRequest.ofSize(25).afterKey("keyval1", '2', 3);
-        PageRequest pageRequest25P1S0B1 = PageRequest.ofSize(25).beforeKey("keyval1", '2', 3);
+        PageRequest pageRequest25P1S0A1 = PageRequest.ofSize(25).afterCursor(PageRequest.Cursor.forKey("keyval1", '2', 3));
+        PageRequest pageRequest25P1S0B1 = PageRequest.ofSize(25).beforeCursor(PageRequest.Cursor.forKey("keyval1", '2', 3));
         PageRequest pageRequest25P1S0A1Match = PageRequest.ofSize(25).afterCursor(new PageRequestCursor("keyval1", '2', 3));
         PageRequest pageRequest25P2S0A1 = PageRequest.ofPage(2).size(25).afterCursor(new PageRequestCursor("keyval1", '2', 3));
-        PageRequest pageRequest25P1S0A2 = PageRequest.ofSize(25).afterKey("keyval2", '2', 3);
+        PageRequest pageRequest25P1S0A2 = PageRequest.ofSize(25).afterCursor(PageRequest.Cursor.forKey("keyval2", '2', 3));
 
         PageRequest.Cursor cursor1 = new PageRequestCursor("keyval1", '2', 3);
         PageRequest.Cursor cursor2 = new PageRequestCursor("keyval2", '2', 3);
@@ -161,7 +164,7 @@ public int size() {
             }
 
             @Override
-            public List elements() {
+            public List elements() {
                 return List.of(keyset);
             }
         };
@@ -191,17 +194,15 @@ public List elements() {
     @Test
     @DisplayName("Should raise IllegalArgumentException when key values are absent")
     void shouldRaiseErrorForMissingKeysetValues() {
-        assertThatIllegalArgumentException().isThrownBy(() -> PageRequest.ofSize(60).afterKey((Object[]) null));
-        assertThatIllegalArgumentException().isThrownBy(() -> PageRequest.ofSize(70).afterKey(new Object[0]));
-        assertThatIllegalArgumentException().isThrownBy(() -> PageRequest.ofSize(80).beforeKey((Object[]) null));
-        assertThatIllegalArgumentException().isThrownBy(() -> PageRequest.ofSize(90).beforeKey(new Object[0]));
+        assertThatIllegalArgumentException().isThrownBy(() -> PageRequest.Cursor.forKey((Object[]) null));
+        assertThatIllegalArgumentException().isThrownBy(() -> PageRequest.Cursor.forKey(new Object[0]));
     }
 
     @Test
-    @DisplayName("Key should be replaced on new instance of PageRequest")
-    void shouldReplaceKey() {
-        PageRequest p1 = PageRequest.ofPage(12).size(30).afterKey("last1", "fname1", 100);
-        PageRequest p2 = p1.beforeKey("lname2", "fname2", 200);
+    @DisplayName("Cursor should be replaced on new instance of PageRequest")
+    void shouldReplaceCursor() {
+        PageRequest p1 = PageRequest.ofPage(12).size(30).afterCursor(PageRequest.Cursor.forKey("last1", "fname1", 100));
+        PageRequest p2 = p1.beforeCursor(PageRequest.Cursor.forKey("lname2", "fname2", 200));
 
         assertSoftly(softly -> {
             softly.assertThat(p1.mode()).isEqualTo(PageRequest.Mode.CURSOR_NEXT);
diff --git a/tck/src/main/java/ee/jakarta/tck/data/standalone/entity/EntityTests.java b/tck/src/main/java/ee/jakarta/tck/data/standalone/entity/EntityTests.java
index cee874247..bf8514f2c 100644
--- a/tck/src/main/java/ee/jakarta/tck/data/standalone/entity/EntityTests.java
+++ b/tck/src/main/java/ee/jakarta/tck/data/standalone/entity/EntityTests.java
@@ -65,6 +65,7 @@
 import jakarta.data.exceptions.NonUniqueResultException;
 import jakarta.data.page.Page;
 import jakarta.data.page.PageRequest;
+import jakarta.data.page.PageRequest.Cursor;
 import jakarta.inject.Inject;
 
 /**
@@ -941,8 +942,9 @@ public void testCursoredPageOf7FromCursor() {
         //                                                                                  ^^^^^ next page ^^^^
 
         Order order = Order.by(Sort.asc("floorOfSquareRoot"), Sort.desc("id"));
-        PageRequest middle7 = PageRequest.ofPage(4).size(7)
-                        .afterKey((short) 5, 5L, 26L); // 20th result is 26; it requires 5 bits and its square root rounds down to 5.
+        PageRequest middle7 = PageRequest.afterCursor(
+                Cursor.forKey((short) 5, 5L, 26L), // 20th result is 26; it requires 5 bits and its square root rounds down to 5.),
+                4L, 7, true);
 
         CursoredPage page;
         try {
@@ -1047,8 +1049,9 @@ public void testCursoredPageWithoutTotalOf9FromCursor() {
         //                                  ^^^^^^^^ slice 2 ^^^^^^^^^
         //                                                                                        ^^^^^^^^ slice 3 ^^^^^^^^^
 
-        PageRequest middle9 = PageRequest.ofPage(4).size(9).withoutTotal()
-                             .afterKey(6L, 46L); // 20th result is 46; its square root rounds down to 6.
+        PageRequest middle9 = PageRequest.afterCursor(
+                Cursor.forKey(6L, 46L), // 20th result is 46; its square root rounds down to 6.
+                4L, 9, false);
         Order order = Order.by(Sort.desc("floorOfSquareRoot"), Sort.asc("id"));
 
         CursoredPage slice;
@@ -1104,7 +1107,7 @@ public void testCursoredPageWithoutTotalOf9FromCursor() {
     @Assertion(id = "133", strategy = "Request a CursoredPage of results where none match the query, expecting an empty CursoredPage with 0 results.")
     public void testCursoredPageWithoutTotalOfNothing() {
         // There are no numbers larger than 30 which have a square root that rounds down to 3.
-        PageRequest pagination = PageRequest.ofSize(33).afterKey(30L).withoutTotal();
+        PageRequest pagination = PageRequest.ofSize(33).afterCursor(Cursor.forKey(30L)).withoutTotal();
 
         CursoredPage slice;
         try {