Spring/spring-data-redis
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAREDIS-749 - Polishing.

Browse Source 
Upgrade to Jedis 3.0. Remove reflective sendCommand calls in favor of public Jedis sendCommand API. Invocations with null arguments by introducing overloads with argument defaulting. Fix exception translation because of a changed exception hierarchy. Fix tests and adapt tests to new transactional results that return null values instead if empty collections for nested array responses.

Reformat code. Add author tags. Add note to reference docs.

Original pull request: #374.
This commit is contained in:
Mark Paluch
2018-12-06 13:07:34 +01:00
parent f4680a180c
commit f253d4128d
16 changed files with 154 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/main/java/org/springframework/data/redis/connection/jedis/JedisClientUtils.java
View File

@@ -18,7 +18,6 @@ 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;
@@ -35,8 +34,6 @@ import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;
/**
@@ -50,7 +47,6 @@ import org.springframework.util.ReflectionUtils;
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<String> KNOWN_COMMANDS;
@@ -65,20 +61,6 @@ class JedisClientUtils {
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", 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);
@@ -149,26 +131,12 @@ class JedisClientUtils {
private static void sendCommand(Client client, String command, byte[][] args) {
if (isKnownCommand(command)) {
ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args);
client.sendCommand(Command.valueOf(command.trim().toUpperCase()), args);
} else {
sendProtocolCommand(client, command, args);
client.sendCommand(() -> SafeEncoder.encode(command.trim().toUpperCase()), 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();
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 + 1);
}
private static boolean isKnownCommand(String command) {
return KNOWN_COMMANDS.contains(command);
}

13
src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterServerCommands.java
View File

@@ -335,8 +335,10 @@ class JedisClusterServerCommands implements RedisClusterServerCommands {
Assert.notNull(pattern, "Pattern must not be null!");
return connection.getClusterCommandExecutor().executeCommandOnSingleNode(
(JedisClusterCommandCallback<Properties>) client -> Converters.toProperties(client.configGet(pattern)), node)
return connection.getClusterCommandExecutor()
.executeCommandOnSingleNode(
(JedisClusterCommandCallback<Properties>) client -> Converters.toProperties(client.configGet(pattern)),
node)
.getValue();
}
@@ -502,8 +504,7 @@ class JedisClusterServerCommands implements RedisClusterServerCommands {
* @see org.springframework.data.redis.connection.RedisServerCommands#migrate(byte[], org.springframework.data.redis.connection.RedisNode, int, org.springframework.data.redis.connection.RedisServerCommands.MigrateOption, long)
*/
@Override
public void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable MigrateOption option,
long timeout) {
public void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable MigrateOption option, long timeout) {
Assert.notNull(key, "Key must not be null!");
Assert.notNull(target, "Target node must not be null!");
@@ -511,8 +512,8 @@ class JedisClusterServerCommands implements RedisClusterServerCommands {
RedisClusterNode node = connection.getTopologyProvider().getTopology().lookup(target.getHost(), target.getPort());
executeCommandOnSingleNode(client -> client.migrate(target.getHost(), target.getPort(),
key, dbIndex, timeoutToUse), node);
executeCommandOnSingleNode(client -> client.migrate(target.getHost(), target.getPort(), key, dbIndex, timeoutToUse),
node);
}
private Long convertListOfStringToTime(List<String> serverTimeInformation) {

8
src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStringCommands.java
View File

@@ -158,12 +158,11 @@ class JedisClusterStringCommands implements RedisStringCommands {
}
} else {
SetParams params = JedisConverters.toSetCommandNxXxArgument(option, null);
JedisConverters.toSetCommandExPxArgument(expiration, params);
SetParams params = JedisConverters.toSetCommandNxXxArgument(option);
JedisConverters.toSetCommandExPxArgument(expiration, params);
try {
return Converters
.stringToBoolean(connection.getCluster().set(key, value, params));
return Converters.stringToBoolean(connection.getCluster().set(key, value, params));
} catch (Exception ex) {
throw convertJedisAccessException(ex);
}
@@ -491,7 +490,6 @@ class JedisClusterStringCommands implements RedisStringCommands {
}
}
/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisStringCommands#bitOp(org.springframework.data.redis.connection.RedisStringCommands.BitOperation, byte[], byte[][])

7
src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterZSetCommands.java
View File

@@ -642,8 +642,7 @@ class JedisClusterZSetCommands implements RedisZSetCommands {
if (ClusterSlotHashUtil.isSameSlotForAllKeys(allKeys)) {
ZParams zparams = new ZParams().weights(weights.toArray())
.aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
ZParams zparams = new ZParams().weights(weights.toArray()).aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
try {
return connection.getCluster().zunionstore(destKey, zparams, sets);
@@ -680,7 +679,6 @@ class JedisClusterZSetCommands implements RedisZSetCommands {
throw new InvalidDataAccessApiUsageException("ZINTERSTORE can only be executed when all keys map to the same slot");
}
/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisZSetCommands#zInterStore(byte[], org.springframework.data.redis.connection.RedisZSetCommands.Aggregate, org.springframework.data.redis.connection.RedisZSetCommands.Weights, byte[][])
@@ -698,8 +696,7 @@ class JedisClusterZSetCommands implements RedisZSetCommands {
if (ClusterSlotHashUtil.isSameSlotForAllKeys(allKeys)) {
ZParams zparams = new ZParams().weights(weights.toArray())
.aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
ZParams zparams = new ZParams().weights(weights.toArray()).aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
try {
return connection.getCluster().zinterstore(destKey, zparams, sets);

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

@@ -38,7 +38,6 @@ import org.springframework.dao.DataAccessException;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.redis.ExceptionTranslationStrategy;
import org.springframework.data.redis.FallbackExceptionTranslationStrategy;
import org.springframework.data.redis.RedisConnectionFailureException;
import org.springframework.data.redis.RedisSystemException;
import org.springframework.data.redis.connection.*;
import org.springframework.data.redis.connection.convert.TransactionResultConverter;
@@ -62,6 +61,7 @@ import org.springframework.util.StringUtils;
* @author Milan Agatonovic
* @author Mark Paluch
* @author Ninad Divadkar
* @author Guy Korland
*/
public class JedisConnection extends AbstractRedisConnection {

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

@@ -88,6 +88,7 @@ import org.springframework.util.StringUtils;
* @author Jungtaek Lim
* @author Mark Paluch
* @author Ninad Divadkar
* @author Guy Korland
*/
abstract public class JedisConverters extends Converters {
@@ -196,7 +197,8 @@ abstract public class JedisConverters extends Converters {
};
GEO_COORDINATE_TO_POINT_CONVERTER = geoCoordinate -> geoCoordinate != null
? new Point(geoCoordinate.getLongitude(), geoCoordinate.getLatitude()) : null;
? new Point(geoCoordinate.getLongitude(), geoCoordinate.getLatitude())
: null;
LIST_GEO_COORDINATE_TO_POINT_CONVERTER = new ListConverter<>(GEO_COORDINATE_TO_POINT_CONVERTER);
BYTES_LIST_TO_LONG_LIST_CONVERTER = new ListConverter<byte[], Long>(new Converter<byte[], Long>() {
@@ -220,8 +222,7 @@ abstract public class JedisConverters extends Converters {
if (command instanceof BitFieldIncrBy) {
BitFieldIncrBy.Overflow overflow = ((BitFieldIncrBy) command)
.getOverflow();
BitFieldIncrBy.Overflow overflow = ((BitFieldIncrBy) command).getOverflow();
if (overflow != null) {
args.add(JedisConverters.toBytes("OVERFLOW"));
args.add(JedisConverters.toBytes(overflow.name()));
@@ -498,51 +499,94 @@ abstract public class JedisConverters extends Converters {
}
/**
* Converts a given {@link Expiration} to the according {@code SET} command argument.<br />
* Converts a given {@link Expiration} to the according {@code SET} command argument.
* <dl>
* <dt>{@link TimeUnit#SECONDS}</dt>
* <dd>{@code EX}</dd>
* <dt>{@link TimeUnit#MILLISECONDS}</dt>
* <dd>{@code PX}</dd>
* <dt>{@link TimeUnit#SECONDS}</dt>
* <dd>{@code EX}</dd>
* </dl>
*
* @param expiration
* @param expiration must not be {@literal null}.
* @return
* @since 1.7
* @since 2.2
*/
public static SetParams toSetCommandExPxArgument(Expiration expiration) {
return toSetCommandExPxArgument(expiration, SetParams.setParams());
}
/**
* Converts a given {@link Expiration} to the according {@code SET} command argument.
* <dl>
* <dt>{@link TimeUnit#MILLISECONDS}</dt>
* <dd>{@code PX}</dd>
* <dt>{@link TimeUnit#SECONDS}</dt>
* <dd>{@code EX}</dd>
* </dl>
*
* @param expiration must not be {@literal null}.
* @param params
* @return
* @since 2.2
*/
public static SetParams toSetCommandExPxArgument(Expiration expiration, SetParams params) {
params = params==null? SetParams.setParams() : params;
if(expiration.getTimeUnit() == TimeUnit.MILLISECONDS) {
return params.px(expiration.getExpirationTime());
}
return params.ex((int)expiration.getExpirationTime());
}
SetParams paramsToUse = params == null ? SetParams.setParams() : params;
if (expiration.getTimeUnit() == TimeUnit.MILLISECONDS) {
return paramsToUse.px(expiration.getExpirationTime());
}
return paramsToUse.ex((int) expiration.getExpirationTime());
}
/**
* Converts a given {@link SetOption} to the according {@code SET} command argument.<br />
* <dl>
* <dt>{@link SetOption#UPSERT}</dt>
* <dd>{@code byte[0]}</dd>
* <dt>{@link SetOption#SET_IF_ABSENT}</dt>
* <dd>{@code NX}</dd>
* <dt>{@link SetOption#SET_IF_PRESENT}</dt>
* <dd>{@code XX}</dd>
* <dt>{@link SetOption#SET_IF_ABSENT}</dt>
* <dd>{@code NX}</dd>
* <dt>{@link SetOption#UPSERT}</dt>
* <dd>{@code byte[0]}</dd>
* </dl>
*
* @param option
* @param option must not be {@literal null}.
* @return
* @since 1.7
* @since 2.2
*/
public static SetParams toSetCommandNxXxArgument(SetOption option) {
return toSetCommandNxXxArgument(option, SetParams.setParams());
}
/**
* Converts a given {@link SetOption} to the according {@code SET} command argument.<br />
* <dl>
* <dt>{@link SetOption#SET_IF_PRESENT}</dt>
* <dd>{@code XX}</dd>
* <dt>{@link SetOption#SET_IF_ABSENT}</dt>
* <dd>{@code NX}</dd>
* <dt>{@link SetOption#UPSERT}</dt>
* <dd>{@code byte[0]}</dd>
* </dl>
*
* @param option must not be {@literal null}.
* @param params
* @return
* @since 2.2
*/
public static SetParams toSetCommandNxXxArgument(SetOption option, SetParams params) {
params = params==null? SetParams.setParams() : params;
switch(option) {
case SET_IF_PRESENT:
return params.xx();
case SET_IF_ABSENT:
return params.nx();
default:
return params;
}
SetParams paramsToUse = params == null ? SetParams.setParams() : params;
switch (option) {
case SET_IF_PRESENT:
return paramsToUse.xx();
case SET_IF_ABSENT:
return paramsToUse.nx();
default:
return paramsToUse;
}
}
private static byte[] boundaryToBytes(Boundary boundary, byte[] inclPrefix, byte[] exclPrefix) {

37
src/main/java/org/springframework/data/redis/connection/jedis/JedisExceptionConverter.java
View File

@@ -15,6 +15,11 @@
*/
package org.springframework.data.redis.connection.jedis;
import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
import redis.clients.jedis.exceptions.JedisConnectionException;
import redis.clients.jedis.exceptions.JedisException;
import redis.clients.jedis.exceptions.JedisRedirectionException;
import java.io.IOException;
import java.net.UnknownHostException;
@@ -25,18 +30,14 @@ import org.springframework.data.redis.ClusterRedirectException;
import org.springframework.data.redis.RedisConnectionFailureException;
import org.springframework.data.redis.TooManyClusterRedirectionsException;
import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
import redis.clients.jedis.exceptions.JedisConnectionException;
import redis.clients.jedis.exceptions.JedisDataException;
import redis.clients.jedis.exceptions.JedisException;
import redis.clients.jedis.exceptions.JedisRedirectionException;
/**
* Converts Exceptions thrown from Jedis to {@link DataAccessException}s
*
* @author Jennifer Hickey
* @author Thomas Darimont
* @author Christoph Strobl
* @author Guy Korland
* @author Mark Paluch
*/
public class JedisExceptionConverter implements Converter<Exception, DataAccessException> {
@@ -49,29 +50,29 @@ public class JedisExceptionConverter implements Converter<Exception, DataAccessE
if (ex instanceof DataAccessException) {
return (DataAccessException) ex;
}
if (ex instanceof JedisDataException) {
if (ex instanceof JedisRedirectionException) {
JedisRedirectionException re = (JedisRedirectionException) ex;
return new ClusterRedirectException(re.getSlot(), re.getTargetNode().getHost(), re.getTargetNode().getPort(),
ex);
}
if (ex instanceof JedisClusterMaxAttemptsException) {
return new TooManyClusterRedirectionsException(ex.getMessage(), ex);
}
return new InvalidDataAccessApiUsageException(ex.getMessage(), ex);
if (ex instanceof JedisClusterMaxAttemptsException) {
return new TooManyClusterRedirectionsException(ex.getMessage(), ex);
}
if (ex instanceof JedisRedirectionException) {
JedisRedirectionException re = (JedisRedirectionException) ex;
return new ClusterRedirectException(re.getSlot(), re.getTargetNode().getHost(), re.getTargetNode().getPort(), ex);
}
if (ex instanceof JedisConnectionException) {
return new RedisConnectionFailureException(ex.getMessage(), ex);
}
if (ex instanceof JedisException) {
return new InvalidDataAccessApiUsageException(ex.getMessage(), ex);
}
if (ex instanceof UnknownHostException) {
return new RedisConnectionFailureException("Unknown host " + ex.getMessage(), ex);
}
if (ex instanceof IOException) {
return new RedisConnectionFailureException("Could not connect to Redis server", ex);
}

4
src/main/java/org/springframework/data/redis/connection/jedis/JedisKeyCommands.java
View File

@@ -21,7 +21,6 @@ import redis.clients.jedis.ScanParams;
import redis.clients.jedis.SortingParams;
import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@@ -686,8 +685,7 @@ class JedisKeyCommands implements RedisKeyCommands {
if (replace) {
this.connection.execute("RESTORE", new byte[][] { key, JedisConverters.toBytes(ttlInMillis),
serializedValue,
this.connection.execute("RESTORE", new byte[][] { key, JedisConverters.toBytes(ttlInMillis), serializedValue,
JedisConverters.toBytes("REPLACE") });
return;
}

11
src/main/java/org/springframework/data/redis/connection/jedis/JedisServerCommands.java
View File

@@ -494,17 +494,16 @@ class JedisServerCommands implements RedisServerCommands {
try {
if (isPipelined()) {
pipeline(connection.newJedisResult(connection.getRequiredPipeline()
.migrate(target.getHost(), target.getPort(), key, dbIndex, timeoutToUse)));
pipeline(connection.newJedisResult(
connection.getRequiredPipeline().migrate(target.getHost(), target.getPort(), key, dbIndex, timeoutToUse)));
return;
}
if (isQueueing()) {
transaction(connection.newJedisResult(connection.getRequiredTransaction()
.migrate(target.getHost(), target.getPort(), key, dbIndex, timeoutToUse)));
transaction(connection.newJedisResult(connection.getRequiredTransaction().migrate(target.getHost(),
target.getPort(), key, dbIndex, timeoutToUse)));
return;
}
connection.getJedis().migrate(target.getHost(), target.getPort(), key, dbIndex,
timeoutToUse);
connection.getJedis().migrate(target.getHost(), target.getPort(), key, dbIndex, timeoutToUse);
} catch (Exception ex) {
throw convertJedisAccessException(ex);
}

2
src/main/java/org/springframework/data/redis/connection/jedis/JedisStringCommands.java
View File

@@ -166,7 +166,7 @@ class JedisStringCommands implements RedisStringCommands {
} else {
try {
SetParams nxxx = JedisConverters.toSetCommandNxXxArgument(option, null);
SetParams nxxx = JedisConverters.toSetCommandNxXxArgument(option);
if (isPipelined()) {

10
src/main/java/org/springframework/data/redis/connection/jedis/JedisZSetCommands.java
View File

@@ -579,7 +579,7 @@ class JedisZSetCommands implements RedisZSetCommands {
}
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisZSetCommands#zUnionStore(byte[], org.springframework.data.redis.connection.RedisZSetCommands.Aggregate, org.springframework.data.redis.connection.RedisZSetCommands.Weights, byte[][])
*/
@@ -594,8 +594,7 @@ class JedisZSetCommands implements RedisZSetCommands {
.format("The number of weights (%d) must match the number of source sets (%d)!", weights.size(), sets.length));
try {
ZParams zparams = new ZParams().weights(weights.toArray())
.aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
ZParams zparams = new ZParams().weights(weights.toArray()).aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
if (isPipelined()) {
pipeline(connection.newJedisResult(connection.getRequiredPipeline().zunionstore(destKey, zparams, sets)));
@@ -637,7 +636,7 @@ class JedisZSetCommands implements RedisZSetCommands {
}
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisZSetCommands#zInterStore(byte[], org.springframework.data.redis.connection.RedisZSetCommands.Aggregate, org.springframework.data.redis.connection.RedisZSetCommands.Weights, byte[][])
*/
@@ -651,8 +650,7 @@ class JedisZSetCommands implements RedisZSetCommands {
.format("The number of weights (%d) must match the number of source sets (%d)!", weights.size(), sets.length));
try {
ZParams zparams = new ZParams().weights(weights.toArray())
.aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
ZParams zparams = new ZParams().weights(weights.toArray()).aggregate(ZParams.Aggregate.valueOf(aggregate.name()));
if (isPipelined()) {
pipeline(connection.newJedisResult(connection.getRequiredPipeline().zinterstore(destKey, zparams, sets)));