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.

Author: christophstrobl

src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java

@@ -1,12 +1,12 @@
 /*
  * Copyright 2011-2016 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.
@@ -47,7 +47,7 @@
 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;
 import org.springframework.data.redis.core.KeyBoundCursor;
@@ -788,8 +788,8 @@ public Boolean expire(byte[] key, long seconds) {
 
 		/*
 		 *  @see DATAREDIS-286 to avoid overflow in Jedis
-		 *  
-		 *  TODO Remove this workaround when we upgrade to a Jedis version that contains a 
+		 *
+		 *  TODO Remove this workaround when we upgrade to a Jedis version that contains a
 		 *  fix for: https://github.com/xetorthio/jedis/pull/575
 		 */
 		if (seconds > Integer.MAX_VALUE) {
@@ -3181,13 +3181,21 @@ public <T> T evalSha(byte[] scriptSha1, ReturnType returnType, int numKeys, byte
 	@Override
 	public Long time() {
 
-		List<String> 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);
+		}
 	}
 
 	/*

src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java

@@ -58,7 +58,7 @@
 
 /**
  * Jedis type converters.
- * 
+ *
  * @author Jennifer Hickey
  * @author Christoph Strobl
  * @author Thomas Darimont
@@ -79,6 +79,7 @@ abstract public class JedisConverters extends Converters {
 	private static final Converter<Object, RedisClusterNode> OBJECT_TO_CLUSTER_NODE_CONVERTER;
 	private static final Converter<Expiration, byte[]> EXPIRATION_TO_COMMAND_OPTION_CONVERTER;
 	private static final Converter<SetOption, byte[]> SET_OPTION_TO_COMMAND_OPTION_CONVERTER;
+	private static final Converter<List<String>, Long> STRING_LIST_TO_TIME_CONVERTER;
 
 	public static final byte[] PLUS_BYTES;
 	public static final byte[] MINUS_BYTES;
@@ -165,6 +166,19 @@ public byte[] convert(SetOption source) {
 			}
 
 		};
+
+		STRING_LIST_TO_TIME_CONVERTER = new Converter<List<String>, Long>() {
+
+			@Override
+			public Long convert(List<String> 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));
+			}
+		};
 	}
 
 	public static Converter<String, byte[]> stringToBytes() {
@@ -173,7 +187,7 @@ public static Converter<String, byte[]> stringToBytes() {
 
 	/**
 	 * {@link ListConverter} converting jedis {@link redis.clients.jedis.Tuple} to {@link Tuple}.
-	 * 
+	 *
 	 * @return
 	 * @since 1.4
 	 */
@@ -343,7 +357,7 @@ public static BitOP toBitOp(BitOperation bitOp) {
 	/**
 	 * Converts a given {@link Boundary} to its binary representation suitable for {@literal ZRANGEBY*} commands, despite
 	 * {@literal ZRANGEBYLEX}.
-	 * 
+	 *
 	 * @param boundary
 	 * @param defaultValue
 	 * @return
@@ -360,7 +374,7 @@ public static byte[] boundaryToBytesForZRange(Boundary boundary, byte[] defaultV
 
 	/**
 	 * Converts a given {@link Boundary} to its binary representation suitable for ZRANGEBYLEX command.
-	 * 
+	 *
 	 * @param boundary
 	 * @return
 	 * @since 1.6
@@ -382,7 +396,7 @@ public static byte[] boundaryToBytesForZRangeByLex(Boundary boundary, byte[] def
 	 * <dt>{@link TimeUnit#MILLISECONDS}</dt>
 	 * <dd>{@code PX}</dd>
 	 * </dl>
-	 * 
+	 *
 	 * @param expiration
 	 * @return
 	 * @since 1.7
@@ -401,7 +415,7 @@ public static byte[] toSetCommandExPxArgument(Expiration expiration) {
 	 * <dt>{@link SetOption#SET_IF_PRESENT}</dt>
 	 * <dd>{@code XX}</dd>
 	 * </dl>
-	 * 
+	 *
 	 * @param option
 	 * @return
 	 * @since 1.7
@@ -442,9 +456,9 @@ private static byte[] boundaryToBytes(Boundary boundary, byte[] inclPrefix, byte
 	 * @return
 	 */
 	public static ScanParams toScanParams(ScanOptions options) {
-		
+
 		ScanParams sp = new ScanParams();
-		
+
 		if (!options.equals(ScanOptions.NONE)) {
 			if (options.getCount() != null) {
 				sp.count(options.getCount().intValue());
@@ -456,4 +470,8 @@ public static ScanParams toScanParams(ScanOptions options) {
 		return sp;
 	}
 
+	static Converter<List<String>, Long> toTimeConverter() {
+		return STRING_LIST_TO_TIME_CONVERTER;
+	}
+
 }

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java

@@ -54,7 +54,6 @@
 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.TransactionResultConverter;
 import org.springframework.data.redis.core.Cursor;
 import org.springframework.data.redis.core.KeyBoundCursor;
@@ -111,7 +110,7 @@
 /**
  * {@code RedisConnection} implementation on top of <a href="https://github.com/mp911de/lettuce">Lettuce</a> Redis
  * client.
- * 
+ *
  * @author Costin Leau
  * @author Jennifer Hickey
  * @author Christoph Strobl
@@ -254,7 +253,7 @@ public T convert(Object source) {
 
 	/**
 	 * Instantiates a new lettuce connection.
-	 * 
+	 *
 	 * @param timeout The connection timeout (in milliseconds)
 	 * @param client The {@link RedisClient} to use when instantiating a native connection
 	 */
@@ -264,7 +263,7 @@ public LettuceConnection(long timeout, RedisClient client) {
 
 	/**
 	 * Instantiates a new lettuce connection.
-	 * 
+	 *
 	 * @param timeout The connection timeout (in milliseconds) * @param client The {@link RedisClient} to use when
 	 *          instantiating a pub/sub connection
 	 * @param pool The connection pool to use for all other native connections
@@ -275,7 +274,7 @@ public LettuceConnection(long timeout, RedisClient client, LettucePool pool) {
 
 	/**
 	 * Instantiates a new lettuce connection.
-	 * 
+	 *
 	 * @param sharedConnection A native connection that is shared with other {@link LettuceConnection}s. Will not be used
 	 *          for transactions or blocking operations
 	 * @param timeout The connection timeout (in milliseconds)
@@ -288,7 +287,7 @@ public LettuceConnection(com.lambdaworks.redis.RedisAsyncConnection<byte[], byte
 
 	/**
 	 * Instantiates a new lettuce connection.
-	 * 
+	 *
 	 * @param sharedConnection A native connection that is shared with other {@link LettuceConnection}s. Should not be
 	 *          used for transactions or blocking operations
 	 * @param timeout The connection timeout (in milliseconds)
@@ -347,7 +346,7 @@ public Object execute(String command, byte[]... args) {
 
 	/**
 	 * 'Native' or 'raw' execution of the given command along-side the given arguments.
-	 * 
+	 *
 	 * @see RedisCommands#execute(String, byte[]...)
 	 * @param command Command to execute
 	 * @param commandOutputTypeHint Type of Output to use, may be (may be {@literal null}).
@@ -3085,16 +3084,19 @@ public void subscribe(MessageListener listener, byte[]... channels) {
 	 */
 	@Override
 	public Long time() {
-		try {
-
-			List<byte[]> 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());
+		try {
 
-			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);
 		}
@@ -3406,7 +3408,7 @@ private <T> T failsafeReadScanValues(List<?> source, @SuppressWarnings("rawtypes
 	/**
 	 * Specifies if pipelined and transaction results should be converted to the expected data type. If false, results of
 	 * {@link #closePipeline()} and {@link #exec()} will be of the type returned by the Lettuce driver
-	 * 
+	 *
 	 * @param convertPipelineAndTxResults Whether or not to convert pipeline and tx results
 	 */
 	public void setConvertPipelineAndTxResults(boolean convertPipelineAndTxResults) {
@@ -3643,7 +3645,7 @@ protected RedisSentinelConnection getSentinelConnection(RedisNode sentinel) {
 
 	/**
 	 * {@link TypeHints} provide {@link CommandOutput} information for a given {@link CommandType}.
-	 * 
+	 *
 	 * @since 1.2.1
 	 */
 	static class TypeHints {
@@ -3804,7 +3806,7 @@ static class TypeHints {
 
 		/**
 		 * Returns the {@link CommandOutput} mapped for given {@link CommandType} or {@link ByteArrayOutput} as default.
-		 * 
+		 *
 		 * @param type
 		 * @return {@link ByteArrayOutput} as default when no matching {@link CommandOutput} available.
 		 */
@@ -3815,7 +3817,7 @@ public CommandOutput getTypeHint(CommandType type) {
 
 		/**
 		 * Returns the {@link CommandOutput} mapped for given {@link CommandType} given {@link CommandOutput} as default.
-		 * 
+		 *
 		 * @param type
 		 * @return
 		 */

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java

@@ -67,7 +67,7 @@
 
 /**
  * Lettuce type converters
- * 
+ *
  * @author Jennifer Hickey
  * @author Christoph Strobl
  * @author Thomas Darimont
@@ -91,6 +91,7 @@ abstract public class LettuceConverters extends Converters {
 	private static final Converter<String[], List<RedisClientInfo>> STRING_TO_LIST_OF_CLIENT_INFO = new StringToRedisClientInfoConverter();
 	private static final Converter<Partitions, List<RedisClusterNode>> PARTITIONS_TO_CLUSTER_NODES;
 	private static Converter<com.lambdaworks.redis.cluster.models.partitions.RedisClusterNode, RedisClusterNode> CLUSTER_NODE_TO_CLUSTER_NODE_CONVERTER;
+	private static final Converter<List<byte[]>, Long> BYTES_LIST_TO_TIME_CONVERTER;
 
 	public static final byte[] PLUS_BYTES;
 	public static final byte[] MINUS_BYTES;
@@ -284,6 +285,19 @@ private Set<Flag> parseFlags(Set<NodeFlag> source) {
 		MINUS_BYTES = toBytes("-");
 		POSITIVE_INFINITY_BYTES = toBytes("+inf");
 		NEGATIVE_INFINITY_BYTES = toBytes("-inf");
+
+		BYTES_LIST_TO_TIME_CONVERTER = new Converter<List<byte[]>, Long>() {
+
+			@Override
+			public Long convert(List<byte[]> 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)));
+			}
+		};
 	}
 
 	public static List<Tuple> toTuple(List<byte[]> list) {
@@ -620,7 +634,7 @@ public static RedisClusterNode toRedisClusterNode(
 
 	/**
 	 * Converts a given {@link Expiration} and {@link SetOption} to the according {@link SetArgs}.<br />
-	 * 
+	 *
 	 * @param expiration can be {@literal null}.
 	 * @param option can be {@literal null}.
 	 * @since 1.7
@@ -657,4 +671,7 @@ public static SetArgs toSetArgs(Expiration expiration, SetOption option) {
 		return args;
 	}
 
+	static Converter<List<byte[]>, Long> toTimeConverter() {
+		return BYTES_LIST_TO_TIME_CONVERTER;
+	}
 }

src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java

@@ -15,8 +15,7 @@
  */
 package org.springframework.data.redis.connection;
 
-import static org.hamcrest.CoreMatchers.*;
-import static org.hamcrest.number.IsCloseTo.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.junit.Assume.*;
 import static org.springframework.data.redis.SpinBarrier.*;
@@ -76,7 +75,7 @@
 
 /**
  * Base test class for AbstractConnection integration tests
- * 
+ *
  * @author Costin Leau
  * @author Jennifer Hickey
  * @author Christoph Strobl
@@ -1922,13 +1921,17 @@ public void testLastSave() {
 
 	/**
 	 * @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<Object> results = getResults();
+		assertThat(results, is(not(empty())));
+		assertThat(results.get(0), notNullValue());
+		assertThat((Long) results.get(0) > 0, equalTo(true));
 	}
 
 	/**