From 7af8fdf946404d29dd5b9957e4258fcfb152dce3 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 19 May 2016 09:00:26 +0200 Subject: [PATCH] DATAREDIS-513 - Fix RedisServerCommands.time() failure when in pipeline mode. We fixed a glitch in Jedis/Lettuce RedisConneciton TIME command when used in pipeline or transaction mode. Original pull request: #199. --- .../connection/jedis/JedisConnection.java | 19 +++++++++++++------ .../connection/jedis/JedisConverters.java | 18 ++++++++++++++++++ .../connection/lettuce/LettuceConnection.java | 18 ++++++++++-------- .../connection/lettuce/LettuceConverters.java | 18 ++++++++++++++++++ .../AbstractConnectionIntegrationTests.java | 11 ++++++++--- 5 files changed, 67 insertions(+), 17 deletions(-) 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 7af4719ad..10331e35b 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 @@ -53,7 +53,6 @@ import org.springframework.data.redis.connection.RedisZSetCommands; import org.springframework.data.redis.connection.ReturnType; import org.springframework.data.redis.connection.SortParameters; import org.springframework.data.redis.connection.Subscription; -import org.springframework.data.redis.connection.convert.Converters; import org.springframework.data.redis.connection.convert.ListConverter; import org.springframework.data.redis.connection.convert.TransactionResultConverter; import org.springframework.data.redis.core.Cursor; @@ -3579,13 +3578,21 @@ public class JedisConnection extends AbstractRedisConnection { @Override public Long time() { - List serverTimeInformation = this.jedis.time(); + try { - Assert.notEmpty(serverTimeInformation, "Received invalid result from server. Expected 2 items in collection."); - Assert.isTrue(serverTimeInformation.size() == 2, - "Received invalid nr of arguments from redis server. Expected 2 received " + serverTimeInformation.size()); + if (isPipelined()) { + pipeline(new JedisResult(pipeline.time(), JedisConverters.toTimeConverter())); + return null; + } - return Converters.toTimeMillis(serverTimeInformation.get(0), serverTimeInformation.get(1)); + if (isQueueing()) { + transaction(new JedisResult(transaction.time(), JedisConverters.toTimeConverter())); + return null; + } + return JedisConverters.toTimeConverter().convert(jedis.time()); + } catch (Exception ex) { + throw convertJedisAccessException(ex); + } } /* diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java index 2fad1e34f..73e009f4c 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java @@ -94,6 +94,7 @@ abstract public class JedisConverters extends Converters { private static final Converter OBJECT_TO_CLUSTER_NODE_CONVERTER; private static final Converter EXPIRATION_TO_COMMAND_OPTION_CONVERTER; private static final Converter SET_OPTION_TO_COMMAND_OPTION_CONVERTER; + private static final Converter, Long> STRING_LIST_TO_TIME_CONVERTER; private static final Converter GEO_COORDINATE_TO_POINT_CONVERTER; private static final ListConverter LIST_GEO_COORDINATE_TO_POINT_CONVERTER; private static final Converter BYTES_TO_STRING_CONVERTER; @@ -194,6 +195,19 @@ abstract public class JedisConverters extends Converters { }; + STRING_LIST_TO_TIME_CONVERTER = new Converter, Long>() { + + @Override + public Long convert(List source) { + + Assert.notEmpty(source, "Received invalid result from server. Expected 2 items in collection."); + Assert.isTrue(source.size() == 2, + "Received invalid nr of arguments from redis server. Expected 2 received " + source.size()); + + return toTimeMillis(source.get(0), source.get(1)); + } + }; + GEO_COORDINATE_TO_POINT_CONVERTER = new Converter() { @Override public Point convert(redis.clients.jedis.GeoCoordinate geoCoordinate) { @@ -493,6 +507,10 @@ abstract public class JedisConverters extends Converters { return sp; } + static Converter, Long> toTimeConverter() { + return STRING_LIST_TO_TIME_CONVERTER; + } + /** * @param source * @return diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java index af5892705..e3fc5cab5 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java @@ -60,7 +60,6 @@ import org.springframework.data.redis.connection.RedisSubscribedConnectionExcept import org.springframework.data.redis.connection.ReturnType; import org.springframework.data.redis.connection.SortParameters; import org.springframework.data.redis.connection.Subscription; -import org.springframework.data.redis.connection.convert.Converters; import org.springframework.data.redis.connection.convert.ListConverter; import org.springframework.data.redis.connection.convert.TransactionResultConverter; import org.springframework.data.redis.core.Cursor; @@ -3464,16 +3463,19 @@ public class LettuceConnection extends AbstractRedisConnection { */ @Override public Long time() { + try { - List result = getConnection().time(); - - Assert.notEmpty(result, "Received invalid result from server. Expected 2 items in collection."); - Assert.isTrue(result.size() == 2, - "Received invalid nr of arguments from redis server. Expected 2 received " + result.size()); - - return Converters.toTimeMillis(new String(result.get(0)), new String(result.get(1))); + if (isPipelined()) { + pipeline(new LettuceResult(getAsyncConnection().time(), LettuceConverters.toTimeConverter())); + return null; + } + if (isQueueing()) { + transaction(new LettuceTxResult(getConnection().time(), LettuceConverters.toTimeConverter())); + return null; + } + return LettuceConverters.toTimeConverter().convert(getConnection().time()); } catch (Exception ex) { throw convertLettuceAccessException(ex); } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java index 9de44e8ee..09e073951 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java @@ -106,6 +106,7 @@ abstract public class LettuceConverters extends Converters { private static final Converter> STRING_TO_LIST_OF_CLIENT_INFO = new StringToRedisClientInfoConverter(); private static final Converter> PARTITIONS_TO_CLUSTER_NODES; private static Converter CLUSTER_NODE_TO_CLUSTER_NODE_CONVERTER; + private static final Converter, Long> BYTES_LIST_TO_TIME_CONVERTER; private static final Converter GEO_COORDINATE_TO_POINT_CONVERTER; private static final ListConverter GEO_COORDINATE_LIST_TO_POINT_LIST_CONVERTER; @@ -302,6 +303,19 @@ abstract public class LettuceConverters extends Converters { POSITIVE_INFINITY_BYTES = toBytes("+inf"); NEGATIVE_INFINITY_BYTES = toBytes("-inf"); + BYTES_LIST_TO_TIME_CONVERTER = new Converter, Long>() { + + @Override + public Long convert(List source) { + + Assert.notEmpty(source, "Received invalid result from server. Expected 2 items in collection."); + Assert.isTrue(source.size() == 2, + "Received invalid nr of arguments from redis server. Expected 2 received " + source.size()); + + return toTimeMillis(LettuceConverters.toString(source.get(0)), LettuceConverters.toString(source.get(1))); + } + }; + GEO_COORDINATE_TO_POINT_CONVERTER = new Converter() { @Override public Point convert(com.lambdaworks.redis.GeoCoordinates geoCoordinate) { @@ -683,6 +697,10 @@ abstract public class LettuceConverters extends Converters { return args; } + static Converter, Long> toTimeConverter() { + return BYTES_LIST_TO_TIME_CONVERTER; + } + /** * Convert {@link Metric} into {@link GeoArgs.Unit}. * 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 9eeb9759b..394277735 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.connection; import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.collection.IsCollectionWithSize.*; +import static org.hamcrest.collection.IsEmptyCollection.*; import static org.hamcrest.core.Is.is; import static org.hamcrest.number.IsCloseTo.*; import static org.junit.Assert.*; @@ -1919,13 +1920,17 @@ public abstract class AbstractConnectionIntegrationTests { /** * @see DATAREDIS-206 + * @see DATAREDIS-513 */ @Test public void testGetTimeShouldRequestServerTime() { - Long time = connectionFactory.getConnection().time(); - assertThat(time, notNullValue()); - assertThat(time > 0, equalTo(true)); + actual.add(connection.time()); + + List results = getResults(); + assertThat(results, is(not(empty()))); + assertThat(results.get(0), notNullValue()); + assertThat((Long) results.get(0) > 0, equalTo(true)); } /**