From 19d97c425316801a767cf99178ef30af730b1570 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 5 Nov 2015 10:42:07 +0100 Subject: [PATCH] Support for multi-threaded cache access Previously, if a `@Cacheable` method was accessed with the same key by multiple threads, the underlying method was invoked several times instead of blocking the threads while the value is computed. This scenario typically affects users that enable caching to avoid calling a costly method too often. When said method can be invoked by an arbitrary number of clients on startup, caching has close to no effect. This commit adds a new method on `Cache` that implements the read-through pattern: ``` T get(Object key, Callable valueLoader); ``` If an entry for a given key is not found, the specified `Callable` is invoked to "load" the value and cache it before returning it to the caller. Because the entire operation is managed by the underlying cache provider, it is much more easier to guarantee that the loader (e.g. the annotated method) will be called only once in case of concurrent access. A new `sync` attribute to the `@Cacheable` annotation has been addded. When this flag is enabled, the caching abstraction invokes the new `Cache` method define above. This new mode bring a set of limitations: * It can't be combined with other cache operations * Only one `@Cacheable` operation can be specified * Only one cache is allowed * `condition` and `unless` attribute are not supported The rationale behind those limitations is that the underlying Cache is taking care of the actual caching operation so we can't really apply any SpEL or multiple caches handling there. Issue: SPR-9254 --- .../cache/aspectj/AbstractCacheAspect.aj | 15 +- .../cache/aspectj/AnyThrow.java | 35 ++++ .../cache/aspectj/JCacheCacheAspect.aj | 12 -- .../AnnotatedClassCacheableService.java | 30 ++++ .../cache/config/CacheableService.java | 11 ++ .../cache/config/DefaultCacheableService.java | 35 +++- .../cache/caffeine/CaffeineCache.java | 26 +++ .../cache/ehcache/EhCacheCache.java | 46 ++++- .../cache/guava/GuavaCache.java | 19 +++ .../cache/jcache/JCacheCache.java | 41 +++++ .../TransactionAwareCacheDecorator.java | 9 +- .../cache/caffeine/CaffeineCacheTests.java | 3 +- .../cache/ehcache/EhCacheCacheTests.java | 3 +- .../cache/guava/GuavaCacheTests.java | 3 +- .../java/org/springframework/cache/Cache.java | 39 +++++ .../cache/annotation/Cacheable.java | 18 ++ .../SpringCacheAnnotationParser.java | 1 + .../cache/concurrent/ConcurrentMapCache.java | 25 +++ .../cache/config/CacheAdviceParser.java | 3 +- .../cache/interceptor/CacheAspectSupport.java | 72 +++++++- .../interceptor/CacheOperationInvoker.java | 4 +- .../cache/interceptor/CacheableOperation.java | 15 +- .../cache/support/NoOpCacheManager.java | 8 +- .../cache/config/spring-cache-4.3.xsd | 7 + .../cache/AbstractCacheTests.java | 106 +++++++++++- .../concurrent/ConcurrentCacheTests.java | 115 ------------- .../concurrent/ConcurrentMapCacheTests.java | 56 +++++++ .../config/AbstractCacheAnnotationTests.java | 95 +++++++++++ .../AnnotatedClassCacheableService.java | 29 ++++ .../cache/config/CacheableService.java | 10 ++ .../cache/config/DefaultCacheableService.java | 32 +++- .../interceptor/CacheSyncFailureTests.java | 158 ++++++++++++++++++ .../cache/config/cache-advice.xml | 3 + 33 files changed, 938 insertions(+), 146 deletions(-) create mode 100644 spring-aspects/src/main/java/org/springframework/cache/aspectj/AnyThrow.java rename {spring-context-support => spring-context}/src/test/java/org/springframework/cache/AbstractCacheTests.java (51%) delete mode 100644 spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentCacheTests.java create mode 100644 spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java create mode 100644 spring-context/src/test/java/org/springframework/cache/interceptor/CacheSyncFailureTests.java 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 20f2adb0a58..3b4b382031e 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 00000000000..d391c0a6bc8 --- /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 3aeeefbd0ed..4587fc00c09 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 224f7ffa955..521e22bc2da 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 17d299d9158..a862f2acbf7 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 c2c923f531c..e801647ed51 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 2bc076cc180..7eaa0423c1d 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 486634146d0..f1b7fd92749 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 55e3c9a3875..b9900c246b9 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 d149a60ba35..c80330846e5 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 4c5a7fdb174..f96c04c0718 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 57aad25908c..5ede180c4fb 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 8d1357552d2..f8541b07c7e 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 f9964e031f6..d50213c001e 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 3e8a7978761..c3d5dca64a3 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 055b49dee34..c258fab6476 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 7a1011405f7..f982b388a08 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 4942e14c0f7..b8aba442424 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 07bcea27aae..f9ec32f474a 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 ecec4ffda92..df461d895fd 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 199791e6a92..a7d956d58f2 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 f9375a9a54e..ec2d0a3f347 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 0b486804a69..faa56c30991 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 18b03ab8025..6b1103acb25 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 97e33a010a1..6e6c98ec7f6 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 cba7db7d517..00000000000 --- 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 00000000000..c0a9e00d280 --- /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 73b9d4d69a2..fdf86d0690f 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 450a01fb024..88886b6a8a4 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 a129f6178bb..f7030351c49 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 f5564df56d6..93b7a239cf1 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 00000000000..c21d5fb35a8 --- /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 86db01a0f3d..e108bb0f908 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 @@ + + +