Adding command BLMove (#324)

* Adding command BLMove --------- Co-authored-by: TJ Zhang <tj.zhang@improving.com>
Bit-Quill · May 30, 2024 · 141ede7 · 141ede7
1 parent f83e519
commit 141ede7
Show file tree

Hide file tree

Showing 10 changed files with 238 additions and 1 deletion.
diff --git a/glide-core/src/protobuf/redis_request.proto b/glide-core/src/protobuf/redis_request.proto
@@ -198,6 +198,7 @@ enum RequestType {
     LSet = 165;
     XDel = 166;
     LMove = 168;
+    BLMove = 169;
 }
 
 message Command {

diff --git a/glide-core/src/request_type.rs b/glide-core/src/request_type.rs
@@ -168,6 +168,7 @@ pub enum RequestType {
     LSet = 165,
     XDel = 166,
     LMove = 168,
+    BLMove = 169,
 }
 
 fn get_two_word_command(first: &str, second: &str) -> Cmd {
@@ -339,6 +340,7 @@ impl From<::protobuf::EnumOrUnknown<ProtobufRequestType>> for RequestType {
             ProtobufRequestType::LSet => RequestType::LSet,
             ProtobufRequestType::XDel => RequestType::XDel,
             ProtobufRequestType::LMove => RequestType::LMove,
+            ProtobufRequestType::BLMove => RequestType::BLMove,
         }
     }
 }
@@ -506,6 +508,7 @@ impl RequestType {
             RequestType::LSet => Some(cmd("LSET")),
             RequestType::XDel => Some(cmd("XDEL")),
             RequestType::LMove => Some(cmd("LMOVE")),
+            RequestType::BLMove => Some(cmd("BLMOVE")),
         }
     }
 }
diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java
@@ -11,6 +11,7 @@
 import static glide.utils.ArrayTransformUtils.mapGeoDataToArray;
 import static redis_request.RedisRequestOuterClass.RequestType.Append;
 import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
+import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
 import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
@@ -1582,4 +1583,18 @@ public CompletableFuture<String> lmove(
                 new String[] {source, destination, wherefrom.toString(), whereto.toString()};
         return commandManager.submitNewCommand(LMove, arguments, this::handleStringOrNullResponse);
     }
+
+    @Override
+    public CompletableFuture<String> blmove(
+            @NonNull String source,
+            @NonNull String destination,
+            @NonNull ListDirection wherefrom,
+            @NonNull ListDirection whereto,
+            double timeout) {
+        String[] arguments =
+                new String[] {
+                    source, destination, wherefrom.toString(), whereto.toString(), Double.toString(timeout)
+                };
+        return commandManager.submitNewCommand(BLMove, arguments, this::handleStringOrNullResponse);
+    }
 }
diff --git a/java/client/src/main/java/glide/api/commands/ListBaseCommands.java b/java/client/src/main/java/glide/api/commands/ListBaseCommands.java
@@ -531,4 +531,49 @@ CompletableFuture<Map<String, String[]>> blmpop(
      */
     CompletableFuture<String> lmove(
             String source, String destination, ListDirection wherefrom, ListDirection whereto);
+
+    /**
+     * Blocks the connection until it pops atomically and removes the left/right-most element to the
+     * list stored at <code>source</code> depending on <code>wherefrom</code>, and pushes the element
+     * at the first/last element of the list stored at <code>destination</code> depending on <code>
+     * wherefrom</code>.<br>
+     * <code>BLMove</code> is the blocking variant of {@link #lmove(String, String, ListDirection,
+     * ListDirection)}.
+     *
+     * @since Redis 6.2.0 and above.
+     * @apiNote
+     *     <ol>
+     *       <li>When in cluster mode, all <code>source</code> and <code>destination</code> must map
+     *           to the same hash slot.
+     *       <li><code>BLMove</code> is a client blocking command, see <a
+     *           href="https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands">Blocking
+     *           Commands</a> for more details and best practices.
+     *     </ol>
+     *
+     * @see <a href="https://valkey.io/commands/blmove/">valkey.io</a> for details.
+     * @param source The key to the source list.
+     * @param destination The key to the destination list.
+     * @param wherefrom The {@link ListDirection} the element should be removed from.
+     * @param whereto The {@link ListDirection} the element should be added to.
+     * @param timeout The number of seconds to wait for a blocking operation to complete. A value of
+     *     <code>0</code> will block indefinitely.
+     * @return The popped element or <code>null</code> if <code>source</code> does not exist.
+     * @example
+     *     <pre>{@code
+     * client.lpush("testKey1", new String[] {"two", "one"}).get();
+     * client.lpush("testKey2", new String[] {"four", "three"}).get();
+     * var result = client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1).get();
+     * assertEquals(result, "one");
+     * String[] upratedArray1 = client.lrange("testKey1", 0, -1).get();
+     * String[] upratedArray2 = client.lrange("testKey2", 0, -1).get();
+     * assertArrayEquals(new String[] {"two"}, updatedArray1);
+     * assertArrayEquals(new String[] {"one", "three", "four"}, updatedArray2);
+     * }</pre>
+     */
+    CompletableFuture<String> blmove(
+            String source,
+            String destination,
+            ListDirection wherefrom,
+            ListDirection whereto,
+            double timeout);
 }
diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java
@@ -15,6 +15,7 @@
 import static glide.utils.ArrayTransformUtils.mapGeoDataToArray;
 import static redis_request.RedisRequestOuterClass.RequestType.Append;
 import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
+import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
 import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
@@ -3832,6 +3833,45 @@ public T lmove(
         return getThis();
     }
 
+    /**
+     * Blocks the connection until it atomically pops and removes the left/right-most element to the
+     * list stored at <code>source</code> depending on <code>wherefrom</code>, and pushes the element
+     * at the first/last element of the list stored at <code>destination</code> depending on <code>
+     * wherefrom</code>.<br>
+     * <code>BLMove</code> is the blocking variant of {@link #lmove(String, String, ListDirection,
+     * ListDirection)}.
+     *
+     * @since Redis 6.2.0 and above.
+     * @apiNote <code>BLMove</code> is a client blocking command, see <a
+     *     href="https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands">Blocking
+     *     Commands</a> for more details and best practices.
+     * @see <a href="https://valkey.io/commands/blmove/">valkey.io</a> for details.
+     * @param source The key to the source list.
+     * @param destination The key to the destination list.
+     * @param wherefrom The {@link ListDirection} the element should be removed from.
+     * @param whereto The {@link ListDirection} the element should be added to.
+     * @param timeout The number of seconds to wait for a blocking operation to complete. A value of
+     *     <code>0</code> will block indefinitely.
+     * @return Command Response - The popped element or <code>null</code> if <code>source</code> does
+     *     not exist.
+     */
+    public T blmove(
+            @NonNull String source,
+            @NonNull String destination,
+            @NonNull ListDirection wherefrom,
+            @NonNull ListDirection whereto,
+            double timeout) {
+        ArgsArray commandArgs =
+                buildArgs(
+                        source,
+                        destination,
+                        wherefrom.toString(),
+                        whereto.toString(),
+                        Double.toString(timeout));
+        protobufTransaction.addCommands(buildCommand(BLMove, commandArgs));
+        return getThis();
+    }
+
     /** Build protobuf {@link Command} object for given command and arguments. */
     protected Command buildCommand(RequestType requestType) {
         return buildCommand(requestType, buildArgs());

diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java
@@ -36,6 +36,7 @@
 import static org.mockito.Mockito.when;
 import static redis_request.RedisRequestOuterClass.RequestType.Append;
 import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
+import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
 import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
@@ -5229,4 +5230,31 @@ public void lset_returns_success() {
         assertEquals(testResponse, response);
         assertEquals(OK, payload);
     }
+
+    @SneakyThrows
+    @Test
+    public void blmove_returns_success() {
+        // setup
+        String key1 = "testKey";
+        String key2 = "testKey2";
+        ListDirection wherefrom = ListDirection.LEFT;
+        ListDirection whereto = ListDirection.RIGHT;
+        String[] arguments = new String[] {key1, key2, wherefrom.toString(), whereto.toString(), "0.1"};
+        String value = "one";
+
+        CompletableFuture<String> testResponse = new CompletableFuture<>();
+        testResponse.complete(value);
+
+        // match on protobuf request
+        when(commandManager.<String>submitNewCommand(eq(BLMove), eq(arguments), any()))
+                .thenReturn(testResponse);
+
+        // exercise
+        CompletableFuture<String> response = service.blmove(key1, key2, wherefrom, whereto, 0.1);
+        String payload = response.get();
+
+        // verify
+        assertEquals(testResponse, response);
+        assertEquals(value, payload);
+    }
 }
diff --git a/java/client/src/test/java/glide/api/models/TransactionTests.java b/java/client/src/test/java/glide/api/models/TransactionTests.java
@@ -26,6 +26,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static redis_request.RedisRequestOuterClass.RequestType.Append;
 import static redis_request.RedisRequestOuterClass.RequestType.BLMPop;
+import static redis_request.RedisRequestOuterClass.RequestType.BLMove;
 import static redis_request.RedisRequestOuterClass.RequestType.BLPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BRPop;
 import static redis_request.RedisRequestOuterClass.RequestType.BZMPop;
@@ -873,6 +874,9 @@ InfScoreBound.NEGATIVE_INFINITY, new ScoreBoundary(3, false), new Limit(1, 2)),
         transaction.lmove("key1", "key2", ListDirection.LEFT, ListDirection.LEFT);
         results.add(Pair.of(LMove, buildArgs("key1", "key2", "LEFT", "LEFT")));
 
+        transaction.blmove("key1", "key2", ListDirection.LEFT, ListDirection.LEFT, 0.1);
+        results.add(Pair.of(BLMove, buildArgs("key1", "key2", "LEFT", "LEFT", "0.1")));
+
         var protobufTransaction = transaction.getProtobufTransaction().build();
 
         for (int idx = 0; idx < protobufTransaction.getCommandsCount(); idx++) {

diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java
@@ -4325,4 +4325,93 @@ public void lmove(BaseClient client) {
                         () -> client.lmove(key1, nonListKey, ListDirection.LEFT, ListDirection.LEFT).get());
         assertInstanceOf(RequestException.class, executionException2.getCause());
     }
+
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void blmove(BaseClient client) {
+        assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("6.2.0"), "This feature added in redis 6.2.0");
+        // setup
+        String key1 = "{key}-1" + UUID.randomUUID();
+        String key2 = "{key}-2" + UUID.randomUUID();
+        String nonExistingKey = "{key}-3" + UUID.randomUUID();
+        String nonListKey = "{key}-4" + UUID.randomUUID();
+        String[] lpushArgs1 = {"four", "three", "two", "one"};
+        String[] lpushArgs2 = {"six", "five", "four"};
+        double timeout = 1;
+
+        // source does not exist or is empty
+        assertNull(client.blmove(key1, key2, ListDirection.LEFT, ListDirection.RIGHT, timeout).get());
+
+        // only source exists, only source elements gets popped, creates a list at nonExistingKey
+        assertEquals(lpushArgs1.length, client.lpush(key1, lpushArgs1).get());
+        assertEquals(
+                "four",
+                client
+                        .blmove(key1, nonExistingKey, ListDirection.RIGHT, ListDirection.LEFT, timeout)
+                        .get());
+        assertArrayEquals(new String[] {"one", "two", "three"}, client.lrange(key1, 0, -1).get());
+
+        // source and destination are the same, performing list rotation, "three" gets popped and added
+        // back
+        assertEquals(
+                "one", client.blmove(key1, key1, ListDirection.LEFT, ListDirection.LEFT, timeout).get());
+        assertArrayEquals(new String[] {"one", "two", "three"}, client.lrange(key1, 0, -1).get());
+
+        // normal use case, "three" gets popped and added to the left of destination
+        assertEquals(lpushArgs2.length, client.lpush(key2, lpushArgs2).get());
+        assertEquals(
+                "three", client.blmove(key1, key2, ListDirection.RIGHT, ListDirection.LEFT, timeout).get());
+        assertArrayEquals(new String[] {"one", "two"}, client.lrange(key1, 0, -1).get());
+        assertArrayEquals(
+                new String[] {"three", "four", "five", "six"}, client.lrange(key2, 0, -1).get());
+
+        // source exists but is not a list type key
+        assertEquals(OK, client.set(nonListKey, "NotAList").get());
+        ExecutionException executionException =
+                assertThrows(
+                        ExecutionException.class,
+                        () ->
+                                client
+                                        .blmove(nonListKey, key1, ListDirection.LEFT, ListDirection.LEFT, timeout)
+                                        .get());
+        assertInstanceOf(RequestException.class, executionException.getCause());
+
+        // destination exists but is not a list type key
+        ExecutionException executionException2 =
+                assertThrows(
+                        ExecutionException.class,
+                        () ->
+                                client
+                                        .blmove(key1, nonListKey, ListDirection.LEFT, ListDirection.LEFT, timeout)
+                                        .get());
+        assertInstanceOf(RequestException.class, executionException2.getCause());
+    }
+
+    @SneakyThrows
+    @ParameterizedTest(autoCloseArguments = false)
+    @MethodSource("getClients")
+    public void blmove_timeout_check(BaseClient client) {
+        assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("6.2.0"), "This feature added in redis 6.2.0");
+        String key1 = "{key}-1" + UUID.randomUUID();
+        String key2 = "{key}-2" + UUID.randomUUID();
+        // create new client with default request timeout (250 millis)
+        try (var testClient =
+                client instanceof RedisClient
+                        ? RedisClient.CreateClient(commonClientConfig().build()).get()
+                        : RedisClusterClient.CreateClient(commonClusterClientConfig().build()).get()) {
+
+            // ensure that commands doesn't time out even if timeout > request timeout
+            assertNull(testClient.blmove(key1, key2, ListDirection.LEFT, ListDirection.LEFT, 1).get());
+
+            // with 0 timeout (no timeout) should never time out,
+            // but we wrap the test with timeout to avoid test failing or stuck forever
+            assertThrows(
+                    TimeoutException.class, // <- future timeout, not command timeout
+                    () ->
+                            testClient
+                                    .blmove(key1, key2, ListDirection.LEFT, ListDirection.LEFT, 0)
+                                    .get(3, TimeUnit.SECONDS));
+        }
+    }
 }
diff --git a/java/integTest/src/test/java/glide/TransactionTestUtilities.java b/java/integTest/src/test/java/glide/TransactionTestUtilities.java
@@ -294,6 +294,10 @@ private static Object[] listCommands(BaseTransaction<?> transaction) {
                     .lmove(listKey7, listKey7, ListDirection.LEFT, ListDirection.LEFT)
                     .lmove(listKey6, listKey7, ListDirection.LEFT, ListDirection.RIGHT)
                     .lrange(listKey6, 0, -1)
+                    .lrange(listKey7, 0, -1)
+                    .blmove(listKey7, listKey7, ListDirection.LEFT, ListDirection.LEFT, 0.1)
+                    .blmove(listKey7, listKey6, ListDirection.RIGHT, ListDirection.LEFT, 0.1)
+                    .lrange(listKey6, 0, -1)
                     .lrange(listKey7, 0, -1);
         }
 
@@ -345,6 +349,10 @@ private static Object[] listCommands(BaseTransaction<?> transaction) {
                                 value1, // lmove(listKey6, listKey5, RIGHT, LEFT)
                                 new String[] {value2, value3}, // lrange(listKey6, 0, -1)
                                 new String[] {value3, value2, value1, value1}, // lrange(listKey7, 0, -1);
+                                value3, // blmove(listKey7, listKey7, LEFT, LEFT, 0.1)
+                                value1, // blmove(listKey7, listKey6, RIGHT, LEFT, 0.1)
+                                new String[] {value1, value2, value3}, // lrange(listKey6, 0, -1)
+                                new String[] {value3, value2, value1}, // lrange(listKey7, 0, -1)
                             });
         }
 

diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java
@@ -710,7 +710,11 @@ public static Stream<Arguments> callCrossSlotCommandsWhichShouldFail() {
                 Arguments.of(
                         "lmove",
                         "6.2.0",
-                        clusterClient.lmove("abc", "def", ListDirection.LEFT, ListDirection.LEFT)));
+                        clusterClient.lmove("abc", "def", ListDirection.LEFT, ListDirection.LEFT)),
+                Arguments.of(
+                        "blmove",
+                        "6.2.0",
+                        clusterClient.blmove("abc", "def", ListDirection.LEFT, ListDirection.LEFT, 1)));
     }
 
     @SneakyThrows