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

fix incorrect implementation of PageRequest.page() #671

Closed
wants to merge 1 commit into from

Conversation

gavinking
Copy link
Contributor

this operation should not propagate the Cursor value

}
return s.append("}").toString();
}

@Override
public PageRequest page(long pageNumber) {
return new Pagination(pageNumber, size, mode, type, requestTotal);
return new Pagination(pageNumber, size, Mode.OFFSET, null, requestTotal);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the important change.

this operation should not propagate the Cursor value
@gavinking
Copy link
Contributor Author

Actually I'm just now realizing that there's a huge confusion about what these copying mutators actually mean for this class.

Currently, a PageRequest mixes together "navigational" methods, and also builder-type methods. That's actually really bad.

We need to split some of these operations onto a PageRequestBuilder, so that it's obvious which operations are capable of producing a PageRequest in an inconsistent state.

@gavinking
Copy link
Contributor Author

Closing this in favor of #672 because there is a much deeper problem here.

@gavinking gavinking closed this Apr 7, 2024
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.

1 participant