From bb48189ccab16bb539f748e2577849b39b55dea0 Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sat, 3 Sep 2022 17:56:49 -0700 Subject: [PATCH] Add utility to adapt a Guava CacheLoader to Caffeine's (fixes #766) The Guava adapters wrap the Caffeine implementations to masquerade under their APIs. Sometimes users wish to use Caffeine's APIs without migrating their Guava CacheLoader. The adapter is now available for use with our cache builder. ```java CacheLoader caffeineLoader = CaffeinatedGuava.caffeinate(guavaLoader); LoadingCache caffeineCache = Caffeine.newBuilder().build(caffeineLoader); ``` --- config/spotbugs/exclude.xml | 5 + .../caffeine/guava/CaffeinatedGuava.java | 23 ++++- .../guava/CaffeinatedGuavaLoadingCache.java | 47 +++++++--- .../caffeine/guava/CaffeinatedGuavaTest.java | 91 +++++++++++++++++++ .../compatibility/CacheBuilderFactory.java | 1 + 5 files changed, 151 insertions(+), 16 deletions(-) diff --git a/config/spotbugs/exclude.xml b/config/spotbugs/exclude.xml index d097bb0293..e0ed5ec959 100644 --- a/config/spotbugs/exclude.xml +++ b/config/spotbugs/exclude.xml @@ -259,6 +259,11 @@ + + + + + diff --git a/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuava.java b/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuava.java index a1441b7f5e..3705232d18 100644 --- a/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuava.java +++ b/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuava.java @@ -15,6 +15,8 @@ */ package com.github.benmanes.caffeine.guava; +import static java.util.Objects.requireNonNull; + import java.lang.reflect.Method; import com.github.benmanes.caffeine.cache.Caffeine; @@ -26,7 +28,7 @@ import com.google.errorprone.annotations.CheckReturnValue; /** - * An adapter to expose a Caffeine cache through the Guava interfaces. + * Static utility methods pertaining to adapting between Caffeine and Guava cache interfaces. * * @author ben.manes@gmail.com (Ben Manes) */ @@ -57,9 +59,7 @@ public static LoadingCache build( Caffeine builder, CacheLoader loader) { @SuppressWarnings("unchecked") CacheLoader castedLoader = (CacheLoader) loader; - return build(builder, hasLoadAll(castedLoader) - ? new BulkLoader<>(castedLoader) - : new SingleLoader<>(castedLoader)); + return build(builder, caffeinate(castedLoader)); } /** @@ -76,6 +76,21 @@ public static LoadingCache build( return new CaffeinatedGuavaLoadingCache<>(builder.build(loader)); } + /** + * Returns a Caffeine cache loader that delegates to a Guava cache loader. + * + * @param loader the cache loader used to obtain new values + * @return a cache loader exposed under the Caffeine APIs + */ + @CheckReturnValue + public static com.github.benmanes.caffeine.cache.CacheLoader caffeinate( + CacheLoader loader) { + requireNonNull(loader); + return hasLoadAll(loader) + ? new BulkLoader<>(loader) + : new SingleLoader<>(loader); + } + static boolean hasLoadAll(CacheLoader cacheLoader) { return hasMethod(cacheLoader, "loadAll", Iterable.class); } diff --git a/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaLoadingCache.java b/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaLoadingCache.java index dfbd6b6b28..3dbc364215 100644 --- a/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaLoadingCache.java +++ b/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaLoadingCache.java @@ -21,15 +21,20 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; + +import org.checkerframework.checker.nullness.qual.Nullable; import com.github.benmanes.caffeine.cache.CacheLoader; -import com.google.common.base.Throwables; import com.google.common.cache.CacheLoader.InvalidCacheLoadException; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ExecutionError; +import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.UncheckedExecutionException; /** @@ -151,20 +156,19 @@ public V load(K key) { } @Override - public V reload(K key, V oldValue) { + public CompletableFuture asyncReload(K key, V oldValue, Executor executor) { + var future = new CompletableFuture(); try { - V value = Futures.getUnchecked(cacheLoader.reload(key, oldValue)); - if (value == null) { - throw new InvalidCacheLoadException("null value"); + ListenableFuture reload = cacheLoader.reload(key, oldValue); + if (reload == null) { + future.completeExceptionally(new InvalidCacheLoadException("null value")); + } else { + Futures.addCallback(reload, new FutureCompleter<>(future), Runnable::run); } - return value; - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new CacheLoaderException(e); - } catch (Exception e) { - Throwables.throwIfUnchecked(e); - throw new RuntimeException(e); + } catch (Throwable t) { + future.completeExceptionally(t); } + return future; } } @@ -201,4 +205,23 @@ public Map loadAll(Set keys) { } } } + + static final class FutureCompleter implements FutureCallback { + final CompletableFuture future; + + FutureCompleter(CompletableFuture future) { + this.future = future; + } + + @Override public void onSuccess(@Nullable V value) { + if (value == null) { + future.completeExceptionally(new InvalidCacheLoadException("null value")); + } else { + future.complete(value); + } + } + @Override public void onFailure(Throwable t) { + future.completeExceptionally(t); + } + } } diff --git a/guava/src/test/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaTest.java b/guava/src/test/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaTest.java index 609eb3cc4e..528594796a 100644 --- a/guava/src/test/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaTest.java +++ b/guava/src/test/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaTest.java @@ -15,13 +15,26 @@ */ package com.github.benmanes.caffeine.guava; +import static com.google.common.truth.Truth.assertThat; + import java.lang.reflect.Constructor; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletionException; + +import org.junit.Assert; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.guava.CaffeinatedGuavaCache.CacheLoaderException; +import com.github.benmanes.caffeine.guava.CaffeinatedGuavaLoadingCache.BulkLoader; +import com.github.benmanes.caffeine.guava.CaffeinatedGuavaLoadingCache.SingleLoader; import com.github.benmanes.caffeine.guava.compatibility.TestingCacheLoaders; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; import com.google.common.testing.SerializableTester; import com.google.common.util.concurrent.MoreExecutors; @@ -82,6 +95,84 @@ public void testReload_throwable() { } } + public void testCacheLoader_null() { + try { + CaffeinatedGuava.caffeinate(null); + Assert.fail(); + } catch (NullPointerException expected) {} + } + + public void testCacheLoader_single() throws Exception { + var error = new Exception(); + var guava = new CacheLoader() { + @Override public Integer load(Integer key) throws Exception { + if (key > 0) { + return -key; + } + throw error; + } + }; + var caffeine = CaffeinatedGuava.caffeinate(guava); + assertThat(caffeine).isNotInstanceOf(BulkLoader.class); + checkSingleLoader(error, guava, caffeine); + } + + public void testCacheLoader_bulk() throws Exception { + var error = new Exception(); + var guava = new CacheLoader() { + @Override public Integer load(Integer key) throws Exception { + if (key > 0) { + return -key; + } + throw error; + } + @Override public ImmutableMap loadAll( + Iterable keys) throws Exception { + if (Iterables.all(keys, key -> key > 0)) { + return Maps.toMap(ImmutableSet.copyOf(keys), key -> -key); + } + throw error; + } + }; + var caffeine = CaffeinatedGuava.caffeinate(guava); + checkSingleLoader(error, guava, caffeine); + checkBulkLoader(error, caffeine); + } + + private static void checkSingleLoader(Exception error, CacheLoader guava, + com.github.benmanes.caffeine.cache.CacheLoader caffeine) throws Exception { + assertThat(caffeine).isInstanceOf(SingleLoader.class); + assertThat(((SingleLoader) caffeine).cacheLoader).isSameInstanceAs(guava); + + assertThat(caffeine.load(1)).isEqualTo(-1); + try { + caffeine.load(-1); + Assert.fail(); + } catch (CacheLoaderException e) { + assertThat(e).hasCauseThat().isSameInstanceAs(error); + } + + assertThat(caffeine.asyncReload(1, 2, Runnable::run).join()).isEqualTo(-1); + try { + caffeine.asyncReload(-1, 2, Runnable::run).join(); + Assert.fail(); + } catch (CompletionException e) { + assertThat(e).hasCauseThat().isSameInstanceAs(error); + } + } + + private void checkBulkLoader(Exception error, + com.github.benmanes.caffeine.cache.CacheLoader caffeine) throws Exception { + assertThat(caffeine).isInstanceOf(BulkLoader.class); + assertThat(caffeine.loadAll(Set.of(1, 2, 3))).isEqualTo(Map.of(1, -1, 2, -2, 3, -3)); + try { + caffeine.loadAll(Set.of(1, -1)); + Assert.fail(); + } catch (CacheLoaderException e) { + assertThat(e).hasCauseThat().isSameInstanceAs(error); + } + } + enum IdentityLoader implements com.github.benmanes.caffeine.cache.CacheLoader { INSTANCE; diff --git a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderFactory.java b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderFactory.java index 7df1160447..89cf3e4915 100644 --- a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderFactory.java +++ b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderFactory.java @@ -36,6 +36,7 @@ * * @author mike nonemacher */ +@SuppressWarnings("CanIgnoreReturnValueSuggester") class CacheBuilderFactory { // Default values contain only 'null', which means don't call the CacheBuilder method (just give // the CacheBuilder default).