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

Keyset positionFunction does not retain directional state and uses position of one-off #2999

Closed
quaff opened this issue Jun 6, 2023 · 2 comments
Labels
type: bug A general bug

Comments

@quaff
Copy link
Contributor

quaff commented Jun 6, 2023


We must use window.positionAt(1) instead of window.positionAt(0) if we want position at the begin of window, we should call getResultWindow() first.


The direction always be FORWARD, it should be BACKWARD if the original direction is BACKWARD.

quaff added a commit to quaff/spring-data-jpa that referenced this issue Jun 6, 2023
Before this commit, positionFunction created by ScrollDelegate retains incorrect state when perform backward scrolling.
1. We must use window.positionAt(1) instead of window.positionAt(0) if we want position at the begin of window, we should call getResultWindow() first.
2. The direction always be FORWARD, it should be BACKWARD if the original direction is BACKWARD.

Closes spring-projectsGH-2999
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 6, 2023
@mp911de
Copy link
Member

mp911de commented Jun 6, 2023

Scrolling forward after retrieving a window is by design as the Window.next() and all scroll position methods suggest forward scrolling. Reversing is rather a by-product and not reflected within the consuming API.

@quaff
Copy link
Contributor Author

quaff commented Jun 6, 2023

It's weird that I cannot using BACKWARD scrolling for iteration.
My test case is broken if scrollDirection is BACKWARD.

	@ParameterizedTest
	@MethodSource("provideParameters")
	void keysetScrolling(Sort.Direction sortDirection, ScrollPosition.Direction scrollDirection) {

		List<List<TestEntity>> contents = new ArrayList<>();

		String propertyName = "index";
		Object anchor = null;
		Sort sort = Sort.by(sortDirection, propertyName);

		Map<String, ?> keys = anchor != null ? Map.of(propertyName, anchor) : Map.of();
		ScrollPosition pos = ScrollPosition.of(keys, scrollDirection);
		while (true) {
			ScrollPosition spToUse = pos;
			Window<TestEntity> window = this.repository.findBy((root, query, cb) -> {
				return null;
			}, q -> q.limit(pageSize).sortBy(sort).scroll(spToUse));
			contents.add(window.getContent());
			if (!window.hasNext()) {
				break;
			}
			pos = window.positionAt(((KeysetScrollPosition) pos).scrollsForward() ? window.size() - 1 : 0);
		}
		// asserts
	}

	private static Stream<Arguments> provideParameters() {
		return cartesian(Sort.Direction.class, ScrollPosition.Direction.class);
	}

	private static Stream<Arguments> cartesian(Class<? extends Enum<?>> enumA, Class<? extends Enum<?>> enumB) {
		return Stream.of(enumA.getEnumConstants())
			.flatMap(a -> Stream.of(enumB.getEnumConstants()).map(b -> Arguments.of(a, b)));
	}

@mp911de mp911de changed the title positionFunction created by ScrollDelegate retains incorrect state when perform backward scrolling Keyset positionFunction does not retain directional state and uses position of one-off Jun 13, 2023
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 13, 2023
@mp911de mp911de added this to the 3.1.1 (2023.0.1) milestone Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants