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 df2ec4703e..8f83a8f94c 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 @@ -1274,7 +1274,7 @@ boolean skipReadBuffer() { if (Async.isReady(future)) { @SuppressWarnings("NullAway") var refresh = cacheLoader.asyncReload(key, future.join(), executor); - refreshFuture[0] = refresh; + refreshFuture[0] = requireNonNull(refresh, "Null future"); } else { // no-op if load is pending return future; @@ -1282,7 +1282,7 @@ boolean skipReadBuffer() { } else { @SuppressWarnings("NullAway") var refresh = cacheLoader.asyncReload(key, oldValue, executor); - refreshFuture[0] = refresh; + refreshFuture[0] = requireNonNull(refresh, "Null future"); } return refreshFuture[0]; } catch (InterruptedException e) { diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java index 8d6a233a80..31856959a1 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java @@ -264,7 +264,9 @@ public CompletableFuture> refreshAll(Iterable keys) { refreshed[0] = true; startTime[0] = asyncCache.cache().statsTicker().read(); try { - return asyncCache.cacheLoader.asyncReload(key, oldValue, asyncCache.cache().executor()); + var reloadFuture = asyncCache.cacheLoader.asyncReload( + key, oldValue, asyncCache.cache().executor()); + return requireNonNull(reloadFuture, "Null future"); } catch (RuntimeException e) { throw e; } catch (InterruptedException e) { @@ -302,7 +304,7 @@ public CompletableFuture> refreshAll(Iterable keys) { // If the entry is absent then discard the refresh and maybe notifying the listener discard[0] = (newValue != null); return null; - } else if (currentValue == newValue) { + } else if ((currentValue == newValue) || (currentValue == castedFuture)) { // If the reloaded value is the same instance then no-op return currentValue; } else if (newValue == Async.getIfReady((CompletableFuture) currentValue)) { 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 index bbe706c78d..9a0552cf5e 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java @@ -116,7 +116,7 @@ default CompletableFuture refresh(K key) { var refreshFuture = (oldValue[0] == null) ? cacheLoader().asyncLoad(key, cache().executor()) : cacheLoader().asyncReload(key, oldValue[0], cache().executor()); - reloading[0] = refreshFuture; + reloading[0] = requireNonNull(refreshFuture, "Null future"); return refreshFuture; } catch (RuntimeException e) { throw e; 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 index e7193b21ac..8f00e254ba 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncLoadingCacheTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncLoadingCacheTest.java @@ -448,6 +448,29 @@ public void refresh(CacheContext context) { await().untilAsserted(() -> assertThat(cache).containsEntry(key, key.negate())); } + @Test(dataProvider = "caches", expectedExceptions = NullPointerException.class) + @CacheSpec(population = Population.EMPTY, compute = Compute.ASYNC) + public void refresh_nullFuture_load(CacheContext context) { + var cache = context.buildAsync((Int key, Executor executor) -> null); + cache.synchronous().refresh(context.absentKey()); + } + + @Test(dataProvider = "caches", expectedExceptions = NullPointerException.class) + @CacheSpec(population = Population.EMPTY, compute = Compute.ASYNC) + public void refresh_nullFuture_reload(CacheContext context) { + var cache = context.buildAsync(new AsyncCacheLoader() { + @Override public CompletableFuture asyncLoad(Int key, Executor executor) { + throw new IllegalStateException(); + } + @Override public CompletableFuture asyncReload( + Int key, Int oldValue, Executor executor) { + return null; + } + }); + cache.synchronous().put(context.absentKey(), context.absentValue()); + cache.synchronous().refresh(context.absentKey()); + } + @Test(dataProvider = "caches", timeOut = 5_000) // Issue #69 @CacheSpec(population = Population.EMPTY, compute = Compute.ASYNC, executor = CacheExecutor.THREADED) @@ -507,6 +530,66 @@ public void refresh_interrupted(AsyncLoadingCache cache, CacheContext } } + @CacheSpec + @Test(dataProvider = "caches") + public void refresh_current_inFlight(AsyncLoadingCache cache, CacheContext context) { + var future = new CompletableFuture(); + cache.put(context.absentKey(), future); + cache.synchronous().refresh(context.absentKey()); + assertThat(cache).containsEntry(context.absentKey(), future); + assertThat(cache.synchronous().policy().refreshes()).isEmpty(); + future.complete(context.absentValue()); + } + + @Test(dataProvider = "caches") + @CacheSpec(compute = Compute.ASYNC, removalListener = Listener.CONSUMING) + public void refresh_current_sameInstance(CacheContext context) { + var future = context.absentValue().asFuture(); + var cache = context.buildAsync((key, executor) -> future); + + cache.put(context.absentKey(), future); + cache.synchronous().refresh(context.absentKey()); + assertThat(context).notifications().isEmpty(); + } + + @CacheSpec + @Test(dataProvider = "caches") + public void refresh_current_failed(AsyncLoadingCache cache, CacheContext context) { + var future = context.absentValue().asFuture(); + cache.put(context.absentKey(), future); + + future.obtrudeException(new Exception()); + assertThat(cache.asMap()).containsKey(context.absentKey()); + + cache.synchronous().refresh(context.absentKey()); + assertThat(cache).containsEntry(context.absentKey(), context.absentValue()); + } + + @Test(dataProvider = "caches") + @CacheSpec(compute = Compute.ASYNC, + removalListener = Listener.CONSUMING, executor = CacheExecutor.THREADED) + public void refresh_current_removed(CacheContext context) { + var started = new AtomicBoolean(); + var done = new AtomicBoolean(); + var cache = context.buildAsync((Int key) -> { + started.set(true); + await().untilTrue(done); + return key; + }); + + cache.put(context.absentKey(), context.absentValue().asFuture()); + cache.synchronous().refresh(context.absentKey()); + await().untilTrue(started); + + cache.synchronous().invalidate(context.absentKey()); + done.set(true); + + await().untilAsserted(() -> { + assertThat(context).removalNotifications().containsExactlyValues( + context.absentKey(), context.absentValue()); + }); + } + /* --------------- AsyncCacheLoader --------------- */ @Test(expectedExceptions = UnsupportedOperationException.class) 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 index dd154a6d20..47479b5f89 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/LoadingCacheTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/LoadingCacheTest.java @@ -930,6 +930,36 @@ public void refresh_error_log(CacheContext context) throws Exception { assertThat(event.getLevel()).isEqualTo(WARN); } + @Test(dataProvider = "caches", expectedExceptions = NullPointerException.class) + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY) + public void refresh_nullFuture_load(CacheContext context) { + var cache = context.build(new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override public CompletableFuture asyncLoad(Int key, Executor executor) { + return null; + } + }); + cache.refresh(context.absentKey()); + } + + @Test(dataProvider = "caches", expectedExceptions = NullPointerException.class) + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY) + public void refresh_nullFuture_reload(CacheContext context) { + var cache = context.build(new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override public CompletableFuture asyncReload( + Int key, Int oldValue, Executor executor) { + return null; + } + }); + cache.put(context.absentKey(), context.absentValue()); + cache.refresh(context.absentKey()); + } + /* --------------- refreshAll --------------- */ @CheckNoEvictions @CheckNoStats @@ -1012,6 +1042,36 @@ public void refreshAll_cancel(LoadingCache cache, CacheContext context assertThat(cache).containsExactlyEntriesIn(context.original()); } + @Test(dataProvider = "caches", expectedExceptions = NullPointerException.class) + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY) + public void refreshAll_nullFuture_load(CacheContext context) { + var cache = context.build(new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override public CompletableFuture asyncLoad(Int key, Executor executor) { + return null; + } + }); + cache.refreshAll(context.absent().keySet()); + } + + @Test(dataProvider = "caches", expectedExceptions = NullPointerException.class) + @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY) + public void refreshAll_nullFuture_reload(CacheContext context) { + var cache = context.build(new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override public CompletableFuture asyncReload( + Int key, Int oldValue, Executor executor) { + return null; + } + }); + cache.put(context.absentKey(), context.absentValue()); + cache.refreshAll(context.absent().keySet()); + } + /* --------------- CacheLoader --------------- */ @Test(expectedExceptions = UnsupportedOperationException.class) 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 923562a3e0..66ea29c138 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 @@ -357,6 +357,40 @@ public void refreshIfNeeded_error_log(CacheContext context) { assertThat(event.getLevel()).isEqualTo(WARN); } + @Test(dataProvider = "caches") + @CacheSpec(implementation = Implementation.Caffeine, + population = Population.EMPTY, refreshAfterWrite = Expire.ONE_MINUTE) + public void refreshIfNeeded_nullFuture(CacheContext context) { + var refreshed = new AtomicBoolean(); + CacheLoader loader = new CacheLoader() { + @Override public Int load(Int key) { + throw new IllegalStateException(); + } + @Override public CompletableFuture asyncReload( + Int key, Int oldValue, Executor executor) { + refreshed.set(true); + return null; + } + }; + TestLoggerFactory.getAllTestLoggers().values() + .forEach(logger -> logger.setEnabledLevels(INFO_LEVELS)); + + var cache = context.isAsync() + ? context.buildAsync(loader).synchronous() + : context.build(loader); + cache.put(context.absentKey(), context.absentValue()); + context.ticker().advance(2, TimeUnit.MINUTES); + cache.get(context.absentKey()); + + var event = Iterables.getOnlyElement(TestLoggerFactory.getLoggingEvents()); + assertThat(event.getThrowable().orElseThrow()).isInstanceOf(NullPointerException.class); + assertThat(event.getLevel()).isEqualTo(WARN); + + assertThat(refreshed.get()).isTrue(); + assertThat(cache.policy().refreshes()).isEmpty(); + assertThat(cache).containsEntry(context.absentKey(), context.absentValue()); + } + /* --------------- getIfPresent --------------- */ @CheckNoEvictions