diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStringCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStringCommands.java index 56c869463..151b8879c 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStringCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStringCommands.java @@ -25,7 +25,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; import org.springframework.dao.DataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -40,7 +39,6 @@ import org.springframework.data.redis.connection.lettuce.LettuceConverters; import org.springframework.data.redis.core.types.Expiration; import org.springframework.data.redis.util.ByteUtils; import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; /** * @author Christoph Strobl @@ -134,39 +132,12 @@ class JedisClusterStringCommands implements RedisStringCommands { Assert.notNull(expiration, "Expiration must not be null!"); Assert.notNull(option, "Option must not be null!"); - if (expiration.isPersistent()) { + SetParams setParams = JedisConverters.toSetCommandExPxArgument(expiration, JedisConverters.toSetCommandNxXxArgument(option)); - if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) { - return set(key, value); - } else { - - // BinaryCluster does not support set with nxxx and binary key/value pairs. - if (ObjectUtils.nullSafeEquals(SetOption.SET_IF_PRESENT, option)) { - throw new UnsupportedOperationException("Jedis does not support SET XX without PX or EX on BinaryCluster."); - } - - return setNX(key, value); - } - } else { - - if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) { - - if (ObjectUtils.nullSafeEquals(TimeUnit.MILLISECONDS, expiration.getTimeUnit())) { - return pSetEx(key, expiration.getExpirationTime(), value); - } else { - return setEx(key, expiration.getExpirationTime(), value); - } - } else { - - SetParams params = JedisConverters.toSetCommandNxXxArgument(option); - JedisConverters.toSetCommandExPxArgument(expiration, params); - - try { - return Converters.stringToBoolean(connection.getCluster().set(key, value, params)); - } catch (Exception ex) { - throw convertJedisAccessException(ex); - } - } + try { + return Converters.stringToBoolean(connection.getCluster().set(key, value, setParams)); + } 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 bcd004ffd..a8efe3350 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 @@ -533,11 +533,15 @@ abstract public class JedisConverters extends Converters { SetParams paramsToUse = params == null ? SetParams.setParams() : params; - if (expiration.getTimeUnit() == TimeUnit.MILLISECONDS) { - return paramsToUse.px(expiration.getExpirationTime()); + if (!expiration.isPersistent()) { + if (expiration.getTimeUnit() == TimeUnit.MILLISECONDS) { + return paramsToUse.px(expiration.getExpirationTime()); + } + + return paramsToUse.ex((int) expiration.getExpirationTime()); } - return paramsToUse.ex((int) expiration.getExpirationTime()); + return params; } /** diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisStringCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisStringCommands.java index 90d291a0b..a1602664e 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisStringCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisStringCommands.java @@ -23,7 +23,6 @@ import redis.clients.jedis.params.SetParams; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; import org.springframework.data.domain.Range; import org.springframework.data.redis.connection.BitFieldSubCommands; @@ -33,7 +32,6 @@ import org.springframework.data.redis.core.types.Expiration; import org.springframework.data.redis.util.ByteUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; /** * @author Christoph Strobl @@ -159,79 +157,26 @@ class JedisStringCommands implements RedisStringCommands { Assert.notNull(expiration, "Expiration must not be null!"); Assert.notNull(option, "Option must not be null!"); - if (expiration.isPersistent()) { + SetParams params = JedisConverters.toSetCommandExPxArgument(expiration, JedisConverters.toSetCommandNxXxArgument(option)); - if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) { - return set(key, value); - } else { + try { + if (isPipelined()) { - try { - SetParams nxxx = JedisConverters.toSetCommandNxXxArgument(option); + pipeline(connection.newJedisResult(connection.getRequiredPipeline().set(key, value, params), + Converters.stringToBooleanConverter(), () -> false)); + return null; + } + if (isQueueing()) { - if (isPipelined()) { - - pipeline(connection.newJedisResult(connection.getRequiredPipeline().set(key, value, nxxx), - Converters.stringToBooleanConverter(), () -> false)); - return null; - } - if (isQueueing()) { - - transaction(connection.newJedisResult(connection.getRequiredTransaction().set(key, value, nxxx), - Converters.stringToBooleanConverter(), () -> false)); - return null; - } - - return Converters.stringToBoolean(connection.getJedis().set(key, value, nxxx)); - } catch (Exception ex) { - throw convertJedisAccessException(ex); - } + transaction(connection.newJedisResult(connection.getRequiredTransaction().set(key, value, params), + Converters.stringToBooleanConverter(), () -> false)); + return null; } - } else { + return Converters.stringToBoolean(connection.getJedis().set(key, value, params)); - if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) { - - if (ObjectUtils.nullSafeEquals(TimeUnit.MILLISECONDS, expiration.getTimeUnit())) { - return pSetEx(key, expiration.getExpirationTime(), value); - } else { - return setEx(key, expiration.getExpirationTime(), value); - } - } else { - - SetParams params = JedisConverters.toSetCommandNxXxArgument(option, null); - JedisConverters.toSetCommandExPxArgument(expiration, params); - - try { - if (isPipelined()) { - - if (expiration.getExpirationTime() > Integer.MAX_VALUE) { - - throw new IllegalArgumentException( - "Expiration.expirationTime must be less than Integer.MAX_VALUE for pipeline in Jedis."); - } - - pipeline(connection.newJedisResult(connection.getRequiredPipeline().set(key, value, params), - Converters.stringToBooleanConverter(), () -> false)); - return null; - } - if (isQueueing()) { - - if (expiration.getExpirationTime() > Integer.MAX_VALUE) { - throw new IllegalArgumentException( - "Expiration.expirationTime must be less than Integer.MAX_VALUE for transactions in Jedis."); - } - - transaction(connection.newJedisResult(connection.getRequiredTransaction().set(key, value, params), - Converters.stringToBooleanConverter(), () -> false)); - return null; - } - - return Converters.stringToBoolean(connection.getJedis().set(key, value, params)); - - } catch (Exception ex) { - throw convertJedisAccessException(ex); - } - } + } catch (Exception ex) { + throw convertJedisAccessException(ex); } } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java index ebf0a2371..bd0c79fb2 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java @@ -1830,11 +1830,15 @@ public class JedisClusterConnectionTests implements ClusterConnectionTests { assertThat(nativeConnection.ttl(KEY_1_BYTES)).isEqualTo(-1L); } - @Test(expected = UnsupportedOperationException.class) // DATAREDIS-316 + @Test // DATAREDIS-316, DATAREDIS-588 public void setWithOptionIfPresentShouldWorkCorrectly() { nativeConnection.set(KEY_1_BYTES, VALUE_1_BYTES); clusterConnection.set(KEY_1_BYTES, VALUE_2_BYTES, Expiration.persistent(), SetOption.ifPresent()); + + assertThat(nativeConnection.exists(KEY_1_BYTES), is(true)); + assertThat(clusterConnection.get(KEY_1_BYTES), is(VALUE_2_BYTES)); + assertThat(nativeConnection.ttl(KEY_1_BYTES), is(-1L)); } @Test // DATAREDIS-315