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.

Sign up for GitHub

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

add boolean Page.hasNextPage() #506

Merged
merged 6 commits into from
Mar 1, 2024
Merged

add boolean Page.hasNextPage() #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a1fd27
add boolean Page.hasNextPage()
gavinking Feb 29, 2024
2395cc3
Update api/src/main/java/jakarta/data/page/Page.java
gavinking Feb 29, 2024
bf078f6
Update api/src/main/java/jakarta/data/page/Page.java
gavinking Feb 29, 2024
adadd9f
Update api/src/main/java/jakarta/data/page/impl/PageRecord.java
gavinking Feb 29, 2024
1951072
propagate hasNext() / hasPrevious() to all appropriate types
gavinking Feb 29, 2024
ed7bec5
Update api/src/main/java/jakarta/data/page/KeysetAwareSlice.java
gavinking Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/src/main/java/jakarta/data/page/KeysetAwareSlice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ public interface KeysetAwareSlice<T> extends Slice<T> {
*/
PageRequest.Cursor getKeysetCursor(int index);

/**
* Returns {@code true} when it is possible to navigate to a previous
* page of results or if it is necessary to request a previous page in order to
* determine whether there are more previous results.
* @return {@code false} if the current page is empty or if it is known
* that there is not a previous page.
*/
boolean hasPrevious();
Copy link
Contributor

@otaviojava otaviojava Mar 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Using false to both makes it unclear if it is empty or the database does not know the previous page.

Are we sure to proceed this way?

I mean, if I receive false I need to check anyway...

Maybe an UnsupportedOperationException when it does not know

Copy link
Contributor

Choose a reason for hiding this comment

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

Using false to both makes it unclear if it is empty or the database does not know the previous page.

Are we sure to proceed this way?

I mean, if I receive false I need to check anyway...

Maybe an UnsupportedOperationException when it does not know

Otavio, are you sure you are reading this correctly? When the value is false is when you should never try to request the page because it is known to not exist. An empty page has no cursors and therefore cannot have a previous. In the other case it is stated there is no previous page. If you get false and then ask for previousPageableRequest anyways, you can expect to get an exception from the latter. There is no point in trying.

Copy link
Contributor

Choose a reason for hiding this comment

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

That was my bad. Thank you for clarifying it.


/**
* <p>Creates a request for the next page
* in a forward direction from the current page. This method computes a
Expand Down
8 changes: 8 additions & 0 deletions api/src/main/java/jakarta/data/page/Slice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ public interface Slice<T> extends Streamable<T> {
*/
int numberOfElements();

/**
* Returns {@code true} if it is known that there are more results or that it is
* necessary to request a next page to determine whether there are more results, so that
* {@link #nextPageRequest()} will definitely not return {@code null}.
* @return {@code false} if this is the last page of results.
*/
boolean hasNext();
Copy link
Contributor

Choose a reason for hiding this comment

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

The same point here.

Include the UnsupportedOperationException if the database does not know.

Copy link
Contributor

Choose a reason for hiding this comment

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

No -that wrecks the whole pattern. The point is that if the provider does not know if there is going to be a next page, then we want the user to request it to find out and the user should be able to do that without having to experience exceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@otaviojava I think the idea here is that if you have a full last page of results you might not know that it is the last page. What @njr-11 is trying to permit here is that there be an "empty last page". Which I think is fine and correct. Providers can in principle use a "fancy" implementation that avoids this, but I don't think we should mandate such fanciness.


/**
* Returns the {@link PageRequest page request} for which this
* slice was obtained.
Expand Down
10 changes: 10 additions & 0 deletions api/src/main/java/jakarta/data/page/impl/KeysetAwarePageRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ public int numberOfElements() {
return content.size();
}

@Override
public boolean hasNext() {
return nextPageRequest != null;
}

@Override
public boolean hasPrevious() {
return previousPageRequest != null;
}

@Override
@SuppressWarnings("unchecked")
public <E> PageRequest<E> pageRequest(Class<E> entityClass) {
Expand Down
10 changes: 10 additions & 0 deletions api/src/main/java/jakarta/data/page/impl/KeysetAwareSliceRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ public int numberOfElements() {
return content.size();
}

@Override
public boolean hasNext() {
return nextPageRequest != null;
}

@Override
public boolean hasPrevious() {
return previousPageRequest != null;
}

@Override
@SuppressWarnings("unchecked")
public <E> PageRequest<E> pageRequest(Class<E> entityClass) {
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/java/jakarta/data/page/impl/PageRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ public int numberOfElements() {
return content.size();
}

@Override
public boolean hasNext() {
return moreResults;
}

@Override
@SuppressWarnings("unchecked")
public <E> PageRequest<E> pageRequest(Class<E> entityClass) {
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/java/jakarta/data/page/impl/SliceRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public int numberOfElements() {
return content.size();
}

@Override
public boolean hasNext() {
return moreResults;
}

@Override
@SuppressWarnings("unchecked")
public <E> PageRequest<E> pageRequest(Class<E> entityClass) {
Expand Down