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).