From 5ff68b2103685497b1183c8032f3fcff2c966250 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 | 28 ++++++++++++------- .../connection/jedis/JedisConverters.java | 18 ++++++++++++ .../connection/lettuce/LettuceConnection.java | 17 ++++++----- .../connection/lettuce/LettuceConverters.java | 17 +++++++++++ .../AbstractConnectionIntegrationTests.java | 12 +++++--- 5 files changed, 71 insertions(+), 21 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 48ea05327..f66201d4c 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 @@ -1,5 +1,5 @@ /* - * Copyright 2011-2015 the original author or authors. + * 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. @@ -786,8 +786,8 @@ public class JedisConnection extends AbstractRedisConnection { /* * @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) { @@ -973,8 +973,8 @@ public class JedisConnection extends AbstractRedisConnection { /* * @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 (millis > Integer.MAX_VALUE) { @@ -3090,13 +3090,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 9c0404fee..8f3447f5e 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 @@ -67,6 +67,7 @@ abstract public class JedisConverters extends Converters { private static final Converter> STRING_TO_CLIENT_INFO_CONVERTER = new StringToRedisClientInfoConverter(); private static final Converter TUPLE_CONVERTER; private static final ListConverter TUPLE_LIST_TO_TUPLE_LIST_CONVERTER; + private static final Converter, Long> STRING_LIST_TO_TIME_CONVERTER; public static final byte[] PLUS_BYTES; public static final byte[] MINUS_BYTES; @@ -96,6 +97,19 @@ abstract public class JedisConverters extends Converters { MINUS_BYTES = toBytes("-"); POSITIVE_INFINITY_BYTES = toBytes("+inf"); NEGATIVE_INFINITY_BYTES = toBytes("-inf"); + + 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)); + } + }; } public static Converter stringToBytes() { @@ -320,4 +334,8 @@ abstract public class JedisConverters extends Converters { return buffer.array(); } + + static Converter, Long> toTimeConverter() { + return STRING_LIST_TO_TIME_CONVERTER; + } } 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 a00497b3d..222b78d4c 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 @@ -3060,16 +3060,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 4649bd41d..10c9e48d8 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 @@ -78,6 +78,7 @@ abstract public class LettuceConverters extends Converters { private static final Converter, Map> BYTES_LIST_TO_MAP; private static final Converter, List> BYTES_LIST_TO_TUPLE_LIST_CONVERTER; private static final Converter> STRING_TO_LIST_OF_CLIENT_INFO = new StringToRedisClientInfoConverter(); + private static final Converter, Long> BYTES_LIST_TO_TIME_CONVERTER; public static final byte[] PLUS_BYTES; public static final byte[] MINUS_BYTES; @@ -203,6 +204,19 @@ abstract public class LettuceConverters extends Converters { MINUS_BYTES = toBytes("-"); 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))); + } + }; } public static List toTuple(List list) { @@ -523,4 +537,7 @@ abstract public class LettuceConverters extends Converters { return toString(buffer.array()); } + static Converter, Long> toTimeConverter() { + return BYTES_LIST_TO_TIME_CONVERTER; + } } 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 c8b15a46c..347da7279 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.redis.connection; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.junit.Assume.*; import static org.springframework.data.redis.SpinBarrier.*; @@ -1919,13 +1919,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)); } /**