From 99511dc49c02f94a1c1a1763e2947f096d0a97a6 Mon Sep 17 00:00:00 2001 From: Nathan Rauh Date: Wed, 6 Mar 2024 11:03:36 -0600 Subject: [PATCH 1/2] PageRecord computes incorrect moreResults when totalElements is uknown Signed-off-by: Nathan Rauh --- .../jakarta/data/page/impl/PageRecord.java | 25 +++- .../data/page/impl/PageRecordTest.java | 115 ++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 api/src/test/java/jakarta/data/page/impl/PageRecordTest.java 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 89e84e9f9..ebb03d814 100644 --- a/api/src/main/java/jakarta/data/page/impl/PageRecord.java +++ b/api/src/main/java/jakarta/data/page/impl/PageRecord.java @@ -29,20 +29,39 @@ * This may be used to simplify implementation of a repository interface. * * @param pageRequest The {@link PageRequest page request} for which this - * slice was obtained + * page was obtained * @param content The page content * @param totalElements The total number of elements across all pages that - * can be requested for the query + * can be requested for the query. A negative value + * indicates that a total count of elements and pages + * is not available. * @param moreResults whether there is a (nonempty) next page of results * @param The type of elements on the page */ public record PageRecord(PageRequest pageRequest, List content, long totalElements, boolean moreResults) implements Page { + /** + * Constructs a new instance, computing the {@link #moreResults} + * component as {@code true} if the page {@code content} is a full + * page of results and the {@code totalElements} is either unavailable + * (indicated by a negative value) or it exceeds the current + * {@linkplain PageRequest#page() page number} multiplied by the + * {@link PageRequest#size() size} of a full page. + * + * @param pageRequest The {@link PageRequest page request} for which + * this page was obtained. + * @param content The page content. + * @param totalElements The total number of elements across all pages + * that can be requested for the query. A negative + * value indicates that a total count of elements + * and pages is not available. + */ public PageRecord(PageRequest pageRequest, List content, long totalElements) { this( pageRequest, content, totalElements, content.size() == pageRequest.size() - && totalElements > pageRequest.size() * pageRequest.page() ); + && (totalElements < 0 + || totalElements > pageRequest.size() * pageRequest.page() )); } @Override diff --git a/api/src/test/java/jakarta/data/page/impl/PageRecordTest.java b/api/src/test/java/jakarta/data/page/impl/PageRecordTest.java new file mode 100644 index 000000000..a2fbfd2c3 --- /dev/null +++ b/api/src/test/java/jakarta/data/page/impl/PageRecordTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2024 Contributors to the Eclipse Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package jakarta.data.page.impl; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import jakarta.data.page.PageRequest; + +import java.util.List; +import java.util.NoSuchElementException; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.SoftAssertions.assertSoftly; + +class PageRecordTest { + + @Test + @DisplayName("The custom constructor can create instances where moreResults is inferred from the other record components.") + void shouldCreateInstanceWithCustomConstructor() { + + PageRequest page2Request = PageRequest.of(String.class).page(2).size(4); + List page2Content = List.of("E", "F", "G", "H"); + PageRecord page2 = new PageRecord<>(page2Request, page2Content, -1L); // unknown total elements + + assertSoftly(softly -> { + softly.assertThat(page2.content()).isEqualTo(page2Content); + softly.assertThat(page2.hasContent()).isEqualTo(true); + softly.assertThat(page2.hasNext()).isEqualTo(true); + softly.assertThat(page2.hasPrevious()).isEqualTo(true); + softly.assertThat(page2.hasTotals()).isEqualTo(false); + softly.assertThat(page2.moreResults()).isEqualTo(true); + softly.assertThat(page2.nextPageRequest()).isEqualTo(PageRequest.of(String.class).page(3).size(4)); + softly.assertThat(page2.numberOfElements()).isEqualTo(4); + softly.assertThat(page2.previousPageRequest()).isEqualTo(PageRequest.of(String.class).page(1).size(4)); + }); + assertThatThrownBy(() -> page2.totalElements()).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> page2.totalPages()).isInstanceOf(IllegalStateException.class); + + PageRequest page5Request = PageRequest.of(String.class).page(5).size(4); + List page5Content = List.of("Q", "R"); + PageRecord page5 = new PageRecord<>(page5Request, page5Content, -999L); // unknown total elements + + assertSoftly(softly -> { + softly.assertThat(page5.content()).isEqualTo(page5Content); + softly.assertThat(page5.hasContent()).isEqualTo(true); + softly.assertThat(page5.hasNext()).isEqualTo(false); + softly.assertThat(page5.hasPrevious()).isEqualTo(true); + softly.assertThat(page5.hasTotals()).isEqualTo(false); + softly.assertThat(page5.moreResults()).isEqualTo(false); + softly.assertThat(page5.numberOfElements()).isEqualTo(2); + softly.assertThat(page5.previousPageRequest()).isEqualTo(PageRequest.of(String.class).page(4).size(4)); + }); + assertThatThrownBy(() -> page5.nextPageRequest()).isInstanceOf(NoSuchElementException.class); + assertThatThrownBy(() -> page5.totalElements()).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> page5.totalPages()).isInstanceOf(IllegalStateException.class); + } + + @Test + @DisplayName("Instances created by the record constructor must adhere to the requirements of the Page interface.") + void shouldCreateInstanceWithRecordConstructor() { + + PageRequest page1Request = PageRequest.of(String.class).page(1).size(5); + List page1Content = List.of("A", "B", "C", "D", "E"); + PageRecord page1 = new PageRecord<>(page1Request, page1Content, 18L, true); + + assertSoftly(softly -> { + softly.assertThat(page1.content()).isEqualTo(page1Content); + softly.assertThat(page1.hasContent()).isEqualTo(true); + softly.assertThat(page1.hasNext()).isEqualTo(true); + softly.assertThat(page1.hasPrevious()).isEqualTo(false); + softly.assertThat(page1.hasTotals()).isEqualTo(true); + softly.assertThat(page1.moreResults()).isEqualTo(true); + softly.assertThat(page1.nextPageRequest()).isEqualTo(PageRequest.of(String.class).page(2).size(5)); + softly.assertThat(page1.numberOfElements()).isEqualTo(5); + softly.assertThat(page1.totalElements()).isEqualTo(18L); + softly.assertThat(page1.totalPages()).isEqualTo(4); + }); + assertThatThrownBy(() -> page1.previousPageRequest()).isInstanceOf(NoSuchElementException.class); + + PageRequest page3Request = PageRequest.of(String.class).page(3).size(5); + List page3Content = List.of("K", "L", "M", "N", "O"); + PageRecord page3 = new PageRecord<>(page3Request, page3Content, 18L, true); + + assertSoftly(softly -> { + softly.assertThat(page3.content()).isEqualTo(page3Content); + softly.assertThat(page3.hasContent()).isEqualTo(true); + softly.assertThat(page3.hasNext()).isEqualTo(true); + softly.assertThat(page3.hasPrevious()).isEqualTo(true); + softly.assertThat(page3.hasTotals()).isEqualTo(true); + softly.assertThat(page3.moreResults()).isEqualTo(true); + softly.assertThat(page3.nextPageRequest()).isEqualTo(PageRequest.of(String.class).page(4).size(5)); + softly.assertThat(page3.numberOfElements()).isEqualTo(5); + softly.assertThat(page3.previousPageRequest()).isEqualTo(PageRequest.of(String.class).page(2).size(5)); + softly.assertThat(page3.totalElements()).isEqualTo(18L); + softly.assertThat(page3.totalPages()).isEqualTo(4); + }); + } + +} From b459b4ec562ea0a6455297e3d89ce1926baa9356 Mon Sep 17 00:00:00 2001 From: Nathan Rauh Date: Wed, 6 Mar 2024 11:12:43 -0600 Subject: [PATCH 2/2] fix alternating highlighting in final entry of return types table --- api/src/main/java/module-info.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/module-info.java b/api/src/main/java/module-info.java index f9bac9919..443171885 100644 --- a/api/src/main/java/module-info.java +++ b/api/src/main/java/module-info.java @@ -529,7 +529,7 @@ * The caller must arrange to {@link java.util.stream.BaseStream#close() close} * all streams that it obtains from repository methods. * - * find...By...(..., PageRequest) + * find...By...(..., PageRequest) * Page<E>, CursoredPage<E> * For use with pagination *