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 572d3295db..53dd096034 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 @@ -401,13 +401,6 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker // Check if we have a cached item matching the conditions Cache.ValueWrapper cacheHit = findCachedItem(contexts.get(CacheableOperation.class)); - // Collect puts from any @Cacheable miss, if no cached item is found - List cachePutRequests = new ArrayList<>(); - if (cacheHit == null) { - collectPutRequests(contexts.get(CacheableOperation.class), - CacheOperationExpressionEvaluator.NO_RESULT, cachePutRequests); - } - Object cacheValue; Object returnValue; @@ -422,6 +415,12 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker cacheValue = unwrapReturnValue(returnValue); } + // Collect puts from any @Cacheable miss, if no cached item is found + List cachePutRequests = new ArrayList<>(); + if (cacheHit == null) { + collectPutRequests(contexts.get(CacheableOperation.class), cacheValue, cachePutRequests); + } + // Collect any explicit @CachePuts collectPutRequests(contexts.get(CachePutOperation.class), cacheValue, cachePutRequests); @@ -558,7 +557,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker @Nullable Object result, Collection putRequests) { for (CacheOperationContext context : contexts) { - if (isConditionPassing(context, result)) { + if (isConditionPassing(context, result) && context.canPutToCache(result)) { Object key = generateKey(context, result); putRequests.add(new CachePutRequest(context, key)); } @@ -832,10 +831,8 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } public void apply(@Nullable Object result) { - if (this.context.canPutToCache(result)) { - for (Cache cache : this.context.getCaches()) { - doPut(cache, this.key, result); - } + for (Cache cache : this.context.getCaches()) { + doPut(cache, this.key, result); } } } diff --git a/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml b/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml index ea6901d7a2..eb9ffce842 100644 --- a/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml +++ b/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml @@ -54,6 +54,7 @@ + @@ -93,6 +94,7 @@ + diff --git a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/AbstractCacheAnnotationTests.java b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/AbstractCacheAnnotationTests.java index 66c3a66c7e..149636bf13 100644 --- a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/AbstractCacheAnnotationTests.java +++ b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/AbstractCacheAnnotationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -504,6 +504,26 @@ public abstract class AbstractCacheAnnotationTests { assertThat(primary.get(id).get()).isSameAs(entity); } + protected void testPutRefersToResultWithUnless(CacheableService service) { + Long id = 42L; + TestEntity entity = new TestEntity(); + entity.setId(id); + Cache primary = this.cm.getCache("primary"); + assertThat(primary.get(id)).isNull(); + assertThat(service.putEvaluatesUnlessBeforeKey(entity)).isNotNull(); + assertThat(primary.get(id).get()).isSameAs(entity); + } + + protected void testPutEvaluatesUnlessBeforeKey(CacheableService service) { + Long id = Long.MIN_VALUE; // return null + TestEntity entity = new TestEntity(); + entity.setId(id); + Cache primary = this.cm.getCache("primary"); + assertThat(primary.get(id)).isNull(); + assertThat(service.putEvaluatesUnlessBeforeKey(entity)).isNull(); + assertThat(primary.get(id)).isNull(); + } + protected void testMultiCacheAndEvict(CacheableService service) { String methodName = "multiCacheAndEvict"; @@ -849,11 +869,31 @@ public abstract class AbstractCacheAnnotationTests { testPutRefersToResult(this.cs); } + @Test + public void testPutRefersToResultWithUnless() { + testPutRefersToResultWithUnless(this.cs); + } + + @Test + public void testPutEvaluatesUnlessBeforeKey() { + testPutEvaluatesUnlessBeforeKey(this.cs); + } + @Test public void testClassPutRefersToResult() { testPutRefersToResult(this.ccs); } + @Test + public void testClassPutRefersToResultWithUnless(){ + testPutRefersToResultWithUnless(this.ccs); + } + + @Test + public void testClassPutEvaluatesUnlessBeforeKey(){ + testPutEvaluatesUnlessBeforeKey(this.ccs); + } + @Test public void testMultiCacheAndEvict() { testMultiCacheAndEvict(this.cs); diff --git a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/AnnotatedClassCacheableService.java b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/AnnotatedClassCacheableService.java index aaf8fb6881..0f9c920112 100644 --- a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/AnnotatedClassCacheableService.java +++ b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/AnnotatedClassCacheableService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -235,4 +235,10 @@ public class AnnotatedClassCacheableService implements CacheableService return arg1; } + @Override + @CachePut(cacheNames = "primary", key = "#result.id", unless = "#result == null") + public TestEntity putEvaluatesUnlessBeforeKey(TestEntity arg1) { + return (arg1.getId() != Long.MIN_VALUE ? arg1 : null); + } + } diff --git a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/CacheableService.java b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/CacheableService.java index 06afcea22f..0bcaed01b9 100644 --- a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/CacheableService.java +++ b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/CacheableService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -93,4 +93,6 @@ public interface CacheableService { TestEntity putRefersToResult(TestEntity arg1); + TestEntity putEvaluatesUnlessBeforeKey(TestEntity arg1); + } diff --git a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/DefaultCacheableService.java b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/DefaultCacheableService.java index 017a719173..87275a13d1 100644 --- a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/DefaultCacheableService.java +++ b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/cache/beans/DefaultCacheableService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -243,4 +243,10 @@ public class DefaultCacheableService implements CacheableService { return arg1; } + @Override + @CachePut(cacheNames = "primary", key = "#result.id", unless = "#result == null") + public TestEntity putEvaluatesUnlessBeforeKey(TestEntity arg1) { + return (arg1.getId() != Long.MIN_VALUE ? arg1 : null); + } + }