Skip to content

Commit 7af8fdf

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
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.
1 parent 2b51966 commit 7af8fdf

File tree

5 files changed

+67
-17
lines changed

5 files changed

+67
-17
lines changed

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
import org.springframework.data.redis.connection.ReturnType;
5454
import org.springframework.data.redis.connection.SortParameters;
5555
import org.springframework.data.redis.connection.Subscription;
56-
import org.springframework.data.redis.connection.convert.Converters;
5756
import org.springframework.data.redis.connection.convert.ListConverter;
5857
import org.springframework.data.redis.connection.convert.TransactionResultConverter;
5958
import org.springframework.data.redis.core.Cursor;
@@ -3579,13 +3578,21 @@ public <T> T evalSha(byte[] scriptSha1, ReturnType returnType, int numKeys, byte
35793578
@Override
35803579
public Long time() {
35813580

3582-
List<String> serverTimeInformation = this.jedis.time();
3581+
try {
35833582

3584-
Assert.notEmpty(serverTimeInformation, "Received invalid result from server. Expected 2 items in collection.");
3585-
Assert.isTrue(serverTimeInformation.size() == 2,
3586-
"Received invalid nr of arguments from redis server. Expected 2 received " + serverTimeInformation.size());
3583+
if (isPipelined()) {
3584+
pipeline(new JedisResult(pipeline.time(), JedisConverters.toTimeConverter()));
3585+
return null;
3586+
}
35873587

3588-
return Converters.toTimeMillis(serverTimeInformation.get(0), serverTimeInformation.get(1));
3588+
if (isQueueing()) {
3589+
transaction(new JedisResult(transaction.time(), JedisConverters.toTimeConverter()));
3590+
return null;
3591+
}
3592+
return JedisConverters.toTimeConverter().convert(jedis.time());
3593+
} catch (Exception ex) {
3594+
throw convertJedisAccessException(ex);
3595+
}
35893596
}
35903597

35913598
/*

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ abstract public class JedisConverters extends Converters {
9494
private static final Converter<Object, RedisClusterNode> OBJECT_TO_CLUSTER_NODE_CONVERTER;
9595
private static final Converter<Expiration, byte[]> EXPIRATION_TO_COMMAND_OPTION_CONVERTER;
9696
private static final Converter<SetOption, byte[]> SET_OPTION_TO_COMMAND_OPTION_CONVERTER;
97+
private static final Converter<List<String>, Long> STRING_LIST_TO_TIME_CONVERTER;
9798
private static final Converter<redis.clients.jedis.GeoCoordinate, Point> GEO_COORDINATE_TO_POINT_CONVERTER;
9899
private static final ListConverter<redis.clients.jedis.GeoCoordinate, Point> LIST_GEO_COORDINATE_TO_POINT_CONVERTER;
99100
private static final Converter<byte[], String> BYTES_TO_STRING_CONVERTER;
@@ -194,6 +195,19 @@ public byte[] convert(SetOption source) {
194195

195196
};
196197

198+
STRING_LIST_TO_TIME_CONVERTER = new Converter<List<String>, Long>() {
199+
200+
@Override
201+
public Long convert(List<String> source) {
202+
203+
Assert.notEmpty(source, "Received invalid result from server. Expected 2 items in collection.");
204+
Assert.isTrue(source.size() == 2,
205+
"Received invalid nr of arguments from redis server. Expected 2 received " + source.size());
206+
207+
return toTimeMillis(source.get(0), source.get(1));
208+
}
209+
};
210+
197211
GEO_COORDINATE_TO_POINT_CONVERTER = new Converter<redis.clients.jedis.GeoCoordinate, Point>() {
198212
@Override
199213
public Point convert(redis.clients.jedis.GeoCoordinate geoCoordinate) {
@@ -493,6 +507,10 @@ public static ScanParams toScanParams(ScanOptions options) {
493507
return sp;
494508
}
495509

510+
static Converter<List<String>, Long> toTimeConverter() {
511+
return STRING_LIST_TO_TIME_CONVERTER;
512+
}
513+
496514
/**
497515
* @param source
498516
* @return

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import org.springframework.data.redis.connection.ReturnType;
6161
import org.springframework.data.redis.connection.SortParameters;
6262
import org.springframework.data.redis.connection.Subscription;
63-
import org.springframework.data.redis.connection.convert.Converters;
6463
import org.springframework.data.redis.connection.convert.ListConverter;
6564
import org.springframework.data.redis.connection.convert.TransactionResultConverter;
6665
import org.springframework.data.redis.core.Cursor;
@@ -3464,16 +3463,19 @@ public Long geoRemove(byte[] key, byte[]... values) {
34643463
*/
34653464
@Override
34663465
public Long time() {
3467-
try {
3468-
3469-
List<byte[]> result = getConnection().time();
34703466

3471-
Assert.notEmpty(result, "Received invalid result from server. Expected 2 items in collection.");
3472-
Assert.isTrue(result.size() == 2,
3473-
"Received invalid nr of arguments from redis server. Expected 2 received " + result.size());
3467+
try {
34743468

3475-
return Converters.toTimeMillis(new String(result.get(0)), new String(result.get(1)));
3469+
if (isPipelined()) {
3470+
pipeline(new LettuceResult(getAsyncConnection().time(), LettuceConverters.toTimeConverter()));
3471+
return null;
3472+
}
3473+
if (isQueueing()) {
3474+
transaction(new LettuceTxResult(getConnection().time(), LettuceConverters.toTimeConverter()));
3475+
return null;
3476+
}
34763477

3478+
return LettuceConverters.toTimeConverter().convert(getConnection().time());
34773479
} catch (Exception ex) {
34783480
throw convertLettuceAccessException(ex);
34793481
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ abstract public class LettuceConverters extends Converters {
106106
private static final Converter<String[], List<RedisClientInfo>> STRING_TO_LIST_OF_CLIENT_INFO = new StringToRedisClientInfoConverter();
107107
private static final Converter<Partitions, List<RedisClusterNode>> PARTITIONS_TO_CLUSTER_NODES;
108108
private static Converter<com.lambdaworks.redis.cluster.models.partitions.RedisClusterNode, RedisClusterNode> CLUSTER_NODE_TO_CLUSTER_NODE_CONVERTER;
109+
private static final Converter<List<byte[]>, Long> BYTES_LIST_TO_TIME_CONVERTER;
109110
private static final Converter<GeoCoordinates, Point> GEO_COORDINATE_TO_POINT_CONVERTER;
110111
private static final ListConverter<GeoCoordinates, Point> GEO_COORDINATE_LIST_TO_POINT_LIST_CONVERTER;
111112

@@ -302,6 +303,19 @@ private Set<Flag> parseFlags(Set<NodeFlag> source) {
302303
POSITIVE_INFINITY_BYTES = toBytes("+inf");
303304
NEGATIVE_INFINITY_BYTES = toBytes("-inf");
304305

306+
BYTES_LIST_TO_TIME_CONVERTER = new Converter<List<byte[]>, Long>() {
307+
308+
@Override
309+
public Long convert(List<byte[]> source) {
310+
311+
Assert.notEmpty(source, "Received invalid result from server. Expected 2 items in collection.");
312+
Assert.isTrue(source.size() == 2,
313+
"Received invalid nr of arguments from redis server. Expected 2 received " + source.size());
314+
315+
return toTimeMillis(LettuceConverters.toString(source.get(0)), LettuceConverters.toString(source.get(1)));
316+
}
317+
};
318+
305319
GEO_COORDINATE_TO_POINT_CONVERTER = new Converter<com.lambdaworks.redis.GeoCoordinates, Point>() {
306320
@Override
307321
public Point convert(com.lambdaworks.redis.GeoCoordinates geoCoordinate) {
@@ -683,6 +697,10 @@ public static SetArgs toSetArgs(Expiration expiration, SetOption option) {
683697
return args;
684698
}
685699

700+
static Converter<List<byte[]>, Long> toTimeConverter() {
701+
return BYTES_LIST_TO_TIME_CONVERTER;
702+
}
703+
686704
/**
687705
* Convert {@link Metric} into {@link GeoArgs.Unit}.
688706
*

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import static org.hamcrest.CoreMatchers.*;
1919
import static org.hamcrest.collection.IsCollectionWithSize.*;
20+
import static org.hamcrest.collection.IsEmptyCollection.*;
2021
import static org.hamcrest.core.Is.is;
2122
import static org.hamcrest.number.IsCloseTo.*;
2223
import static org.junit.Assert.*;
@@ -1919,13 +1920,17 @@ public void testLastSave() {
19191920

19201921
/**
19211922
* @see DATAREDIS-206
1923+
* @see DATAREDIS-513
19221924
*/
19231925
@Test
19241926
public void testGetTimeShouldRequestServerTime() {
19251927

1926-
Long time = connectionFactory.getConnection().time();
1927-
assertThat(time, notNullValue());
1928-
assertThat(time > 0, equalTo(true));
1928+
actual.add(connection.time());
1929+
1930+
List<Object> results = getResults();
1931+
assertThat(results, is(not(empty())));
1932+
assertThat(results.get(0), notNullValue());
1933+
assertThat((Long) results.get(0) > 0, equalTo(true));
19291934
}
19301935

19311936
/**

0 commit comments

Comments
 (0)