diff --git a/spring-aspects/src/main/java/org/springframework/cache/aspectj/AbstractCacheAspect.aj b/spring-aspects/src/main/java/org/springframework/cache/aspectj/AbstractCacheAspect.aj index 20f2adb0a5..3b4b382031 100644 --- a/spring-aspects/src/main/java/org/springframework/cache/aspectj/AbstractCacheAspect.aj +++ b/spring-aspects/src/main/java/org/springframework/cache/aspectj/AbstractCacheAspect.aj @@ -67,11 +67,22 @@ public abstract aspect AbstractCacheAspect extends CacheAspectSupport implements CacheOperationInvoker aspectJInvoker = new CacheOperationInvoker() { public Object invoke() { - return proceed(cachedObject); + try { + return proceed(cachedObject); + } + catch (Throwable ex) { + throw new ThrowableWrapper(ex); + } } }; - return execute(aspectJInvoker, thisJoinPoint.getTarget(), method, thisJoinPoint.getArgs()); + try { + return execute(aspectJInvoker, thisJoinPoint.getTarget(), method, thisJoinPoint.getArgs()); + } + catch (CacheOperationInvoker.ThrowableWrapper th) { + AnyThrow.throwUnchecked(th.getOriginal()); + return null; // never reached + } } /** diff --git a/spring-aspects/src/main/java/org/springframework/cache/aspectj/AnyThrow.java b/spring-aspects/src/main/java/org/springframework/cache/aspectj/AnyThrow.java new file mode 100644 index 0000000000..d391c0a6bc --- /dev/null +++ b/spring-aspects/src/main/java/org/springframework/cache/aspectj/AnyThrow.java @@ -0,0 +1,35 @@ +/* + * Copyright 2002-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cache.aspectj; + +/** + * Utility to trick the compiler to throw a valid checked + * exceptions within the interceptor. + * + * @author Stephane Nicoll + */ +class AnyThrow { + + static void throwUnchecked(Throwable e) { + AnyThrow.throwAny(e); + } + + @SuppressWarnings("unchecked") + private static void throwAny(Throwable e) throws E { + throw (E) e; + } +} diff --git a/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj b/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj index 3aeeefbd0e..4587fc00c0 100644 --- a/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj +++ b/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj @@ -109,16 +109,4 @@ public aspect JCacheCacheAspect extends JCacheAspectSupport { execution(@CacheRemoveAll * *(..)); - private static class AnyThrow { - - private static void throwUnchecked(Throwable e) { - AnyThrow.throwAny(e); - } - - @SuppressWarnings("unchecked") - private static void throwAny(Throwable e) throws E { - throw (E)e; - } - } - } diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java index 224f7ffa95..521e22bc2d 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java @@ -27,6 +27,7 @@ import org.springframework.cache.annotation.Caching; /** * @author Costin Leau * @author Phillip Webb + * @author Stephane Nicoll */ @Cacheable("testCache") public class AnnotatedClassCacheableService implements CacheableService { @@ -44,11 +45,28 @@ public class AnnotatedClassCacheableService implements CacheableService return null; } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object cacheSync(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object cacheSyncNull(Object arg1) { + return null; + } + @Override public Object conditional(int field) { return null; } + @Override + public Object conditionalSync(int field) { + return null; + } + @Override public Object unless(int arg) { return arg; @@ -168,6 +186,18 @@ public class AnnotatedClassCacheableService implements CacheableService throw new UnsupportedOperationException(arg1.toString()); } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object throwCheckedSync(Object arg1) throws Exception { + throw new IOException(arg1.toString()); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object throwUncheckedSync(Object arg1) { + throw new UnsupportedOperationException(arg1.toString()); + } + // multi annotations @Override diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java index 17d299d915..a862f2acbf 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java @@ -21,6 +21,7 @@ package org.springframework.cache.config; * * @author Costin Leau * @author Phillip Webb + * @author Stephane Nicoll */ public interface CacheableService { @@ -28,6 +29,10 @@ public interface CacheableService { T cacheNull(Object arg1); + T cacheSync(Object arg1); + + T cacheSyncNull(Object arg1); + void invalidate(Object arg1); void evictEarly(Object arg1); @@ -42,6 +47,8 @@ public interface CacheableService { T conditional(int field); + T conditionalSync(int field); + T unless(int arg); T key(Object arg1, Object arg2); @@ -72,6 +79,10 @@ public interface CacheableService { T throwUnchecked(Object arg1); + T throwCheckedSync(Object arg1) throws Exception; + + T throwUncheckedSync(Object arg1); + // multi annotations T multiCache(Object arg1); diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java index c2c923f531..e801647ed5 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java @@ -29,6 +29,7 @@ import org.springframework.cache.annotation.Caching; * * @author Costin Leau * @author Phillip Webb + * @author Stephane Nicoll */ public class DefaultCacheableService implements CacheableService { @@ -47,6 +48,18 @@ public class DefaultCacheableService implements CacheableService { return null; } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long cacheSync(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long cacheSyncNull(Object arg1) { + return null; + } + @Override @CacheEvict("testCache") public void invalidate(Object arg1) { @@ -81,11 +94,17 @@ public class DefaultCacheableService implements CacheableService { } @Override - @Cacheable(cacheNames = "testCache", condition = "#classField == 3") + @Cacheable(cacheNames = "testCache", condition = "#p0 == 3") public Long conditional(int classField) { return counter.getAndIncrement(); } + @Override + @Cacheable(cacheNames = "testCache", sync = true, condition = "#p0 == 3") + public Long conditionalSync(int field) { + return counter.getAndIncrement(); + } + @Override @Cacheable(cacheNames = "testCache", unless = "#result > 10") public Long unless(int arg) { @@ -99,7 +118,7 @@ public class DefaultCacheableService implements CacheableService { } @Override - @Cacheable("testCache") + @Cacheable(cacheNames = "testCache") public Long varArgsKey(Object... args) { return counter.getAndIncrement(); } @@ -176,6 +195,18 @@ public class DefaultCacheableService implements CacheableService { throw new UnsupportedOperationException(arg1.toString()); } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long throwCheckedSync(Object arg1) throws Exception { + throw new IOException(arg1.toString()); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long throwUncheckedSync(Object arg1) { + throw new UnsupportedOperationException(arg1.toString()); + } + // multi annotations @Override diff --git a/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCache.java b/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCache.java index 2bc076cc18..7eaa0423c1 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCache.java +++ b/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCache.java @@ -16,6 +16,7 @@ package org.springframework.cache.caffeine; +import java.util.concurrent.Callable; import java.util.function.Function; import com.github.benmanes.caffeine.cache.LoadingCache; @@ -88,6 +89,12 @@ public class CaffeineCache extends AbstractValueAdaptingCache { return super.get(key); } + @SuppressWarnings("unchecked") + @Override + public T get(Object key, final Callable valueLoader) { + return (T) fromStoreValue(this.cache.get(key, new LoadFunction(valueLoader))); + } + @Override protected Object lookup(Object key) { return this.cache.getIfPresent(key); @@ -133,4 +140,23 @@ public class CaffeineCache extends AbstractValueAdaptingCache { } } + private class LoadFunction implements Function { + + private final Callable valueLoader; + + public LoadFunction(Callable valueLoader) { + this.valueLoader = valueLoader; + } + + @Override + public Object apply(Object o) { + try { + return toStoreValue(valueLoader.call()); + } + catch (Exception ex) { + throw new ValueRetrievalException(o, valueLoader, ex); + } + } + } + } diff --git a/spring-context-support/src/main/java/org/springframework/cache/ehcache/EhCacheCache.java b/spring-context-support/src/main/java/org/springframework/cache/ehcache/EhCacheCache.java index 486634146d..f1b7fd9274 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/ehcache/EhCacheCache.java +++ b/spring-context-support/src/main/java/org/springframework/cache/ehcache/EhCacheCache.java @@ -16,6 +16,8 @@ package org.springframework.cache.ehcache; +import java.util.concurrent.Callable; + import net.sf.ehcache.Ehcache; import net.sf.ehcache.Element; import net.sf.ehcache.Status; @@ -62,10 +64,47 @@ public class EhCacheCache implements Cache { @Override public ValueWrapper get(Object key) { - Element element = this.cache.get(key); + Element element = lookup(key); return toValueWrapper(element); } + @SuppressWarnings("unchecked") + @Override + public T get(Object key, Callable valueLoader) { + Element element = lookup(key); + if (element != null) { + return (T) element.getObjectValue(); + } + else { + this.cache.acquireWriteLockOnKey(key); + try { + element = lookup(key); // One more attempt with the write lock + if (element != null) { + return (T) element.getObjectValue(); + } + else { + return loadValue(key, valueLoader); + } + } + finally { + this.cache.releaseWriteLockOnKey(key); + } + } + + } + + private T loadValue(Object key, Callable valueLoader) { + T value; + try { + value = valueLoader.call(); + } + catch (Exception ex) { + throw new ValueRetrievalException(key, valueLoader, ex); + } + put(key, value); + return value; + } + @Override @SuppressWarnings("unchecked") public T get(Object key, Class type) { @@ -98,6 +137,11 @@ public class EhCacheCache implements Cache { this.cache.removeAll(); } + + private Element lookup(Object key) { + return this.cache.get(key); + } + private ValueWrapper toValueWrapper(Element element) { return (element != null ? new SimpleValueWrapper(element.getObjectValue()) : null); } diff --git a/spring-context-support/src/main/java/org/springframework/cache/guava/GuavaCache.java b/spring-context-support/src/main/java/org/springframework/cache/guava/GuavaCache.java index 55e3c9a387..b9900c246b 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/guava/GuavaCache.java +++ b/spring-context-support/src/main/java/org/springframework/cache/guava/GuavaCache.java @@ -93,6 +93,25 @@ public class GuavaCache extends AbstractValueAdaptingCache { return super.get(key); } + @SuppressWarnings("unchecked") + @Override + public T get(Object key, final Callable valueLoader) { + try { + return (T) fromStoreValue(this.cache.get(key, new Callable() { + @Override + public Object call() throws Exception { + return toStoreValue(valueLoader.call()); + } + })); + } + catch (ExecutionException ex) { + throw new ValueRetrievalException(key, valueLoader, ex.getCause()); + } + catch (UncheckedExecutionException ex) { + throw new ValueRetrievalException(key, valueLoader, ex.getCause()); + } + } + @Override protected Object lookup(Object key) { return this.cache.getIfPresent(key); diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/JCacheCache.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/JCacheCache.java index d149a60ba3..c80330846e 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/JCacheCache.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/JCacheCache.java @@ -16,6 +16,11 @@ package org.springframework.cache.jcache; +import java.util.concurrent.Callable; +import javax.cache.processor.EntryProcessor; +import javax.cache.processor.EntryProcessorException; +import javax.cache.processor.MutableEntry; + import org.springframework.cache.support.AbstractValueAdaptingCache; import org.springframework.util.Assert; @@ -69,6 +74,16 @@ public class JCacheCache extends AbstractValueAdaptingCache { return this.cache.get(key); } + @Override + public T get(Object key, Callable valueLoader) { + try { + return this.cache.invoke(key, new ValueLoaderEntryProcessor(), valueLoader); + } + catch (EntryProcessorException ex) { + throw new ValueRetrievalException(key, valueLoader, ex.getCause()); + } + } + @Override public void put(Object key, Object value) { this.cache.put(key, toStoreValue(value)); @@ -90,4 +105,30 @@ public class JCacheCache extends AbstractValueAdaptingCache { this.cache.removeAll(); } + + private class ValueLoaderEntryProcessor implements EntryProcessor { + + @SuppressWarnings("unchecked") + @Override + public T process(MutableEntry entry, Object... arguments) + throws EntryProcessorException { + Callable valueLoader = (Callable) arguments[0]; + if (entry.exists()) { + return (T) fromStoreValue(entry.getValue()); + } + else { + T value; + try { + value = valueLoader.call(); + } + catch (Exception ex) { + throw new EntryProcessorException("Value loader '" + valueLoader + "' failed " + + "to compute value for key '" + entry.getKey() + "'", ex); + } + entry.setValue(toStoreValue(value)); + return value; + } + } + } + } diff --git a/spring-context-support/src/main/java/org/springframework/cache/transaction/TransactionAwareCacheDecorator.java b/spring-context-support/src/main/java/org/springframework/cache/transaction/TransactionAwareCacheDecorator.java index 4c5a7fdb17..f96c04c071 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/transaction/TransactionAwareCacheDecorator.java +++ b/spring-context-support/src/main/java/org/springframework/cache/transaction/TransactionAwareCacheDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -16,6 +16,8 @@ package org.springframework.cache.transaction; +import java.util.concurrent.Callable; + import org.springframework.cache.Cache; import org.springframework.transaction.support.TransactionSynchronizationAdapter; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -72,6 +74,11 @@ public class TransactionAwareCacheDecorator implements Cache { return this.targetCache.get(key, type); } + @Override + public T get(Object key, Callable valueLoader) { + return this.targetCache.get(key, valueLoader); + } + @Override public void put(final Object key, final Object value) { if (TransactionSynchronizationManager.isSynchronizationActive()) { diff --git a/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java b/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java index 57aad25908..5ede180c4f 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java @@ -52,7 +52,7 @@ public class CaffeineCacheTests extends AbstractCacheTests { } @Test - public void putIfAbsentNullValue() throws Exception { + public void testPutIfAbsentNullValue() throws Exception { CaffeineCache cache = getCache(); Object key = new Object(); @@ -66,4 +66,5 @@ public class CaffeineCacheTests extends AbstractCacheTests { assertEquals(null, wrapper.get()); assertEquals(value, cache.get(key).get()); // not changed } + } diff --git a/spring-context-support/src/test/java/org/springframework/cache/ehcache/EhCacheCacheTests.java b/spring-context-support/src/test/java/org/springframework/cache/ehcache/EhCacheCacheTests.java index 8d1357552d..f8541b07c7 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/ehcache/EhCacheCacheTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/ehcache/EhCacheCacheTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2010-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -82,4 +82,5 @@ public class EhCacheCacheTests extends AbstractCacheTests { Thread.sleep(5 * 1000); assertNull(cache.get(key)); } + } diff --git a/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheTests.java b/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheTests.java index f9964e031f..d50213c001 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -64,4 +64,5 @@ public class GuavaCacheTests extends AbstractCacheTests { assertEquals(null, wrapper.get()); assertEquals(value, cache.get(key).get()); // not changed } + } diff --git a/spring-context/src/main/java/org/springframework/cache/Cache.java b/spring-context/src/main/java/org/springframework/cache/Cache.java index 3e8a797876..c3d5dca64a 100644 --- a/spring-context/src/main/java/org/springframework/cache/Cache.java +++ b/spring-context/src/main/java/org/springframework/cache/Cache.java @@ -16,6 +16,8 @@ package org.springframework.cache; +import java.util.concurrent.Callable; + /** * Interface that defines common cache operations. * @@ -73,6 +75,23 @@ public interface Cache { */ T get(Object key, Class type); + /** + * Return the value to which this cache maps the specified key, obtaining + * that value from {@code valueLoader} if necessary. This method provides + * a simple substitute for the conventional "if cached, return; otherwise + * create, cache and return" pattern. + *

If possible, implementations should ensure that the loading operation + * is synchronized so that the specified {@code valueLoader} is only called + * once in case of concurrent access on the same key. + *

If the {@code valueLoader} throws an exception, it is wrapped in + * a {@link ValueRetrievalException} + * @param key the key whose associated value is to be returned + * @return the value to which this cache maps the specified key + * @throws ValueRetrievalException if the {@code valueLoader} throws an exception + * @since 4.3 + */ + T get(Object key, Callable valueLoader); + /** * Associate the specified value with the specified key in this cache. *

If the cache previously contained a mapping for this key, the old @@ -133,4 +152,24 @@ public interface Cache { Object get(); } + /** + * TODO + */ + @SuppressWarnings("serial") + class ValueRetrievalException extends RuntimeException { + + private final Object key; + + public ValueRetrievalException(Object key, Callable loader, Throwable ex) { + super(String.format("Value for key '%s' could not " + + "be loaded using '%s'", key, loader), ex); + this.key = key; + } + + public Object getKey() { + return key; + } + + } + } diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java b/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java index 055b49dee3..c258fab647 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java @@ -22,6 +22,7 @@ import java.lang.annotation.Inherited; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.concurrent.Callable; import org.springframework.core.annotation.AliasFor; @@ -154,4 +155,21 @@ public @interface Cacheable { */ String unless() default ""; + /** + * Synchronize the invocation of the underlying method if several threads are + * attempting to load a value for the same key. The synchronization leads to + * a couple of limitations: + *

    + *
  1. {@link #unless()} is not supported
    2. + *
  2. Only one cache may be specified
    3. + *
  3. No other cache-related operation can be combined
    4. + *
+ * This is effectively a hint and the actual cache provider that you are + * using may not support it in a synchronized fashion. Check your provider + * documentation for more details on the actual semantics. + * @since 4.3 + * @see org.springframework.cache.Cache#get(Object, Callable) + */ + boolean sync() default false; + } diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java b/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java index 7a1011405f..f982b388a0 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java @@ -107,6 +107,7 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria op.setKeyGenerator(cacheable.keyGenerator()); op.setCacheManager(cacheable.cacheManager()); op.setCacheResolver(cacheable.cacheResolver()); + op.setSync(cacheable.sync()); op.setName(ae.toString()); defaultConfig.applyDefault(op); diff --git a/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java b/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java index 4942e14c0f..b8aba44242 100644 --- a/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java +++ b/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java @@ -16,6 +16,7 @@ package org.springframework.cache.concurrent; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -96,6 +97,30 @@ public class ConcurrentMapCache extends AbstractValueAdaptingCache { return this.store.get(key); } + @SuppressWarnings("unchecked") + @Override + public T get(Object key, Callable valueLoader) { + if (this.store.containsKey(key)) { + return (T) get(key).get(); + } + else { + synchronized (this.store) { + if (this.store.containsKey(key)) { + return (T) get(key).get(); + } + T value; + try { + value = valueLoader.call(); + } + catch (Exception ex) { + throw new ValueRetrievalException(key, valueLoader, ex); + } + put(key, value); + return value; + } + } + } + @Override public void put(Object key, Object value) { this.store.put(key, toStoreValue(value)); diff --git a/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java b/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java index 07bcea27aa..f9ec32f474 100644 --- a/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java +++ b/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -109,6 +109,7 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser { nameHolder.setSource(parserContext.extractSource(opElement)); CacheableOperation op = prop.merge(opElement, parserContext.getReaderContext(), new CacheableOperation()); op.setUnless(getAttributeValue(opElement, "unless", "")); + op.setSync(Boolean.valueOf(getAttributeValue(opElement, "sync", "false"))); Collection col = cacheOpMap.get(nameHolder); if (col == null) { 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 ecec4ffda9..df461d895f 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 @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.logging.Log; @@ -328,7 +329,34 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker return targetClass; } - private Object execute(CacheOperationInvoker invoker, CacheOperationContexts contexts) { + private Object execute(final CacheOperationInvoker invoker, CacheOperationContexts contexts) { + // Special handling of synchronized invocation + if (contexts.isSynchronized()) { + CacheOperationContext context = contexts.get(CacheableOperation.class).iterator().next(); + if (isConditionPassing(context, ExpressionEvaluator.NO_RESULT)) { + Object key = generateKey(context, ExpressionEvaluator.NO_RESULT); + Cache cache = context.getCaches().iterator().next(); + try { + return cache.get(key, new Callable() { + @Override + public Object call() throws Exception { + return invokeOperation(invoker); + } + }); + } + catch (Cache.ValueRetrievalException ex) { + // The invoker wraps any Throwable in a ThrowableWrapper instance so we + // can just make sure that one bubbles up the stack. + throw (CacheOperationInvoker.ThrowableWrapper) ex.getCause(); + } + } + else { + // No caching required, only call the underlying method + return invokeOperation(invoker); + } + } + + // Process any early evictions processCacheEvicts(contexts.get(CacheEvictOperation.class), true, ExpressionEvaluator.NO_RESULT); @@ -374,7 +402,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker for (CacheOperationContext context : cachePutContexts) { try { if (!context.isConditionPassing(ExpressionEvaluator.RESULT_UNAVAILABLE)) { - excluded.add(context); + excluded.add(context); } } catch (VariableNotAvailableException e) { @@ -385,7 +413,6 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker // check if all puts have been excluded by condition return cachePutContexts.size() != excluded.size(); - } private void processCacheEvicts(Collection contexts, boolean beforeInvocation, Object result) { @@ -504,18 +531,57 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker private final MultiValueMap, CacheOperationContext> contexts = new LinkedMultiValueMap, CacheOperationContext>(); + private final boolean sync; + public CacheOperationContexts(Collection operations, Method method, Object[] args, Object target, Class targetClass) { for (CacheOperation operation : operations) { this.contexts.add(operation.getClass(), getOperationContext(operation, method, args, target, targetClass)); } + this.sync = determineSyncFlag(method); } public Collection get(Class operationClass) { Collection result = this.contexts.get(operationClass); return (result != null ? result : Collections.emptyList()); } + + public boolean isSynchronized() { + return this.sync; + } + + private boolean determineSyncFlag(Method method) { + List cacheOperationContexts = this.contexts.get(CacheableOperation.class); + if (cacheOperationContexts == null) { // No @Cacheable operation + return false; + } + boolean syncEnabled = false; + for (CacheOperationContext cacheOperationContext : cacheOperationContexts) { + if (((CacheableOperation) cacheOperationContext.getOperation()).isSync()) { + syncEnabled = true; + break; + } + } + if (syncEnabled) { + if (this.contexts.size() > 1) { + throw new IllegalStateException("@Cacheable(sync = true) cannot be combined with other cache operations on '" + method + "'"); + } + if (cacheOperationContexts.size() > 1) { + throw new IllegalStateException("Only one @Cacheable(sync = true) entry is allowed on '" + method + "'"); + } + CacheOperationContext cacheOperationContext = cacheOperationContexts.iterator().next(); + CacheableOperation operation = (CacheableOperation) cacheOperationContext.getOperation(); + if (cacheOperationContext.getCaches().size() > 1) { + throw new IllegalStateException("@Cacheable(sync = true) only allows a single cache on '" + operation + "'"); + } + if (StringUtils.hasText(operation.getUnless())) { + throw new IllegalStateException("@Cacheable(sync = true) does not support unless attribute on '" + operation + "'"); + } + return true; + } + return false; + } } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationInvoker.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationInvoker.java index 199791e6a9..a7d956d58f 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationInvoker.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationInvoker.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -42,7 +42,7 @@ public interface CacheOperationInvoker { * Wrap any exception thrown while invoking {@link #invoke()} */ @SuppressWarnings("serial") - public static class ThrowableWrapper extends RuntimeException { + class ThrowableWrapper extends RuntimeException { private final Throwable original; diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheableOperation.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheableOperation.java index f9375a9a54..ec2d0a3f34 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheableOperation.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheableOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 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. @@ -27,6 +27,8 @@ public class CacheableOperation extends CacheOperation { private String unless; + private boolean sync; + public String getUnless() { return unless; @@ -36,12 +38,23 @@ public class CacheableOperation extends CacheOperation { this.unless = unless; } + public boolean isSync() { + return sync; + } + + public void setSync(boolean sync) { + this.sync = sync; + } + @Override protected StringBuilder getOperationDescription() { StringBuilder sb = super.getOperationDescription(); sb.append(" | unless='"); sb.append(this.unless); sb.append("'"); + sb.append(" | sync='"); + sb.append(this.sync); + sb.append("'"); return sb; } } diff --git a/spring-context/src/main/java/org/springframework/cache/support/NoOpCacheManager.java b/spring-context/src/main/java/org/springframework/cache/support/NoOpCacheManager.java index 0b486804a6..faa56c3099 100644 --- a/spring-context/src/main/java/org/springframework/cache/support/NoOpCacheManager.java +++ b/spring-context/src/main/java/org/springframework/cache/support/NoOpCacheManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -20,6 +20,7 @@ import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -99,6 +100,11 @@ public class NoOpCacheManager implements CacheManager { return null; } + @Override + public T get(Object key, Callable valueLoader) { + return null; + } + @Override public String getName() { return this.name; diff --git a/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.3.xsd b/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.3.xsd index 18b03ab802..6b1103acb2 100644 --- a/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.3.xsd +++ b/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.3.xsd @@ -261,6 +261,13 @@ The SpEL expression used to veto the method caching.]]> + + + + + diff --git a/spring-context-support/src/test/java/org/springframework/cache/AbstractCacheTests.java b/spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java similarity index 51% rename from spring-context-support/src/test/java/org/springframework/cache/AbstractCacheTests.java rename to spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java index 97e33a010a..6e6c98ec7f 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/AbstractCacheTests.java +++ b/spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java @@ -16,10 +16,17 @@ package org.springframework.cache; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; import java.util.UUID; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import static org.hamcrest.core.Is.*; import static org.junit.Assert.*; /** @@ -27,6 +34,9 @@ import static org.junit.Assert.*; */ public abstract class AbstractCacheTests { + @Rule + public final ExpectedException thrown = ExpectedException.none(); + protected final static String CACHE_NAME = "testCache"; protected abstract T getCache(); @@ -59,7 +69,7 @@ public abstract class AbstractCacheTests { assertEquals(value, cache.get(key).get()); assertEquals(value, cache.get(key, String.class)); assertEquals(value, cache.get(key, Object.class)); - assertEquals(value, cache.get(key, null)); + assertEquals(value, cache.get(key, (Class) null)); cache.put(key, null); assertNotNull(cache.get(key)); @@ -106,6 +116,100 @@ public abstract class AbstractCacheTests { assertNull(cache.get("enescu")); } + @Test + public void testCacheGetCallable() { + doTestCacheGetCallable("test"); + } + + @Test + public void testCacheGetCallableWithNull() { + doTestCacheGetCallable(null); + } + + private void doTestCacheGetCallable(Object returnValue) { + T cache = getCache(); + + String key = createRandomKey(); + + assertNull(cache.get(key)); + Object value = cache.get(key, () -> returnValue ); + assertEquals(returnValue, value); + assertEquals(value, cache.get(key).get()); + } + + @Test + public void testCacheGetCallableNotInvokedWithHit() { + doTestCacheGetCallableNotInvokedWithHit("existing"); + } + + @Test + public void testCacheGetCallableNotInvokedWithHitNull() { + doTestCacheGetCallableNotInvokedWithHit(null); + } + + private void doTestCacheGetCallableNotInvokedWithHit(Object initialValue) { + T cache = getCache(); + + String key = createRandomKey(); + cache.put(key, initialValue); + + Object value = cache.get(key, () -> { + throw new IllegalStateException("Should not have been invoked"); + }); + assertEquals(initialValue, value); + } + + @Test + public void testCacheGetCallableFail() { + T cache = getCache(); + + String key = createRandomKey(); + assertNull(cache.get(key)); + + try { + cache.get(key, () -> { + throw new UnsupportedOperationException("Expected exception"); + }); + } + catch (Cache.ValueRetrievalException ex) { + assertNotNull(ex.getCause()); + assertEquals(UnsupportedOperationException.class, ex.getCause().getClass()); + } + } + + /** + * Test that a call to get with a Callable concurrently properly synchronize the + * invocations. + */ + @Test + public void testCacheGetSynchronized() throws InterruptedException { + T cache = getCache(); + final AtomicInteger counter = new AtomicInteger(); + final List results = new CopyOnWriteArrayList<>(); + final CountDownLatch latch = new CountDownLatch(10); + + String key = createRandomKey(); + Runnable run = () -> { + try { + Integer value = cache.get(key, () -> { + Thread.sleep(50); // make sure the thread will overlap + return counter.incrementAndGet(); + }); + results.add(value); + } + finally { + latch.countDown(); + } + }; + + for (int i = 0; i < 10; i++) { + new Thread(run).start(); + } + latch.await(); + + assertEquals(10, results.size()); + results.forEach(r -> assertThat(r, is(1))); // Only one method got invoked + } private String createRandomKey() { return UUID.randomUUID().toString(); diff --git a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentCacheTests.java b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentCacheTests.java deleted file mode 100644 index cba7db7d51..0000000000 --- a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentCacheTests.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright 2002-2014 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cache.concurrent; - -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; - -import org.junit.Before; -import org.junit.Test; - -import org.springframework.cache.Cache; - -import static org.junit.Assert.*; - -/** - * @author Costin Leau - * @author Juergen Hoeller - * @author Stephane Nicoll - */ -public class ConcurrentCacheTests { - - protected final static String CACHE_NAME = "testCache"; - - protected ConcurrentMap nativeCache; - - protected Cache cache; - - - @Before - public void setUp() throws Exception { - nativeCache = new ConcurrentHashMap(); - cache = new ConcurrentMapCache(CACHE_NAME, nativeCache, true); - cache.clear(); - } - - - @Test - public void testCacheName() throws Exception { - assertEquals(CACHE_NAME, cache.getName()); - } - - @Test - public void testNativeCache() throws Exception { - assertSame(nativeCache, cache.getNativeCache()); - } - - @Test - public void testCachePut() throws Exception { - Object key = "enescu"; - Object value = "george"; - - assertNull(cache.get(key)); - assertNull(cache.get(key, String.class)); - assertNull(cache.get(key, Object.class)); - - cache.put(key, value); - assertEquals(value, cache.get(key).get()); - assertEquals(value, cache.get(key, String.class)); - assertEquals(value, cache.get(key, Object.class)); - assertEquals(value, cache.get(key, null)); - - cache.put(key, null); - assertNotNull(cache.get(key)); - assertNull(cache.get(key).get()); - assertNull(cache.get(key, String.class)); - assertNull(cache.get(key, Object.class)); - } - - @Test - public void testCachePutIfAbsent() throws Exception { - Object key = new Object(); - Object value = "initialValue"; - - assertNull(cache.get(key)); - assertNull(cache.putIfAbsent(key, value)); - assertEquals(value, cache.get(key).get()); - assertEquals("initialValue", cache.putIfAbsent(key, "anotherValue").get()); - assertEquals(value, cache.get(key).get()); // not changed - } - - @Test - public void testCacheRemove() throws Exception { - Object key = "enescu"; - Object value = "george"; - - assertNull(cache.get(key)); - cache.put(key, value); - } - - @Test - public void testCacheClear() throws Exception { - assertNull(cache.get("enescu")); - cache.put("enescu", "george"); - assertNull(cache.get("vlaicu")); - cache.put("vlaicu", "aurel"); - cache.clear(); - assertNull(cache.get("vlaicu")); - assertNull(cache.get("enescu")); - } - -} diff --git a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java new file mode 100644 index 0000000000..c0a9e00d28 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2002-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cache.concurrent; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import org.junit.Before; +import org.junit.Ignore; + +import org.springframework.cache.AbstractCacheTests; + +/** + * @author Costin Leau + * @author Juergen Hoeller + * @author Stephane Nicoll + */ +public class ConcurrentMapCacheTests extends AbstractCacheTests { + + protected ConcurrentMap nativeCache; + + protected ConcurrentMapCache cache; + + + @Before + public void setUp() throws Exception { + nativeCache = new ConcurrentHashMap(); + cache = new ConcurrentMapCache(CACHE_NAME, nativeCache, true); + cache.clear(); + } + + @Override + protected ConcurrentMapCache getCache() { + return this.cache; + } + + @Override + protected ConcurrentMap getNativeCache() { + return this.nativeCache; + } + +} diff --git a/spring-context/src/test/java/org/springframework/cache/config/AbstractCacheAnnotationTests.java b/spring-context/src/test/java/org/springframework/cache/config/AbstractCacheAnnotationTests.java index 73b9d4d69a..fdf86d0690 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/AbstractCacheAnnotationTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/AbstractCacheAnnotationTests.java @@ -105,6 +105,32 @@ public abstract class AbstractCacheAnnotationTests { assertNull("Cached value should be null", r3); } + public void testCacheableSync(CacheableService service) throws Exception { + Object o1 = new Object(); + + Object r1 = service.cacheSync(o1); + Object r2 = service.cacheSync(o1); + Object r3 = service.cacheSync(o1); + + assertSame(r1, r2); + assertSame(r1, r3); + } + + public void testCacheableSyncNull(CacheableService service) throws Exception { + Object o1 = new Object(); + assertNull(cm.getCache("testCache").get(o1)); + + Object r1 = service.cacheSyncNull(o1); + Object r2 = service.cacheSyncNull(o1); + Object r3 = service.cacheSyncNull(o1); + + assertSame(r1, r2); + assertSame(r1, r3); + + assertEquals(r3, cm.getCache("testCache").get(o1).get()); + assertNull("Cached value should be null", r3); + } + public void testEvict(CacheableService service) throws Exception { Object o1 = new Object(); @@ -225,6 +251,18 @@ public abstract class AbstractCacheAnnotationTests { assertSame(r3, r4); } + public void testConditionalExpressionSync(CacheableService service) throws Exception { + Object r1 = service.conditionalSync(4); + Object r2 = service.conditionalSync(4); + + assertNotSame(r1, r2); + + Object r3 = service.conditionalSync(3); + Object r4 = service.conditionalSync(3); + + assertSame(r3, r4); + } + public void testUnlessExpression(CacheableService service) throws Exception { Cache cache = cm.getCache("testCache"); cache.clear(); @@ -311,6 +349,28 @@ public abstract class AbstractCacheAnnotationTests { } } + public void testCheckedThrowableSync(CacheableService service) throws Exception { + String arg = UUID.randomUUID().toString(); + try { + service.throwCheckedSync(arg); + fail("Excepted exception"); + } catch (Exception ex) { + ex.printStackTrace(); + assertEquals("Wrong exception type", IOException.class, ex.getClass()); + assertEquals(arg, ex.getMessage()); + } + } + + public void testUncheckedThrowableSync(CacheableService service) throws Exception { + try { + service.throwUncheckedSync(Long.valueOf(1)); + fail("Excepted exception"); + } catch (RuntimeException ex) { + assertEquals("Wrong exception type", UnsupportedOperationException.class, ex.getClass()); + assertEquals("1", ex.getMessage()); + } + } + public void testNullArg(CacheableService service) { Object r1 = service.cache(null); assertSame(r1, service.cache(null)); @@ -483,6 +543,16 @@ public abstract class AbstractCacheAnnotationTests { testCacheableNull(cs); } + @Test + public void testCacheableSync() throws Exception { + testCacheableSync(cs); + } + + @Test + public void testCacheableSyncNull() throws Exception { + testCacheableSyncNull(cs); + } + @Test public void testInvalidate() throws Exception { testEvict(cs); @@ -518,6 +588,11 @@ public abstract class AbstractCacheAnnotationTests { testConditionalExpression(cs); } + @Test + public void testConditionalExpressionSync() throws Exception { + testConditionalExpressionSync(cs); + } + @Test public void testUnlessExpression() throws Exception { testUnlessExpression(cs); @@ -677,6 +752,16 @@ public abstract class AbstractCacheAnnotationTests { testCheckedThrowable(ccs); } + @Test + public void testCheckedExceptionSync() throws Exception { + testCheckedThrowableSync(cs); + } + + @Test + public void testClassCheckedExceptionSync() throws Exception { + testCheckedThrowableSync(ccs); + } + @Test public void testUncheckedException() throws Exception { testUncheckedThrowable(cs); @@ -687,6 +772,16 @@ public abstract class AbstractCacheAnnotationTests { testUncheckedThrowable(ccs); } + @Test + public void testUncheckedExceptionSync() throws Exception { + testUncheckedThrowableSync(cs); + } + + @Test + public void testClassUncheckedExceptionSync() throws Exception { + testUncheckedThrowableSync(ccs); + } + @Test public void testUpdate() { testCacheUpdate(cs); diff --git a/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java b/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java index 450a01fb02..88886b6a8a 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java +++ b/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java @@ -46,11 +46,28 @@ public class AnnotatedClassCacheableService implements CacheableService return null; } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object cacheSync(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object cacheSyncNull(Object arg1) { + return null; + } + @Override public Object conditional(int field) { return null; } + @Override + public Object conditionalSync(int field) { + return null; + } + @Override @Cacheable(cacheNames = "testCache", unless = "#result > 10") public Object unless(int arg) { @@ -171,6 +188,18 @@ public class AnnotatedClassCacheableService implements CacheableService throw new UnsupportedOperationException(arg1.toString()); } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object throwCheckedSync(Object arg1) throws Exception { + throw new IOException(arg1.toString()); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Object throwUncheckedSync(Object arg1) { + throw new UnsupportedOperationException(arg1.toString()); + } + // multi annotations @Override diff --git a/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java b/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java index a129f6178b..f7030351c4 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java +++ b/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java @@ -29,6 +29,10 @@ public interface CacheableService { T cacheNull(Object arg1); + T cacheSync(Object arg1); + + T cacheSyncNull(Object arg1); + void invalidate(Object arg1); void evictEarly(Object arg1); @@ -43,6 +47,8 @@ public interface CacheableService { T conditional(int field); + T conditionalSync(int field); + T unless(int arg); T key(Object arg1, Object arg2); @@ -73,6 +79,10 @@ public interface CacheableService { T throwUnchecked(Object arg1); + T throwCheckedSync(Object arg1) throws Exception; + + T throwUncheckedSync(Object arg1); + // multi annotations T multiCache(Object arg1); diff --git a/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java b/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java index f5564df56d..93b7a239cf 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java +++ b/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java @@ -48,6 +48,18 @@ public class DefaultCacheableService implements CacheableService { return null; } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long cacheSync(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long cacheSyncNull(Object arg1) { + return null; + } + @Override @CacheEvict("testCache") public void invalidate(Object arg1) { @@ -82,11 +94,17 @@ public class DefaultCacheableService implements CacheableService { } @Override - @Cacheable(cacheNames = "testCache", condition = "#classField == 3") + @Cacheable(cacheNames = "testCache", condition = "#p0 == 3") public Long conditional(int classField) { return counter.getAndIncrement(); } + @Override + @Cacheable(cacheNames = "testCache", sync = true, condition = "#p0 == 3") + public Long conditionalSync(int classField) { + return counter.getAndIncrement(); + } + @Override @Cacheable(cacheNames = "testCache", unless = "#result > 10") public Long unless(int arg) { @@ -177,6 +195,18 @@ public class DefaultCacheableService implements CacheableService { throw new UnsupportedOperationException(arg1.toString()); } + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long throwCheckedSync(Object arg1) throws Exception { + throw new IOException(arg1.toString()); + } + + @Override + @Cacheable(cacheNames = "testCache", sync = true) + public Long throwUncheckedSync(Object arg1) { + throw new UnsupportedOperationException(arg1.toString()); + } + // multi annotations @Override diff --git a/spring-context/src/test/java/org/springframework/cache/interceptor/CacheSyncFailureTests.java b/spring-context/src/test/java/org/springframework/cache/interceptor/CacheSyncFailureTests.java new file mode 100644 index 0000000000..c21d5fb35a --- /dev/null +++ b/spring-context/src/test/java/org/springframework/cache/interceptor/CacheSyncFailureTests.java @@ -0,0 +1,158 @@ +/* + * Copyright 2002-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cache.interceptor; + +import java.util.concurrent.atomic.AtomicLong; + +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import org.springframework.cache.CacheManager; +import org.springframework.cache.CacheTestUtils; +import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.cache.annotation.Caching; +import org.springframework.cache.annotation.CachingConfigurerSupport; +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * Provides various failure scenario linked to the use of {@link Cacheable#sync()}. + * + * @author Stephane Nicoll + * @since 4.3 + */ +public class CacheSyncFailureTests { + + @Rule + public final ExpectedException thrown = ExpectedException.none(); + + private ConfigurableApplicationContext context; + + private SimpleService simpleService; + + @Before + public void setUp() { + this.context = new AnnotationConfigApplicationContext(Config.class); + this.simpleService = context.getBean(SimpleService.class); + } + + @After + public void closeContext() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void unlessSync() { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("@Cacheable(sync = true) does not support unless attribute"); + this.simpleService.unlessSync("key"); + } + + @Test + public void severalCachesSync() { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("@Cacheable(sync = true) only allows a single cache"); + this.simpleService.severalCachesSync("key"); + } + + @Test + public void severalCachesWithResolvedSync() { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("@Cacheable(sync = true) only allows a single cache"); + this.simpleService.severalCachesWithResolvedSync("key"); + } + + @Test + public void syncWithAnotherOperation() { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("@Cacheable(sync = true) cannot be combined with other cache operations"); + this.simpleService.syncWithAnotherOperation("key"); + } + + @Test + public void syncWithTwoGetOperations() { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("Only one @Cacheable(sync = true) entry is allowed"); + this.simpleService.syncWithTwoGetOperations("key"); + } + + + static class SimpleService { + + private final AtomicLong counter = new AtomicLong(); + + @Cacheable(cacheNames = "testCache", sync = true, unless = "#result > 10") + public Object unlessSync(Object arg1) { + return this.counter.getAndIncrement(); + } + + @Cacheable(cacheNames = {"testCache", "anotherTestCache"}, sync = true) + public Object severalCachesSync(Object arg1) { + return this.counter.getAndIncrement(); + } + + @Cacheable(cacheResolver = "testCacheResolver", sync = true) + public Object severalCachesWithResolvedSync(Object arg1) { + return this.counter.getAndIncrement(); + } + + @Cacheable(cacheNames = "testCache", sync = true) + @CacheEvict(cacheNames = "anotherTestCache", key = "#arg1") + public Object syncWithAnotherOperation(Object arg1) { + return this.counter.getAndIncrement(); + } + + @Caching(cacheable = { + @Cacheable(cacheNames = "testCache", sync = true), + @Cacheable(cacheNames = "anotherTestCache", sync = true) + }) + public Object syncWithTwoGetOperations(Object arg1) { + return this.counter.getAndIncrement(); + } + } + + @Configuration + @EnableCaching + static class Config extends CachingConfigurerSupport { + + @Override + @Bean + public CacheManager cacheManager() { + return CacheTestUtils.createSimpleCacheManager("testCache", "anotherTestCache"); + } + + @Bean + public CacheResolver testCacheResolver() { + return new NamedCacheResolver(cacheManager(), "testCache", "anotherTestCache"); + } + + @Bean + public SimpleService simpleService() { + return new SimpleService(); + } + } + +} 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 86db01a0f3..e108bb0f90 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 @@ -11,7 +11,10 @@ + + +