From e31d122e54d760cb23b483a488272d8f1338e12c Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@vmware.com>
Date: Wed, 23 Feb 2022 15:25:23 +0100
Subject: [PATCH] Polishing.

Add since tags. Consistently document scan command with try-with-resources guidance. Use try-with-resources in tests.

See #2260
Original pull request: #2263.
---
 .../data/redis/core/BoundHashOperations.java      |  8 +++++---
 .../data/redis/core/BoundSetOperations.java       |  8 ++++++--
 .../data/redis/core/BoundZSetOperations.java      |  9 +++++----
 .../data/redis/core/HashOperations.java           |  7 ++++---
 .../data/redis/core/RedisOperations.java          |  6 ++++--
 .../data/redis/core/SetOperations.java            |  9 +++++----
 .../data/redis/core/ZSetOperations.java           |  9 +++++----
 .../DefaultHashOperationsIntegrationTests.java    | 15 ++++++++-------
 .../DefaultSetOperationsIntegrationTests.java     | 11 ++++++-----
 .../DefaultZSetOperationsIntegrationTests.java    | 10 +++++-----
 .../RedisClusterTemplateIntegrationTests.java     |  9 +++++++++
 .../redis/core/RedisTemplateIntegrationTests.java | 10 +++++-----
 12 files changed, 67 insertions(+), 44 deletions(-)

diff --git a/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java b/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java
index 843e082bc..c05a613bd 100644
--- a/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java
@@ -202,10 +202,12 @@ public interface BoundHashOperations<H, HK, HV> extends BoundKeyOperations<H> {
 	Map<HK, HV> entries();
 
 	/**
-	 * Use a {@link Cursor} to iterate over entries in the hash.
+	 * Use a {@link Cursor} to iterate over entries in hash at the bound key. <br />
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
 	 *
-	 * @param options
-	 * @return
+	 * @param options must not be {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
 	 * @since 1.4
 	 */
 	Cursor<Map.Entry<HK, HV>> scan(ScanOptions options);
diff --git a/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java b/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java
index eee8a0374..9bbcad038 100644
--- a/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/BoundSetOperations.java
@@ -253,8 +253,12 @@ public interface BoundSetOperations<K, V> extends BoundKeyOperations<K> {
 	List<V> randomMembers(long count);
 
 	/**
-	 * @param options
-	 * @return {@literal null} when used in pipeline / transaction.
+	 * Use a {@link Cursor} to iterate over entries in set at {@code key}. <br />
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
+	 *
+	 * @param options must not be {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
 	 * @since 1.4
 	 */
 	@Nullable
diff --git a/src/main/java/org/springframework/data/redis/core/BoundZSetOperations.java b/src/main/java/org/springframework/data/redis/core/BoundZSetOperations.java
index 064467c07..8f883d68b 100644
--- a/src/main/java/org/springframework/data/redis/core/BoundZSetOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/BoundZSetOperations.java
@@ -794,11 +794,12 @@ public interface BoundZSetOperations<K, V> extends BoundKeyOperations<K> {
 	Long unionAndStore(Collection<K> otherKeys, K destKey, Aggregate aggregate);
 
 	/**
-	 * Iterate over elements in zset at the bound key. <br />
-	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leak.
+	 * Use a {@link Cursor} to iterate over entries in zset at the bound key. <br />
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
 	 *
-	 * @param options
-	 * @return
+	 * @param options must not be {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
 	 * @since 1.4
 	 */
 	Cursor<TypedTuple<V>> scan(ScanOptions options);
diff --git a/src/main/java/org/springframework/data/redis/core/HashOperations.java b/src/main/java/org/springframework/data/redis/core/HashOperations.java
index ba3b5fdd7..6baff4644 100644
--- a/src/main/java/org/springframework/data/redis/core/HashOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/HashOperations.java
@@ -210,11 +210,12 @@ public interface HashOperations<H, HK, HV> {
 
 	/**
 	 * Use a {@link Cursor} to iterate over entries in hash at {@code key}. <br />
-	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leak.
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
 	 *
 	 * @param key must not be {@literal null}.
-	 * @param options
-	 * @return {@literal null} when used in pipeline / transaction.
+	 * @param options must not be {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
 	 * @since 1.4
 	 */
 	Cursor<Map.Entry<HK, HV>> scan(H key, ScanOptions options);
diff --git a/src/main/java/org/springframework/data/redis/core/RedisOperations.java b/src/main/java/org/springframework/data/redis/core/RedisOperations.java
index e9a153b56..20505059f 100644
--- a/src/main/java/org/springframework/data/redis/core/RedisOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/RedisOperations.java
@@ -262,10 +262,12 @@ public interface RedisOperations<K, V> {
 
 	/**
 	 * Use a {@link Cursor} to iterate over keys. <br />
-	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leak.
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
 	 *
 	 * @param options must not be {@literal null}.
-	 * @return never {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
+	 * @since 2.7
 	 * @see <a href="https://redis.io/commands/scan">Redis Documentation: SCAN</a>
 	 */
 	Cursor<K> scan(ScanOptions options);
diff --git a/src/main/java/org/springframework/data/redis/core/SetOperations.java b/src/main/java/org/springframework/data/redis/core/SetOperations.java
index 219228c2c..267518b1f 100644
--- a/src/main/java/org/springframework/data/redis/core/SetOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/SetOperations.java
@@ -372,12 +372,13 @@ public interface SetOperations<K, V> {
 	List<V> randomMembers(K key, long count);
 
 	/**
-	 * Iterate over elements in set at {@code key}. <br />
-	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leak.
+	 * Use a {@link Cursor} to iterate over entries set at {@code key}. <br />
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
 	 *
 	 * @param key
-	 * @param options
-	 * @return
+	 * @param options must not be {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
 	 * @since 1.4
 	 */
 	Cursor<V> scan(K key, ScanOptions options);
diff --git a/src/main/java/org/springframework/data/redis/core/ZSetOperations.java b/src/main/java/org/springframework/data/redis/core/ZSetOperations.java
index 383ab0c46..c237bd3c4 100644
--- a/src/main/java/org/springframework/data/redis/core/ZSetOperations.java
+++ b/src/main/java/org/springframework/data/redis/core/ZSetOperations.java
@@ -947,12 +947,13 @@ public interface ZSetOperations<K, V> {
 	Long unionAndStore(K key, Collection<K> otherKeys, K destKey, Aggregate aggregate, Weights weights);
 
 	/**
-	 * Iterate over elements in zset at {@code key}. <br />
-	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leak.
+	 * Use a {@link Cursor} to iterate over entries zset at {@code key}. <br />
+	 * <strong>Important:</strong> Call {@link Cursor#close()} when done to avoid resource leaks.
 	 *
 	 * @param key
-	 * @param options
-	 * @return {@literal null} when used in pipeline / transaction.
+	 * @param options must not be {@literal null}.
+	 * @return the result cursor providing access to the scan result. Must be closed once fully processed (e.g. through a
+	 *         try-with-resources clause).
 	 * @see <a href="https://redis.io/commands/zscan">Redis Documentation: ZSCAN</a>
 	 * @since 1.4
 	 */
diff --git a/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsIntegrationTests.java
index f7ce0daa6..ccd8e1641 100644
--- a/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsIntegrationTests.java
+++ b/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsIntegrationTests.java
@@ -131,17 +131,18 @@ public class DefaultHashOperationsIntegrationTests<K, HK, HV> {
 		hashOps.put(key, key1, val1);
 		hashOps.put(key, key2, val2);
 
-		Cursor<Map.Entry<HK, HV>> it = hashOps.scan(key, ScanOptions.scanOptions().count(1).build());
 
 		long count = 0;
-		while (it.hasNext()) {
-			Map.Entry<HK, HV> entry = it.next();
-			assertThat(entry.getKey()).isIn(key1, key2);
-			assertThat(entry.getValue()).isIn(val1, val2);
-			count++;
+		try (Cursor<Map.Entry<HK, HV>> it = hashOps.scan(key, ScanOptions.scanOptions().count(1).build())) {
+
+			while (it.hasNext()) {
+				Map.Entry<HK, HV> entry = it.next();
+				assertThat(entry.getKey()).isIn(key1, key2);
+				assertThat(entry.getValue()).isIn(val1, val2);
+				count++;
+			}
 		}
 
-		it.close();
 		assertThat(count).isEqualTo(hashOps.size(key));
 	}
 
diff --git a/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsIntegrationTests.java
index ade23b667..c78be6bd2 100644
--- a/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsIntegrationTests.java
+++ b/src/test/java/org/springframework/data/redis/core/DefaultSetOperationsIntegrationTests.java
@@ -208,13 +208,14 @@ public class DefaultSetOperationsIntegrationTests<K, V> {
 		V v3 = valueFactory.instance();
 
 		setOps.add(key, v1, v2, v3);
-		Cursor<V> it = setOps.scan(key, ScanOptions.scanOptions().count(1).build());
 		long count = 0;
-		while (it.hasNext()) {
-			assertThat(it.next()).isIn(v1, v2, v3);
-			count++;
+		try (Cursor<V> it = setOps.scan(key, ScanOptions.scanOptions().count(1).build())) {
+			while (it.hasNext()) {
+				assertThat(it.next()).isIn(v1, v2, v3);
+				count++;
+			}
 		}
-		it.close();
+
 		assertThat(count).isEqualTo(setOps.size(key));
 	}
 
diff --git a/src/test/java/org/springframework/data/redis/core/DefaultZSetOperationsIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/DefaultZSetOperationsIntegrationTests.java
index fa86155f3..93a036b7a 100644
--- a/src/test/java/org/springframework/data/redis/core/DefaultZSetOperationsIntegrationTests.java
+++ b/src/test/java/org/springframework/data/redis/core/DefaultZSetOperationsIntegrationTests.java
@@ -497,13 +497,13 @@ public class DefaultZSetOperationsIntegrationTests<K, V> {
 		zSetOps.add(key, values);
 
 		int count = 0;
-		Cursor<TypedTuple<V>> it = zSetOps.scan(key, ScanOptions.scanOptions().count(2).build());
-		while (it.hasNext()) {
-			assertThat(it.next()).isIn(tuple1, tuple2, tuple3);
-			count++;
+		try (Cursor<TypedTuple<V>> it = zSetOps.scan(key, ScanOptions.scanOptions().count(2).build())) {
+			while (it.hasNext()) {
+				assertThat(it.next()).isIn(tuple1, tuple2, tuple3);
+				count++;
+			}
 		}
 
-		it.close();
 		assertThat(count).isEqualTo(3);
 	}
 
diff --git a/src/test/java/org/springframework/data/redis/core/RedisClusterTemplateIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/RedisClusterTemplateIntegrationTests.java
index cb06472ca..5e21ff1c8 100644
--- a/src/test/java/org/springframework/data/redis/core/RedisClusterTemplateIntegrationTests.java
+++ b/src/test/java/org/springframework/data/redis/core/RedisClusterTemplateIntegrationTests.java
@@ -16,6 +16,7 @@
 package org.springframework.data.redis.core;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assumptions.*;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -141,6 +142,14 @@ public class RedisClusterTemplateIntegrationTests<K, V> extends RedisTemplateInt
 		super.testGetExpireMillisUsingPipelining();
 	}
 
+	@ParameterizedRedisTest
+	void testScan() {
+
+		// Only Lettuce supports cluster-wide scanning
+		assumeThat(redisTemplate.getConnectionFactory()).isInstanceOf(LettuceConnectionFactory.class);
+		super.testScan();
+	}
+
 	@Parameters
 	public static Collection<Object[]> testParams() {
 
diff --git a/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java
index 8f470920d..916761bc7 100644
--- a/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java
+++ b/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java
@@ -133,13 +133,13 @@ public class RedisTemplateIntegrationTests<K, V> {
 		V value1 = valueFactory.instance();
 		assumeThat(key1 instanceof String || key1 instanceof byte[]).isTrue();
 		redisTemplate.opsForValue().set(key1, value1);
-		Cursor<K> cursor = redisTemplate.scan(ScanOptions.scanOptions().count(1).build());
 		long count = 0;
-		while (cursor.hasNext()) {
-			assertThat(cursor.next()).isEqualTo(key1);
-			count++;
+		try (Cursor<K> cursor = redisTemplate.scan(ScanOptions.scanOptions().count(1).build())) {
+			while (cursor.hasNext()) {
+				assertThat(cursor.next()).isEqualTo(key1);
+				count++;
+			}
 		}
-		cursor.close();
 		assertThat(count).isEqualTo(1);
 	}