From b122518c323993e3d6139a2fab67301c2e2906c7 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. --- .github/workflows/build.yml | 9 +++ .../guava/compatibility/CacheBuilderTest.java | 2 + .../guava/compatibility/CacheLoadingTest.java | 52 ++++++++++++++++ .../guava/compatibility/CacheManualTest.java | 60 ++++++++++++++++++- .../policy/greedy_dual/GdsfPolicy.java | 2 +- 5 files changed, 123 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b5ee5d33c8..db720ef9c7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -223,6 +223,7 @@ jobs: api.codacy.com:443 codecov.io:443 coveralls.io:443 + deepsource.io:443 fastly.com:443 nodejs.org:443 raw.githubusercontent.com @@ -260,6 +261,14 @@ jobs: with: project-token: ${{ secrets.CODACY_PROJECT_TOKEN }} continue-on-error: true + - name: Publish to DeepSource + env: + DEEPSOURCE_DSN: ${{ secrets.DEEPSOURCE_DSN }} + continue-on-error: true + run: | + curl https://deepsource.io/cli | sh + ./bin/deepsource report --analyzer test-coverage --key java \ + --value-file ./build/reports/jacoco/jacocoFullReport/jacocoFullReport.xml - name: Publish to SonarQube uses: ./.github/actions/run-gradle env: 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..0d850a0811 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 { + 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