From 461f020e3814f4db804c66bb99f176d3976a0540 Mon Sep 17 00:00:00 2001 From: Chris Povirk Date: Thu, 5 Dec 2024 09:27:57 -0500 Subject: [PATCH] Allow caches to have nullable type arguments for their value types. This does not mean that the cache will "contain" `null`, since `null` is never cached. However, it does allow users to declare whether they want "a cache whose loads might return `null`" (which thus can return `null` from cache operations) or "a cache whose loads will never return `null`" (which thus doesn't require users to handle `null` when they request that the cache return or load a value). This PR follows up on https://github.com/ben-manes/caffeine/pull/1805. Ideally I will come back to write some Kotlin tests for this. For the moment, I can only say that the results look good in my testing inside Google. --- .../benmanes/caffeine/cache/AsyncCache.java | 60 +++++++++---------- .../caffeine/cache/AsyncCacheLoader.java | 11 ++-- .../caffeine/cache/AsyncLoadingCache.java | 9 ++- .../github/benmanes/caffeine/cache/Cache.java | 36 +++++------ .../benmanes/caffeine/cache/CacheLoader.java | 16 ++--- .../benmanes/caffeine/cache/Caffeine.java | 10 ++-- .../benmanes/caffeine/cache/LoadingCache.java | 14 ++--- 7 files changed, 75 insertions(+), 81 deletions(-) diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCache.java index 90ae3d7c4e..7731c07633 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCache.java @@ -25,7 +25,6 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.NullUnmarked; import org.jspecify.annotations.Nullable; /** @@ -41,7 +40,7 @@ * @param the type of mapped values */ @NullMarked -public interface AsyncCache { +public interface AsyncCache { /** * Returns the future associated with the {@code key} in this cache, or {@code null} if there is @@ -49,11 +48,10 @@ public interface AsyncCache { * * @param key the key whose associated value is to be returned * @return the future value to which the specified key is mapped, or {@code null} if this cache - * does not contain a mapping for the key + * does not contain a mapping for the key * @throws NullPointerException if the specified key is null */ - @Nullable - CompletableFuture getIfPresent(K key); + @Nullable CompletableFuture<@NonNull V> getIfPresent(K key); /** * Returns the future associated with the {@code key} in this cache, obtaining that value from @@ -73,9 +71,7 @@ public interface AsyncCache { * @return the current (existing or computed) future value associated with the specified key * @throws NullPointerException if the specified key or mappingFunction is null */ - @NullUnmarked - @NonNull CompletableFuture get( - @NonNull K key, @NonNull Function mappingFunction); + CompletableFuture get(K key, Function mappingFunction); /** * Returns the future associated with the {@code key} in this cache, obtaining that value from @@ -93,21 +89,16 @@ public interface AsyncCache { * * @param key the key with which the specified value is to be associated * @param mappingFunction the function to asynchronously compute a value, optionally using the - * given executor + * given executor * @return the current (existing or computed) future value associated with the specified key * @throws NullPointerException if the specified key or mappingFunction is null, or if the * future returned by the mappingFunction is null * @throws RuntimeException or Error if the mappingFunction does when constructing the future, * in which case the mapping is left unestablished */ - @NullUnmarked - @NonNull CompletableFuture get( - @NonNull K key, - @NonNull - BiFunction< - ? super @NonNull K, - ? super @NonNull Executor, - ? extends @NonNull CompletableFuture> + CompletableFuture get( + K key, + BiFunction> mappingFunction); /** @@ -129,14 +120,16 @@ public interface AsyncCache { * @param keys the keys whose associated values are to be returned * @param mappingFunction the function to asynchronously compute the values * @return a future containing an unmodifiable mapping of keys to values for the specified keys in - * this cache + * this cache * @throws NullPointerException if the specified collection is null or contains a null element, or - * if the future returned by the mappingFunction is null + * if the future returned by the mappingFunction is null * @throws RuntimeException or Error if the mappingFunction does so, in which case the mapping is - * left unestablished + * left unestablished */ - CompletableFuture> getAll(Iterable keys, - Function, ? extends Map> mappingFunction); + CompletableFuture> getAll( + Iterable keys, + Function, ? extends Map> + mappingFunction); /** * Returns the future of a map of the values associated with the {@code keys}, creating or @@ -157,17 +150,21 @@ CompletableFuture> getAll(Iterable keys, * * @param keys the keys whose associated values are to be returned * @param mappingFunction the function to asynchronously compute the values, optionally using the - * given executor + * given executor * @return a future containing an unmodifiable mapping of keys to values for the specified keys in - * this cache + * this cache * @throws NullPointerException if the specified collection is null or contains a null element, or - * if the future returned by the mappingFunction is null + * if the future returned by the mappingFunction is null * @throws RuntimeException or Error if the mappingFunction does so, in which case the mapping is - * left unestablished + * left unestablished */ - CompletableFuture> getAll(Iterable keys, - BiFunction, ? super Executor, - ? extends CompletableFuture>> mappingFunction); + CompletableFuture> getAll( + Iterable keys, + BiFunction< + ? super Set, + ? super Executor, + ? extends CompletableFuture>> + mappingFunction); /** * Associates {@code value} with {@code key} in this cache. If the cache previously contained a @@ -181,7 +178,7 @@ CompletableFuture> getAll(Iterable keys, * @param valueFuture the value to be associated with the specified key * @throws NullPointerException if the specified key or value is null */ - void put(K key, CompletableFuture valueFuture); + void put(K key, CompletableFuture valueFuture); /** * Returns a view of the entries stored in this cache as a thread-safe map. Modifications made to @@ -198,8 +195,7 @@ CompletableFuture> getAll(Iterable keys, * * @return a thread-safe view of this cache supporting all of the optional {@link Map} operations */ - @NullUnmarked - @NonNull ConcurrentMap<@NonNull K, @NonNull CompletableFuture> asMap(); + ConcurrentMap> asMap(); /** * Returns a view of the entries stored in this cache as a synchronous {@link Cache}. A mapping is diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java index 8abff8d7e1..d2ac373ad1 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java @@ -24,6 +24,7 @@ import java.util.function.BiFunction; import java.util.function.Function; +import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -48,7 +49,7 @@ @NullMarked @FunctionalInterface @SuppressWarnings("PMD.SignatureDeclareThrowsException") -public interface AsyncCacheLoader { +public interface AsyncCacheLoader { /** * Asynchronously computes or retrieves the value corresponding to {@code key}. @@ -63,7 +64,7 @@ public interface AsyncCacheLoader { * treated like any other {@code Exception} in all respects except that, when it is * caught, the thread's interrupt status is set */ - CompletableFuture asyncLoad(K key, Executor executor) throws Exception; + CompletableFuture asyncLoad(K key, Executor executor) throws Exception; /** * Asynchronously computes or retrieves the values corresponding to {@code keys}. This method is @@ -89,7 +90,7 @@ public interface AsyncCacheLoader { * treated like any other {@code Exception} in all respects except that, when it is * caught, the thread's interrupt status is set */ - default CompletableFuture> asyncLoadAll( + default CompletableFuture> asyncLoadAll( Set keys, Executor executor) throws Exception { throw new UnsupportedOperationException(); } @@ -115,8 +116,8 @@ public interface AsyncCacheLoader { * treated like any other {@code Exception} in all respects except that, when it is * caught, the thread's interrupt status is set */ - default CompletableFuture asyncReload( - K key, V oldValue, Executor executor) throws Exception { + default CompletableFuture asyncReload( + K key, @NonNull V oldValue, Executor executor) throws Exception { return asyncLoad(key, executor); } diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncLoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncLoadingCache.java index f714203e92..1e54ec0751 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncLoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncLoadingCache.java @@ -20,7 +20,7 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.NullUnmarked; +import org.jspecify.annotations.Nullable; /** * A semi-persistent mapping from keys to values. Values are automatically loaded by the cache @@ -34,7 +34,7 @@ * @param the type of mapped values */ @NullMarked -public interface AsyncLoadingCache extends AsyncCache { +public interface AsyncLoadingCache extends AsyncCache { /** * Returns the future associated with the {@code key} in this cache, obtaining that value from @@ -52,8 +52,7 @@ public interface AsyncLoadingCache extends AsyncCache { * @throws RuntimeException or Error if the {@link AsyncCacheLoader} does when constructing the * future, in which case the mapping is left unestablished */ - @NullUnmarked - @NonNull CompletableFuture get(@NonNull K key); + CompletableFuture get(K key); /** * Returns the future of a map of the values associated with {@code keys}, creating or retrieving @@ -81,7 +80,7 @@ public interface AsyncLoadingCache extends AsyncCache { * {@link AsyncCacheLoader#asyncLoadAll} returns {@code null}, or fails when constructing * the future, in which case the mapping is left unestablished */ - CompletableFuture> getAll(Iterable keys); + CompletableFuture> getAll(Iterable keys); /** * Returns a view of the entries stored in this cache as a synchronous {@link LoadingCache}. A diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java index 8f825ed90a..2e2dd682a3 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java @@ -22,7 +22,6 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.NullUnmarked; import org.jspecify.annotations.Nullable; import com.github.benmanes.caffeine.cache.stats.CacheStats; @@ -40,7 +39,7 @@ * @param the type of mapped values */ @NullMarked -public interface Cache { +public interface Cache { /** * Returns the value associated with the {@code key} in this cache, or {@code null} if there is no @@ -48,7 +47,7 @@ public interface Cache { * * @param key the key whose associated value is to be returned * @return the value to which the specified key is mapped, or {@code null} if this cache does not - * contain a mapping for the key + * contain a mapping for the key * @throws NullPointerException if the specified key is null */ @Nullable @@ -72,15 +71,14 @@ public interface Cache { * @param key the key with which the specified value is to be associated * @param mappingFunction the function to compute a value * @return the current (existing or computed) value associated with the specified key, or null if - * the computed value is null + * the computed value is null * @throws NullPointerException if the specified key or mappingFunction is null * @throws IllegalStateException if the computation detectably attempts a recursive update to this - * cache that would otherwise never complete + * cache that would otherwise never complete * @throws RuntimeException or Error if the mappingFunction does so, in which case the mapping is - * left unestablished + * left unestablished */ - @NullUnmarked - V get(@NonNull K key, Function mappingFunction); + V get(K key, Function mappingFunction); /** * Returns a map of the values associated with the {@code keys} in this cache. The returned map @@ -93,7 +91,7 @@ public interface Cache { * @return an unmodifiable mapping of keys to values for the specified keys found in this cache * @throws NullPointerException if the specified collection is null or contains a null element */ - Map getAllPresent(Iterable keys); + Map getAllPresent(Iterable keys); /** * Returns a map of the values associated with the {@code keys}, creating or retrieving those @@ -117,12 +115,14 @@ public interface Cache { * @param mappingFunction the function to compute the values * @return an unmodifiable mapping of keys to values for the specified keys in this cache * @throws NullPointerException if the specified collection is null or contains a null element, or - * if the map returned by the mappingFunction is null + * if the map returned by the mappingFunction is null * @throws RuntimeException or Error if the mappingFunction does so, in which case the mapping is - * left unestablished + * left unestablished */ - Map getAll(Iterable keys, - Function, ? extends Map> mappingFunction); + Map getAll( + Iterable keys, + Function, ? extends Map> + mappingFunction); /** * Associates the {@code value} with the {@code key} in this cache. If the cache previously @@ -136,7 +136,7 @@ Map getAll(Iterable keys, * @param value value to be associated with the specified key * @throws NullPointerException if the specified key or value is null */ - void put(K key, V value); + void put(K key, @NonNull V value); /** * Copies all of the mappings from the specified map to the cache. The effect of this call is @@ -146,9 +146,9 @@ Map getAll(Iterable keys, * * @param map the mappings to be stored in this cache * @throws NullPointerException if the specified map is null or the specified map contains null - * keys or values + * keys or values */ - void putAll(Map map); + void putAll(Map map); /** * Discards any cached value for the {@code key}. The behavior of this operation is undefined for @@ -210,7 +210,7 @@ Map getAll(Iterable keys, * * @return a thread-safe view of this cache supporting all of the optional {@link Map} operations */ - ConcurrentMap asMap(); + ConcurrentMap asMap(); /** * Performs any pending maintenance operations needed by the cache. Exactly which activities are @@ -228,5 +228,5 @@ Map getAll(Iterable keys, * * @return access to inspect and perform advanced operations based on the cache's characteristics */ - Policy policy(); + Policy policy(); } diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java index 3d8b9ca642..a8dff17fb4 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java @@ -24,6 +24,7 @@ import java.util.concurrent.Executor; import java.util.function.Function; +import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -47,7 +48,7 @@ @NullMarked @FunctionalInterface @SuppressWarnings({"FunctionalInterfaceMethodChanged", "PMD.SignatureDeclareThrowsException"}) -public interface CacheLoader extends AsyncCacheLoader { +public interface CacheLoader extends AsyncCacheLoader { /** * Computes or retrieves the value corresponding to {@code key}. @@ -61,7 +62,6 @@ public interface CacheLoader extends AsyncCacheLoader { * treated like any other {@code Exception} in all respects except that, when it is * caught, the thread's interrupt status is set */ - @Nullable V load(K key) throws Exception; /** @@ -87,7 +87,7 @@ public interface CacheLoader extends AsyncCacheLoader { * treated like any other {@code Exception} in all respects except that, when it is * caught, the thread's interrupt status is set */ - default Map loadAll(Set keys) throws Exception { + default Map loadAll(Set keys) throws Exception { throw new UnsupportedOperationException(); } @@ -101,7 +101,7 @@ public interface CacheLoader extends AsyncCacheLoader { * @return the future value associated with {@code key} */ @Override - default CompletableFuture asyncLoad(K key, Executor executor) throws Exception { + default CompletableFuture asyncLoad(K key, Executor executor) throws Exception { requireNonNull(key); requireNonNull(executor); return CompletableFuture.supplyAsync(() -> { @@ -136,7 +136,7 @@ public interface CacheLoader extends AsyncCacheLoader { * that key; may not contain null values */ @Override - default CompletableFuture> asyncLoadAll( + default CompletableFuture> asyncLoadAll( Set keys, Executor executor) throws Exception { requireNonNull(keys); requireNonNull(executor); @@ -171,7 +171,7 @@ public interface CacheLoader extends AsyncCacheLoader { * treated like any other {@code Exception} in all respects except that, when it is * caught, the thread's interrupt status is set */ - default @Nullable V reload(K key, V oldValue) throws Exception { + default V reload(K key, @NonNull V oldValue) throws Exception { return load(key); } @@ -193,8 +193,8 @@ public interface CacheLoader extends AsyncCacheLoader { * {@code null} if the mapping is to be removed */ @Override - default CompletableFuture asyncReload( - K key, V oldValue, Executor executor) throws Exception { + default CompletableFuture asyncReload( + K key, @NonNull V oldValue, Executor executor) throws Exception { requireNonNull(key); requireNonNull(executor); return CompletableFuture.supplyAsync(() -> { diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java index f4e5f23428..4691f0e111 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java @@ -1056,7 +1056,7 @@ boolean isBounded() { * @param the value type of the cache * @return a cache having the requested features */ - public Cache build() { + public Cache build() { requireWeightWithWeigher(); requireNonLoadingCache(); @@ -1081,7 +1081,7 @@ public Cache build() { * @param the value type of the loader * @return a cache having the requested features */ - public LoadingCache build( + public LoadingCache build( CacheLoader loader) { requireWeightWithWeigher(); @@ -1112,7 +1112,7 @@ public LoadingCache build( * @param the value type of the cache * @return a cache having the requested features */ - public AsyncCache buildAsync() { + public AsyncCache buildAsync() { requireState(valueStrength == null, "Weak or soft values can not be combined with AsyncCache"); requireState(isStrongKeys() || (evictionListener == null), "Weak keys cannot be combined with eviction listener and AsyncLoadingCache"); @@ -1144,7 +1144,7 @@ public AsyncCache buildAsync() { * @param the value type of the loader * @return a cache having the requested features */ - public AsyncLoadingCache buildAsync( + public AsyncLoadingCache buildAsync( CacheLoader loader) { return buildAsync((AsyncCacheLoader) loader); } @@ -1167,7 +1167,7 @@ public AsyncLoadingCache buildAsync( * @param the value type of the loader * @return a cache having the requested features */ - public AsyncLoadingCache buildAsync( + public AsyncLoadingCache buildAsync( AsyncCacheLoader loader) { requireState(valueStrength == null, "Weak or soft values can not be combined with AsyncLoadingCache"); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LoadingCache.java index f354e84fb9..14caf119e3 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LoadingCache.java @@ -21,7 +21,7 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.NullUnmarked; +import org.jspecify.annotations.Nullable; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -37,7 +37,7 @@ * @param the type of mapped values */ @NullMarked -public interface LoadingCache extends Cache { +public interface LoadingCache extends Cache { /** * Returns the value associated with the {@code key} in this cache, obtaining that value from @@ -64,8 +64,7 @@ public interface LoadingCache extends Cache { * @throws RuntimeException or Error if the {@link CacheLoader} does so, in which case the mapping * is left unestablished */ - @NullUnmarked - V get(@NonNull K key); + V get(K key); /** * Returns a map of the values associated with the {@code keys}, creating or retrieving those @@ -92,7 +91,7 @@ public interface LoadingCache extends Cache { * {@link CacheLoader#loadAll} returns {@code null}, or returns a map containing null keys * or values. In all cases, the mapping is left unestablished. */ - Map getAll(Iterable keys); + Map getAll(Iterable keys); /** * Loads a new value for the {@code key}, asynchronously. While the new value is loading the @@ -113,8 +112,7 @@ public interface LoadingCache extends Cache { * @throws NullPointerException if the specified key is null */ @CanIgnoreReturnValue - @NullUnmarked - @NonNull CompletableFuture refresh(@NonNull K key); + CompletableFuture refresh(K key); /** * Loads a new value for each {@code key}, asynchronously. While the new value is loading the @@ -134,5 +132,5 @@ public interface LoadingCache extends Cache { * @throws NullPointerException if the specified collection is null or contains a null element */ @CanIgnoreReturnValue - CompletableFuture> refreshAll(Iterable keys); + CompletableFuture> refreshAll(Iterable keys); }