Apply time to idle in RedisCache.retrieve(…).

We now correctly apply time to idle on retrievals.

Original Pull Request: #2948

Author: mp911de

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

@@ -423,7 +423,12 @@ protected String convertKey(Object key) {
 	}
 
 	private CompletableFuture<ValueWrapper> retrieveValue(Object key) {
-		return getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key)) //
+
+		CompletableFuture<byte[]> retrieve = getCacheConfiguration().isTimeToIdleEnabled()
+				? getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key), getTimeToLive(key))
+				: getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key));
+
+		return retrieve //
 				.thenApply(binaryValue -> binaryValue != null ? deserializeCacheValue(binaryValue) : null) //
 				.thenApply(this::toValueWrapper);
 	}

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

@@ -213,10 +213,10 @@ default CompletableFuture<byte[]> retrieve(String name, byte[] key) {
 	/**
 	 * Write the given key/value pair to Redis and set the expiration time if defined.
 	 *
-	 * @param name The cache name must not be {@literal null}.
-	 * @param key The key for the cache entry. Must not be {@literal null}.
-	 * @param value The value stored for the key. Must not be {@literal null}.
-	 * @param ttl Optional expiration time. Can be {@literal null}.
+	 * @param name cache name must not be {@literal null}.
+	 * @param key key for the cache entry. Must not be {@literal null}.
+	 * @param value value stored for the key. Must not be {@literal null}.
+	 * @param ttl optional expiration time. Can be {@literal null}.
 	 */
 	void put(String name, byte[] key, byte[] value, @Nullable Duration ttl);
 
@@ -225,21 +225,21 @@ default CompletableFuture<byte[]> retrieve(String name, byte[] key) {
 	 * <p>
 	 * This operation is non-blocking.
 	 *
-	 * @param name The cache name must not be {@literal null}.
-	 * @param key The key for the cache entry. Must not be {@literal null}.
-	 * @param value The value stored for the key. Must not be {@literal null}.
-	 * @param ttl Optional expiration time. Can be {@literal null}.
+	 * @param name cache name must not be {@literal null}.
+	 * @param key key for the cache entry. Must not be {@literal null}.
+	 * @param value value stored for the key. Must not be {@literal null}.
+	 * @param ttl optional expiration time. Can be {@literal null}.
 	 * @since 3.2
 	 */
 	CompletableFuture<Void> store(String name, byte[] key, byte[] value, @Nullable Duration ttl);
 
 	/**
 	 * Write the given value to Redis if the key does not already exist.
 	 *
-	 * @param name The cache name must not be {@literal null}.
-	 * @param key The key for the cache entry. Must not be {@literal null}.
-	 * @param value The value stored for the key. Must not be {@literal null}.
-	 * @param ttl Optional expiration time. Can be {@literal null}.
+	 * @param name cache name must not be {@literal null}.
+	 * @param key key for the cache entry. Must not be {@literal null}.
+	 * @param value value stored for the key. Must not be {@literal null}.
+	 * @param ttl optional expiration time. Can be {@literal null}.
 	 * @return {@literal null} if the value has been written, the value stored for the key if it already exists.
 	 */
 	@Nullable
@@ -248,16 +248,16 @@ default CompletableFuture<byte[]> retrieve(String name, byte[] key) {
 	/**
 	 * Remove the given key from Redis.
 	 *
-	 * @param name The cache name must not be {@literal null}.
-	 * @param key The key for the cache entry. Must not be {@literal null}.
+	 * @param name cache name must not be {@literal null}.
+	 * @param key key for the cache entry. Must not be {@literal null}.
 	 */
 	void remove(String name, byte[] key);
 
 	/**
 	 * Remove all keys following the given pattern.
 	 *
-	 * @param name The cache name must not be {@literal null}.
-	 * @param pattern The pattern for the keys to remove. Must not be {@literal null}.
+	 * @param name cache name must not be {@literal null}.
+	 * @param pattern pattern for the keys to remove. Must not be {@literal null}.
 	 */
 	void clean(String name, byte[] pattern);
 

src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java

@@ -16,9 +16,6 @@
 package org.springframework.data.redis.cache;
 
 import static org.assertj.core.api.Assertions.*;
-import static org.awaitility.Awaitility.*;
-
-import io.netty.util.concurrent.DefaultThreadFactory;
 
 import java.io.Serializable;
 import java.nio.charset.StandardCharsets;
@@ -29,20 +26,16 @@
 import java.util.Date;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.LinkedBlockingDeque;
-import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
-import java.util.stream.IntStream;
 
 import org.junit.jupiter.api.BeforeEach;
+
 import org.springframework.cache.Cache.ValueWrapper;
 import org.springframework.cache.interceptor.SimpleKey;
 import org.springframework.cache.interceptor.SimpleKeyGenerator;
@@ -478,11 +471,10 @@ void cacheGetWithTimeToIdleExpirationWhenEntryNotExpiredShouldReturnValue() {
 
 		assertThat(unwrap(cache.get(this.key))).isEqualTo(this.sample);
 
-		for (int count = 0; count < 5; count++) {
+		doWithConnection(connection -> {
 
-			await().atMost(Duration.ofMillis(100));
-			assertThat(unwrap(cache.get(this.key))).isEqualTo(this.sample);
-		}
+			assertThat(connection.keyCommands().ttl(this.binaryCacheKey)).isGreaterThan(1);
+		});
 	}
 
 	@EnabledOnCommand("GETEX")
@@ -496,9 +488,9 @@ void cacheGetWithTimeToIdleExpirationAfterEntryExpiresShouldReturnNull() {
 
 		assertThat(unwrap(cache.get(this.key))).isEqualTo(this.sample);
 
-		await().atMost(Duration.ofMillis(200));
-
-		assertThat(cache.get(this.cacheKey, Person.class)).isNull();
+		doWithConnection(connection -> {
+			assertThat(connection.keyCommands().ttl(this.binaryCacheKey)).isGreaterThan(1);
+		});
 	}
 
 	@ParameterizedRedisTest // GH-2650
@@ -533,6 +525,30 @@ void retrieveReturnsCachedValue() throws Exception {
 		assertThat(value.get(5, TimeUnit.SECONDS)).isNotNull();
 		assertThat(value.get().get()).isEqualTo(this.sample);
 		assertThat(value).isDone();
+
+		doWithConnection(connection -> {
+			assertThat(connection.keyCommands().ttl(this.binaryCacheKey)).isEqualTo(-1);
+		});
+	}
+
+	@ParameterizedRedisTest // GH-2890
+	@EnabledOnRedisDriver(RedisDriver.LETTUCE)
+	void retrieveAppliesTimeToIdle() throws ExecutionException, InterruptedException {
+
+		doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample));
+
+		RedisCache cache = new RedisCache("cache", usingRedisCacheWriter(),
+				usingRedisCacheConfiguration(withTtiExpiration()));
+
+		CompletableFuture<ValueWrapper> value = cache.retrieve(this.key);
+
+		assertThat(value).isNotNull();
+		assertThat(value.get().get()).isEqualTo(this.sample);
+		assertThat(value).isDone();
+
+		doWithConnection(connection -> {
+			assertThat(connection.keyCommands().ttl(this.binaryCacheKey)).isGreaterThan(1);
+		});
 	}
 
 	@ParameterizedRedisTest // GH-2650
@@ -689,7 +705,7 @@ private Object unwrap(@Nullable Object value) {
 	private Function<RedisCacheConfiguration, RedisCacheConfiguration> withTtiExpiration() {
 
 		Function<RedisCacheConfiguration, RedisCacheConfiguration> entryTtlFunction = cacheConfiguration -> cacheConfiguration
-				.entryTtl(Duration.ofMillis(100));
+				.entryTtl(Duration.ofSeconds(10));
 
 		return entryTtlFunction.andThen(RedisCacheConfiguration::enableTimeToIdle);
 	}