Revised handling of allowNullValues for asynchronous retrieval

Includes revised cacheNames javadoc and equals/hashCode for SimpleValueWrapper.

See gh-31637

spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCache.java

@@ -140,7 +140,7 @@ public class CaffeineCache extends AbstractValueAdaptingCache {
 	public CompletableFuture<?> retrieve(Object key) {
 		CompletableFuture<?> result = getAsyncCache().getIfPresent(key);
 		if (result != null && isAllowNullValues()) {
-			result = result.handle((value, ex) -> fromStoreValue(value));
+			result = result.thenApply(this::toValueWrapper);
 		}
 		return result;
 	}

spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCacheManager.java

@@ -48,9 +48,10 @@ import org.springframework.util.ObjectUtils;
  * A {@link CaffeineSpec}-compliant expression value can also be applied
  * via the {@link #setCacheSpecification "cacheSpecification"} bean property.
  *
- * <p>Supports the {@link Cache#retrieve(Object)} and
+ * <p>Supports the asynchronous {@link Cache#retrieve(Object)} and
  * {@link Cache#retrieve(Object, Supplier)} operations through Caffeine's
- * {@link AsyncCache}, when configured via {@link #setAsyncCacheMode}.
+ * {@link AsyncCache}, when configured via {@link #setAsyncCacheMode},
+ * with early-determined cache misses.
  *
  * <p>Requires Caffeine 3.0 or higher, as of Spring Framework 6.1.
  *
@@ -198,6 +199,11 @@ public class CaffeineCacheManager implements CacheManager {
 	 * <p>By default, this cache manager builds regular native Caffeine caches.
 	 * To switch to async caches which can also be used through the synchronous API
 	 * but come with support for {@code Cache#retrieve}, set this flag to {@code true}.
+	 * <p>Note that while null values in the cache are tolerated in async cache mode,
+	 * the recommendation is to disallow null values through
+	 * {@link #setAllowNullValues setAllowNullValues(false)}. This makes the semantics
+	 * of CompletableFuture-based access simpler and optimizes retrieval performance
+	 * since a Caffeine-provided CompletableFuture handle does not have to get wrapped.
 	 * @since 6.1
 	 * @see Caffeine#buildAsync()
 	 * @see Cache#retrieve(Object)

spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheManagerTests.java

@@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.cache.Cache;
 import org.springframework.cache.CacheManager;
+import org.springframework.cache.support.SimpleValueWrapper;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -170,9 +171,9 @@ class CaffeineCacheManagerTests {
 		assertThat(cache1.get("key3", () -> (String) null)).isNull();
 		assertThat(cache1.get("key3", () -> (String) null)).isNull();
 
-		assertThat(cache1.retrieve("key1").join()).isEqualTo("value1");
-		assertThat(cache1.retrieve("key2").join()).isEqualTo(2);
-		assertThat(cache1.retrieve("key3").join()).isNull();
+		assertThat(cache1.retrieve("key1").join()).isEqualTo(new SimpleValueWrapper("value1"));
+		assertThat(cache1.retrieve("key2").join()).isEqualTo(new SimpleValueWrapper(2));
+		assertThat(cache1.retrieve("key3").join()).isEqualTo(new SimpleValueWrapper(null));
 		cache1.evict("key3");
 		assertThat(cache1.retrieve("key3")).isNull();
 		assertThat(cache1.retrieve("key3", () -> CompletableFuture.completedFuture("value3")).join())
@@ -184,6 +185,44 @@ class CaffeineCacheManagerTests {
 		assertThat(cache1.retrieve("key3", () -> CompletableFuture.completedFuture(null)).join()).isNull();
 	}
 
+	@Test
+	void asyncModeWithoutNullValues() {
+		CaffeineCacheManager cm = new CaffeineCacheManager();
+		cm.setAsyncCacheMode(true);
+		cm.setAllowNullValues(false);
+
+		Cache cache1 = cm.getCache("c1");
+		assertThat(cache1).isInstanceOf(CaffeineCache.class);
+		Cache cache1again = cm.getCache("c1");
+		assertThat(cache1).isSameAs(cache1again);
+		Cache cache2 = cm.getCache("c2");
+		assertThat(cache2).isInstanceOf(CaffeineCache.class);
+		Cache cache2again = cm.getCache("c2");
+		assertThat(cache2).isSameAs(cache2again);
+		Cache cache3 = cm.getCache("c3");
+		assertThat(cache3).isInstanceOf(CaffeineCache.class);
+		Cache cache3again = cm.getCache("c3");
+		assertThat(cache3).isSameAs(cache3again);
+
+		cache1.put("key1", "value1");
+		assertThat(cache1.get("key1").get()).isEqualTo("value1");
+		cache1.put("key2", 2);
+		assertThat(cache1.get("key2").get()).isEqualTo(2);
+		cache1.evict("key3");
+		assertThat(cache1.get("key3")).isNull();
+		assertThat(cache1.get("key3", () -> "value3")).isEqualTo("value3");
+		assertThat(cache1.get("key3", () -> "value3")).isEqualTo("value3");
+		cache1.evict("key3");
+
+		assertThat(cache1.retrieve("key1").join()).isEqualTo("value1");
+		assertThat(cache1.retrieve("key2").join()).isEqualTo(2);
+		assertThat(cache1.retrieve("key3")).isNull();
+		assertThat(cache1.retrieve("key3", () -> CompletableFuture.completedFuture("value3")).join())
+				.isEqualTo("value3");
+		assertThat(cache1.retrieve("key3", () -> CompletableFuture.completedFuture("value3")).join())
+				.isEqualTo("value3");
+	}
+
 	@Test
 	void changeCaffeineRecreateCache() {
 		CaffeineCacheManager cm = new CaffeineCacheManager("c1");

spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineReactiveCachingTests.java

@@ -20,7 +20,8 @@ import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicLong;
 
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
@@ -43,9 +44,10 @@ import static org.assertj.core.api.Assertions.assertThat;
  */
 public class CaffeineReactiveCachingTests {
 
-	@Test
-	void withCaffeineAsyncCache() {
-		ApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class, ReactiveCacheableService.class);
+	@ParameterizedTest
+	@ValueSource(classes = {AsyncCacheModeConfig.class, AsyncCacheModeConfig.class})
+	void cacheHitDetermination(Class<?> configClass) {
+		ApplicationContext ctx = new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class);
 		ReactiveCacheableService service = ctx.getBean(ReactiveCacheableService.class);
 
 		Object key = new Object();
@@ -128,12 +130,26 @@ public class CaffeineReactiveCachingTests {
 
 	@Configuration(proxyBeanMethods = false)
 	@EnableCaching
-	static class Config {
+	static class AsyncCacheModeConfig {
+
+		@Bean
+		CacheManager cacheManager() {
+			CaffeineCacheManager cm = new CaffeineCacheManager("first");
+			cm.setAsyncCacheMode(true);
+			return cm;
+		}
+	}
+
+
+	@Configuration(proxyBeanMethods = false)
+	@EnableCaching
+	static class AsyncCacheModeWithoutNullValuesConfig {
 
 		@Bean
 		CacheManager cacheManager() {
 			CaffeineCacheManager ccm = new CaffeineCacheManager("first");
 			ccm.setAsyncCacheMode(true);
+			ccm.setAllowNullValues(false);
 			return ccm;
 		}
 	}

spring-context/src/main/java/org/springframework/cache/Cache.java

@@ -116,21 +116,23 @@ public interface Cache {
 	 * <p>Can return {@code null} if the cache can immediately determine that
 	 * it contains no mapping for this key (e.g. through an in-memory key map).
 	 * Otherwise, the cached value will be returned in the {@link CompletableFuture},
-	 * with {@code null} indicating a late-determined cache miss (and a nested
-	 * {@link ValueWrapper} potentially indicating a nullable cached value).
+	 * with {@code null} indicating a late-determined cache miss. A nested
+	 * {@link ValueWrapper} potentially indicates a nullable cached value;
+	 * the cached value may also be represented as a plain element if null
+	 * values are not supported. Calling code needs to be prepared to handle
+	 * all those variants of the result returned by this method.
 	 * @param key the key whose associated value is to be returned
 	 * @return the value to which this cache maps the specified key, contained
 	 * within a {@link CompletableFuture} which may also be empty when a cache
 	 * miss has been late-determined. A straight {@code null} being returned
 	 * means that the cache immediately determined that it contains no mapping
 	 * for this key. A {@link ValueWrapper} contained within the
-	 * {@code CompletableFuture} can indicate a cached value that is potentially
+	 * {@code CompletableFuture} indicates a cached value that is potentially
 	 * {@code null}; this is sensible in a late-determined scenario where a regular
 	 * CompletableFuture-contained {@code null} indicates a cache miss. However,
-	 * an early-determined cache will usually return the plain cached value here,
-	 * and a late-determined cache may also return a plain value if it does not
-	 * support the actual caching of {@code null} values. Spring's common cache
-	 * processing can deal with all variants of these implementation strategies.
+	 * a cache may also return a plain value if it does not support the actual
+	 * caching of {@code null} values, avoiding the extra level of value wrapping.
+	 * Spring's cache processing can deal with all such implementation strategies.
 	 * @since 6.1
 	 * @see #retrieve(Object, Supplier)
 	 */
@@ -149,11 +151,14 @@ public interface Cache {
 	 * <p>If possible, implementations should ensure that the loading operation
 	 * is synchronized so that the specified {@code valueLoader} is only called
 	 * once in case of concurrent access on the same key.
-	 * <p>If the {@code valueLoader} throws an exception, it will be propagated
+	 * <p>Null values are generally not supported by this method. The provided
+	 * {@link CompletableFuture} handle produces a value or raises an exception.
+	 * If the {@code valueLoader} raises an exception, it will be propagated
 	 * to the {@code CompletableFuture} handle returned from here.
 	 * @param key the key whose associated value is to be returned
-	 * @return the value to which this cache maps the specified key,
-	 * contained within a {@link CompletableFuture}
+	 * @return the value to which this cache maps the specified key, contained
+	 * within a {@link CompletableFuture} which will never be {@code null}.
+	 * The provided future is expected to produce a value or raise an exception.
 	 * @since 6.1
 	 * @see #retrieve(Object)
 	 * @see #get(Object, Callable)

spring-context/src/main/java/org/springframework/cache/annotation/CacheConfig.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,6 +32,7 @@ import java.lang.annotation.Target;
  * @author Stephane Nicoll
  * @author Sam Brannen
  * @since 4.1
+ * @see Cacheable
  */
 @Target(ElementType.TYPE)
 @Retention(RetentionPolicy.RUNTIME)
@@ -42,8 +43,10 @@ public @interface CacheConfig {
 	 * Names of the default caches to consider for caching operations defined
 	 * in the annotated class.
 	 * <p>If none is set at the operation level, these are used instead of the default.
-	 * <p>May be used to determine the target cache (or caches), matching the
-	 * qualifier value or the bean names of a specific bean definition.
+	 * <p>Names may be used to determine the target cache(s), to be resolved via the
+	 * configured {@link #cacheResolver()} which typically delegates to
+	 * {@link org.springframework.cache.CacheManager#getCache}.
+	 * For further details see {@link Cacheable#cacheNames()}.
 	 */
 	String[] cacheNames() default {};
 

spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java

@@ -70,8 +70,17 @@ public @interface Cacheable {
 
 	/**
 	 * Names of the caches in which method invocation results are stored.
-	 * <p>Names may be used to determine the target cache (or caches), matching
-	 * the qualifier value or bean name of a specific bean definition.
+	 * <p>Names may be used to determine the target cache(s), to be resolved via the
+	 * configured {@link #cacheResolver()} which typically delegates to
+	 * {@link org.springframework.cache.CacheManager#getCache}.
+	 * <p>This will usually be a single cache name. If multiple names are specified,
+	 * they will be consulted for a cache hit in the order of definition, and they
+	 * will all receive a put/evict request for the same newly cached value.
+	 * <p>Note that asynchronous/reactive cache access may not fully consult all
+	 * specified caches, depending on the target cache. In the case of late-determined
+	 * cache misses (e.g. with Redis), further caches will not get consulted anymore.
+	 * As a consequence, specifying multiple cache names in an async cache mode setup
+	 * only makes sense with early-determined cache misses (e.g. with Caffeine).
 	 * @since 4.2
 	 * @see #value
 	 * @see CacheConfig#cacheNames

spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java

@@ -160,7 +160,8 @@ public class ConcurrentMapCache extends AbstractValueAdaptingCache {
 	@Nullable
 	public CompletableFuture<?> retrieve(Object key) {
 		Object value = lookup(key);
-		return (value != null ? CompletableFuture.completedFuture(fromStoreValue(value)) : null);
+		return (value != null ? CompletableFuture.completedFuture(
+				isAllowNullValues() ? toValueWrapper(value) : fromStoreValue(value)) : null);
 	}
 
 	@SuppressWarnings("unchecked")

spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCacheManager.java

@@ -22,6 +22,7 @@ import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.function.Supplier;
 
 import org.springframework.beans.factory.BeanClassLoaderAware;
 import org.springframework.cache.Cache;
@@ -35,11 +36,15 @@ import org.springframework.lang.Nullable;
  * the set of cache names is pre-defined through {@link #setCacheNames}, with no
  * dynamic creation of further cache regions at runtime.
  *
+ * <p>Supports the asynchronous {@link Cache#retrieve(Object)} and
+ * {@link Cache#retrieve(Object, Supplier)} operations through basic
+ * {@code CompletableFuture} adaptation, with early-determined cache misses.
+ *
  * <p>Note: This is by no means a sophisticated CacheManager; it comes with no
  * cache configuration options. However, it may be useful for testing or simple
  * caching scenarios. For advanced local caching needs, consider
- * {@link org.springframework.cache.jcache.JCacheCacheManager} or
- * {@link org.springframework.cache.caffeine.CaffeineCacheManager}.
+ * {@link org.springframework.cache.caffeine.CaffeineCacheManager} or
+ * {@link org.springframework.cache.jcache.JCacheCacheManager}.
  *
  * @author Juergen Hoeller
  * @since 3.1

spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

@@ -508,7 +508,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
 
 		if (cacheHit != null && !hasCachePut(contexts)) {
 			// If there are no put requests, just use the cache hit
-			cacheValue = (cacheHit instanceof Cache.ValueWrapper wrapper ? wrapper.get() : cacheHit);
+			cacheValue = unwrapCacheValue(cacheHit);
 			returnValue = wrapCacheValue(method, cacheValue);
 		}
 		else {

spring-context/src/main/java/org/springframework/cache/support/SimpleValueWrapper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
 
 package org.springframework.cache.support;
 
+import java.util.Objects;
+
 import org.springframework.cache.Cache.ValueWrapper;
 import org.springframework.lang.Nullable;
 
@@ -50,4 +52,19 @@ public class SimpleValueWrapper implements ValueWrapper {
 		return this.value;
 	}
 
+	@Override
+	public boolean equals(@Nullable Object other) {
+		return (this == other || (other instanceof ValueWrapper wrapper && Objects.equals(get(), wrapper.get())));
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hashCode(this.value);
+	}
+
+	@Override
+	public String toString() {
+		return "ValueWrapper for [" + this.value + "]";
+	}
+
 }

spring-context/src/test/java/org/springframework/cache/annotation/ReactiveCachingTests.java

@@ -29,7 +29,6 @@ import org.springframework.cache.Cache;
 import org.springframework.cache.CacheManager;
 import org.springframework.cache.concurrent.ConcurrentMapCache;
 import org.springframework.cache.concurrent.ConcurrentMapCacheManager;
-import org.springframework.cache.support.SimpleValueWrapper;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -48,6 +47,7 @@ public class ReactiveCachingTests {
 
 	@ParameterizedTest
 	@ValueSource(classes = {EarlyCacheHitDeterminationConfig.class,
+			EarlyCacheHitDeterminationWithoutNullValuesConfig.class,
 			LateCacheHitDeterminationConfig.class,
 			LateCacheHitDeterminationWithValueWrapperConfig.class})
 	void cacheHitDetermination(Class<?> configClass) {
@@ -143,6 +143,19 @@ public class ReactiveCachingTests {
 	}
 
 
+	@Configuration(proxyBeanMethods = false)
+	@EnableCaching
+	static class EarlyCacheHitDeterminationWithoutNullValuesConfig {
+
+		@Bean
+		CacheManager cacheManager() {
+			ConcurrentMapCacheManager cm = new ConcurrentMapCacheManager("first");
+			cm.setAllowNullValues(false);
+			return cm;
+		}
+	}
+
+
 	@Configuration(proxyBeanMethods = false)
 	@EnableCaching
 	static class LateCacheHitDeterminationConfig {
@@ -177,12 +190,7 @@ public class ReactiveCachingTests {
 						@Override
 						public CompletableFuture<?> retrieve(Object key) {
 							Object value = lookup(key);
-							if (value != null) {
-								return CompletableFuture.completedFuture(new SimpleValueWrapper(fromStoreValue(value)));
-							}
-							else {
-								return CompletableFuture.completedFuture(null);
-							}
+							return CompletableFuture.completedFuture(value != null ? toValueWrapper(value) : null);
 						}
 					};
 				}