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 414b71a2bd..1e03becc11 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 @@ -40,7 +40,6 @@ import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; -import org.springframework.context.ApplicationContext; import org.springframework.context.expression.AnnotatedElementKey; import org.springframework.expression.EvaluationContext; import org.springframework.util.Assert; @@ -340,12 +339,12 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker Object key = generateKey(context, CacheOperationExpressionEvaluator.NO_RESULT); Cache cache = context.getCaches().iterator().next(); try { - return cache.get(key, new Callable() { + return wrapCacheValue(method, cache.get(key, new Callable() { @Override public Object call() throws Exception { - return invokeOperation(invoker); + return unwrapReturnValue(invokeOperation(invoker)); } - }); + })); } catch (Cache.ValueRetrievalException ex) { // The invoker wraps any Throwable in a ThrowableWrapper instance so we @@ -380,18 +379,12 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker if (cacheHit != null && cachePutRequests.isEmpty() && !hasCachePut(contexts)) { // If there are no put requests, just use the cache hit cacheValue = cacheHit.get(); - if (method.getReturnType() == Optional.class && - (cacheValue == null || cacheValue.getClass() != Optional.class)) { - returnValue = Optional.ofNullable(cacheValue); - } - else { - returnValue = cacheValue; - } + returnValue = wrapCacheValue(method, cacheValue); } else { // Invoke the method if we don't have a cache hit returnValue = invokeOperation(invoker); - cacheValue = ObjectUtils.unwrapOptional(returnValue); + cacheValue = unwrapReturnValue(returnValue); } // Collect any explicit @CachePuts @@ -408,6 +401,18 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker return returnValue; } + private Object wrapCacheValue(Method method, Object cacheValue) { + if (method.getReturnType() == Optional.class && + (cacheValue == null || cacheValue.getClass() != Optional.class)) { + return Optional.ofNullable(cacheValue); + } + return cacheValue; + } + + private Object unwrapReturnValue(Object returnValue) { + return ObjectUtils.unwrapOptional(returnValue); + } + private boolean hasCachePut(CacheOperationContexts contexts) { // Evaluate the conditions *without* the result object because we don't have it yet... Collection cachePutContexts = contexts.get(CachePutOperation.class); 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 b33b8be756..12939b9c68 100644 --- a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java +++ b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java @@ -149,6 +149,22 @@ public class CacheReproTests { assertSame(tb2, cache.get("tb1").get()); } + @Test + public void spr14853AdaptsToOptionalWithSync() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr14853Config.class); + Spr14853Service bean = context.getBean(Spr14853Service.class); + Cache cache = context.getBean(CacheManager.class).getCache("itemCache"); + + TestBean tb = new TestBean("tb1"); + bean.insertItem(tb); + assertSame(tb, bean.findById("tb1").get()); + assertSame(tb, cache.get("tb1").get()); + + cache.clear(); + TestBean tb2 = bean.findById("tb1").get(); + assertNotSame(tb, tb2); + assertSame(tb2, cache.get("tb1").get()); + } @Configuration @EnableCaching @@ -341,4 +357,33 @@ public class CacheReproTests { } } + public static class Spr14853Service { + + @Cacheable(value = "itemCache", sync = true) + public Optional findById(String id) { + return Optional.of(new TestBean(id)); + } + + @CachePut(cacheNames = "itemCache", key = "#item.name") + public TestBean insertItem(TestBean item) { + return item; + } + + } + + @Configuration + @EnableCaching + public static class Spr14853Config { + + @Bean + public CacheManager cacheManager() { + return new ConcurrentMapCacheManager(); + } + + @Bean + public Spr14853Service service() { + return new Spr14853Service(); + } + } + }