Address PR feedback.

Signed-off-by: Yury-Fridlyand <[email protected]>

Author: Yury-Fridlyand

java/client/src/main/java/glide/api/BaseClient.java

@@ -1,5 +1,9 @@
 package glide.api;
 
+import static glide.ffi.resolvers.SocketListenerResolver.getSocket;
+
+import glide.connectors.handlers.CallbackDispatcher;
+import glide.connectors.handlers.ChannelHandler;
 import glide.ffi.resolvers.RedisValueResolver;
 import glide.managers.BaseCommandResponseResolver;
 import glide.managers.CommandManager;
@@ -43,4 +47,17 @@ public void close() throws ExecutionException {
             throw new RuntimeException(e);
         }
     }
+
+    protected static ChannelHandler buildChannelHandler() throws InterruptedException {
+        CallbackDispatcher callbackDispatcher = new CallbackDispatcher();
+        return new ChannelHandler(callbackDispatcher, getSocket());
+    }
+
+    protected static ConnectionManager buildConnectionManager(ChannelHandler channelHandler) {
+        return new ConnectionManager(channelHandler);
+    }
+
+    protected static CommandManager buildCommandManager(ChannelHandler channelHandler) {
+        return new CommandManager(channelHandler);
+    }
 }

java/client/src/main/java/glide/api/RedisClient.java

@@ -1,15 +1,11 @@
 package glide.api;
 
-import static glide.ffi.resolvers.SocketListenerResolver.getSocket;
-
 import glide.api.commands.BaseCommands;
 import glide.api.models.configuration.RedisClientConfiguration;
-import glide.connectors.handlers.CallbackDispatcher;
 import glide.connectors.handlers.ChannelHandler;
 import glide.managers.CommandManager;
 import glide.managers.ConnectionManager;
 import glide.managers.models.Command;
-import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -45,23 +41,10 @@ public static CompletableFuture<RedisClient> CreateClient(RedisClientConfigurati
         }
     }
 
-    protected static ChannelHandler buildChannelHandler() throws InterruptedException {
-        CallbackDispatcher callbackDispatcher = new CallbackDispatcher();
-        return new ChannelHandler(callbackDispatcher, getSocket());
-    }
-
-    protected static ConnectionManager buildConnectionManager(ChannelHandler channelHandler) {
-        return new ConnectionManager(channelHandler);
-    }
-
-    protected static CommandManager buildCommandManager(ChannelHandler channelHandler) {
-        return new CommandManager(channelHandler);
-    }
-
     @Override
     public CompletableFuture<Object> customCommand(String[] args) {
         Command command =
                 Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build();
-        return commandManager.submitNewCommand(command, Optional.empty(), this::handleObjectResponse);
+        return commandManager.submitNewCommand(command, this::handleObjectResponse);
     }
 }

java/client/src/main/java/glide/api/RedisClusterClient.java

@@ -1,18 +1,14 @@
 package glide.api;
 
-import static glide.api.RedisClient.buildChannelHandler;
-import static glide.api.RedisClient.buildCommandManager;
-import static glide.api.RedisClient.buildConnectionManager;
-
 import glide.api.commands.ClusterBaseCommands;
 import glide.api.models.ClusterValue;
 import glide.api.models.configuration.RedisClusterClientConfiguration;
-import glide.api.models.configuration.Route;
+import glide.api.models.configuration.RequestRoutingConfiguration.Route;
 import glide.connectors.handlers.ChannelHandler;
 import glide.managers.CommandManager;
 import glide.managers.ConnectionManager;
 import glide.managers.models.Command;
-import java.util.Optional;
+import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -53,15 +49,26 @@ public static CompletableFuture<RedisClusterClient> CreateClient(
     public CompletableFuture<ClusterValue<Object>> customCommand(String[] args) {
         Command command =
                 Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build();
+        // TODO if a command returns a map as a single value, ClusterValue misleads user
         return commandManager.submitNewCommand(
-                command, Optional.empty(), response -> ClusterValue.of(handleObjectResponse(response)));
+                command, response -> ClusterValue.of(handleObjectResponse(response)));
     }
 
     @Override
+    @SuppressWarnings("unchecked")
     public CompletableFuture<ClusterValue<Object>> customCommand(String[] args, Route route) {
         Command command =
-                Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build();
+                Command.builder()
+                        .requestType(Command.RequestType.CUSTOM_COMMAND)
+                        .arguments(args)
+                        .route(route)
+                        .build();
+
         return commandManager.submitNewCommand(
-                command, Optional.of(route), response -> ClusterValue.of(handleObjectResponse(response)));
+                command,
+                response ->
+                        route.isSingleNodeRoute()
+                                ? ClusterValue.ofSingleValue(handleObjectResponse(response))
+                                : ClusterValue.ofMultiValue((Map<String, Object>) handleObjectResponse(response)));
     }
 }

java/client/src/main/java/glide/api/commands/BaseCommands.java

@@ -7,19 +7,19 @@ public interface BaseCommands {
 
     /**
      * Executes a single command, without checking inputs. Every part of the command, including
-     * subcommands, should be added as a separate value in {@code args}.
+     * subcommands, should be added as a separate value in args.
      *
      * @remarks This function should only be used for single-response commands. Commands that don't
-     *     return response (such as <em>SUBSCRIBE</em>), or that return potentially more than a single
-     *     response (such as <em>XREAD</em>), or that change the client's behavior (such as entering
-     *     <em>pub</em>/<em>sub</em> mode on <em>RESP2</em> connections) shouldn't be called using
-     *     this function.
-     * @example Returns a list of all <em>pub</em>/<em>sub</em> clients:
-     *     <p><code>
-     * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get();
-     * </code>
-     * @param args Arguments for the custom command including the command name
-     * @return A <em>CompletableFuture</em> with response result from Redis
+     *     return response (such as SUBSCRIBE), or that return potentially more than a single response
+     *     (such as XREAD), or that change the client's behavior (such as entering pub/sub mode on
+     *     RESP2 connections) shouldn't be called using this function.
+     * @example Returns a list of all pub/sub clients:
+     *     <pre>
+     * Object result = client.customCommand(new String[]{"CLIENT","LIST","TYPE", "PUBSUB"}).get();
+     * </pre>
+     *
+     * @param args arguments for the custom command
+     * @return a CompletableFuture with response result from Redis
      */
     CompletableFuture<Object> customCommand(String[] args);
 }

java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java

@@ -1,7 +1,7 @@
 package glide.api.commands;
 
 import glide.api.models.ClusterValue;
-import glide.api.models.configuration.Route;
+import glide.api.models.configuration.RequestRoutingConfiguration.Route;
 import java.util.concurrent.CompletableFuture;
 
 /**

java/client/src/main/java/glide/api/models/ClusterValue.java

@@ -21,7 +21,7 @@ private ClusterValue() {}
      * Check with {@link #hasMultiData()} prior to accessing the data.
      */
     public Map<String, T> getMultiValue() {
-        assert hasMultiData();
+        assert hasMultiData() : "No multi value stored";
         return multiValue;
     }
 
@@ -30,11 +30,11 @@ public Map<String, T> getMultiValue() {
      * Check with {@link #hasSingleData()} ()} prior to accessing the data.
      */
     public T getSingleValue() {
-        assert hasSingleData();
+        assert hasSingleData() : "No single value stored";
         return singleValue;
     }
 
-    /** A constructor for the value. */
+    /** A constructor for the value with type auto-detection. */
     @SuppressWarnings("unchecked")
     public static <T> ClusterValue<T> of(Object data) {
         var res = new ClusterValue<T>();
@@ -46,6 +46,20 @@ public static <T> ClusterValue<T> of(Object data) {
         return res;
     }
 
+    /** A constructor for the value. */
+    public static <T> ClusterValue<T> ofSingleValue(T data) {
+        var res = new ClusterValue<T>();
+        res.singleValue = data;
+        return res;
+    }
+
+    /** A constructor for the value. */
+    public static <T> ClusterValue<T> ofMultiValue(Map<String, T> data) {
+        var res = new ClusterValue<T>();
+        res.multiValue = data;
+        return res;
+    }
+
     /** Check that multi-value is stored in this object. Use it prior to accessing the data. */
     public boolean hasMultiData() {
         return multiValue != null;

java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java

@@ -0,0 +1,91 @@
+package glide.api.models.configuration;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import redis_request.RedisRequestOuterClass.SimpleRoutes;
+import redis_request.RedisRequestOuterClass.SlotTypes;
+
+/** Request routing configuration. */
+public class RequestRoutingConfiguration {
+
+    /**
+     * Basic interface. Please use one of the following implementations:
+     *
+     * <ul>
+     *   <li>{@link SimpleRoute}
+     *   <li>{@link SlotIdRoute}
+     *   <li>{@link SlotKeyRoute}
+     * </ul>
+     */
+    public interface Route {
+        boolean isSingleNodeRoute();
+    }
+
+    @RequiredArgsConstructor
+    @Getter
+    public enum SimpleRoute implements Route {
+        /** Route request to all nodes. */
+        ALL_NODES(SimpleRoutes.AllNodes),
+        /** Route request to all primary nodes. */
+        ALL_PRIMARIES(SimpleRoutes.AllPrimaries),
+        /** Route request to a random node. */
+        RANDOM(SimpleRoutes.Random);
+
+        private final SimpleRoutes protobufMapping;
+
+        @Override
+        public boolean isSingleNodeRoute() {
+            return this == RANDOM;
+        }
+    }
+
+    @RequiredArgsConstructor
+    @Getter
+    public enum SlotType {
+        PRIMARY(SlotTypes.Primary),
+        REPLICA(SlotTypes.Replica);
+
+        private final SlotTypes slotTypes;
+    }
+
+    /**
+     * Request routing configuration overrides the {@link ReadFrom} connection configuration.<br>
+     * If {@link SlotType#REPLICA} is used, the request will be routed to a replica, even if the
+     * strategy is {@link ReadFrom#PRIMARY}.
+     */
+    @RequiredArgsConstructor
+    @Getter
+    public static class SlotIdRoute implements Route {
+        /**
+         * Slot number. There are 16384 slots in a redis cluster, and each shard manages a slot range.
+         * Unless the slot is known, it's better to route using {@link SlotType#PRIMARY}.
+         */
+        private final int slotId;
+
+        private final SlotType slotType;
+
+        @Override
+        public boolean isSingleNodeRoute() {
+            return true;
+        }
+    }
+
+    /**
+     * Request routing configuration overrides the {@link ReadFrom} connection configuration.<br>
+     * If {@link SlotType#REPLICA} is used, the request will be routed to a replica, even if the
+     * strategy is {@link ReadFrom#PRIMARY}.
+     */
+    @RequiredArgsConstructor
+    @Getter
+    public static class SlotKeyRoute implements Route {
+        /** The request will be sent to nodes managing this key. */
+        private final String slotKey;
+
+        private final SlotType slotType;
+
+        @Override
+        public boolean isSingleNodeRoute() {
+            return true;
+        }
+    }
+}