Fix custom command cluster & transaction implementation. (#100) (valk…

…ey-io#985) * Fix custom command cluter & transaction implementation. Signed-off-by: Yury-Fridlyand <[email protected]>
Bit-Quill · Feb 20, 2024 · a35ed7d · a35ed7d
1 parent cda685a
commit a35ed7d
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 25 deletions.
diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java
@@ -16,10 +16,10 @@
 import glide.managers.CommandManager;
 import glide.managers.ConnectionManager;
 import java.util.Arrays;
-import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import lombok.NonNull;
+import response.ResponseOuterClass.Response;
 
 /**
  * Async (non-blocking) client for Redis in Cluster mode. Use {@link #CreateClient} to request a
@@ -53,18 +53,19 @@ public CompletableFuture<ClusterValue<Object>> customCommand(@NonNull String[] a
     }
 
     @Override
-    @SuppressWarnings("unchecked")
-    public CompletableFuture<ClusterValue<Object>> customCommand(
-            @NonNull String[] args, @NonNull Route route) {
+    public CompletableFuture<ClusterValue<Object>> customCommand(String[] args, Route route) {
         return commandManager.submitNewCommand(
-                CustomCommand,
-                args,
-                route,
-                response ->
-                        route.isSingleNodeRoute()
-                                ? ClusterValue.ofSingleValue(handleObjectOrNullResponse(response))
-                                : ClusterValue.ofMultiValue(
-                                        (Map<String, Object>) handleObjectOrNullResponse(response)));
+                CustomCommand, args, route, response -> handleCustomCommandResponse(route, response));
+    }
+
+    protected ClusterValue<Object> handleCustomCommandResponse(Route route, Response response) {
+        if (route.isSingleNodeRoute()) {
+            return ClusterValue.ofSingleValue(handleObjectOrNullResponse(response));
+        }
+        if (response.hasConstantResponse()) {
+            return ClusterValue.ofSingleValue(handleStringResponse(response));
+        }
+        return ClusterValue.ofMultiValue(handleMapResponse(response));
     }
 
     @Override

diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java
@@ -62,13 +62,13 @@ public abstract class BaseTransaction<T extends BaseTransaction<T>> {
      *     this function.
      * @example Returns a list of all pub/sub clients:
      *     <pre>
-     * Object result = client.customCommand("CLIENT","LIST","TYPE", "PUBSUB").get();
+     * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get();
      * </pre>
      *
      * @param args Arguments for the custom command.
      * @return A response from Redis with an <code>Object</code>.
      */
-    public T customCommand(String... args) {
+    public T customCommand(String[] args) {
 
         ArgsArray commandArgs = buildArgs(args);
         protobufTransaction.addCommands(buildCommand(CustomCommand, commandArgs));

diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java
@@ -27,8 +27,10 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import redis_request.RedisRequestOuterClass.RedisRequest;
+import response.ResponseOuterClass.ConstantResponse;
 import response.ResponseOuterClass.Response;
 
+@SuppressWarnings("unchecked,resource")
 public class RedisClusterClientTest {
 
     RedisClusterClient service;
@@ -94,6 +96,19 @@ public void custom_command_with_multi_node_route_returns_multi_value() {
         assertEquals(data, value.getMultiValue());
     }
 
+    @Test
+    @SneakyThrows
+    public void custom_command_returns_single_value_on_constant_response() {
+        var commandManager =
+                new TestCommandManager(
+                        Response.newBuilder().setConstantResponse(ConstantResponse.OK).build());
+
+        var client = new TestClient(commandManager, "OK");
+
+        var value = client.customCommand(TEST_ARGS, ALL_NODES).get();
+        assertEquals("OK", value.getSingleValue());
+    }
+
     private static class TestClient extends RedisClusterClient {
 
         private final Object object;
@@ -116,7 +131,7 @@ private static class TestCommandManager extends CommandManager {
 
         public TestCommandManager(Response responseToReturn) {
             super(null);
-            response = responseToReturn;
+            response = responseToReturn != null ? responseToReturn : Response.newBuilder().build();
         }
 
         @Override
@@ -154,7 +169,7 @@ public void ping_with_message_returns_success() {
         // setup
         String message = "RETURN OF THE PONG";
         String[] arguments = new String[] {message};
-        CompletableFuture<String> testResponse = new CompletableFuture();
+        CompletableFuture<String> testResponse = new CompletableFuture<>();
         testResponse.complete(message);
 
         Route route = ALL_PRIMARIES;

diff --git a/...st/src/test/java/glide/TestUtilities.java → .../java/glide/TransactionTestUtilities.java b/...st/src/test/java/glide/TestUtilities.java → .../java/glide/TransactionTestUtilities.java
@@ -7,21 +7,21 @@
 import java.util.Set;
 import java.util.UUID;
 
-public class TestUtilities {
+public class TransactionTestUtilities {
     private static final String key1 = "{key}" + UUID.randomUUID();
     private static final String key2 = "{key}" + UUID.randomUUID();
     private static final String key3 = "{key}" + UUID.randomUUID();
     private static final String key4 = "{key}" + UUID.randomUUID();
     private static final String value1 = "{value}" + UUID.randomUUID();
     private static final String value2 = "{value}" + UUID.randomUUID();
 
-    public static BaseTransaction transactionTest(BaseTransaction baseTransaction) {
+    public static BaseTransaction<?> transactionTest(BaseTransaction<?> baseTransaction) {
 
         baseTransaction.set(key1, value1);
         baseTransaction.get(key1);
 
         baseTransaction.set(key2, value2, SetOptions.builder().returnOldValue(true).build());
-        baseTransaction.customCommand("MGET", key1, key2);
+        baseTransaction.customCommand(new String[] {"MGET", key1, key2});
 
         baseTransaction.mset(Map.of(key1, value2, key2, value1));
         baseTransaction.mget(new String[] {key1, key2});

diff --git a/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java b/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java
@@ -1,8 +1,8 @@
 /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */
 package glide.cluster;
 
-import static glide.TestUtilities.transactionTest;
-import static glide.TestUtilities.transactionTestResult;
+import static glide.TransactionTestUtilities.transactionTest;
+import static glide.TransactionTestUtilities.transactionTestResult;
 import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -45,7 +45,7 @@ public static void teardown() {
     @Test
     @SneakyThrows
     public void custom_command_info() {
-        ClusterTransaction transaction = new ClusterTransaction().customCommand("info");
+        ClusterTransaction transaction = new ClusterTransaction().customCommand(new String[] {"info"});
         Object[] result = clusterClient.exec(transaction).get(10, TimeUnit.SECONDS);
         assertTrue(((String) result[0]).contains("# Stats"));
     }

diff --git a/java/integTest/src/test/java/glide/standalone/TransactionTests.java b/java/integTest/src/test/java/glide/standalone/TransactionTests.java
@@ -1,8 +1,8 @@
 /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */
 package glide.standalone;
 
-import static glide.TestUtilities.transactionTest;
-import static glide.TestUtilities.transactionTestResult;
+import static glide.TransactionTestUtilities.transactionTest;
+import static glide.TransactionTestUtilities.transactionTestResult;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -45,7 +45,7 @@ public static void teardown() {
     @Test
     @SneakyThrows
     public void custom_command_info() {
-        Transaction transaction = new Transaction().customCommand("info");
+        Transaction transaction = new Transaction().customCommand(new String[] {"info"});
         Object[] result = client.exec(transaction).get(10, TimeUnit.SECONDS);
         assertTrue(((String) result[0]).contains("# Stats"));
     }