AsyncCache#getAll could skip storing additional mappings (fixes #655)

ben-manes · Jan 25, 2022 · 408f3a5 · 408f3a5
1 parent d212cd6
commit 408f3a5
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 3 deletions.
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java
@@ -275,9 +275,6 @@ private void fillProxies(Map<? extends K, ? extends V> result) {
 
     /** Adds to the cache any extra entries computed that were not requested. */
     private void addNewEntries(Map<? extends K, ? extends V> result) {
-      if (proxies.size() == result.size()) {
-        return;
-      }
       result.forEach((key, value) -> {
         if (!proxies.containsKey(key)) {
           cache.put(key, CompletableFuture.completedFuture(value));

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java
@@ -25,6 +25,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static java.util.function.Function.identity;
 import static java.util.stream.Collectors.toMap;
+import static java.util.stream.Collectors.toSet;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -436,6 +437,25 @@ public void getAllFunction_exceeds(AsyncCache<Int, Int> cache, CacheContext cont
     assertThat(context).stats().hits(0).misses(result.size()).success(1).failures(0);
   }
 
+  @Test(dataProvider = "caches")
+  @CacheSpec(removalListener = { Listener.DEFAULT, Listener.REJECTING })
+  public void getAllFunction_different(AsyncCache<Int, Int> cache, CacheContext context) {
+    int requested = ThreadLocalRandom.current().nextInt(1, context.absent().size() / 2);
+    var requestedKeys = context.absentKeys().stream().limit(requested).collect(toSet());
+    var actual = context.absent().entrySet().stream()
+        .filter(entry -> !requestedKeys.contains(entry.getKey()))
+        .limit(requested)
+        .collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
+    var result = cache.getAll(requestedKeys, keys -> {
+      return actual;
+    }).join();
+
+    assertThat(result).isEmpty();
+    assertThat(cache).hasSize(context.initialSize() + actual.size());
+    assertThat(cache.synchronous().asMap()).containsAtLeastEntriesIn(actual);
+    assertThat(context).stats().hits(0).misses(actual.size()).success(1).failures(0);
+  }
+
   @Test(dataProvider = "caches")
   @CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL },
       removalListener = { Listener.DEFAULT, Listener.REJECTING })
@@ -653,6 +673,25 @@ public void getAllBifunction_exceeds(AsyncCache<Int, Int> cache, CacheContext co
     assertThat(context).stats().hits(0).misses(result.size()).success(1).failures(0);
   }
 
+  @Test(dataProvider = "caches")
+  @CacheSpec(removalListener = { Listener.DEFAULT, Listener.REJECTING })
+  public void getAllBifunction_different(AsyncCache<Int, Int> cache, CacheContext context) {
+    int requested = ThreadLocalRandom.current().nextInt(1, context.absent().size() / 2);
+    var requestedKeys = context.absentKeys().stream().limit(requested).collect(toSet());
+    var actual = context.absent().entrySet().stream()
+        .filter(entry -> !requestedKeys.contains(entry.getKey()))
+        .limit(requested)
+        .collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
+    var result = cache.getAll(requestedKeys, (keys, executor) -> {
+      return CompletableFuture.completedFuture(actual);
+    }).join();
+
+    assertThat(result).isEmpty();
+    assertThat(cache).hasSize(context.initialSize() + actual.size());
+    assertThat(cache.synchronous().asMap()).containsAtLeastEntriesIn(actual);
+    assertThat(context).stats().hits(0).misses(actual.size()).success(1).failures(0);
+  }
+
   @Test(dataProvider = "caches")
   @CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL },
       removalListener = { Listener.DEFAULT, Listener.REJECTING })

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncLoadingCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncLoadingCacheTest.java
@@ -217,6 +217,17 @@ public void getAll_exceeds(AsyncLoadingCache<Int, Int> cache, CacheContext conte
     assertThat(context).stats().hits(0).misses(result.size()).success(1).failures(0);
   }
 
+  @Test(dataProvider = "caches")
+  @CacheSpec(loader = Loader.BULK_DIFFERENT,
+      removalListener = { Listener.DEFAULT, Listener.REJECTING })
+  public void getAll_different(AsyncLoadingCache<Int, Int> cache, CacheContext context) {
+    var result = cache.getAll(context.absentKeys()).join();
+
+    assertThat(result).isEmpty();
+    assertThat(cache.asMap()).containsAtLeastEntriesIn(result);
+    assertThat(context).stats().hits(0).misses(context.absent().size()).success(1).failures(0);
+  }
+
   @Test(dataProvider = "caches")
   @CacheSpec(loader = { Loader.NEGATIVE, Loader.BULK_NEGATIVE },
       population = { Population.SINGLETON, Population.PARTIAL, Population.FULL },

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheSpec.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheSpec.java
@@ -476,6 +476,19 @@ enum Loader implements CacheLoader<Int, Int> {
         return result;
       }
     },
+    /** A bulk-only loader that loads only keys that were not requested. */
+    BULK_DIFFERENT {
+      @Override public Int load(Int key) {
+        throw new UnsupportedOperationException();
+      }
+      @Override public Map<Int, Int> loadAll(Set<? extends Int> keys) throws Exception {
+        var result = new HashMap<Int, Int>(keys.size());
+        for (Int key : keys) {
+          result.put(key.negate(), key);
+        }
+        return result;
+      }
+    },
     /** A bulk-only loader that loads more than requested. */
     BULK_NEGATIVE_EXCEEDS {
       @Override public Int load(Int key) {