Polishing.

Let Cursor extend CloseableCursor. Refine nullability annotations and align nullability behavior in tests with actual nullability usage.

See: #1575
Original Pull Request: #2014

Author: mp911de

src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterSetCommands.java

@@ -452,7 +452,7 @@ protected ScanIteration<byte[]> doScan(long cursorId, ScanOptions options) {
 				ScanParams params = JedisConverters.toScanParams(options);
 				redis.clients.jedis.ScanResult<byte[]> result = connection.getCluster().sscan(key,
 						JedisConverters.toBytes(cursorId), params);
-				return new ScanIteration<>(Long.valueOf(result.getCursor()), result.getResult());
+				return new ScanIteration<>(Long.parseLong(result.getCursor()), result.getResult());
 			}
 		}.open();
 	}

src/main/java/org/springframework/data/redis/connection/jedis/JedisKeyCommands.java

@@ -169,7 +169,7 @@ protected ScanIteration<byte[]> doScan(long cursorId, ScanOptions options) {
 
 				ScanParams params = JedisConverters.toScanParams(options);
 				redis.clients.jedis.ScanResult<String> result = connection.getJedis().scan(Long.toString(cursorId), params);
-				return new ScanIteration<>(Long.valueOf(result.getCursor()),
+				return new ScanIteration<>(Long.parseLong(result.getCursor()),
 						JedisConverters.stringListToByteList().convert(result.getResult()));
 			}
 

src/main/java/org/springframework/data/redis/core/ConvertingCursor.java

@@ -15,8 +15,6 @@
  */
 package org.springframework.data.redis.core;
 
-import java.io.IOException;
-
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
@@ -81,7 +79,7 @@ public void remove() {
 	 * @see java.io.Closeable#close()
 	 */
 	@Override
-	public void close() throws IOException {
+	public void close() {
 		delegate.close();
 	}
 

src/main/java/org/springframework/data/redis/core/Cursor.java

@@ -15,9 +15,8 @@
  */
 package org.springframework.data.redis.core;
 
-import java.io.Closeable;
-
 import org.springframework.data.redis.util.BoundedIterator;
+import org.springframework.data.util.CloseableIterator;
 
 /**
  * Cursor abstraction to scan over the keyspace or elements within a data structure using a variant of a {@code SCAN}
@@ -28,7 +27,7 @@
  * @param <T>
  * @since 1.4
  */
-public interface Cursor<T> extends BoundedIterator<T>, Closeable {
+public interface Cursor<T> extends BoundedIterator<T>, CloseableIterator<T> {
 
 	/**
 	 * Get the reference cursor. <br>
@@ -39,19 +38,19 @@ public interface Cursor<T> extends BoundedIterator<T>, Closeable {
 	long getCursorId();
 
 	/**
-	 * @return Returns true if cursor closed.
+	 * @return {@code true} if cursor closed.
 	 */
 	boolean isClosed();
 
 	/**
 	 * Opens cursor and returns itself.
 	 *
-	 * @return
+	 * @return the opened cursor.
 	 */
 	Cursor<T> open();
 
 	/**
-	 * @return Returns the current position of the cursor.
+	 * @return the current position of the cursor.
 	 */
 	long getPosition();
 

src/main/java/org/springframework/data/redis/core/ScanCursor.java

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.redis.core;
 
-import java.io.IOException;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
@@ -39,10 +38,10 @@
  */
 public abstract class ScanCursor<T> implements Cursor<T> {
 
-	private @Nullable CursorState state;
+	private CursorState state;
 	private long cursorId;
-	private @Nullable Iterator<T> delegate;
-	private @Nullable final ScanOptions scanOptions;
+	private Iterator<T> delegate;
+	private final ScanOptions scanOptions;
 	private long position;
 	private final long limit;
 
@@ -56,7 +55,7 @@ public ScanCursor() {
 	/**
 	 * Crates new {@link ScanCursor} with {@code id=0}.
 	 *
-	 * @param options
+	 * @param options the scan options to apply.
 	 */
 	public ScanCursor(ScanOptions options) {
 		this(0, options);
@@ -65,7 +64,7 @@ public ScanCursor(ScanOptions options) {
 	/**
 	 * Crates new {@link ScanCursor} with {@link ScanOptions#NONE}
 	 *
-	 * @param cursorId
+	 * @param cursorId the cursor Id.
 	 */
 	public ScanCursor(long cursorId) {
 		this(cursorId, ScanOptions.NONE);
@@ -74,22 +73,22 @@ public ScanCursor(long cursorId) {
 	/**
 	 * Crates new {@link ScanCursor}
 	 *
-	 * @param cursorId
-	 * @param options Defaulted to {@link ScanOptions#NONE} if nulled.
+	 * @param cursorId the cursor Id.
+	 * @param options Defaulted to {@link ScanOptions#NONE} if {@code null}.
 	 */
-	public ScanCursor(long cursorId, ScanOptions options) {
+	public ScanCursor(long cursorId, @Nullable ScanOptions options) {
 
 		this.scanOptions = options != null ? options : ScanOptions.NONE;
 		this.cursorId = cursorId;
 		this.state = CursorState.READY;
-		this.delegate = Collections.<T> emptyList().iterator();
+		this.delegate = Collections.emptyIterator();
 		this.limit = -1;
 	}
 
 	/**
 	 * Crates a new {@link ScanCursor}.
 	 *
-	 * @param source
+	 * @param source source cursor.
 	 * @param limit
 	 * @since 2.5
 	 */
@@ -144,14 +143,7 @@ protected void doOpen(long cursorId) {
 
 	private void processScanResult(ScanIteration<T> result) {
 
-		if (result == null) {
-
-			resetDelegate();
-			state = CursorState.FINISHED;
-			return;
-		}
-
-		cursorId = Long.valueOf(result.getCursorId());
+		cursorId = result.getCursorId();
 
 		if (isFinished(cursorId)) {
 			state = CursorState.FINISHED;
@@ -176,7 +168,7 @@ protected boolean isFinished(long cursorId) {
 	}
 
 	private void resetDelegate() {
-		delegate = Collections.<T> emptyList().iterator();
+		delegate = Collections.emptyIterator();
 	}
 
 	/*
@@ -209,11 +201,7 @@ public boolean hasNext() {
 			return true;
 		}
 
-		if (cursorId > 0) {
-			return true;
-		}
-
-		return false;
+		return cursorId > 0;
 	}
 
 	private void assertCursorIsOpen() {
@@ -266,7 +254,7 @@ public void remove() {
 	 * @see java.io.Closeable#close()
 	 */
 	@Override
-	public final void close() throws IOException {
+	public final void close() {
 
 		try {
 			doClose();
@@ -326,7 +314,8 @@ enum CursorState {
 	}
 
 	/**
-	 * Wrapper for a concrete {@link ScanCursor} forwarding {@link #doScan(long, ScanOptions)}.
+	 * Wrapper for a concrete {@link ScanCursor} forwarding {@link #doScan(long, ScanOptions)}, {@link #doClose()} and
+	 * {@link #isClosed()}.
 	 *
 	 * @param <T>
 	 * @since 2.5
@@ -344,5 +333,15 @@ public ScanCursorWrapper(ScanCursor<T> delegate, long limit) {
 		protected ScanIteration<T> doScan(long cursorId, ScanOptions options) {
 			return delegate.doScan(cursorId, options);
 		}
+
+		@Override
+		protected void doClose() {
+			delegate.close();
+		}
+
+		@Override
+		public boolean isClosed() {
+			return delegate.isClosed();
+		}
 	}
 }

src/test/java/org/springframework/data/redis/core/ScanCursorUnitTests.java

@@ -24,7 +24,6 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.Queue;
-import java.util.Stack;
 
 import org.junit.jupiter.api.Test;
 
@@ -237,6 +236,26 @@ void limitShouldNotLimitOriginalCursor() {
 		assertThat(result).hasSize(3);
 	}
 
+	@Test // GH-1575
+	void decoratedCursorShouldForwardClose() {
+
+		LinkedList<ScanIteration<String>> values = new LinkedList<>();
+		values.add(createIteration(1, "spring"));
+		values.add(createIteration(2, "data"));
+		values.add(createIteration(3, "redis"));
+		values.add(createIteration(0));
+		Cursor<String> cursor = initCursor(values);
+		Cursor<String> limited = cursor.limit(1);
+
+		assertThat(cursor.isClosed()).isFalse();
+		assertThat(limited.isClosed()).isFalse();
+
+		limited.close();
+
+		assertThat(cursor.isClosed()).isTrue();
+		assertThat(limited.isClosed()).isTrue();
+	}
+
 	private CapturingCursorDummy initCursor(Queue<ScanIteration<String>> values) {
 		CapturingCursorDummy cursor = new CapturingCursorDummy(values);
 		cursor.open();
@@ -247,11 +266,9 @@ private ScanIteration<String> createIteration(long cursorId, String... values) {
 		return new ScanIteration<>(cursorId, values.length > 0 ? Arrays.asList(values) : Collections.<String> emptyList());
 	}
 
-	private class CapturingCursorDummy extends ScanCursor<String> {
+	private static class CapturingCursorDummy extends ScanCursor<String> {
 
-		private Queue<ScanIteration<String>> values;
-
-		private Stack<Long> cursors;
+		private final Queue<ScanIteration<String>> values;
 
 		CapturingCursorDummy(Queue<ScanIteration<String>> values) {
 			this.values = values;
@@ -260,11 +277,12 @@ private class CapturingCursorDummy extends ScanCursor<String> {
 		@Override
 		protected ScanIteration<String> doScan(long cursorId, ScanOptions options) {
 
-			if (cursors == null) {
-				cursors = new Stack<>();
+			ScanIteration<String> iteration = this.values.poll();
+
+			if (iteration == null) {
+				iteration = new ScanIteration<>(0, Collections.emptyList());
 			}
-			this.cursors.push(cursorId);
-			return this.values.poll();
+			return iteration;
 		}
 	}
 }