diff --git a/pom.xml b/pom.xml index da45839e46..87bc0755eb 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 3.3.0-SNAPSHOT + 3.3.0-GH-2783-SNAPSHOT Spring Data Redis Spring Data module for Redis @@ -29,6 +29,7 @@ 1.01 4.1.100.Final spring.data.redis + 6.1.1-SNAPSHOT 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 11debd8710..38989f2097 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -62,8 +62,7 @@ public class RedisCache extends AbstractValueAdaptingCache { static final byte[] BINARY_NULL_VALUE = RedisSerializer.java().serialize(NullValue.INSTANCE); - static final String CACHE_RETRIEVAL_UNSUPPORTED_OPERATION_EXCEPTION_MESSAGE = - "The Redis driver configured with RedisCache through RedisCacheWriter does not support CompletableFuture-based retrieval"; + static final String CACHE_RETRIEVAL_UNSUPPORTED_OPERATION_EXCEPTION_MESSAGE = "The Redis driver configured with RedisCache through RedisCacheWriter does not support CompletableFuture-based retrieval"; private final Lock lock = new ReentrantLock(); @@ -74,16 +73,16 @@ public class RedisCache extends AbstractValueAdaptingCache { private final 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. + * 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 executing the necessary Redis commands; must not be {@literal null}. - * @param cacheConfiguration {@link RedisCacheConfiguration} applied to this {@link RedisCache} on creation; - * must not be {@literal null}. + * @param cacheWriter {@link RedisCacheWriter} used to perform {@link RedisCache} operations by executing the + * necessary Redis commands; must not be {@literal null}. + * @param cacheConfiguration {@link RedisCacheConfiguration} applied to this {@link RedisCache} on creation; must not + * be {@literal null}. * @throws IllegalArgumentException if either the given {@link RedisCacheWriter} or {@link RedisCacheConfiguration} - * are {@literal null} or the given {@link String} name for this {@link RedisCache} is {@literal null}. + * are {@literal null} or the given {@link String} name for this {@link RedisCache} is {@literal null}. */ protected RedisCache(String name, RedisCacheWriter cacheWriter, RedisCacheConfiguration cacheConfiguration) { @@ -117,11 +116,11 @@ protected RedisCacheWriter getCacheWriter() { } /** - * Gets the configured {@link ConversionService} used to convert {@link Object cache keys} to a {@link String} - * when accessing entries in the cache. + * Gets the configured {@link ConversionService} used to convert {@link Object cache keys} to a {@link String} when + * 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. + * @return the configured {@link ConversionService} used to convert {@link Object cache keys} to a {@link String} when + * accessing entries in the cache. * @see RedisCacheConfiguration#getConversionService() * @see #getCacheConfiguration() */ @@ -175,8 +174,8 @@ private T getSynchronized(Object key, Callable valueLoader) { } /** - * Loads the {@link Object} using the given {@link Callable valueLoader} and {@link #put(Object, Object) puts} - * the {@link Object loaded value} in the cache. + * Loads the {@link Object} using the given {@link Callable valueLoader} and {@link #put(Object, Object) puts} the + * {@link Object loaded value} in the cache. * * @param {@link Class type} of the loaded {@link Object cache value}. * @param key {@link Object key} mapped to the loaded {@link Object cache value}. @@ -281,13 +280,13 @@ public void evict(Object key) { } @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 +297,10 @@ public CompletableFuture retrieve(Object key, Supplier { + 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 +312,7 @@ public CompletableFuture retrieve(Object key, Supplier value); + return getCacheWriter().store(getName(), binaryKey, binaryValue, timeToLive).thenApply(v -> value); }); }); } @@ -440,15 +438,18 @@ 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); } - 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 ebf1b5d8a1..1c4cb65595 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 @@ void cacheGetWithTimeToIdleExpirationAfterEntryExpiresShouldReturnNull() { 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 @@ void retrieveLoadedValueUsingJedis() { @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().disableCachingNullValues()); + + CompletableFuture value = cache.retrieve(this.key); + + assertThat(value).isNotNull(); + assertThat(value.get(5, TimeUnit.SECONDS)).isNotNull(); + assertThat(value.get().get()).isEqualTo(this.sample); + assertThat(value).isDone(); + } + + @ParameterizedRedisTest // GH-2650 + @EnabledOnRedisDriver(RedisDriver.LETTUCE) + void retrieveReturnsCachedNullableValue() throws Exception { + + doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample)); + RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration()); - 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().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) - @SuppressWarnings("unchecked") void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception { String testValue = "TestValue"; @@ -622,13 +647,12 @@ void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception { 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(); } @@ -666,8 +690,9 @@ void retrieveStoresLoadedValue() throws Exception { 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 @@ -678,11 +703,18 @@ void retrieveReturnsNull() throws Exception { 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 45d1747d02..2a8b92f196 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 @@ class RedisCacheUnitTests { @Test // GH-2650 - @SuppressWarnings("unchecked") void cacheRetrieveValueCallsCacheWriterRetrieveCorrectly() throws Exception { RedisCacheWriter mockCacheWriter = mock(RedisCacheWriter.class); @@ -53,10 +46,10 @@ void cacheRetrieveValueCallsCacheWriterRetrieveCorrectly() throws Exception { 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 @@ void cacheRetrieveValueCallsCacheWriterRetrieveCorrectly() throws Exception { } private CompletableFuture usingCompletedFuture(T value) { - return CompletableFuture.completedFuture(value); + return CompletableFuture.completedFuture(value); } }