From ef2d79113857bf7e64c86e3ce309857111ed178e Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Thu, 1 Feb 2024 20:48:21 -0800 Subject: [PATCH 01/36] Java: Add commands for GET/SET/INFO/PING Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/BaseClient.java | 138 ++++++++-- .../src/main/java/glide/api/RedisClient.java | 28 +- .../java/glide/api/RedisClusterClient.java | 70 +++-- .../java/glide/api/commands/BaseCommands.java | 17 +- .../api/commands/ClusterBaseCommands.java | 16 +- .../api/commands/ClusterServerCommands.java | 58 ++++ .../api/commands/ConnectionCommands.java | 29 ++ .../glide/api/commands/ServerCommands.java | 33 +++ .../glide/api/commands/StringCommands.java | 46 ++++ .../java/glide/api/models/ClusterValue.java | 19 +- .../api/models/commands/InfoOptions.java | 64 +++++ .../glide/api/models/commands/SetOptions.java | 130 +++++++++ .../RequestRoutingConfiguration.java | 20 +- .../java/glide/managers/CommandManager.java | 123 +++++---- .../java/glide/managers/models/Command.java | 29 -- .../java/glide/ExceptionHandlingTests.java | 15 +- .../test/java/glide/api/RedisClientTest.java | 252 +++++++++++++++++- .../glide/api/RedisClusterClientTest.java | 180 ++++++++++++- .../glide/managers/CommandManagerTest.java | 113 ++++---- 19 files changed, 1151 insertions(+), 229 deletions(-) create mode 100644 java/client/src/main/java/glide/api/commands/ClusterServerCommands.java create mode 100644 java/client/src/main/java/glide/api/commands/ConnectionCommands.java create mode 100644 java/client/src/main/java/glide/api/commands/ServerCommands.java create mode 100644 java/client/src/main/java/glide/api/commands/StringCommands.java create mode 100644 java/client/src/main/java/glide/api/models/commands/InfoOptions.java create mode 100644 java/client/src/main/java/glide/api/models/commands/SetOptions.java delete mode 100644 java/client/src/main/java/glide/managers/models/Command.java diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 042ab91354..4dd5d1c0dc 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -2,8 +2,15 @@ package glide.api; import static glide.ffi.resolvers.SocketListenerResolver.getSocket; +import static redis_request.RedisRequestOuterClass.RequestType.GetString; +import static redis_request.RedisRequestOuterClass.RequestType.Ping; +import static redis_request.RedisRequestOuterClass.RequestType.SetString; +import glide.api.commands.ConnectionCommands; +import glide.api.commands.StringCommands; +import glide.api.models.commands.SetOptions; import glide.api.models.configuration.BaseClientConfiguration; +import glide.api.models.exceptions.RedisException; import glide.connectors.handlers.CallbackDispatcher; import glide.connectors.handlers.ChannelHandler; import glide.connectors.resources.Platform; @@ -13,31 +20,21 @@ import glide.managers.BaseCommandResponseResolver; import glide.managers.CommandManager; import glide.managers.ConnectionManager; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.function.BiFunction; import lombok.AllArgsConstructor; +import org.apache.commons.lang3.ArrayUtils; import response.ResponseOuterClass.Response; /** Base Client class for Redis */ @AllArgsConstructor -public abstract class BaseClient implements AutoCloseable { +public abstract class BaseClient implements AutoCloseable, StringCommands, ConnectionCommands { protected final ConnectionManager connectionManager; protected final CommandManager commandManager; - /** - * Extracts the response from the Protobuf response and either throws an exception or returns the - * appropriate response as an Object - * - * @param response Redis protobuf message - * @return Response Object - */ - protected Object handleObjectResponse(Response response) { - // convert protobuf response into Object and then Object into T - return new BaseCommandResponseResolver(RedisValueResolver::valueFromPointer).apply(response); - } - /** * Async request for an async (non-blocking) Redis client. * @@ -74,8 +71,8 @@ protected static CompletableFuture CreateClient( * Closes this resource, relinquishing any underlying resources. This method is invoked * automatically on objects managed by the try-with-resources statement. * - *

see: AutoCloseable::close() + * @see AutoCloseable::close() */ @Override public void close() throws ExecutionException { @@ -100,4 +97,115 @@ protected static ConnectionManager buildConnectionManager(ChannelHandler channel protected static CommandManager buildCommandManager(ChannelHandler channelHandler) { return new CommandManager(channelHandler); } + + /** + * Extracts the response from the Protobuf response and either throws an exception or returns the + * appropriate response as an Object. + * + * @param response Redis protobuf message + * @return Response Object + */ + protected Object handleObjectResponse(Response response) { + // convert protobuf response into Object + return new BaseCommandResponseResolver(RedisValueResolver::valueFromPointer).apply(response); + } + + /** + * Checks that the Response is empty. + * + * @return An empty response + */ + protected Void handleVoidResponse(Response response) { + Object value = handleObjectResponse(response); + if (value == null) { + return null; + } + throw new RedisException( + "Unexpected return type from Redis: got " + + value.getClass().getSimpleName() + + " expected null"); + } + + /** + * Extracts the response value from the Redis response and either throws an exception or returns + * the value as a String. + * + * @param response Redis protobuf message + * @return Response as a String + */ + protected String handleStringResponse(Response response) { + Object value = handleObjectResponse(response); + if (value instanceof String) { + return (String) value; + } + throw new RedisException( + "Unexpected return type from Redis: got " + + value.getClass().getSimpleName() + + " expected String"); + } + + /** + * Extracts the response value from the Redis response and either throws an exception or returns + * the value as an Object[]. + * + * @param response Redis protobuf message + * @return Response as an Object[] + */ + protected Object[] handleArrayResponse(Response response) { + Object value = handleObjectResponse(response); + if (value instanceof Object[]) { + return (Object[]) value; + } + throw new RedisException( + "Unexpected return type from Redis: got " + + value.getClass().getSimpleName() + + " expected Object[]"); + } + + /** + * Extracts the response value from the Redis response and either throws an exception or returns + * the value as a Map. + * + * @param response Redis protobuf message + * @return Response as a Map. + */ + @SuppressWarnings("unchecked") + protected Map handleMapResponse(Response response) { + Object value = handleObjectResponse(response); + if (value instanceof Map) { + return (Map) value; + } + throw new RedisException( + "Unexpected return type from Redis: got " + + value.getClass().getSimpleName() + + " expected Map"); + } + + @Override + public CompletableFuture ping() { + return commandManager.submitNewCommand(Ping, new String[0], this::handleStringResponse); + } + + @Override + public CompletableFuture ping(String msg) { + return commandManager.submitNewCommand(Ping, new String[] {msg}, this::handleStringResponse); + } + + @Override + public CompletableFuture get(String key) { + return commandManager.submitNewCommand( + GetString, new String[] {key}, this::handleStringResponse); + } + + @Override + public CompletableFuture set(String key, String value) { + return commandManager.submitNewCommand( + SetString, new String[] {key, value}, this::handleVoidResponse); + } + + @Override + public CompletableFuture set(String key, String value, SetOptions options) { + String[] arguments = ArrayUtils.addAll(new String[] {key, value}, options.toArgs()); + return commandManager.submitNewCommand(SetString, arguments, this::handleStringResponse); + } } diff --git a/java/client/src/main/java/glide/api/RedisClient.java b/java/client/src/main/java/glide/api/RedisClient.java index c39ebfb753..1ba7f3a976 100644 --- a/java/client/src/main/java/glide/api/RedisClient.java +++ b/java/client/src/main/java/glide/api/RedisClient.java @@ -1,18 +1,24 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api; +import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; +import static redis_request.RedisRequestOuterClass.RequestType.Info; + import glide.api.commands.BaseCommands; +import glide.api.commands.ConnectionCommands; +import glide.api.commands.ServerCommands; +import glide.api.models.commands.InfoOptions; import glide.api.models.configuration.RedisClientConfiguration; import glide.managers.CommandManager; import glide.managers.ConnectionManager; -import glide.managers.models.Command; import java.util.concurrent.CompletableFuture; /** * Async (non-blocking) client for Redis in Standalone mode. Use {@link #CreateClient} to request a * client to Redis. */ -public class RedisClient extends BaseClient implements BaseCommands { +public class RedisClient extends BaseClient + implements BaseCommands, ConnectionCommands, ServerCommands { protected RedisClient(ConnectionManager connectionManager, CommandManager commandManager) { super(connectionManager, commandManager); @@ -22,16 +28,24 @@ protected RedisClient(ConnectionManager connectionManager, CommandManager comman * Async request for an async (non-blocking) Redis client in Standalone mode. * * @param config Redis client Configuration - * @return a Future to connect and return a RedisClient + * @return A Future to connect and return a RedisClient */ public static CompletableFuture CreateClient(RedisClientConfiguration config) { return CreateClient(config, RedisClient::new); } @Override - public CompletableFuture customCommand(String[] args) { - Command command = - Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build(); - return commandManager.submitNewCommand(command, this::handleObjectResponse); + public CompletableFuture customCommand(String... args) { + return commandManager.submitNewCommand(CustomCommand, args, this::handleStringResponse); + } + + @Override + public CompletableFuture info() { + return commandManager.submitNewCommand(Info, new String[0], this::handleStringResponse); + } + + @Override + public CompletableFuture info(InfoOptions options) { + return commandManager.submitNewCommand(Info, options.toArgs(), this::handleStringResponse); } } diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 948ee7240b..159a3db577 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -1,21 +1,27 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api; +import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; +import static redis_request.RedisRequestOuterClass.RequestType.Info; + import glide.api.commands.ClusterBaseCommands; +import glide.api.commands.ClusterServerCommands; import glide.api.models.ClusterValue; +import glide.api.models.commands.InfoOptions; import glide.api.models.configuration.RedisClusterClientConfiguration; import glide.api.models.configuration.RequestRoutingConfiguration.Route; import glide.managers.CommandManager; import glide.managers.ConnectionManager; -import glide.managers.models.Command; import java.util.Map; +import java.util.Optional; import java.util.concurrent.CompletableFuture; /** * Async (non-blocking) client for Redis in Cluster mode. Use {@link #CreateClient} to request a * client to Redis. */ -public class RedisClusterClient extends BaseClient implements ClusterBaseCommands { +public class RedisClusterClient extends BaseClient + implements ClusterBaseCommands, ClusterServerCommands { protected RedisClusterClient(ConnectionManager connectionManager, CommandManager commandManager) { super(connectionManager, commandManager); @@ -25,7 +31,7 @@ protected RedisClusterClient(ConnectionManager connectionManager, CommandManager * Async request for an async (non-blocking) Redis client in Cluster mode. * * @param config Redis cluster client Configuration - * @return a Future to connect and return a ClusterClient + * @return A Future to connect and return a RedisClusterClient */ public static CompletableFuture CreateClient( RedisClusterClientConfiguration config) { @@ -33,29 +39,61 @@ public static CompletableFuture CreateClient( } @Override - public CompletableFuture> customCommand(String[] args) { - Command command = - Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build(); + public CompletableFuture> customCommand(String... args) { // TODO if a command returns a map as a single value, ClusterValue misleads user return commandManager.submitNewCommand( - command, response -> ClusterValue.of(handleObjectResponse(response))); + CustomCommand, + args, + Optional.empty(), + response -> ClusterValue.of(handleObjectResponse(response))); } @Override @SuppressWarnings("unchecked") - public CompletableFuture> customCommand(String[] args, Route route) { - Command command = - Command.builder() - .requestType(Command.RequestType.CUSTOM_COMMAND) - .arguments(args) - .route(route) - .build(); - + public CompletableFuture> customCommand(Route route, String... args) { return commandManager.submitNewCommand( - command, + CustomCommand, + args, + Optional.ofNullable(route), response -> route.isSingleNodeRoute() ? ClusterValue.ofSingleValue(handleObjectResponse(response)) : ClusterValue.ofMultiValue((Map) handleObjectResponse(response))); } + + @Override + public CompletableFuture> info() { + return commandManager.submitNewCommand( + Info, + new String[0], + Optional.empty(), + response -> ClusterValue.of(handleStringResponse(response))); + } + + @Override + public CompletableFuture> info(Route route) { + return commandManager.submitNewCommand( + Info, + new String[0], + Optional.ofNullable(route), + response -> ClusterValue.of(handleStringResponse(response))); + } + + @Override + public CompletableFuture> info(InfoOptions options) { + return commandManager.submitNewCommand( + Info, + options.toArgs(), + Optional.empty(), + response -> ClusterValue.of(handleStringResponse(response))); + } + + @Override + public CompletableFuture> info(InfoOptions options, Route route) { + return commandManager.submitNewCommand( + Info, + options.toArgs(), + Optional.ofNullable(route), + response -> ClusterValue.of(handleStringResponse(response))); + } } diff --git a/java/client/src/main/java/glide/api/commands/BaseCommands.java b/java/client/src/main/java/glide/api/commands/BaseCommands.java index a1315be971..3c2cd85f8d 100644 --- a/java/client/src/main/java/glide/api/commands/BaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/BaseCommands.java @@ -8,19 +8,20 @@ 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 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: - *

- * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get(); - * - * @param args Arguments for the custom command including the command name - * @return A CompletableFuture with response result from Redis + * @example Returns a list of all pub/sub clients: + * 

+     * Object result = client.customCommand("CLIENT","LIST","TYPE", "PUBSUB").get();
+     *
+ * + * @param args Arguments for the custom command. + * @return Response from Redis containing an Object. */ - CompletableFuture customCommand(String[] args); + CompletableFuture customCommand(String... args); } diff --git a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java index b58808e0fe..ffed32e8aa 100644 --- a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java @@ -21,12 +21,12 @@ public interface ClusterBaseCommands { * this function. * @example Returns a list of all pub/sub clients: *

- * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get(); + * Object result = client.customCommand("CLIENT", "LIST", "TYPE", "PUBSUB").get(); * - * @param args Arguments for the custom command including the command name - * @return A CompletableFuture with response result from Redis + * @param args Arguments for the custom command including the command name. + * @return Response from Redis containing an Object. */ - CompletableFuture> customCommand(String[] args); + CompletableFuture> customCommand(String... args); /** * Executes a single command, without checking inputs. Every part of the command, including @@ -39,11 +39,11 @@ public interface ClusterBaseCommands { * this function. * @example Returns a list of all pub/sub clients: *

- * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get(); + * Object result = client.customCommand("CLIENT", "LIST", "TYPE", "PUBSUB").get(); * - * @param args Arguments for the custom command including the command name * @param route Routing configuration for the command - * @return A CompletableFuture with response result from Redis + * @param args Arguments for the custom command including the command name + * @return Response from Redis containing an Object. */ - CompletableFuture> customCommand(String[] args, Route route); + CompletableFuture> customCommand(Route route, String... args); } diff --git a/java/client/src/main/java/glide/api/commands/ClusterServerCommands.java b/java/client/src/main/java/glide/api/commands/ClusterServerCommands.java new file mode 100644 index 0000000000..768408aad9 --- /dev/null +++ b/java/client/src/main/java/glide/api/commands/ClusterServerCommands.java @@ -0,0 +1,58 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.commands; + +import glide.api.models.ClusterValue; +import glide.api.models.commands.InfoOptions; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; +import java.util.concurrent.CompletableFuture; + +/** + * Server Management Commands interface. + * + * @see Server Management Commands + */ +public interface ClusterServerCommands { + + /** + * Get information and statistics about the Redis server. DEFAULT option is assumed + * + * @see redis.io for details. {@link + * InfoOptions.Section#DEFAULT} option is assumed. + * @return Response from Redis cluster containing a String. + */ + CompletableFuture> info(); + + /** + * Get information and statistics about the Redis server. DEFAULT option is assumed + * + * @see redis.io for details. + * @param route Routing configuration for the command + * @return Response from Redis cluster containing a String. + */ + CompletableFuture> info(Route route); + + /** + * Get information and statistics about the Redis server. + * + * @see redis.io for details. + * @param options - A list of {@link InfoOptions.Section} values specifying which sections of + * information to retrieve. When no parameter is provided, the {@link + * InfoOptions.Section#DEFAULT} option is assumed. + * @return Response from Redis cluster containing a String with the requested + * Sections. + */ + CompletableFuture> info(InfoOptions options); + + /** + * Get information and statistics about the Redis server. + * + * @see redis.io for details. + * @param options - A list of {@link InfoOptions.Section} values specifying which sections of + * information to retrieve. When no parameter is provided, the {@link + * InfoOptions.Section#DEFAULT} option is assumed. + * @param route Routing configuration for the command + * @return Response from Redis cluster containing a String with the requested + * Sections. + */ + CompletableFuture> info(InfoOptions options, Route route); +} diff --git a/java/client/src/main/java/glide/api/commands/ConnectionCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionCommands.java new file mode 100644 index 0000000000..91e55ece1a --- /dev/null +++ b/java/client/src/main/java/glide/api/commands/ConnectionCommands.java @@ -0,0 +1,29 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.commands; + +import java.util.concurrent.CompletableFuture; + +/** + * Connection Management Commands interface. + * + * @see: Connection Management Commands + */ +public interface ConnectionCommands { + + /** + * Ping the Redis server. + * + * @see redis.io for details. + * @return Response from Redis containing a String. + */ + CompletableFuture ping(); + + /** + * Ping the Redis server. + * + * @see redis.io for details. + * @param msg The ping argument that will be returned. + * @return Response from Redis containing a String. + */ + CompletableFuture ping(String msg); +} diff --git a/java/client/src/main/java/glide/api/commands/ServerCommands.java b/java/client/src/main/java/glide/api/commands/ServerCommands.java new file mode 100644 index 0000000000..392b88c721 --- /dev/null +++ b/java/client/src/main/java/glide/api/commands/ServerCommands.java @@ -0,0 +1,33 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.commands; + +import glide.api.models.commands.InfoOptions; +import glide.api.models.commands.InfoOptions.Section; +import java.util.concurrent.CompletableFuture; + +/** + * Server Management Commands interface. + * + * @see Server Management Commands + */ +public interface ServerCommands { + + /** + * Get information and statistics about the Redis server. No argument is provided, so the {@link + * Section#DEFAULT} option is assumed. + * + * @see redis.io for details. + * @return Response from Redis containing a String. + */ + CompletableFuture info(); + + /** + * Get information and statistics about the Redis server. + * + * @see redis.io for details. + * @param options A list of {@link Section} values specifying which sections of information to + * retrieve. When no parameter is provided, the {@link Section#DEFAULT} option is assumed. + * @return Response from Redis containing a String with the requested sections. + */ + CompletableFuture info(InfoOptions options); +} diff --git a/java/client/src/main/java/glide/api/commands/StringCommands.java b/java/client/src/main/java/glide/api/commands/StringCommands.java new file mode 100644 index 0000000000..57fd76beaf --- /dev/null +++ b/java/client/src/main/java/glide/api/commands/StringCommands.java @@ -0,0 +1,46 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.commands; + +import glide.api.models.commands.SetOptions; +import glide.api.models.commands.SetOptions.ConditionalSet; +import glide.api.models.commands.SetOptions.SetOptionsBuilder; +import java.util.concurrent.CompletableFuture; + +/** String Commands interface to handle single commands that return Strings. */ +public interface StringCommands { + + /** + * Get the value associated with the given key, or null if no such value exists. + * + * @see redis.io for details. + * @param key The key to retrieve from the database. + * @return Response from Redis. key exists, returns the value of + * key as a String. Otherwise, return null. + */ + CompletableFuture get(String key); + + /** + * Set the given key with the given value. + * + * @see redis.io for details. + * @param key The key to store. + * @param value The value to store with the given key. + * @return Response from Redis. + */ + CompletableFuture set(String key, String value); + + /** + * Set the given key with the given value. Return value is dependent on the passed options. + * + * @see redis.io for details. + * @param key The key to store. + * @param value The value to store with the given key. + * @param options The Set options. + * @return Response from Redis containing a String or null response. The + * old value as a String if {@link SetOptionsBuilder#returnOldValue(boolean)} is + * set. Otherwise, if the value isn't set because of {@link ConditionalSet#ONLY_IF_EXISTS} or + * {@link ConditionalSet#ONLY_IF_DOES_NOT_EXIST} conditions, return null. + * Otherwise, return OK. + */ + CompletableFuture set(String key, String value, SetOptions options); +} diff --git a/java/client/src/main/java/glide/api/models/ClusterValue.java b/java/client/src/main/java/glide/api/models/ClusterValue.java index a690f154c5..0dd25cd034 100644 --- a/java/client/src/main/java/glide/api/models/ClusterValue.java +++ b/java/client/src/main/java/glide/api/models/ClusterValue.java @@ -1,14 +1,21 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.models; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; import java.util.Map; /** - * union-like type which can store single-value or multi-value retrieved from Redis. The - * multi-value, if defined, contains the routed value as a Map containing a cluster - * node address to cluster node value. + * Represents a Response object from a Redis server with cluster-mode enabled. The response type may + * depend on the submitted {@link Route}. * - * @param The wrapped data type + * @remark ClusterValue stores values in a union-like object. It contains a single-value or + * multi-value response from Redis. If the command's routing is to one node use {@link + * #getSingleValue()} to return a response of type T. Otherwise, use {@link + * #getMultiValue()} to return a Map of address: nodeResponse where + * address is of type string and nodeResponse is of type + * T. + * @see Redis cluster specification + * @param The wrapped response type */ public class ClusterValue { private Map multiValue = null; @@ -19,7 +26,7 @@ private ClusterValue() {} /** * Get per-node value.
- * Check with {@link #hasMultiData()} prior to accessing the data. + * Asserts if {@link #hasMultiData()} is false. */ public Map getMultiValue() { assert hasMultiData() : "No multi value stored"; @@ -28,7 +35,7 @@ public Map getMultiValue() { /** * Get the single value.
- * Check with {@link #hasSingleData()} ()} prior to accessing the data. + * Asserts if {@link #hasSingleData()} is false. */ public T getSingleValue() { assert hasSingleData() : "No single value stored"; diff --git a/java/client/src/main/java/glide/api/models/commands/InfoOptions.java b/java/client/src/main/java/glide/api/models/commands/InfoOptions.java new file mode 100644 index 0000000000..8826a4afee --- /dev/null +++ b/java/client/src/main/java/glide/api/models/commands/InfoOptions.java @@ -0,0 +1,64 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.models.commands; + +import glide.api.commands.ServerCommands; +import java.util.List; +import lombok.Builder; +import lombok.Singular; + +/** + * Optional arguments to {@link ServerCommands#info(InfoOptions)} + * + * @see redis.io + */ +@Builder +public final class InfoOptions { + + @Singular private final List

sections; + + public enum Section { + /** SERVER: General information about the Redis server */ + SERVER, + /** CLIENTS: Client connections section */ + CLIENTS, + /** MEMORY: Memory consumption related information */ + MEMORY, + /** PERSISTENCE: RDB and AOF related information */ + PERSISTENCE, + /** STATS: General statistics */ + STATS, + /** REPLICATION: Master/replica replication information */ + REPLICATION, + /** CPU: CPU consumption statistics */ + CPU, + /** COMMANDSTATS: Redis command statistics */ + COMMANDSTATS, + /** LATENCYSTATS: Redis command latency percentile distribution statistics */ + LATENCYSTATS, + /** SENTINEL: Redis Sentinel section (only applicable to Sentinel instances) */ + SENTINEL, + /** CLUSTER: Redis Cluster section */ + CLUSTER, + /** MODULES: Modules section */ + MODULES, + /** KEYSPACE: Database related statistics */ + KEYSPACE, + /** ERRORSTATS: Redis error statistics */ + ERRORSTATS, + /** ALL: Return all sections (excluding module generated ones) */ + ALL, + /** DEFAULT: Return only the default set of sections */ + DEFAULT, + /** EVERYTHING: Includes all and modules */ + EVERYTHING, + } + + /** + * Converts options enum into a String[] to add to a Redis request. + * + * @return String[] + */ + public String[] toArgs() { + return sections.stream().map(Object::toString).toArray(String[]::new); + } +} diff --git a/java/client/src/main/java/glide/api/models/commands/SetOptions.java b/java/client/src/main/java/glide/api/models/commands/SetOptions.java new file mode 100644 index 0000000000..18b8385bcd --- /dev/null +++ b/java/client/src/main/java/glide/api/models/commands/SetOptions.java @@ -0,0 +1,130 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.models.commands; + +import glide.api.commands.StringCommands; +import glide.api.models.exceptions.RequestException; +import java.util.LinkedList; +import java.util.List; +import lombok.Builder; +import lombok.Getter; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import redis_request.RedisRequestOuterClass.Command; + +/** + * Optional arguments for {@link StringCommands#set(String, String, SetOptions)} command. + * + * @see redis.io + */ +@Builder +public final class SetOptions { + + /** + * If conditionalSet is not set the value will be set regardless of prior value + * existence. If value isn't set because of the condition, command will return null. + */ + private final ConditionalSet conditionalSet; + + /** + * Set command to return the old string stored at key, or null if + * key did not exist. An error is returned and SET aborted if the value stored + * at key + * is not a string. Equivalent to GET in the Redis API. + */ + private final boolean returnOldValue; + + /** If not set, no expiry time will be set for the value. */ + private final TimeToLive expiry; + + /** Conditions which define whether new value should be set or not. */ + @RequiredArgsConstructor + @Getter + public enum ConditionalSet { + /** + * Only set the key if it does not already exist. Equivalent to XX in the Redis + * API. + */ + ONLY_IF_EXISTS("XX"), + /** Only set the key if it already exists. Equivalent to NX in the Redis API. */ + ONLY_IF_DOES_NOT_EXIST("NX"); + + private final String redisApi; + } + + /** Configuration of value lifetime. */ + @Builder + public static final class TimeToLive { + /** Expiry type for the time to live */ + @NonNull private TimeToLiveType type; + + /** + * The amount of time to live before the key expires. Ignored when {@link + * TimeToLiveType#KEEP_EXISTING} type is set. + */ + private Integer count; + } + + /** Types of value expiration configuration. */ + @RequiredArgsConstructor + @Getter + public enum TimeToLiveType { + /** + * Retain the time to live associated with the key. Equivalent to KEEPTTL in the + * Redis API. + */ + KEEP_EXISTING("KEEPTTL"), + /** + * Set the specified expire time, in seconds. Equivalent to EX in the Redis API. + */ + SECONDS("EX"), + /** + * Set the specified expire time, in milliseconds. Equivalent to PX in the Redis + * API. + */ + MILLISECONDS("PX"), + /** + * Set the specified Unix time at which the key will expire, in seconds. Equivalent to + * EXAT in the Redis API. + */ + UNIX_SECONDS("EXAT"), + /** + * Set the specified Unix time at which the key will expire, in milliseconds. Equivalent to + * PXAT in the Redis API. + */ + UNIX_MILLISECONDS("PXAT"); + + private final String redisApi; + } + + /** String representation of {@link #returnOldValue} when set. */ + public static String RETURN_OLD_VALUE = "GET"; + + /** + * Converts SetOptions into a String[] to add to a {@link Command} arguments. + * + * @return String[] + */ + public String[] toArgs() { + List optionArgs = new LinkedList<>(); + if (conditionalSet != null) { + optionArgs.add(conditionalSet.redisApi); + } + + if (returnOldValue) { + optionArgs.add(RETURN_OLD_VALUE); + } + + if (expiry != null) { + optionArgs.add(expiry.type.redisApi); + if (expiry.type != TimeToLiveType.KEEP_EXISTING) { + if (expiry.count == null) { + throw new RequestException( + "Set command received expiry type " + expiry.type + ", but count was not set."); + } + optionArgs.add(expiry.count.toString()); + } + } + + return optionArgs.toArray(new String[0]); + } +} diff --git a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java index acb729af72..cfaf7dc5ca 100644 --- a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java +++ b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java @@ -3,6 +3,8 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; +import redis_request.RedisRequestOuterClass.SimpleRoutes; +import redis_request.RedisRequestOuterClass.SlotTypes; /** Request routing configuration. */ public class RequestRoutingConfiguration { @@ -20,13 +22,17 @@ public interface Route { boolean isSingleNodeRoute(); } + @RequiredArgsConstructor + @Getter public enum SimpleRoute implements Route { /** Route request to all nodes. */ - ALL_NODES, + ALL_NODES(SimpleRoutes.AllNodes), /** Route request to all primary nodes. */ - ALL_PRIMARIES, + ALL_PRIMARIES(SimpleRoutes.AllPrimaries), /** Route request to a random node. */ - RANDOM; + RANDOM(SimpleRoutes.Random); + + private final SimpleRoutes protobufMapping; @Override public boolean isSingleNodeRoute() { @@ -34,9 +40,13 @@ public boolean isSingleNodeRoute() { } } + @RequiredArgsConstructor + @Getter public enum SlotType { - PRIMARY, - REPLICA, + PRIMARY(SlotTypes.Primary), + REPLICA(SlotTypes.Replica); + + private final SlotTypes slotTypes; } /** diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index 36663893d1..5991475286 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -8,17 +8,16 @@ import glide.api.models.exceptions.ClosingException; import glide.connectors.handlers.CallbackDispatcher; import glide.connectors.handlers.ChannelHandler; -import glide.managers.models.Command; import java.util.Optional; import java.util.concurrent.CompletableFuture; import lombok.RequiredArgsConstructor; import redis_request.RedisRequestOuterClass; +import redis_request.RedisRequestOuterClass.Command; import redis_request.RedisRequestOuterClass.Command.ArgsArray; import redis_request.RedisRequestOuterClass.RedisRequest; import redis_request.RedisRequestOuterClass.RequestType; import redis_request.RedisRequestOuterClass.Routes; import redis_request.RedisRequestOuterClass.SimpleRoutes; -import redis_request.RedisRequestOuterClass.SlotTypes; import response.ResponseOuterClass.Response; /** @@ -34,21 +33,52 @@ public class CommandManager { /** * Build a command and send. * - * @param command The command to execute + * @param requestType Redis command type + * @param arguments Redis command arguments * @param responseHandler The handler for the response object * @return A result promise of type T */ public CompletableFuture submitNewCommand( - Command command, RedisExceptionCheckedFunction responseHandler) { + RedisRequestOuterClass.RequestType requestType, + String[] arguments, + RedisExceptionCheckedFunction responseHandler) { + + RedisRequest.Builder command = prepareRedisRequest(requestType, arguments); + return submitNewCommand(command, responseHandler); + } + + /** + * Build a command and send. + * + * @param requestType Redis command type + * @param arguments Redis command arguments + * @param route Command routing parameters + * @param responseHandler The handler for the response object + * @return A result promise of type T + */ + public CompletableFuture submitNewCommand( + RequestType requestType, + String[] arguments, + Optional route, + RedisExceptionCheckedFunction responseHandler) { + + RedisRequest.Builder command = prepareRedisRequest(requestType, arguments, route); + return submitNewCommand(command, responseHandler); + } + + /** + * Take a redis request and send to channel. + * + * @param command The Redis command request as a builder to execute + * @param responseHandler The handler for the response object + * @return A result promise of type T + */ + protected CompletableFuture submitNewCommand( + RedisRequest.Builder command, RedisExceptionCheckedFunction responseHandler) { // write command request to channel // when complete, convert the response to our expected type T using the given responseHandler return channel - .write( - prepareRedisRequest( - command.getRequestType(), - command.getArguments(), - Optional.ofNullable(command.getRoute())), - true) + .write(command, true) .exceptionally(this::exceptionHandler) .thenApplyAsync(responseHandler::apply); } @@ -71,68 +101,59 @@ private Response exceptionHandler(Throwable e) { } /** - * Build a protobuf command/transaction request object.
- * Used by {@link CommandManager}. + * Build a protobuf command request object with routing options.
* - * @param command - Redis command - * @param args - Redis command arguments as string array - * @return An uncompleted request. CallbackDispatcher is responsible to complete it by adding a - * callback id. + * @param requestType Redis command type + * @param arguments Redis command arguments + * @param route Command routing parameters + * @return An uncompleted request. {@link CallbackDispatcher} is responsible to complete it by + * adding a callback id. */ - private RedisRequestOuterClass.RedisRequest.Builder prepareRedisRequest( - Command.RequestType command, String[] args) { - RedisRequestOuterClass.Command.ArgsArray.Builder commandArgs = - RedisRequestOuterClass.Command.ArgsArray.newBuilder(); - for (var arg : args) { + protected RedisRequest.Builder prepareRedisRequest( + RequestType requestType, String[] arguments, Optional route) { + ArgsArray.Builder commandArgs = ArgsArray.newBuilder(); + for (var arg : arguments) { commandArgs.addArgs(arg); } - // TODO: set route properly when no RouteOptions given - return RedisRequestOuterClass.RedisRequest.newBuilder() - .setSingleCommand( - RedisRequestOuterClass.Command.newBuilder() - .setRequestType(mapRequestTypes(command)) - .setArgsArray(commandArgs.build()) - .build()) - .setRoute( - RedisRequestOuterClass.Routes.newBuilder() - .setSimpleRoutes(RedisRequestOuterClass.SimpleRoutes.AllNodes) - .build()); - } + var builder = + RedisRequest.newBuilder() + .setSingleCommand( + Command.newBuilder() + .setRequestType(requestType) + .setArgsArray(commandArgs.build()) + .build()); - private RequestType mapRequestTypes(Command.RequestType inType) { - switch (inType) { - case CUSTOM_COMMAND: - return RequestType.CustomCommand; - } - throw new RuntimeException("Unsupported request type"); + return prepareRedisRequestRoute(builder, route); } /** - * Build a protobuf command/transaction request object with routing options.
- * Used by {@link CommandManager}. + * Build a protobuf command request object with routing options.
* - * @param command Redis command type - * @param args Redis command arguments - * @param route Command routing parameters + * @param requestType Redis command type + * @param arguments Redis command arguments * @return An uncompleted request. {@link CallbackDispatcher} is responsible to complete it by * adding a callback id. */ - private RedisRequest.Builder prepareRedisRequest( - Command.RequestType command, String[] args, Optional route) { + protected RedisRequest.Builder prepareRedisRequest(RequestType requestType, String[] arguments) { ArgsArray.Builder commandArgs = ArgsArray.newBuilder(); - for (var arg : args) { + for (var arg : arguments) { commandArgs.addArgs(arg); } var builder = RedisRequest.newBuilder() .setSingleCommand( - RedisRequestOuterClass.Command.newBuilder() - .setRequestType(mapRequestTypes(command)) + Command.newBuilder() + .setRequestType(requestType) .setArgsArray(commandArgs.build()) .build()); + return prepareRedisRequestRoute(builder, Optional.empty()); + } + + private RedisRequest.Builder prepareRedisRequestRoute( + RedisRequest.Builder builder, Optional route) { if (route.isEmpty()) { return builder; } @@ -149,7 +170,7 @@ private RedisRequest.Builder prepareRedisRequest( RedisRequestOuterClass.SlotIdRoute.newBuilder() .setSlotId(((SlotIdRoute) route.get()).getSlotId()) .setSlotType( - SlotTypes.forNumber( + RedisRequestOuterClass.SlotTypes.forNumber( ((SlotIdRoute) route.get()).getSlotType().ordinal())))); } else if (route.get() instanceof SlotKeyRoute) { builder.setRoute( @@ -158,7 +179,7 @@ private RedisRequest.Builder prepareRedisRequest( RedisRequestOuterClass.SlotKeyRoute.newBuilder() .setSlotKey(((SlotKeyRoute) route.get()).getSlotKey()) .setSlotType( - SlotTypes.forNumber( + RedisRequestOuterClass.SlotTypes.forNumber( ((SlotKeyRoute) route.get()).getSlotType().ordinal())))); } else { throw new IllegalArgumentException("Unknown type of route"); diff --git a/java/client/src/main/java/glide/managers/models/Command.java b/java/client/src/main/java/glide/managers/models/Command.java deleted file mode 100644 index 4b45f38593..0000000000 --- a/java/client/src/main/java/glide/managers/models/Command.java +++ /dev/null @@ -1,29 +0,0 @@ -/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ -package glide.managers.models; - -import glide.api.models.configuration.RequestRoutingConfiguration.Route; -import lombok.Builder; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.NonNull; - -/** Base Command class to send a single request to Redis. */ -@Builder -@Getter -@EqualsAndHashCode -public class Command { - - /** Redis command request type */ - @NonNull final RequestType requestType; - - /** Request routing configuration */ - final Route route; - - /** List of Arguments for the Redis command request */ - @Builder.Default final String[] arguments = new String[] {}; - - public enum RequestType { - /** Call a custom command with list of string arguments */ - CUSTOM_COMMAND, - } -} diff --git a/java/client/src/test/java/glide/ExceptionHandlingTests.java b/java/client/src/test/java/glide/ExceptionHandlingTests.java index a4a534c5d6..43bdb224d0 100644 --- a/java/client/src/test/java/glide/ExceptionHandlingTests.java +++ b/java/client/src/test/java/glide/ExceptionHandlingTests.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import static response.ResponseOuterClass.RequestErrorType.Disconnect; import static response.ResponseOuterClass.RequestErrorType.ExecAbort; import static response.ResponseOuterClass.RequestErrorType.Timeout; @@ -26,8 +27,6 @@ import glide.managers.BaseCommandResponseResolver; import glide.managers.CommandManager; import glide.managers.ConnectionManager; -import glide.managers.models.Command; -import glide.managers.models.Command.RequestType; import io.netty.channel.ChannelFuture; import java.io.IOException; import java.util.concurrent.CancellationException; @@ -95,7 +94,7 @@ public void channel_is_closed_when_disconnected_on_command() { var channelHandler = new TestChannelHandler(callbackDispatcher); var commandManager = new CommandManager(channelHandler); - var future = commandManager.submitNewCommand(createDummyCommand(), r -> null); + var future = commandManager.submitNewCommand(CustomCommand, new String[0], r -> null); callbackDispatcher.completeRequest(null); var exception = assertThrows(ExecutionException.class, future::get); // a ClosingException thrown from CallbackDispatcher::completeRequest and then @@ -112,7 +111,7 @@ public void channel_is_not_closed_when_error_was_in_command_pipeline() { var channelHandler = new TestChannelHandler(callbackDispatcher); var commandManager = new CommandManager(channelHandler); - var future = commandManager.submitNewCommand(createDummyCommand(), r -> null); + var future = commandManager.submitNewCommand(CustomCommand, new String[0], r -> null); callbackDispatcher.completeRequest(null); var exception = assertThrows(ExecutionException.class, future::get); // a RequestException thrown from CallbackDispatcher::completeRequest and then @@ -129,7 +128,7 @@ public void command_manager_rethrows_non_RedisException_too() { var channelHandler = new TestChannelHandler(callbackDispatcher); var commandManager = new CommandManager(channelHandler); - var future = commandManager.submitNewCommand(createDummyCommand(), r -> null); + var future = commandManager.submitNewCommand(CustomCommand, new String[0], r -> null); callbackDispatcher.completeRequest(null); var exception = assertThrows(ExecutionException.class, future::get); // a IOException thrown from CallbackDispatcher::completeRequest and then wrapped @@ -210,7 +209,7 @@ public void dont_close_connection_when_callback_dispatcher_receives_response_wit var channelHandler = new TestChannelHandler(callbackDispatcher); var commandManager = new CommandManager(channelHandler); - var future = commandManager.submitNewCommand(createDummyCommand(), r -> null); + var future = commandManager.submitNewCommand(CustomCommand, new String[0], r -> null); var response = Response.newBuilder() .setCallbackIdx(0) @@ -280,10 +279,6 @@ private static RedisClientConfiguration createDummyConfig() { return RedisClientConfiguration.builder().build(); } - private static Command createDummyCommand() { - return Command.builder().requestType(RequestType.CUSTOM_COMMAND).build(); - } - /** Test ChannelHandler extension which allows to validate whether the channel was closed. */ private static class TestChannelHandler extends ChannelHandler { diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index ff8c356cff..419cc3c896 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -1,18 +1,35 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api; +import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; +import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_EXISTS; +import static glide.api.models.commands.SetOptions.RETURN_OLD_VALUE; +import static glide.api.models.commands.SetOptions.TimeToLiveType.KEEP_EXISTING; +import static glide.api.models.commands.SetOptions.TimeToLiveType.UNIX_SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; +import static redis_request.RedisRequestOuterClass.RequestType.GetString; +import static redis_request.RedisRequestOuterClass.RequestType.Info; +import static redis_request.RedisRequestOuterClass.RequestType.Ping; +import static redis_request.RedisRequestOuterClass.RequestType.SetString; +import glide.api.models.commands.InfoOptions; +import glide.api.models.commands.SetOptions; import glide.managers.CommandManager; import glide.managers.ConnectionManager; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; +import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import redis_request.RedisRequestOuterClass; public class RedisClientTest { @@ -29,18 +46,23 @@ public void setUp() { service = new RedisClient(connectionManager, commandManager); } + @SneakyThrows @Test - public void customCommand_success() throws ExecutionException, InterruptedException { + public void customCommand_returns_success() { // setup String key = "testKey"; Object value = "testValue"; String cmd = "GETSTRING"; + String[] arguments = new String[] {cmd, key}; CompletableFuture testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn(value); - when(commandManager.submitNewCommand(any(), any())).thenReturn(testResponse); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(CustomCommand), eq(arguments), any())) + .thenReturn(testResponse); // exercise - CompletableFuture response = service.customCommand(new String[] {cmd, key}); + CompletableFuture response = service.customCommand(arguments); String payload = (String) response.get(); // verify @@ -48,27 +70,243 @@ public void customCommand_success() throws ExecutionException, InterruptedExcept assertEquals(value, payload); } + @SneakyThrows @Test - public void customCommand_interruptedException() throws ExecutionException, InterruptedException { + public void customCommand_throws_InterruptedException() { // setup String key = "testKey"; Object value = "testValue"; String cmd = "GETSTRING"; + String[] arguments = new String[] {cmd, key}; CompletableFuture testResponse = mock(CompletableFuture.class); InterruptedException interruptedException = new InterruptedException(); when(testResponse.get()).thenThrow(interruptedException); - when(commandManager.submitNewCommand(any(), any())).thenReturn(testResponse); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(CustomCommand), eq(arguments), any())) + .thenReturn(testResponse); // exercise InterruptedException exception = assertThrows( InterruptedException.class, () -> { - CompletableFuture response = service.customCommand(new String[] {cmd, key}); + CompletableFuture response = service.customCommand(arguments); response.get(); }); // verify assertEquals(interruptedException, exception); } + + @SneakyThrows + @Test + public void ping_returns_success() { + // setup + CompletableFuture testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn("PONG"); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(Ping), eq(new String[0]), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.ping(); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals("PONG", payload); + } + + @SneakyThrows + @Test + public void ping_with_message_returns_success() { + // setup + String message = "RETURN OF THE PONG"; + String[] arguments = new String[] {message}; + CompletableFuture testResponse = new CompletableFuture(); + testResponse.complete(message); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(Ping), eq(arguments), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.ping(message); + String pong = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(message, pong); + } + + @SneakyThrows + @Test + public void info_returns_success() { + // setup + CompletableFuture testResponse = mock(CompletableFuture.class); + String testPayload = "Key: Value"; + when(testResponse.get()).thenReturn(testPayload); + when(commandManager.submitNewCommand(eq(Info), eq(new String[0]), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.info(); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(testPayload, payload); + } + + @SneakyThrows + @Test + public void info_with_multiple_InfoOptions_returns_success() { + // setup + String[] arguments = + new String[] {InfoOptions.Section.ALL.toString(), InfoOptions.Section.DEFAULT.toString()}; + CompletableFuture testResponse = mock(CompletableFuture.class); + String testPayload = "Key: Value"; + when(testResponse.get()).thenReturn(testPayload); + when(commandManager.submitNewCommand(eq(Info), eq(arguments), any())) + .thenReturn(testResponse); + + // exercise + InfoOptions options = + InfoOptions.builder() + .section(InfoOptions.Section.ALL) + .section(InfoOptions.Section.DEFAULT) + .build(); + CompletableFuture response = service.info(options); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(testPayload, payload); + } + + @SneakyThrows + @Test + public void info_with_empty_InfoOptions_returns_success() { + // setup + CompletableFuture testResponse = mock(CompletableFuture.class); + String testPayload = "Key: Value"; + when(testResponse.get()).thenReturn(testPayload); + when(commandManager.submitNewCommand(eq(Info), eq(new String[0]), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.info(InfoOptions.builder().build()); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(testPayload, payload); + } + + @SneakyThrows + @Test + public void get_returns_success() { + // setup + String key = "testKey"; + String value = "testValue"; + CompletableFuture testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn(value); + when(commandManager.submitNewCommand(eq(GetString), eq(new String[] {key}), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.get(key); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(value, payload); + } + + @SneakyThrows + @Test + public void set_returns_success() { + // setup + String key = "testKey"; + String value = "testValue"; + CompletableFuture testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn(null); + when(commandManager.submitNewCommand(eq(SetString), eq(new String[] {key, value}), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.set(key, value); + Object nullResponse = response.get(); + + // verify + assertEquals(testResponse, response); + assertNull(nullResponse); + } + + @SneakyThrows + @Test + public void set_with_SetOptions_OnlyIfExists_returns_success() { + // setup + String key = "testKey"; + String value = "testValue"; + SetOptions setOptions = + SetOptions.builder() + .conditionalSet(ONLY_IF_EXISTS) + .returnOldValue(false) + .expiry( + SetOptions.TimeToLive.builder() + .type(SetOptions.TimeToLiveType.KEEP_EXISTING) + .build()) + .build(); + String[] arguments = + new String[] {key, value, ONLY_IF_EXISTS.getRedisApi(), KEEP_EXISTING.getRedisApi()}; + + CompletableFuture testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn(null); + when(commandManager.submitNewCommand(eq(SetString), eq(arguments), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.set(key, value, setOptions); + + // verify + assertEquals(testResponse, response); + assertNull(response.get()); + } + + @SneakyThrows + @Test + public void set_with_SetOptions_OnlyIfDoesNotExist_returns_success() { + // setup + String key = "testKey"; + String value = "testValue"; + SetOptions setOptions = + SetOptions.builder() + .conditionalSet(ONLY_IF_DOES_NOT_EXIST) + .returnOldValue(true) + .expiry(SetOptions.TimeToLive.builder().type(UNIX_SECONDS).count(60).build()) + .build(); + String[] arguments = + new String[] { + key, + value, + ONLY_IF_DOES_NOT_EXIST.getRedisApi(), + RETURN_OLD_VALUE, + UNIX_SECONDS.getRedisApi(), + "60" + }; + CompletableFuture testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn(value); + when(commandManager.submitNewCommand(eq(SetString), eq(arguments), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.set(key, value, setOptions); + + // verify + assertNotNull(response); + assertEquals(value, response.get()); + } } diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index 32c459cafa..abba4a9587 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -1,30 +1,57 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_NODES; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; +import static redis_request.RedisRequestOuterClass.RequestType.Info; -import glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute; +import glide.api.models.ClusterValue; +import glide.api.models.commands.InfoOptions; +import glide.api.models.configuration.RequestRoutingConfiguration; import glide.managers.CommandManager; +import glide.managers.ConnectionManager; import glide.managers.RedisExceptionCheckedFunction; -import glide.managers.models.Command; +import java.util.HashMap; import java.util.Map; import java.util.concurrent.CompletableFuture; import lombok.SneakyThrows; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import redis_request.RedisRequestOuterClass; import response.ResponseOuterClass.Response; public class RedisClusterClientTest { + RedisClusterClient service; + + ConnectionManager connectionManager; + + CommandManager commandManager; + + @BeforeEach + public void setUp() { + connectionManager = mock(ConnectionManager.class); + commandManager = mock(CommandManager.class); + service = new RedisClusterClient(connectionManager, commandManager); + } + @Test @SneakyThrows - public void custom_command_returns_single_value() { + public void customCommand_returns_single_value() { var commandManager = new TestCommandManager(null); var client = new TestClient(commandManager, "TEST"); - var value = client.customCommand(new String[0]).get(); + var value = client.customCommand().get(); assertAll( () -> assertTrue(value.hasSingleData()), () -> assertEquals("TEST", value.getSingleValue())); @@ -32,13 +59,13 @@ public void custom_command_returns_single_value() { @Test @SneakyThrows - public void custom_command_returns_multi_value() { + public void customCommand_returns_multi_value() { var commandManager = new TestCommandManager(null); var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(new String[0]).get(); + var value = client.customCommand().get(); assertAll( () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); } @@ -46,26 +73,26 @@ public void custom_command_returns_multi_value() { @Test @SneakyThrows // test checks that even a map returned as a single value when single node route is used - public void custom_command_with_single_node_route_returns_single_value() { + public void customCommand_with_single_node_route_returns_single_value() { var commandManager = new TestCommandManager(null); var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(new String[0], SimpleRoute.RANDOM).get(); + var value = client.customCommand(RANDOM).get(); assertAll( () -> assertTrue(value.hasSingleData()), () -> assertEquals(data, value.getSingleValue())); } @Test @SneakyThrows - public void custom_command_with_multi_node_route_returns_multi_value() { + public void customCommand_with_multi_node_route_returns_multi_value() { var commandManager = new TestCommandManager(null); var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(new String[0], SimpleRoute.ALL_NODES).get(); + var value = client.customCommand(ALL_NODES).get(); assertAll( () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); } @@ -96,8 +123,139 @@ public TestCommandManager(Response responseToReturn) { @Override public CompletableFuture submitNewCommand( - Command command, RedisExceptionCheckedFunction responseHandler) { + RedisRequestOuterClass.RedisRequest.Builder command, + RedisExceptionCheckedFunction responseHandler) { return CompletableFuture.supplyAsync(() -> responseHandler.apply(response)); } } + + @SneakyThrows + @Test + public void customCommand_success() { + // setup + String key = "testKey"; + String value = "testValue"; + String cmd = "GETSTRING"; + String[] arguments = new String[] {cmd, key}; + CompletableFuture> testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn(ClusterValue.of(value)); + when(commandManager.>submitNewCommand( + eq(CustomCommand), any(), any(), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture> response = service.customCommand(arguments); + + // verify + assertEquals(testResponse, response); + ClusterValue clusterValue = response.get(); + assertTrue(clusterValue.hasSingleData()); + String payload = (String) clusterValue.getSingleValue(); + assertEquals(value, payload); + } + + @SneakyThrows + @Test + public void customCommand_interruptedException() { + // setup + String key = "testKey"; + String cmd = "GETSTRING"; + String[] arguments = new String[] {cmd, key}; + CompletableFuture> testResponse = mock(CompletableFuture.class); + InterruptedException interruptedException = new InterruptedException(); + when(testResponse.get()).thenThrow(interruptedException); + when(commandManager.>submitNewCommand( + eq(CustomCommand), any(), any(), any())) + .thenReturn(testResponse); + + // exercise + InterruptedException exception = + assertThrows( + InterruptedException.class, + () -> { + CompletableFuture> response = service.customCommand(arguments); + response.get(); + }); + + // verify + assertEquals(interruptedException, exception); + } + + @SneakyThrows + @Test + public void info_returns_string() { + // setup + CompletableFuture> testResponse = mock(CompletableFuture.class); + Map testPayload = new HashMap(); + testPayload.put("addr1", "value1"); + testPayload.put("addr1", "value2"); + testPayload.put("addr1", "value3"); + when(testResponse.get()).thenReturn(ClusterValue.of(testPayload)); + when(commandManager.>submitNewCommand( + eq(Info), eq(new String[0]), any(), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture> response = service.info(); + + // verify + assertEquals(testResponse, response); + ClusterValue clusterValue = response.get(); + assertTrue(clusterValue.hasMultiData()); + Map payload = clusterValue.getMultiValue(); + assertEquals(testPayload, payload); + } + + @SneakyThrows + @Test + public void info_with_route_returns_string() { + // setup + CompletableFuture> testResponse = mock(CompletableFuture.class); + Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); + RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_NODES; + when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); + when(commandManager.>submitNewCommand(eq(Info), any(), any(), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture> response = service.info(route); + + // verify + assertEquals(testResponse, response); + ClusterValue clusterValue = response.get(); + assertTrue(clusterValue.hasMultiData()); + Map clusterMap = clusterValue.getMultiValue(); + assertEquals("addr1 result", clusterMap.get("addr1")); + assertEquals("addr2 result", clusterMap.get("addr2")); + } + + @SneakyThrows + @Test + public void info_with_route_with_infoOptions_returns_string() { + // setup + String[] infoArguments = new String[] {"ALL", "DEFAULT"}; + CompletableFuture> testResponse = mock(CompletableFuture.class); + Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); + when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); + RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_PRIMARIES; + when(commandManager.>submitNewCommand( + eq(Info), eq(infoArguments), any(), any())) + .thenReturn(testResponse); + + // exercise + InfoOptions options = + InfoOptions.builder() + .section(InfoOptions.Section.ALL) + .section(InfoOptions.Section.DEFAULT) + .build(); + CompletableFuture> response = service.info(options, route); + + // verify + assertEquals(testResponse, response); + ClusterValue clusterValue = response.get(); + assertTrue(clusterValue.hasMultiData()); + Map clusterMap = clusterValue.getMultiValue(); + assertEquals("addr1 result", clusterMap.get("addr1")); + assertEquals("addr2 result", clusterMap.get("addr2")); + } } diff --git a/java/client/src/test/java/glide/managers/CommandManagerTest.java b/java/client/src/test/java/glide/managers/CommandManagerTest.java index 8e5010a905..6cf48bf15a 100644 --- a/java/client/src/test/java/glide/managers/CommandManagerTest.java +++ b/java/client/src/test/java/glide/managers/CommandManagerTest.java @@ -12,15 +12,17 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotIdRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotType; +import glide.api.models.exceptions.ClosingException; import glide.connectors.handlers.ChannelHandler; -import glide.managers.models.Command; -import java.util.Map; +import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -28,8 +30,6 @@ import org.junit.jupiter.params.provider.EnumSource; import org.mockito.ArgumentCaptor; import redis_request.RedisRequestOuterClass.RedisRequest; -import redis_request.RedisRequestOuterClass.SimpleRoutes; -import redis_request.RedisRequestOuterClass.SlotTypes; import response.ResponseOuterClass.Response; public class CommandManagerTest { @@ -38,12 +38,8 @@ public class CommandManagerTest { CommandManager service; - Command command; - @BeforeEach void init() { - command = Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).build(); - channelHandler = mock(ChannelHandler.class); service = new CommandManager(channelHandler); } @@ -64,7 +60,10 @@ public void submitNewCommand_return_Object_result() { // exercise CompletableFuture result = service.submitNewCommand( - command, new BaseCommandResponseResolver((ptr) -> ptr == pointer ? respObject : null)); + CustomCommand, + new String[0], + Optional.empty(), + new BaseCommandResponseResolver((ptr) -> ptr == pointer ? respObject : null)); Object respPointer = result.get(); // verify @@ -83,7 +82,10 @@ public void submitNewCommand_return_Null_result() { // exercise CompletableFuture result = service.submitNewCommand( - command, new BaseCommandResponseResolver((p) -> new RuntimeException(""))); + CustomCommand, + new String[0], + Optional.empty(), + new BaseCommandResponseResolver((p) -> new RuntimeException(""))); Object respPointer = result.get(); // verify @@ -107,7 +109,10 @@ public void submitNewCommand_return_String_result() { // exercise CompletableFuture result = service.submitNewCommand( - command, new BaseCommandResponseResolver((p) -> p == pointer ? testString : null)); + CustomCommand, + new String[0], + Optional.empty(), + new BaseCommandResponseResolver((p) -> p == pointer ? testString : null)); Object respPointer = result.get(); // verify @@ -115,24 +120,45 @@ public void submitNewCommand_return_String_result() { assertEquals(testString, respPointer); } + @SneakyThrows + @Test + public void submitNewCommand_throws_ClosingException() { + // setup + String errorMsg = "Closing"; + Response closingErrorResponse = Response.newBuilder().setClosingError(errorMsg).build(); + BaseCommandResponseResolver handler = + new BaseCommandResponseResolver((ptr) -> closingErrorResponse); + + CompletableFuture futureResponse = new CompletableFuture<>(); + when(channelHandler.write(any(), anyBoolean())).thenReturn(futureResponse); + ClosingException closingException = new ClosingException(errorMsg); + futureResponse.completeExceptionally(closingException); + + // exercise + ExecutionException e = + assertThrows( + ExecutionException.class, + () -> { + CompletableFuture result = + service.submitNewCommand(CustomCommand, new String[0], Optional.empty(), handler); + result.get(); + }); + + // verify + assertEquals(closingException, e.getCause()); + assertEquals(errorMsg, e.getCause().getMessage()); + } + @ParameterizedTest @EnumSource(value = SimpleRoute.class) public void prepare_request_with_simple_routes(SimpleRoute routeType) { CompletableFuture future = new CompletableFuture<>(); when(channelHandler.write(any(), anyBoolean())).thenReturn(future); - var command = - Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).route(routeType).build(); ArgumentCaptor captor = ArgumentCaptor.forClass(RedisRequest.Builder.class); - var protobufToClientRouteMapping = - Map.of( - SimpleRoutes.AllNodes, SimpleRoute.ALL_NODES, - SimpleRoutes.AllPrimaries, SimpleRoute.ALL_PRIMARIES, - SimpleRoutes.Random, SimpleRoute.RANDOM); - - service.submitNewCommand(command, r -> null); + service.submitNewCommand(CustomCommand, new String[0], Optional.of(routeType), r -> null); verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); @@ -141,8 +167,7 @@ public void prepare_request_with_simple_routes(SimpleRoute routeType) { () -> assertTrue(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertEquals( - routeType, - protobufToClientRouteMapping.get(requestBuilder.getRoute().getSimpleRoutes())), + routeType.getProtobufMapping(), requestBuilder.getRoute().getSimpleRoutes()), () -> assertFalse(requestBuilder.getRoute().hasSlotIdRoute()), () -> assertFalse(requestBuilder.getRoute().hasSlotKeyRoute())); } @@ -152,32 +177,21 @@ public void prepare_request_with_simple_routes(SimpleRoute routeType) { public void prepare_request_with_slot_id_routes(SlotType slotType) { CompletableFuture future = new CompletableFuture<>(); when(channelHandler.write(any(), anyBoolean())).thenReturn(future); - var command = - Command.builder() - .requestType(Command.RequestType.CUSTOM_COMMAND) - .route(new SlotIdRoute(42, slotType)) - .build(); ArgumentCaptor captor = ArgumentCaptor.forClass(RedisRequest.Builder.class); - service.submitNewCommand(command, r -> null); + service.submitNewCommand( + CustomCommand, new String[0], Optional.of(new SlotIdRoute(42, slotType)), r -> null); verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); - var protobufToClientRouteMapping = - Map.of( - SlotTypes.Primary, SlotType.PRIMARY, - SlotTypes.Replica, SlotType.REPLICA); - assertAll( () -> assertTrue(requestBuilder.hasRoute()), () -> assertTrue(requestBuilder.getRoute().hasSlotIdRoute()), () -> assertEquals( - slotType, - protobufToClientRouteMapping.get( - requestBuilder.getRoute().getSlotIdRoute().getSlotType())), + slotType.getSlotTypes(), requestBuilder.getRoute().getSlotIdRoute().getSlotType()), () -> assertEquals(42, requestBuilder.getRoute().getSlotIdRoute().getSlotId()), () -> assertFalse(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertFalse(requestBuilder.getRoute().hasSlotKeyRoute())); @@ -188,32 +202,21 @@ public void prepare_request_with_slot_id_routes(SlotType slotType) { public void prepare_request_with_slot_key_routes(SlotType slotType) { CompletableFuture future = new CompletableFuture<>(); when(channelHandler.write(any(), anyBoolean())).thenReturn(future); - var command = - Command.builder() - .requestType(Command.RequestType.CUSTOM_COMMAND) - .route(new SlotKeyRoute("TEST", slotType)) - .build(); ArgumentCaptor captor = ArgumentCaptor.forClass(RedisRequest.Builder.class); - service.submitNewCommand(command, r -> null); + service.submitNewCommand( + CustomCommand, new String[0], Optional.of(new SlotKeyRoute("TEST", slotType)), r -> null); verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); - var protobufToClientRouteMapping = - Map.of( - SlotTypes.Primary, SlotType.PRIMARY, - SlotTypes.Replica, SlotType.REPLICA); - assertAll( () -> assertTrue(requestBuilder.hasRoute()), () -> assertTrue(requestBuilder.getRoute().hasSlotKeyRoute()), () -> assertEquals( - slotType, - protobufToClientRouteMapping.get( - requestBuilder.getRoute().getSlotKeyRoute().getSlotType())), + slotType.getSlotTypes(), requestBuilder.getRoute().getSlotKeyRoute().getSlotType()), () -> assertEquals("TEST", requestBuilder.getRoute().getSlotKeyRoute().getSlotKey()), () -> assertFalse(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertFalse(requestBuilder.getRoute().hasSlotIdRoute())); @@ -223,15 +226,13 @@ public void prepare_request_with_slot_key_routes(SlotType slotType) { public void prepare_request_with_unknown_route_type() { CompletableFuture future = new CompletableFuture<>(); when(channelHandler.write(any(), anyBoolean())).thenReturn(future); - var command = - Command.builder() - .requestType(Command.RequestType.CUSTOM_COMMAND) - .route(() -> false) - .build(); var exception = assertThrows( - IllegalArgumentException.class, () -> service.submitNewCommand(command, r -> null)); + IllegalArgumentException.class, + () -> + service.submitNewCommand( + CustomCommand, new String[0], Optional.of(() -> false), r -> null)); assertEquals("Unknown type of route", exception.getMessage()); } } From 6cb503986f101eaa9f063e85e5d469d4b31127e8 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Thu, 1 Feb 2024 21:47:58 -0800 Subject: [PATCH 02/36] Spotless Signed-off-by: Andrew Carbonetto --- .../client/src/main/java/glide/managers/CommandManager.java | 6 +++--- java/client/src/test/java/glide/api/RedisClientTest.java | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index 5991475286..70c3a6620a 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -39,9 +39,9 @@ public class CommandManager { * @return A result promise of type T */ public CompletableFuture submitNewCommand( - RedisRequestOuterClass.RequestType requestType, - String[] arguments, - RedisExceptionCheckedFunction responseHandler) { + RedisRequestOuterClass.RequestType requestType, + String[] arguments, + RedisExceptionCheckedFunction responseHandler) { RedisRequest.Builder command = prepareRedisRequest(requestType, arguments); return submitNewCommand(command, responseHandler); diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index 419cc3c896..a3c8411f17 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -28,8 +28,6 @@ import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; -import redis_request.RedisRequestOuterClass; public class RedisClientTest { From 4efa9a35fc99267a7ed90c17fa28d268270948ec Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 09:40:34 -0800 Subject: [PATCH 03/36] Add OK response to base commands Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/BaseClient.java | 18 +++++++++------ .../glide/api/commands/StringCommands.java | 3 ++- .../src/main/java/glide/api/models/Ok.java | 23 +++++++++++++++++++ .../managers/BaseCommandResponseResolver.java | 3 ++- 4 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 java/client/src/main/java/glide/api/models/Ok.java diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 4dd5d1c0dc..dd8ab5920f 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -8,6 +8,7 @@ import glide.api.commands.ConnectionCommands; import glide.api.commands.StringCommands; +import glide.api.models.Ok; import glide.api.models.commands.SetOptions; import glide.api.models.configuration.BaseClientConfiguration; import glide.api.models.exceptions.RedisException; @@ -115,15 +116,15 @@ protected Object handleObjectResponse(Response response) { * * @return An empty response */ - protected Void handleVoidResponse(Response response) { + protected Ok handleOkResponse(Response response) { Object value = handleObjectResponse(response); - if (value == null) { - return null; + if (value == Ok.INSTANCE) { + return Ok.INSTANCE; } throw new RedisException( "Unexpected return type from Redis: got " + value.getClass().getSimpleName() - + " expected null"); + + " expected simple-string Ok"); } /** @@ -135,9 +136,12 @@ protected Void handleVoidResponse(Response response) { */ protected String handleStringResponse(Response response) { Object value = handleObjectResponse(response); - if (value instanceof String) { + if (value instanceof String || value == null) { return (String) value; } + if (value instanceof Ok) { + return Ok.INSTANCE.toString(); + } throw new RedisException( "Unexpected return type from Redis: got " + value.getClass().getSimpleName() @@ -198,9 +202,9 @@ public CompletableFuture get(String key) { } @Override - public CompletableFuture set(String key, String value) { + public CompletableFuture set(String key, String value) { return commandManager.submitNewCommand( - SetString, new String[] {key, value}, this::handleVoidResponse); + SetString, new String[] {key, value}, this::handleOkResponse); } @Override diff --git a/java/client/src/main/java/glide/api/commands/StringCommands.java b/java/client/src/main/java/glide/api/commands/StringCommands.java index 57fd76beaf..76f62ca13b 100644 --- a/java/client/src/main/java/glide/api/commands/StringCommands.java +++ b/java/client/src/main/java/glide/api/commands/StringCommands.java @@ -1,6 +1,7 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.commands; +import glide.api.models.Ok; import glide.api.models.commands.SetOptions; import glide.api.models.commands.SetOptions.ConditionalSet; import glide.api.models.commands.SetOptions.SetOptionsBuilder; @@ -27,7 +28,7 @@ public interface StringCommands { * @param value The value to store with the given key. * @return Response from Redis. */ - CompletableFuture set(String key, String value); + CompletableFuture set(String key, String value); /** * Set the given key with the given value. Return value is dependent on the passed options. diff --git a/java/client/src/main/java/glide/api/models/Ok.java b/java/client/src/main/java/glide/api/models/Ok.java new file mode 100644 index 0000000000..cb9f7e4c17 --- /dev/null +++ b/java/client/src/main/java/glide/api/models/Ok.java @@ -0,0 +1,23 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.models; + +import response.ResponseOuterClass; + +/** + * Represents a successful Response from Redis without a value. "Ok" represents the status of the + * response not the actual value. + * + * @see Simple Strings + * response + */ +public class Ok { + + public static Ok INSTANCE = new Ok(); + + // Constructor is private - use Ok.INSTANCE instead + private Ok() {} + + public String toString() { + return ResponseOuterClass.ConstantResponse.OK.toString(); + } +} diff --git a/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java b/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java index 695bf05cab..36e8a92b8d 100644 --- a/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java +++ b/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java @@ -1,6 +1,7 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.managers; +import glide.api.models.Ok; import glide.api.models.exceptions.RedisException; import lombok.AllArgsConstructor; import response.ResponseOuterClass.Response; @@ -26,7 +27,7 @@ public Object apply(Response response) throws RedisException { if (response.hasConstantResponse()) { // Return "OK" - return response.getConstantResponse().toString(); + return Ok.INSTANCE; } if (response.hasRespPointer()) { // Return the shared value - which may be a null value From 02a3b276c5607685bea3843e3ad047f235b72fb6 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 10:36:39 -0800 Subject: [PATCH 04/36] Fix OK test Signed-off-by: Andrew Carbonetto --- java/client/src/test/java/glide/api/RedisClientTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index a3c8411f17..ddfcd96197 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -20,6 +20,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.Ping; import static redis_request.RedisRequestOuterClass.RequestType.SetString; +import glide.api.models.Ok; import glide.api.models.commands.InfoOptions; import glide.api.models.commands.SetOptions; import glide.managers.CommandManager; @@ -235,12 +236,12 @@ public void set_returns_success() { .thenReturn(testResponse); // exercise - CompletableFuture response = service.set(key, value); - Object nullResponse = response.get(); + CompletableFuture response = service.set(key, value); + Object okResponse = response.get(); // verify assertEquals(testResponse, response); - assertNull(nullResponse); + assertNull(okResponse); } @SneakyThrows From 78c3b74f53e3768a40659b8dc9367bb80010f8a9 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 10:37:11 -0800 Subject: [PATCH 05/36] Fix: Info with cluster mode routes Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/RedisClusterClient.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 159a3db577..af4665070d 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -67,7 +67,7 @@ public CompletableFuture> info() { Info, new String[0], Optional.empty(), - response -> ClusterValue.of(handleStringResponse(response))); + response -> ClusterValue.of(handleObjectResponse(response))); } @Override @@ -76,7 +76,7 @@ public CompletableFuture> info(Route route) { Info, new String[0], Optional.ofNullable(route), - response -> ClusterValue.of(handleStringResponse(response))); + response -> ClusterValue.of(handleObjectResponse(response))); } @Override @@ -85,7 +85,7 @@ public CompletableFuture> info(InfoOptions options) { Info, options.toArgs(), Optional.empty(), - response -> ClusterValue.of(handleStringResponse(response))); + response -> ClusterValue.of(handleObjectResponse(response))); } @Override @@ -94,6 +94,6 @@ public CompletableFuture> info(InfoOptions options, Route r Info, options.toArgs(), Optional.ofNullable(route), - response -> ClusterValue.of(handleStringResponse(response))); + response -> ClusterValue.of(handleObjectResponse(response))); } } From a874fb15197fb5d99cb6b0121bcaad8bca3a9d00 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 11:29:40 -0800 Subject: [PATCH 06/36] Update docs and tests for review comments Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/BaseClient.java | 10 +++++-- .../api/commands/ClusterBaseCommands.java | 2 +- .../glide/api/models/commands/SetOptions.java | 3 +- .../RequestRoutingConfiguration.java | 2 +- .../test/java/glide/api/RedisClientTest.java | 29 ------------------- .../glide/api/RedisClusterClientTest.java | 27 ----------------- .../glide/managers/CommandManagerTest.java | 29 ------------------- 7 files changed, 10 insertions(+), 92 deletions(-) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index dd8ab5920f..dff0ead884 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -112,9 +112,10 @@ protected Object handleObjectResponse(Response response) { } /** - * Checks that the Response is empty. + * Checks that the Response contains the simple-string Ok. * - * @return An empty response + * @return An Ok response + * @throws RedisException if there's a type mismatch */ protected Ok handleOkResponse(Response response) { Object value = handleObjectResponse(response); @@ -133,6 +134,7 @@ protected Ok handleOkResponse(Response response) { * * @param response Redis protobuf message * @return Response as a String + * @throws RedisException if there's a type mismatch */ protected String handleStringResponse(Response response) { Object value = handleObjectResponse(response); @@ -154,6 +156,7 @@ protected String handleStringResponse(Response response) { * * @param response Redis protobuf message * @return Response as an Object[] + * @throws RedisException if there's a type mismatch */ protected Object[] handleArrayResponse(Response response) { Object value = handleObjectResponse(response); @@ -171,7 +174,8 @@ protected Object[] handleArrayResponse(Response response) { * the value as a Map. * * @param response Redis protobuf message - * @return Response as a Map. + * @return Response as a Map + * @throws RedisException if there's a type mismatch */ @SuppressWarnings("unchecked") protected Map handleMapResponse(Response response) { diff --git a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java index ffed32e8aa..63de950879 100644 --- a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java @@ -39,7 +39,7 @@ public interface ClusterBaseCommands { * this function. * @example Returns a list of all pub/sub clients: *

- * Object result = client.customCommand("CLIENT", "LIST", "TYPE", "PUBSUB").get(); + * Object result = client.customCommand(ALL_NODES, "CLIENT", "LIST", "TYPE", "PUBSUB").get(); * * @param route Routing configuration for the command * @param args Arguments for the custom command including the command name diff --git a/java/client/src/main/java/glide/api/models/commands/SetOptions.java b/java/client/src/main/java/glide/api/models/commands/SetOptions.java index 18b8385bcd..982d09ba4f 100644 --- a/java/client/src/main/java/glide/api/models/commands/SetOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/SetOptions.java @@ -28,8 +28,7 @@ public final class SetOptions { /** * Set command to return the old string stored at key, or null if * key did not exist. An error is returned and SET aborted if the value stored - * at key - * is not a string. Equivalent to GET in the Redis API. + * at key is not a string. Equivalent to GET in the Redis API. */ private final boolean returnOldValue; diff --git a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java index cfaf7dc5ca..51be1a6f54 100644 --- a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java +++ b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java @@ -89,4 +89,4 @@ public boolean isSingleNodeRoute() { return true; } } -} +} \ No newline at end of file diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index ddfcd96197..d70e4fcd0e 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -69,35 +69,6 @@ public void customCommand_returns_success() { assertEquals(value, payload); } - @SneakyThrows - @Test - public void customCommand_throws_InterruptedException() { - // setup - String key = "testKey"; - Object value = "testValue"; - String cmd = "GETSTRING"; - String[] arguments = new String[] {cmd, key}; - CompletableFuture testResponse = mock(CompletableFuture.class); - InterruptedException interruptedException = new InterruptedException(); - when(testResponse.get()).thenThrow(interruptedException); - - // match on protobuf request - when(commandManager.submitNewCommand(eq(CustomCommand), eq(arguments), any())) - .thenReturn(testResponse); - - // exercise - InterruptedException exception = - assertThrows( - InterruptedException.class, - () -> { - CompletableFuture response = service.customCommand(arguments); - response.get(); - }); - - // verify - assertEquals(interruptedException, exception); - } - @SneakyThrows @Test public void ping_returns_success() { diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index abba4a9587..1c48bfa1ec 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -154,33 +154,6 @@ public void customCommand_success() { assertEquals(value, payload); } - @SneakyThrows - @Test - public void customCommand_interruptedException() { - // setup - String key = "testKey"; - String cmd = "GETSTRING"; - String[] arguments = new String[] {cmd, key}; - CompletableFuture> testResponse = mock(CompletableFuture.class); - InterruptedException interruptedException = new InterruptedException(); - when(testResponse.get()).thenThrow(interruptedException); - when(commandManager.>submitNewCommand( - eq(CustomCommand), any(), any(), any())) - .thenReturn(testResponse); - - // exercise - InterruptedException exception = - assertThrows( - InterruptedException.class, - () -> { - CompletableFuture> response = service.customCommand(arguments); - response.get(); - }); - - // verify - assertEquals(interruptedException, exception); - } - @SneakyThrows @Test public void info_returns_string() { diff --git a/java/client/src/test/java/glide/managers/CommandManagerTest.java b/java/client/src/test/java/glide/managers/CommandManagerTest.java index 6cf48bf15a..67989043d8 100644 --- a/java/client/src/test/java/glide/managers/CommandManagerTest.java +++ b/java/client/src/test/java/glide/managers/CommandManagerTest.java @@ -120,35 +120,6 @@ public void submitNewCommand_return_String_result() { assertEquals(testString, respPointer); } - @SneakyThrows - @Test - public void submitNewCommand_throws_ClosingException() { - // setup - String errorMsg = "Closing"; - Response closingErrorResponse = Response.newBuilder().setClosingError(errorMsg).build(); - BaseCommandResponseResolver handler = - new BaseCommandResponseResolver((ptr) -> closingErrorResponse); - - CompletableFuture futureResponse = new CompletableFuture<>(); - when(channelHandler.write(any(), anyBoolean())).thenReturn(futureResponse); - ClosingException closingException = new ClosingException(errorMsg); - futureResponse.completeExceptionally(closingException); - - // exercise - ExecutionException e = - assertThrows( - ExecutionException.class, - () -> { - CompletableFuture result = - service.submitNewCommand(CustomCommand, new String[0], Optional.empty(), handler); - result.get(); - }); - - // verify - assertEquals(closingException, e.getCause()); - assertEquals(errorMsg, e.getCause().getMessage()); - } - @ParameterizedTest @EnumSource(value = SimpleRoute.class) public void prepare_request_with_simple_routes(SimpleRoute routeType) { From 516c2584b7ffb644e6d08ce9a067929e68b45e79 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 11:36:59 -0800 Subject: [PATCH 07/36] Spotless and review comments Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/BaseClient.java | 15 ++++++--------- .../java/glide/api/commands/StringCommands.java | 6 +++++- .../RequestRoutingConfiguration.java | 2 +- .../src/test/java/glide/api/RedisClientTest.java | 1 - .../java/glide/api/RedisClusterClientTest.java | 1 - .../java/glide/managers/CommandManagerTest.java | 2 -- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index dff0ead884..4b6bda03f8 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -122,10 +122,9 @@ protected Ok handleOkResponse(Response response) { if (value == Ok.INSTANCE) { return Ok.INSTANCE; } + String className = (value == null) ? "null" : value.getClass().getSimpleName(); throw new RedisException( - "Unexpected return type from Redis: got " - + value.getClass().getSimpleName() - + " expected simple-string Ok"); + "Unexpected return type from Redis: got " + className + " expected simple-string Ok"); } /** @@ -163,10 +162,9 @@ protected Object[] handleArrayResponse(Response response) { if (value instanceof Object[]) { return (Object[]) value; } + String className = (value == null) ? "null" : value.getClass().getSimpleName(); throw new RedisException( - "Unexpected return type from Redis: got " - + value.getClass().getSimpleName() - + " expected Object[]"); + "Unexpected return type from Redis: got " + className + " expected Object[]"); } /** @@ -183,10 +181,9 @@ protected Map handleMapResponse(Response response) { if (value instanceof Map) { return (Map) value; } + String className = (value == null) ? "null" : value.getClass().getSimpleName(); throw new RedisException( - "Unexpected return type from Redis: got " - + value.getClass().getSimpleName() - + " expected Map"); + "Unexpected return type from Redis: got " + className + " expected Map"); } @Override diff --git a/java/client/src/main/java/glide/api/commands/StringCommands.java b/java/client/src/main/java/glide/api/commands/StringCommands.java index 76f62ca13b..983888b399 100644 --- a/java/client/src/main/java/glide/api/commands/StringCommands.java +++ b/java/client/src/main/java/glide/api/commands/StringCommands.java @@ -7,7 +7,11 @@ import glide.api.models.commands.SetOptions.SetOptionsBuilder; import java.util.concurrent.CompletableFuture; -/** String Commands interface to handle single commands that return Strings. */ +/** + * String Commands interface to handle single commands. + * + * @see String Commands + */ public interface StringCommands { /** diff --git a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java index 51be1a6f54..cfaf7dc5ca 100644 --- a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java +++ b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java @@ -89,4 +89,4 @@ public boolean isSingleNodeRoute() { return true; } } -} \ No newline at end of file +} diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index d70e4fcd0e..6d32c8b763 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -9,7 +9,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index 1c48bfa1ec..bef77683b0 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -5,7 +5,6 @@ import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; diff --git a/java/client/src/test/java/glide/managers/CommandManagerTest.java b/java/client/src/test/java/glide/managers/CommandManagerTest.java index 67989043d8..991cec15c5 100644 --- a/java/client/src/test/java/glide/managers/CommandManagerTest.java +++ b/java/client/src/test/java/glide/managers/CommandManagerTest.java @@ -18,11 +18,9 @@ import glide.api.models.configuration.RequestRoutingConfiguration.SlotIdRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotType; -import glide.api.models.exceptions.ClosingException; import glide.connectors.handlers.ChannelHandler; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; From a3f0bb17c607770179b2c29b2bf9eef2549ebd13 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 12:58:22 -0800 Subject: [PATCH 08/36] revert RequestRoutingConfiguration.java Signed-off-by: Andrew Carbonetto --- .../RequestRoutingConfiguration.java | 20 ++++--------- .../glide/managers/CommandManagerTest.java | 30 +++++++++++++++++-- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java index cfaf7dc5ca..acb729af72 100644 --- a/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java +++ b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java @@ -3,8 +3,6 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; -import redis_request.RedisRequestOuterClass.SimpleRoutes; -import redis_request.RedisRequestOuterClass.SlotTypes; /** Request routing configuration. */ public class RequestRoutingConfiguration { @@ -22,17 +20,13 @@ public interface Route { boolean isSingleNodeRoute(); } - @RequiredArgsConstructor - @Getter public enum SimpleRoute implements Route { /** Route request to all nodes. */ - ALL_NODES(SimpleRoutes.AllNodes), + ALL_NODES, /** Route request to all primary nodes. */ - ALL_PRIMARIES(SimpleRoutes.AllPrimaries), + ALL_PRIMARIES, /** Route request to a random node. */ - RANDOM(SimpleRoutes.Random); - - private final SimpleRoutes protobufMapping; + RANDOM; @Override public boolean isSingleNodeRoute() { @@ -40,13 +34,9 @@ public boolean isSingleNodeRoute() { } } - @RequiredArgsConstructor - @Getter public enum SlotType { - PRIMARY(SlotTypes.Primary), - REPLICA(SlotTypes.Replica); - - private final SlotTypes slotTypes; + PRIMARY, + REPLICA, } /** diff --git a/java/client/src/test/java/glide/managers/CommandManagerTest.java b/java/client/src/test/java/glide/managers/CommandManagerTest.java index 991cec15c5..eb4bcc0bfd 100644 --- a/java/client/src/test/java/glide/managers/CommandManagerTest.java +++ b/java/client/src/test/java/glide/managers/CommandManagerTest.java @@ -19,6 +19,7 @@ import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotType; import glide.connectors.handlers.ChannelHandler; +import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; import lombok.SneakyThrows; @@ -28,6 +29,8 @@ import org.junit.jupiter.params.provider.EnumSource; import org.mockito.ArgumentCaptor; import redis_request.RedisRequestOuterClass.RedisRequest; +import redis_request.RedisRequestOuterClass.SimpleRoutes; +import redis_request.RedisRequestOuterClass.SlotTypes; import response.ResponseOuterClass.Response; public class CommandManagerTest { @@ -131,12 +134,19 @@ public void prepare_request_with_simple_routes(SimpleRoute routeType) { verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); + var protobufToClientRouteMapping = + Map.of( + SimpleRoutes.AllNodes, SimpleRoute.ALL_NODES, + SimpleRoutes.AllPrimaries, SimpleRoute.ALL_PRIMARIES, + SimpleRoutes.Random, SimpleRoute.RANDOM); + assertAll( () -> assertTrue(requestBuilder.hasRoute()), () -> assertTrue(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertEquals( - routeType.getProtobufMapping(), requestBuilder.getRoute().getSimpleRoutes()), + routeType, + protobufToClientRouteMapping.get(requestBuilder.getRoute().getSimpleRoutes())), () -> assertFalse(requestBuilder.getRoute().hasSlotIdRoute()), () -> assertFalse(requestBuilder.getRoute().hasSlotKeyRoute())); } @@ -155,12 +165,19 @@ public void prepare_request_with_slot_id_routes(SlotType slotType) { verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); + var protobufToClientRouteMapping = + Map.of( + SlotTypes.Primary, SlotType.PRIMARY, + SlotTypes.Replica, SlotType.REPLICA); + assertAll( () -> assertTrue(requestBuilder.hasRoute()), () -> assertTrue(requestBuilder.getRoute().hasSlotIdRoute()), () -> assertEquals( - slotType.getSlotTypes(), requestBuilder.getRoute().getSlotIdRoute().getSlotType()), + slotType, + protobufToClientRouteMapping.get( + requestBuilder.getRoute().getSlotIdRoute().getSlotType())), () -> assertEquals(42, requestBuilder.getRoute().getSlotIdRoute().getSlotId()), () -> assertFalse(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertFalse(requestBuilder.getRoute().hasSlotKeyRoute())); @@ -180,12 +197,19 @@ public void prepare_request_with_slot_key_routes(SlotType slotType) { verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); + var protobufToClientRouteMapping = + Map.of( + SlotTypes.Primary, SlotType.PRIMARY, + SlotTypes.Replica, SlotType.REPLICA); + assertAll( () -> assertTrue(requestBuilder.hasRoute()), () -> assertTrue(requestBuilder.getRoute().hasSlotKeyRoute()), () -> assertEquals( - slotType.getSlotTypes(), requestBuilder.getRoute().getSlotKeyRoute().getSlotType()), + slotType, + protobufToClientRouteMapping.get( + requestBuilder.getRoute().getSlotKeyRoute().getSlotType())), () -> assertEquals("TEST", requestBuilder.getRoute().getSlotKeyRoute().getSlotKey()), () -> assertFalse(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertFalse(requestBuilder.getRoute().hasSlotIdRoute())); From 2abfc004b8ee23f3f57bca45304bcbe55fa38d84 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 13:20:50 -0800 Subject: [PATCH 09/36] Add IT tests Signed-off-by: Andrew Carbonetto --- .../test/java/glide/cluster/CommandTests.java | 93 ++++++++++++++- .../java/glide/standalone/CommandTests.java | 106 +++++++++++++++++- 2 files changed, 191 insertions(+), 8 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 4ad99b552f..c5d1725d9e 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1,13 +1,22 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.cluster; +import static glide.api.models.commands.InfoOptions.Section.CLUSTER; +import static glide.api.models.commands.InfoOptions.Section.CPU; +import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; +import static glide.api.models.commands.InfoOptions.Section.MEMORY; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import glide.TestConfiguration; import glide.api.RedisClusterClient; +import glide.api.models.ClusterValue; +import glide.api.models.commands.InfoOptions; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClusterClientConfiguration; +import java.util.List; import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; @@ -27,19 +36,20 @@ public static void init() { .address(NodeAddress.builder().port(TestConfiguration.CLUSTER_PORTS[0]).build()) .requestTimeout(5000) .build()) - .get(10, TimeUnit.SECONDS); + .get(10, SECONDS); } @AfterAll @SneakyThrows - public static void deinit() { + public static void teardown() { clusterClient.close(); } @Test @SneakyThrows public void custom_command_info() { - var data = clusterClient.customCommand(new String[] {"info"}).get(10, TimeUnit.SECONDS); + ClusterValue data = clusterClient.customCommand(new String[] {"info"}).get(10, SECONDS); + assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { assertTrue(((String) info).contains("# Stats")); } @@ -51,4 +61,81 @@ public void custom_command_ping() { var data = clusterClient.customCommand(new String[] {"ping"}).get(10, TimeUnit.SECONDS); assertEquals("PONG", data.getSingleValue()); } + + @Test + @SneakyThrows + public void info_without_options() { + ClusterValue data = clusterClient.info().get(10, SECONDS); + assertTrue(data.hasMultiData()); + for (var info : data.getMultiValue().values()) { + for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace")) { + assertTrue(info.contains("# " + section), "Section " + section + " is missing"); + } + } + } + + @Test + @SneakyThrows + public void info_with_route() { + ClusterValue data = clusterClient.info(RANDOM).get(10, SECONDS); + assertTrue(data.hasSingleData()); + String infoData = data.getSingleValue(); + for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace")) { + assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); + } + } + + @Test + @SneakyThrows + public void info_with_multiple_options() { + InfoOptions options = InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); + ClusterValue data = clusterClient.info(options).get(10, SECONDS); + assertTrue(data.hasMultiData()); + for (var info : data.getMultiValue().values()) { + + for (var section : options.toArgs()) { + assertTrue(info.toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); + } + } + } + + @Test + @SneakyThrows + public void info_with_everything_option() { + InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); + ClusterValue data = clusterClient.info(options).get(10, SECONDS); + assertTrue(data.hasMultiData()); + for (var info : data.getMultiValue().values()) { + + for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace")) { + assertTrue(info.contains("# " + section), "Section " + section + " is missing"); + } + } + } + + @Test + @SneakyThrows + public void info_with_everything_option_and_route() { + InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); + ClusterValue data = clusterClient.info(options, RANDOM).get(10, SECONDS); + assertTrue(data.hasSingleData()); + String infoData = data.getSingleValue(); + for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace")) { + assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); + } + } + + @Test + @SneakyThrows + public void ping() { + String data = clusterClient.ping().get(10, SECONDS); + assertEquals("PONG", data); + } + + @Test + @SneakyThrows + public void ping_with_message() { + String data = clusterClient.ping("H3LL0").get(10, SECONDS); + assertEquals("H3LL0", data); + } } diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 0c3f0723f5..6e1326f60c 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -1,22 +1,36 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.standalone; +import static glide.api.models.commands.InfoOptions.Section.CLUSTER; +import static glide.api.models.commands.InfoOptions.Section.CPU; +import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; +import static glide.api.models.commands.InfoOptions.Section.MEMORY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static java.util.concurrent.TimeUnit.SECONDS; import glide.TestConfiguration; import glide.api.RedisClient; +import glide.api.models.Ok; +import glide.api.models.commands.InfoOptions; +import glide.api.models.commands.SetOptions; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClientConfiguration; -import java.util.concurrent.TimeUnit; +import java.util.List; +import java.util.Random; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; public class CommandTests { - private static RedisClient regularClient = null; + private static final String KEY_NAME = "key"; + private static final String INITIAL_VALUE = "VALUE"; + private static final String ANOTHER_VALUE = "VALUE2"; + @BeforeAll @SneakyThrows public static void init() { @@ -26,19 +40,101 @@ public static void init() { .address( NodeAddress.builder().port(TestConfiguration.STANDALONE_PORTS[0]).build()) .build()) - .get(10, TimeUnit.SECONDS); + .get(10, SECONDS); } @AfterAll @SneakyThrows - public static void deinit() { + public static void teardown() { regularClient.close(); } @Test @SneakyThrows public void custom_command_info() { - var data = regularClient.customCommand(new String[] {"info"}).get(10, TimeUnit.SECONDS); + var data = regularClient.customCommand(new String[] {"info"}).get(10, SECONDS); assertTrue(((String) data).contains("# Stats")); } + + @Test + @SneakyThrows + public void info_without_options() { + String data = regularClient.info().get(10, SECONDS); + for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace")) { + assertTrue(data.contains("# " + section), "Section " + section + " is missing"); + } + } + + @Test + @SneakyThrows + public void info_with_multiple_options() { + InfoOptions options = InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); + String data = regularClient.info(options).get(10, SECONDS); + for (var section : options.toArgs()) { + assertTrue(data.toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); + } + } + + @Test + @SneakyThrows + public void info_with_everything_option() { + InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); + String data = regularClient.info(options).get(10, SECONDS); + for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace")) { + assertTrue(data.contains("# " + section), "Section " + section + " is missing"); + } + } + + @Test + @SneakyThrows + public void set_and_get_without_options() { + Ok ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); + assertEquals(Ok.INSTANCE, ok); + + String data = regularClient.get(KEY_NAME).get(10, SECONDS); + assertEquals(INITIAL_VALUE, data); + } + + @Test + @SneakyThrows + public void get_missing_value() { + var data = regularClient.get("invalid").get(10, SECONDS); + assertNull(data); + } + + @Test + @SneakyThrows + public void set_overwrite_value_and_returnOldValue_returns_string() { + var ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); + assertEquals(Ok.INSTANCE, ok); + + var options = SetOptions.builder().returnOldValue(true).build(); + var data = regularClient.set(KEY_NAME, ANOTHER_VALUE, options).get(10, SECONDS); + assertEquals(INITIAL_VALUE, data); + } + + @Test + @SneakyThrows + public void set_missing_value_and_returnOldValue_is_null() { + var ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); + assertEquals(Ok.INSTANCE, ok); + + var options = SetOptions.builder().returnOldValue(true).build(); + var data = regularClient.set("another", ANOTHER_VALUE, options).get(10, SECONDS); + assertNull(data); + } + + @Test + @SneakyThrows + public void ping() { + var data = regularClient.ping().get(10, SECONDS); + assertEquals("PONG", data); + } + + @Test + @SneakyThrows + public void ping_with_message() { + var data = regularClient.ping("H3LL0").get(10, SECONDS); + assertEquals("H3LL0", data); + } } From ad8895b0cc1efaf437e5befbd7206031755415d1 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 13:21:49 -0800 Subject: [PATCH 10/36] Make Route nonnull Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/RedisClusterClient.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index af4665070d..f45f31500a 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -15,6 +15,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import lombok.NonNull; /** * Async (non-blocking) client for Redis in Cluster mode. Use {@link #CreateClient} to request a @@ -50,11 +51,11 @@ public CompletableFuture> customCommand(String... args) { @Override @SuppressWarnings("unchecked") - public CompletableFuture> customCommand(Route route, String... args) { + public CompletableFuture> customCommand(@NonNull Route route, String... args) { return commandManager.submitNewCommand( CustomCommand, args, - Optional.ofNullable(route), + Optional.of(route), response -> route.isSingleNodeRoute() ? ClusterValue.ofSingleValue(handleObjectResponse(response)) @@ -71,11 +72,11 @@ public CompletableFuture> info() { } @Override - public CompletableFuture> info(Route route) { + public CompletableFuture> info(@NonNull Route route) { return commandManager.submitNewCommand( Info, new String[0], - Optional.ofNullable(route), + Optional.of(route), response -> ClusterValue.of(handleObjectResponse(response))); } @@ -89,11 +90,11 @@ public CompletableFuture> info(InfoOptions options) { } @Override - public CompletableFuture> info(InfoOptions options, Route route) { + public CompletableFuture> info(InfoOptions options, @NonNull Route route) { return commandManager.submitNewCommand( Info, options.toArgs(), - Optional.ofNullable(route), + Optional.of(route), response -> ClusterValue.of(handleObjectResponse(response))); } } From f85625fab9d3ffeabcae7ff0880538f5de381245 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 14:00:48 -0800 Subject: [PATCH 11/36] Spotless Signed-off-by: Andrew Carbonetto --- .../java/glide/api/RedisClusterClient.java | 3 +- .../test/java/glide/cluster/CommandTests.java | 69 +++++++++++++++++-- .../java/glide/standalone/CommandTests.java | 42 +++++++++-- 3 files changed, 99 insertions(+), 15 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index f45f31500a..f39b0da8ef 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -51,7 +51,8 @@ public CompletableFuture> customCommand(String... args) { @Override @SuppressWarnings("unchecked") - public CompletableFuture> customCommand(@NonNull Route route, String... args) { + public CompletableFuture> customCommand( + @NonNull Route route, String... args) { return commandManager.submitNewCommand( CustomCommand, args, diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index c5d1725d9e..852303817f 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -68,7 +68,19 @@ public void info_without_options() { ClusterValue data = clusterClient.info().get(10, SECONDS); assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { - for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace")) { + for (var section : + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Errorstats", + "Cluster", + "Keyspace")) { assertTrue(info.contains("# " + section), "Section " + section + " is missing"); } } @@ -80,7 +92,19 @@ public void info_with_route() { ClusterValue data = clusterClient.info(RANDOM).get(10, SECONDS); assertTrue(data.hasSingleData()); String infoData = data.getSingleValue(); - for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace")) { + for (var section : + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Errorstats", + "Cluster", + "Keyspace")) { assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); } } @@ -88,13 +112,16 @@ public void info_with_route() { @Test @SneakyThrows public void info_with_multiple_options() { - InfoOptions options = InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); + InfoOptions options = + InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); ClusterValue data = clusterClient.info(options).get(10, SECONDS); assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { - for (var section : options.toArgs()) { - assertTrue(info.toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); + for (var section : options.toArgs()) { + assertTrue( + info.toLowerCase().contains("# " + section.toLowerCase()), + "Section " + section + " is missing"); } } } @@ -107,7 +134,21 @@ public void info_with_everything_option() { assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { - for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace")) { + for (var section : + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Commandstats", + "Errorstats", + "Latencystats", + "Cluster", + "Keyspace")) { assertTrue(info.contains("# " + section), "Section " + section + " is missing"); } } @@ -120,7 +161,21 @@ public void info_with_everything_option_and_route() { ClusterValue data = clusterClient.info(options, RANDOM).get(10, SECONDS); assertTrue(data.hasSingleData()); String infoData = data.getSingleValue(); - for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace")) { + for (var section : + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Commandstats", + "Errorstats", + "Latencystats", + "Cluster", + "Keyspace")) { assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); } } diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 6e1326f60c..76277a28c5 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -5,10 +5,10 @@ import static glide.api.models.commands.InfoOptions.Section.CPU; import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; import static glide.api.models.commands.InfoOptions.Section.MEMORY; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static java.util.concurrent.TimeUnit.SECONDS; import glide.TestConfiguration; import glide.api.RedisClient; @@ -18,7 +18,6 @@ import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClientConfiguration; import java.util.List; -import java.util.Random; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -60,7 +59,19 @@ public void custom_command_info() { @SneakyThrows public void info_without_options() { String data = regularClient.info().get(10, SECONDS); - for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace")) { + for (var section : + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Errorstats", + "Cluster", + "Keyspace")) { assertTrue(data.contains("# " + section), "Section " + section + " is missing"); } } @@ -68,10 +79,13 @@ public void info_without_options() { @Test @SneakyThrows public void info_with_multiple_options() { - InfoOptions options = InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); + InfoOptions options = + InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); String data = regularClient.info(options).get(10, SECONDS); - for (var section : options.toArgs()) { - assertTrue(data.toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); + for (var section : options.toArgs()) { + assertTrue( + data.toLowerCase().contains("# " + section.toLowerCase()), + "Section " + section + " is missing"); } } @@ -80,7 +94,21 @@ public void info_with_multiple_options() { public void info_with_everything_option() { InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); String data = regularClient.info(options).get(10, SECONDS); - for (var section : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace")) { + for (var section : + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Commandstats", + "Errorstats", + "Latencystats", + "Cluster", + "Keyspace")) { assertTrue(data.contains("# " + section), "Section " + section + " is missing"); } } From 5766c3d3cab6b049c12043bc83be8e808a23362a Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 15:14:15 -0800 Subject: [PATCH 12/36] Apply review comments to make class/feilds final Signed-off-by: Andrew Carbonetto --- .../java/glide/api/RedisClusterClient.java | 2 + .../src/main/java/glide/api/models/Ok.java | 4 +- .../glide/api/models/commands/SetOptions.java | 6 +- .../glide/api/RedisClusterClientTest.java | 43 +++-- .../test/java/glide/cluster/CommandTests.java | 177 ++++++++++++------ .../java/glide/standalone/CommandTests.java | 59 +++--- 6 files changed, 184 insertions(+), 107 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index f39b0da8ef..56b2fa48f0 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -41,6 +41,7 @@ public static CompletableFuture CreateClient( @Override public CompletableFuture> customCommand(String... args) { + if (args.length == 0) throw new IllegalArgumentException("No arguments provided"); // TODO if a command returns a map as a single value, ClusterValue misleads user return commandManager.submitNewCommand( CustomCommand, @@ -53,6 +54,7 @@ public CompletableFuture> customCommand(String... args) { @SuppressWarnings("unchecked") public CompletableFuture> customCommand( @NonNull Route route, String... args) { + if (args.length == 0) throw new IllegalArgumentException("No arguments provided"); return commandManager.submitNewCommand( CustomCommand, args, diff --git a/java/client/src/main/java/glide/api/models/Ok.java b/java/client/src/main/java/glide/api/models/Ok.java index cb9f7e4c17..0d50aee5bc 100644 --- a/java/client/src/main/java/glide/api/models/Ok.java +++ b/java/client/src/main/java/glide/api/models/Ok.java @@ -10,9 +10,9 @@ * @see Simple Strings * response */ -public class Ok { +public final class Ok { - public static Ok INSTANCE = new Ok(); + public static final Ok INSTANCE = new Ok(); // Constructor is private - use Ok.INSTANCE instead private Ok() {} diff --git a/java/client/src/main/java/glide/api/models/commands/SetOptions.java b/java/client/src/main/java/glide/api/models/commands/SetOptions.java index 982d09ba4f..f9e2a7740f 100644 --- a/java/client/src/main/java/glide/api/models/commands/SetOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/SetOptions.java @@ -3,7 +3,7 @@ import glide.api.commands.StringCommands; import glide.api.models.exceptions.RequestException; -import java.util.LinkedList; +import java.util.ArrayList; import java.util.List; import lombok.Builder; import lombok.Getter; @@ -96,7 +96,7 @@ public enum TimeToLiveType { } /** String representation of {@link #returnOldValue} when set. */ - public static String RETURN_OLD_VALUE = "GET"; + public static final String RETURN_OLD_VALUE = "GET"; /** * Converts SetOptions into a String[] to add to a {@link Command} arguments. @@ -104,7 +104,7 @@ public enum TimeToLiveType { * @return String[] */ public String[] toArgs() { - List optionArgs = new LinkedList<>(); + List optionArgs = new ArrayList<>(); if (conditionalSet != null) { optionArgs.add(conditionalSet.redisApi); } diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index bef77683b0..b4934383ee 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -5,6 +5,7 @@ import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -21,6 +22,7 @@ import glide.managers.RedisExceptionCheckedFunction; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; @@ -50,7 +52,7 @@ public void customCommand_returns_single_value() { var client = new TestClient(commandManager, "TEST"); - var value = client.customCommand().get(); + var value = client.customCommand("test").get(); assertAll( () -> assertTrue(value.hasSingleData()), () -> assertEquals("TEST", value.getSingleValue())); @@ -64,7 +66,7 @@ public void customCommand_returns_multi_value() { var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand().get(); + var value = client.customCommand("test").get(); assertAll( () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); } @@ -78,7 +80,7 @@ public void customCommand_with_single_node_route_returns_single_value() { var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(RANDOM).get(); + var value = client.customCommand(RANDOM, "Test").get(); assertAll( () -> assertTrue(value.hasSingleData()), () -> assertEquals(data, value.getSingleValue())); } @@ -91,7 +93,7 @@ public void customCommand_with_multi_node_route_returns_multi_value() { var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(ALL_NODES).get(); + var value = client.customCommand(ALL_NODES, "Test").get(); assertAll( () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); } @@ -139,7 +141,7 @@ public void customCommand_success() { CompletableFuture> testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn(ClusterValue.of(value)); when(commandManager.>submitNewCommand( - eq(CustomCommand), any(), any(), any())) + eq(CustomCommand), eq(arguments), any(), any())) .thenReturn(testResponse); // exercise @@ -147,23 +149,30 @@ public void customCommand_success() { // verify assertEquals(testResponse, response); - ClusterValue clusterValue = response.get(); + ClusterValue clusterValue = response.get(); assertTrue(clusterValue.hasSingleData()); String payload = (String) clusterValue.getSingleValue(); assertEquals(value, payload); } + @Test + @SneakyThrows + public void customCommand_requires_args() { + assertThrows(IllegalArgumentException.class, () -> service.customCommand()); + assertThrows(IllegalArgumentException.class, () -> service.customCommand(RANDOM)); + } + @SneakyThrows @Test public void info_returns_string() { // setup - CompletableFuture> testResponse = mock(CompletableFuture.class); - Map testPayload = new HashMap(); + CompletableFuture> testResponse = mock(CompletableFuture.class); + Map testPayload = new HashMap(); testPayload.put("addr1", "value1"); - testPayload.put("addr1", "value2"); - testPayload.put("addr1", "value3"); + testPayload.put("addr2", "value2"); + testPayload.put("addr3", "value3"); when(testResponse.get()).thenReturn(ClusterValue.of(testPayload)); - when(commandManager.>submitNewCommand( + when(commandManager.>submitNewCommand( eq(Info), eq(new String[0]), any(), any())) .thenReturn(testResponse); @@ -171,7 +180,6 @@ public void info_returns_string() { CompletableFuture> response = service.info(); // verify - assertEquals(testResponse, response); ClusterValue clusterValue = response.get(); assertTrue(clusterValue.hasMultiData()); Map payload = clusterValue.getMultiValue(); @@ -182,18 +190,17 @@ public void info_returns_string() { @Test public void info_with_route_returns_string() { // setup - CompletableFuture> testResponse = mock(CompletableFuture.class); + CompletableFuture> testResponse = mock(CompletableFuture.class); Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_NODES; when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); - when(commandManager.>submitNewCommand(eq(Info), any(), any(), any())) + when(commandManager.>submitNewCommand(eq(Info), any(), any(), any())) .thenReturn(testResponse); // exercise CompletableFuture> response = service.info(route); // verify - assertEquals(testResponse, response); ClusterValue clusterValue = response.get(); assertTrue(clusterValue.hasMultiData()); Map clusterMap = clusterValue.getMultiValue(); @@ -206,12 +213,12 @@ public void info_with_route_returns_string() { public void info_with_route_with_infoOptions_returns_string() { // setup String[] infoArguments = new String[] {"ALL", "DEFAULT"}; - CompletableFuture> testResponse = mock(CompletableFuture.class); + CompletableFuture> testResponse = mock(CompletableFuture.class); Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_PRIMARIES; - when(commandManager.>submitNewCommand( - eq(Info), eq(infoArguments), any(), any())) + when(commandManager.>submitNewCommand( + eq(Info), eq(infoArguments), eq(Optional.of(route)), any())) .thenReturn(testResponse); // exercise diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 852303817f..25dd898b83 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1,21 +1,32 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.cluster; +import static glide.api.models.commands.InfoOptions.Section.CLIENTS; import static glide.api.models.commands.InfoOptions.Section.CLUSTER; +import static glide.api.models.commands.InfoOptions.Section.COMMANDSTATS; import static glide.api.models.commands.InfoOptions.Section.CPU; import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; import static glide.api.models.commands.InfoOptions.Section.MEMORY; +import static glide.api.models.commands.InfoOptions.Section.REPLICATION; +import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; +import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_EXISTS; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_NODES; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; +import static glide.api.models.configuration.RequestRoutingConfiguration.SlotType.PRIMARY; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import glide.TestConfiguration; import glide.api.RedisClusterClient; import glide.api.models.ClusterValue; import glide.api.models.commands.InfoOptions; +import glide.api.models.commands.SetOptions; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClusterClientConfiguration; +import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; import java.util.List; import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; @@ -27,6 +38,13 @@ public class CommandTests { private static RedisClusterClient clusterClient = null; + private static final String KEY_NAME = "key"; + private static final String INITIAL_VALUE = "VALUE"; + private static final String ANOTHER_VALUE = "VALUE2"; + + private static final List DEFAULT_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace"); + private static final List EVERYTHING_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace"); + @BeforeAll @SneakyThrows public static void init() { @@ -68,19 +86,7 @@ public void info_without_options() { ClusterValue data = clusterClient.info().get(10, SECONDS); assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { - for (var section : - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Errorstats", - "Cluster", - "Keyspace")) { + for (var section : DEFAULT_INFO_SECTIONS) { assertTrue(info.contains("# " + section), "Section " + section + " is missing"); } } @@ -92,19 +98,7 @@ public void info_with_route() { ClusterValue data = clusterClient.info(RANDOM).get(10, SECONDS); assertTrue(data.hasSingleData()); String infoData = data.getSingleValue(); - for (var section : - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Errorstats", - "Cluster", - "Keyspace")) { + for (var section : DEFAULT_INFO_SECTIONS) { assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); } } @@ -134,21 +128,7 @@ public void info_with_everything_option() { assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { - for (var section : - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Commandstats", - "Errorstats", - "Latencystats", - "Cluster", - "Keyspace")) { + for (var section : DEFAULT_INFO_SECTIONS) { assertTrue(info.contains("# " + section), "Section " + section + " is missing"); } } @@ -161,25 +141,36 @@ public void info_with_everything_option_and_route() { ClusterValue data = clusterClient.info(options, RANDOM).get(10, SECONDS); assertTrue(data.hasSingleData()); String infoData = data.getSingleValue(); - for (var section : - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Commandstats", - "Errorstats", - "Latencystats", - "Cluster", - "Keyspace")) { + for (var section : EVERYTHING_INFO_SECTIONS) { assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); } } + @Test + @SneakyThrows + public void info_with_routing_and_options() { + var slotData = clusterClient.customCommand("cluster", "slots").get(10, SECONDS); + /* + Nested Object arrays like + 1) 1) (integer) 0 + 2) (integer) 5460 + 3) 1) "127.0.0.1" + 2) (integer) 7000 + 3) "92d73b6eb847604b63c7f7cbbf39b148acdd1318" + 4) (empty array) + */ + // Extracting first slot key + var slotKey = (String)((Object[])((Object[])((Object[])slotData.getSingleValue())[0])[2])[2]; + + var options = InfoOptions.builder().section(CLIENTS).section(COMMANDSTATS).section(REPLICATION).build(); + var routing = new SlotKeyRoute(slotKey, PRIMARY); + var data = clusterClient.info(options, routing).get(10, SECONDS); + + for (var section : options.toArgs()) { + assertTrue(data.getSingleValue().toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); + } + } + @Test @SneakyThrows public void ping() { @@ -193,4 +184,78 @@ public void ping_with_message() { String data = clusterClient.ping("H3LL0").get(10, SECONDS); assertEquals("H3LL0", data); } + + @Test + @SneakyThrows + public void custom_command_del_returns_a_number() { + clusterClient.set("DELME", INITIAL_VALUE).get(10, SECONDS); + var del = clusterClient.customCommand("DEL", "DELME").get(10, SECONDS); + assertEquals(1L, del.getSingleValue()); + var data = clusterClient.get("DELME").get(10, SECONDS); + assertNull(data); + } + + @Test + public void set_requires_a_value() { + assertThrows(NullPointerException.class, () -> clusterClient.set("SET", null)); + } + + @Test + public void set_requires_a_key() { + assertThrows(NullPointerException.class, () -> clusterClient.set(null, INITIAL_VALUE)); + } + + @Test + public void get_requires_a_key() { + assertThrows(NullPointerException.class, () -> clusterClient.get(null)); + } + + @Test + public void ping_with_message_requires_a_message() { + assertThrows(NullPointerException.class, () -> clusterClient.ping(null)); + } + + @Test + public void custom_command_requires_args() { + assertThrows(IllegalArgumentException.class, () -> clusterClient.customCommand()); + assertThrows(IllegalArgumentException.class, () -> clusterClient.customCommand(ALL_NODES)); + } + + @Test + @SneakyThrows + public void set_only_if_exists_overwrite() { + var options = SetOptions.builder().conditionalSet(ONLY_IF_EXISTS).build(); + clusterClient.set("set_only_if_exists_overwrite", INITIAL_VALUE).get(10, SECONDS); + clusterClient.set("set_only_if_exists_overwrite", ANOTHER_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_only_if_exists_overwrite").get(10, SECONDS); + assertEquals(ANOTHER_VALUE, data); + } + + @Test + @SneakyThrows + public void set_only_if_exists_missing_key() { + var options = SetOptions.builder().conditionalSet(ONLY_IF_EXISTS).build(); + clusterClient.set("set_only_if_exists_missing_key", ANOTHER_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_only_if_exists_missing_key").get(10, SECONDS); + assertNull(data); + } + + @Test + @SneakyThrows + public void set_only_if_does_not_exists_missing_key() { + var options = SetOptions.builder().conditionalSet(ONLY_IF_DOES_NOT_EXIST).build(); + clusterClient.set("set_only_if_does_not_exists_missing_key", ANOTHER_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_only_if_does_not_exists_missing_key").get(10, SECONDS); + assertEquals(ANOTHER_VALUE, data); + } + + @Test + @SneakyThrows + public void set_only_if_does_not_exists_existing_key() { + var options = SetOptions.builder().conditionalSet(ONLY_IF_DOES_NOT_EXIST).build(); + clusterClient.set("set_only_if_does_not_exists_existing_key", INITIAL_VALUE).get(10, SECONDS); + clusterClient.set("set_only_if_does_not_exists_existing_key", ANOTHER_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_only_if_does_not_exists_existing_key").get(10, SECONDS); + assertEquals(INITIAL_VALUE, data); + } } diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 76277a28c5..bcd8949587 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -30,6 +30,35 @@ public class CommandTests { private static final String INITIAL_VALUE = "VALUE"; private static final String ANOTHER_VALUE = "VALUE2"; + private static final List DEFAULT_INFO_SECTIONS = + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Errorstats", + "Cluster", + "Keyspace"); + private static final List EVERYTHING_INFO_SECTIONS = + List.of( + "Server", + "Clients", + "Memory", + "Persistence", + "Stats", + "Replication", + "CPU", + "Modules", + "Commandstats", + "Errorstats", + "Latencystats", + "Cluster", + "Keyspace"); + @BeforeAll @SneakyThrows public static void init() { @@ -59,19 +88,7 @@ public void custom_command_info() { @SneakyThrows public void info_without_options() { String data = regularClient.info().get(10, SECONDS); - for (var section : - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Errorstats", - "Cluster", - "Keyspace")) { + for (var section : DEFAULT_INFO_SECTIONS) { assertTrue(data.contains("# " + section), "Section " + section + " is missing"); } } @@ -94,21 +111,7 @@ public void info_with_multiple_options() { public void info_with_everything_option() { InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); String data = regularClient.info(options).get(10, SECONDS); - for (var section : - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Commandstats", - "Errorstats", - "Latencystats", - "Cluster", - "Keyspace")) { + for (var section : EVERYTHING_INFO_SECTIONS) { assertTrue(data.contains("# " + section), "Section " + section + " is missing"); } } From b113b8ada09a5a65adcdb68c2e73303ad321ee0c Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 15:19:31 -0800 Subject: [PATCH 13/36] Add more IT tests; fix redis version infooptions Signed-off-by: Andrew Carbonetto --- .../test/java/glide/TestConfiguration.java | 1 + .../test/java/glide/cluster/CommandTests.java | 74 ++++++++++++++++--- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/java/integTest/src/test/java/glide/TestConfiguration.java b/java/integTest/src/test/java/glide/TestConfiguration.java index fedfb389a5..1b726bdc40 100644 --- a/java/integTest/src/test/java/glide/TestConfiguration.java +++ b/java/integTest/src/test/java/glide/TestConfiguration.java @@ -7,6 +7,7 @@ public final class TestConfiguration { // All redis servers are hosted on localhost public static final int[] STANDALONE_PORTS = getPortsFromProperty("test.redis.standalone.ports"); public static final int[] CLUSTER_PORTS = getPortsFromProperty("test.redis.cluster.ports"); + public static final String REDIS_VERSION = System.getProperty("test.redis.version"); private static int[] getPortsFromProperty(String propName) { return Arrays.stream(System.getProperty(propName).split(",")) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 25dd898b83..cf8921a3ed 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -10,6 +10,9 @@ import static glide.api.models.commands.InfoOptions.Section.REPLICATION; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_EXISTS; +import static glide.api.models.commands.SetOptions.TimeToLiveType.KEEP_EXISTING; +import static glide.api.models.commands.SetOptions.TimeToLiveType.MILLISECONDS; +import static glide.api.models.commands.SetOptions.TimeToLiveType.UNIX_SECONDS; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_NODES; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static glide.api.models.configuration.RequestRoutingConfiguration.SlotType.PRIMARY; @@ -24,6 +27,7 @@ import glide.api.models.ClusterValue; import glide.api.models.commands.InfoOptions; import glide.api.models.commands.SetOptions; +import glide.api.models.commands.SetOptions.TimeToLive; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClusterClientConfiguration; import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; @@ -106,16 +110,16 @@ public void info_with_route() { @Test @SneakyThrows public void info_with_multiple_options() { - InfoOptions options = - InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); - ClusterValue data = clusterClient.info(options).get(10, SECONDS); - assertTrue(data.hasMultiData()); + var builder = InfoOptions.builder().section(CLUSTER); + if (TestConfiguration.REDIS_VERSION.startsWith("7")) { + builder.section(CPU).section(MEMORY); + } + var options = builder.build(); + var data = clusterClient.info(options).get(10, SECONDS); for (var info : data.getMultiValue().values()) { - for (var section : options.toArgs()) { - assertTrue( - info.toLowerCase().contains("# " + section.toLowerCase()), - "Section " + section + " is missing"); + for (var section : options.toArgs()) { + assertTrue(info.toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); } } } @@ -162,7 +166,11 @@ public void info_with_routing_and_options() { // Extracting first slot key var slotKey = (String)((Object[])((Object[])((Object[])slotData.getSingleValue())[0])[2])[2]; - var options = InfoOptions.builder().section(CLIENTS).section(COMMANDSTATS).section(REPLICATION).build(); + var builder = InfoOptions.builder().section(CLIENTS); + if (TestConfiguration.REDIS_VERSION.startsWith("7")) { + builder.section(COMMANDSTATS).section(REPLICATION); + } + var options = builder.build(); var routing = new SlotKeyRoute(slotKey, PRIMARY); var data = clusterClient.info(options, routing).get(10, SECONDS); @@ -258,4 +266,52 @@ public void set_only_if_does_not_exists_existing_key() { var data = clusterClient.get("set_only_if_does_not_exists_existing_key").get(10, SECONDS); assertEquals(INITIAL_VALUE, data); } + + @Test + @SneakyThrows + public void set_value_with_ttl_and_update_value_with_keeping_ttl() { + var options = SetOptions.builder().expiry( + SetOptions.TimeToLive.builder().count(2000).type(MILLISECONDS).build()).build(); + clusterClient.set("set_value_with_ttl_and_update_value_with_keeping_ttl", INITIAL_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_value_with_ttl_and_update_value_with_keeping_ttl").get(10, SECONDS); + assertEquals(INITIAL_VALUE, data); + + options = SetOptions.builder().expiry(TimeToLive.builder().type(KEEP_EXISTING).build()).build(); + clusterClient.set("set_value_with_ttl_and_update_value_with_keeping_ttl", ANOTHER_VALUE, options).get(10, SECONDS); + data = clusterClient.get("set_value_with_ttl_and_update_value_with_keeping_ttl").get(10, SECONDS); + assertEquals(ANOTHER_VALUE, data); + + Thread.sleep(2222); // sleep a bit more than TTL + + data = clusterClient.get("set_value_with_ttl_and_update_value_with_keeping_ttl").get(10, SECONDS); + assertNull(data); + } + + @Test + @SneakyThrows + public void set_value_with_ttl_and_update_value_with_new_ttl() { + var options = SetOptions.builder().expiry(TimeToLive.builder().count(100500).type(MILLISECONDS).build()).build(); + clusterClient.set("set_value_with_ttl_and_update_value_with_new_ttl", INITIAL_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_value_with_ttl_and_update_value_with_new_ttl").get(10, SECONDS); + assertEquals(INITIAL_VALUE, data); + + options = SetOptions.builder().expiry(TimeToLive.builder().count(2000).type(MILLISECONDS).build()).build(); + clusterClient.set("set_value_with_ttl_and_update_value_with_new_ttl", ANOTHER_VALUE, options).get(10, SECONDS); + data = clusterClient.get("set_value_with_ttl_and_update_value_with_new_ttl").get(10, SECONDS); + assertEquals(ANOTHER_VALUE, data); + + Thread.sleep(2222); // sleep a bit more than new TTL + + data = clusterClient.get("set_value_with_ttl_and_update_value_with_new_ttl").get(10, SECONDS); + assertNull(data); + } + + @Test + @SneakyThrows + public void set_expired_value() { // expiration is in the past + var options = SetOptions.builder().expiry(TimeToLive.builder().count(100500).type(UNIX_SECONDS).build()).build(); + clusterClient.set("set_expired_value", INITIAL_VALUE, options).get(10, SECONDS); + var data = clusterClient.get("set_expired_value").get(10, SECONDS); + assertNull(data); + } } From 6c8e04b9f3d35f6f5be9ad13c1b4fd1a5644c5ff Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 15:38:08 -0800 Subject: [PATCH 14/36] Fix Redis versioned tests Signed-off-by: Andrew Carbonetto --- java/integTest/build.gradle | 33 ++++++++++++++++--- .../test/java/glide/cluster/CommandTests.java | 6 ++-- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/java/integTest/build.gradle b/java/integTest/build.gradle index 63c0417247..43dc4a495e 100644 --- a/java/integTest/build.gradle +++ b/java/integTest/build.gradle @@ -27,17 +27,25 @@ dependencies { def standaloneRedisPorts = [] def clusterRedisPorts = [] +def redisVersion = "" ext { - extractPortsFromClusterManagerOutput = { String output, ArrayList dest -> + extractPortsFromClusterManagerOutput = { String output -> + var res = [] for (def line : output.split("\n")) { if (!line.startsWith("CLUSTER_NODES=")) continue def addresses = line.split("=")[1].split(",") for (def address : addresses) - dest << address.split(":")[1] + res << address.split(":")[1] } + return res + } + extractRedisVersion = { String output -> + // Line in format like + // Redis server v=7.2.3 sha=00000000:0 malloc=jemalloc-5.3.0 bits=64 build=7504b1fedf883f2 + return output.split(" ")[2].split("=")[1] } } @@ -68,7 +76,7 @@ tasks.register('startCluster') { commandLine 'python3', 'cluster_manager.py', 'start', '--cluster-mode' standardOutput = os } - extractPortsFromClusterManagerOutput(os.toString(), clusterRedisPorts) + clusterRedisPorts = extractPortsFromClusterManagerOutput(os.toString()) } } } @@ -81,12 +89,25 @@ tasks.register('startStandalone') { commandLine 'python3', 'cluster_manager.py', 'start', '-r', '0' standardOutput = os } - extractPortsFromClusterManagerOutput(os.toString(), standaloneRedisPorts) + standaloneRedisPorts = extractPortsFromClusterManagerOutput(os.toString()) + } + } +} + +tasks.register('getRedisVersion') { + doLast { + new ByteArrayOutputStream().withStream { os -> + exec { + commandLine 'redis-server', '-v' + standardOutput = os + } + redisVersion = extractRedisVersion(os.toString()) } } } test.dependsOn 'stopAllBeforeTests' +test.dependsOn 'getRedisVersion' stopAllBeforeTests.finalizedBy 'clearDirs' clearDirs.finalizedBy 'startStandalone' clearDirs.finalizedBy 'startCluster' @@ -97,8 +118,10 @@ tasks.withType(Test) { doFirst { println "Cluster ports = ${clusterRedisPorts}" println "Standalone ports = ${standaloneRedisPorts}" + println "Redis version = ${redisVersion}" systemProperty 'test.redis.standalone.ports', standaloneRedisPorts.join(',') systemProperty 'test.redis.cluster.ports', clusterRedisPorts.join(',') + systemProperty 'test.redis.version', redisVersion } testLogging { @@ -110,4 +133,4 @@ tasks.withType(Test) { afterTest { desc, result -> logger.quiet "${desc.className}.${desc.name}: ${result.resultType} ${(result.getEndTime() - result.getStartTime())/1000}s" } -} +} \ No newline at end of file diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index cf8921a3ed..de9eb11537 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -6,6 +6,7 @@ import static glide.api.models.commands.InfoOptions.Section.COMMANDSTATS; import static glide.api.models.commands.InfoOptions.Section.CPU; import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; +import static glide.api.models.commands.InfoOptions.Section.LATENCYSTATS; import static glide.api.models.commands.InfoOptions.Section.MEMORY; import static glide.api.models.commands.InfoOptions.Section.REPLICATION; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; @@ -117,7 +118,6 @@ public void info_with_multiple_options() { var options = builder.build(); var data = clusterClient.info(options).get(10, SECONDS); for (var info : data.getMultiValue().values()) { - for (var section : options.toArgs()) { assertTrue(info.toLowerCase().contains("# " + section.toLowerCase()), "Section " + section + " is missing"); } @@ -146,7 +146,9 @@ public void info_with_everything_option_and_route() { assertTrue(data.hasSingleData()); String infoData = data.getSingleValue(); for (var section : EVERYTHING_INFO_SECTIONS) { - assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); + if (!TestConfiguration.REDIS_VERSION.startsWith("7") && section.equals("Latencystats")) { + assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); + } } } From 4262a4b4d9e556d9254902241f86cf201d03f771 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 16:12:50 -0800 Subject: [PATCH 15/36] Remove redis 6 issue with LatencyStats Signed-off-by: Andrew Carbonetto --- .../integTest/src/test/java/glide/cluster/CommandTests.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index de9eb11537..eb526ac859 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -48,7 +48,7 @@ public class CommandTests { private static final String ANOTHER_VALUE = "VALUE2"; private static final List DEFAULT_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace"); - private static final List EVERYTHING_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace"); + private static final List EVERYTHING_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Cluster", "Keyspace"); @BeforeAll @SneakyThrows @@ -146,9 +146,7 @@ public void info_with_everything_option_and_route() { assertTrue(data.hasSingleData()); String infoData = data.getSingleValue(); for (var section : EVERYTHING_INFO_SECTIONS) { - if (!TestConfiguration.REDIS_VERSION.startsWith("7") && section.equals("Latencystats")) { - assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); - } + assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); } } From 8c22212386d84926c2098c52e2b5ba9761390be0 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 16:14:57 -0800 Subject: [PATCH 16/36] Remove redis 6 issue with LatencyStats Signed-off-by: Andrew Carbonetto --- .../integTest/src/test/java/glide/standalone/CommandTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index bcd8949587..76516455cd 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -55,7 +55,8 @@ public class CommandTests { "Modules", "Commandstats", "Errorstats", - "Latencystats", + // not available in Redis 6 + // "Latencystats", "Cluster", "Keyspace"); From e96c2849313e57d2b987c6a1c85f20d48a312eda Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 16:33:26 -0800 Subject: [PATCH 17/36] Remove EVERYTHING info() test Signed-off-by: Andrew Carbonetto --- .../test/java/glide/cluster/CommandTests.java | 13 ---------- .../java/glide/standalone/CommandTests.java | 26 ------------------- 2 files changed, 39 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index eb526ac859..bcbc790006 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -48,7 +48,6 @@ public class CommandTests { private static final String ANOTHER_VALUE = "VALUE2"; private static final List DEFAULT_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace"); - private static final List EVERYTHING_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Cluster", "Keyspace"); @BeforeAll @SneakyThrows @@ -138,18 +137,6 @@ public void info_with_everything_option() { } } - @Test - @SneakyThrows - public void info_with_everything_option_and_route() { - InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); - ClusterValue data = clusterClient.info(options, RANDOM).get(10, SECONDS); - assertTrue(data.hasSingleData()); - String infoData = data.getSingleValue(); - for (var section : EVERYTHING_INFO_SECTIONS) { - assertTrue(infoData.contains("# " + section), "Section " + section + " is missing"); - } - } - @Test @SneakyThrows public void info_with_routing_and_options() { diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 76516455cd..bc9a8d490d 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -43,22 +43,6 @@ public class CommandTests { "Errorstats", "Cluster", "Keyspace"); - private static final List EVERYTHING_INFO_SECTIONS = - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Commandstats", - "Errorstats", - // not available in Redis 6 - // "Latencystats", - "Cluster", - "Keyspace"); @BeforeAll @SneakyThrows @@ -107,16 +91,6 @@ public void info_with_multiple_options() { } } - @Test - @SneakyThrows - public void info_with_everything_option() { - InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); - String data = regularClient.info(options).get(10, SECONDS); - for (var section : EVERYTHING_INFO_SECTIONS) { - assertTrue(data.contains("# " + section), "Section " + section + " is missing"); - } - } - @Test @SneakyThrows public void set_and_get_without_options() { From 59e7b8f378fff4e0449dc76aa60d42c46e1572ad Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Fri, 2 Feb 2024 16:41:54 -0800 Subject: [PATCH 18/36] spotless Signed-off-by: Andrew Carbonetto --- java/integTest/src/test/java/glide/standalone/CommandTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index bc9a8d490d..f0115a5857 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -3,7 +3,6 @@ import static glide.api.models.commands.InfoOptions.Section.CLUSTER; import static glide.api.models.commands.InfoOptions.Section.CPU; -import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; import static glide.api.models.commands.InfoOptions.Section.MEMORY; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; From ee9054530974bf9f8e3007d40251ffe630980c57 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Fri, 2 Feb 2024 16:55:55 -0800 Subject: [PATCH 19/36] Fix IT. Signed-off-by: Yury-Fridlyand --- .../test/java/glide/TestConfiguration.java | 4 +- .../test/java/glide/cluster/CommandTests.java | 14 ++++--- .../java/glide/standalone/CommandTests.java | 37 ++++++++++--------- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/java/integTest/src/test/java/glide/TestConfiguration.java b/java/integTest/src/test/java/glide/TestConfiguration.java index 1b726bdc40..aa6bfd35b9 100644 --- a/java/integTest/src/test/java/glide/TestConfiguration.java +++ b/java/integTest/src/test/java/glide/TestConfiguration.java @@ -1,13 +1,15 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide; +import java.lang.Runtime.Version; import java.util.Arrays; public final class TestConfiguration { // All redis servers are hosted on localhost public static final int[] STANDALONE_PORTS = getPortsFromProperty("test.redis.standalone.ports"); public static final int[] CLUSTER_PORTS = getPortsFromProperty("test.redis.cluster.ports"); - public static final String REDIS_VERSION = System.getProperty("test.redis.version"); + public static final Version REDIS_VERSION = + Version.parse(System.getProperty("test.redis.version")); private static int[] getPortsFromProperty(String propName) { return Arrays.stream(System.getProperty(propName).split(",")) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index bcbc790006..14ac21c2fa 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -47,7 +47,11 @@ public class CommandTests { private static final String INITIAL_VALUE = "VALUE"; private static final String ANOTHER_VALUE = "VALUE2"; - private static final List DEFAULT_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace"); + public static final List DEFAULT_INFO_SECTIONS = List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Errorstats", "Cluster", "Keyspace"); + public static final List EVERYTHING_INFO_SECTIONS = TestConfiguration.REDIS_VERSION.feature() >= 7 + // Latencystats was added in redis 7 + ? List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Latencystats", "Cluster", "Keyspace") + : List.of("Server", "Clients", "Memory", "Persistence", "Stats", "Replication", "CPU", "Modules", "Commandstats", "Errorstats", "Cluster", "Keyspace"); @BeforeAll @SneakyThrows @@ -65,6 +69,7 @@ public static void init() { @SneakyThrows public static void teardown() { clusterClient.close(); + Runtime.Version.parse("1.2.3"); } @Test @@ -111,7 +116,7 @@ public void info_with_route() { @SneakyThrows public void info_with_multiple_options() { var builder = InfoOptions.builder().section(CLUSTER); - if (TestConfiguration.REDIS_VERSION.startsWith("7")) { + if (TestConfiguration.REDIS_VERSION.feature() >= 7) { builder.section(CPU).section(MEMORY); } var options = builder.build(); @@ -130,8 +135,7 @@ public void info_with_everything_option() { ClusterValue data = clusterClient.info(options).get(10, SECONDS); assertTrue(data.hasMultiData()); for (var info : data.getMultiValue().values()) { - - for (var section : DEFAULT_INFO_SECTIONS) { + for (var section : EVERYTHING_INFO_SECTIONS) { assertTrue(info.contains("# " + section), "Section " + section + " is missing"); } } @@ -154,7 +158,7 @@ public void info_with_routing_and_options() { var slotKey = (String)((Object[])((Object[])((Object[])slotData.getSingleValue())[0])[2])[2]; var builder = InfoOptions.builder().section(CLIENTS); - if (TestConfiguration.REDIS_VERSION.startsWith("7")) { + if (TestConfiguration.REDIS_VERSION.feature() >= 7) { builder.section(COMMANDSTATS).section(REPLICATION); } var options = builder.build(); diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index f0115a5857..2521d76da6 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -3,7 +3,10 @@ import static glide.api.models.commands.InfoOptions.Section.CLUSTER; import static glide.api.models.commands.InfoOptions.Section.CPU; +import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; import static glide.api.models.commands.InfoOptions.Section.MEMORY; +import static glide.cluster.CommandTests.DEFAULT_INFO_SECTIONS; +import static glide.cluster.CommandTests.EVERYTHING_INFO_SECTIONS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -16,7 +19,6 @@ import glide.api.models.commands.SetOptions; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClientConfiguration; -import java.util.List; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -29,20 +31,6 @@ public class CommandTests { private static final String INITIAL_VALUE = "VALUE"; private static final String ANOTHER_VALUE = "VALUE2"; - private static final List DEFAULT_INFO_SECTIONS = - List.of( - "Server", - "Clients", - "Memory", - "Persistence", - "Stats", - "Replication", - "CPU", - "Modules", - "Errorstats", - "Cluster", - "Keyspace"); - @BeforeAll @SneakyThrows public static void init() { @@ -64,7 +52,7 @@ public static void teardown() { @Test @SneakyThrows public void custom_command_info() { - var data = regularClient.customCommand(new String[] {"info"}).get(10, SECONDS); + var data = regularClient.customCommand("info").get(10, SECONDS); assertTrue(((String) data).contains("# Stats")); } @@ -80,8 +68,11 @@ public void info_without_options() { @Test @SneakyThrows public void info_with_multiple_options() { - InfoOptions options = - InfoOptions.builder().section(CLUSTER).section(CPU).section(MEMORY).build(); + var builder = InfoOptions.builder().section(CLUSTER); + if (TestConfiguration.REDIS_VERSION.feature() >= 7) { + builder.section(CPU).section(MEMORY); + } + var options = builder.build(); String data = regularClient.info(options).get(10, SECONDS); for (var section : options.toArgs()) { assertTrue( @@ -90,6 +81,16 @@ public void info_with_multiple_options() { } } + @Test + @SneakyThrows + public void info_with_everything_option() { + InfoOptions options = InfoOptions.builder().section(EVERYTHING).build(); + String data = regularClient.info(options).get(10, SECONDS); + for (var section : EVERYTHING_INFO_SECTIONS) { + assertTrue(data.contains("# " + section), "Section " + section + " is missing"); + } + } + @Test @SneakyThrows public void set_and_get_without_options() { From 210094c06575c2c814235986ef0e632b2d9f74d4 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Fri, 2 Feb 2024 16:58:47 -0800 Subject: [PATCH 20/36] cleanup Signed-off-by: Yury-Fridlyand --- java/integTest/src/test/java/glide/cluster/CommandTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 14ac21c2fa..5ac3cbf77f 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -6,7 +6,6 @@ import static glide.api.models.commands.InfoOptions.Section.COMMANDSTATS; import static glide.api.models.commands.InfoOptions.Section.CPU; import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; -import static glide.api.models.commands.InfoOptions.Section.LATENCYSTATS; import static glide.api.models.commands.InfoOptions.Section.MEMORY; import static glide.api.models.commands.InfoOptions.Section.REPLICATION; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; @@ -69,7 +68,6 @@ public static void init() { @SneakyThrows public static void teardown() { clusterClient.close(); - Runtime.Version.parse("1.2.3"); } @Test From 677c98cb9d16f9c11b22f4a69c6e28abcbef74ca Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Fri, 2 Feb 2024 17:12:15 -0800 Subject: [PATCH 21/36] Minor fixes. Signed-off-by: Yury-Fridlyand --- .../src/test/java/glide/api/RedisClusterClientTest.java | 9 +++++---- java/integTest/build.gradle | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index b4934383ee..9f4721c1b3 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -141,7 +141,7 @@ public void customCommand_success() { CompletableFuture> testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn(ClusterValue.of(value)); when(commandManager.>submitNewCommand( - eq(CustomCommand), eq(arguments), any(), any())) + eq(CustomCommand), eq(arguments), eq(Optional.empty()), any())) .thenReturn(testResponse); // exercise @@ -173,7 +173,7 @@ public void info_returns_string() { testPayload.put("addr3", "value3"); when(testResponse.get()).thenReturn(ClusterValue.of(testPayload)); when(commandManager.>submitNewCommand( - eq(Info), eq(new String[0]), any(), any())) + eq(Info), eq(new String[0]), eq(Optional.empty()), any())) .thenReturn(testResponse); // exercise @@ -194,7 +194,8 @@ public void info_with_route_returns_string() { Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_NODES; when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); - when(commandManager.>submitNewCommand(eq(Info), any(), any(), any())) + when(commandManager.>submitNewCommand( + eq(Info), eq(new String[0]), eq(Optional.of(route)), any())) .thenReturn(testResponse); // exercise @@ -230,7 +231,7 @@ public void info_with_route_with_infoOptions_returns_string() { CompletableFuture> response = service.info(options, route); // verify - assertEquals(testResponse, response); + assertEquals(testResponse.get(), response.get()); ClusterValue clusterValue = response.get(); assertTrue(clusterValue.hasMultiData()); Map clusterMap = clusterValue.getMultiValue(); diff --git a/java/integTest/build.gradle b/java/integTest/build.gradle index 43dc4a495e..4aab6eb87e 100644 --- a/java/integTest/build.gradle +++ b/java/integTest/build.gradle @@ -133,4 +133,4 @@ tasks.withType(Test) { afterTest { desc, result -> logger.quiet "${desc.className}.${desc.name}: ${result.resultType} ${(result.getEndTime() - result.getStartTime())/1000}s" } -} \ No newline at end of file +} From 70cd993f6939e3f8798afb8c02784e2de60460e1 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Mon, 5 Feb 2024 11:41:37 -0800 Subject: [PATCH 22/36] Remove OK object Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/BaseClient.java | 26 +++---------------- .../glide/api/commands/StringCommands.java | 3 +-- .../src/main/java/glide/api/models/Ok.java | 23 ---------------- .../managers/BaseCommandResponseResolver.java | 6 ++--- .../test/java/glide/api/RedisClientTest.java | 3 +-- .../java/glide/standalone/CommandTests.java | 14 +++++----- 6 files changed, 16 insertions(+), 59 deletions(-) delete mode 100644 java/client/src/main/java/glide/api/models/Ok.java diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 4b6bda03f8..b405b2320f 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -8,7 +8,6 @@ import glide.api.commands.ConnectionCommands; import glide.api.commands.StringCommands; -import glide.api.models.Ok; import glide.api.models.commands.SetOptions; import glide.api.models.configuration.BaseClientConfiguration; import glide.api.models.exceptions.RedisException; @@ -33,6 +32,8 @@ @AllArgsConstructor public abstract class BaseClient implements AutoCloseable, StringCommands, ConnectionCommands { + public static final String OK = "OK"; + protected final ConnectionManager connectionManager; protected final CommandManager commandManager; @@ -111,22 +112,6 @@ protected Object handleObjectResponse(Response response) { return new BaseCommandResponseResolver(RedisValueResolver::valueFromPointer).apply(response); } - /** - * Checks that the Response contains the simple-string Ok. - * - * @return An Ok response - * @throws RedisException if there's a type mismatch - */ - protected Ok handleOkResponse(Response response) { - Object value = handleObjectResponse(response); - if (value == Ok.INSTANCE) { - return Ok.INSTANCE; - } - String className = (value == null) ? "null" : value.getClass().getSimpleName(); - throw new RedisException( - "Unexpected return type from Redis: got " + className + " expected simple-string Ok"); - } - /** * Extracts the response value from the Redis response and either throws an exception or returns * the value as a String. @@ -140,9 +125,6 @@ protected String handleStringResponse(Response response) { if (value instanceof String || value == null) { return (String) value; } - if (value instanceof Ok) { - return Ok.INSTANCE.toString(); - } throw new RedisException( "Unexpected return type from Redis: got " + value.getClass().getSimpleName() @@ -203,9 +185,9 @@ public CompletableFuture get(String key) { } @Override - public CompletableFuture set(String key, String value) { + public CompletableFuture set(String key, String value) { return commandManager.submitNewCommand( - SetString, new String[] {key, value}, this::handleOkResponse); + SetString, new String[] {key, value}, this::handleStringResponse); } @Override diff --git a/java/client/src/main/java/glide/api/commands/StringCommands.java b/java/client/src/main/java/glide/api/commands/StringCommands.java index 983888b399..ee2aac263c 100644 --- a/java/client/src/main/java/glide/api/commands/StringCommands.java +++ b/java/client/src/main/java/glide/api/commands/StringCommands.java @@ -1,7 +1,6 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.commands; -import glide.api.models.Ok; import glide.api.models.commands.SetOptions; import glide.api.models.commands.SetOptions.ConditionalSet; import glide.api.models.commands.SetOptions.SetOptionsBuilder; @@ -32,7 +31,7 @@ public interface StringCommands { * @param value The value to store with the given key. * @return Response from Redis. */ - CompletableFuture set(String key, String value); + CompletableFuture set(String key, String value); /** * Set the given key with the given value. Return value is dependent on the passed options. diff --git a/java/client/src/main/java/glide/api/models/Ok.java b/java/client/src/main/java/glide/api/models/Ok.java deleted file mode 100644 index 0d50aee5bc..0000000000 --- a/java/client/src/main/java/glide/api/models/Ok.java +++ /dev/null @@ -1,23 +0,0 @@ -/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ -package glide.api.models; - -import response.ResponseOuterClass; - -/** - * Represents a successful Response from Redis without a value. "Ok" represents the status of the - * response not the actual value. - * - * @see Simple Strings - * response - */ -public final class Ok { - - public static final Ok INSTANCE = new Ok(); - - // Constructor is private - use Ok.INSTANCE instead - private Ok() {} - - public String toString() { - return ResponseOuterClass.ConstantResponse.OK.toString(); - } -} diff --git a/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java b/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java index 36e8a92b8d..f9b7ed87ab 100644 --- a/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java +++ b/java/client/src/main/java/glide/managers/BaseCommandResponseResolver.java @@ -1,7 +1,8 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.managers; -import glide.api.models.Ok; +import static glide.api.BaseClient.OK; + import glide.api.models.exceptions.RedisException; import lombok.AllArgsConstructor; import response.ResponseOuterClass.Response; @@ -26,8 +27,7 @@ public Object apply(Response response) throws RedisException { assert !response.hasRequestError() : "Unhandled response request error"; if (response.hasConstantResponse()) { - // Return "OK" - return Ok.INSTANCE; + return OK; } if (response.hasRespPointer()) { // Return the shared value - which may be a null value diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index 6d32c8b763..9c4a41555d 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -19,7 +19,6 @@ import static redis_request.RedisRequestOuterClass.RequestType.Ping; import static redis_request.RedisRequestOuterClass.RequestType.SetString; -import glide.api.models.Ok; import glide.api.models.commands.InfoOptions; import glide.api.models.commands.SetOptions; import glide.managers.CommandManager; @@ -206,7 +205,7 @@ public void set_returns_success() { .thenReturn(testResponse); // exercise - CompletableFuture response = service.set(key, value); + CompletableFuture response = service.set(key, value); Object okResponse = response.get(); // verify diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 2521d76da6..cf7eec86a1 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -1,6 +1,7 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.standalone; +import static glide.api.BaseClient.OK; import static glide.api.models.commands.InfoOptions.Section.CLUSTER; import static glide.api.models.commands.InfoOptions.Section.CPU; import static glide.api.models.commands.InfoOptions.Section.EVERYTHING; @@ -14,7 +15,6 @@ import glide.TestConfiguration; import glide.api.RedisClient; -import glide.api.models.Ok; import glide.api.models.commands.InfoOptions; import glide.api.models.commands.SetOptions; import glide.api.models.configuration.NodeAddress; @@ -94,8 +94,8 @@ public void info_with_everything_option() { @Test @SneakyThrows public void set_and_get_without_options() { - Ok ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); - assertEquals(Ok.INSTANCE, ok); + String ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); + assertEquals(OK, ok); String data = regularClient.get(KEY_NAME).get(10, SECONDS); assertEquals(INITIAL_VALUE, data); @@ -111,8 +111,8 @@ public void get_missing_value() { @Test @SneakyThrows public void set_overwrite_value_and_returnOldValue_returns_string() { - var ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); - assertEquals(Ok.INSTANCE, ok); + String ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); + assertEquals(OK, ok); var options = SetOptions.builder().returnOldValue(true).build(); var data = regularClient.set(KEY_NAME, ANOTHER_VALUE, options).get(10, SECONDS); @@ -122,8 +122,8 @@ public void set_overwrite_value_and_returnOldValue_returns_string() { @Test @SneakyThrows public void set_missing_value_and_returnOldValue_is_null() { - var ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); - assertEquals(Ok.INSTANCE, ok); + String ok = regularClient.set(KEY_NAME, INITIAL_VALUE).get(10, SECONDS); + assertEquals(OK, ok); var options = SetOptions.builder().returnOldValue(true).build(); var data = regularClient.set("another", ANOTHER_VALUE, options).get(10, SECONDS); From 2dd1a03bdc5298a3a2f5b41b0d3b7c9ebfb9fdae Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Mon, 5 Feb 2024 12:00:38 -0800 Subject: [PATCH 23/36] Revert change to customCommand Signed-off-by: Andrew Carbonetto --- .../main/java/glide/api/RedisClusterClient.java | 7 ++----- .../java/glide/api/commands/BaseCommands.java | 2 +- .../glide/api/commands/ClusterBaseCommands.java | 8 ++++---- .../java/glide/api/RedisClusterClientTest.java | 15 ++++----------- .../src/test/java/glide/cluster/CommandTests.java | 10 ++-------- 5 files changed, 13 insertions(+), 29 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 56b2fa48f0..e78a68eef0 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -40,8 +40,7 @@ public static CompletableFuture CreateClient( } @Override - public CompletableFuture> customCommand(String... args) { - if (args.length == 0) throw new IllegalArgumentException("No arguments provided"); + public CompletableFuture> customCommand(String[] args) { // TODO if a command returns a map as a single value, ClusterValue misleads user return commandManager.submitNewCommand( CustomCommand, @@ -52,9 +51,7 @@ public CompletableFuture> customCommand(String... args) { @Override @SuppressWarnings("unchecked") - public CompletableFuture> customCommand( - @NonNull Route route, String... args) { - if (args.length == 0) throw new IllegalArgumentException("No arguments provided"); + public CompletableFuture> customCommand(String[] args, Route route) { return commandManager.submitNewCommand( CustomCommand, args, diff --git a/java/client/src/main/java/glide/api/commands/BaseCommands.java b/java/client/src/main/java/glide/api/commands/BaseCommands.java index 3c2cd85f8d..8ffb56d702 100644 --- a/java/client/src/main/java/glide/api/commands/BaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/BaseCommands.java @@ -23,5 +23,5 @@ public interface BaseCommands { * @param args Arguments for the custom command. * @return Response from Redis containing an Object. */ - CompletableFuture customCommand(String... args); + CompletableFuture customCommand(String[] args); } diff --git a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java index 63de950879..c763c9ef0b 100644 --- a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java @@ -21,12 +21,12 @@ public interface ClusterBaseCommands { * this function. * @example Returns a list of all pub/sub clients: *

- * Object result = client.customCommand("CLIENT", "LIST", "TYPE", "PUBSUB").get(); + * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get(); * * @param args Arguments for the custom command including the command name. * @return Response from Redis containing an Object. */ - CompletableFuture> customCommand(String... args); + CompletableFuture> customCommand(String[] args); /** * Executes a single command, without checking inputs. Every part of the command, including @@ -39,11 +39,11 @@ public interface ClusterBaseCommands { * this function. * @example Returns a list of all pub/sub clients: *

- * Object result = client.customCommand(ALL_NODES, "CLIENT", "LIST", "TYPE", "PUBSUB").get(); + * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }, RANDOM).get(); * * @param route Routing configuration for the command * @param args Arguments for the custom command including the command name * @return Response from Redis containing an Object. */ - CompletableFuture> customCommand(Route route, String... args); + CompletableFuture> customCommand(String[] args, Route route); } diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index 9f4721c1b3..64d2aacf2a 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -52,7 +52,7 @@ public void customCommand_returns_single_value() { var client = new TestClient(commandManager, "TEST"); - var value = client.customCommand("test").get(); + var value = client.customCommand(new String[] {"test"}).get(); assertAll( () -> assertTrue(value.hasSingleData()), () -> assertEquals("TEST", value.getSingleValue())); @@ -66,7 +66,7 @@ public void customCommand_returns_multi_value() { var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand("test").get(); + var value = client.customCommand(new String[] {"test"}).get(); assertAll( () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); } @@ -80,7 +80,7 @@ public void customCommand_with_single_node_route_returns_single_value() { var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(RANDOM, "Test").get(); + var value = client.customCommand(new String[] {"Test"}, RANDOM).get(); assertAll( () -> assertTrue(value.hasSingleData()), () -> assertEquals(data, value.getSingleValue())); } @@ -93,7 +93,7 @@ public void customCommand_with_multi_node_route_returns_multi_value() { var data = Map.of("key1", "value1", "key2", "value2"); var client = new TestClient(commandManager, data); - var value = client.customCommand(ALL_NODES, "Test").get(); + var value = client.customCommand(new String[] {"Test"}, ALL_NODES).get(); assertAll( () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); } @@ -155,13 +155,6 @@ public void customCommand_success() { assertEquals(value, payload); } - @Test - @SneakyThrows - public void customCommand_requires_args() { - assertThrows(IllegalArgumentException.class, () -> service.customCommand()); - assertThrows(IllegalArgumentException.class, () -> service.customCommand(RANDOM)); - } - @SneakyThrows @Test public void info_returns_string() { diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 5ac3cbf77f..f35d35a45a 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -142,7 +142,7 @@ public void info_with_everything_option() { @Test @SneakyThrows public void info_with_routing_and_options() { - var slotData = clusterClient.customCommand("cluster", "slots").get(10, SECONDS); + var slotData = clusterClient.customCommand(new String[] {"cluster", "slots"}).get(10, SECONDS); /* Nested Object arrays like 1) 1) (integer) 0 @@ -186,7 +186,7 @@ public void ping_with_message() { @SneakyThrows public void custom_command_del_returns_a_number() { clusterClient.set("DELME", INITIAL_VALUE).get(10, SECONDS); - var del = clusterClient.customCommand("DEL", "DELME").get(10, SECONDS); + var del = clusterClient.customCommand(new String[] {"DEL", "DELME"}).get(10, SECONDS); assertEquals(1L, del.getSingleValue()); var data = clusterClient.get("DELME").get(10, SECONDS); assertNull(data); @@ -212,12 +212,6 @@ public void ping_with_message_requires_a_message() { assertThrows(NullPointerException.class, () -> clusterClient.ping(null)); } - @Test - public void custom_command_requires_args() { - assertThrows(IllegalArgumentException.class, () -> clusterClient.customCommand()); - assertThrows(IllegalArgumentException.class, () -> clusterClient.customCommand(ALL_NODES)); - } - @Test @SneakyThrows public void set_only_if_exists_overwrite() { From 4a56401a7b170856bfac14c0a966126234661b4e Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Mon, 5 Feb 2024 12:02:22 -0800 Subject: [PATCH 24/36] Rename command files Signed-off-by: Andrew Carbonetto --- java/client/src/main/java/glide/api/BaseClient.java | 5 +++-- java/client/src/main/java/glide/api/RedisClient.java | 8 ++++---- .../src/main/java/glide/api/RedisClusterClient.java | 6 +++--- ...ionCommands.java => ConnectionManagementCommands.java} | 2 +- ...usterBaseCommands.java => GenericClusterCommands.java} | 5 +++-- .../commands/{BaseCommands.java => GenericCommands.java} | 4 ++-- ...Commands.java => ServerManagementClusterCommands.java} | 2 +- ...{ServerCommands.java => ServerManagementCommands.java} | 2 +- .../main/java/glide/api/models/commands/InfoOptions.java | 4 ++-- .../src/test/java/glide/api/RedisClusterClientTest.java | 1 - 10 files changed, 20 insertions(+), 19 deletions(-) rename java/client/src/main/java/glide/api/commands/{ConnectionCommands.java => ConnectionManagementCommands.java} (94%) rename java/client/src/main/java/glide/api/commands/{ClusterBaseCommands.java => GenericClusterCommands.java} (94%) rename java/client/src/main/java/glide/api/commands/{BaseCommands.java => GenericCommands.java} (90%) rename java/client/src/main/java/glide/api/commands/{ClusterServerCommands.java => ServerManagementClusterCommands.java} (97%) rename java/client/src/main/java/glide/api/commands/{ServerCommands.java => ServerManagementCommands.java} (96%) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index b405b2320f..9ad3782d02 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -6,7 +6,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.Ping; import static redis_request.RedisRequestOuterClass.RequestType.SetString; -import glide.api.commands.ConnectionCommands; +import glide.api.commands.ConnectionManagementCommands; import glide.api.commands.StringCommands; import glide.api.models.commands.SetOptions; import glide.api.models.configuration.BaseClientConfiguration; @@ -30,7 +30,8 @@ /** Base Client class for Redis */ @AllArgsConstructor -public abstract class BaseClient implements AutoCloseable, StringCommands, ConnectionCommands { +public abstract class BaseClient + implements AutoCloseable, StringCommands, ConnectionManagementCommands { public static final String OK = "OK"; diff --git a/java/client/src/main/java/glide/api/RedisClient.java b/java/client/src/main/java/glide/api/RedisClient.java index 1ba7f3a976..fe5b25dea6 100644 --- a/java/client/src/main/java/glide/api/RedisClient.java +++ b/java/client/src/main/java/glide/api/RedisClient.java @@ -4,9 +4,9 @@ import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import static redis_request.RedisRequestOuterClass.RequestType.Info; -import glide.api.commands.BaseCommands; -import glide.api.commands.ConnectionCommands; -import glide.api.commands.ServerCommands; +import glide.api.commands.ConnectionManagementCommands; +import glide.api.commands.GenericCommands; +import glide.api.commands.ServerManagementCommands; import glide.api.models.commands.InfoOptions; import glide.api.models.configuration.RedisClientConfiguration; import glide.managers.CommandManager; @@ -18,7 +18,7 @@ * client to Redis. */ public class RedisClient extends BaseClient - implements BaseCommands, ConnectionCommands, ServerCommands { + implements GenericCommands, ConnectionManagementCommands, ServerManagementCommands { protected RedisClient(ConnectionManager connectionManager, CommandManager commandManager) { super(connectionManager, commandManager); diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index e78a68eef0..1e47183e03 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -4,8 +4,8 @@ import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import static redis_request.RedisRequestOuterClass.RequestType.Info; -import glide.api.commands.ClusterBaseCommands; -import glide.api.commands.ClusterServerCommands; +import glide.api.commands.GenericClusterCommands; +import glide.api.commands.ServerManagementClusterCommands; import glide.api.models.ClusterValue; import glide.api.models.commands.InfoOptions; import glide.api.models.configuration.RedisClusterClientConfiguration; @@ -22,7 +22,7 @@ * client to Redis. */ public class RedisClusterClient extends BaseClient - implements ClusterBaseCommands, ClusterServerCommands { + implements GenericClusterCommands, ServerManagementClusterCommands { protected RedisClusterClient(ConnectionManager connectionManager, CommandManager commandManager) { super(connectionManager, commandManager); diff --git a/java/client/src/main/java/glide/api/commands/ConnectionCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java similarity index 94% rename from java/client/src/main/java/glide/api/commands/ConnectionCommands.java rename to java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java index 91e55ece1a..aaaf096206 100644 --- a/java/client/src/main/java/glide/api/commands/ConnectionCommands.java +++ b/java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java @@ -8,7 +8,7 @@ * * @see: Connection Management Commands */ -public interface ConnectionCommands { +public interface ConnectionManagementCommands { /** * Ping the Redis server. diff --git a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java b/java/client/src/main/java/glide/api/commands/GenericClusterCommands.java similarity index 94% rename from java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java rename to java/client/src/main/java/glide/api/commands/GenericClusterCommands.java index c763c9ef0b..7d58bd4580 100644 --- a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/GenericClusterCommands.java @@ -6,9 +6,10 @@ import java.util.concurrent.CompletableFuture; /** - * Base Commands interface to handle generic command and transaction requests with routing options. + * Generic Commands interface to handle generic command and transaction requests with routing + * options. */ -public interface ClusterBaseCommands { +public interface GenericClusterCommands { /** * Executes a single command, without checking inputs. Every part of the command, including diff --git a/java/client/src/main/java/glide/api/commands/BaseCommands.java b/java/client/src/main/java/glide/api/commands/GenericCommands.java similarity index 90% rename from java/client/src/main/java/glide/api/commands/BaseCommands.java rename to java/client/src/main/java/glide/api/commands/GenericCommands.java index 8ffb56d702..78db356c74 100644 --- a/java/client/src/main/java/glide/api/commands/BaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/GenericCommands.java @@ -3,8 +3,8 @@ import java.util.concurrent.CompletableFuture; -/** Base Commands interface to handle generic command and transaction requests. */ -public interface BaseCommands { +/** Generic Commands interface to handle generic command and transaction requests. */ +public interface GenericCommands { /** * Executes a single command, without checking inputs. Every part of the command, including diff --git a/java/client/src/main/java/glide/api/commands/ClusterServerCommands.java b/java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java similarity index 97% rename from java/client/src/main/java/glide/api/commands/ClusterServerCommands.java rename to java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java index 768408aad9..214f53a9a9 100644 --- a/java/client/src/main/java/glide/api/commands/ClusterServerCommands.java +++ b/java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java @@ -11,7 +11,7 @@ * * @see Server Management Commands */ -public interface ClusterServerCommands { +public interface ServerManagementClusterCommands { /** * Get information and statistics about the Redis server. DEFAULT option is assumed diff --git a/java/client/src/main/java/glide/api/commands/ServerCommands.java b/java/client/src/main/java/glide/api/commands/ServerManagementCommands.java similarity index 96% rename from java/client/src/main/java/glide/api/commands/ServerCommands.java rename to java/client/src/main/java/glide/api/commands/ServerManagementCommands.java index 392b88c721..3d6dcd99c0 100644 --- a/java/client/src/main/java/glide/api/commands/ServerCommands.java +++ b/java/client/src/main/java/glide/api/commands/ServerManagementCommands.java @@ -10,7 +10,7 @@ * * @see Server Management Commands */ -public interface ServerCommands { +public interface ServerManagementCommands { /** * Get information and statistics about the Redis server. No argument is provided, so the {@link diff --git a/java/client/src/main/java/glide/api/models/commands/InfoOptions.java b/java/client/src/main/java/glide/api/models/commands/InfoOptions.java index 8826a4afee..8b518ab87b 100644 --- a/java/client/src/main/java/glide/api/models/commands/InfoOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/InfoOptions.java @@ -1,13 +1,13 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.models.commands; -import glide.api.commands.ServerCommands; +import glide.api.commands.ServerManagementCommands; import java.util.List; import lombok.Builder; import lombok.Singular; /** - * Optional arguments to {@link ServerCommands#info(InfoOptions)} + * Optional arguments to {@link ServerManagementCommands#info(InfoOptions)} * * @see redis.io */ diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index 64d2aacf2a..c9391b077b 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -5,7 +5,6 @@ import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; From 42b0498da2e1ef89947cf529d4e56b0893b91904 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Mon, 5 Feb 2024 12:28:01 -0800 Subject: [PATCH 25/36] Add ping(route) to Cluster client Signed-off-by: Andrew Carbonetto --- .../java/glide/api/RedisClusterClient.java | 18 ++++++- .../ConnectionManagementClusterCommands.java | 30 +++++++++++ .../glide/api/RedisClusterClientTest.java | 50 +++++++++++++++++++ .../test/java/glide/cluster/CommandTests.java | 21 ++++++-- 4 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 1e47183e03..cfd6d2f21b 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -3,7 +3,9 @@ import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import static redis_request.RedisRequestOuterClass.RequestType.Info; +import static redis_request.RedisRequestOuterClass.RequestType.Ping; +import glide.api.commands.ConnectionManagementClusterCommands; import glide.api.commands.GenericClusterCommands; import glide.api.commands.ServerManagementClusterCommands; import glide.api.models.ClusterValue; @@ -22,7 +24,9 @@ * client to Redis. */ public class RedisClusterClient extends BaseClient - implements GenericClusterCommands, ServerManagementClusterCommands { + implements ConnectionManagementClusterCommands, + GenericClusterCommands, + ServerManagementClusterCommands { protected RedisClusterClient(ConnectionManager connectionManager, CommandManager commandManager) { super(connectionManager, commandManager); @@ -62,6 +66,18 @@ public CompletableFuture> customCommand(String[] args, Rout : ClusterValue.ofMultiValue((Map) handleObjectResponse(response))); } + @Override + public CompletableFuture ping(@NonNull Route route) { + return commandManager.submitNewCommand( + Ping, new String[0], Optional.of(route), this::handleStringResponse); + } + + @Override + public CompletableFuture ping(@NonNull String msg, @NonNull Route route) { + return commandManager.submitNewCommand( + Ping, new String[] {msg}, Optional.of(route), this::handleStringResponse); + } + @Override public CompletableFuture> info() { return commandManager.submitNewCommand( diff --git a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java new file mode 100644 index 0000000000..504bbc56da --- /dev/null +++ b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java @@ -0,0 +1,30 @@ +/** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.api.commands; + +import glide.api.models.configuration.RequestRoutingConfiguration; +import java.util.concurrent.CompletableFuture; + +/** + * Connection Management Commands interface. + * + * @see: Connection Management Commands + */ +public interface ConnectionManagementClusterCommands { + + /** + * Ping the Redis server. + * + * @see redis.io for details. + * @return Response from Redis containing a String. + */ + CompletableFuture ping(RequestRoutingConfiguration.Route route); + + /** + * Ping the Redis server. + * + * @see redis.io for details. + * @param msg The ping argument that will be returned. + * @return Response from Redis containing a String. + */ + CompletableFuture ping(String msg, RequestRoutingConfiguration.Route route); +} diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index c9391b077b..e2a1996b13 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -2,6 +2,7 @@ package glide.api; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_NODES; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_PRIMARIES; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -12,6 +13,7 @@ import static org.mockito.Mockito.when; import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import static redis_request.RedisRequestOuterClass.RequestType.Info; +import static redis_request.RedisRequestOuterClass.RequestType.Ping; import glide.api.models.ClusterValue; import glide.api.models.commands.InfoOptions; @@ -154,6 +156,54 @@ public void customCommand_success() { assertEquals(value, payload); } + @SneakyThrows + @Test + public void ping_returns_success() { + // setup + CompletableFuture testResponse = mock(CompletableFuture.class); + when(testResponse.get()).thenReturn("PONG"); + + RequestRoutingConfiguration.Route route = ALL_NODES; + + // match on protobuf request + when(commandManager.submitNewCommand( + eq(Ping), eq(new String[0]), eq(Optional.of(route)), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.ping(route); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals("PONG", payload); + } + + @SneakyThrows + @Test + public void ping_with_message_returns_success() { + // setup + String message = "RETURN OF THE PONG"; + String[] arguments = new String[] {message}; + CompletableFuture testResponse = new CompletableFuture(); + testResponse.complete(message); + + RequestRoutingConfiguration.Route route = ALL_PRIMARIES; + + // match on protobuf request + when(commandManager.submitNewCommand( + eq(Ping), eq(arguments), eq(Optional.of(route)), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.ping(message, route); + String pong = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(message, pong); + } + @SneakyThrows @Test public void info_returns_string() { diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index f35d35a45a..c5c81a09b8 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -14,6 +14,7 @@ import static glide.api.models.commands.SetOptions.TimeToLiveType.MILLISECONDS; import static glide.api.models.commands.SetOptions.TimeToLiveType.UNIX_SECONDS; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_NODES; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_PRIMARIES; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; import static glide.api.models.configuration.RequestRoutingConfiguration.SlotType.PRIMARY; import static java.util.concurrent.TimeUnit.SECONDS; @@ -30,6 +31,7 @@ import glide.api.models.commands.SetOptions.TimeToLive; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClusterClientConfiguration; +import glide.api.models.configuration.RequestRoutingConfiguration; import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; import java.util.List; import java.util.concurrent.TimeUnit; @@ -182,6 +184,20 @@ public void ping_with_message() { assertEquals("H3LL0", data); } + @Test + @SneakyThrows + public void ping_with_route() { + String data = clusterClient.ping(ALL_NODES).get(10, SECONDS); + assertEquals("PONG", data); + } + + @Test + @SneakyThrows + public void ping_with_message_with_route() { + String data = clusterClient.ping("H3LL0", ALL_PRIMARIES).get(10, SECONDS); + assertEquals("H3LL0", data); + } + @Test @SneakyThrows public void custom_command_del_returns_a_number() { @@ -207,11 +223,6 @@ public void get_requires_a_key() { assertThrows(NullPointerException.class, () -> clusterClient.get(null)); } - @Test - public void ping_with_message_requires_a_message() { - assertThrows(NullPointerException.class, () -> clusterClient.ping(null)); - } - @Test @SneakyThrows public void set_only_if_exists_overwrite() { From b780bfc2802db649afd420ce08d49c1f30a4eb14 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 15:01:39 -0800 Subject: [PATCH 26/36] Clean up javadocs; and remove extra test Signed-off-by: Andrew Carbonetto --- .../src/main/java/glide/api/RedisClient.java | 2 +- .../glide/api/commands/StringCommands.java | 19 ++++++----- .../java/glide/managers/CommandManager.java | 6 ++-- .../glide/api/RedisClusterClientTest.java | 34 +++---------------- 4 files changed, 18 insertions(+), 43 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClient.java b/java/client/src/main/java/glide/api/RedisClient.java index fe5b25dea6..76dcce8cd6 100644 --- a/java/client/src/main/java/glide/api/RedisClient.java +++ b/java/client/src/main/java/glide/api/RedisClient.java @@ -35,7 +35,7 @@ public static CompletableFuture CreateClient(RedisClientConfigurati } @Override - public CompletableFuture customCommand(String... args) { + public CompletableFuture customCommand(String[] args) { return commandManager.submitNewCommand(CustomCommand, args, this::handleStringResponse); } diff --git a/java/client/src/main/java/glide/api/commands/StringCommands.java b/java/client/src/main/java/glide/api/commands/StringCommands.java index ee2aac263c..2664f32b68 100644 --- a/java/client/src/main/java/glide/api/commands/StringCommands.java +++ b/java/client/src/main/java/glide/api/commands/StringCommands.java @@ -14,12 +14,13 @@ public interface StringCommands { /** - * Get the value associated with the given key, or null if no such value exists. + * Get the value associated with the given key, or null if no such value + * exists. * * @see redis.io for details. * @param key The key to retrieve from the database. - * @return Response from Redis. key exists, returns the value of - * key as a String. Otherwise, return null. + * @return Response from Redis. If key exists, returns the value of + * key as a String. Otherwise, return null. */ CompletableFuture get(String key); @@ -29,7 +30,7 @@ public interface StringCommands { * @see redis.io for details. * @param key The key to store. * @param value The value to store with the given key. - * @return Response from Redis. + * @return Response from Redis containing "OK". */ CompletableFuture set(String key, String value); @@ -40,11 +41,11 @@ public interface StringCommands { * @param key The key to store. * @param value The value to store with the given key. * @param options The Set options. - * @return Response from Redis containing a String or null response. The - * old value as a String if {@link SetOptionsBuilder#returnOldValue(boolean)} is - * set. Otherwise, if the value isn't set because of {@link ConditionalSet#ONLY_IF_EXISTS} or - * {@link ConditionalSet#ONLY_IF_DOES_NOT_EXIST} conditions, return null. - * Otherwise, return OK. + * @return Response from Redis containing a String or null response. If + * the value is successfully set, return "OK". If value isn't set because of + * {@link ConditionalSet#ONLY_IF_EXISTS} or {@link ConditionalSet#ONLY_IF_DOES_NOT_EXIST} + * conditions, return null. If {@link SetOptionsBuilder#returnOldValue(boolean)} + * is set, return the old value as a String. */ CompletableFuture set(String key, String value, SetOptions options); } diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index 70c3a6620a..b481b5b311 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -101,12 +101,12 @@ private Response exceptionHandler(Throwable e) { } /** - * Build a protobuf command request object with routing options.
+ * Build a protobuf command request object with routing options. * * @param requestType Redis command type * @param arguments Redis command arguments * @param route Command routing parameters - * @return An uncompleted request. {@link CallbackDispatcher} is responsible to complete it by + * @return An incomplete request. {@link CallbackDispatcher} is responsible to complete it by * adding a callback id. */ protected RedisRequest.Builder prepareRedisRequest( @@ -128,7 +128,7 @@ protected RedisRequest.Builder prepareRedisRequest( } /** - * Build a protobuf command request object with routing options.
+ * Build a protobuf command request object. * * @param requestType Redis command type * @param arguments Redis command arguments diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index e2a1996b13..b41b16fb73 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -11,7 +11,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static redis_request.RedisRequestOuterClass.RequestType.CustomCommand; import static redis_request.RedisRequestOuterClass.RequestType.Info; import static redis_request.RedisRequestOuterClass.RequestType.Ping; @@ -48,7 +47,7 @@ public void setUp() { @Test @SneakyThrows - public void customCommand_returns_single_value() { + public void custom_command_returns_single_value() { var commandManager = new TestCommandManager(null); var client = new TestClient(commandManager, "TEST"); @@ -61,7 +60,7 @@ public void customCommand_returns_single_value() { @Test @SneakyThrows - public void customCommand_returns_multi_value() { + public void custom_command_returns_multi_value() { var commandManager = new TestCommandManager(null); var data = Map.of("key1", "value1", "key2", "value2"); @@ -75,7 +74,7 @@ public void customCommand_returns_multi_value() { @Test @SneakyThrows // test checks that even a map returned as a single value when single node route is used - public void customCommand_with_single_node_route_returns_single_value() { + public void custom_command_with_single_node_route_returns_single_value() { var commandManager = new TestCommandManager(null); var data = Map.of("key1", "value1", "key2", "value2"); @@ -88,7 +87,7 @@ public void customCommand_with_single_node_route_returns_single_value() { @Test @SneakyThrows - public void customCommand_with_multi_node_route_returns_multi_value() { + public void custom_command_with_multi_node_route_returns_multi_value() { var commandManager = new TestCommandManager(null); var data = Map.of("key1", "value1", "key2", "value2"); @@ -131,31 +130,6 @@ public CompletableFuture submitNewCommand( } } - @SneakyThrows - @Test - public void customCommand_success() { - // setup - String key = "testKey"; - String value = "testValue"; - String cmd = "GETSTRING"; - String[] arguments = new String[] {cmd, key}; - CompletableFuture> testResponse = mock(CompletableFuture.class); - when(testResponse.get()).thenReturn(ClusterValue.of(value)); - when(commandManager.>submitNewCommand( - eq(CustomCommand), eq(arguments), eq(Optional.empty()), any())) - .thenReturn(testResponse); - - // exercise - CompletableFuture> response = service.customCommand(arguments); - - // verify - assertEquals(testResponse, response); - ClusterValue clusterValue = response.get(); - assertTrue(clusterValue.hasSingleData()); - String payload = (String) clusterValue.getSingleValue(); - assertEquals(value, payload); - } - @SneakyThrows @Test public void ping_returns_success() { From 40520a7c187e9f172d3a5efcacbfec927de75df2 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 15:53:45 -0800 Subject: [PATCH 27/36] Fix CI run Signed-off-by: Andrew Carbonetto --- .../api/commands/ConnectionManagementClusterCommands.java | 8 +++++--- .../src/main/java/glide/api/commands/GenericCommands.java | 2 +- .../src/test/java/glide/standalone/CommandTests.java | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java index 504bbc56da..761184a4fe 100644 --- a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java +++ b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java @@ -1,7 +1,7 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.commands; -import glide.api.models.configuration.RequestRoutingConfiguration; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; import java.util.concurrent.CompletableFuture; /** @@ -14,17 +14,19 @@ public interface ConnectionManagementClusterCommands { /** * Ping the Redis server. * + * @param route Routing configuration for the command * @see redis.io for details. * @return Response from Redis containing a String. */ - CompletableFuture ping(RequestRoutingConfiguration.Route route); + CompletableFuture ping(Route route); /** * Ping the Redis server. * * @see redis.io for details. * @param msg The ping argument that will be returned. + * @param route Routing configuration for the command * @return Response from Redis containing a String. */ - CompletableFuture ping(String msg, RequestRoutingConfiguration.Route route); + CompletableFuture ping(String msg, Route route); } diff --git a/java/client/src/main/java/glide/api/commands/GenericCommands.java b/java/client/src/main/java/glide/api/commands/GenericCommands.java index 78db356c74..4f214f36cc 100644 --- a/java/client/src/main/java/glide/api/commands/GenericCommands.java +++ b/java/client/src/main/java/glide/api/commands/GenericCommands.java @@ -17,7 +17,7 @@ public interface GenericCommands { * this function. * @example Returns a list of all pub/sub clients: * 
-     * Object result = client.customCommand("CLIENT","LIST","TYPE", "PUBSUB").get();
+     * Object result = client.customCommand(new String[]{ "CLIENT", "LIST", "TYPE", "PUBSUB" }).get();
      *
* * @param args Arguments for the custom command. diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index cf7eec86a1..134ae543d8 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -52,7 +52,7 @@ public static void teardown() { @Test @SneakyThrows public void custom_command_info() { - var data = regularClient.customCommand("info").get(10, SECONDS); + var data = regularClient.customCommand(new String[] {"info"}).get(10, SECONDS); assertTrue(((String) data).contains("# Stats")); } From 55a0097886501f32662615df83102c03af950dbc Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 16:00:20 -0800 Subject: [PATCH 28/36] Revert documentation changes in ClusterValue Signed-off-by: Andrew Carbonetto --- .../java/glide/api/models/ClusterValue.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/ClusterValue.java b/java/client/src/main/java/glide/api/models/ClusterValue.java index 0dd25cd034..a690f154c5 100644 --- a/java/client/src/main/java/glide/api/models/ClusterValue.java +++ b/java/client/src/main/java/glide/api/models/ClusterValue.java @@ -1,21 +1,14 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.models; -import glide.api.models.configuration.RequestRoutingConfiguration.Route; import java.util.Map; /** - * Represents a Response object from a Redis server with cluster-mode enabled. The response type may - * depend on the submitted {@link Route}. + * union-like type which can store single-value or multi-value retrieved from Redis. The + * multi-value, if defined, contains the routed value as a Map containing a cluster + * node address to cluster node value. * - * @remark ClusterValue stores values in a union-like object. It contains a single-value or - * multi-value response from Redis. If the command's routing is to one node use {@link - * #getSingleValue()} to return a response of type T. Otherwise, use {@link - * #getMultiValue()} to return a Map of address: nodeResponse where - * address is of type string and nodeResponse is of type - * T. - * @see Redis cluster specification - * @param The wrapped response type + * @param The wrapped data type */ public class ClusterValue { private Map multiValue = null; @@ -26,7 +19,7 @@ private ClusterValue() {} /** * Get per-node value.
- * Asserts if {@link #hasMultiData()} is false. + * Check with {@link #hasMultiData()} prior to accessing the data. */ public Map getMultiValue() { assert hasMultiData() : "No multi value stored"; @@ -35,7 +28,7 @@ public Map getMultiValue() { /** * Get the single value.
- * Asserts if {@link #hasSingleData()} is false. + * Check with {@link #hasSingleData()} ()} prior to accessing the data. */ public T getSingleValue() { assert hasSingleData() : "No single value stored"; From d082f3294b90b3878ff97ca09b19a5bb6e09602a Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 17:52:49 -0800 Subject: [PATCH 29/36] Change SetOptions.expiry to use constructors Signed-off-by: Andrew Carbonetto --- .../glide/api/models/commands/SetOptions.java | 79 +++++++++++++++---- .../test/java/glide/api/RedisClientTest.java | 20 ++--- .../test/java/glide/cluster/CommandTests.java | 23 +++--- 3 files changed, 78 insertions(+), 44 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/commands/SetOptions.java b/java/client/src/main/java/glide/api/models/commands/SetOptions.java index f9e2a7740f..61376d38d6 100644 --- a/java/client/src/main/java/glide/api/models/commands/SetOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/SetOptions.java @@ -1,13 +1,18 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.api.models.commands; +import static glide.api.models.commands.SetOptions.ExpiryType.KEEP_EXISTING; +import static glide.api.models.commands.SetOptions.ExpiryType.MILLISECONDS; +import static glide.api.models.commands.SetOptions.ExpiryType.SECONDS; +import static glide.api.models.commands.SetOptions.ExpiryType.UNIX_MILLISECONDS; +import static glide.api.models.commands.SetOptions.ExpiryType.UNIX_SECONDS; + import glide.api.commands.StringCommands; import glide.api.models.exceptions.RequestException; import java.util.ArrayList; import java.util.List; import lombok.Builder; import lombok.Getter; -import lombok.NonNull; import lombok.RequiredArgsConstructor; import redis_request.RedisRequestOuterClass.Command; @@ -33,7 +38,7 @@ public final class SetOptions { private final boolean returnOldValue; /** If not set, no expiry time will be set for the value. */ - private final TimeToLive expiry; + private final Expiry expiry; /** Conditions which define whether new value should be set or not. */ @RequiredArgsConstructor @@ -51,45 +56,85 @@ public enum ConditionalSet { } /** Configuration of value lifetime. */ - @Builder - public static final class TimeToLive { + public static final class Expiry { + /** Expiry type for the time to live */ - @NonNull private TimeToLiveType type; + private final ExpiryType type; /** * The amount of time to live before the key expires. Ignored when {@link - * TimeToLiveType#KEEP_EXISTING} type is set. + * ExpiryType#KEEP_EXISTING} type is set. */ - private Integer count; - } + private Long count; + + private Expiry(ExpiryType type) { + this.type = type; + } + + private Expiry(ExpiryType type, Long count) { + this.type = type; + this.count = count; + } - /** Types of value expiration configuration. */ - @RequiredArgsConstructor - @Getter - public enum TimeToLiveType { /** * Retain the time to live associated with the key. Equivalent to KEEPTTL in the * Redis API. */ - KEEP_EXISTING("KEEPTTL"), + public static Expiry KeepExisting() { + return new Expiry(KEEP_EXISTING); + } + /** * Set the specified expire time, in seconds. Equivalent to EX in the Redis API. + * + * @param seconds time to expire, in seconds + * @return Expiry */ - SECONDS("EX"), + public static Expiry Seconds(Long seconds) { + return new Expiry(SECONDS, seconds); + } + /** * Set the specified expire time, in milliseconds. Equivalent to PX in the Redis * API. + * + * @param milliseconds time to expire, in milliseconds + * @return Expiry */ - MILLISECONDS("PX"), + public static Expiry Milliseconds(Long milliseconds) { + return new Expiry(MILLISECONDS, milliseconds); + } + /** * Set the specified Unix time at which the key will expire, in seconds. Equivalent to * EXAT in the Redis API. + * + * @param unixSeconds unix time to expire, in seconds + * @return Expiry */ - UNIX_SECONDS("EXAT"), + public static Expiry UnixSeconds(Long unixSeconds) { + return new Expiry(UNIX_SECONDS, unixSeconds); + } + /** * Set the specified Unix time at which the key will expire, in milliseconds. Equivalent to * PXAT in the Redis API. + * + * @param unixMilliseconds unix time to expire, in milliseconds + * @return Expiry */ + public static Expiry UnixMilliseconds(Long unixMilliseconds) { + return new Expiry(UNIX_MILLISECONDS, unixMilliseconds); + } + } + + /** Types of value expiration configuration. */ + @RequiredArgsConstructor + protected enum ExpiryType { + KEEP_EXISTING("KEEPTTL"), + SECONDS("EX"), + MILLISECONDS("PX"), + UNIX_SECONDS("EXAT"), UNIX_MILLISECONDS("PXAT"); private final String redisApi; @@ -115,7 +160,7 @@ public String[] toArgs() { if (expiry != null) { optionArgs.add(expiry.type.redisApi); - if (expiry.type != TimeToLiveType.KEEP_EXISTING) { + if (expiry.type != KEEP_EXISTING) { if (expiry.count == null) { throw new RequestException( "Set command received expiry type " + expiry.type + ", but count was not set."); diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index 9c4a41555d..840ee088e8 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -4,8 +4,6 @@ import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_EXISTS; import static glide.api.models.commands.SetOptions.RETURN_OLD_VALUE; -import static glide.api.models.commands.SetOptions.TimeToLiveType.KEEP_EXISTING; -import static glide.api.models.commands.SetOptions.TimeToLiveType.UNIX_SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -21,6 +19,7 @@ import glide.api.models.commands.InfoOptions; import glide.api.models.commands.SetOptions; +import glide.api.models.commands.SetOptions.Expiry; import glide.managers.CommandManager; import glide.managers.ConnectionManager; import java.util.concurrent.CompletableFuture; @@ -223,13 +222,9 @@ public void set_with_SetOptions_OnlyIfExists_returns_success() { SetOptions.builder() .conditionalSet(ONLY_IF_EXISTS) .returnOldValue(false) - .expiry( - SetOptions.TimeToLive.builder() - .type(SetOptions.TimeToLiveType.KEEP_EXISTING) - .build()) + .expiry(Expiry.KeepExisting()) .build(); - String[] arguments = - new String[] {key, value, ONLY_IF_EXISTS.getRedisApi(), KEEP_EXISTING.getRedisApi()}; + String[] arguments = new String[] {key, value, ONLY_IF_EXISTS.getRedisApi(), "KEEPTTL"}; CompletableFuture testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn(null); @@ -254,16 +249,11 @@ public void set_with_SetOptions_OnlyIfDoesNotExist_returns_success() { SetOptions.builder() .conditionalSet(ONLY_IF_DOES_NOT_EXIST) .returnOldValue(true) - .expiry(SetOptions.TimeToLive.builder().type(UNIX_SECONDS).count(60).build()) + .expiry(Expiry.UnixSeconds(60L)) .build(); String[] arguments = new String[] { - key, - value, - ONLY_IF_DOES_NOT_EXIST.getRedisApi(), - RETURN_OLD_VALUE, - UNIX_SECONDS.getRedisApi(), - "60" + key, value, ONLY_IF_DOES_NOT_EXIST.getRedisApi(), RETURN_OLD_VALUE, "EXAT", "60" }; CompletableFuture testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn(value); diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index c5c81a09b8..d43e1d2165 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -10,9 +10,9 @@ import static glide.api.models.commands.InfoOptions.Section.REPLICATION; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_DOES_NOT_EXIST; import static glide.api.models.commands.SetOptions.ConditionalSet.ONLY_IF_EXISTS; -import static glide.api.models.commands.SetOptions.TimeToLiveType.KEEP_EXISTING; -import static glide.api.models.commands.SetOptions.TimeToLiveType.MILLISECONDS; -import static glide.api.models.commands.SetOptions.TimeToLiveType.UNIX_SECONDS; +import static glide.api.models.commands.SetOptions.Expiry.KeepExisting; +import static glide.api.models.commands.SetOptions.Expiry.Milliseconds; +import static glide.api.models.commands.SetOptions.Expiry.UnixSeconds; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_NODES; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.ALL_PRIMARIES; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute.RANDOM; @@ -28,7 +28,7 @@ import glide.api.models.ClusterValue; import glide.api.models.commands.InfoOptions; import glide.api.models.commands.SetOptions; -import glide.api.models.commands.SetOptions.TimeToLive; +import glide.api.models.commands.SetOptions.Expiry; import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.RedisClusterClientConfiguration; import glide.api.models.configuration.RequestRoutingConfiguration; @@ -264,13 +264,12 @@ public void set_only_if_does_not_exists_existing_key() { @Test @SneakyThrows public void set_value_with_ttl_and_update_value_with_keeping_ttl() { - var options = SetOptions.builder().expiry( - SetOptions.TimeToLive.builder().count(2000).type(MILLISECONDS).build()).build(); + SetOptions options = SetOptions.builder().expiry(Milliseconds(2000L)).build(); clusterClient.set("set_value_with_ttl_and_update_value_with_keeping_ttl", INITIAL_VALUE, options).get(10, SECONDS); var data = clusterClient.get("set_value_with_ttl_and_update_value_with_keeping_ttl").get(10, SECONDS); assertEquals(INITIAL_VALUE, data); - options = SetOptions.builder().expiry(TimeToLive.builder().type(KEEP_EXISTING).build()).build(); + options = SetOptions.builder().expiry(KeepExisting()).build(); clusterClient.set("set_value_with_ttl_and_update_value_with_keeping_ttl", ANOTHER_VALUE, options).get(10, SECONDS); data = clusterClient.get("set_value_with_ttl_and_update_value_with_keeping_ttl").get(10, SECONDS); assertEquals(ANOTHER_VALUE, data); @@ -284,12 +283,12 @@ public void set_value_with_ttl_and_update_value_with_keeping_ttl() { @Test @SneakyThrows public void set_value_with_ttl_and_update_value_with_new_ttl() { - var options = SetOptions.builder().expiry(TimeToLive.builder().count(100500).type(MILLISECONDS).build()).build(); + SetOptions options = SetOptions.builder().expiry(Milliseconds(100500L)).build(); clusterClient.set("set_value_with_ttl_and_update_value_with_new_ttl", INITIAL_VALUE, options).get(10, SECONDS); - var data = clusterClient.get("set_value_with_ttl_and_update_value_with_new_ttl").get(10, SECONDS); + String data = clusterClient.get("set_value_with_ttl_and_update_value_with_new_ttl").get(10, SECONDS); assertEquals(INITIAL_VALUE, data); - options = SetOptions.builder().expiry(TimeToLive.builder().count(2000).type(MILLISECONDS).build()).build(); + options = SetOptions.builder().expiry(Milliseconds(2000L)).build(); clusterClient.set("set_value_with_ttl_and_update_value_with_new_ttl", ANOTHER_VALUE, options).get(10, SECONDS); data = clusterClient.get("set_value_with_ttl_and_update_value_with_new_ttl").get(10, SECONDS); assertEquals(ANOTHER_VALUE, data); @@ -303,9 +302,9 @@ public void set_value_with_ttl_and_update_value_with_new_ttl() { @Test @SneakyThrows public void set_expired_value() { // expiration is in the past - var options = SetOptions.builder().expiry(TimeToLive.builder().count(100500).type(UNIX_SECONDS).build()).build(); + SetOptions options = SetOptions.builder().expiry(UnixSeconds(100500L)).build(); clusterClient.set("set_expired_value", INITIAL_VALUE, options).get(10, SECONDS); - var data = clusterClient.get("set_expired_value").get(10, SECONDS); + String data = clusterClient.get("set_expired_value").get(10, SECONDS); assertNull(data); } } From 8388c49de3819c0be4a4b3b9f5f3ffa2b21e81ad Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 18:16:39 -0800 Subject: [PATCH 30/36] Update cluster mode javadocs Signed-off-by: Andrew Carbonetto --- .../java/glide/api/RedisClusterClient.java | 4 +-- .../ConnectionManagementClusterCommands.java | 10 ++++--- .../api/commands/GenericClusterCommands.java | 4 +++ .../ServerManagementClusterCommands.java | 28 ++++++++++++------- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index cfd6d2f21b..45c046a181 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -73,9 +73,9 @@ public CompletableFuture ping(@NonNull Route route) { } @Override - public CompletableFuture ping(@NonNull String msg, @NonNull Route route) { + public CompletableFuture ping(@NonNull String str, @NonNull Route route) { return commandManager.submitNewCommand( - Ping, new String[] {msg}, Optional.of(route), this::handleStringResponse); + Ping, new String[] {str}, Optional.of(route), this::handleStringResponse); } @Override diff --git a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java index 761184a4fe..5b92cb147b 100644 --- a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java +++ b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java @@ -14,7 +14,8 @@ public interface ConnectionManagementClusterCommands { /** * Ping the Redis server. * - * @param route Routing configuration for the command + * @param route Routing configuration for the command. Client will route the command to the nodes + * defined. * @see redis.io for details. * @return Response from Redis containing a String. */ @@ -24,9 +25,10 @@ public interface ConnectionManagementClusterCommands { * Ping the Redis server. * * @see redis.io for details. - * @param msg The ping argument that will be returned. - * @param route Routing configuration for the command + * @param str The ping argument that will be returned. + * @param route Routing configuration for the command. Client will route the command to the nodes + * defined. * @return Response from Redis containing a String. */ - CompletableFuture ping(String msg, Route route); + CompletableFuture ping(String str, Route route); } diff --git a/java/client/src/main/java/glide/api/commands/GenericClusterCommands.java b/java/client/src/main/java/glide/api/commands/GenericClusterCommands.java index 7d58bd4580..b3622257a0 100644 --- a/java/client/src/main/java/glide/api/commands/GenericClusterCommands.java +++ b/java/client/src/main/java/glide/api/commands/GenericClusterCommands.java @@ -15,6 +15,8 @@ public interface GenericClusterCommands { * Executes a single command, without checking inputs. Every part of the command, including * subcommands, should be added as a separate value in {@code args}. * + *

The command will be routed to all primaries. + * * @remarks This function should only be used for single-response commands. Commands that don't * 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 @@ -33,6 +35,8 @@ public interface GenericClusterCommands { * Executes a single command, without checking inputs. Every part of the command, including * subcommands, should be added as a separate value in {@code args}. * + *

Client will route the command to the nodes defined by route. + * * @remarks This function should only be used for single-response commands. Commands that don't * 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 diff --git a/java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java b/java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java index 214f53a9a9..362c3a215d 100644 --- a/java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java +++ b/java/client/src/main/java/glide/api/commands/ServerManagementClusterCommands.java @@ -14,11 +14,13 @@ public interface ServerManagementClusterCommands { /** - * Get information and statistics about the Redis server. DEFAULT option is assumed + * Get information and statistics about the Redis server. DEFAULT option is assumed. The command + * will be routed to all primaries. * * @see redis.io for details. {@link * InfoOptions.Section#DEFAULT} option is assumed. - * @return Response from Redis cluster containing a String. + * @return Response from Redis cluster with a String containing the information for + * the sections requested. */ CompletableFuture> info(); @@ -26,20 +28,24 @@ public interface ServerManagementClusterCommands { * Get information and statistics about the Redis server. DEFAULT option is assumed * * @see redis.io for details. - * @param route Routing configuration for the command - * @return Response from Redis cluster containing a String. + * @param route Routing configuration for the command. Client will route the command to the nodes + * defined. + * @return Response from Redis cluster with a String with the requested Sections. + * When specifying a route other than a single node, it returns a dictionary + * where each address is the key and its corresponding node response is the value. */ CompletableFuture> info(Route route); /** - * Get information and statistics about the Redis server. + * Get information and statistics about the Redis server. The command will be routed to all + * primaries. * * @see redis.io for details. * @param options - A list of {@link InfoOptions.Section} values specifying which sections of * information to retrieve. When no parameter is provided, the {@link * InfoOptions.Section#DEFAULT} option is assumed. - * @return Response from Redis cluster containing a String with the requested - * Sections. + * @return Response from Redis cluster with a String containing the information for + * the sections requested. Sections. */ CompletableFuture> info(InfoOptions options); @@ -50,9 +56,11 @@ public interface ServerManagementClusterCommands { * @param options - A list of {@link InfoOptions.Section} values specifying which sections of * information to retrieve. When no parameter is provided, the {@link * InfoOptions.Section#DEFAULT} option is assumed. - * @param route Routing configuration for the command - * @return Response from Redis cluster containing a String with the requested - * Sections. + * @param route Routing configuration for the command. Client will route the command to the nodes + * defined. + * @return Response from Redis cluster with a String with the requested Sections. + * When specifying a route other than a single node, it returns a dictionary + * where each address is the key and its corresponding node response is the value. */ CompletableFuture> info(InfoOptions options, Route route); } From a96658b6d40bd39954e3920cb0aa20635a9eb5d6 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 18:29:23 -0800 Subject: [PATCH 31/36] Update javadocs to be consistent with Node client Signed-off-by: Andrew Carbonetto --- java/client/src/main/java/glide/api/BaseClient.java | 4 ++-- .../glide/api/commands/ConnectionManagementCommands.java | 6 +++--- .../java/glide/api/commands/ServerManagementCommands.java | 6 ++++-- .../src/main/java/glide/api/commands/StringCommands.java | 6 +++--- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 9ad3782d02..44fb287f94 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -175,8 +175,8 @@ public CompletableFuture ping() { } @Override - public CompletableFuture ping(String msg) { - return commandManager.submitNewCommand(Ping, new String[] {msg}, this::handleStringResponse); + public CompletableFuture ping(String str) { + return commandManager.submitNewCommand(Ping, new String[] {str}, this::handleStringResponse); } @Override diff --git a/java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java index aaaf096206..3e6ff7cbc5 100644 --- a/java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java +++ b/java/client/src/main/java/glide/api/commands/ConnectionManagementCommands.java @@ -14,7 +14,7 @@ public interface ConnectionManagementCommands { * Ping the Redis server. * * @see redis.io for details. - * @return Response from Redis containing a String. + * @return Response from Redis containing a String with "PONG". */ CompletableFuture ping(); @@ -23,7 +23,7 @@ public interface ConnectionManagementCommands { * * @see redis.io for details. * @param msg The ping argument that will be returned. - * @return Response from Redis containing a String. + * @return Response from Redis containing a String with a copy of the argument. */ - CompletableFuture ping(String msg); + CompletableFuture ping(String str); } diff --git a/java/client/src/main/java/glide/api/commands/ServerManagementCommands.java b/java/client/src/main/java/glide/api/commands/ServerManagementCommands.java index 3d6dcd99c0..a65d0e0619 100644 --- a/java/client/src/main/java/glide/api/commands/ServerManagementCommands.java +++ b/java/client/src/main/java/glide/api/commands/ServerManagementCommands.java @@ -17,7 +17,8 @@ public interface ServerManagementCommands { * Section#DEFAULT} option is assumed. * * @see redis.io for details. - * @return Response from Redis containing a String. + * @return Response from Redis containing a String with the information for the + * default sections. */ CompletableFuture info(); @@ -27,7 +28,8 @@ public interface ServerManagementCommands { * @see redis.io for details. * @param options A list of {@link Section} values specifying which sections of information to * retrieve. When no parameter is provided, the {@link Section#DEFAULT} option is assumed. - * @return Response from Redis containing a String with the requested sections. + * @return Response from Redis containing a String with the information for the + * sections requested. */ CompletableFuture info(InfoOptions options); } diff --git a/java/client/src/main/java/glide/api/commands/StringCommands.java b/java/client/src/main/java/glide/api/commands/StringCommands.java index 2664f32b68..8037a4682c 100644 --- a/java/client/src/main/java/glide/api/commands/StringCommands.java +++ b/java/client/src/main/java/glide/api/commands/StringCommands.java @@ -18,17 +18,17 @@ public interface StringCommands { * exists. * * @see redis.io for details. - * @param key The key to retrieve from the database. + * @param key The key to retrieve from the database. * @return Response from Redis. If key exists, returns the value of * key as a String. Otherwise, return null. */ CompletableFuture get(String key); /** - * Set the given key with the given value. + * Set the given key with the given value. * * @see redis.io for details. - * @param key The key to store. + * @param key The key to store. * @param value The value to store with the given key. * @return Response from Redis containing "OK". */ From 189badb7c729ea3f6d47c0950fd2785ab0918c02 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 18:32:11 -0800 Subject: [PATCH 32/36] minor update Signed-off-by: Andrew Carbonetto --- .../glide/api/commands/ConnectionManagementClusterCommands.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java index 5b92cb147b..31c51c72b8 100644 --- a/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java +++ b/java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java @@ -14,9 +14,9 @@ public interface ConnectionManagementClusterCommands { /** * Ping the Redis server. * + * @see redis.io for details. * @param route Routing configuration for the command. Client will route the command to the nodes * defined. - * @see redis.io for details. * @return Response from Redis containing a String. */ CompletableFuture ping(Route route); From fcd13df0f35819b9f14b006201b6f1c51829e402 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 18:35:26 -0800 Subject: [PATCH 33/36] minor update Signed-off-by: Andrew Carbonetto --- java/client/src/main/java/glide/managers/CommandManager.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index b481b5b311..816f581288 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -18,6 +18,7 @@ import redis_request.RedisRequestOuterClass.RequestType; import redis_request.RedisRequestOuterClass.Routes; import redis_request.RedisRequestOuterClass.SimpleRoutes; +import redis_request.RedisRequestOuterClass.SlotTypes; import response.ResponseOuterClass.Response; /** @@ -170,7 +171,7 @@ private RedisRequest.Builder prepareRedisRequestRoute( RedisRequestOuterClass.SlotIdRoute.newBuilder() .setSlotId(((SlotIdRoute) route.get()).getSlotId()) .setSlotType( - RedisRequestOuterClass.SlotTypes.forNumber( + SlotTypes.forNumber( ((SlotIdRoute) route.get()).getSlotType().ordinal())))); } else if (route.get() instanceof SlotKeyRoute) { builder.setRoute( @@ -179,7 +180,7 @@ private RedisRequest.Builder prepareRedisRequestRoute( RedisRequestOuterClass.SlotKeyRoute.newBuilder() .setSlotKey(((SlotKeyRoute) route.get()).getSlotKey()) .setSlotType( - RedisRequestOuterClass.SlotTypes.forNumber( + SlotTypes.forNumber( ((SlotKeyRoute) route.get()).getSlotType().ordinal())))); } else { throw new IllegalArgumentException("Unknown type of route"); From dc43db82d44d92c73e0496c15ca257f57f8423d0 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 18:39:39 -0800 Subject: [PATCH 34/36] remove check in SetOptions Signed-off-by: Andrew Carbonetto --- .../main/java/glide/api/models/commands/SetOptions.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/commands/SetOptions.java b/java/client/src/main/java/glide/api/models/commands/SetOptions.java index 61376d38d6..05f4cd5bec 100644 --- a/java/client/src/main/java/glide/api/models/commands/SetOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/SetOptions.java @@ -8,7 +8,6 @@ import static glide.api.models.commands.SetOptions.ExpiryType.UNIX_SECONDS; import glide.api.commands.StringCommands; -import glide.api.models.exceptions.RequestException; import java.util.ArrayList; import java.util.List; import lombok.Builder; @@ -161,10 +160,8 @@ public String[] toArgs() { if (expiry != null) { optionArgs.add(expiry.type.redisApi); if (expiry.type != KEEP_EXISTING) { - if (expiry.count == null) { - throw new RequestException( - "Set command received expiry type " + expiry.type + ", but count was not set."); - } + assert expiry.count != null + : "Set command received expiry type " + expiry.type + ", but count was not set."; optionArgs.add(expiry.count.toString()); } } From 247ffdb403151452c15255b53e6132cb81e4a4ad Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 18:47:31 -0800 Subject: [PATCH 35/36] Update OK statis string Signed-off-by: Andrew Carbonetto --- java/client/src/main/java/glide/api/BaseClient.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 44fb287f94..d1b98f0a79 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -26,6 +26,7 @@ import java.util.function.BiFunction; import lombok.AllArgsConstructor; import org.apache.commons.lang3.ArrayUtils; +import response.ResponseOuterClass; import response.ResponseOuterClass.Response; /** Base Client class for Redis */ @@ -33,7 +34,8 @@ public abstract class BaseClient implements AutoCloseable, StringCommands, ConnectionManagementCommands { - public static final String OK = "OK"; + /** Redis simple string response with "OK" */ + public static final String OK = ResponseOuterClass.ConstantResponse.OK.toString(); protected final ConnectionManager connectionManager; protected final CommandManager commandManager; From cb8cac48f440d013d31203d38f8b6582e491dd76 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 6 Feb 2024 21:55:55 -0800 Subject: [PATCH 36/36] Clean upRedisClusterClientTest.java Signed-off-by: Andrew Carbonetto --- .../test/java/glide/api/RedisClusterClientTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index b41b16fb73..bb299010d6 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -16,7 +16,7 @@ import glide.api.models.ClusterValue; import glide.api.models.commands.InfoOptions; -import glide.api.models.configuration.RequestRoutingConfiguration; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; import glide.managers.CommandManager; import glide.managers.ConnectionManager; import glide.managers.RedisExceptionCheckedFunction; @@ -137,7 +137,7 @@ public void ping_returns_success() { CompletableFuture testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn("PONG"); - RequestRoutingConfiguration.Route route = ALL_NODES; + Route route = ALL_NODES; // match on protobuf request when(commandManager.submitNewCommand( @@ -162,7 +162,7 @@ public void ping_with_message_returns_success() { CompletableFuture testResponse = new CompletableFuture(); testResponse.complete(message); - RequestRoutingConfiguration.Route route = ALL_PRIMARIES; + Route route = ALL_PRIMARIES; // match on protobuf request when(commandManager.submitNewCommand( @@ -208,7 +208,7 @@ public void info_with_route_returns_string() { // setup CompletableFuture> testResponse = mock(CompletableFuture.class); Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); - RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_NODES; + Route route = ALL_NODES; when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); when(commandManager.>submitNewCommand( eq(Info), eq(new String[0]), eq(Optional.of(route)), any())) @@ -233,7 +233,7 @@ public void info_with_route_with_infoOptions_returns_string() { CompletableFuture> testResponse = mock(CompletableFuture.class); Map testClusterValue = Map.of("addr1", "addr1 result", "addr2", "addr2 result"); when(testResponse.get()).thenReturn(ClusterValue.of(testClusterValue)); - RequestRoutingConfiguration.Route route = RequestRoutingConfiguration.SimpleRoute.ALL_PRIMARIES; + Route route = ALL_PRIMARIES; when(commandManager.>submitNewCommand( eq(Info), eq(infoArguments), eq(Optional.of(route)), any())) .thenReturn(testResponse);