Fix Cluster sort.

Jedis Cluster sort now considers if the destination key is sharing the same slot as the source key to use same-slot sorting.

Additionally, sort results that do not map to the same slot replace the destination key with a list instead of checking the key type and appending results.

Closes #2341

Author: mp911de

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

@@ -47,7 +47,6 @@
 import org.springframework.data.redis.core.ScanOptions;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 import org.springframework.util.ObjectUtils;
 
 /**
@@ -541,23 +540,19 @@ public Long sort(byte[] key, SortParameters params, byte[] storeKey) {
 
 		Assert.notNull(key, "Key must not be null!");
 
-		List<byte[]> sorted = sort(key, params);
-		if (!CollectionUtils.isEmpty(sorted)) {
-
-			byte[][] arr = new byte[sorted.size()][];
-			switch (type(key)) {
-
-				case SET:
-					connection.setCommands().sAdd(storeKey, sorted.toArray(arr));
-					return 1L;
-				case LIST:
-					connection.listCommands().lPush(storeKey, sorted.toArray(arr));
-					return 1L;
-				default:
-					throw new IllegalArgumentException("sort and store is only supported for SET and LIST");
+		if (ClusterSlotHashUtil.isSameSlotForAllKeys(key, storeKey)) {
+			try {
+				return connection.getCluster().sort(key, JedisConverters.toSortingParams(params), storeKey);
+			} catch (Exception ex) {
+				throw convertJedisAccessException(ex);
 			}
 		}
-		return 0L;
+
+		List<byte[]> sorted = sort(key, params);
+		byte[][] arr = new byte[sorted.size()][];
+		connection.keyCommands().unlink(storeKey);
+		connection.listCommands().lPush(storeKey, sorted.toArray(arr));
+		return (long) sorted.size();
 	}
 
 	/*

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClusterKeyCommands.java

@@ -33,7 +33,6 @@
 import org.springframework.data.redis.core.ScanOptions;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 
 /**
  * @author Christoph Strobl
@@ -229,21 +228,9 @@ public Long sort(byte[] key, SortParameters params, byte[] storeKey) {
 		}
 
 		List<byte[]> sorted = sort(key, params);
-		if (!CollectionUtils.isEmpty(sorted)) {
-
-			byte[][] arr = new byte[sorted.size()][];
-			switch (type(key)) {
-
-				case SET:
-					connection.setCommands().sAdd(storeKey, sorted.toArray(arr));
-					return 1L;
-				case LIST:
-					connection.listCommands().lPush(storeKey, sorted.toArray(arr));
-					return 1L;
-				default:
-					throw new IllegalArgumentException("sort and store is only supported for SET and LIST");
-			}
-		}
-		return 0L;
+		byte[][] arr = new byte[sorted.size()][];
+		connection.keyCommands().unlink(storeKey);
+		connection.listCommands().lPush(storeKey, sorted.toArray(arr));
+		return (long) sorted.size();
 	}
 }

src/test/java/org/springframework/data/redis/connection/ClusterConnectionTests.java

@@ -568,8 +568,8 @@ public interface ClusterConnectionTests {
 	// DATAREDIS-315
 	void sortAndStoreShouldAddSortedValuesValuesCorrectly();
 
-	// DATAREDIS-315
-	void sortAndStoreShouldReturnZeroWhenListDoesNotExist();
+	// DATAREDIS-315, GH-2341
+	void sortAndStoreShouldReplaceDestinationList();
 
 	// DATAREDIS-315
 	void sortShouldReturnValuesCorrectly();

src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java

@@ -1906,13 +1906,18 @@ public void sortAndStoreShouldAddSortedValuesValuesCorrectly() {
 
 		nativeConnection.lpush(KEY_1, VALUE_2, VALUE_1);
 
-		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(1L);
+		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(2L);
 		assertThat(nativeConnection.exists(KEY_2_BYTES)).isTrue();
 	}
 
-	@Test // DATAREDIS-315
-	public void sortAndStoreShouldReturnZeroWhenListDoesNotExist() {
-		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(0L);
+	@Test // DATAREDIS-315, GH-2341
+	public void sortAndStoreShouldReplaceDestinationList() {
+
+		nativeConnection.lpush(KEY_1, VALUE_2, VALUE_1);
+		nativeConnection.lpush(KEY_2_BYTES, VALUE_3_BYTES);
+
+		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(2L);
+		assertThat(nativeConnection.llen(KEY_2_BYTES)).isEqualTo(2);
 	}
 
 	@Test // DATAREDIS-315

src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java

@@ -1937,13 +1937,18 @@ public void sortAndStoreShouldAddSortedValuesValuesCorrectly() {
 
 		nativeConnection.lpush(KEY_1, VALUE_2, VALUE_1);
 
-		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(1L);
+		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(2L);
 		assertThat(nativeConnection.exists(KEY_2)).isEqualTo(1L);
 	}
 
-	@Test // DATAREDIS-315
-	public void sortAndStoreShouldReturnZeroWhenListDoesNotExist() {
-		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(0L);
+	@Test // DATAREDIS-315, GH-2341
+	public void sortAndStoreShouldReplaceDestinationList() {
+
+		nativeConnection.lpush(KEY_1, VALUE_2, VALUE_1);
+		nativeConnection.lpush(KEY_2, VALUE_3);
+
+		assertThat(clusterConnection.sort(KEY_1_BYTES, new DefaultSortParameters().alpha(), KEY_2_BYTES)).isEqualTo(2L);
+		assertThat(nativeConnection.llen(KEY_2)).isEqualTo(2);
 	}
 
 	@Test // DATAREDIS-315