Skip to content

Commit

Permalink
AsyncCache#getAll could skip storing additional mappings (fixes #655)
Browse files Browse the repository at this point in the history
  • Loading branch information
ben-manes committed Mar 20, 2022
1 parent 7458245 commit 0cef551
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,6 @@ private void fillProxies(Map<K, V> result) {

/** Adds to the cache any extra entries computed that were not requested. */
private void addNewEntries(Map<K, V> result) {
if (proxies.size() == result.size()) {
return;
}
result.forEach((key, value) -> {
if (!proxies.containsKey(key)) {
cache.put(key, CompletableFuture.completedFuture(value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@
import static com.github.benmanes.caffeine.testing.Awaits.await;
import static com.github.benmanes.caffeine.testing.IsFutureValue.futureOf;
import static com.google.common.collect.Streams.stream;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.aMapWithSize;
import static org.hamcrest.Matchers.anEmptyMap;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
Expand All @@ -42,6 +45,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
Expand Down Expand Up @@ -453,6 +457,21 @@ public void getAllFunction_exceeds(AsyncCache<Integer, Integer> cache, CacheCont
verifyStats(context, verifier -> verifier.hits(0).misses(result.size()).success(1).failures(0));
}

@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DEFAULT, Listener.REJECTING })
public void getAllFunction_different(AsyncCache<Integer, Integer> cache, CacheContext context) {
Map<Integer, Integer> actual = context.absentKeys().stream()
.collect(toMap(k -> -k, identity()));
Map<Integer, Integer> result = cache.getAll(context.absentKeys(), keys -> actual).join();
@SuppressWarnings({"unchecked", "rawtypes"})
Entry<Integer, Integer>[] array = actual.entrySet().toArray(new Map.Entry[0]);

assertThat(result, is(anEmptyMap()));
assertThat(cache.synchronous().asMap().entrySet(), hasItems(array));
assertThat(cache.asMap(), aMapWithSize(context.original().size() + actual.size()));
verifyStats(context, verifier -> verifier.hits(0).misses(actual.size()).success(1).failures(0));
}

@CheckNoWriter
@Test(dataProvider = "caches")
@CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL },
Expand Down Expand Up @@ -719,6 +738,23 @@ public void getAllBifunction_exceeds(AsyncCache<Integer, Integer> cache, CacheCo
verifyStats(context, verifier -> verifier.hits(0).misses(result.size()).success(1).failures(0));
}

@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DEFAULT, Listener.REJECTING })
public void getAllBifunction_different(AsyncCache<Integer, Integer> cache, CacheContext context) {
Map<Integer, Integer> actual = context.absentKeys().stream()
.collect(toMap(k -> -k, identity()));
Map<Integer, Integer> result = cache.getAll(context.absentKeys(), (keys, executor) -> {
return CompletableFuture.completedFuture(actual);
}).join();
@SuppressWarnings({"unchecked", "rawtypes"})
Entry<Integer, Integer>[] array = actual.entrySet().toArray(new Map.Entry[0]);

assertThat(result, is(anEmptyMap()));
assertThat(cache.synchronous().asMap().entrySet(), hasItems(array));
assertThat(cache.asMap(), is(aMapWithSize(context.original().size() + actual.size())));
verifyStats(context, verifier -> verifier.hits(0).misses(actual.size()).success(1).failures(0));
}

@CheckNoWriter
@Test(dataProvider = "caches")
@CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
import static com.github.benmanes.caffeine.testing.Awaits.await;
import static com.github.benmanes.caffeine.testing.IsFutureValue.futureOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.anEmptyMap;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
Expand All @@ -31,6 +33,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
Expand Down Expand Up @@ -237,6 +240,21 @@ public void getAll_exceeds(AsyncLoadingCache<Integer, Integer> cache, CacheConte
verifyStats(context, verifier -> verifier.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<Integer, Integer> cache, CacheContext context) {
Map<Integer, Integer> result = cache.getAll(context.absentKeys()).join();
@SuppressWarnings({"unchecked", "rawtypes"})
Entry<Integer, CompletableFuture<Integer>>[] expected =
result.entrySet().toArray(new Map.Entry[0]);

assertThat(result, is(anEmptyMap()));
assertThat(cache.asMap().entrySet(), hasItems(expected));
verifyStats(context, verifier ->
verifier.hits(0).misses(context.absent().size()).success(1).failures(0));
}

@CheckNoWriter
@Test(dataProvider = "caches")
@CacheSpec(loader = { Loader.NEGATIVE, Loader.BULK_NEGATIVE },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static java.util.Objects.requireNonNull;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -53,6 +55,7 @@
import com.github.benmanes.caffeine.cache.testing.RemovalListeners.ConsumingRemovalListener;
import com.github.benmanes.caffeine.testing.ConcurrentTestHarness;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.testing.TestingExecutors;

Expand Down Expand Up @@ -508,6 +511,15 @@ enum Loader implements CacheLoader<Integer, Integer> {
return result;
}
},
/** A bulk-only loader that loads only keys that were not requested. */
BULK_DIFFERENT {
@Override public Integer load(Integer key) {
throw new UnsupportedOperationException();
}
@Override public Map<Integer, Integer> loadAll(Iterable<? extends Integer> keys) {
return Streams.stream(keys).collect(toMap(k -> -k, identity()));
}
},
/** A bulk-only loader that loads more than requested. */
BULK_NEGATIVE_EXCEEDS {
@Override public Integer load(Integer key) {
Expand Down

0 comments on commit 0cef551

Please sign in to comment.