Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PageRecord computes incorrect moreResults when totalElements unknown #524

Conversation

njr-11
Copy link
Contributor

@njr-11 njr-11 commented Mar 6, 2024

I noticed this warning when building the project,

[WARNING] Javadoc Warnings
[WARNING] /Users/njr/lgit/data/api/src/main/java/jakarta/data/page/impl/PageRecord.java:42: warning: no comment
[WARNING] public PageRecord(PageRequest<T> pageRequest, List<T> content, long totalElements) {
[WARNING] ^
[WARNING] 1 warning

When I looked into correcting it by adding the missing JavaDoc, I realized the documentation would need to state how the constructor computes the moreResults record component and started to do so, but then realized the logic is incorrect for the path where the totalElements is unknown, always computing a false value for moreResults. I added a unit test to demonstrate the error and corrected the code.

Also, I'm fixing a minor display issue in the return types table where the alternating highlighting is out of sync for the last entry of the table, causing it to appear like the information applies to the last two entries lumped together as one.

@njr-11 njr-11 added this to the Jakarta Data 1.0 milestone Mar 6, 2024
public PageRecord(PageRequest<T> pageRequest, List<T> content, long totalElements) {
this( pageRequest, content, totalElements,
content.size() == pageRequest.size()
&& totalElements > pageRequest.size() * pageRequest.page() );
&& (totalElements < 0
|| totalElements > pageRequest.size() * pageRequest.page() ));
Copy link
Contributor

@gavinking gavinking Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's definitely what I intended it to say. Not quite sure how I managed to mess that up.

@otaviojava otaviojava merged commit 67b9fe3 into jakartaee:main Mar 6, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants