From a3967b9ad9e2e44c1a0cc04599a72c2320c6fee5 Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sun, 1 Sep 2024 21:45:39 -0700 Subject: [PATCH] sync with guava's cache test changes Note that since Caffeine's refresh is linearizable, we invert the test that asserts the stale reload populates the cache and instead we discard it as no longer reliable. The tests were generalized from using internal methods and moved into those test classes. --- .../guava/compatibility/CacheBuilderTest.java | 2 + .../guava/compatibility/CacheLoadingTest.java | 52 ++++++++++++++++ .../guava/compatibility/CacheManualTest.java | 60 ++++++++++++++++++- .../policy/greedy_dual/GdsfPolicy.java | 2 +- 4 files changed, 114 insertions(+), 2 deletions(-) diff --git a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderTest.java b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderTest.java index c1e611c96f..2c1cc1cf8c 100644 --- a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderTest.java +++ b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheBuilderTest.java @@ -547,6 +547,8 @@ public void testRemovalNotification_clear_basher() throws InterruptedException { // notification. assertEquals(expectedKeys, Sets.union(cache.asMap().keySet(), removalNotifications.keySet())); assertTrue(Collections.disjoint(cache.asMap().keySet(), removalNotifications.keySet())); + threadPool.shutdown(); + threadPool.awaitTermination(300, SECONDS); } /** diff --git a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java index b348a12e40..63158a203a 100644 --- a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java +++ b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheLoadingTest.java @@ -24,12 +24,14 @@ import static java.util.Arrays.asList; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import java.time.Duration; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -54,6 +56,7 @@ import com.google.common.util.concurrent.ExecutionError; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.common.util.concurrent.Uninterruptibles; @@ -2235,6 +2238,55 @@ public String load(String key) { assertEquals(refreshKey + suffix, map.get(refreshKey)); } + @SuppressWarnings({"CheckReturnValue", "FutureReturnValueIgnored"}) + public void testLongAsyncRefresh() throws Exception { + FakeTicker ticker = new FakeTicker(); + AtomicInteger counter = new AtomicInteger(); + CountDownLatch reloadStarted = new CountDownLatch(1); + CountDownLatch reloadExpired = new CountDownLatch(1); + CountDownLatch reloadCompleted = new CountDownLatch(1); + + ListeningExecutorService refreshExecutor = MoreExecutors.listeningDecorator( + Executors.newSingleThreadExecutor()); + try { + Caffeine builder = Caffeine.newBuilder() + .expireAfterWrite(100, MILLISECONDS) + .refreshAfterWrite(5, MILLISECONDS) + .executor(refreshExecutor) + .ticker(ticker::read); + + CacheLoader loader = + new CacheLoader() { + @Override public String load(String key) { + return key + "Load-" + counter.incrementAndGet(); + } + @Override public ListenableFuture reload(String key, String oldValue) { + ListenableFuture future = refreshExecutor.submit(() -> { + reloadStarted.countDown(); + reloadExpired.await(); + return key + "Reload"; + }); + future.addListener(reloadCompleted::countDown, refreshExecutor); + return future; + } + }; + LoadingCache cache = CaffeinatedGuava.build(builder, loader); + + assertThat(cache.get("test")).isEqualTo("testLoad-1"); + + ticker.advance(10, MILLISECONDS); // so that the next call will trigger refresh + assertThat(cache.get("test")).isEqualTo("testLoad-1"); + reloadStarted.await(); + ticker.advance(500, MILLISECONDS); // so that the entry expires during the reload + reloadExpired.countDown(); + + assertThat(cache.get("test")).isEqualTo("testLoad-2"); + } finally { + refreshExecutor.shutdown(); + refreshExecutor.awaitTermination(Duration.ofMinutes(1)); + } + } + static Callable throwing(final Exception exception) { return new Callable() { @Override public T call() throws Exception { diff --git a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheManualTest.java b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheManualTest.java index 4220199e0b..6f56f28a60 100644 --- a/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheManualTest.java +++ b/guava/src/test/java/com/github/benmanes/caffeine/guava/compatibility/CacheManualTest.java @@ -15,7 +15,11 @@ package com.github.benmanes.caffeine.guava.compatibility; import static java.util.Arrays.asList; -import junit.framework.TestCase; + +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.guava.CaffeinatedGuava; @@ -23,6 +27,9 @@ import com.google.common.cache.CacheStats; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.UncheckedExecutionException; + +import junit.framework.TestCase; /** * @author Charles Fry @@ -155,4 +162,55 @@ public void testGetAllPresent() { assertEquals(6, stats.hitCount()); } + public void testLoadDifferentKeyInLoader() throws ExecutionException, InterruptedException { + Cache cache = CaffeinatedGuava.build(Caffeine.newBuilder()); + String key1 = "key1"; + String key2 = "key2"; + + // loads a different key, should work + assertEquals(key2, cache.get(key1, () -> cache.get(key2, () -> key2))); + } + + public void testRecursiveLoad() throws InterruptedException { + Cache cache = CaffeinatedGuava.build(Caffeine.newBuilder()); + String key = "key"; + + // recursive load, this should fail + Callable loader = () -> cache.get(key, () -> key); + testLoadThrows(key, cache, loader); + } + + public void testRecursiveLoadWithProxy() throws InterruptedException { + String key = "key"; + String otherKey = "otherKey"; + Cache cache = CaffeinatedGuava.build(Caffeine.newBuilder()); + + // recursive load (same as the initial one), this should fail + Callable loader = () -> cache.get(key, () -> key); + // loads another key, is ok + Callable proxyLoader = () -> cache.get(otherKey, loader); + testLoadThrows(key, cache, proxyLoader); + } + + private void testLoadThrows(String key, Cache cache, Callable loader) + throws InterruptedException { + CountDownLatch doneSignal = new CountDownLatch(1); + Thread thread = new Thread(() -> { + try { + cache.get(key, loader); + } catch (UncheckedExecutionException | ExecutionException e) { + doneSignal.countDown(); + } + }); + thread.start(); + + boolean done = doneSignal.await(1, TimeUnit.SECONDS); + if (!done) { + StringBuilder builder = new StringBuilder(); + for (StackTraceElement trace : thread.getStackTrace()) { + builder.append("\tat ").append(trace).append('\n'); + } + fail(builder.toString()); + } + } } diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/GdsfPolicy.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/GdsfPolicy.java index b91d84af49..a836b50dd2 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/GdsfPolicy.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/GdsfPolicy.java @@ -210,7 +210,7 @@ public boolean equals(Object o) { return false; } var node = (Node) o; - return (key == node.key) && (priority == node.priority); + return compareTo(node) == 0; } @Override