diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCache.java b/src/main/java/org/springframework/data/redis/cache/RedisCache.java index 11debd871..d2ca99549 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -281,13 +281,13 @@ public class RedisCache extends AbstractValueAdaptingCache { } @Override - public CompletableFuture retrieve(Object key) { + public CompletableFuture retrieve(Object key) { if (!getCacheWriter().supportsAsyncRetrieve()) { throw new UnsupportedOperationException(CACHE_RETRIEVAL_UNSUPPORTED_OPERATION_EXCEPTION_MESSAGE); } - return retrieveValue(key).thenApply(this::nullSafeDeserializedStoreValue); + return retrieveValue(key); } @Override @@ -298,10 +298,10 @@ public class RedisCache extends AbstractValueAdaptingCache { throw new UnsupportedOperationException(CACHE_RETRIEVAL_UNSUPPORTED_OPERATION_EXCEPTION_MESSAGE); } - return retrieveValue(key).thenCompose(bytes -> { + return retrieveValue(key).thenCompose(wrapper -> { - if (bytes != null) { - return CompletableFuture.completedFuture((T) nullSafeDeserializedStoreValue(bytes)); + if (wrapper != null) { + return CompletableFuture.completedFuture((T) wrapper.get()); } return valueLoader.get().thenCompose(value -> { @@ -313,8 +313,7 @@ public class RedisCache extends AbstractValueAdaptingCache { Duration timeToLive = getTimeToLive(key, cacheValue); - return getCacheWriter().store(getName(), binaryKey, binaryValue, timeToLive) - .thenApply(v -> value); + return getCacheWriter().store(getName(), binaryKey, binaryValue, timeToLive).thenApply(v -> value); }); }); } @@ -447,8 +446,10 @@ public class RedisCache extends AbstractValueAdaptingCache { throw new IllegalStateException(message); } - private CompletableFuture retrieveValue(Object key) { - return getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key)); + private CompletableFuture retrieveValue(Object key) { + return getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key)) // + .thenApply(binaryValue -> binaryValue != null ? deserializeCacheValue(binaryValue) : null) // + .thenApply(this::toValueWrapper); } @Nullable diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java index ce6514322..3a291eabe 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java @@ -15,10 +15,8 @@ */ package org.springframework.data.redis.cache; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; -import static org.awaitility.Awaitility.await; +import static org.assertj.core.api.Assertions.*; +import static org.awaitility.Awaitility.*; import io.netty.util.concurrent.DefaultThreadFactory; @@ -575,8 +573,7 @@ public class RedisCacheTests { void retrieveCacheValueUsingJedis() { assertThatExceptionOfType(UnsupportedOperationException.class) - .isThrownBy(() -> this.cache.retrieve(this.binaryCacheKey)) - .withMessageContaining("RedisCache"); + .isThrownBy(() -> this.cache.retrieve(this.binaryCacheKey)).withMessageContaining("RedisCache"); } @ParameterizedRedisTest // GH-2650 @@ -590,23 +587,51 @@ public class RedisCacheTests { @ParameterizedRedisTest // GH-2650 @EnabledOnRedisDriver(RedisDriver.LETTUCE) - @SuppressWarnings("unchecked") void retrieveReturnsCachedValue() throws Exception { doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample)); - RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration()); + RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), + usingRedisCacheConfiguration().disableCachingNullValues()); - CompletableFuture value = (CompletableFuture) cache.retrieve(this.key); + CompletableFuture value = cache.retrieve(this.key); assertThat(value).isNotNull(); - assertThat(value.get()).isEqualTo(this.sample); + assertThat(value.get(5, TimeUnit.SECONDS)).isNotNull(); + assertThat(value.get().get()).isEqualTo(this.sample); assertThat(value).isDone(); } @ParameterizedRedisTest // GH-2650 @EnabledOnRedisDriver(RedisDriver.LETTUCE) - @SuppressWarnings("unchecked") + void retrieveReturnsCachedNullableValue() throws Exception { + + doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample)); + + RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration()); + + CompletableFuture value = cache.retrieve(this.key); + + assertThat(value).isNotNull(); + assertThat(value.get().get()).isEqualTo(this.sample); + assertThat(value).isDone(); + } + + @ParameterizedRedisTest // GH-2783 + @EnabledOnRedisDriver(RedisDriver.LETTUCE) + void retrieveReturnsCachedNullValue() throws Exception { + + doWithConnection(connection -> connection.set(binaryCacheKey, binaryNullValue)); + + CompletableFuture value = (CompletableFuture) cache.retrieve(this.key); + ValueWrapper wrapper = value.get(5, TimeUnit.SECONDS); + + assertThat(wrapper).isNotNull(); + assertThat(wrapper.get()).isNull(); + } + + @ParameterizedRedisTest // GH-2650 + @EnabledOnRedisDriver(RedisDriver.LETTUCE) void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception { String testValue = "TestValue"; @@ -622,13 +647,12 @@ public class RedisCacheTests { cacheWriter.lock("cache"); - CompletableFuture value = (CompletableFuture) cache.retrieve(this.key); - + CompletableFuture value = cache.retrieve(this.key); assertThat(value).isNotDone(); cacheWriter.unlock("cache"); - assertThat(value.get(15L, TimeUnit.MILLISECONDS)).isEqualTo(testValue); + assertThat(value.get(15L, TimeUnit.MILLISECONDS).get()).isEqualTo(testValue); assertThat(value).isDone(); } @@ -665,8 +689,9 @@ public class RedisCacheTests { cache.retrieve(this.key, valueLoaderSupplier).get(); - doWithConnection(connection -> - assertThat(connection.keyCommands().exists("cache::key-1".getBytes(StandardCharsets.UTF_8))).isTrue()); + doWithConnection( + connection -> assertThat(connection.keyCommands().exists("cache::key-1".getBytes(StandardCharsets.UTF_8))) + .isTrue()); } @ParameterizedRedisTest // GH-2650 @@ -677,11 +702,18 @@ public class RedisCacheTests { RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration()); - CompletableFuture value = cache.retrieve(this.key); + CompletableFuture value = cache.retrieve(this.key); assertThat(value).isNotNull(); - assertThat(value.get()).isNull(); + assertThat(value.get(5, TimeUnit.SECONDS).get()).isNull(); assertThat(value).isDone(); + + doWithConnection(connection -> connection.keyCommands().del(this.binaryCacheKey)); + + value = cache.retrieve(this.key); + + assertThat(value).isNotNull(); + assertThat(value.get(5, TimeUnit.SECONDS)).isNull(); } private CompletableFuture usingCompletedFuture(T value) { diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java index 45d1747d0..2a8b92f19 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java @@ -15,20 +15,14 @@ */ package org.springframework.data.redis.cache; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.Test; +import org.springframework.cache.Cache.ValueWrapper; import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair; /** @@ -40,7 +34,6 @@ import org.springframework.data.redis.serializer.RedisSerializationContext.Seria class RedisCacheUnitTests { @Test // GH-2650 - @SuppressWarnings("unchecked") void cacheRetrieveValueCallsCacheWriterRetrieveCorrectly() throws Exception { RedisCacheWriter mockCacheWriter = mock(RedisCacheWriter.class); @@ -53,10 +46,10 @@ class RedisCacheUnitTests { RedisCache cache = new RedisCache("TestCache", mockCacheWriter, cacheConfiguration); - CompletableFuture value = (CompletableFuture) cache.retrieve("TestKey"); + CompletableFuture value = cache.retrieve("TestKey"); assertThat(value).isNotNull(); - assertThat(new String(value.get())).isEqualTo("TEST"); + assertThat(new String((byte[]) value.get().get())).isEqualTo("TEST"); verify(mockCacheWriter, times(1)).retrieve(eq("TestCache"), isA(byte[].class)); verify(mockCacheWriter).supportsAsyncRetrieve(); @@ -64,6 +57,6 @@ class RedisCacheUnitTests { } private CompletableFuture usingCompletedFuture(T value) { - return CompletableFuture.completedFuture(value); + return CompletableFuture.completedFuture(value); } }