From 8f7f15f8f575ea2019f09e3a052bb2cabec2c9c9 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 3 Oct 2017 09:59:06 +0200 Subject: [PATCH 1/3] DATAREDIS-698 - Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 617c18c957..d15db14de6 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 2.1.0.BUILD-SNAPSHOT + 2.1.0.DATAREDIS-698-SNAPSHOT Spring Data Redis From 5dcb1613d7fad170b6bb1b0ee50af66b838937e5 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 3 Oct 2017 18:30:25 +0200 Subject: [PATCH 2/3] DATAREDIS-698 - Add support for HSTRLEN. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now support HSTRLEN command throughout RedisHashCommand for Lettuce and Jedis in both an imperative and reactive manner. However, Jedis not natively supporting HSTRLEN via its API we’ve come up with some more reflective invocation allowing to execute commands currently not known by Jedis. We also added this behavior to the cluster implementation which as of now also supports RedisClusterConnection#execute. --- .../DefaultStringRedisConnection.java | 23 ++- .../DefaultedRedisClusterConnection.java | 23 +++ .../connection/DefaultedRedisConnection.java | 7 + .../connection/ReactiveHashCommands.java | 79 +++++++++ .../connection/RedisClusterConnection.java | 22 +++ .../redis/connection/RedisHashCommands.java | 12 ++ .../connection/StringRedisConnection.java | 12 ++ .../connection/jedis/JedisClientUtils.java | 158 ++++++++++++++++++ .../jedis/JedisClusterConnection.java | 36 +++- .../jedis/JedisClusterHashCommands.java | 13 ++ .../connection/jedis/JedisConnection.java | 74 +------- .../connection/jedis/JedisHashCommands.java | 15 ++ .../lettuce/LettuceHashCommands.java | 26 +++ .../lettuce/LettuceReactiveHashCommands.java | 16 ++ .../data/redis/core/BoundHashOperations.java | 11 ++ .../core/DefaultBoundHashOperations.java | 11 ++ .../redis/core/DefaultHashOperations.java | 14 ++ .../data/redis/core/HashOperations.java | 12 ++ .../AbstractConnectionIntegrationTests.java | 30 ++++ .../connection/ClusterConnectionTests.java | 9 + .../jedis/JedisClusterConnectionTests.java | 48 +++++- .../LettuceClusterConnectionTests.java | 21 +++ .../LettuceReactiveHashCommandsTests.java | 26 +++ .../core/DefaultHashOperationsTests.java | 19 +++ 24 files changed, 642 insertions(+), 75 deletions(-) create mode 100644 src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java 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 aede7d0b95..f2724bc92a 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java @@ -3201,6 +3201,16 @@ public Cursor> hScan(byte[] key, ScanOptions options) { return this.delegate.hScan(key, options); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisSetCommands#hStrLen(byte[], byte[]) + */ + @Nullable + @Override + public Long hStrLen(byte[] key, byte[] field) { + return convertAndReturn(delegate.hStrLen(key, field), identityConverter); + } + /* * (non-Javadoc) * @see org.springframework.data.redis.connection.RedisServerCommands#setClientName(java.lang.String) @@ -3263,6 +3273,15 @@ public String setValue(String value) { }); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.StringRedisConnection#hStrLen(java.lang.String, java.lang.String) + */ + @Override + public Long hStrLen(String key, String field) { + return hStrLen(serialize(key), serialize(field)); + } + /* * (non-Javadoc) * @see org.springframework.data.redis.connection.StringRedisConnection#sScan(java.lang.String, org.springframework.data.redis.core.ScanOptions) @@ -3472,8 +3491,8 @@ private T convertAndReturn(@Nullable Object value, Converter converter) { return null; } - - return value == null ? null : ObjectUtils.nullSafeEquals(converter, identityConverter) ? (T) value : (T) converter.convert(value); + return value == null ? null + : ObjectUtils.nullSafeEquals(converter, identityConverter) ? (T) value : (T) converter.convert(value); } private void addResultConverter(Converter converter) { 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 9fbefc4282..fe210456e2 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java @@ -15,10 +15,14 @@ */ package org.springframework.data.redis.connection; +import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Properties; import org.springframework.data.redis.core.types.RedisClientInfo; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * @author Christoph Strobl @@ -131,4 +135,23 @@ default Long time(RedisClusterNode node) { default List getClientList(RedisClusterNode node) { return serverCommands().getClientList(node); } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisClusterConnection#execute(String, byte[], Collection) + */ + @Nullable + @Override + 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); + + return (T) execute(command, allArgs.toArray(new byte[allArgs.size()][])); + } } diff --git a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java index 62591279bf..5ee509042c 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java @@ -916,6 +916,13 @@ default Cursor> hScan(byte[] key, ScanOptions options) { return hashCommands().hScan(key, options); } + /** @deprecated in favor of {@link RedisConnection#hashCommands()}. */ + @Override + @Deprecated + default Long hStrLen(byte[] key, byte[] field) { + return hashCommands().hStrLen(key, field); + } + // GEO COMMANDS /** @deprecated in favor of {@link RedisConnection#geoCommands()}}. */ 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 03979020d4..bf9a3ab61f 100644 --- a/src/main/java/org/springframework/data/redis/connection/ReactiveHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/ReactiveHashCommands.java @@ -578,4 +578,83 @@ default Flux> hGetAll(ByteBuffer key) { * @see Redis Documentation: HGETALL */ Flux>>> hGetAll(Publisher commands); + + /** + * @author Christoph Strobl + * @see Redis Documentation: HSTRLEN + * @since 2.1 + */ + class HStrLenCommand extends KeyCommand { + + private ByteBuffer field; + + /** + * Creates a new {@link HStrLenCommand} given a {@code key}. + * + * @param key can be {@literal null}. + * @param field must not be {@literal null}. + */ + private HStrLenCommand(@Nullable ByteBuffer key, ByteBuffer field) { + + super(key); + this.field = field; + } + + /** + * Specify the {@code field} within the hash to get the length of the {@code value} of.ø + * + * @param field must not be {@literal null}. + * @return new instance of {@link HStrLenCommand}. + */ + public static HStrLenCommand lengthOf(ByteBuffer field) { + + Assert.notNull(field, "Field must not be null!"); + return new HStrLenCommand(null, field); + } + + /** + * Define the {@code key} the hash is stored at. + * + * @param key must not be {@literal null}. + * @return new instance of {@link HStrLenCommand}. + */ + public HStrLenCommand from(ByteBuffer key) { + return new HStrLenCommand(key, field); + } + + /** + * @return {@literal null} if not already set. + */ + @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. + * + * @param key must not be {@literal null}. + * @param field must not be {@literal null}. + * @return never {@literal null}. + * @since 2.1 + */ + default Mono hStrLen(ByteBuffer key, ByteBuffer field) { + + Assert.notNull(key, "Key must not be null!"); + Assert.notNull(field, "Field must not be null!"); + + return hStrLen(Mono.just(HStrLenCommand.lengthOf(field).from(key))).next().map(NumericResponse::getOutput); + } + + /** + * 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. + * + * @param commands must not be {@literal null}. + * @return never {@literal null}. + * @since 2.1 + */ + Flux> hStrLen(Publisher commands); } 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 7af3f274e8..8bb79ef607 100644 --- a/src/main/java/org/springframework/data/redis/connection/RedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/RedisClusterConnection.java @@ -15,6 +15,7 @@ */ package org.springframework.data.redis.connection; +import java.util.Collection; import java.util.Set; import org.springframework.lang.Nullable; @@ -56,6 +57,27 @@ public interface RedisClusterConnection extends RedisConnection, RedisClusterCom @Nullable byte[] randomKey(RedisClusterNode node); + /** + * Execute the given command for the {@code key} provided potentially appending args.
+ * This method, other than {@link #execute(String, byte[]...)}, dispatches the command to the {@code key} serving + * master node. + * + * 
+	 * 
+	 * // SET foo bar EX 10 NX
+	 * execute("SET", "foo".getBytes(), asBinaryList("bar", "EX", 10, "NX")
+	 * 
+	 *
+ * + * @param command must not be {@literal null}. + * @param key must not be {@literal null}. + * @param args must not be {@literal null}. + * @return command result as delivered by the underlying Redis driver. Can be {@literal null}. + * @since 2.1 + */ + @Nullable + T execute(String command, byte[] key, Collection args); + /** * Get {@link RedisClusterServerCommands}. * diff --git a/src/main/java/org/springframework/data/redis/connection/RedisHashCommands.java b/src/main/java/org/springframework/data/redis/connection/RedisHashCommands.java index d4bb421b6f..7da765a3d1 100644 --- a/src/main/java/org/springframework/data/redis/connection/RedisHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/RedisHashCommands.java @@ -183,4 +183,16 @@ public interface RedisHashCommands { * @see Redis Documentation: HSCAN */ Cursor> hScan(byte[] key, ScanOptions options); + + /** + * Returns the length of the value associated with {@code field} in the hash stored at {@code key}. If the key or the + * field do not exist, {@code 0} is returned. + * + * @param key must not be {@literal null}. + * @param field must not be {@literal null}. + * @return {@literal null} when used in pipeline / transaction. + * @since 2.1 + */ + @Nullable + Long hStrLen(byte[] key, byte[] field); } diff --git a/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java b/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java index 86e24b8c15..1b8ad7eacb 100644 --- a/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/StringRedisConnection.java @@ -1509,6 +1509,18 @@ interface StringTuple extends Tuple { */ Cursor> hScan(String key, ScanOptions options); + /** + * Returns the length of the value associated with {@code field} in the hash stored at {@code key}. If the key or the + * field do not exist, {@code 0} is returned. + * + * @param key must not be {@literal null}. + * @param field must not be {@literal null}. + * @return {@literal null} when used in pipeline / transaction. + * @since 2.1 + */ + @Nullable + Long hStrLen(String key, String field); + // ------------------------------------------------------------------------- // Methods dealing with HyperLogLog // ------------------------------------------------------------------------- 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 new file mode 100644 index 0000000000..eb79a26079 --- /dev/null +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java @@ -0,0 +1,158 @@ +/* + * Copyright 2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.connection.jedis; + +import redis.clients.jedis.BinaryJedis; +import redis.clients.jedis.Builder; +import redis.clients.jedis.Client; +import redis.clients.jedis.Connection; +import redis.clients.jedis.Jedis; +import redis.clients.jedis.Protocol; +import redis.clients.jedis.Protocol.Command; +import redis.clients.jedis.Queable; +import redis.clients.jedis.Response; +import redis.clients.util.RedisOutputStream; +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.function.Supplier; + +import org.springframework.beans.DirectFieldAccessor; +import org.springframework.lang.Nullable; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; + +/** + * @author Christoph Strobl + * @since 2.1 + */ +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; + + static { + + CLIENT_FIELD = ReflectionUtils.findField(BinaryJedis.class, "client", Client.class); + ReflectionUtils.makeAccessible(CLIENT_FIELD); + + PROTOCOL_SEND_COMMAND = ReflectionUtils.findMethod(Protocol.class, "sendCommand", RedisOutputStream.class, + byte[].class, byte[][].class); + 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 }); + } catch (Exception e) { + throw new NoClassDefFoundError( + "Could not find required flavor of command required by 'redis.clients.jedis.Connection#sendCommand'."); + } + + ReflectionUtils.makeAccessible(SEND_COMMAND); + + GET_RESPONSE = ReflectionUtils.findMethod(Queable.class, "getResponse", Builder.class); + ReflectionUtils.makeAccessible(GET_RESPONSE); + } + + @Nullable + static T execute(String command, Collection keys, Collection args, Supplier jedis) { + + List mArgs = new ArrayList<>(keys); + mArgs.addAll(args); + + Client client = retrieveClient(jedis.get()); + sendCommand(client, command, mArgs.toArray(new byte[mArgs.size()][])); + + 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) { + + Client client = retrieveClient(jedis); + + if (isKnownCommand(command)) { + ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args); + } else { + sendProtocolCommand(client, command, args); + } + + return client; + } + + static void sendCommand(Client client, String command, byte[][] args) { + + if (isKnownCommand(command)) { + ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args); + } else { + sendProtocolCommand(client, command, args); + } + } + + static void sendProtocolCommand(Client client, String command, byte[][] args) { + + DirectFieldAccessor dfa = new DirectFieldAccessor(client); + + client.connect(); + + RedisOutputStream os = (RedisOutputStream) dfa.getPropertyValue("outputStream"); + ReflectionUtils.invokeMethod(PROTOCOL_SEND_COMMAND, null, os, SafeEncoder.encode(command), args); + + Integer pipelinedCommands = (Integer) dfa.getPropertyValue("pipelinedCommands"); + dfa.setPropertyValue("pipelinedCommands", pipelinedCommands.intValue() + 1); + } + + static boolean isKnownCommand(String command) { + + try { + Command.valueOf(command); + return true; + } catch (IllegalArgumentException e) { + return false; + } + } + + 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"; + } + }); + } + +} 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 217596b49f..3167a8ad99 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,7 +23,10 @@ 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; @@ -130,8 +133,34 @@ public JedisClusterConnection(JedisCluster cluster, ClusterCommandExecutor execu @Override public Object execute(String command, byte[]... args) { - // TODO: execute command on all nodes? or throw exception requiring to execute command on a specific node - throw new UnsupportedOperationException("Execute is currently not supported in cluster mode."); + Assert.notNull(command, "Command must not be null!"); + + return clusterCommandExecutor + .executeCommandOnArbitraryNode((JedisClusterCommandCallback) client -> JedisClientUtils.execute(command, + Collections.emptyList(), Arrays.asList(args), () -> client)) + .getValue(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisClusterConnection#execute(String, byte[], java.util.Collection) + */ + @Nullable + @Override + public 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!"); + + Collection commandArgs = new ArrayList<>(); + commandArgs.add(key); + commandArgs.addAll(args); + + RedisClusterNode keyMaster = topologyProvider.getTopology().getKeyServingMasterNode(key); + + return clusterCommandExecutor.executeCommandOnSingleNode((JedisClusterCommandCallback) client -> JedisClientUtils + .execute(command, Collections.emptyList(), commandArgs, () -> client), keyMaster).getValue(); } /* @@ -787,8 +816,7 @@ static class JedisClusterNodeResourceProvider implements ClusterNodeResourceProv 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/JedisClusterHashCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterHashCommands.java index 7cad0b3cd4..7ba089a47e 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterHashCommands.java @@ -18,6 +18,7 @@ import redis.clients.jedis.ScanParams; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -29,6 +30,7 @@ import org.springframework.data.redis.core.ScanCursor; import org.springframework.data.redis.core.ScanIteration; import org.springframework.data.redis.core.ScanOptions; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** @@ -286,7 +288,18 @@ protected ScanIteration> doScan(long cursorId, ScanOptions }.open(); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisHashCommands#hStrLen(byte[], byte[]) + */ + @Nullable + @Override + public Long hStrLen(byte[] key, byte[] field) { + return Long.class.cast(connection.execute("HSTRLEN", key, Collections.singleton(field))); + } + private DataAccessException convertJedisAccessException(Exception ex) { + return connection.convertJedisAccessException(ex); } } 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 1ee1e844a9..834f231ae2 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 @@ -15,22 +15,15 @@ */ package org.springframework.data.redis.connection.jedis; -import redis.clients.jedis.BinaryJedis; import redis.clients.jedis.BinaryJedisPubSub; -import redis.clients.jedis.Builder; import redis.clients.jedis.Client; -import redis.clients.jedis.Connection; import redis.clients.jedis.Jedis; import redis.clients.jedis.Pipeline; -import redis.clients.jedis.Protocol.Command; -import redis.clients.jedis.Queable; import redis.clients.jedis.Response; import redis.clients.jedis.Transaction; import redis.clients.jedis.exceptions.JedisDataException; import redis.clients.util.Pool; -import java.lang.reflect.Field; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedList; @@ -48,10 +41,8 @@ import org.springframework.data.redis.connection.convert.TransactionResultConverter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; -import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -70,38 +61,10 @@ */ public class JedisConnection extends AbstractRedisConnection { - private static final Field CLIENT_FIELD; - private static final Method SEND_COMMAND; - private static final Method GET_RESPONSE; - private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy( JedisConverters.exceptionConverter()); - static { - - CLIENT_FIELD = ReflectionUtils.findField(BinaryJedis.class, "client", Client.class); - ReflectionUtils.makeAccessible(CLIENT_FIELD); - - 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 }); - } catch (Exception e) { - throw new NoClassDefFoundError( - "Could not find required flavor of command required by 'redis.clients.jedis.Connection#sendCommand'."); - } - - ReflectionUtils.makeAccessible(SEND_COMMAND); - - GET_RESPONSE = ReflectionUtils.findMethod(Queable.class, "getResponse", Builder.class); - ReflectionUtils.makeAccessible(GET_RESPONSE); - } - private final Jedis jedis; - private final Client client; private @Nullable Transaction transaction; private final @Nullable Pool pool; /** @@ -117,6 +80,7 @@ public class JedisConnection extends AbstractRedisConnection { private Queue>> txResults = new LinkedList<>(); class JedisResult extends FutureResult> { + public JedisResult(Response resultHolder, Converter converter) { super(resultHolder, converter); } @@ -180,9 +144,6 @@ public JedisConnection(Jedis jedis, Pool pool, int dbIndex) { */ protected JedisConnection(Jedis jedis, @Nullable Pool pool, int dbIndex, String clientName) { - // extract underlying connection for batch operations - client = (Client) ReflectionUtils.getField(CLIENT_FIELD, jedis); - this.jedis = jedis; this.pool = pool; this.dbIndex = dbIndex; @@ -319,21 +280,13 @@ public Object execute(String command, byte[]... args) { Collections.addAll(mArgs, args); } - ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), - mArgs.toArray(new byte[mArgs.size()][])); + Client client = JedisClientUtils.sendCommand(this.jedis, command, mArgs.toArray(new byte[mArgs.size()][])); + if (isQueueing() || isPipelined()) { Object target = (isPipelined() ? pipeline : transaction); @SuppressWarnings("unchecked") - Response result = (Response) ReflectionUtils.invokeMethod(GET_RESPONSE, target, - new Builder() { - public Object build(Object data) { - return data; - } - - public String toString() { - return "Object"; - } - }); + + Response result = JedisClientUtils.getGetResponse(target); if (isPipelined()) { pipeline(new JedisResult(result)); } else { @@ -378,19 +331,6 @@ public void close() throws DataAccessException { } // else close the connection normally (doing the try/catch dance) Exception exc = null; - if (isQueueing()) { - try { - client.quit(); - } catch (Exception o_O) { - // ignore exception - } - try { - client.disconnect(); - } catch (Exception o_O) { - // ignore exception - } - return; - } try { jedis.quit(); } catch (Exception ex) { @@ -433,7 +373,7 @@ public boolean isClosed() { */ @Override public boolean isQueueing() { - return client.isInMulti(); + return JedisClientUtils.isInMulti(jedis); } /* @@ -652,7 +592,7 @@ JedisStatusResult newStatusResult(Response response) { return new JedisStatusResult(response); } - JedisStatusResult newStatusResult(Response response, Converter converter) { + JedisStatusResult newStatusResult(Response response, Converter converter) { return new JedisStatusResult(response, converter); } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisHashCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisHashCommands.java index 2250b35b1f..78ed241524 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisHashCommands.java @@ -31,6 +31,7 @@ import org.springframework.data.redis.core.KeyBoundCursor; import org.springframework.data.redis.core.ScanIteration; import org.springframework.data.redis.core.ScanOptions; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** @@ -414,6 +415,20 @@ protected void doClose() { }.open(); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisHashCommands#hStrLen(byte[], byte[]) + */ + @Nullable + @Override + public Long hStrLen(byte[] key, byte[] field) { + + Assert.notNull(key, "Key must not be null!"); + Assert.notNull(field, "Field must not be null!"); + + return Long.class.cast(connection.execute("HSTRLEN", key, field)); + } + private boolean isPipelined() { return connection.isPipelined(); } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceHashCommands.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceHashCommands.java index 771d9563c1..109af3970f 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceHashCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceHashCommands.java @@ -35,6 +35,7 @@ import org.springframework.data.redis.core.KeyBoundCursor; import org.springframework.data.redis.core.ScanIteration; import org.springframework.data.redis.core.ScanOptions; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** @@ -418,6 +419,31 @@ protected void doClose() { }.open(); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.RedisHashCommands#hStrLen(byte[], byte[]) + */ + @Nullable + @Override + public Long hStrLen(byte[] key, byte[] field) { + + Assert.notNull(key, "Key must not be null!"); + Assert.notNull(field, "Field must not be null!"); + try { + if (isPipelined()) { + pipeline(connection.newLettuceResult(getAsyncConnection().hstrlen(key, field))); + return null; + } + if (isQueueing()) { + transaction(connection.newLettuceTxResult(getConnection().hstrlen(key, field))); + return null; + } + return getConnection().hstrlen(key, field); + } catch (Exception ex) { + throw convertLettuceAccessException(ex); + } + } + private boolean isPipelined() { return connection.isPipelined(); } 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 8a23b757f9..99827eafb8 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 @@ -214,4 +214,20 @@ public Flux>> return Mono.just(new CommandResponse<>(command, result.flatMapMany(v -> Flux.fromStream(v.entrySet().stream())))); })); } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.connection.ReactiveHashCommands#hstrlen(org.reactivestreams.Publisher) + */ + @Override + public Flux> hStrLen(Publisher commands) { + + 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!"); + + return cmd.hstrlen(command.getKey(), command.getField()).map(value -> new NumericResponse<>(command, value)); + })); + } } diff --git a/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java b/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java index b4a4a90154..e9b43d64c1 100644 --- a/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java +++ b/src/main/java/org/springframework/data/redis/core/BoundHashOperations.java @@ -96,6 +96,17 @@ public interface BoundHashOperations extends BoundKeyOperations { @Nullable Set keys(); + /** + * Returns the length of the value associated with {@code hashKey}. If the {@code hashKey} do not exist, {@code 0} is + * returned. + * + * @param hashKey must not be {@literal null}. + * @return {@literal null} when used in pipeline / transaction. + * @since 2.1 + */ + @Nullable + Long lengthOfValue(HK hashKey); + /** * Get size of hash at the bound key. * diff --git a/src/main/java/org/springframework/data/redis/core/DefaultBoundHashOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultBoundHashOperations.java index 76686f95d6..7f0f8d76de 100644 --- a/src/main/java/org/springframework/data/redis/core/DefaultBoundHashOperations.java +++ b/src/main/java/org/springframework/data/redis/core/DefaultBoundHashOperations.java @@ -22,6 +22,7 @@ import java.util.Set; import org.springframework.data.redis.connection.DataType; +import org.springframework.lang.Nullable; /** * Default implementation for {@link HashOperations}. @@ -118,6 +119,16 @@ public Set keys() { return ops.keys(getKey()); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.BoundHashOperations#lengthOfValue(java.lang.Object, java.lang.Object) + */ + @Nullable + @Override + public Long lengthOfValue(HK hashKey) { + return ops.lengthOfValue(getKey(), hashKey); + } + /* * (non-Javadoc) * @see org.springframework.data.redis.core.BoundHashOperations#size() diff --git a/src/main/java/org/springframework/data/redis/core/DefaultHashOperations.java b/src/main/java/org/springframework/data/redis/core/DefaultHashOperations.java index 3dd849fe64..8aa55055e0 100644 --- a/src/main/java/org/springframework/data/redis/core/DefaultHashOperations.java +++ b/src/main/java/org/springframework/data/redis/core/DefaultHashOperations.java @@ -24,6 +24,7 @@ import java.util.Set; import org.springframework.core.convert.converter.Converter; +import org.springframework.lang.Nullable; /** * Default implementation of {@link HashOperations}. @@ -114,6 +115,19 @@ public Long size(K key) { return execute(connection -> connection.hLen(rawKey), true); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.HashOperations#lengthOfValue(java.lang.Object, java.lang.Object) + */ + @Nullable + @Override + public Long lengthOfValue(K key, HK hashKey) { + + byte[] rawKey = rawKey(key); + byte[] rawHashKey = rawHashKey(hashKey); + return execute(connection -> connection.hStrLen(rawKey, rawHashKey), true); + } + /* * (non-Javadoc) * @see org.springframework.data.redis.core.HashOperations#putAll(java.lang.Object, java.util.Map) diff --git a/src/main/java/org/springframework/data/redis/core/HashOperations.java b/src/main/java/org/springframework/data/redis/core/HashOperations.java index e278e2d47d..948db810bd 100644 --- a/src/main/java/org/springframework/data/redis/core/HashOperations.java +++ b/src/main/java/org/springframework/data/redis/core/HashOperations.java @@ -96,6 +96,18 @@ public interface HashOperations { */ Set keys(H key); + /** + * Returns the length of the value associated with {@code hashKey}. If either the {@code key} or the {@code hashKey} + * do not exist, {@code 0} is returned. + * + * @param key must not be {@literal null}. + * @param hashKey must not be {@literal null}. + * @return {@literal null} when used in pipeline / transaction. + * @since 2.1 + */ + @Nullable + Long lengthOfValue(H key, HK hashKey); + /** * Get size of hash at {@code key}. * diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java index fc1e269e03..316bf61999 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -2648,6 +2648,36 @@ public void geoRadiusByMemberShouldApplyLimit() { assertThat(((GeoResults>) results.get(1)).getContent(), hasSize(2)); } + @Test // DATAREDIS-698 + public void hStrLenReturnsFieldLength() { + + actual.add(connection.hSet("hash-hstrlen", "key-1", "value-1")); + actual.add(connection.hSet("hash-hstrlen", "key-2", "value-2")); + actual.add(connection.hStrLen("hash-hstrlen", "key-2")); + + verifyResults( + Arrays.asList(new Object[] { Boolean.TRUE, Boolean.TRUE, Long.valueOf("value-2".length()) })); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenFieldDoesNotExist() { + + actual.add(connection.hSet("hash-hstrlen", "key-1", "value-1")); + actual.add(connection.hStrLen("hash-hstrlen", "key-2")); + + verifyResults( + Arrays.asList(new Object[] { Boolean.TRUE, 0L })); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenKeyDoesNotExist() { + + actual.add(connection.hStrLen("hash-no-exist", "key-2")); + + verifyResults( + Arrays.asList(new Object[] { 0L })); + } + protected void verifyResults(List expected) { assertEquals(expected, getResults()); } diff --git a/src/test/java/org/springframework/data/redis/connection/ClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/ClusterConnectionTests.java index 1bf5590159..6be7493b85 100644 --- a/src/test/java/org/springframework/data/redis/connection/ClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/ClusterConnectionTests.java @@ -633,4 +633,13 @@ public interface ClusterConnectionTests { // DATAREDIS-438 void geoRemoveDeletesMembers(); + + // DATAREDIS-698 + void hStrLenReturnsFieldLength(); + + // DATAREDIS-698 + void hStrLenReturnsZeroWhenFieldDoesNotExist(); + + // DATAREDIS-698 + void hStrLenReturnsZeroWhenKeyDoesNotExist(); } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java index b5fc03e0d8..973af65a01 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java @@ -22,7 +22,6 @@ import static org.springframework.data.redis.connection.RedisGeoCommands.GeoRadiusCommandArgs.*; import static org.springframework.data.redis.core.ScanOptions.*; -import org.springframework.data.redis.connection.RedisClusterNode.SlotRange; import redis.clients.jedis.HostAndPort; import redis.clients.jedis.JedisCluster; import redis.clients.jedis.JedisPool; @@ -57,6 +56,7 @@ import org.springframework.data.redis.connection.DefaultSortParameters; import org.springframework.data.redis.connection.DefaultTuple; import org.springframework.data.redis.connection.RedisClusterNode; +import org.springframework.data.redis.connection.RedisClusterNode.SlotRange; import org.springframework.data.redis.connection.RedisGeoCommands.GeoLocation; import org.springframework.data.redis.connection.RedisListCommands.Position; import org.springframework.data.redis.connection.RedisNode; @@ -1729,7 +1729,8 @@ public void zRangeByLexShouldReturnResultCorrectly() { public void infoShouldCollectionInfoFromAllClusterNodes() { Properties singleNodeInfo = clusterConnection.serverCommands().info(new RedisClusterNode("127.0.0.1", 7380)); - assertThat(Double.valueOf(clusterConnection.serverCommands().info().size()), closeTo(singleNodeInfo.size() * 3, 12d)); + assertThat(Double.valueOf(clusterConnection.serverCommands().info().size()), + closeTo(singleNodeInfo.size() * 3, 12d)); } @Test // DATAREDIS-315 @@ -2135,4 +2136,47 @@ public void geoRemoveDeletesMembers() { assertThat(clusterConnection.geoRemove(KEY_1_BYTES, ARIGENTO.getName()), is(1L)); } + + @Test(expected = IllegalArgumentException.class) // DATAREDIS-689 + public void executeWithNoKeyAndArgsThrowsException() { + clusterConnection.execute("KEYS", null, Collections.singletonList("*".getBytes())); + } + + @Test // DATAREDIS-689 + public void executeWithArgs() { + + assertThat(clusterConnection.execute("SET", KEY_1_BYTES, VALUE_1_BYTES), is("OK".getBytes())); + + assertThat(nativeConnection.get(KEY_1), is(VALUE_1)); + } + + @Test // DATAREDIS-689 + public void executeWithKeyAndArgs() { + + assertThat(clusterConnection.execute("SET", KEY_1_BYTES, Collections.singletonList(VALUE_1_BYTES)), + is("OK".getBytes())); + + assertThat(nativeConnection.get(KEY_1), is(VALUE_1)); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsFieldLength() { + + nativeConnection.hset(KEY_1, KEY_2, VALUE_3); + + assertThat(clusterConnection.hashCommands().hStrLen(KEY_1_BYTES, KEY_2_BYTES), is(Long.valueOf(VALUE_3.length()))); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenFieldDoesNotExist() { + + nativeConnection.hset(KEY_1, KEY_2, VALUE_3); + + assertThat(clusterConnection.hashCommands().hStrLen(KEY_1_BYTES, KEY_3_BYTES), is(0L)); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenKeyDoesNotExist() { + assertThat(clusterConnection.hashCommands().hStrLen(KEY_1_BYTES, KEY_1_BYTES), is(0L)); + } } diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java index 6b4f09bc68..589d70f62e 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java @@ -2145,4 +2145,25 @@ public void geoRemoveDeletesMembers() { assertThat(clusterConnection.geoRemove(KEY_1_BYTES, ARIGENTO_BYTES.getName()), is(1L)); } + + @Test // DATAREDIS-698 + public void hStrLenReturnsFieldLength() { + + nativeConnection.hset(KEY_1, KEY_2, VALUE_3); + + assertThat(clusterConnection.hashCommands().hStrLen(KEY_1_BYTES, KEY_2_BYTES), is(Long.valueOf(VALUE_3.length()))); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenFieldDoesNotExist() { + + nativeConnection.hset(KEY_1, KEY_2, VALUE_3); + + assertThat(clusterConnection.hashCommands().hStrLen(KEY_1_BYTES, KEY_3_BYTES), is(0L)); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenKeyDoesNotExist() { + assertThat(clusterConnection.hashCommands().hStrLen(KEY_1_BYTES, KEY_1_BYTES), is(0L)); + } } diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommandsTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommandsTests.java index b7780c8ccc..a6dd4524c9 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommandsTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceReactiveHashCommandsTests.java @@ -205,6 +205,32 @@ public void hGetAllShouldReturnEntriesCorrectly() { assertTrue(list.containsAll(expected.entrySet())); }) // .verifyComplete(); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsFieldLength() { + + nativeCommands.hset(KEY_1, FIELD_1, VALUE_1); + nativeCommands.hset(KEY_1, FIELD_2, VALUE_2); + + StepVerifier.create(connection.hashCommands().hStrLen(KEY_1_BBUFFER, FIELD_1_BBUFFER)) + .expectNext(Long.valueOf(VALUE_1.length())) // + .verifyComplete(); + } + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenFieldDoesNotExist() { + + nativeCommands.hset(KEY_1, FIELD_2, VALUE_3); + + StepVerifier.create(connection.hashCommands().hStrLen(KEY_1_BBUFFER, FIELD_1_BBUFFER)).expectNext(0L) // + .verifyComplete(); + } + + @Test // DATAREDIS-698 + public void hStrLenReturnsZeroWhenKeyDoesNotExist() { + + StepVerifier.create(connection.hashCommands().hStrLen(KEY_1_BBUFFER, FIELD_1_BBUFFER)).expectNext(0L) // + .verifyComplete(); } } diff --git a/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsTests.java b/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsTests.java index 465082ea0b..fd60829cc6 100644 --- a/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsTests.java +++ b/src/test/java/org/springframework/data/redis/core/DefaultHashOperationsTests.java @@ -17,6 +17,7 @@ import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; +import static org.junit.Assume.*; import java.io.IOException; import java.util.Arrays; @@ -172,4 +173,22 @@ public void testHScanReadsValuesFully() throws IOException { assertThat(count, is(hashOps.size(key))); } + @Test // DATAREDIS-698 + @IfProfileValue(name = "redisVersion", value = "3.0.3+") + public void lengthOfValue() throws IOException { + + assumeThat(hashValueFactory instanceof StringObjectFactory, is(true)); + + K key = keyFactory.instance(); + HK key1 = hashKeyFactory.instance(); + HV val1 = hashValueFactory.instance(); + HK key2 = hashKeyFactory.instance(); + HV val2 = hashValueFactory.instance(); + + hashOps.put(key, key1, val1); + hashOps.put(key, key2, val2); + + assertThat(hashOps.lengthOfValue(key, key1), is(Long.valueOf(val1.toString().length()))); + } + } From be3d937798da9a8a314f79a95965fa2701082f1b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 6 Oct 2017 13:52:45 +0200 Subject: [PATCH 3/3] DATAREDIS-698 - Polishing. Eagerly initialize known commands. Preallocate Jedis response builder. Replace list concatenation with array copy. Reject null arguments in execute() Remove inversion through collections with direct byte array creation. Reorder signatures, visibility modifiers, Javadoc. --- .../DefaultStringRedisConnection.java | 13 +- .../DefaultedRedisClusterConnection.java | 15 ++- .../connection/ReactiveHashCommands.java | 11 +- .../connection/RedisClusterConnection.java | 2 +- .../connection/jedis/JedisClientUtils.java | 126 ++++++++++++------ .../jedis/JedisClusterConnection.java | 32 +++-- .../connection/jedis/JedisConnection.java | 17 +-- .../lettuce/LettuceReactiveHashCommands.java | 4 +- .../DefaultStringRedisConnectionTests.java | 13 +- 9 files changed, 136 insertions(+), 97 deletions(-) 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 f2724bc92a..e87dbd62cc 100644 --- a/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java @@ -15,17 +15,8 @@ */ package org.springframework.data.redis.connection; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Properties; -import java.util.Queue; -import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; @@ -3047,7 +3038,7 @@ public void openPipeline() { */ @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 fe210456e2..7b06aebd25 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 @@ default List getClientList(RedisClusterNode node) { */ @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 bf9a3ab61f..a737afd0b7 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 HStrLenCommand from(ByteBuffer key) { } /** - * @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 @@ default Mono hStrLen(ByteBuffer key, ByteBuffer 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 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 8bb79ef607..b690ac475a 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 eb79a26079..3fa9537b93 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 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 @@ static void sendCommand(Client client, String command, byte[][] args) { } } - 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 @@ static void sendProtocolCommand(Client client, String command, byte[][] args) { 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 3167a8ad99..40ac08398f 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.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 JedisClusterConnection(JedisCluster cluster, ClusterCommandExecutor execu * (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 T execute(String command, byte[] key, Collection args) { 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 @@ static class JedisClusterNodeResourceProvider implements ClusterNodeResourceProv 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 834f231ae2..475d89af3e 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.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 RedisHyperLogLogCommands hyperLogLogCommands() { */ @Override public Object execute(String command, byte[]... args) { - Assert.hasText(command, "a valid command needs to be specified"); + + Assert.hasText(command, "A valid command needs to be specified!"); + Assert.notNull(args, "Arguments must not be null!"); + 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()][])); + 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 99827eafb8..326b638a0c 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 @@ public Flux> hStrLen(Publisher 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 cd3a7c2772..8b86c0d98c 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 @@ 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 void testEvalSha() { @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" })); }