Polishing.

Use SetCondition instead of SetOption, remove implementations using SetOption and tests. Reduce tests to avoid cartesian products caused by matrix testing on all sorts of levels, instead, test conversions to driver-specific argument objects and ensure that argument objects are applied.

Revert deprecations of setIf… methods accepting Duration/Expiration.

See spring-projects#3324

Author: mp911de

src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.redis.cache;
 
-import org.springframework.data.redis.connection.SetCondition;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
@@ -40,6 +39,7 @@
 import org.springframework.data.redis.connection.RedisConnection;
 import org.springframework.data.redis.connection.RedisConnectionFactory;
 import org.springframework.data.redis.connection.RedisStringCommands;
+import org.springframework.data.redis.connection.SetCondition;
 import org.springframework.data.redis.core.ScanOptions;
 import org.springframework.data.redis.core.types.Expiration;
 import org.springframework.data.redis.util.ByteUtils;
@@ -213,8 +213,7 @@ public void enable(Consumer<CacheLockingConfiguration> configurationConsumer) {
 		public CacheLockingConfiguration sleepTime(Duration sleepTime) {
 
 			Assert.notNull(sleepTime, "Lock sleep time must not be null");
-			Assert.isTrue(isPositiveDuration(sleepTime),
-					"Lock sleep time must not be null zero or negative");
+			Assert.isTrue(isPositiveDuration(sleepTime), "Lock sleep time must not be null zero or negative");
 
 			this.lockSleepTime = sleepTime;
 
@@ -356,7 +355,8 @@ public void put(String name, byte[] key, byte[] value, @Nullable Duration ttl) {
 	private void doPut(RedisConnection connection, String name, byte[] key, byte[] value, @Nullable Duration ttl) {
 
 		if (isPositiveDuration(ttl)) {
-			connection.stringCommands().set(key, value, SetCondition.upsert(), Expiration.from(ttl.toMillis(), TimeUnit.MILLISECONDS));
+			connection.stringCommands().set(key, value, SetCondition.upsert(),
+					Expiration.from(ttl.toMillis(), TimeUnit.MILLISECONDS));
 		} else {
 			connection.stringCommands().set(key, value);
 		}
@@ -750,7 +750,8 @@ private Mono<Boolean> doStore(byte[] cacheKey, byte[] value, @Nullable Duration
 			ByteBuffer wrappedValue = ByteBuffer.wrap(value);
 
 			if (isPositiveDuration(ttl)) {
-				return connection.stringCommands().set(wrappedKey, wrappedValue, SetCondition.upsert(), Expiration.from(ttl.toMillis(), TimeUnit.MILLISECONDS));
+				return connection.stringCommands().set(wrappedKey, wrappedValue, SetCondition.upsert(),
+						Expiration.from(ttl.toMillis(), TimeUnit.MILLISECONDS));
 			} else {
 				return connection.stringCommands().set(wrappedKey, wrappedValue);
 			}
@@ -784,8 +785,7 @@ private Mono<Long> doClear(byte[] pattern, ReactiveRedisConnection connection) {
 				keys = commands.scan(ScanOptions.scanOptions().count(clearBatchSize).match(pattern).build());
 			}
 
-			return keys
-					.buffer(clearBatchSize) //
+			return keys.buffer(clearBatchSize) //
 					.flatMap(commands::mUnlink) //
 					.collect(Collectors.summingLong(Long::longValue));
 		}
@@ -840,8 +840,7 @@ private Mono<Void> waitForLock(ReactiveRedisConnection connection, String cacheN
 					.then();
 		}
 
-		private <T> CompletableFuture<T> doWithConnection(
-				Function<ReactiveRedisConnection, Mono<T>> callback) {
+		private <T> CompletableFuture<T> doWithConnection(Function<ReactiveRedisConnection, Mono<T>> callback) {
 
 			ReactiveRedisConnectionFactory cf = (ReactiveRedisConnectionFactory) connectionFactory;
 

src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java

@@ -778,11 +778,6 @@ public Boolean set(byte[] key, byte[] value) {
 		return convertAndReturn(delegate.set(key, value), Converters.identityConverter());
 	}
 
-	@Override
-	public Boolean set(byte[] key, byte[] value, Expiration expiration, SetOption option) {
-		return convertAndReturn(delegate.set(key, value, expiration, option), Converters.identityConverter());
-	}
-
 	@Override
 	public byte[] setGet(byte[] key, byte[] value, Expiration expiration, SetOption option) {
 		return convertAndReturn(delegate.setGet(key, value, expiration, option), Converters.identityConverter());

src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java

@@ -373,20 +373,6 @@ default byte[] setGet(byte[] key, byte[] value, SetCondition condition, Expirati
 		return stringCommands().setGet(key, value, condition, expiration);
 	}
 
-	/** @deprecated in favor of {@link RedisConnection#stringCommands()}}. */
-	@Override
-	@Deprecated
-	default Boolean set(byte[] key, byte[] value, Expiration expiration, SetOption option) {
-		return stringCommands().set(key, value, expiration, option);
-	}
-
-	/** @deprecated in favor of {@link RedisConnection#stringCommands()}}. */
-	@Override
-	@Deprecated
-	default byte[] setGet(byte[] key, byte[] value, Expiration expiration, SetOption option) {
-		return stringCommands().setGet(key, value, expiration, option);
-	}
-
 	/** @deprecated in favor of {@link RedisConnection#stringCommands()}}. */
 	@Override
 	@Deprecated

src/main/java/org/springframework/data/redis/connection/ReactiveStringCommands.java

@@ -63,14 +63,12 @@ public interface ReactiveStringCommands {
 	class SetCommand extends KeyCommand {
 
 		private final @Nullable ByteBuffer value;
-		private final @Nullable Expiration expiration;
-		@Deprecated(since = "4.1")
+		private final Expiration expiration;
 		private final @Nullable SetOption option;
-		private final @Nullable SetCondition condition;
+		private final SetCondition condition;
 
-		private SetCommand(@Nullable ByteBuffer key, @Nullable ByteBuffer value,
-						   @Nullable Expiration expiration, @Nullable SetOption option,
-						   @Nullable SetCondition condition) {
+		private SetCommand(@Nullable ByteBuffer key, @Nullable ByteBuffer value, Expiration expiration,
+				@Nullable SetOption option, SetCondition condition) {
 
 			super(key);
 
@@ -90,7 +88,7 @@ public static SetCommand set(ByteBuffer key) {
 
 			Assert.notNull(key, "Key must not be null");
 
-			return new SetCommand(key, null, null, null, null);
+			return new SetCommand(key, null, Expiration.persistent(), null, SetCondition.upsert());
 		}
 
 		/**
@@ -120,7 +118,8 @@ public SetCommand expiring(Expiration expiration) {
 		}
 
 		/**
-		 * Applies {@link SetOption}. Constructs a new command instance with all previously configured properties.
+		 * Applies {@link SetOption} and {@link SetCondition}. Constructs a new command instance with all previously
+		 * configured properties.
 		 *
 		 * @param option must not be {@literal null}.
 		 * @return a new {@link SetCommand} with {@link SetOption} applied.
@@ -131,14 +130,15 @@ public SetCommand withSetOption(SetOption option) {
 
 			Assert.notNull(option, "SetOption must not be null");
 
-			return new SetCommand(getKey(), value, expiration, option, condition);
+			return new SetCommand(getKey(), value, expiration, option, option.toSetCondition());
 		}
 
 		/**
 		 * Applies {@link SetCondition}. Constructs a new command instance with all previously configured properties.
 		 *
 		 * @param condition must not be {@literal null}.
 		 * @return a new {@link SetCommand} with {@link SetCondition} applied.
+		 * @since 4.1
 		 */
 		public SetCommand withCondition(SetCondition condition) {
 
@@ -155,14 +155,13 @@ public SetCommand withCondition(SetCondition condition) {
 		}
 
 		/**
-		 * @return optional of expiration.
+		 * @return optional expiration.
 		 */
 		public Optional<Expiration> getExpiration() {
 			return Optional.ofNullable(expiration);
 		}
 
 		/**
-		 * @return
 		 * @deprecated since 4.1 in favor of {@link #getCondition()}.
 		 */
 		@Deprecated(since = "4.1")
@@ -171,7 +170,7 @@ public Optional<SetOption> getOption() {
 		}
 
 		/**
-		 * @return optional of command condition.
+		 * @return optional command condition.
 		 */
 		public Optional<SetCondition> getCondition() {
 			return Optional.ofNullable(condition);
@@ -192,8 +191,7 @@ default Mono<Boolean> set(ByteBuffer key, ByteBuffer value) {
 		Assert.notNull(key, "Key must not be null");
 		Assert.notNull(value, "Value must not be null");
 
-		return set(Mono.just(SetCommand.set(key).value(value).withCondition(SetCondition.upsert())))
-				.next()
+		return set(Mono.just(SetCommand.set(key).value(value).withCondition(SetCondition.upsert()))).next()
 				.map(BooleanResponse::getOutput);
 	}
 
@@ -231,14 +229,13 @@ default Mono<Boolean> set(ByteBuffer key, ByteBuffer value, Expiration expiratio
 	 * @see <a href="https://redis.io/commands/set">Redis Documentation: SET</a>
 	 * @since 4.1
 	 */
-	default  Mono<Boolean> set(ByteBuffer key, ByteBuffer value, SetCondition condition, Expiration expiration) {
+	default Mono<Boolean> set(ByteBuffer key, ByteBuffer value, SetCondition condition, Expiration expiration) {
 
 		Assert.notNull(key, "Key must not be null");
 		Assert.notNull(condition, "SetCondition must not be null");
 		Assert.notNull(expiration, "Expiration must not be null");
 
-		return set(Mono.just(SetCommand.set(key).value(value).withCondition(condition).expiring(expiration)))
-				.next()
+		return set(Mono.just(SetCommand.set(key).value(value).withCondition(condition).expiring(expiration))).next()
 				.map(BooleanResponse::getOutput);
 	}
 
@@ -277,15 +274,14 @@ default Mono<ByteBuffer> setGet(ByteBuffer key, ByteBuffer value, Expiration exp
 	}
 
 	/**
-	 * Set value with {@link SetCondition} and {@link Expiration} for {@code key}. Return the old string
-	 * stored at key, or empty if key did not exist. An error is returned and SET aborted if the value stored at key is
-	 * not a string.
+	 * Set value with {@link SetCondition} and {@link Expiration} for {@code key}. Return the old string stored at key, or
+	 * empty if key did not exist. An error is returned and SET aborted if the value stored at key is not a string.
 	 *
 	 * @param key must not be {@literal null}.
 	 * @param value must not be {@literal null}.
 	 * @param condition must not be {@literal null}.
-	 * @param expiration must not be {@literal null}. Use {@link Expiration#persistent()} for no expiration time
-	 *                      or {@link Expiration#keepTtl()} to keep the existing.
+	 * @param expiration must not be {@literal null}. Use {@link Expiration#persistent()} for no expiration time or
+	 *          {@link Expiration#keepTtl()} to keep the existing.
 	 * @return the old value stored at key, or empty if key did not exist.
 	 * @see <a href="https://redis.io/commands/set">Redis Documentation: SET</a>
 	 * @since 4.1
@@ -297,8 +293,7 @@ default Mono<ByteBuffer> setGet(ByteBuffer key, ByteBuffer value, SetCondition c
 		Assert.notNull(condition, "SetCondition must not be null");
 		Assert.notNull(expiration, "Expiration must not be null");
 
-		return setGet(Mono.just(SetCommand.set(key).value(value).withCondition(condition).expiring(expiration)))
-				.next()
+		return setGet(Mono.just(SetCommand.set(key).value(value).withCondition(condition).expiring(expiration))).next()
 				.map(CommandResponse::getOutput);
 	}
 

src/main/java/org/springframework/data/redis/connection/RedisStringCommands.java

@@ -142,40 +142,46 @@ enum BitOperation {
 	 * @deprecated since 4.1 in favor of {@link #set(byte[], byte[], SetCondition, Expiration)}.
 	 */
 	@Deprecated(since = "4.1")
-	Boolean set(byte @NonNull [] key, byte @NonNull [] value, @NonNull Expiration expiration, @NonNull SetOption option);
+	default Boolean set(byte @NonNull [] key, byte @NonNull [] value, @NonNull Expiration expiration,
+			@NonNull SetOption option) {
+		return set(key, value, option != null ? option.toSetCondition() : SetCondition.upsert(), expiration);
+	}
 
 	/**
-	 * Set {@code value} for {@code key}. Return the old string stored at key, or {@literal null} if key did not exist. An
-	 * error is returned and SET aborted if the value stored at key is not a string.
+	 * Set {@code value} for {@code key} applying timeouts from {@code expiration} if set and inserting/updating values
+	 * depending on {@code option}.
 	 *
 	 * @param key must not be {@literal null}.
 	 * @param value must not be {@literal null}.
+	 * @param condition must not be {@literal null}.
 	 * @param expiration must not be {@literal null}. Use {@link Expiration#persistent()} to not set any ttl or
 	 *          {@link Expiration#keepTtl()} to keep the existing expiration.
-	 * @param option must not be {@literal null}. Use {@link SetOption#upsert()} to add non-existing.
 	 * @return {@literal null} when used in pipeline / transaction.
-	 * @since 3.5
 	 * @see <a href="https://redis.io/commands/set">Redis Documentation: SET</a>
-	 * @deprecated since 4.1 in favor of {@link #set(byte[], byte[], SetCondition, Expiration)}.
+	 * @since 4.1
 	 */
-	@Deprecated(since = "4.1")
-	byte[] setGet(byte @NonNull [] key, byte @NonNull [] value, @NonNull Expiration expiration,
-			@NonNull SetOption option);
+	Boolean set(byte @NonNull [] key, byte @NonNull [] value, @NonNull SetCondition condition,
+			@NonNull Expiration expiration);
 
 	/**
-	 * Set {@code value} for {@code key} applying timeouts from {@code expiration} if set and inserting/updating values
-	 * depending on {@code option}.
+	 * Set {@code value} for {@code key}. Return the old string stored at key, or {@literal null} if key did not exist. An
+	 * error is returned and SET aborted if the value stored at key is not a string.
 	 *
 	 * @param key must not be {@literal null}.
 	 * @param value must not be {@literal null}.
-	 * @param condition must not be {@literal null}.
 	 * @param expiration must not be {@literal null}. Use {@link Expiration#persistent()} to not set any ttl or
 	 *          {@link Expiration#keepTtl()} to keep the existing expiration.
+	 * @param option must not be {@literal null}. Use {@link SetOption#upsert()} to add non-existing.
 	 * @return {@literal null} when used in pipeline / transaction.
+	 * @since 3.5
 	 * @see <a href="https://redis.io/commands/set">Redis Documentation: SET</a>
-	 * @since 4.1
+	 * @deprecated since 4.1 in favor of {@link #set(byte[], byte[], SetCondition, Expiration)}.
 	 */
-	Boolean set(byte @NonNull [] key, byte @NonNull [] value, @NonNull SetCondition condition, @NonNull Expiration expiration);
+	@Deprecated(since = "4.1")
+	default byte[] setGet(byte @NonNull [] key, byte @NonNull [] value, @NonNull Expiration expiration,
+			@NonNull SetOption option) {
+		return setGet(key, value, option != null ? option.toSetCondition() : SetCondition.upsert(), expiration);
+	}
 
 	/**
 	 * Set {@code value} for {@code key}. Return the old string stored at key, or {@literal null} if key did not exist. An
@@ -470,6 +476,18 @@ public static SetOption ifPresent() {
 		public static SetOption ifAbsent() {
 			return SET_IF_ABSENT;
 		}
+
+		/**
+		 * Create {@link SetCondition} from this {@link SetOption}.
+		 */
+		public SetCondition toSetCondition() {
+			return switch (this) {
+				case UPSERT -> SetCondition.upsert();
+				case SET_IF_ABSENT -> SetCondition.ifAbsent();
+				case SET_IF_PRESENT -> SetCondition.ifPresent();
+			};
+		}
+
 	}
 
 }