Add support for Scroll API to allow Keyset- and Offset-based scrolling
#2787
Conversation
|
The name of |
|
|
||
| return OffsetCursorRequest.ofSize(getPageSize(), getSort()).withOffset(getOffset()); | ||
| } | ||
|
|
There was a problem hiding this comment.
The created CursorRequest points to page 0 and element offset 0 regardless of the page number. Shouldn't Pageable.getOffset() be passed into ofSize?
| * @param sort the cursor result sort order. | ||
| * @return a new {@link CursorRequest} with {@link Sort} applied. | ||
| */ | ||
| CursorRequest withSort(Sort sort); |
There was a problem hiding this comment.
Why does a request need to know about the sort? I agree that there's value in the ability to change the window size with each request, but changing result order while scrolling through data does not make senes, does it?
There was a problem hiding this comment.
This was introduced to carry over the final sort from a query method that either defines a sort or accepts a Sort argument.
| * @return the {@link CursorRequest} to obtain the next cursor window. | ||
| * @throws IllegalStateException if there is no {@link #hasNext() next} cursor window. | ||
| */ | ||
| CursorRequest nextCursorRequest(); |
There was a problem hiding this comment.
Would it make senes to expose a Position rather than the next request.
mp911de
changed the title
CursorWindow API to allow Keyset- and Offset-based cursor windowingScroll API to allow Keyset- and Offset-based scrolling
| * @param direction must not be {@literal null}. | ||
| * @return a new {@link KeysetScrollPosition} for the given keyset and {@link Direction}. | ||
| */ | ||
| public static KeysetScrollPosition of(Map<String, ?> keys, Direction direction) { |
There was a problem hiding this comment.
This is impossible to use currently since Direction is declared package private.
| * @since 3.1 | ||
| * @see ScrollPosition | ||
| */ | ||
| public interface Scroll<T> extends Streamable<T> { |
There was a problem hiding this comment.
I think Window was the better term here. It aligned nicely with ScrollPosition.
| } | ||
|
|
||
| // obtain the next Scroll | ||
| users = repository.findFirst10ByLastnameOrderByFirstname("Doe", users.lastScrollPosition()); |
There was a problem hiding this comment.
When reading this, I had a hard time to find out what the ScrollPosition actually does. Is it including, like give me everything starting (inclusive) at a certain position. Or is it excluding, like return the data and start after the position.
My point here is, should we call it lastScrollPosition or would it better be nextScrollPosition which is also not quite accurate. Does it maybe make sense to have a scroll position that has startAt and startAfter?
Does this read any better ScrollPosition.after(users.lastScrollPosition()) or is it even more confusing?
The intend of WindowIterator is to support users who need to iterate multiple windows. It keeps track of the position and loads the next window if needed so that the user does not have to interact with the position at all. Also remove the Window methods to get the frist/last position and enforce the index based variant. Update the documentation to make use of the newly introduced API.
Refactor WindowIterator to return individual objects during scrolling.
The intend of WindowIterator is to support users who need to iterate multiple windows. It keeps track of the position and loads the next window if needed so that the user does not have to interact with the position at all. Also remove the Window methods to get the first/last position and enforce the index based variant. Update the documentation to make use of the newly introduced API. See: #2151 Original Pull Request: #2787
Refactor WindowIterator to return individual objects during scrolling. Original Pull Request: #2787
|
Merged to main development line. |
Discussed spring-projects/spring-data-commons#2151 and implemented in spring-projects/spring-data-commons#2787 we can build on top and provide basic support for both imperative and reactive repositories. The support will be available only on the repository level in the first iteration, think ```java import java.util.UUID; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Window; import org.springframework.data.neo4j.repository.Neo4jRepository; public interface ScrollingRepository extends Neo4jRepository<ScrollingEntity, UUID> { Window<ScrollingEntity> findTop4By(Sort sort, ScrollPosition position); } ``` and other derived finder methods that have a limit and a stable sort. If requested, further support can be added to the templates, too. Closes #2691.
Discussed in spring-projects/spring-data-commons#2151 and implemented in spring-projects/spring-data-commons#2787 we can build on top and provide basic support for both imperative and reactive repositories. The support will be available only on the repository level in the first iteration, think ```java import java.util.UUID; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Window; import org.springframework.data.neo4j.repository.Neo4jRepository; public interface ScrollingRepository extends Neo4jRepository<ScrollingEntity, UUID> { Window<ScrollingEntity> findTop4By(Sort sort, ScrollPosition position); } ``` and other derived finder methods that have a limit and a stable sort. If requested, further support can be added to the templates, too. Closes #2691.
Tasks:
Closes #2151