Skip to content

Commit

Permalink
Issue #672 review comment to add builder method with all values speci…
Browse files Browse the repository at this point in the history 
…fied
  • Loading branch information
@njr-11
njr-11 committed Apr 9, 2024
1 parent 395e341 commit 26ffc6f
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 78 deletions.
13 changes: 7 additions & 6 deletions api/src/main/java/jakarta/data/page/CursoredPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -78,12 +79,12 @@
* in an instance of {@link PageRequest.Cursor Cursor} and identifying the last
* result on the current page.</p>
*
* <p>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:</p>
* <p>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:</p>
*
* <pre>
* Employee emp = ...
Expand Down
59 changes: 46 additions & 13 deletions api/src/main/java/jakarta/data/page/PageRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -94,30 +111,46 @@ static PageRequest ofSize(int maxPageSize) {
* <p>Requests {@linkplain CursoredPage cursor-based pagination} in the forward direction,
* starting after the specified key.</p>
*
* @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);
}

/**
* <p>Requests {@linkplain CursoredPage cursor-based pagination} in the previous page
* direction relative to the specified key.</p>
* direction relative to the specified cursor.</p>
*
* @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);
}

/**
* <p>Requests {@linkplain CursoredPage cursor-based pagination} in the forward direction,
Expand Down
10 changes: 0 additions & 10 deletions api/src/main/java/jakarta/data/page/Pagination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 10 additions & 16 deletions api/src/main/java/jakarta/data/page/impl/CursoredPageRecord.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,6 @@ public record CursoredPageRecord<T>
PageRequest nextPageRequest, PageRequest previousPageRequest)
implements CursoredPage<T> {

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,
Expand All @@ -75,8 +61,16 @@ private static final PageRequest before(PageRequest.Cursor newCursor, PageReques
(List<T> content, List<PageRequest.Cursor> 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
Expand Down
14 changes: 6 additions & 8 deletions api/src/main/java/jakarta/data/page/impl/PageRecord.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
41 changes: 21 additions & 20 deletions api/src/test/java/jakarta/data/page/PageRequestCursorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<PageRequest, Integer> map = new HashMap<>();

assertSoftly(softly -> {
Expand All @@ -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 -> {

Expand All @@ -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);
Expand All @@ -161,7 +164,7 @@ public int size() {
}

@Override
public List elements() {
public List<?> elements() {
return List.of(keyset);
}
};
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 8 additions & 5 deletions tck/src/main/java/ee/jakarta/tck/data/standalone/entity/EntityTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -941,8 +942,9 @@ public void testCursoredPageOf7FromCursor() {
// ^^^^^ next page ^^^^

Order<NaturalNumber> 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<NaturalNumber> page;
try {
Expand Down Expand Up @@ -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<NaturalNumber> order = Order.by(Sort.desc("floorOfSquareRoot"), Sort.asc("id"));

CursoredPage<NaturalNumber> slice;
Expand Down Expand Up @@ -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<NaturalNumber> slice;
try {
Expand Down

0 comments on commit 26ffc6f

Please sign in to comment.