From d26d0c9a59f04d4ee871b935920164f781a9940c Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Thu, 11 Aug 2016 02:09:31 -0700 Subject: [PATCH] Fix reference cache triggering maintenance after a write (fixed #111) This oversight caused write methods to not try to clean-up collected entries for caching using only the weak/soft reference configurations. The maintenance would be triggered after enough reads or if manually calling cleanUp(). Most of the changes are improvements to the unit tests, as test flaws allowed this bug to be introduced. Primarily the cause was due to not being strict enough when evaluating the removal notifications. The tests now highlight a slight difference in Guava & Caffeine regarding if replacing a value with the same instance triggers a notification. Other small mistakes in the reference cache tests were uncovered and fixed. --- .../caffeine/cache/BoundedLocalCache.java | 5 + .../benmanes/caffeine/cache/AsMapTest.java | 63 +++++--- .../benmanes/caffeine/cache/CacheTest.java | 26 ++-- .../benmanes/caffeine/cache/EvictionTest.java | 3 +- .../caffeine/cache/ExpirationTest.java | 36 ++++- .../caffeine/cache/ReferenceTest.java | 138 ++++++++++-------- .../caffeine/cache/RefreshAfterWriteTest.java | 8 +- .../cache/testing/GuavaCacheFromContext.java | 10 +- .../testing/HasRemovalNotifications.java | 14 +- gradle/dependencies.gradle | 8 +- gradle/wrapper/gradle-wrapper.properties | 2 +- 11 files changed, 199 insertions(+), 114 deletions(-) diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java index d03d12d640..d32b98a213 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java @@ -915,6 +915,8 @@ void afterWrite(@Nullable Node node, Runnable task, long now) { } catch (RuntimeException e) { logger.log(Level.SEVERE, "Exception thrown when performing the maintenance task", e); } + } else { + scheduleAfterWrite(); } } @@ -2187,6 +2189,9 @@ V remap(K key, Object keyRef, BiFunction rema afterWrite(node, new UpdateTask(node, weightedDifference), now); } else { afterRead(node, now, /* recordHit */ false); + if (cause[0] == RemovalCause.COLLECTED) { + scheduleDrainBuffers(); + } } } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java index db5a6b2a9f..c816315cc0 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java @@ -341,7 +341,11 @@ public void put_replace_sameValue(Map map, CacheContext contex } int count = context.firstMiddleLastKeys().size(); assertThat(map.size(), is(context.original().size())); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + if (context.isGuava() || context.isAsync()) { + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + } else { + assertThat(context.consumedNotifications(), hasSize(0)); + } } @CheckNoStats @@ -443,20 +447,23 @@ public void putAll_replace(Map map, CacheContext context) { @CacheSpec(population = { Population.PARTIAL, Population.FULL }, removalListener = { Listener.DEFAULT, Listener.CONSUMING }) public void putAll_mixed(Map map, CacheContext context) { - Map expect = new HashMap<>(context.original()); Map entries = new HashMap<>(); - for (int i = 0; i < 2 * context.original().size(); i++) { - int value = ((i % 2) == 0) ? i : (i + 1); - entries.put(i, value); - } - expect.putAll(entries); + Map replaced = new HashMap<>(); + context.original().forEach((key, value) -> { + if ((key % 2) == 0) { + value++; + replaced.put(key, value); + } + entries.put(key, value); + }); map.putAll(entries); - assertThat(map, is(equalTo(expect))); - assertThat(map, hasRemovalNotifications(context, entries.size() / 2, RemovalCause.REPLACED)); + assertThat(map, is(equalTo(entries))); + Map expect = (context.isGuava() || context.isAsync()) ? entries : replaced; + assertThat(map, hasRemovalNotifications(context, expect.size(), RemovalCause.REPLACED)); verifyWriter(context, (verifier, writer) -> { - verifier.wroteAll(entries); + verifier.wroteAll(replaced); }); } @@ -694,8 +701,12 @@ public void replace_sameValue(Map map, CacheContext context) { } assertThat(map.size(), is(context.original().size())); - int count = context.firstMiddleLastKeys().size(); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + if (context.isGuava() || context.isAsync()) { + int count = context.firstMiddleLastKeys().size(); + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + } else { + assertThat(context.consumedNotifications(), hasSize(0)); + } } @CheckNoStats @@ -814,8 +825,12 @@ public void replaceConditionally_sameValue(Map map, CacheConte } assertThat(map.size(), is(context.original().size())); - int count = context.firstMiddleLastKeys().size(); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + if (context.isGuava() || context.isAsync()) { + int count = context.firstMiddleLastKeys().size(); + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + } else { + assertThat(context.consumedNotifications(), hasSize(0)); + } } @CheckNoStats @@ -870,7 +885,12 @@ public void replaceAll_writerFails(Map map, CacheContext conte public void replaceAll_sameValue(Map map, CacheContext context) { map.replaceAll((key, value) -> value); assertThat(map, is(equalTo(context.original()))); - assertThat(map, hasRemovalNotifications(context, map.size(), RemovalCause.REPLACED)); + + if (context.isGuava() || context.isAsync()) { + assertThat(map, hasRemovalNotifications(context, map.size(), RemovalCause.REPLACED)); + } else { + assertThat(context.consumedNotifications(), hasSize(0)); + } } @CacheSpec @@ -1204,7 +1224,12 @@ public void compute_sameValue(Map map, CacheContext context) { assertThat(map.get(key), is(value)); } assertThat(map.size(), is(context.original().size())); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + + if (context.isGuava() || context.isAsync()) { + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + } else { + assertThat(context.consumedNotifications(), hasSize(0)); + } } @CheckNoWriter @@ -1341,7 +1366,11 @@ public void merge_sameValue(Map map, CacheContext context) { assertThat(map.get(key), is(value)); } assertThat(map.size(), is(context.original().size())); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + if (context.isGuava() || context.isAsync()) { + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + } else { + assertThat(context.consumedNotifications(), hasSize(0)); + } } @CheckNoWriter diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java index bc12c3f1aa..d3fa5c36f6 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java @@ -273,7 +273,10 @@ public void put_replace_sameValue(Cache cache, CacheContext co assertThat(cache.estimatedSize(), is(context.initialSize())); int count = context.firstMiddleLastKeys().size(); - assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + + if (context.isGuava() || context.isAsync()) { + assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.REPLACED)); + } } @Test(dataProvider = "caches") @@ -374,20 +377,23 @@ public void putAll_replace(Cache cache, CacheContext context) @CacheSpec(population = { Population.PARTIAL, Population.FULL }, removalListener = { Listener.DEFAULT, Listener.CONSUMING }) public void putAll_mixed(Cache cache, CacheContext context) { - Map expect = new HashMap<>(context.original()); Map entries = new HashMap<>(); - for (int i = 0; i < 2 * context.initialSize(); i++) { - int value = ((i % 2) == 0) ? i : (i + 1); - entries.put(i, value); - } - expect.putAll(entries); + Map replaced = new HashMap<>(); + context.original().forEach((key, value) -> { + if ((key % 2) == 0) { + value++; + replaced.put(key, value); + } + entries.put(key, value); + }); cache.putAll(entries); - assertThat(cache.asMap(), is(equalTo(expect))); - assertThat(cache, hasRemovalNotifications(context, entries.size() / 2, RemovalCause.REPLACED)); + assertThat(cache.asMap(), is(equalTo(entries))); + Map expect = (context.isGuava() || context.isAsync()) ? entries : replaced; + assertThat(cache, hasRemovalNotifications(context, expect.size(), RemovalCause.REPLACED)); verifyWriter(context, (verifier, writer) -> { - verifier.wroteAll(entries); + verifier.wroteAll(replaced); }); } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java index 8682299964..759f4f812f 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java @@ -522,7 +522,8 @@ public void maximumSize_decrease(Cache cache, } @Test(dataProvider = "caches") - @CacheSpec(implementation = Implementation.Caffeine, maximumSize = Maximum.FULL, + @CacheSpec(implementation = Implementation.Caffeine, + maximumSize = Maximum.FULL, weigher = { CacheWeigher.DEFAULT, CacheWeigher.TEN }, removalListener = { Listener.DEFAULT, Listener.CONSUMING }) public void maximumSize_decrease_min(Cache cache, CacheContext context, Eviction eviction) { diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java index 25ffe11b77..60fdea91a1 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java @@ -150,6 +150,10 @@ public void put_replace(Cache cache, CacheContext context) { assertThat(cache.getIfPresent(context.middleKey()), is(nullValue())); assertThat(cache.estimatedSize(), is(2L)); + if (context.isGuava()) { + cache.cleanUp(); + } + long count = context.initialSize() - 1; assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); @@ -205,6 +209,10 @@ public void putAll_replace(Cache cache, CacheContext context) assertThat(cache.getIfPresent(context.middleKey()), is(nullValue())); assertThat(cache.estimatedSize(), is(2L)); + if (context.isGuava()) { + cache.cleanUp(); + } + long count = context.initialSize() - 1; assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); @@ -295,9 +303,12 @@ public void invalidateAll_full(Cache cache, CacheContext conte context.ticker().advance(1, TimeUnit.MINUTES); cache.invalidateAll(); - long count = context.initialSize(); - assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); - verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); + if (!context.isGuava()) { + // https://github.com/google/guava/issues/2101 + long count = context.initialSize(); + assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); + verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); + } } @Test(dataProvider = "caches", expectedExceptions = DeleteException.class) @@ -428,7 +439,7 @@ public void refresh_writerFails(LoadingCache cache, CacheConte expireAfterAccess = {Expire.DISABLED, Expire.ONE_MINUTE}, expireAfterWrite = {Expire.DISABLED, Expire.ONE_MINUTE}) public void get(AsyncLoadingCache cache, CacheContext context) { - context.ticker().advance(1, TimeUnit.SECONDS); + context.ticker().advance(2, TimeUnit.MINUTES); cache.get(context.firstKey()); cache.get(context.middleKey(), k -> context.absentValue()); @@ -560,9 +571,12 @@ public void clear(Map map, CacheContext context) { context.ticker().advance(1, TimeUnit.MINUTES); map.clear(); - long count = context.initialSize(); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); - verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); + if (!context.isGuava()) { + // https://github.com/google/guava/issues/2101 + long count = context.initialSize(); + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); + verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); + } } @Test(dataProvider = "caches", expectedExceptions = DeleteException.class) @@ -630,6 +644,10 @@ public void put_replace(Map map, CacheContext context) { assertThat(map.get(context.middleKey()), is(nullValue())); assertThat(map.size(), is(2)); + if (context.isGuava()) { + context.cleanUp(); + } + long count = context.initialSize() - 1; assertThat(map, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); @@ -828,6 +846,10 @@ public void computeIfPresent(Map map, CacheContext context) { assertThat(map.computeIfPresent(key, (k, v) -> value), is(nullValue())); assertThat(map.size(), is(0)); + if (context.isGuava()) { + context.cleanUp(); + } + long count = context.initialSize(); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.EXPIRED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.EXPIRED)); diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReferenceTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReferenceTest.java index e3d25d0201..1892bf6049 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReferenceTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReferenceTest.java @@ -19,8 +19,11 @@ import static com.github.benmanes.caffeine.cache.testing.HasRemovalNotifications.hasRemovalNotifications; import static com.github.benmanes.caffeine.testing.IsEmptyMap.emptyMap; import static com.github.benmanes.caffeine.testing.IsFutureValue.futureOf; +import static java.util.stream.Collectors.toMap; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import java.util.List; @@ -51,6 +54,7 @@ import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; import com.google.common.collect.Maps; import com.google.common.testing.GcFinalization; @@ -64,6 +68,7 @@ @SuppressWarnings("BoxedPrimitiveConstructor") @Test(groups = "slow", dataProviderClass = CacheProvider.class) public final class ReferenceTest { + // These tests require that the JVM uses -XX:SoftRefLRUPolicyMSPerMB=0 so that soft references // can be reliably garbage collected (by making them behave as weak references). @@ -107,6 +112,9 @@ public void get(Cache cache, CacheContext context) { assertThat(cache.get(key, k -> context.absentValue()), is(context.absentValue())); long count = context.initialSize() - cache.estimatedSize() + 1; + if (context.population() != Population.SINGLETON) { + assertThat(count, is(greaterThan(1L))); + } assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -153,6 +161,9 @@ public void put(Cache cache, CacheContext context) { cache.put(key, context.absentValue()); long count = context.initialSize() - cache.estimatedSize() + 1; + if (context.population() != Population.SINGLETON) { + assertThat(count, is(greaterThan(1L))); + } assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -187,7 +198,12 @@ public void putAll(Cache cache, CacheContext context) { GcFinalization.awaitFullGc(); cache.putAll(entries); + if (context.population() != Population.SINGLETON) { + assertThat(context.initialSize() - cache.estimatedSize(), is(greaterThan(0L))); + } long count = context.initialSize() - cache.estimatedSize() + 3; + assertThat(count, is(greaterThan(3L))); + assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -222,6 +238,8 @@ public void invalidate(Cache cache, CacheContext context) { cache.invalidate(key); long count = context.initialSize() - cache.estimatedSize(); + + assertThat(count, is(greaterThan(0L))); assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -256,6 +274,8 @@ public void invalidateAll(Cache cache, CacheContext context) { cache.invalidateAll(keys); long count = context.initialSize() - cache.estimatedSize(); + + assertThat(count, is(greaterThan(0L))); assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -289,8 +309,14 @@ public void invalidateAll_full(Cache cache, CacheContext conte cache.invalidateAll(); long count = context.initialSize() - cache.estimatedSize(); - assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); - verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); + + assertThat(count, is(greaterThan(0L))); + + if (!context.isGuava()) { + // https://github.com/google/guava/issues/2101 + assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); + verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); + } } @Test(dataProvider = "caches", expectedExceptions = DeleteException.class) @@ -332,6 +358,8 @@ public void cleanUp(Cache cache, CacheContext context) { cache.cleanUp(); long count = context.initialSize() - cache.estimatedSize(); + + assertThat(count, is(greaterThan(0L))); assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -400,6 +428,8 @@ public void getAll(LoadingCache cache, CacheContext context) { assertThat(cache.getAll(keys), is(Maps.uniqueIndex(keys, Functions.identity()))); long count = context.initialSize() - cache.estimatedSize() + keys.size(); + + assertThat(count, is(greaterThan((long) keys.size()))); assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -436,6 +466,8 @@ public void refresh(LoadingCache cache, CacheContext context) cache.refresh(key); long count = context.initialSize() - cache.estimatedSize() + 1; + + assertThat(count, is(greaterThan(1L))); assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> { verifier.deleted(key, context.original().get(key), RemovalCause.COLLECTED); @@ -460,7 +492,7 @@ public void refresh_writerFails(LoadingCache cache, CacheConte /* ---------------- AsyncLoadingCache -------------- */ @Test(dataProvider = "caches") - @CacheSpec(keys = ReferenceType.STRONG, values = {ReferenceType.WEAK, ReferenceType.SOFT}, + @CacheSpec(keys = ReferenceType.WEAK, values = ReferenceType.STRONG, expireAfterAccess = Expire.DISABLED, expireAfterWrite = Expire.DISABLED, maximumSize = Maximum.DISABLED, weigher = CacheWeigher.DEFAULT, population = Population.FULL, stats = Stats.ENABLED, removalListener = Listener.CONSUMING) @@ -468,85 +500,52 @@ public void getIfPresent(AsyncLoadingCache cache, CacheContext Integer key = context.firstKey(); context.clear(); GcFinalization.awaitFullGc(); - assertThat(cache.getIfPresent(key), is(nullValue())); + assertThat(cache.getIfPresent(key), is(not(nullValue()))); } @Test(dataProvider = "caches") - @CacheSpec(keys = ReferenceType.STRONG, values = {ReferenceType.WEAK, ReferenceType.SOFT}, + @CacheSpec(keys = ReferenceType.WEAK, values = ReferenceType.STRONG, expireAfterAccess = Expire.DISABLED, expireAfterWrite = Expire.DISABLED, maximumSize = Maximum.DISABLED, weigher = CacheWeigher.DEFAULT, - population = Population.SINGLETON, stats = Stats.ENABLED, loader = Loader.IDENTITY, + population = Population.FULL, stats = Stats.ENABLED, loader = Loader.IDENTITY, removalListener = Listener.CONSUMING) public void get(AsyncLoadingCache cache, CacheContext context) { - Integer key = context.firstKey(); context.clear(); GcFinalization.awaitFullGc(); - assertThat(cache.get(key), is(futureOf(key))); - - assertThat(cache, hasRemovalNotifications(context, 1, RemovalCause.COLLECTED)); - verifyWriter(context, (verifier, writer) -> verifier.deletions(1, RemovalCause.COLLECTED)); - } + assertThat(cache.get(context.absentKey()), is(futureOf(context.absentKey()))); - @Test(dataProvider = "caches", expectedExceptions = DeleteException.class) - @CacheSpec(keys = ReferenceType.STRONG, values = {ReferenceType.WEAK, ReferenceType.SOFT}, - implementation = Implementation.Caffeine, expireAfterAccess = Expire.DISABLED, - expireAfterWrite = Expire.DISABLED, maximumSize = Maximum.DISABLED, - weigher = CacheWeigher.DEFAULT, population = Population.FULL, stats = Stats.ENABLED, - compute = Compute.SYNC, removalListener = Listener.CONSUMING, writer = Writer.EXCEPTIONAL) - public void get_writerFails(AsyncLoadingCache cache, CacheContext context) { - Integer key = context.firstKey(); - try { - context.clear(); - GcFinalization.awaitFullGc(); - cache.get(key); - } finally { - context.disableRejectingCacheWriter(); - assertThat(cache.synchronous().asMap().isEmpty(), is(false)); - } + long count = context.initialSize(); + assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); + verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @Test(dataProvider = "caches") - @CacheSpec(keys = ReferenceType.STRONG, values = {ReferenceType.WEAK, ReferenceType.SOFT}, + @CacheSpec(keys = ReferenceType.WEAK, values = ReferenceType.STRONG, expireAfterAccess = Expire.DISABLED, expireAfterWrite = Expire.DISABLED, maximumSize = Maximum.DISABLED, weigher = CacheWeigher.DEFAULT, population = Population.FULL, stats = Stats.ENABLED, - loader = {Loader.IDENTITY, Loader.BULK_IDENTITY}, removalListener = Listener.CONSUMING) + loader = {Loader.NEGATIVE, Loader.BULK_NEGATIVE}, removalListener = Listener.CONSUMING) public void getAll(AsyncLoadingCache cache, CacheContext context) { - Set keys = context.firstMiddleLastKeys(); + Set keys = ImmutableSet.of(context.firstKey(), context.lastKey(), context.absentKey()); context.clear(); GcFinalization.awaitFullGc(); - assertThat(cache.getAll(keys).join(), is(Maps.uniqueIndex(keys, Functions.identity()))); + assertThat(cache.getAll(keys).join(), + is(keys.stream().collect(toMap(Function.identity(), key -> -key)))); - long count = context.initialSize() - cache.synchronous().estimatedSize() + keys.size(); + long count = context.initialSize() - cache.synchronous().estimatedSize() + 1; + + assertThat(count, is(greaterThan((long) keys.size()))); assertThat(cache, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } - @Test(dataProvider = "caches", expectedExceptions = DeleteException.class) - @CacheSpec(keys = ReferenceType.STRONG, values = {ReferenceType.WEAK, ReferenceType.SOFT}, - implementation = Implementation.Caffeine, expireAfterAccess = Expire.DISABLED, - expireAfterWrite = Expire.DISABLED, maximumSize = Maximum.DISABLED, - weigher = CacheWeigher.DEFAULT, population = Population.FULL, stats = Stats.ENABLED, - compute = Compute.SYNC, removalListener = Listener.CONSUMING, writer = Writer.EXCEPTIONAL) - public void getAll_writerFails(AsyncLoadingCache cache, CacheContext context) { - Set keys = context.firstMiddleLastKeys(); - try { - context.clear(); - GcFinalization.awaitFullGc(); - cache.getAll(keys); - } finally { - context.disableRejectingCacheWriter(); - assertThat(cache.synchronous().asMap().isEmpty(), is(false)); - } - } - @Test(dataProvider = "caches") - @CacheSpec(keys = ReferenceType.STRONG, values = {ReferenceType.WEAK, ReferenceType.SOFT}, + @CacheSpec(keys = ReferenceType.WEAK, values = ReferenceType.STRONG, expireAfterAccess = Expire.DISABLED, expireAfterWrite = Expire.DISABLED, maximumSize = Maximum.DISABLED, weigher = CacheWeigher.DEFAULT, population = Population.FULL, stats = Stats.ENABLED, removalListener = Listener.CONSUMING) public void put(AsyncLoadingCache cache, CacheContext context) { - Integer key = context.firstKey(); + Integer key = context.absentKey(); context.clear(); GcFinalization.awaitFullGc(); cache.put(key, CompletableFuture.completedFuture(context.absentValue())); @@ -615,8 +614,13 @@ public void clear(Map map, CacheContext context) { map.clear(); long count = context.initialSize() - map.size(); - assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); - verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); + + assertThat(count, is(greaterThan(0L))); + if (!context.isGuava()) { + // https://github.com/google/guava/issues/2101 + assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); + verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); + } } @Test(dataProvider = "caches", expectedExceptions = DeleteException.class) @@ -649,6 +653,8 @@ public void putIfAbsent(Map map, CacheContext context) { assertThat(map.get(key), is(context.absentValue())); long count = context.initialSize() - map.size() + 1; + + assertThat(count, is(greaterThan(1L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -684,6 +690,8 @@ public void put(Map map, CacheContext context) { assertThat(map.get(key), is(context.absentValue())); long count = context.initialSize() - map.size() + 1; + + assertThat(count, is(greaterThan(1L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -746,6 +754,8 @@ public void remove(Map map, CacheContext context) { assertThat(map.remove(key), is(nullValue())); long count = context.initialSize() - map.size(); + + assertThat(count, is(greaterThan(0L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -775,11 +785,14 @@ public void remove_writerFails(Map map, CacheContext context) population = Population.FULL, stats = Stats.ENABLED, removalListener = Listener.CONSUMING) public void removeConditionally(Map map, CacheContext context) { Integer key = context.firstKey(); + Integer value = context.original().get(key); context.clear(); GcFinalization.awaitFullGc(); - assertThat(map.remove(key, context.absentValue()), is(false)); + assertThat(map.remove(key, value), is(true)); - long count = context.initialSize() - map.size(); + long count = context.initialSize() - map.size() - 1; + + assertThat(count, is(greaterThan(0L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -798,6 +811,8 @@ public void computeIfAbsent(Map map, CacheContext context) { assertThat(map.computeIfAbsent(key, k -> context.absentValue()), is(context.absentValue())); long count = context.initialSize() - map.size() + 1; + + assertThat(count, is(greaterThan(1L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -831,7 +846,12 @@ public void computeIfPresent(Map map, CacheContext context) { GcFinalization.awaitFullGc(); assertThat(map.computeIfPresent(key, (k, v) -> context.absentValue()), is(nullValue())); + if (context.isGuava()) { + context.cleanUp(); + } long count = context.initialSize() - map.size(); + + assertThat(count, is(greaterThan(0L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -853,6 +873,8 @@ public void compute(Map map, CacheContext context) { }), is(context.absentValue())); long count = context.initialSize() - map.size() + 1; + + assertThat(count, is(greaterThan(1L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } @@ -889,6 +911,8 @@ public void merge(Map map, CacheContext context) { }), is(context.absentValue())); long count = context.initialSize() - map.size() + 1; + + assertThat(count, is(greaterThan(1L))); assertThat(map, hasRemovalNotifications(context, count, RemovalCause.COLLECTED)); verifyWriter(context, (verifier, writer) -> verifier.deletions(count, RemovalCause.COLLECTED)); } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java index 1f37a1d6d5..3b36085b00 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RefreshAfterWriteTest.java @@ -163,7 +163,7 @@ public void get(LoadingCache cache, CacheContext context) { cache.get(context.absentKey()); context.ticker().advance(45, TimeUnit.SECONDS); - assertThat(cache.getIfPresent(context.absentKey()), is(-context.absentKey())); + assertThat(cache.getIfPresent(context.firstKey()), is(-context.firstKey())); assertThat(cache, hasRemovalNotifications(context, 1, RemovalCause.REPLACED)); } @@ -177,7 +177,7 @@ public void get(AsyncLoadingCache cache, CacheContext context) cache.get(context.absentKey()); context.ticker().advance(45, TimeUnit.SECONDS); - assertThat(cache.getIfPresent(context.absentKey()), is(futureOf(-context.absentKey()))); + assertThat(cache.getIfPresent(context.firstKey()), is(futureOf(-context.firstKey()))); assertThat(cache, hasRemovalNotifications(context, 1, RemovalCause.REPLACED)); } @@ -311,8 +311,8 @@ public void invalidate(CacheContext context) { refresh.set(true); await().until(() -> cache.getIfPresent(key), is(refreshed)); - assertThat(cache, hasRemovalNotifications(context, 1, RemovalCause.EXPLICIT)); - assertThat(context, both(hasLoadSuccessCount(1)).and(hasLoadFailureCount(0))); + await().until(() -> cache, hasRemovalNotifications(context, 1, RemovalCause.EXPLICIT)); + await().until(() -> context, both(hasLoadSuccessCount(1)).and(hasLoadFailureCount(0))); } /* ---------------- Policy -------------- */ diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java index 623880c789..4d9e9409fd 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java @@ -206,13 +206,12 @@ public void invalidateAll(Iterable keys) { @Override public void invalidateAll() { - invalidateAll(cache.asMap().keySet()); + cache.invalidateAll(); } @Override public long estimatedSize() { - // https://github.com/google/guava/issues/2108 - return Math.max(0L, cache.size()); + return cache.size(); } @Override @@ -236,11 +235,6 @@ public boolean containsValue(Object value) { return delegate().containsValue(value); } @Override - public int size() { - // https://github.com/google/guava/issues/2108 - return Math.max(0, delegate().size()); - } - @Override public void clear() { // https://github.com/google/guava/issues/2101 invalidateAll(); diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/HasRemovalNotifications.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/HasRemovalNotifications.java index 19fec0053b..93a410a29b 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/HasRemovalNotifications.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/HasRemovalNotifications.java @@ -17,7 +17,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -32,6 +31,7 @@ import org.hamcrest.TypeSafeDiagnosingMatcher; import com.github.benmanes.caffeine.cache.RemovalCause; +import com.github.benmanes.caffeine.cache.testing.CacheSpec.Listener; import com.github.benmanes.caffeine.cache.testing.RemovalListeners.ConsumingRemovalListener; import com.github.benmanes.caffeine.testing.DescriptionBuilder; @@ -61,14 +61,18 @@ public void describeTo(Description description) { protected boolean matchesSafely(Object ignored, Description description) { DescriptionBuilder desc = new DescriptionBuilder(description); - List> notifications = context.consumedNotifications(); - if (!notifications.isEmpty()) { + if (context.removalListenerType == Listener.CONSUMING) { + List> notifications = context.consumedNotifications(); ForkJoinPool.commonPool().awaitQuiescence(10, TimeUnit.SECONDS); - desc.expectThat("notification size", notifications, hasSize(count)); + int size = 0; for (RemovalNotification notification : notifications) { - checkNotification(notification); + if (notification.getCause() == cause) { + checkNotification(notification); + size++; + } } + desc.expectThat("notification count", size, is(count)); } return desc.matches(); diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 07395d207b..b00130b207 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -47,7 +47,7 @@ ext { jcache_tck: '1.0.1', jctools: '1.2.1', junit: '4.12', - mockito: '2.0.96-beta', + mockito: '2.0.100-beta', pax_exam: '4.9.1', testng: '6.9.12', truth: '0.24', @@ -57,9 +57,9 @@ ext { concurrentlinkedhashmap: '1.4.2', ehcache2: '2.10.2.2.21', ehcache3: '3.1.1', - elastic_search: '5.0.0-alpha4', - infinispan: '9.0.0.Alpha3', - jackrabbit: '1.5.6', + elastic_search: '5.0.0-alpha5', + infinispan: '9.0.0.Alpha4', + jackrabbit: '1.5.7', jamm: '0.3.1', java_object_layout: '0.5', koloboke: '0.6.8', diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index c6b2cfb2e3..be7efe5430 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-3.0-rc-1-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-3.0-rc-2-bin.zip