Polishing.

Refactor WindowIterator to return individual objects during scrolling.

Original Pull Request: #2787

Author: mp911de

src/main/asciidoc/repositories-scrolling.adoc

@@ -20,20 +20,19 @@ do {
 
   // obtain the next Scroll
   users = repository.findFirst10ByLastnameOrderByFirstname("Doe", users.positionAt(users.size() - 1));
-} while (!users.isEmpty() && !users.isLast());
+} while (!users.isEmpty() && users.hasNext());
 ----
 
-`WindowIterator` provides a specialized implementation that simplifies consumption of multiple `Window` instances by removing the need to check for the presence of a next `Window` and applying the `ScrollPosition`.
+`WindowIterator` provides a utility to simplify scrolling across ``Window``s by removing the need to check for the presence of a next `Window` and applying the `ScrollPosition`.
 
 [source,java]
 ----
 WindowIterator<User> users = WindowIterator.of(position -> repository.findFirst10ByLastnameOrderByFirstname("Doe", position))
   .startingAt(OffsetScrollPosition.initial());
 
 while (users.hasNext()) {
-  users.next().forEach(user -> {
-    // consume the user
-  });
+  User u = users.next();
+  // consume the user
 }
 ----
 
@@ -56,7 +55,8 @@ interface UserRepository extends Repository<User, Long> {
 WindowIterator<User> users = WindowIterator.of(position -> repository.findFirst10ByLastnameOrderByFirstname("Doe", position))
   .startingAt(OffsetScrollPosition.initial()); <1>
 ----
-<1> Start from the initial offset at position 0;
+
+<1> Start from the initial offset at position `0`.
 ====
 
 [[repositories.scrolling.keyset]]
@@ -76,7 +76,7 @@ The database needs only constructing a much smaller result from the given keyset
 
 [WARNING]
 ====
-Keyset-Filtering requires properties used for sorting to be non nullable.
+Keyset-Filtering requires the keyset properties (those used for sorting) to be non-nullable.
 This limitation applies due to the store specific `null` value handling of comparison operators as well as the need to run queries against an indexed source.
 Keyset-Filtering on nullable properties will lead to unexpected results.
 ====

src/main/java/org/springframework/data/domain/KeysetScrollPosition.java

@@ -129,7 +129,7 @@ public String toString() {
 	/**
 	 * Keyset scrolling direction.
 	 */
-	enum Direction {
+	public enum Direction {
 
 		/**
 		 * Forward (default) direction to scroll from the beginning of the results to their end.

src/main/java/org/springframework/data/domain/Window.java

@@ -24,8 +24,8 @@
 
 /**
  * A set of data consumed from an underlying query result. A {@link Window} is similar to {@link Slice} in the sense
- * that it contains a subset of the actual query results for easier consumption of large result sets. The window is less
- * opinionated about the actual data retrieval, whether the query has used index/offset, keyset-based pagination or
+ * that it contains a subset of the actual query results for easier scrolling across large result sets. The window is
+ * less opinionated about the actual data retrieval, whether the query has used index/offset, keyset-based pagination or
  * cursor resume tokens.
  *
  * @author Mark Paluch
@@ -93,16 +93,33 @@ default boolean isLast() {
 	/**
 	 * Returns if there is a next window.
 	 *
-	 * @return if there is a next window window.
+	 * @return if there is a next window.
 	 */
 	boolean hasNext();
 
+	/**
+	 * Returns whether the underlying scroll mechanism can provide a {@link ScrollPosition} at {@code index}.
+	 *
+	 * @param index
+	 * @return {@code true} if a {@link ScrollPosition} can be created; {@code false} otherwise.
+	 * @see #positionAt(int)
+	 */
+	default boolean hasPosition(int index) {
+		try {
+			return positionAt(index) != null;
+		} catch (IllegalStateException e) {
+			return false;
+		}
+	}
+
 	/**
 	 * Returns the {@link ScrollPosition} at {@code index}.
 	 *
 	 * @param index
 	 * @return
 	 * @throws IndexOutOfBoundsException if the index is out of range ({@code index < 0 || index >= size()}).
+	 * @throws IllegalStateException if the underlying scroll mechanism cannot provide a scroll position for the given
+	 *           object.
 	 */
 	ScrollPosition positionAt(int index);
 
@@ -112,6 +129,8 @@ default boolean isLast() {
 	 * @param object
 	 * @return
 	 * @throws NoSuchElementException if the object is not part of the result.
+	 * @throws IllegalStateException if the underlying scroll mechanism cannot provide a scroll position for the given
+	 *           object.
 	 */
 	default ScrollPosition positionAt(T object) {
 

src/main/java/org/springframework/data/domain/WindowIterator.java

@@ -15,37 +15,40 @@
  */
 package org.springframework.data.domain;
 
-import java.util.ArrayList;
 import java.util.Iterator;
-import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.function.Function;
 
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
- * An {@link Iterator} over multiple {@link Window Windows} obtained via a {@link Function window function}, that keeps track of
- * the current {@link ScrollPosition} returning the Window {@link Window#getContent() content} on {@link #next()}.
+ * An {@link Iterator} over multiple {@link Window Windows} obtained via a {@link Function window function}, that keeps
+ * track of the current {@link ScrollPosition} allowing scrolling across all result elements.
+ *
  * <pre class="code">
- * WindowIterator&lt;User&gt; users = WindowIterator.of(position -> repository.findFirst10By...("spring", position))
+ * WindowIterator&lt;User&gt; users = WindowIterator.of(position -> repository.findFirst10By…("spring", position))
  *   .startingAt(OffsetScrollPosition.initial());
+ *
  * while (users.hasNext()) {
- *   users.next().forEach(user -> {
- *     // consume the user
- *   });
+ *   User u = users.next();
+ *   // consume user
  * }
  * </pre>
  *
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @since 3.1
  */
-public class WindowIterator<T> implements Iterator<List<T>> {
+public class WindowIterator<T> implements Iterator<T> {
 
 	private final Function<ScrollPosition, Window<T>> windowFunction;
+
 	private ScrollPosition currentPosition;
 
-	@Nullable //
-	private Window<T> currentWindow;
+	private @Nullable Window<T> currentWindow;
+
+	private @Nullable Iterator<T> currentIterator;
 
 	/**
 	 * Entrypoint to create a new {@link WindowIterator} for the given windowFunction.
@@ -55,42 +58,57 @@ public class WindowIterator<T> implements Iterator<List<T>> {
 	 * @return new instance of {@link WindowIteratorBuilder}.
 	 */
 	public static <T> WindowIteratorBuilder<T> of(Function<ScrollPosition, Window<T>> windowFunction) {
-		return new WindowIteratorBuilder(windowFunction);
+		return new WindowIteratorBuilder<>(windowFunction);
 	}
 
 	WindowIterator(Function<ScrollPosition, Window<T>> windowFunction, ScrollPosition position) {
 
 		this.windowFunction = windowFunction;
 		this.currentPosition = position;
-		this.currentWindow = doScroll();
 	}
 
 	@Override
 	public boolean hasNext() {
-		return currentWindow != null;
-	}
 
-	@Override
-	public List<T> next() {
+		// use while loop instead of recursion to fetch the next window.
+		do {
+			if (currentWindow == null) {
+				currentWindow = windowFunction.apply(currentPosition);
+			}
+
+			if (currentIterator == null) {
+				if (currentWindow != null) {
+					currentIterator = currentWindow.iterator();
+				}
+			}
+
+			if (currentIterator != null) {
 
-		List<T> toReturn = new ArrayList<>(currentWindow.getContent());
-		currentPosition = currentWindow.positionAt(currentWindow.size() -1);
-		currentWindow = doScroll();
-		return toReturn;
+				if (currentIterator.hasNext()) {
+					return true;
+				}
+
+				if (currentWindow != null && currentWindow.hasNext()) {
+
+					currentPosition = currentWindow.positionAt(currentWindow.size() - 1);
+					currentIterator = null;
+					currentWindow = null;
+					continue;
+				}
+			}
+
+			return false;
+		} while (true);
 	}
 
-	@Nullable
-	Window<T> doScroll() {
+	@Override
+	public T next() {
 
-		if (currentWindow != null && !currentWindow.hasNext()) {
-			return null;
+		if (!hasNext()) {
+			throw new NoSuchElementException();
 		}
 
-		Window<T> window = windowFunction.apply(currentPosition);
-		if (window.isEmpty() && window.isLast()) {
-			return null;
-		}
-		return window;
+		return currentIterator.next();
 	}
 
 	/**
@@ -102,15 +120,25 @@ Window<T> doScroll() {
 	 */
 	public static class WindowIteratorBuilder<T> {
 
-		private Function<ScrollPosition, Window<T>> windowFunction;
+		private final Function<ScrollPosition, Window<T>> windowFunction;
 
 		WindowIteratorBuilder(Function<ScrollPosition, Window<T>> windowFunction) {
+
+			Assert.notNull(windowFunction, "WindowFunction must not be null");
+
 			this.windowFunction = windowFunction;
 		}
 
+		/**
+		 * Create a {@link WindowIterator} given {@link ScrollPosition}.
+		 *
+		 * @param position
+		 * @return
+		 */
 		public WindowIterator<T> startingAt(ScrollPosition position) {
 
-			Assert.state(windowFunction != null, "WindowFunction cannot not be null");
+			Assert.notNull(position, "ScrollPosition must not be null");
+
 			return new WindowIterator<>(windowFunction, position);
 		}
 	}