Guava compatibility for bulk get iteration order (fixed #220)

ben-manes · Feb 13, 2019 · 60c9886 · 60c9886
1 parent faf7370
commit 60c9886
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 18 deletions.
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java
@@ -98,18 +98,20 @@ default Map<K, V> loadSequentially(Iterable<? extends K> keys) {
 
   /** Batch loads the missing entries. */
   default Map<K, V> loadInBulk(Iterable<? extends K> keys) {
-    Map<K, V> found = cache().getAllPresent(keys);
     Set<K> keysToLoad = new LinkedHashSet<>();
+    Map<K, V> found = cache().getAllPresent(keys);
+    Map<K, V> result = new LinkedHashMap<>(found.size());
     for (K key : keys) {
-      if (!found.containsKey(key)) {
+      V value = found.get(key);
+      if (value == null) {
         keysToLoad.add(key);
       }
+      result.put(key, value);
     }
     if (keysToLoad.isEmpty()) {
       return found;
     }
 
-    Map<K, V> result = new LinkedHashMap<>(found);
     bulkLoad(keysToLoad, result);
     return Collections.unmodifiableMap(result);
   }
@@ -129,7 +131,9 @@ default void bulkLoad(Set<K> keysToLoad, Map<K, V> result) {
       });
       for (K key : keysToLoad) {
         V value = loaded.get(key);
-        if (value != null) {
+        if (value == null) {
+          result.remove(key);
+        } else {
           result.put(key, value);
         }
       }

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
@@ -285,6 +285,21 @@ public void getAllPresent_ordered_absent(
     assertThat(result, is(equalTo(keys)));
   }
 
+  @CheckNoWriter
+  @Test(dataProvider = "caches")
+  @CacheSpec(loader = { Loader.NEGATIVE, Loader.BULK_NEGATIVE },
+      population = { Population.SINGLETON, Population.PARTIAL },
+      removalListener = { Listener.DEFAULT, Listener.REJECTING })
+  public void getAllPresent_ordered_partial(
+      AsyncLoadingCache<Integer, Integer> cache, CacheContext context) {
+    List<Integer> keys = new ArrayList<>(context.original().keySet());
+    keys.addAll(context.absentKeys());
+    Collections.shuffle(keys);
+
+    List<Integer> result = new ArrayList<>(cache.getAll(keys).join().keySet());
+    assertThat(result, is(equalTo(keys)));
+  }
+
   @CheckNoWriter
   @Test(dataProvider = "caches")
   @CacheSpec(loader = { Loader.NEGATIVE, Loader.BULK_NEGATIVE },

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java
@@ -620,13 +620,7 @@ public void adapt_increaseWindow(Cache<Integer, Integer> cache, CacheContext con
     long windowSize = localCache.windowWeightedSize();
     long windowMaximum = localCache.windowMaximum();
 
-    localCache.setPreviousSampleHitRate(0.80);
-    localCache.setMissesInSample(sampleSize / 2);
-    localCache.setHitsInSample(sampleSize - localCache.missesInSample());
-    localCache.climb();
-
-    // Fill main protected space
-    cache.asMap().keySet().stream().forEach(cache::getIfPresent);
+    adapt(cache, localCache, sampleSize);
 
     assertThat(localCache.mainProtectedMaximum(),
         is(either(lessThan(protectedMaximum)).or(is(0L))));
@@ -650,13 +644,7 @@ public void adapt_decreaseWindow(Cache<Integer, Integer> cache, CacheContext con
     long windowSize = localCache.windowWeightedSize();
     long windowMaximum = localCache.windowMaximum();
 
-    localCache.setPreviousSampleHitRate(0.80);
-    localCache.setMissesInSample(sampleSize / 2);
-    localCache.setHitsInSample(sampleSize - localCache.missesInSample());
-    localCache.climb();
-
-    // Fill main protected space
-    cache.asMap().keySet().stream().forEach(cache::getIfPresent);
+    adapt(cache, localCache, sampleSize);
 
     assertThat(localCache.mainProtectedMaximum(), is(greaterThan(protectedMaximum)));
     assertThat(localCache.mainProtectedWeightedSize(), is(greaterThan(protectedSize)));
@@ -680,4 +668,15 @@ private BoundedLocalCache<Integer, Integer> prepareForAdaption(
     cache.asMap().keySet().stream().forEach(cache::getIfPresent);
     return localCache;
   }
+
+  private void adapt(Cache<Integer, Integer> cache,
+      BoundedLocalCache<Integer, Integer> localCache, int sampleSize) {
+    localCache.setPreviousSampleHitRate(0.80);
+    localCache.setMissesInSample(sampleSize / 2);
+    localCache.setHitsInSample(sampleSize - localCache.missesInSample());
+    localCache.climb();
+
+    // Fill main protected space
+    cache.asMap().keySet().stream().forEach(cache::getIfPresent);
+  }
 }
diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/LoadingCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/LoadingCacheTest.java
@@ -288,6 +288,21 @@ public void getAllPresent_ordered_absent(
     assertThat(result, is(equalTo(keys)));
   }
 
+  @CheckNoWriter
+  @Test(dataProvider = "caches")
+  @CacheSpec(loader = { Loader.NEGATIVE, Loader.BULK_NEGATIVE },
+      population = { Population.SINGLETON, Population.PARTIAL },
+      removalListener = { Listener.DEFAULT, Listener.REJECTING })
+  public void getAllPresent_ordered_partial(
+      LoadingCache<Integer, Integer> cache, CacheContext context) {
+    List<Integer> keys = new ArrayList<>(context.original().keySet());
+    keys.addAll(context.absentKeys());
+    Collections.shuffle(keys);
+
+    List<Integer> result = new ArrayList<>(cache.getAll(keys).keySet());
+    assertThat(result, is(equalTo(keys)));
+  }
+
   @CheckNoWriter
   @Test(dataProvider = "caches")
   @CacheSpec(loader = { Loader.NEGATIVE, Loader.BULK_NEGATIVE },