Fix bulk loading of entries with PassthroughLoadingCache
During the review of I5906a1f2 the PassthroughLoadingCache added
the mechanism for bulk-loading in the getAll() method. However,
the new flavour of loading was not covered by additional tests
and then failed to honour the method because of the casting of
a regular Map to an ImmutableMap.
Rectify the situation and add one extra test to document the
expected behaviour.
Release-Notes: Fix bulk loading of entries for disabled caches
Change-Id: I7a24deebeef7f6ae853d11c1ee458b53296e1a44
diff --git a/java/com/google/gerrit/server/cache/mem/PassthroughLoadingCache.java b/java/com/google/gerrit/server/cache/mem/PassthroughLoadingCache.java
index 4ed8cfa..ded21aa 100644
--- a/java/com/google/gerrit/server/cache/mem/PassthroughLoadingCache.java
+++ b/java/com/google/gerrit/server/cache/mem/PassthroughLoadingCache.java
@@ -122,8 +122,8 @@
}
@SuppressWarnings("unchecked")
- public ImmutableMap<K, V> getAllBulk(Iterable<? extends K> keys) throws Exception {
- return (ImmutableMap<K, V>) cacheLoader.loadAll(keys);
+ private ImmutableMap<K, V> getAllBulk(Iterable<? extends K> keys) throws Exception {
+ return (ImmutableMap<K, V>) ImmutableMap.copyOf(cacheLoader.loadAll(keys));
}
@Override
diff --git a/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java b/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java
index e0c0c34..9b504a9 100644
--- a/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java
+++ b/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java
@@ -20,10 +20,14 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.Weigher;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.CacheDef;
import com.google.inject.TypeLiteral;
import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.concurrent.BrokenBarrierException;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ExecutionException;
@@ -33,6 +37,8 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
@@ -90,6 +96,17 @@
assertThat(cacheValue.get()).isEqualTo(TEST_CACHE_KEY);
}
+ @Test
+ public void shouldLoadAllKeysWithDisabledCache() throws Exception {
+ LoadingCache<Integer, Integer> disabledCache =
+ memoryCacheFactory.build(newCacheDef(0), newCacheLoader(identity()), CacheBackend.CAFFEINE);
+
+ List<Integer> keys = Arrays.asList(1, 2);
+ ImmutableMap<Integer, Integer> entries = disabledCache.getAll(keys);
+
+ assertThat(entries).containsExactly(1, 1, 2, 2);
+ }
+
private void assertCacheEvictionIsNotBlocking(LoadingCache<Integer, Integer> disabledCache)
throws InterruptedException, BrokenBarrierException, TimeoutException, ExecutionException {
ScheduledFuture<Integer> cacheValue =
@@ -120,6 +137,12 @@
}
return v;
}
+
+ @Override
+ public Map<Integer, Integer> loadAll(Iterable<? extends Integer> keys) throws Exception {
+ return StreamSupport.stream(keys.spliterator(), false)
+ .collect(Collectors.toMap(identity(), identity()));
+ }
};
}
