Skip to content

Commit f9a660c

Browse files
mp911dechristophstrobl
mp911de
authored and
christophstrobl
committed
Expose ValueWrapper to differentiate between cached null values and absent cache mapping.
We now use ValueWrapper to differentiate in the async API between cache misses and cached null values. Closes: #2783 Original Pull Request: #2785
1 parent f31a604 commit f9a660c

File tree

3 files changed

+66
-40
lines changed

3 files changed

+66
-40
lines changed

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,13 @@ public void evict(Object key) {
281281
}
282282

283283
@Override
284-
public CompletableFuture<?> retrieve(Object key) {
284+
public CompletableFuture<ValueWrapper> retrieve(Object key) {
285285

286286
if (!getCacheWriter().supportsAsyncRetrieve()) {
287287
throw new UnsupportedOperationException(CACHE_RETRIEVAL_UNSUPPORTED_OPERATION_EXCEPTION_MESSAGE);
288288
}
289289

290-
return retrieveValue(key).thenApply(this::nullSafeDeserializedStoreValue);
290+
return retrieveValue(key);
291291
}
292292

293293
@Override
@@ -298,10 +298,10 @@ public <T> CompletableFuture<T> retrieve(Object key, Supplier<CompletableFuture<
298298
throw new UnsupportedOperationException(CACHE_RETRIEVAL_UNSUPPORTED_OPERATION_EXCEPTION_MESSAGE);
299299
}
300300

301-
return retrieveValue(key).thenCompose(bytes -> {
301+
return retrieveValue(key).thenCompose(wrapper -> {
302302

303-
if (bytes != null) {
304-
return CompletableFuture.completedFuture((T) nullSafeDeserializedStoreValue(bytes));
303+
if (wrapper != null) {
304+
return CompletableFuture.completedFuture((T) wrapper.get());
305305
}
306306

307307
return valueLoader.get().thenCompose(value -> {
@@ -313,8 +313,7 @@ public <T> CompletableFuture<T> retrieve(Object key, Supplier<CompletableFuture<
313313

314314
Duration timeToLive = getTimeToLive(key, cacheValue);
315315

316-
return getCacheWriter().store(getName(), binaryKey, binaryValue, timeToLive)
317-
.thenApply(v -> value);
316+
return getCacheWriter().store(getName(), binaryKey, binaryValue, timeToLive).thenApply(v -> value);
318317
});
319318
});
320319
}
@@ -447,8 +446,10 @@ protected String convertKey(Object key) {
447446
throw new IllegalStateException(message);
448447
}
449448

450-
private CompletableFuture<byte[]> retrieveValue(Object key) {
451-
return getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key));
449+
private CompletableFuture<ValueWrapper> retrieveValue(Object key) {
450+
return getCacheWriter().retrieve(getName(), createAndConvertCacheKey(key)) //
451+
.thenApply(binaryValue -> binaryValue != null ? deserializeCacheValue(binaryValue) : null) //
452+
.thenApply(this::toValueWrapper);
452453
}
453454

454455
@Nullable

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

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@
1515
*/
1616
package org.springframework.data.redis.cache;
1717

18-
import static org.assertj.core.api.Assertions.assertThat;
19-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
20-
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
21-
import static org.awaitility.Awaitility.await;
18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.awaitility.Awaitility.*;
2220

2321
import io.netty.util.concurrent.DefaultThreadFactory;
2422

@@ -575,8 +573,7 @@ void cacheGetWithTimeToIdleExpirationAfterEntryExpiresShouldReturnNull() {
575573
void retrieveCacheValueUsingJedis() {
576574

577575
assertThatExceptionOfType(UnsupportedOperationException.class)
578-
.isThrownBy(() -> this.cache.retrieve(this.binaryCacheKey))
579-
.withMessageContaining("RedisCache");
576+
.isThrownBy(() -> this.cache.retrieve(this.binaryCacheKey)).withMessageContaining("RedisCache");
580577
}
581578

582579
@ParameterizedRedisTest // GH-2650
@@ -590,23 +587,51 @@ void retrieveLoadedValueUsingJedis() {
590587

591588
@ParameterizedRedisTest // GH-2650
592589
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
593-
@SuppressWarnings("unchecked")
594590
void retrieveReturnsCachedValue() throws Exception {
595591

596592
doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample));
597593

594+
RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(),
595+
usingRedisCacheConfiguration().disableCachingNullValues());
596+
597+
CompletableFuture<ValueWrapper> value = cache.retrieve(this.key);
598+
599+
assertThat(value).isNotNull();
600+
assertThat(value.get(5, TimeUnit.SECONDS)).isNotNull();
601+
assertThat(value.get().get()).isEqualTo(this.sample);
602+
assertThat(value).isDone();
603+
}
604+
605+
@ParameterizedRedisTest // GH-2650
606+
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
607+
void retrieveReturnsCachedNullableValue() throws Exception {
608+
609+
doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample));
610+
598611
RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration());
599612

600-
CompletableFuture<Person> value = (CompletableFuture<Person>) cache.retrieve(this.key);
613+
CompletableFuture<ValueWrapper> value = cache.retrieve(this.key);
601614

602615
assertThat(value).isNotNull();
603-
assertThat(value.get()).isEqualTo(this.sample);
616+
assertThat(value.get().get()).isEqualTo(this.sample);
604617
assertThat(value).isDone();
605618
}
606619

620+
@ParameterizedRedisTest // GH-2783
621+
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
622+
void retrieveReturnsCachedNullValue() throws Exception {
623+
624+
doWithConnection(connection -> connection.set(binaryCacheKey, binaryNullValue));
625+
626+
CompletableFuture<ValueWrapper> value = (CompletableFuture<ValueWrapper>) cache.retrieve(this.key);
627+
ValueWrapper wrapper = value.get(5, TimeUnit.SECONDS);
628+
629+
assertThat(wrapper).isNotNull();
630+
assertThat(wrapper.get()).isNull();
631+
}
632+
607633
@ParameterizedRedisTest // GH-2650
608634
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
609-
@SuppressWarnings("unchecked")
610635
void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception {
611636

612637
String testValue = "TestValue";
@@ -622,13 +647,12 @@ void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception {
622647

623648
cacheWriter.lock("cache");
624649

625-
CompletableFuture<String> value = (CompletableFuture<String>) cache.retrieve(this.key);
626-
650+
CompletableFuture<ValueWrapper> value = cache.retrieve(this.key);
627651
assertThat(value).isNotDone();
628652

629653
cacheWriter.unlock("cache");
630654

631-
assertThat(value.get(15L, TimeUnit.MILLISECONDS)).isEqualTo(testValue);
655+
assertThat(value.get(15L, TimeUnit.MILLISECONDS).get()).isEqualTo(testValue);
632656
assertThat(value).isDone();
633657
}
634658

@@ -665,8 +689,9 @@ void retrieveStoresLoadedValue() throws Exception {
665689

666690
cache.retrieve(this.key, valueLoaderSupplier).get();
667691

668-
doWithConnection(connection ->
669-
assertThat(connection.keyCommands().exists("cache::key-1".getBytes(StandardCharsets.UTF_8))).isTrue());
692+
doWithConnection(
693+
connection -> assertThat(connection.keyCommands().exists("cache::key-1".getBytes(StandardCharsets.UTF_8)))
694+
.isTrue());
670695
}
671696

672697
@ParameterizedRedisTest // GH-2650
@@ -677,11 +702,18 @@ void retrieveReturnsNull() throws Exception {
677702

678703
RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration());
679704

680-
CompletableFuture<?> value = cache.retrieve(this.key);
705+
CompletableFuture<ValueWrapper> value = cache.retrieve(this.key);
681706

682707
assertThat(value).isNotNull();
683-
assertThat(value.get()).isNull();
708+
assertThat(value.get(5, TimeUnit.SECONDS).get()).isNull();
684709
assertThat(value).isDone();
710+
711+
doWithConnection(connection -> connection.keyCommands().del(this.binaryCacheKey));
712+
713+
value = cache.retrieve(this.key);
714+
715+
assertThat(value).isNotNull();
716+
assertThat(value.get(5, TimeUnit.SECONDS)).isNull();
685717
}
686718

687719
private <T> CompletableFuture<T> usingCompletedFuture(T value) {

‎src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,14 @@
1515
*/
1616
package org.springframework.data.redis.cache;
1717

18-
import static org.assertj.core.api.Assertions.assertThat;
19-
import static org.mockito.ArgumentMatchers.any;
20-
import static org.mockito.ArgumentMatchers.anyString;
21-
import static org.mockito.ArgumentMatchers.eq;
22-
import static org.mockito.ArgumentMatchers.isA;
23-
import static org.mockito.Mockito.doReturn;
24-
import static org.mockito.Mockito.mock;
25-
import static org.mockito.Mockito.times;
26-
import static org.mockito.Mockito.verify;
27-
import static org.mockito.Mockito.verifyNoMoreInteractions;
18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.mockito.ArgumentMatchers.*;
20+
import static org.mockito.Mockito.*;
2821

2922
import java.util.concurrent.CompletableFuture;
3023

3124
import org.junit.jupiter.api.Test;
25+
import org.springframework.cache.Cache.ValueWrapper;
3226
import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair;
3327

3428
/**
@@ -40,7 +34,6 @@
4034
class RedisCacheUnitTests {
4135

4236
@Test // GH-2650
43-
@SuppressWarnings("unchecked")
4437
void cacheRetrieveValueCallsCacheWriterRetrieveCorrectly() throws Exception {
4538

4639
RedisCacheWriter mockCacheWriter = mock(RedisCacheWriter.class);
@@ -53,17 +46,17 @@ void cacheRetrieveValueCallsCacheWriterRetrieveCorrectly() throws Exception {
5346

5447
RedisCache cache = new RedisCache("TestCache", mockCacheWriter, cacheConfiguration);
5548

56-
CompletableFuture<byte[]> value = (CompletableFuture<byte[]>) cache.retrieve("TestKey");
49+
CompletableFuture<ValueWrapper> value = cache.retrieve("TestKey");
5750

5851
assertThat(value).isNotNull();
59-
assertThat(new String(value.get())).isEqualTo("TEST");
52+
assertThat(new String((byte[]) value.get().get())).isEqualTo("TEST");
6053

6154
verify(mockCacheWriter, times(1)).retrieve(eq("TestCache"), isA(byte[].class));
6255
verify(mockCacheWriter).supportsAsyncRetrieve();
6356
verifyNoMoreInteractions(mockCacheWriter);
6457
}
6558

6659
private <T> CompletableFuture<T> usingCompletedFuture(T value) {
67-
return CompletableFuture.completedFuture(value);
60+
return CompletableFuture.completedFuture(value);
6861
}
6962
}

0 commit comments

Comments
 (0)