diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index ae8b1c6bef..8bd07dcdec 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -652,9 +652,8 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker @Nullable Object result, Collection putRequests) { for (CacheOperationContext context : contexts) { - if (isConditionPassing(context, result) && context.canPutToCache(result)) { - Object key = generateKey(context, result); - putRequests.add(new CachePutRequest(context, key)); + if (isConditionPassing(context, result)) { + putRequests.add(new CachePutRequest(context)); } } } @@ -943,22 +942,16 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker private final CacheOperationContext context; - private final Object key; - - public CachePutRequest(CacheOperationContext context, Object key) { + public CachePutRequest(CacheOperationContext context) { this.context = context; - this.key = key; } @Nullable public Object apply(@Nullable Object result) { if (result instanceof CompletableFuture future) { return future.whenComplete((value, ex) -> { - if (ex != null) { - performEvict(ex); - } - else { - performPut(value); + if (ex == null) { + performCachePut(value); } }); } @@ -968,27 +961,20 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker return returnValue; } } - performPut(result); + performCachePut(result); return null; } - void performPut(@Nullable Object value) { - if (logger.isTraceEnabled()) { - logger.trace("Creating cache entry for key '" + this.key + "' in cache(s) " + - this.context.getCacheNames()); - } - for (Cache cache : this.context.getCaches()) { - doPut(cache, this.key, value); - } - } - - void performEvict(Throwable cause) { - if (logger.isTraceEnabled()) { - logger.trace("Removing cache entry for key '" + this.key + "' from cache(s) " + - this.context.getCacheNames() + " due to exception: " + cause); - } - for (Cache cache : this.context.getCaches()) { - doEvict(cache, this.key, false); + public void performCachePut(@Nullable Object value) { + if (this.context.canPutToCache(value)) { + Object key = generateKey(this.context, value); + if (logger.isTraceEnabled()) { + logger.trace("Creating cache entry for key '" + key + "' in cache(s) " + + this.context.getCacheNames()); + } + for (Cache cache : this.context.getCaches()) { + doPut(cache, key, value); + } } } } @@ -1017,11 +1003,10 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } @Override public void onError(Throwable t) { - this.request.performEvict(t); } @Override public void onComplete() { - this.request.performPut(this.cacheValue); + this.request.performCachePut(this.cacheValue); } } @@ -1107,7 +1092,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } else { return adapter.fromPublisher(Mono.from(adapter.toPublisher(result)) - .doOnSuccess(request::performPut).doOnError(request::performEvict)); + .doOnSuccess(request::performCachePut)); } } return NOT_HANDLED; diff --git a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java index d1c8ff0bbb..c07b3f3dad 100644 --- a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java +++ b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java @@ -197,6 +197,11 @@ class CacheReproTests { assertThat(bean.findById("tb1").join()).isSameAs(tb); assertThat(cache.get("tb1").get()).isSameAs(tb); + tb = bean.findById("tb2").join(); + assertThat(tb).isNotNull(); + assertThat(bean.findById("tb2").join()).isNotSameAs(tb); + assertThat(cache.get("tb2")).isNull(); + context.close(); } @@ -247,6 +252,11 @@ class CacheReproTests { assertThat(bean.findById("tb1").block()).isSameAs(tb); assertThat(cache.get("tb1").get()).isSameAs(tb); + tb = bean.findById("tb2").block(); + assertThat(tb).isNotNull(); + assertThat(bean.findById("tb2").block()).isNotSameAs(tb); + assertThat(cache.get("tb2")).isNull(); + context.close(); } @@ -297,6 +307,11 @@ class CacheReproTests { assertThat(bean.findById("tb1").collectList().block()).isEqualTo(tb); assertThat(cache.get("tb1").get()).isEqualTo(tb); + tb = bean.findById("tb2").collectList().block(); + assertThat(tb).isNotEmpty(); + assertThat(bean.findById("tb2").collectList().block()).isNotEqualTo(tb); + assertThat(cache.get("tb2")).isNull(); + context.close(); } @@ -548,7 +563,7 @@ class CacheReproTests { public static class Spr14235FutureService { - @Cacheable(value = "itemCache") + @Cacheable(value = "itemCache", unless = "#result.name == 'tb2'") public CompletableFuture findById(String id) { return CompletableFuture.completedFuture(new TestBean(id)); } @@ -581,7 +596,7 @@ class CacheReproTests { public static class Spr14235MonoService { - @Cacheable(value = "itemCache") + @Cacheable(value = "itemCache", unless = "#result.name == 'tb2'") public Mono findById(String id) { return Mono.just(new TestBean(id)); } @@ -616,7 +631,7 @@ class CacheReproTests { private int counter = 0; - @Cacheable(value = "itemCache") + @Cacheable(value = "itemCache", unless = "#result[0].name == 'tb2'") public Flux findById(String id) { return Flux.just(new TestBean(id), new TestBean(id + (counter++))); }