Refactor code and cleanup compiler warnings in Redis caching infrastr…

…ucture. * Apply Java 17 syntax try-with-resources in DefaultRedisCacheWriter execute methods. * Organize source code * Edit Javadoc. Closes #2733
spring-projects · Oct 12, 2023 · 747a7d0 · 747a7d0
1 parent df4be6f
commit 747a7d0
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 81 deletions.
diff --git a/src/main/java/org/springframework/data/redis/cache/BatchStrategies.java b/src/main/java/org/springframework/data/redis/cache/BatchStrategies.java
@@ -28,18 +28,15 @@
 import org.springframework.util.Assert;
 
 /**
- * A collection of predefined {@link BatchStrategy} implementations using {@code KEYS} or {@code SCAN} command.
+ * Collection of predefined {@link BatchStrategy} implementations using the Redis {@code KEYS} or {@code SCAN} command.
  *
  * @author Mark Paluch
  * @author Christoph Strobl
+ * @author John Blum
  * @since 2.6
  */
 public abstract class BatchStrategies {
 
-	private BatchStrategies() {
-		// can't touch this - oh-oh oh oh oh-oh-oh
-	}
-
 	/**
 	 * A {@link BatchStrategy} using a single {@code KEYS} and {@code DEL} command to remove all matching keys.
 	 * {@code KEYS} scans the entire keyspace of the Redis database and can block the Redis worker thread for a long time
@@ -68,6 +65,10 @@ public static BatchStrategy scan(int batchSize) {
 		return new Scan(batchSize);
 	}
 
+	private BatchStrategies() {
+		// can't touch this - oh-oh oh oh oh-oh-oh
+	}
+
 	/**
 	 * {@link BatchStrategy} using {@code KEYS}.
 	 */
@@ -108,9 +109,11 @@ public long cleanCache(RedisConnection connection, String name, byte[] pattern)
 			long count = 0;
 
 			PartitionIterator<byte[]> partitions = new PartitionIterator<>(cursor, batchSize);
+
 			while (partitions.hasNext()) {
 
 				List<byte[]> keys = partitions.next();
+
 				count += keys.size();
 
 				if (keys.size() > 0) {
@@ -141,7 +144,7 @@ static class PartitionIterator<T> implements Iterator<List<T>> {
 
 		@Override
 		public boolean hasNext() {
-			return iterator.hasNext();
+			return this.iterator.hasNext();
 		}
 
 		@Override
@@ -151,9 +154,10 @@ public List<T> next() {
 				throw new NoSuchElementException();
 			}
 
-			List<T> list = new ArrayList<>(size);
-			while (list.size() < size && iterator.hasNext()) {
-				list.add(iterator.next());
+			List<T> list = new ArrayList<>(this.size);
+
+			while (list.size() < this.size && this.iterator.hasNext()) {
+				list.add(this.iterator.next());
 			}
 
 			return list;

diff --git a/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java b/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java
@@ -102,27 +102,6 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
 		this.batchStrategy = batchStrategy;
 	}
 
-	@Override
-	public void put(String name, byte[] key, byte[] value, @Nullable Duration ttl) {
-
-		Assert.notNull(name, "Name must not be null");
-		Assert.notNull(key, "Key must not be null");
-		Assert.notNull(value, "Value must not be null");
-
-		execute(name, connection -> {
-
-			if (shouldExpireWithin(ttl)) {
-				connection.set(key, value, Expiration.from(ttl.toMillis(), TimeUnit.MILLISECONDS), SetOption.upsert());
-			} else {
-				connection.set(key, value);
-			}
-
-			return "OK";
-		});
-
-		statistics.incPuts(name);
-	}
-
 	@Override
 	public byte[] get(String name, byte[] key) {
 		return get(name, key, null);
@@ -135,8 +114,8 @@ public byte[] get(String name, byte[] key, @Nullable Duration ttl) {
 		Assert.notNull(key, "Key must not be null");
 
 		byte[] result = shouldExpireWithin(ttl)
-			? execute(name, connection -> connection.getEx(key, Expiration.from(ttl)))
-			: execute(name, connection -> connection.get(key));
+			? execute(name, connection -> connection.stringCommands().getEx(key, Expiration.from(ttl)))
+			: execute(name, connection -> connection.stringCommands().get(key));
 
 		statistics.incGets(name);
 
@@ -149,6 +128,28 @@ public byte[] get(String name, byte[] key, @Nullable Duration ttl) {
 		return result;
 	}
 
+	@Override
+	public void put(String name, byte[] key, byte[] value, @Nullable Duration ttl) {
+
+		Assert.notNull(name, "Name must not be null");
+		Assert.notNull(key, "Key must not be null");
+		Assert.notNull(value, "Value must not be null");
+
+		execute(name, connection -> {
+
+			if (shouldExpireWithin(ttl)) {
+				connection.stringCommands()
+						.set(key, value, Expiration.from(ttl.toMillis(), TimeUnit.MILLISECONDS), SetOption.upsert());
+			} else {
+				connection.stringCommands().set(key, value);
+			}
+
+			return "OK";
+		});
+
+		statistics.incPuts(name);
+	}
+
 	@Override
 	public byte[] putIfAbsent(String name, byte[] key, byte[] value, @Nullable Duration ttl) {
 
@@ -167,17 +168,17 @@ public byte[] putIfAbsent(String name, byte[] key, byte[] value, @Nullable Durat
 				boolean put;
 
 				if (shouldExpireWithin(ttl)) {
-					put = connection.set(key, value, Expiration.from(ttl), SetOption.ifAbsent());
+					put = isTrue(connection.stringCommands().set(key, value, Expiration.from(ttl), SetOption.ifAbsent()));
 				} else {
-					put = connection.setNX(key, value);
+					put = isTrue(connection.stringCommands().setNX(key, value));
 				}
 
 				if (put) {
 					statistics.incPuts(name);
 					return null;
 				}
 
-				return connection.get(key);
+				return connection.stringCommands().get(key);
 
 			} finally {
 				if (isLockingCacheWriter()) {
@@ -193,7 +194,7 @@ public void remove(String name, byte[] key) {
 		Assert.notNull(name, "Name must not be null");
 		Assert.notNull(key, "Key must not be null");
 
-		execute(name, connection -> connection.del(key));
+		execute(name, connection -> connection.keyCommands().del(key));
 		statistics.incDeletes(name);
 	}
 
@@ -257,6 +258,16 @@ void lock(String name) {
 		execute(name, connection -> doLock(name, name, null, connection));
 	}
 
+	@Nullable
+	private Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
+			RedisConnection connection) {
+
+		Expiration expiration = Expiration.from(this.lockTtl.getTimeToLive(contextualKey, contextualValue));
+
+		return connection.stringCommands()
+				.set(createCacheLockKey(name), new byte[0], expiration, SetOption.SET_IF_ABSENT);
+	}
+
 	/**
 	 * Explicitly remove a write lock from a cache.
 	 *
@@ -266,20 +277,13 @@ void unlock(String name) {
 		executeLockFree(connection -> doUnlock(name, connection));
 	}
 
-	private Boolean doLock(String name, Object contextualKey, Object contextualValue, RedisConnection connection) {
-
-		Expiration expiration = lockTtl == null ? Expiration.persistent()
-				: Expiration.from(lockTtl.getTimeToLive(contextualKey, contextualValue));
-
-		return connection.set(createCacheLockKey(name), new byte[0], expiration, SetOption.SET_IF_ABSENT);
-	}
-
+	@Nullable
 	private Long doUnlock(String name, RedisConnection connection) {
-		return connection.del(createCacheLockKey(name));
+		return connection.keyCommands().del(createCacheLockKey(name));
 	}
 
 	boolean doCheckLock(String name, RedisConnection connection) {
-		return connection.exists(createCacheLockKey(name));
+		return isTrue(connection.keyCommands().exists(createCacheLockKey(name)));
 	}
 
 	/**
@@ -291,24 +295,16 @@ private boolean isLockingCacheWriter() {
 
 	private <T> T execute(String name, Function<RedisConnection, T> callback) {
 
-		RedisConnection connection = connectionFactory.getConnection();
-
-		try {
+		try (RedisConnection connection = connectionFactory.getConnection()) {
 			checkAndPotentiallyWaitUntilUnlocked(name, connection);
 			return callback.apply(connection);
-		} finally {
-			connection.close();
 		}
 	}
 
 	private void executeLockFree(Consumer<RedisConnection> callback) {
 
-		RedisConnection connection = connectionFactory.getConnection();
-
-		try {
+		try (RedisConnection connection = connectionFactory.getConnection()) {
 			callback.accept(connection);
-		} finally {
-			connection.close();
 		}
 	}
 
@@ -337,11 +333,15 @@ private void checkAndPotentiallyWaitUntilUnlocked(String name, RedisConnection c
 		}
 	}
 
-	private static boolean shouldExpireWithin(@Nullable Duration ttl) {
-		return ttl != null && !ttl.isZero() && !ttl.isNegative();
-	}
-
 	private static byte[] createCacheLockKey(String name) {
 		return (name + "~lock").getBytes(StandardCharsets.UTF_8);
 	}
+
+	private boolean isTrue(@Nullable Boolean value) {
+		return Boolean.TRUE.equals(value);
+	}
+
+	private static boolean shouldExpireWithin(@Nullable Duration ttl) {
+		return ttl != null && !ttl.isZero() && !ttl.isNegative();
+	}
 }
diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCache.java b/src/main/java/org/springframework/data/redis/cache/RedisCache.java
@@ -24,8 +24,10 @@
 import java.util.Map.Entry;
 import java.util.StringJoiner;
 import java.util.concurrent.Callable;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Supplier;
 
 import org.springframework.cache.Cache;
 import org.springframework.cache.support.AbstractValueAdaptingCache;
@@ -37,6 +39,7 @@
 import org.springframework.data.redis.serializer.RedisSerializationContext;
 import org.springframework.data.redis.serializer.RedisSerializer;
 import org.springframework.data.redis.util.ByteUtils;
+import org.springframework.data.redis.util.RedisAssertions;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
@@ -69,7 +72,8 @@ public class RedisCache extends AbstractValueAdaptingCache {
 	private final String name;
 
 	/**
-	 * Create a new {@link RedisCache} with the given {@link String name}.
+	 * Create a new {@link RedisCache} with the given {@link String name} and {@link RedisCacheConfiguration},
+	 * using the {@link RedisCacheWriter} to execute Redis commands supporting the cache operations.
 	 *
 	 * @param name {@link String name} for this {@link Cache}; must not be {@literal null}.
 	 * @param cacheWriter {@link RedisCacheWriter} used to perform {@link RedisCache} operations by
@@ -81,11 +85,11 @@ public class RedisCache extends AbstractValueAdaptingCache {
 	 */
 	protected RedisCache(String name, RedisCacheWriter cacheWriter, RedisCacheConfiguration cacheConfiguration) {
 
-		super(cacheConfiguration.getAllowCacheNullValues());
+		super(RedisAssertions.requireNonNull(cacheConfiguration, "CacheConfiguration must not be null")
+				.getAllowCacheNullValues());
 
 		Assert.notNull(name, "Name must not be null");
 		Assert.notNull(cacheWriter, "CacheWriter must not be null");
-		Assert.notNull(cacheConfiguration, "CacheConfiguration must not be null");
 
 		this.name = name;
 		this.cacheWriter = cacheWriter;
@@ -116,7 +120,7 @@ protected RedisCacheWriter getCacheWriter() {
 	 * accessing entries in the cache.
 	 *
 	 * @return the configured {@link ConversionService} used to convert {@link Object cache keys} to a {@link String} when
-	 *         accessing entries in the cache.
+	 * accessing entries in the cache.
 	 * @see RedisCacheConfiguration#getConversionService()
 	 * @see #getCacheConfiguration()
 	 */
@@ -163,7 +167,6 @@ private <T> T getSynchronized(Object key, Callable<T> valueLoader) {
 
 		try {
 			ValueWrapper result = get(key);
-
 			return result != null ? (T) result.get() : loadCacheValue(key, valueLoader);
 		} finally {
 			lock.unlock();
@@ -285,10 +288,19 @@ public void evict(Object key) {
 	 */
 	@Nullable
 	protected Object preProcessCacheValue(@Nullable Object value) {
-
 		return value != null ? value : isAllowNullValues() ? NullValue.INSTANCE : null;
 	}
 
+	@Override
+	public CompletableFuture<?> retrieve(Object key) {
+		return super.retrieve(key);
+	}
+
+	@Override
+	public <T> CompletableFuture<T> retrieve(Object key, Supplier<CompletableFuture<T>> valueLoader) {
+		return super.retrieve(key, valueLoader);
+	}
+
 	/**
 	 * Serialize the given {@link String cache key}.
 	 *
@@ -321,7 +333,7 @@ protected byte[] serializeCacheValue(Object value) {
 	 *
 	 * @param value array of bytes to deserialize; must not be {@literal null}.
 	 * @return an {@link Object} deserialized from the array of bytes using the configured value
-	 *         {@link RedisSerializationContext.SerializationPair}; can be {@literal null}.
+	 * {@link RedisSerializationContext.SerializationPair}; can be {@literal null}.
 	 * @see RedisCacheConfiguration#getValueSerializationPair()
 	 */
 	@Nullable
@@ -382,9 +394,8 @@ protected String convertKey(Object key) {
 			return key.toString();
 		}
 
-		String message = String.format(
-				"Cannot convert cache key %s to String; Please register a suitable Converter"
-						+ " via 'RedisCacheConfiguration.configureKeyConverters(...)' or override '%s.toString()'",
+		String message = String.format("Cannot convert cache key %s to String; Please register a suitable Converter"
+				+ " via 'RedisCacheConfiguration.configureKeyConverters(...)' or override '%s.toString()'",
 				source, key.getClass().getName());
 
 		throw new IllegalStateException(message);