diff --git a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java index f2724bc92..689ce22c8 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java @@ -3047,7 +3047,7 @@ public class DefaultStringRedisConnection implements StringRedisConnection, Deco */ @Override public Object execute(String command) { - return execute(command, (byte[][]) null); + return execute(command, EMPTY_2D_BYTE_ARRAY); } /* diff --git a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java index fe210456e..7b06aebd2 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java @@ -15,7 +15,6 @@ */ package org.springframework.data.redis.connection; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Properties; @@ -142,16 +141,22 @@ public interface DefaultedRedisClusterConnection extends RedisClusterConnection, */ @Nullable @Override + @SuppressWarnings("unchecked") default T execute(String command, byte[] key, Collection args) { Assert.notNull(command, "Command must not be null!"); Assert.notNull(key, "Key must not be null!"); Assert.notNull(args, "Args must not be null!"); - ArrayList allArgs = new ArrayList(); - allArgs.add(key); - allArgs.addAll(args); + byte[][] commandArgs = new byte[args.size() + 1][]; - return (T) execute(command, allArgs.toArray(new byte[allArgs.size()][])); + commandArgs[0] = key; + int targetIndex = 1; + + for (byte[] binaryArgument : args) { + commandArgs[targetIndex++] = binaryArgument; + } + + return (T) execute(command, commandArgs); } } diff --git a/src/main/java/org/springframework/data/redis/connection/ReactiveHashCommands.java b/src/main/java/org/springframework/data/redis/connection/ReactiveHashCommands.java index bf9a3ab61..a737afd0b 100644 --- a/src/main/java/org/springframework/data/redis/connection/ReactiveHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/ReactiveHashCommands.java @@ -623,17 +623,16 @@ public interface ReactiveHashCommands { } /** - * @return {@literal null} if not already set. + * @return the field. */ - @Nullable public ByteBuffer getField() { return field; } } /** - * Get the length of the value associated with {@code hashKey}. If either the {@code key} or the {@code hashKey} do - * not exist, {@code 0} is emitted. + * Get the length of the value associated with {@code field}. If either the {@code key} or the {@code field} do not + * exist, {@code 0} is emitted. * * @param key must not be {@literal null}. * @param field must not be {@literal null}. @@ -649,8 +648,8 @@ public interface ReactiveHashCommands { } /** - * Get the length of the value associated with {@code hashKey}. If either the {@code key} or the {@code hashKey} do - * not exist, {@code 0} is emitted. + * Get the length of the value associated with {@code field}. If either the {@code key} or the {@code field} do not + * exist, {@code 0} is emitted. * * @param commands must not be {@literal null}. * @return never {@literal null}. diff --git a/src/main/java/org/springframework/data/redis/connection/RedisClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/RedisClusterConnection.java index 8bb79ef60..b690ac475 100644 --- a/src/main/java/org/springframework/data/redis/connection/RedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/RedisClusterConnection.java @@ -65,7 +65,7 @@ public interface RedisClusterConnection extends RedisConnection, RedisClusterCom * 
 	 * 
 	 * // SET foo bar EX 10 NX
-	 * execute("SET", "foo".getBytes(), asBinaryList("bar", "EX", 10, "NX")
+	 * execute("SET", "foo".getBytes(), asBinaryList("bar", "EX", 10, "NX"))
 	 * 
 	 *
* diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java index eb79a2607..3fa9537b9 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java @@ -29,26 +29,31 @@ import redis.clients.util.SafeEncoder; import java.lang.reflect.Field; import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; +import java.util.Arrays; +import java.util.Set; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.springframework.beans.DirectFieldAccessor; -import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; /** + * Utility class to dispatch arbitrary Redis commands using Jedis commands. + * * @author Christoph Strobl + * @author Mark Paluch * @since 2.1 */ +@SuppressWarnings({ "unchecked", "ConstantConditions" }) class JedisClientUtils { private static final Field CLIENT_FIELD; private static final Method SEND_COMMAND; private static final Method GET_RESPONSE; private static final Method PROTOCOL_SEND_COMMAND; + private static final Set KNOWN_COMMANDS; + private static final Builder OBJECT_BUILDER; static { @@ -60,12 +65,12 @@ class JedisClientUtils { ReflectionUtils.makeAccessible(PROTOCOL_SEND_COMMAND); try { + Class commandType = ClassUtils.isPresent("redis.clients.jedis.ProtocolCommand", null) ? ClassUtils.forName("redis.clients.jedis.ProtocolCommand", null) : ClassUtils.forName("redis.clients.jedis.Protocol$Command", null); - SEND_COMMAND = ReflectionUtils.findMethod(Connection.class, "sendCommand", - new Class[] { commandType, byte[][].class }); + SEND_COMMAND = ReflectionUtils.findMethod(Connection.class, "sendCommand", commandType, byte[][].class); } catch (Exception e) { throw new NoClassDefFoundError( "Could not find required flavor of command required by 'redis.clients.jedis.Connection#sendCommand'."); @@ -75,38 +80,56 @@ class JedisClientUtils { GET_RESPONSE = ReflectionUtils.findMethod(Queable.class, "getResponse", Builder.class); ReflectionUtils.makeAccessible(GET_RESPONSE); + + KNOWN_COMMANDS = Arrays.stream(Command.values()).map(Enum::name).collect(Collectors.toSet()); + + OBJECT_BUILDER = new Builder() { + public Object build(Object data) { + return data; + } + + public String toString() { + return "Object"; + } + }; } - @Nullable - static T execute(String command, Collection keys, Collection args, Supplier jedis) { + /** + * Execute an arbitrary on the supplied {@link Jedis} instance. + * + * @param command the command. + * @param keys must not be {@literal null}, may be empty. + * @param args must not be {@literal null}, may be empty. + * @param jedis must not be {@literal null}. + * @return the response, can be be {@literal null}. + */ + static T execute(String command, byte[][] keys, byte[][] args, Supplier jedis) { - List mArgs = new ArrayList<>(keys); - mArgs.addAll(args); + byte[][] commandArgs = getCommandArguments(keys, args); - Client client = retrieveClient(jedis.get()); - sendCommand(client, command, mArgs.toArray(new byte[mArgs.size()][])); + Client client = sendCommand(command, commandArgs, jedis.get()); return (T) client.getOne(); } - static Client retrieveClient(Jedis jedis) { - return (Client) ReflectionUtils.getField(CLIENT_FIELD, jedis); - } - - static Client sendCommand(Jedis jedis, String command, byte[][] args) { + /** + * Send a Redis command and retrieve the {@link Client} for response retrieval. + * + * @param command the command. + * @param args must not be {@literal null}, may be empty. + * @param jedis must not be {@literal null}. + * @return the {@link Client} instance used to send the command. + */ + static Client sendCommand(String command, byte[][] args, Jedis jedis) { Client client = retrieveClient(jedis); - if (isKnownCommand(command)) { - ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args); - } else { - sendProtocolCommand(client, command, args); - } + sendCommand(client, command, args); return client; } - static void sendCommand(Client client, String command, byte[][] args) { + private static void sendCommand(Client client, String command, byte[][] args) { if (isKnownCommand(command)) { ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args); @@ -115,8 +138,9 @@ class JedisClientUtils { } } - static void sendProtocolCommand(Client client, String command, byte[][] args) { + private static void sendProtocolCommand(Client client, String command, byte[][] args) { + // quite expensive to construct for each command invocation DirectFieldAccessor dfa = new DirectFieldAccessor(client); client.connect(); @@ -125,34 +149,52 @@ class JedisClientUtils { ReflectionUtils.invokeMethod(PROTOCOL_SEND_COMMAND, null, os, SafeEncoder.encode(command), args); Integer pipelinedCommands = (Integer) dfa.getPropertyValue("pipelinedCommands"); - dfa.setPropertyValue("pipelinedCommands", pipelinedCommands.intValue() + 1); + dfa.setPropertyValue("pipelinedCommands", pipelinedCommands + 1); } - static boolean isKnownCommand(String command) { + private static boolean isKnownCommand(String command) { + return KNOWN_COMMANDS.contains(command); + } - try { - Command.valueOf(command); - return true; - } catch (IllegalArgumentException e) { - return false; + private static byte[][] getCommandArguments(byte[][] keys, byte[][] args) { + + if (keys.length == 0) { + return args; } + + if (args.length == 0) { + return keys; + } + + byte[][] commandArgs = new byte[keys.length + args.length][]; + + System.arraycopy(keys, 0, commandArgs, 0, keys.length); + System.arraycopy(args, 0, commandArgs, keys.length, args.length); + + return commandArgs; } + /** + * @param jedis the client instance. + * @return {@literal true} if the connection has entered {@literal MULTI} state. + */ static boolean isInMulti(Jedis jedis) { return retrieveClient(jedis).isInMulti(); } - static Response getGetResponse(Object target) { - - return (Response) ReflectionUtils.invokeMethod(GET_RESPONSE, target, new Builder() { - public Object build(Object data) { - return data; - } - - public String toString() { - return "Object"; - } - }); + /** + * Retrieve the {@link Response} object from a {@link redis.clients.jedis.Transaction} or a + * {@link redis.clients.jedis.Pipeline} for response synchronization. + * + * @param target a {@link redis.clients.jedis.Transaction} or {@link redis.clients.jedis.Pipeline}, must not be + * {@literal null}. + * @return the {@link Response} wrapper object. + */ + static Response getResponse(Object target) { + return (Response) ReflectionUtils.invokeMethod(GET_RESPONSE, target, OBJECT_BUILDER); } + private static Client retrieveClient(Jedis jedis) { + return (Client) ReflectionUtils.getField(CLIENT_FIELD, jedis); + } } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java index 3167a8ad9..40ac08398 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java @@ -23,10 +23,7 @@ import redis.clients.jedis.JedisCluster; import redis.clients.jedis.JedisClusterConnectionHandler; import redis.clients.jedis.JedisPool; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -70,6 +67,8 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection { private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy( JedisConverters.exceptionConverter()); + private static final byte[][] EMPTY_2D_BYTE_ARRAY = new byte[0][]; + private final Log log = LogFactory.getLog(getClass()); private final JedisCluster cluster; @@ -130,14 +129,16 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection { * (non-Javadoc) * @see org.springframework.data.redis.connection.RedisCommands#execute(java.lang.String, byte[][]) */ + @Nullable @Override public Object execute(String command, byte[]... args) { Assert.notNull(command, "Command must not be null!"); + Assert.notNull(args, "Args must not be null!"); return clusterCommandExecutor .executeCommandOnArbitraryNode((JedisClusterCommandCallback) client -> JedisClientUtils.execute(command, - Collections.emptyList(), Arrays.asList(args), () -> client)) + EMPTY_2D_BYTE_ARRAY, args, () -> client)) .getValue(); } @@ -153,14 +154,26 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection { Assert.notNull(key, "Key must not be null!"); Assert.notNull(args, "Args must not be null!"); - Collection commandArgs = new ArrayList<>(); - commandArgs.add(key); - commandArgs.addAll(args); + byte[][] commandArgs = getCommandArguments(key, args); RedisClusterNode keyMaster = topologyProvider.getTopology().getKeyServingMasterNode(key); return clusterCommandExecutor.executeCommandOnSingleNode((JedisClusterCommandCallback) client -> JedisClientUtils - .execute(command, Collections.emptyList(), commandArgs, () -> client), keyMaster).getValue(); + .execute(command, EMPTY_2D_BYTE_ARRAY, commandArgs, () -> client), keyMaster).getValue(); + } + + private static byte[][] getCommandArguments(byte[] key, Collection args) { + + byte[][] commandArgs = new byte[args.size() + 1][]; + + commandArgs[0] = key; + int targetIndex = 1; + + for (byte[] binaryArgument : args) { + commandArgs[targetIndex++] = binaryArgument; + } + + return commandArgs; } /* @@ -816,7 +829,8 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection { PropertyAccessor accessor = new DirectFieldAccessFallbackBeanWrapper(cluster); this.connectionHandler = accessor.isReadableProperty("connectionHandler") - ? (JedisClusterConnectionHandler) accessor.getPropertyValue("connectionHandler") : null; + ? (JedisClusterConnectionHandler) accessor.getPropertyValue("connectionHandler") + : null; } else { this.connectionHandler = null; } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index 834f231ae..475d89af3 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java @@ -42,7 +42,6 @@ import org.springframework.data.redis.connection.convert.TransactionResultConver import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -273,20 +272,18 @@ public class JedisConnection extends AbstractRedisConnection { */ @Override public Object execute(String command, byte[]... args) { - Assert.hasText(command, "a valid command needs to be specified"); - try { - List mArgs = new ArrayList<>(); - if (!ObjectUtils.isEmpty(args)) { - Collections.addAll(mArgs, args); - } - Client client = JedisClientUtils.sendCommand(this.jedis, command, mArgs.toArray(new byte[mArgs.size()][])); + Assert.hasText(command, "A valid command needs to be specified!"); + Assert.notNull(args, "Arguments must not be null!"); + + try { + + Client client = JedisClientUtils.sendCommand(command, args, this.jedis); if (isQueueing() || isPipelined()) { - Object target = (isPipelined() ? pipeline : transaction); - @SuppressWarnings("unchecked") - Response result = JedisClientUtils.getGetResponse(target); + Response result = JedisClientUtils + .getResponse(isPipelined() ? getRequiredPipeline() : getRequiredTransaction()); if (isPipelined()) { pipeline(new JedisResult(result)); } else { diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommands.java index 99827eafb..326b638a0 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommands.java @@ -224,8 +224,8 @@ class LettuceReactiveHashCommands implements ReactiveHashCommands { return connection.execute(cmd -> Flux.from(commands).flatMap(command -> { - Assert.notNull(command.getKey(), "Command.getKey() must not be null!"); - Assert.notNull(command.getField(), "Command.getField() must not be null!"); + Assert.notNull(command.getKey(), "Key must not be null!"); + Assert.notNull(command.getField(), "Field must not be null!"); return cmd.hstrlen(command.getKey(), command.getField()).map(value -> new NumericResponse<>(command, value)); })); diff --git a/src/test/java/org/springframework/data/redis/connection/DefaultStringRedisConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/DefaultStringRedisConnectionTests.java index cd3a7c277..8b86c0d98 100644 --- a/src/test/java/org/springframework/data/redis/connection/DefaultStringRedisConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/DefaultStringRedisConnectionTests.java @@ -18,16 +18,7 @@ package org.springframework.data.redis.connection; import static org.junit.Assert.*; import static org.mockito.Mockito.*; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.Set; +import java.util.*; import java.util.concurrent.TimeUnit; import org.junit.Before; @@ -1702,7 +1693,7 @@ public class DefaultStringRedisConnectionTests { @Test public void testExecute() { - doReturn("foo").when(nativeConnection).execute("something", (byte[][]) null); + doReturn("foo").when(nativeConnection).execute("something", new byte[0][]); actual.add(connection.execute("something")); verifyResults(Arrays.asList(new Object[] { "foo" })); }