From f91b0c7ba86d71390f6aa8c7df4807ceb2a7b518 Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 28 Jun 2024 01:16:30 -0700 Subject: [PATCH] Change sscan cursor to be a String Also fix bug in SharedCommandTests --- .../src/main/java/glide/api/BaseClient.java | 9 ++-- .../glide/api/commands/SetBaseCommands.java | 18 ++++---- .../glide/api/models/BaseTransaction.java | 11 +++-- .../models/commands/scan/SScanOptions.java | 4 +- .../test/java/glide/api/RedisClientTest.java | 10 ++--- .../glide/api/models/TransactionTests.java | 4 +- .../test/java/glide/SharedCommandTests.java | 43 ++++++++----------- .../java/glide/TransactionTestUtilities.java | 8 ++-- 8 files changed, 49 insertions(+), 58 deletions(-) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index c82905fb97..34c23e7ba9 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -2788,16 +2788,15 @@ public CompletableFuture sortStore(@NonNull String key, @NonNull String de } @Override - public CompletableFuture sscan(@NonNull String key, long cursor) { - String[] arguments = new String[] {key, Long.toString(cursor)}; + public CompletableFuture sscan(@NonNull String key, String cursor) { + String[] arguments = new String[] {key, cursor}; return commandManager.submitNewCommand(SScan, arguments, this::handleArrayResponse); } @Override public CompletableFuture sscan( - @NonNull String key, long cursor, @NonNull SScanOptions sscanOptions) { - String[] arguments = - concatenateArrays(new String[] {key, Long.toString(cursor)}, sscanOptions.toArgs()); + @NonNull String key, String cursor, @NonNull SScanOptions sScanOptions) { + String[] arguments = concatenateArrays(new String[] {key, cursor}, sScanOptions.toArgs()); return commandManager.submitNewCommand(SScan, arguments, this::handleArrayResponse); } } diff --git a/java/client/src/main/java/glide/api/commands/SetBaseCommands.java b/java/client/src/main/java/glide/api/commands/SetBaseCommands.java index ea373661de..bc8e137257 100644 --- a/java/client/src/main/java/glide/api/commands/SetBaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/SetBaseCommands.java @@ -568,19 +568,19 @@ public interface SetBaseCommands { * @example * 
{@code
      * // Assume key contains a set with 200 members
-     * long cursor = 0;
+     * String cursor = "0";
      * Object[] result;
      * do {
      *   result = client.sscan(key1, cursor).get();
-     *   cursor = Long.valueOf(result[0].toString());
+     *   cursor = result[0].toString();
      *   Object[] stringResults = (Object[]) result[1];
      *
      *   System.out.println("\nSSCAN iteration:");
      *   Arrays.asList(stringResults).stream().forEach(i -> System.out.print(i + ", "));
-     * } while (cursor != 0);
+     * } while (!cursor.equals("0"));
      * }
*/ - CompletableFuture sscan(String key, long cursor); + CompletableFuture sscan(String key, String cursor); /** * Iterates incrementally over a set. @@ -588,7 +588,7 @@ public interface SetBaseCommands { * @see valkey.io for details. * @param key The key of the set. * @param cursor The cursor that points to the next iteration of results. - * @param sscanOptions The {@link SScanOptions}. + * @param sScanOptions The {@link SScanOptions}. * @return An Array of Objects. The first element is always the * cursor for the next iteration of results. 0 will be the cursor * returned on the last iteration of the set. The second element is always an @@ -596,17 +596,17 @@ public interface SetBaseCommands { * @example * 
{@code
      * // Assume key contains a set with 200 members
-     * long cursor = 0;
+     * String cursor = "0";
      * Object[] result;
      * do {
      *   result = client.sscan(key1, cursor, SScanOptions.builder().matchPattern("*").count(20L).build()).get();
-     *   cursor = Long.valueOf(result[0].toString());
+     *   cursor = result[0].toString();
      *   Object[] stringResults = (Object[]) result[1];
      *
      *   System.out.println("\nSSCAN iteration:");
      *   Arrays.asList(stringResults).stream().forEach(i -> System.out.print(i + ", "));
-     * } while (cursor != 0);
+     * } while (!cursor.equals("0"));
      * }
*/ - CompletableFuture sscan(String key, long cursor, SScanOptions sscanOptions); + CompletableFuture sscan(String key, String cursor, SScanOptions sScanOptions); } diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java index 737fdb56f5..49f033698b 100644 --- a/java/client/src/main/java/glide/api/models/BaseTransaction.java +++ b/java/client/src/main/java/glide/api/models/BaseTransaction.java @@ -5139,8 +5139,8 @@ public T sortStore(@NonNull String key, @NonNull String destination) { * the cursor returned on the last iteration of the set. The second element is * always an Array of the subset of the set held in key. */ - public T sscan(@NonNull String key, long cursor) { - protobufTransaction.addCommands(buildCommand(SScan, buildArgs(key, Long.toString(cursor)))); + public T sscan(@NonNull String key, String cursor) { + protobufTransaction.addCommands(buildCommand(SScan, buildArgs(key, cursor))); return getThis(); } @@ -5150,16 +5150,15 @@ public T sscan(@NonNull String key, long cursor) { * @see valkey.io for details. * @param key The key of the set. * @param cursor The cursor that points to the next iteration of results. - * @param sscanOptions The {@link SScanOptions}. + * @param sScanOptions The {@link SScanOptions}. * @return Command Response - An Array of Objects. The first element is * always the cursor for the next iteration of results. 0 will be * the cursor returned on the last iteration of the set. The second element is * always an Array of the subset of the set held in key. */ - public T sscan(@NonNull String key, long cursor, @NonNull SScanOptions sscanOptions) { + public T sscan(@NonNull String key, String cursor, @NonNull SScanOptions sScanOptions) { ArgsArray commandArgs = - buildArgs( - concatenateArrays(new String[] {key, Long.toString(cursor)}, sscanOptions.toArgs())); + buildArgs(concatenateArrays(new String[] {key, cursor}, sScanOptions.toArgs())); protobufTransaction.addCommands(buildCommand(SScan, commandArgs)); return getThis(); } diff --git a/java/client/src/main/java/glide/api/models/commands/scan/SScanOptions.java b/java/client/src/main/java/glide/api/models/commands/scan/SScanOptions.java index 9c81999b4c..9a3f123545 100644 --- a/java/client/src/main/java/glide/api/models/commands/scan/SScanOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/scan/SScanOptions.java @@ -5,8 +5,8 @@ import lombok.experimental.SuperBuilder; /** - * Optional arguments for {@link SetBaseCommands#sscan(String, long)}, {@link - * SetBaseCommands#sscan(String, long, SScanOptions)}. + * Optional arguments for {@link SetBaseCommands#sscan(String, String)}, {@link + * SetBaseCommands#sscan(String, String, SScanOptions)}. * * @see valkey.io */ diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index 040b5365e5..8a0c18917b 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -8888,8 +8888,8 @@ public void sort_with_options_returns_success() { public void sscan_returns_success() { // setup String key = "testKey"; - long cursor = 0; - String[] arguments = new String[] {key, Long.toString(cursor)}; + String cursor = "0"; + String[] arguments = new String[] {key, cursor}; Object[] value = new Object[] {0L, new String[] {"hello", "world"}}; CompletableFuture testResponse = new CompletableFuture<>(); @@ -8992,11 +8992,9 @@ public void sortStore_with_options_returns_success() { public void sscan_with_options_returns_success() { // setup String key = "testKey"; - long cursor = 0; + String cursor = "0"; String[] arguments = - new String[] { - key, Long.toString(cursor), MATCH_OPTION_STRING, "*", COUNT_OPTION_STRING, "1" - }; + new String[] {key, cursor, MATCH_OPTION_STRING, "*", COUNT_OPTION_STRING, "1"}; Object[] value = new Object[] {0L, new String[] {"hello", "world"}}; CompletableFuture testResponse = new CompletableFuture<>(); diff --git a/java/client/src/test/java/glide/api/models/TransactionTests.java b/java/client/src/test/java/glide/api/models/TransactionTests.java index e2debeac2e..62e17215bf 100644 --- a/java/client/src/test/java/glide/api/models/TransactionTests.java +++ b/java/client/src/test/java/glide/api/models/TransactionTests.java @@ -1170,10 +1170,10 @@ InfScoreBound.NEGATIVE_INFINITY, new ScoreBoundary(3, false), new Limit(1, 2)), transaction.sortStore("key1", "key2"); results.add(Pair.of(Sort, buildArgs("key1", STORE_COMMAND_STRING, "key2"))); - transaction.sscan("key1", 0); + transaction.sscan("key1", "0"); results.add(Pair.of(SScan, buildArgs("key1", "0"))); - transaction.sscan("key1", 0, SScanOptions.builder().matchPattern("*").count(10L).build()); + transaction.sscan("key1", "0", SScanOptions.builder().matchPattern("*").count(10L).build()); results.add(Pair.of(SScan, buildArgs("key1", "0", "MATCH", "*", "COUNT", "10"))); var protobufTransaction = transaction.getProtobufTransaction().build(); diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java index 2f82a6044a..6f7c738833 100644 --- a/java/integTest/src/test/java/glide/SharedCommandTests.java +++ b/java/integTest/src/test/java/glide/SharedCommandTests.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -7018,7 +7019,7 @@ public void lcsIdx(BaseClient client) { public void sscan(BaseClient client) { String key1 = "{key}-1" + UUID.randomUUID(); String key2 = "{key}-2" + UUID.randomUUID(); - long initialCursor = 0; + String initialCursor = "0"; long defaultCount = 10; String[] numberMembers = new String[125]; for (int i = 0; i < numberMembers.length; i++) { @@ -7032,24 +7033,18 @@ public void sscan(BaseClient client) { // Empty set Object[] result = client.sscan(key1, initialCursor).get(); - assertEquals(String.valueOf(initialCursor), result[resultCursorIndex]); + assertEquals(initialCursor, result[resultCursorIndex]); assertDeepEquals(new String[] {}, result[resultCollectionIndex]); // Negative cursor - result = client.sscan(key1, -1).get(); - assertEquals(String.valueOf(initialCursor), result[resultCursorIndex]); + result = client.sscan(key1, "-1").get(); + assertEquals(initialCursor, result[resultCursorIndex]); assertDeepEquals(new String[] {}, result[resultCollectionIndex]); // Result contains the whole set assertEquals(charMembers.length, client.sadd(key1, charMembers).get()); - // Sleep after sadd() for eventual consistency. - // TODO: Replace sleep with WAIT request to enforce strong consistency. - try { - Thread.sleep(1000); - } catch (InterruptedException ex) { - } result = client.sscan(key1, initialCursor).get(); - assertEquals(String.valueOf(initialCursor), result[resultCursorIndex]); + assertEquals(initialCursor, result[resultCursorIndex]); assertEquals(charMembers.length, ((Object[]) result[resultCollectionIndex]).length); final Set resultMembers = Arrays.stream((Object[]) result[resultCollectionIndex]).collect(Collectors.toSet()); @@ -7059,27 +7054,27 @@ public void sscan(BaseClient client) { result = client.sscan(key1, initialCursor, SScanOptions.builder().matchPattern("a").build()).get(); - assertEquals(String.valueOf(initialCursor), result[resultCursorIndex]); + assertEquals(initialCursor, result[resultCursorIndex]); assertDeepEquals(new String[] {"a"}, result[resultCollectionIndex]); // Result contains a subset of the key assertEquals(numberMembers.length, client.sadd(key1, numberMembers).get()); - // Sleep after sadd() for eventual consistency. - // TODO: Replace sleep with WAIT request to enforce strong consistency. - try { - Thread.sleep(1000); - } catch (InterruptedException ex) { - } - long resultCursor = 0; + String resultCursor = "0"; final Set secondResultValues = new HashSet<>(); do { result = client.sscan(key1, resultCursor).get(); - resultCursor = Long.parseLong(result[resultCursorIndex].toString()); + resultCursor = result[resultCursorIndex].toString(); + secondResultValues.addAll( + Arrays.stream((Object[]) result[resultCollectionIndex]).collect(Collectors.toSet())); + + if (resultCursor.equals("0")) { + break; + } // Scan with result cursor has a different set Object[] secondResult = client.sscan(key1, resultCursor).get(); - long newResultCursor = (Long.parseLong(secondResult[resultCursorIndex].toString())); - assertTrue(resultCursor != newResultCursor); + String newResultCursor = secondResult[resultCursorIndex].toString(); + assertNotEquals(resultCursor, newResultCursor); resultCursor = newResultCursor; assertFalse( Arrays.deepEquals( @@ -7088,7 +7083,7 @@ public void sscan(BaseClient client) { secondResultValues.addAll( Arrays.stream((Object[]) secondResult[resultCollectionIndex]) .collect(Collectors.toSet())); - } while (resultCursor != 0); // 0 is returned for the cursor of the last iteration. + } while (!resultCursor.equals("0")); // 0 is returned for the cursor of the last iteration. assertTrue( secondResultValues.containsAll(numberMembersSet), @@ -7139,7 +7134,7 @@ public void sscan(BaseClient client) { executionException = assertThrows( ExecutionException.class, - () -> client.sscan(key1, -1, SScanOptions.builder().count(-1L).build()).get()); + () -> client.sscan(key1, "-1", SScanOptions.builder().count(-1L).build()).get()); assertInstanceOf(RequestException.class, executionException.getCause()); } } diff --git a/java/integTest/src/test/java/glide/TransactionTestUtilities.java b/java/integTest/src/test/java/glide/TransactionTestUtilities.java index 4f2373a9d3..8e4813562b 100644 --- a/java/integTest/src/test/java/glide/TransactionTestUtilities.java +++ b/java/integTest/src/test/java/glide/TransactionTestUtilities.java @@ -521,8 +521,8 @@ private static Object[] setCommands(BaseTransaction transaction) { transaction .sadd(setKey1, new String[] {"baz", "foo"}) .srem(setKey1, new String[] {"foo"}) - .sscan(setKey1, 0) - .sscan(setKey1, 0, SScanOptions.builder().matchPattern("*").count(10L).build()) + .sscan(setKey1, "0") + .sscan(setKey1, "0", SScanOptions.builder().matchPattern("*").count(10L).build()) .scard(setKey1) .sismember(setKey1, "baz") .smembers(setKey1) @@ -554,8 +554,8 @@ private static Object[] setCommands(BaseTransaction transaction) { new Object[] { 2L, // sadd(setKey1, new String[] {"baz", "foo"}); 1L, // srem(setKey1, new String[] {"foo"}); - new Object[] {"0", new String[] {"baz"}}, // sscan(setKey1, 0) - new Object[] {"0", new String[] {"baz"}}, // sscan(key1, 0, match "*", count(10L)) + new Object[] {"0", new String[] {"baz"}}, // sscan(setKey1, "0") + new Object[] {"0", new String[] {"baz"}}, // sscan(key1, "0", match "*", count(10L)) 1L, // scard(setKey1); true, // sismember(setKey1, "baz") Set.of("baz"), // smembers(setKey1);