diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 06ae9204cc..ee3b02e0c9 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -1,5 +1,9 @@ package glide.api; +import static glide.ffi.resolvers.SocketListenerResolver.getSocket; + +import glide.connectors.handlers.CallbackDispatcher; +import glide.connectors.handlers.ChannelHandler; import glide.ffi.resolvers.RedisValueResolver; import glide.managers.BaseCommandResponseResolver; import glide.managers.CommandManager; @@ -43,4 +47,17 @@ public void close() throws ExecutionException { throw new RuntimeException(e); } } + + protected static ChannelHandler buildChannelHandler() throws InterruptedException { + CallbackDispatcher callbackDispatcher = new CallbackDispatcher(); + return new ChannelHandler(callbackDispatcher, getSocket()); + } + + protected static ConnectionManager buildConnectionManager(ChannelHandler channelHandler) { + return new ConnectionManager(channelHandler); + } + + protected static CommandManager buildCommandManager(ChannelHandler channelHandler) { + return new CommandManager(channelHandler); + } } diff --git a/java/client/src/main/java/glide/api/RedisClient.java b/java/client/src/main/java/glide/api/RedisClient.java index e5230c03f9..9f3897b8a6 100644 --- a/java/client/src/main/java/glide/api/RedisClient.java +++ b/java/client/src/main/java/glide/api/RedisClient.java @@ -1,15 +1,11 @@ package glide.api; -import static glide.ffi.resolvers.SocketListenerResolver.getSocket; - import glide.api.commands.BaseCommands; import glide.api.models.configuration.RedisClientConfiguration; -import glide.connectors.handlers.CallbackDispatcher; import glide.connectors.handlers.ChannelHandler; import glide.managers.CommandManager; import glide.managers.ConnectionManager; import glide.managers.models.Command; -import java.util.Optional; import java.util.concurrent.CompletableFuture; /** @@ -45,23 +41,10 @@ public static CompletableFuture CreateClient(RedisClientConfigurati } } - protected static ChannelHandler buildChannelHandler() throws InterruptedException { - CallbackDispatcher callbackDispatcher = new CallbackDispatcher(); - return new ChannelHandler(callbackDispatcher, getSocket()); - } - - protected static ConnectionManager buildConnectionManager(ChannelHandler channelHandler) { - return new ConnectionManager(channelHandler); - } - - protected static CommandManager buildCommandManager(ChannelHandler channelHandler) { - return new CommandManager(channelHandler); - } - @Override public CompletableFuture customCommand(String[] args) { Command command = Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build(); - return commandManager.submitNewCommand(command, Optional.empty(), this::handleObjectResponse); + return commandManager.submitNewCommand(command, this::handleObjectResponse); } } diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 2ccfd66adb..1b6903d715 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -1,18 +1,14 @@ package glide.api; -import static glide.api.RedisClient.buildChannelHandler; -import static glide.api.RedisClient.buildCommandManager; -import static glide.api.RedisClient.buildConnectionManager; - import glide.api.commands.ClusterBaseCommands; import glide.api.models.ClusterValue; import glide.api.models.configuration.RedisClusterClientConfiguration; -import glide.api.models.configuration.Route; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; import glide.connectors.handlers.ChannelHandler; import glide.managers.CommandManager; import glide.managers.ConnectionManager; import glide.managers.models.Command; -import java.util.Optional; +import java.util.Map; import java.util.concurrent.CompletableFuture; /** @@ -53,15 +49,26 @@ public static CompletableFuture CreateClient( public CompletableFuture> customCommand(String[] args) { Command command = Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build(); + // TODO if a command returns a map as a single value, ClusterValue misleads user return commandManager.submitNewCommand( - command, Optional.empty(), response -> ClusterValue.of(handleObjectResponse(response))); + command, response -> ClusterValue.of(handleObjectResponse(response))); } @Override + @SuppressWarnings("unchecked") public CompletableFuture> customCommand(String[] args, Route route) { Command command = - Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).arguments(args).build(); + Command.builder() + .requestType(Command.RequestType.CUSTOM_COMMAND) + .arguments(args) + .route(route) + .build(); + return commandManager.submitNewCommand( - command, Optional.of(route), response -> ClusterValue.of(handleObjectResponse(response))); + command, + response -> + route.isSingleNodeRoute() + ? ClusterValue.ofSingleValue(handleObjectResponse(response)) + : ClusterValue.ofMultiValue((Map) handleObjectResponse(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 58bf7e5983..4e78106319 100644 --- a/java/client/src/main/java/glide/api/commands/BaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/BaseCommands.java @@ -7,19 +7,19 @@ public interface BaseCommands { /** * Executes a single command, without checking inputs. Every part of the command, including - * subcommands, should be added as a separate value in {@code args}. + * subcommands, should be added as a separate value in args. * * @remarks This function should only be used for single-response commands. Commands that don't - * return response (such as 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 + * 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 + * @return a CompletableFuture with response result from Redis */ 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 e90e5c7aa2..81d9f96d8a 100644 --- a/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java +++ b/java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java @@ -1,7 +1,7 @@ package glide.api.commands; import glide.api.models.ClusterValue; -import glide.api.models.configuration.Route; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; import java.util.concurrent.CompletableFuture; /** 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 d264137f11..34fa7182f5 100644 --- a/java/client/src/main/java/glide/api/models/ClusterValue.java +++ b/java/client/src/main/java/glide/api/models/ClusterValue.java @@ -21,7 +21,7 @@ private ClusterValue() {} * Check with {@link #hasMultiData()} prior to accessing the data. */ public Map getMultiValue() { - assert hasMultiData(); + assert hasMultiData() : "No multi value stored"; return multiValue; } @@ -30,11 +30,11 @@ public Map getMultiValue() { * Check with {@link #hasSingleData()} ()} prior to accessing the data. */ public T getSingleValue() { - assert hasSingleData(); + assert hasSingleData() : "No single value stored"; return singleValue; } - /** A constructor for the value. */ + /** A constructor for the value with type auto-detection. */ @SuppressWarnings("unchecked") public static ClusterValue of(Object data) { var res = new ClusterValue(); @@ -46,6 +46,20 @@ public static ClusterValue of(Object data) { return res; } + /** A constructor for the value. */ + public static ClusterValue ofSingleValue(T data) { + var res = new ClusterValue(); + res.singleValue = data; + return res; + } + + /** A constructor for the value. */ + public static ClusterValue ofMultiValue(Map data) { + var res = new ClusterValue(); + res.multiValue = data; + return res; + } + /** Check that multi-value is stored in this object. Use it prior to accessing the data. */ public boolean hasMultiData() { return multiValue != null; 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 new file mode 100644 index 0000000000..be65d5875f --- /dev/null +++ b/java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java @@ -0,0 +1,91 @@ +package glide.api.models.configuration; + +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import redis_request.RedisRequestOuterClass.SimpleRoutes; +import redis_request.RedisRequestOuterClass.SlotTypes; + +/** Request routing configuration. */ +public class RequestRoutingConfiguration { + + /** + * Basic interface. Please use one of the following implementations: + * + *
    + *
  • {@link SimpleRoute} + *
  • {@link SlotIdRoute} + *
  • {@link SlotKeyRoute} + *
+ */ + public interface Route { + boolean isSingleNodeRoute(); + } + + @RequiredArgsConstructor + @Getter + public enum SimpleRoute implements Route { + /** Route request to all nodes. */ + ALL_NODES(SimpleRoutes.AllNodes), + /** Route request to all primary nodes. */ + ALL_PRIMARIES(SimpleRoutes.AllPrimaries), + /** Route request to a random node. */ + RANDOM(SimpleRoutes.Random); + + private final SimpleRoutes protobufMapping; + + @Override + public boolean isSingleNodeRoute() { + return this == RANDOM; + } + } + + @RequiredArgsConstructor + @Getter + public enum SlotType { + PRIMARY(SlotTypes.Primary), + REPLICA(SlotTypes.Replica); + + private final SlotTypes slotTypes; + } + + /** + * Request routing configuration overrides the {@link ReadFrom} connection configuration.
+ * If {@link SlotType#REPLICA} is used, the request will be routed to a replica, even if the + * strategy is {@link ReadFrom#PRIMARY}. + */ + @RequiredArgsConstructor + @Getter + public static class SlotIdRoute implements Route { + /** + * Slot number. There are 16384 slots in a redis cluster, and each shard manages a slot range. + * Unless the slot is known, it's better to route using {@link SlotType#PRIMARY}. + */ + private final int slotId; + + private final SlotType slotType; + + @Override + public boolean isSingleNodeRoute() { + return true; + } + } + + /** + * Request routing configuration overrides the {@link ReadFrom} connection configuration.
+ * If {@link SlotType#REPLICA} is used, the request will be routed to a replica, even if the + * strategy is {@link ReadFrom#PRIMARY}. + */ + @RequiredArgsConstructor + @Getter + public static class SlotKeyRoute implements Route { + /** The request will be sent to nodes managing this key. */ + private final String slotKey; + + private final SlotType slotType; + + @Override + public boolean isSingleNodeRoute() { + return true; + } + } +} diff --git a/java/client/src/main/java/glide/api/models/configuration/Route.java b/java/client/src/main/java/glide/api/models/configuration/Route.java deleted file mode 100644 index e2b7c014aa..0000000000 --- a/java/client/src/main/java/glide/api/models/configuration/Route.java +++ /dev/null @@ -1,126 +0,0 @@ -package glide.api.models.configuration; - -import java.util.Optional; -import lombok.Builder; -import lombok.Getter; - -/** Request routing configuration. */ -public class Route { - - public enum RouteType { - /** Route request to all nodes. */ - ALL_NODES, - /** Route request to all primary nodes. */ - ALL_PRIMARIES, - /** Route request to a random node. */ - RANDOM, - /** Route request to the primary node that contains the slot with the given id. */ - PRIMARY_SLOT_ID, - /** Route request to the replica node that contains the slot with the given id. */ - REPLICA_SLOT_ID, - /** Route request to the primary node that contains the slot that the given key matches. */ - PRIMARY_SLOT_KEY, - /** Route request to the replica node that contains the slot that the given key matches. */ - REPLICA_SLOT_KEY, - } - - @Getter private final RouteType routeType; - - private final Optional slotId; - - private final Optional slotKey; - - public Integer getSlotId() { - assert slotId.isPresent(); - return slotId.get(); - } - - public String getSlotKey() { - assert slotKey.isPresent(); - return slotKey.get(); - } - - private Route(RouteType routeType, Integer slotId) { - this.routeType = routeType; - this.slotId = Optional.of(slotId); - this.slotKey = Optional.empty(); - } - - private Route(RouteType routeType, String slotKey) { - this.routeType = routeType; - this.slotId = Optional.empty(); - this.slotKey = Optional.of(slotKey); - } - - private Route(RouteType routeType) { - this.routeType = routeType; - this.slotId = Optional.empty(); - this.slotKey = Optional.empty(); - } - - public static class Builder { - private final RouteType routeType; - private int slotId; - private boolean slotIdSet = false; - private String slotKey; - private boolean slotKeySet = false; - - /** - * Request routing configuration overrides the {@link ReadFrom} connection configuration.
- * If {@link RouteType#REPLICA_SLOT_ID} or {@link RouteType#REPLICA_SLOT_KEY} is used, the - * request will be routed to a replica, even if the strategy is {@link ReadFrom#PRIMARY}. - */ - public Builder(RouteType routeType) { - this.routeType = routeType; - } - - /** - * Slot number. There are 16384 slots in a redis cluster, and each shard manages a slot range. - * Unless the slot is known, it's better to route using {@link RouteType#PRIMARY_SLOT_KEY} or - * {@link RouteType#REPLICA_SLOT_KEY}.
- * Could be used with {@link RouteType#PRIMARY_SLOT_ID} or {@link RouteType#REPLICA_SLOT_ID} - * only. - */ - public Builder setSlotId(int slotId) { - if (!(routeType == RouteType.PRIMARY_SLOT_ID || routeType == RouteType.REPLICA_SLOT_ID)) { - throw new IllegalArgumentException( - "Slot ID could be set for corresponding types of route only"); - } - this.slotId = slotId; - slotIdSet = true; - return this; - } - - /** - * The request will be sent to nodes managing this key.
- * Could be used with {@link RouteType#PRIMARY_SLOT_KEY} or {@link RouteType#REPLICA_SLOT_KEY} - * only. - */ - public Builder setSlotKey(String slotKey) { - if (!(routeType == RouteType.PRIMARY_SLOT_KEY || routeType == RouteType.REPLICA_SLOT_KEY)) { - throw new IllegalArgumentException( - "Slot key could be set for corresponding types of route only"); - } - this.slotKey = slotKey; - slotKeySet = true; - return this; - } - - public Route build() { - if (routeType == RouteType.PRIMARY_SLOT_ID || routeType == RouteType.REPLICA_SLOT_ID) { - if (!slotIdSet) { - throw new IllegalArgumentException("Slot ID is missing"); - } - return new Route(routeType, slotId); - } - if (routeType == RouteType.PRIMARY_SLOT_KEY || routeType == RouteType.REPLICA_SLOT_KEY) { - if (!slotKeySet) { - throw new IllegalArgumentException("Slot key is missing"); - } - return new Route(routeType, slotKey); - } - - return new Route(routeType); - } - } -} diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index 587243ca06..3d2a08dc6c 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -1,6 +1,9 @@ package glide.managers; -import glide.api.models.configuration.Route; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; +import glide.api.models.configuration.RequestRoutingConfiguration.SimpleRoute; +import glide.api.models.configuration.RequestRoutingConfiguration.SlotIdRoute; +import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; import glide.connectors.handlers.CallbackDispatcher; import glide.connectors.handlers.ChannelHandler; import glide.managers.models.Command; @@ -11,10 +14,6 @@ import redis_request.RedisRequestOuterClass.Command.ArgsArray; import redis_request.RedisRequestOuterClass.RedisRequest; import redis_request.RedisRequestOuterClass.RequestType; -import redis_request.RedisRequestOuterClass.SimpleRoutes; -import redis_request.RedisRequestOuterClass.SlotIdRoute; -import redis_request.RedisRequestOuterClass.SlotKeyRoute; -import redis_request.RedisRequestOuterClass.SlotTypes; import response.ResponseOuterClass.Response; /** @@ -31,18 +30,20 @@ public class CommandManager { * Build a command and send. * * @param command The command to execute - * @param route The routing options * @param responseHandler The handler for the response object * @return A result promise of type T */ public CompletableFuture submitNewCommand( - Command command, - Optional route, - RedisExceptionCheckedFunction responseHandler) { + Command 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(), route), true) + .write( + prepareRedisRequest( + command.getRequestType(), + command.getArguments(), + Optional.ofNullable(command.getRoute())), + true) .thenApplyAsync(responseHandler::apply); } @@ -83,57 +84,28 @@ private RedisRequest.Builder prepareRedisRequest( return builder; } - switch (route.get().getRouteType()) { - case RANDOM: - case ALL_NODES: - case ALL_PRIMARIES: - builder.setRoute( - RedisRequestOuterClass.Routes.newBuilder() - .setSimpleRoutes(getSimpleRoutes(route.get().getRouteType())) - .build()); - break; - case PRIMARY_SLOT_KEY: - case REPLICA_SLOT_KEY: - builder.setRoute( - RedisRequestOuterClass.Routes.newBuilder() - .setSlotKeyRoute( - SlotKeyRoute.newBuilder() - .setSlotKey(route.get().getSlotKey()) - .setSlotType(getSlotTypes(route.get().getRouteType())))); - break; - case PRIMARY_SLOT_ID: - case REPLICA_SLOT_ID: - builder.setRoute( - RedisRequestOuterClass.Routes.newBuilder() - .setSlotIdRoute( - SlotIdRoute.newBuilder() - .setSlotId(route.get().getSlotId()) - .setSlotType(getSlotTypes(route.get().getRouteType())))); + if (route.get() instanceof SimpleRoute) { + builder.setRoute( + RedisRequestOuterClass.Routes.newBuilder() + .setSimpleRoutes(((SimpleRoute) route.get()).getProtobufMapping()) + .build()); + } else if (route.get() instanceof SlotIdRoute) { + builder.setRoute( + RedisRequestOuterClass.Routes.newBuilder() + .setSlotIdRoute( + RedisRequestOuterClass.SlotIdRoute.newBuilder() + .setSlotId(((SlotIdRoute) route.get()).getSlotId()) + .setSlotType(((SlotIdRoute) route.get()).getSlotType().getSlotTypes()))); + } else if (route.get() instanceof SlotKeyRoute) { + builder.setRoute( + RedisRequestOuterClass.Routes.newBuilder() + .setSlotKeyRoute( + RedisRequestOuterClass.SlotKeyRoute.newBuilder() + .setSlotKey(((SlotKeyRoute) route.get()).getSlotKey()) + .setSlotType(((SlotKeyRoute) route.get()).getSlotType().getSlotTypes()))); + } else { + throw new IllegalArgumentException("Unknown type of route"); } return builder; } - - private SimpleRoutes getSimpleRoutes(Route.RouteType routeType) { - switch (routeType) { - case RANDOM: - return SimpleRoutes.Random; - case ALL_NODES: - return SimpleRoutes.AllNodes; - case ALL_PRIMARIES: - return SimpleRoutes.AllPrimaries; - } - throw new IllegalStateException("Unreachable code"); - } - - private SlotTypes getSlotTypes(Route.RouteType routeType) { - switch (routeType) { - case PRIMARY_SLOT_ID: - case PRIMARY_SLOT_KEY: - return SlotTypes.Primary; - case REPLICA_SLOT_ID: - case REPLICA_SLOT_KEY: - return SlotTypes.Replica; - } - throw new IllegalStateException("Unreachable code"); - } } diff --git a/java/client/src/main/java/glide/managers/models/Command.java b/java/client/src/main/java/glide/managers/models/Command.java index 9935c80849..8f9e2b0c98 100644 --- a/java/client/src/main/java/glide/managers/models/Command.java +++ b/java/client/src/main/java/glide/managers/models/Command.java @@ -1,5 +1,6 @@ package glide.managers.models; +import glide.api.models.configuration.RequestRoutingConfiguration.Route; import lombok.Builder; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -14,6 +15,9 @@ 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[] {}; diff --git a/java/client/src/test/java/glide/api/RedisClientCreateTest.java b/java/client/src/test/java/glide/api/RedisClientCreateTest.java index 3a18154851..5d083748fb 100644 --- a/java/client/src/test/java/glide/api/RedisClientCreateTest.java +++ b/java/client/src/test/java/glide/api/RedisClientCreateTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; import glide.api.models.configuration.RedisClientConfiguration; @@ -21,24 +22,23 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; -import org.mockito.Mockito; public class RedisClientCreateTest { - private MockedStatic mockedClient; + private MockedStatic mockedClient; private ChannelHandler channelHandler; private ConnectionManager connectionManager; private CommandManager commandManager; @BeforeEach public void init() { - mockedClient = Mockito.mockStatic(RedisClient.class); + mockedClient = mockStatic(BaseClient.class); channelHandler = mock(ChannelHandler.class); commandManager = mock(CommandManager.class); connectionManager = mock(ConnectionManager.class); - mockedClient.when(RedisClient::buildChannelHandler).thenReturn(channelHandler); + mockedClient.when(BaseClient::buildChannelHandler).thenReturn(channelHandler); mockedClient.when(() -> buildConnectionManager(channelHandler)).thenReturn(connectionManager); mockedClient.when(() -> buildCommandManager(channelHandler)).thenReturn(commandManager); } @@ -58,7 +58,6 @@ public void createClient_withConfig_successfullyReturnsRedisClient() { RedisClientConfiguration config = RedisClientConfiguration.builder().build(); when(connectionManager.connectToRedis(eq(config))).thenReturn(connectToRedisFuture); - mockedClient.when(() -> CreateClient(config)).thenCallRealMethod(); // exercise CompletableFuture result = CreateClient(config); @@ -79,13 +78,11 @@ public void createClient_errorOnConnectionThrowsExecutionException() { RedisClientConfiguration config = RedisClientConfiguration.builder().build(); when(connectionManager.connectToRedis(eq(config))).thenReturn(connectToRedisFuture); - mockedClient.when(() -> CreateClient(config)).thenCallRealMethod(); // exercise CompletableFuture result = CreateClient(config); - ExecutionException executionException = - assertThrows(ExecutionException.class, () -> result.get()); + ExecutionException executionException = assertThrows(ExecutionException.class, result::get); // verify assertEquals(exception, executionException.getCause()); diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index 5ee376564c..e3d8c4329b 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -36,7 +36,7 @@ public void customCommand_success() throws ExecutionException, InterruptedExcept String cmd = "GETSTRING"; CompletableFuture testResponse = mock(CompletableFuture.class); when(testResponse.get()).thenReturn(value); - when(commandManager.submitNewCommand(any(), any(), any())).thenReturn(testResponse); + when(commandManager.submitNewCommand(any(), any())).thenReturn(testResponse); // exercise CompletableFuture response = service.customCommand(new String[] {cmd, key}); @@ -56,7 +56,7 @@ public void customCommand_interruptedException() throws ExecutionException, Inte CompletableFuture testResponse = mock(CompletableFuture.class); InterruptedException interruptedException = new InterruptedException(); when(testResponse.get()).thenThrow(interruptedException); - when(commandManager.submitNewCommand(any(), any(), any())).thenReturn(testResponse); + when(commandManager.submitNewCommand(any(), any())).thenReturn(testResponse); // exercise InterruptedException exception = diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java new file mode 100644 index 0000000000..2d4bd4eda7 --- /dev/null +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -0,0 +1,101 @@ +package glide.api; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import glide.managers.CommandManager; +import glide.managers.RedisExceptionCheckedFunction; +import glide.managers.models.Command; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import lombok.SneakyThrows; +import org.junit.jupiter.api.Test; +import response.ResponseOuterClass.Response; + +public class RedisClusterClientTest { + + @Test + @SneakyThrows + public void custom_command_returns_single_value() { + var commandManager = new TestCommandManager(null); + + var client = new TestClient(commandManager, "TEST"); + + var value = client.customCommand(new String[0]).get(); + assertAll( + () -> assertTrue(value.hasSingleData()), + () -> assertEquals("TEST", value.getSingleValue())); + } + + @Test + @SneakyThrows + public void custom_command_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(); + assertAll( + () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); + } + + @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() { + 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], () -> true).get(); + assertAll( + () -> assertTrue(value.hasSingleData()), () -> assertEquals(data, value.getSingleValue())); + } + + @Test + @SneakyThrows + public void custom_command_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], () -> false).get(); + assertAll( + () -> assertTrue(value.hasMultiData()), () -> assertEquals(data, value.getMultiValue())); + } + + private static class TestClient extends RedisClusterClient { + + private final Object object; + + public TestClient(CommandManager commandManager, Object objectToReturn) { + super(null, commandManager); + object = objectToReturn; + } + + @Override + protected Object handleObjectResponse(Response response) { + return object; + } + } + + private static class TestCommandManager extends CommandManager { + + private final Response response; + + public TestCommandManager(Response responseToReturn) { + super(null); + response = responseToReturn; + } + + @Override + public CompletableFuture submitNewCommand( + Command command, RedisExceptionCheckedFunction responseHandler) { + return CompletableFuture.supplyAsync(() -> responseHandler.apply(response)); + } + } +} diff --git a/java/client/src/test/java/glide/api/models/ClusterValueTests.java b/java/client/src/test/java/glide/api/models/ClusterValueTests.java index 93b7837f37..24965aed72 100644 --- a/java/client/src/test/java/glide/api/models/ClusterValueTests.java +++ b/java/client/src/test/java/glide/api/models/ClusterValueTests.java @@ -20,7 +20,10 @@ public void handle_null() { () -> assertFalse(value.hasMultiData()), () -> assertTrue(value.hasSingleData()), () -> assertNull(value.getSingleValue()), - () -> assertThrows(Throwable.class, value::getMultiValue)); + () -> + assertEquals( + "No multi value stored", + assertThrows(Throwable.class, value::getMultiValue).getMessage())); } @Test @@ -30,7 +33,10 @@ public void handle_single_data() { () -> assertFalse(value.hasMultiData()), () -> assertTrue(value.hasSingleData()), () -> assertEquals(42, value.getSingleValue()), - () -> assertThrows(Throwable.class, value::getMultiValue)); + () -> + assertEquals( + "No multi value stored", + assertThrows(Throwable.class, value::getMultiValue).getMessage())); } @Test @@ -42,6 +48,27 @@ public void handle_multi_data() { () -> assertFalse(value.hasSingleData()), () -> assertNotNull(value.getMultiValue()), () -> assertEquals(data, value.getMultiValue()), - () -> assertThrows(Throwable.class, value::getSingleValue)); + () -> + assertEquals( + "No single value stored", + assertThrows(Throwable.class, value::getSingleValue).getMessage())); + } + + @Test + public void single_value_ctor() { + var value = ClusterValue.ofSingleValue(Map.of("config1", "param1", "config2", "param2")); + assertAll( + () -> assertFalse(value.hasMultiData()), + () -> assertTrue(value.hasSingleData()), + () -> assertNotNull(value.getSingleValue())); + } + + @Test + public void multi_value_ctor() { + var value = ClusterValue.ofMultiValue(Map.of("config1", "param1", "config2", "param2")); + assertAll( + () -> assertTrue(value.hasMultiData()), + () -> assertFalse(value.hasSingleData()), + () -> assertNotNull(value.getMultiValue())); } } diff --git a/java/client/src/test/java/glide/api/models/RouteBuilderTests.java b/java/client/src/test/java/glide/api/models/RouteBuilderTests.java deleted file mode 100644 index 7da2b00677..0000000000 --- a/java/client/src/test/java/glide/api/models/RouteBuilderTests.java +++ /dev/null @@ -1,93 +0,0 @@ -package glide.api.models; - -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 glide.api.models.configuration.Route; -import glide.api.models.configuration.Route.RouteType; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; - -public class RouteBuilderTests { - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_ID", "REPLICA_SLOT_ID"}) - public void slot_id_is_required(RouteType routeType) { - var exception = - assertThrows(IllegalArgumentException.class, () -> new Route.Builder(routeType).build()); - assertEquals("Slot ID is missing", exception.getMessage()); - } - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_KEY", "REPLICA_SLOT_KEY"}) - public void slot_key_is_required(RouteType routeType) { - var exception = - assertThrows(IllegalArgumentException.class, () -> new Route.Builder(routeType).build()); - assertEquals("Slot key is missing", exception.getMessage()); - } - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_KEY", "REPLICA_SLOT_KEY", "ALL_NODES", "ALL_PRIMARIES", "RANDOM"}) - public void slot_id_not_acceptable(RouteType routeType) { - var exception = - assertThrows( - IllegalArgumentException.class, () -> new Route.Builder(routeType).setSlotId(42)); - assertEquals( - "Slot ID could be set for corresponding types of route only", exception.getMessage()); - } - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_ID", "REPLICA_SLOT_ID", "ALL_NODES", "ALL_PRIMARIES", "RANDOM"}) - public void slot_key_not_acceptable(RouteType routeType) { - var exception = - assertThrows( - IllegalArgumentException.class, () -> new Route.Builder(routeType).setSlotKey("D'oh")); - assertEquals( - "Slot key could be set for corresponding types of route only", exception.getMessage()); - } - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_ID", "REPLICA_SLOT_ID"}) - public void build_with_slot_id(RouteType routeType) { - var route = new Route.Builder(routeType).setSlotId(42).build(); - assertAll( - () -> assertEquals(routeType, route.getRouteType()), - () -> assertEquals(42, route.getSlotId()), - () -> assertThrows(Throwable.class, () -> route.getSlotKey())); - } - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_KEY", "REPLICA_SLOT_KEY"}) - public void build_with_slot_key(RouteType routeType) { - var route = new Route.Builder(routeType).setSlotKey("test").build(); - assertAll( - () -> assertEquals(routeType, route.getRouteType()), - () -> assertEquals("test", route.getSlotKey()), - () -> assertThrows(Throwable.class, () -> route.getSlotId())); - } - - @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"ALL_NODES", "ALL_PRIMARIES", "RANDOM"}) - public void build_simple_route(RouteType routeType) { - var route = new Route.Builder(routeType).build(); - assertAll( - () -> assertEquals(routeType, route.getRouteType()), - () -> assertThrows(Throwable.class, () -> route.getSlotKey()), - () -> assertThrows(Throwable.class, () -> route.getSlotId())); - } -} diff --git a/java/client/src/test/java/glide/managers/CommandManagerTest.java b/java/client/src/test/java/glide/managers/CommandManagerTest.java index 5a4875a465..438021ffbb 100644 --- a/java/client/src/test/java/glide/managers/CommandManagerTest.java +++ b/java/client/src/test/java/glide/managers/CommandManagerTest.java @@ -13,10 +13,11 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static response.ResponseOuterClass.RequestErrorType.UNRECOGNIZED; -import static response.ResponseOuterClass.RequestErrorType.Unspecified; -import glide.api.models.configuration.Route; -import glide.api.models.configuration.Route.RouteType; +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.api.models.exceptions.ConnectionException; import glide.api.models.exceptions.ExecAbortException; @@ -24,8 +25,6 @@ import glide.api.models.exceptions.TimeoutException; 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 org.junit.jupiter.api.BeforeEach; @@ -34,8 +33,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; import response.ResponseOuterClass.RequestError; import response.ResponseOuterClass.Response; @@ -72,9 +69,7 @@ public void submitNewCommand_returnObjectResult() // exercise CompletableFuture result = service.submitNewCommand( - command, - Optional.empty(), - new BaseCommandResponseResolver((ptr) -> ptr == pointer ? respObject : null)); + command, new BaseCommandResponseResolver((ptr) -> ptr == pointer ? respObject : null)); Object respPointer = result.get(); // verify @@ -92,9 +87,7 @@ public void submitNewCommand_returnNullResult() throws ExecutionException, Inter // exercise CompletableFuture result = service.submitNewCommand( - command, - Optional.empty(), - new BaseCommandResponseResolver((p) -> new RuntimeException(""))); + command, new BaseCommandResponseResolver((p) -> new RuntimeException(""))); Object respPointer = result.get(); // verify @@ -118,9 +111,7 @@ public void submitNewCommand_returnStringResult() // exercise CompletableFuture result = service.submitNewCommand( - command, - Optional.empty(), - new BaseCommandResponseResolver((p) -> p == pointer ? testString : null)); + command, new BaseCommandResponseResolver((p) -> p == pointer ? testString : null)); Object respPointer = result.get(); // verify @@ -147,9 +138,7 @@ public void submitNewCommand_throwClosingException() { () -> { CompletableFuture result = service.submitNewCommand( - command, - Optional.empty(), - new BaseCommandResponseResolver((ptr) -> new Object())); + command, new BaseCommandResponseResolver((ptr) -> new Object())); result.get(); }); @@ -183,8 +172,7 @@ public void BaseCommandResponseResolver_handles_all_errors( ExecutionException.class, () -> { CompletableFuture result = - service.submitNewCommand( - command, Optional.empty(), new BaseCommandResponseResolver((ptr) -> null)); + service.submitNewCommand(command, new BaseCommandResponseResolver((ptr) -> null)); result.get(); }); @@ -210,23 +198,17 @@ public void BaseCommandResponseResolver_handles_all_errors( } @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"ALL_NODES", "ALL_PRIMARIES", "RANDOM"}) - public void prepare_request_with_simple_routes(RouteType routeType) { + @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 protocSimpleRouteToClientSimpleRoute = - Map.of( - SimpleRoutes.Random, RouteType.RANDOM, - SimpleRoutes.AllNodes, RouteType.ALL_NODES, - SimpleRoutes.AllPrimaries, RouteType.ALL_PRIMARIES); + var command = + Command.builder().requestType(Command.RequestType.CUSTOM_COMMAND).route(routeType).build(); ArgumentCaptor captor = ArgumentCaptor.forClass(RedisRequest.Builder.class); - service.submitNewCommand(command, Optional.of(new Route.Builder(routeType).build()), r -> null); + service.submitNewCommand(command, r -> null); verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); @@ -235,31 +217,26 @@ public void prepare_request_with_simple_routes(RouteType routeType) { () -> assertTrue(requestBuilder.getRoute().hasSimpleRoutes()), () -> assertEquals( - routeType, - protocSimpleRouteToClientSimpleRoute.get( - requestBuilder.getRoute().getSimpleRoutes())), + routeType.getProtobufMapping(), requestBuilder.getRoute().getSimpleRoutes()), () -> assertFalse(requestBuilder.getRoute().hasSlotIdRoute()), () -> assertFalse(requestBuilder.getRoute().hasSlotKeyRoute())); } @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_ID", "REPLICA_SLOT_ID"}) - public void prepare_request_with_slot_id_routes(RouteType routeType) { + @EnumSource(value = SlotType.class) + public void prepare_request_with_slot_id_routes(SlotType slotType) { CompletableFuture future = new CompletableFuture<>(); when(channelHandler.write(any(), anyBoolean())).thenReturn(future); - - var protocSlotTypeToClientSlotIdRoute = - Map.of( - SlotTypes.Primary, RouteType.PRIMARY_SLOT_ID, - SlotTypes.Replica, RouteType.REPLICA_SLOT_ID); + 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, Optional.of(new Route.Builder(routeType).setSlotId(42).build()), r -> null); + service.submitNewCommand(command, r -> null); verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); @@ -268,32 +245,27 @@ public void prepare_request_with_slot_id_routes(RouteType routeType) { () -> assertTrue(requestBuilder.getRoute().hasSlotIdRoute()), () -> assertEquals( - routeType, - protocSlotTypeToClientSlotIdRoute.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())); } @ParameterizedTest - @EnumSource( - value = RouteType.class, - names = {"PRIMARY_SLOT_KEY", "REPLICA_SLOT_KEY"}) - public void prepare_request_with_slot_key_routes(RouteType routeType) { + @EnumSource(value = SlotType.class) + public void prepare_request_with_slot_key_routes(SlotType slotType) { CompletableFuture future = new CompletableFuture<>(); when(channelHandler.write(any(), anyBoolean())).thenReturn(future); - - var protocSlotTypeToClientSlotKeyRoute = - Map.of( - SlotTypes.Primary, RouteType.PRIMARY_SLOT_KEY, - SlotTypes.Replica, RouteType.REPLICA_SLOT_KEY); + 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, Optional.of(new Route.Builder(routeType).setSlotKey("TEST").build()), r -> null); + service.submitNewCommand(command, r -> null); verify(channelHandler).write(captor.capture(), anyBoolean()); var requestBuilder = captor.getValue(); @@ -302,11 +274,25 @@ public void prepare_request_with_slot_key_routes(RouteType routeType) { () -> assertTrue(requestBuilder.getRoute().hasSlotKeyRoute()), () -> assertEquals( - routeType, - protocSlotTypeToClientSlotKeyRoute.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())); } + + @Test + 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)); + assertEquals("Unknown type of route", exception.getMessage()); + } }