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

Jump to bottom

Align OffsetScrolling to zero-based indexes #3415

Closed
wants to merge 3 commits into from
Closed

Align OffsetScrolling to zero-based indexes #3415

wants to merge 3 commits into from

Conversation

christophstrobl
Copy link
Member

mp911de
mp911de reviewed Apr 3, 2024
View reviewed changes
...g-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java Outdated
@@ -259,7 +259,7 @@ public Query createQuery(JpaParametersParameterAccessor accessor) {
@SuppressWarnings("ConstantConditions")
private Query restrictMaxResultsIfNecessary(Query query, @Nullable ScrollPosition scrollPosition) {

if (scrollPosition instanceof OffsetScrollPosition offset) {
if (scrollPosition instanceof OffsetScrollPosition offset && !offset.isInitial()) {
query.setFirstResult(Math.toIntExact(offset.getOffset()));
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be +1 as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

it should, guess we're lacking a test for that one

Copy link
Member Author

Choose a reason for hiding this comment

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

nope the test was there but the testsetup should have changed

@mp911de mp911de linked an issue Apr 10, 2024 that may be closed by this pull request
Align OffsetScrolling to zero-based indexes #3409
Closed
@mp911de mp911de added the type: enhancement A general enhancement label Apr 10, 2024
@mp911de mp911de changed the title Align scroll implementation to changes in data-commons. Align scroll implementation to changes in Spring Data Commons Apr 10, 2024
@mp911de mp911de changed the title Align scroll implementation to changes in Spring Data Commons Align OffsetScrolling to zero-based indexes Apr 10, 2024
mp911de pushed a commit that referenced this pull request Apr 10, 2024
@christophstrobl @mp911de
Align OffsetScrolling to zero-based indexes.
0206de8 
Closes: #3409
Original pull request: #3415
mp911de added a commit that referenced this pull request Apr 10, 2024
@mp911de
Polishing.
4162b76 
Use PositionFunction from OffsetScrollPosition.

See: #3409
Original pull request: #3415
@mp911de mp911de closed this Apr 10, 2024
@mp911de mp911de deleted the issue/3409 branch April 10, 2024 08:48
@mp911de mp911de added this to the 3.3 RC1 (2024.0.0) milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mp911de mp911de mp911de left review comments

Assignees
No one assigned
Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
3.3 RC1 (2024.0.0)
Development

Successfully merging this pull request may close these issues.

Align OffsetScrolling to zero-based indexes
2 participants
@christophstrobl @mp911de