From 0e36402bd29d602f5f253ae2adddfca60f3f37b8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 1 Nov 2014 08:26:48 +0100 Subject: [PATCH] Revised retrieval of cache strategy beans Issue: SPR-12336 --- .../interceptor/AbstractCacheInterceptor.java | 14 +- .../AnnotationJCacheOperationSource.java | 93 ++++++------ .../DefaultJCacheOperationSource.java | 46 +++--- .../interceptor/JCacheAspectSupport.java | 19 ++- .../interceptor/JCacheInterceptorTests.java | 34 +++-- .../cache/interceptor/CacheAspectSupport.java | 133 ++++++++++-------- .../cache/config/EnableCachingTests.java | 47 +++++-- 7 files changed, 210 insertions(+), 176 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractCacheInterceptor.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractCacheInterceptor.java index f4dee40711..cac85264c3 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractCacheInterceptor.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractCacheInterceptor.java @@ -28,6 +28,7 @@ import org.springframework.cache.interceptor.AbstractCacheInvoker; import org.springframework.cache.interceptor.CacheErrorHandler; import org.springframework.cache.interceptor.CacheOperationInvocationContext; import org.springframework.cache.interceptor.CacheOperationInvoker; +import org.springframework.util.CollectionUtils; /** * A base interceptor for JSR-107 cache annotations. @@ -41,12 +42,15 @@ abstract class AbstractCacheInterceptor, A protected final Log logger = LogFactory.getLog(getClass()); + protected AbstractCacheInterceptor(CacheErrorHandler errorHandler) { super(errorHandler); } - protected abstract Object invoke(CacheOperationInvocationContext context, - CacheOperationInvoker invoker) throws Throwable; + + protected abstract Object invoke(CacheOperationInvocationContext context, CacheOperationInvoker invoker) + throws Throwable; + /** * Resolve the cache to use. @@ -68,15 +72,15 @@ abstract class AbstractCacheInterceptor, A * @return the singe element or {@code null} if the collection is empty */ static Cache extractFrom(Collection caches) { - if (caches == null || caches.size() == 0) { + if (CollectionUtils.isEmpty(caches)) { return null; } else if (caches.size() == 1) { return caches.iterator().next(); } else { - throw new IllegalStateException("Unsupported cache resolution result " - + caches + " JSR-107 only supports a single cache."); + throw new IllegalStateException("Unsupported cache resolution result " + caches + + ": JSR-107 only supports a single cache."); } } diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AnnotationJCacheOperationSource.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AnnotationJCacheOperationSource.java index e2146c6f66..622982bcdd 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AnnotationJCacheOperationSource.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AnnotationJCacheOperationSource.java @@ -43,35 +43,13 @@ import org.springframework.util.StringUtils; */ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJCacheOperationSource { - /** - * Locate or create an instance of the specified {@code type}. - * @param type the type of the bean to manage - * @return the required bean - */ - protected abstract T getBean(Class type); - - /** - * Return the default {@link CacheResolver} if none is set. - */ - protected abstract CacheResolver getDefaultCacheResolver(); - - /** - * Return the default exception {@link CacheResolver} if none is set. - */ - protected abstract CacheResolver getDefaultExceptionCacheResolver(); - - /** - * Return the default {@link KeyGenerator} if none is set. - */ - protected abstract KeyGenerator getDefaultKeyGenerator(); - - @Override protected JCacheOperation findCacheOperation(Method method, Class targetType) { CacheResult cacheResult = method.getAnnotation(CacheResult.class); CachePut cachePut = method.getAnnotation(CachePut.class); CacheRemove cacheRemove = method.getAnnotation(CacheRemove.class); CacheRemoveAll cacheRemoveAll = method.getAnnotation(CacheRemoveAll.class); + int found = countNonNull(cacheResult, cachePut, cacheRemove, cacheRemoveAll); if (found == 0) { return null; @@ -79,8 +57,8 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC if (found > 1) { throw new IllegalStateException("More than one cache annotation found on '" + method + "'"); } - CacheDefaults defaults = getCacheDefaults(method, targetType); + CacheDefaults defaults = getCacheDefaults(method, targetType); if (cacheResult != null) { return createCacheResultOperation(method, defaults, cacheResult); } @@ -103,9 +81,7 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC return targetType.getAnnotation(CacheDefaults.class); } - - protected CacheResultOperation createCacheResultOperation(Method method, CacheDefaults defaults, - CacheResult ann) { + protected CacheResultOperation createCacheResultOperation(Method method, CacheDefaults defaults, CacheResult ann) { String cacheName = determineCacheName(method, defaults, ann.cacheName()); CacheResolverFactory cacheResolverFactory = determineCacheResolverFactory(defaults, ann.cacheResolverFactory()); @@ -123,49 +99,39 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC return new CacheResultOperation(methodDetails, cacheResolver, keyGenerator, exceptionCacheResolver); } - protected CachePutOperation createCachePutOperation(Method method, CacheDefaults defaults, - CachePut ann) { + protected CachePutOperation createCachePutOperation(Method method, CacheDefaults defaults, CachePut ann) { String cacheName = determineCacheName(method, defaults, ann.cacheName()); CacheResolverFactory cacheResolverFactory = determineCacheResolverFactory(defaults, ann.cacheResolverFactory()); KeyGenerator keyGenerator = determineKeyGenerator(defaults, ann.cacheKeyGenerator()); CacheMethodDetails methodDetails = createMethodDetails(method, ann, cacheName); - CacheResolver cacheResolver = getCacheResolver(cacheResolverFactory, methodDetails); - return new CachePutOperation(methodDetails, cacheResolver, keyGenerator); } - protected CacheRemoveOperation createCacheRemoveOperation(Method method, CacheDefaults defaults, - CacheRemove ann) { + protected CacheRemoveOperation createCacheRemoveOperation(Method method, CacheDefaults defaults, CacheRemove ann) { String cacheName = determineCacheName(method, defaults, ann.cacheName()); CacheResolverFactory cacheResolverFactory = determineCacheResolverFactory(defaults, ann.cacheResolverFactory()); KeyGenerator keyGenerator = determineKeyGenerator(defaults, ann.cacheKeyGenerator()); CacheMethodDetails methodDetails = createMethodDetails(method, ann, cacheName); - CacheResolver cacheResolver = getCacheResolver(cacheResolverFactory, methodDetails); - return new CacheRemoveOperation(methodDetails, cacheResolver, keyGenerator); } - protected CacheRemoveAllOperation createCacheRemoveAllOperation(Method method, CacheDefaults defaults, - CacheRemoveAll ann) { + protected CacheRemoveAllOperation createCacheRemoveAllOperation(Method method, CacheDefaults defaults, CacheRemoveAll ann) { String cacheName = determineCacheName(method, defaults, ann.cacheName()); CacheResolverFactory cacheResolverFactory = determineCacheResolverFactory(defaults, ann.cacheResolverFactory()); CacheMethodDetails methodDetails = createMethodDetails(method, ann, cacheName); - CacheResolver cacheResolver = getCacheResolver(cacheResolverFactory, methodDetails); - return new CacheRemoveAllOperation(methodDetails, cacheResolver); } - private CacheMethodDetails createMethodDetails( - Method method, A annotation, String cacheName) { + private CacheMethodDetails createMethodDetails(Method method, A annotation, String cacheName) { return new DefaultCacheMethodDetails(method, annotation, cacheName); } @@ -181,6 +147,7 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC protected CacheResolver getExceptionCacheResolver(CacheResolverFactory factory, CacheMethodDetails details) { + if (factory != null) { javax.cache.annotation.CacheResolver cacheResolver = factory.getExceptionCacheResolver(details); return new CacheResolverAdapter(cacheResolver); @@ -192,6 +159,7 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC protected CacheResolverFactory determineCacheResolverFactory(CacheDefaults defaults, Class candidate) { + if (!CacheResolverFactory.class.equals(candidate)) { return getBean(candidate); } @@ -203,8 +171,7 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC } } - protected KeyGenerator determineKeyGenerator(CacheDefaults defaults, - Class candidate) { + protected KeyGenerator determineKeyGenerator(CacheDefaults defaults, Class candidate) { if (!CacheKeyGenerator.class.equals(candidate)) { return new KeyGeneratorAdapter(this, getBean(candidate)); } @@ -233,28 +200,48 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC */ protected String generateDefaultCacheName(Method method) { Class[] parameterTypes = method.getParameterTypes(); - List parameters = new ArrayList(); + List parameters = new ArrayList(parameterTypes.length); for (Class parameterType : parameterTypes) { parameters.add(parameterType.getName()); } - StringBuilder sb = new StringBuilder(); - sb.append(method.getDeclaringClass().getName()) - .append(".") - .append(method.getName()) - .append("(") - .append(StringUtils.collectionToCommaDelimitedString(parameters)) - .append(")"); + StringBuilder sb = new StringBuilder(method.getDeclaringClass().getName()); + sb.append(".").append(method.getName()); + sb.append("(").append(StringUtils.collectionToCommaDelimitedString(parameters)).append(")"); return sb.toString(); } private int countNonNull(Object... instances) { int result = 0; - for (Object o : instances) { - if (o != null) { + for (Object instance : instances) { + if (instance != null) { result += 1; } } return result; } + + + /** + * Locate or create an instance of the specified cache strategy {@code type}. + * @param type the type of the bean to manage + * @return the required bean + */ + protected abstract T getBean(Class type); + + /** + * Return the default {@link CacheResolver} if none is set. + */ + protected abstract CacheResolver getDefaultCacheResolver(); + + /** + * Return the default exception {@link CacheResolver} if none is set. + */ + protected abstract CacheResolver getDefaultExceptionCacheResolver(); + + /** + * Return the default {@link KeyGenerator} if none is set. + */ + protected abstract KeyGenerator getDefaultKeyGenerator(); + } diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java index aaeec8ec8b..d81cab52e5 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java @@ -16,11 +16,10 @@ package org.springframework.cache.jcache.interceptor; -import java.util.Map; - import org.springframework.beans.BeanUtils; -import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.cache.CacheManager; import org.springframework.cache.interceptor.CacheResolver; @@ -40,7 +39,7 @@ import org.springframework.util.Assert; * @since 4.1 */ public class DefaultJCacheOperationSource extends AnnotationJCacheOperationSource - implements InitializingBean, SmartInitializingSingleton, ApplicationContextAware { + implements ApplicationContextAware, InitializingBean, SmartInitializingSingleton { private CacheManager cacheManager; @@ -54,6 +53,7 @@ public class DefaultJCacheOperationSource extends AnnotationJCacheOperationSourc private ApplicationContext applicationContext; + /** * Set the default {@link CacheManager} to use to lookup cache by name. Only mandatory * if the {@linkplain CacheResolver cache resolvers} have not been set. @@ -104,24 +104,33 @@ public class DefaultJCacheOperationSource extends AnnotationJCacheOperationSourc this.applicationContext = applicationContext; } + @Override public void afterPropertiesSet() { this.adaptedKeyGenerator = new KeyGeneratorAdapter(this, this.keyGenerator); } @Override - public void afterSingletonsInstantiated() { // Make sure those are initialized on startup - Assert.notNull(getDefaultCacheResolver(), "Cache resolver should have been initialized."); - Assert.notNull(getDefaultExceptionCacheResolver(), "Exception cache resolver should have been initialized."); + public void afterSingletonsInstantiated() { + // Make sure those are initialized on startup... + Assert.notNull(getDefaultCacheResolver(), "Cache resolver should have been initialized"); + Assert.notNull(getDefaultExceptionCacheResolver(), "Exception cache resolver should have been initialized"); } + @Override protected T getBean(Class type) { - Map map = BeanFactoryUtils.beansOfTypeIncludingAncestors(this.applicationContext, type); - if (map.size() == 1) { - return map.values().iterator().next(); + try { + return this.applicationContext.getBean(type); } - else { + catch (NoUniqueBeanDefinitionException ex) { + throw new IllegalStateException("No unique [" + type.getName() + "] bean found in application context - " + + "mark one as primary, or declare a more specific implementation type for your cache", ex); + } + catch (NoSuchBeanDefinitionException ex) { + if (logger.isDebugEnabled()) { + logger.debug("No bean of type [" + type.getName() + "] found in application context", ex); + } return BeanUtils.instantiateClass(type); } } @@ -149,11 +158,16 @@ public class DefaultJCacheOperationSource extends AnnotationJCacheOperationSourc private CacheManager getCacheManager() { if (this.cacheManager == null) { - this.cacheManager = this.applicationContext.getBean(CacheManager.class); - if (this.cacheManager == null) { - throw new IllegalStateException("No bean of type CacheManager could be found. " + - "Register a CacheManager bean or remove the @EnableCaching annotation " + - "from your configuration."); + try { + this.cacheManager = this.applicationContext.getBean(CacheManager.class); + } + catch (NoUniqueBeanDefinitionException ex) { + throw new IllegalStateException("No unique bean of type CacheManager found. "+ + "Mark one as primary or declare a specific CacheManager to use."); + } + catch (NoSuchBeanDefinitionException ex) { + throw new IllegalStateException("No bean of type CacheManager found. Register a CacheManager "+ + "bean or remove the @EnableCaching annotation from your configuration."); } } return this.cacheManager; diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheAspectSupport.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheAspectSupport.java index 2c62c4768e..b20f7bbe25 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheAspectSupport.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheAspectSupport.java @@ -78,9 +78,9 @@ public class JCacheAspectSupport extends AbstractCacheInvoker implements Initial } public void afterPropertiesSet() { - Assert.state(this.cacheOperationSource != null, "The 'cacheOperationSource' property is required: " + + Assert.state(getCacheOperationSource() != null, "The 'cacheOperationSource' property is required: " + "If there are no cacheable methods, then don't use a cache aspect."); - Assert.state(this.getErrorHandler() != null, "The 'errorHandler' is required."); + Assert.state(getErrorHandler() != null, "The 'errorHandler' is required"); this.cacheResultInterceptor = new CacheResultInterceptor(getErrorHandler()); this.cachePutInterceptor = new CachePutInterceptor(getErrorHandler()); @@ -92,8 +92,7 @@ public class JCacheAspectSupport extends AbstractCacheInvoker implements Initial protected Object execute(CacheOperationInvoker invoker, Object target, Method method, Object[] args) { - // check whether aspect is enabled - // to cope with cases where the AJ is pulled in automatically + // Check whether aspect is enabled to cope with cases where the AJ is pulled in automatically if (this.initialized) { Class targetClass = getTargetClass(target); JCacheOperation operation = getCacheOperationSource().getCacheOperation(method, targetClass); @@ -108,9 +107,9 @@ public class JCacheAspectSupport extends AbstractCacheInvoker implements Initial } @SuppressWarnings("unchecked") - private CacheOperationInvocationContext createCacheOperationInvocationContext(Object target, - Object[] args, - JCacheOperation operation) { + private CacheOperationInvocationContext createCacheOperationInvocationContext( + Object target, Object[] args, JCacheOperation operation) { + return new DefaultCacheInvocationContext( (JCacheOperation) operation, target, args); } @@ -124,12 +123,10 @@ public class JCacheAspectSupport extends AbstractCacheInvoker implements Initial } @SuppressWarnings("unchecked") - private Object execute(CacheOperationInvocationContext context, - CacheOperationInvoker invoker) { - + private Object execute(CacheOperationInvocationContext context, CacheOperationInvoker invoker) { CacheOperationInvoker adapter = new CacheOperationInvokerAdapter(invoker); - BasicOperation operation = context.getOperation(); + if (operation instanceof CacheResultOperation) { return cacheResultInterceptor.invoke( (CacheOperationInvocationContext) context, adapter); diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheInterceptorTests.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheInterceptorTests.java index aadfa9cba2..d99aeff1af 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheInterceptorTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheInterceptorTests.java @@ -20,7 +20,6 @@ import java.lang.reflect.Method; import org.junit.Test; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cache.CacheManager; import org.springframework.cache.interceptor.CacheOperationInvoker; import org.springframework.cache.interceptor.CacheResolver; @@ -51,11 +50,11 @@ public class JCacheInterceptorTests extends AbstractJCacheTests { try { interceptor.execute(dummyInvoker, service, m, new Object[] {"myId"}); } - catch (IllegalStateException e) { - assertTrue(e.getMessage().contains("JSR-107 only supports a single cache.")); + catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("JSR-107 only supports a single cache")); } - catch (Throwable t) { - fail("Unexpected: " + t); + catch (Throwable ex) { + fail("Unexpected: " + ex); } } @@ -71,17 +70,17 @@ public class JCacheInterceptorTests extends AbstractJCacheTests { try { interceptor.execute(dummyInvoker, service, m, new Object[] {"myId"}); } - catch (IllegalStateException e) { - assertTrue(e.getMessage().contains("Cache could not have been resolved for")); + catch (IllegalStateException ex) { + assertTrue(ex.getMessage().contains("Cache could not have been resolved for")); } - catch (Throwable t) { - fail("Unexpected: " + t); + catch (Throwable ex) { + fail("Unexpected: " + ex); } } @Test - public void cacheManagerMandatoryIfCacheResolverNotSetSet() { - thrown.expect(NoSuchBeanDefinitionException.class); + public void cacheManagerMandatoryIfCacheResolverNotSet() { + thrown.expect(IllegalStateException.class); createOperationSource(null, null, null, defaultKeyGenerator); } @@ -93,23 +92,21 @@ public class JCacheInterceptorTests extends AbstractJCacheTests { @Test public void cacheResultReturnsProperType() throws Throwable { JCacheInterceptor interceptor = createInterceptor(createOperationSource( - cacheManager, defaultCacheResolver, - defaultExceptionCacheResolver, defaultKeyGenerator)); + cacheManager, defaultCacheResolver, defaultExceptionCacheResolver, defaultKeyGenerator)); AnnotatedJCacheableService service = new AnnotatedJCacheableService(cacheManager.getCache("default")); - Method m = ReflectionUtils.findMethod(AnnotatedJCacheableService.class, "cache", String.class); + Method method = ReflectionUtils.findMethod(AnnotatedJCacheableService.class, "cache", String.class); CacheOperationInvoker invoker = new DummyInvoker(0L); - Object execute = interceptor.execute(invoker, service, m, new Object[] {"myId"}); + Object execute = interceptor.execute(invoker, service, method, new Object[] {"myId"}); assertNotNull("result cannot be null.", execute); assertEquals("Wrong result type", Long.class, execute.getClass()); assertEquals("Wrong result", 0L, execute); } protected JCacheOperationSource createOperationSource(CacheManager cacheManager, - CacheResolver cacheResolver, - CacheResolver exceptionCacheResolver, - KeyGenerator keyGenerator) { + CacheResolver cacheResolver, CacheResolver exceptionCacheResolver, KeyGenerator keyGenerator) { + DefaultJCacheOperationSource source = new DefaultJCacheOperationSource(); source.setApplicationContext(new StaticApplicationContext()); source.setCacheManager(cacheManager); @@ -129,6 +126,7 @@ public class JCacheInterceptorTests extends AbstractJCacheTests { return interceptor; } + private static class DummyInvoker implements CacheOperationInvoker { private final Object result; 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 cb4f5779e7..676a194b6b 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 @@ -30,6 +30,8 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.framework.AopProxyUtils; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils; import org.springframework.cache.Cache; @@ -98,9 +100,8 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker /** * Set one or more cache operation sources which are used to find the cache - * attributes. If more than one source is provided, they will be aggregated using a - * {@link CompositeCacheOperationSource}. - * @param cacheOperationSources must not be {@code null} + * attributes. If more than one source is provided, they will be aggregated + * using a {@link CompositeCacheOperationSource}. */ public void setCacheOperationSources(CacheOperationSource... cacheOperationSources) { Assert.notEmpty(cacheOperationSources, "At least 1 CacheOperationSource needs to be specified"); @@ -132,9 +133,8 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } /** - * Set the {@link CacheManager} to use to create a default {@link CacheResolver}. Replace - * the current {@link CacheResolver}, if any. - * + * Set the {@link CacheManager} to use to create a default {@link CacheResolver}. + * Replace the current {@link CacheResolver}, if any. * @see #setCacheResolver(CacheResolver) * @see SimpleCacheResolver */ @@ -151,7 +151,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker * @see SimpleCacheResolver */ public void setCacheResolver(CacheResolver cacheResolver) { - Assert.notNull(cacheResolver); + Assert.notNull(cacheResolver, "CacheResolver must not be null"); this.cacheResolver = cacheResolver; } @@ -159,7 +159,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker * Return the default {@link CacheResolver} that this cache aspect delegates to. */ public CacheResolver getCacheResolver() { - return cacheResolver; + return this.cacheResolver; } @Override @@ -167,28 +167,33 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker this.applicationContext = applicationContext; } + public void afterPropertiesSet() { - Assert.state(this.cacheOperationSource != null, "The 'cacheOperationSources' property is required: " + + Assert.state(getCacheOperationSource() != null, "The 'cacheOperationSources' property is required: " + "If there are no cacheable methods, then don't use a cache aspect."); - Assert.state(this.getErrorHandler() != null, "The 'errorHandler' is required."); + Assert.state(getErrorHandler() != null, "The 'errorHandler' property is required"); } @Override public void afterSingletonsInstantiated() { - if (getCacheResolver() == null) { // lazy initialize cache resolver - CacheManager cacheManager = this.applicationContext.getBean(CacheManager.class); - if (cacheManager == null) { - throw new IllegalStateException("No bean of type CacheManager could be found. " + - "Register a CacheManager bean or remove the @EnableCaching annotation " + - "from your configuration."); + if (getCacheResolver() == null) { + // Lazily initialize cache resolver via default cache manager... + try { + setCacheManager(this.applicationContext.getBean(CacheManager.class)); + } + catch (NoUniqueBeanDefinitionException ex) { + throw new IllegalStateException("No CacheResolver specified, and no unique bean of type " + + "CacheManager found. Mark one as primary or declare a specific CacheManager to use."); + } + catch (NoSuchBeanDefinitionException ex) { + throw new IllegalStateException("No CacheResolver specified, and no bean of type CacheManager found. " + + "Register a CacheManager bean or remove the @EnableCaching annotation from your configuration."); } - setCacheManager(cacheManager); } - Assert.state(this.cacheResolver != null, "'cacheResolver' is required. Either set the cache resolver " + - "to use or set the cache manager to create a default cache resolver based on it."); this.initialized = true; } + /** * Convenience method to return a String representation of this Method * for use in logging. Can be overridden in subclasses to provide a @@ -203,19 +208,20 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker return ClassUtils.getQualifiedMethodName(specificMethod); } - protected Collection getCaches(CacheOperationInvocationContext context, - CacheResolver cacheResolver) { + protected Collection getCaches( + CacheOperationInvocationContext context, CacheResolver cacheResolver) { + Collection caches = cacheResolver.resolveCaches(context); if (caches.isEmpty()) { - throw new IllegalStateException("No cache could be resolved for '" + context.getOperation() - + "' using resolver '" + cacheResolver - + "'. At least one cache should be provided per cache operation."); + throw new IllegalStateException("No cache could be resolved for '" + + context.getOperation() + "' using resolver '" + cacheResolver + + "'. At least one cache should be provided per cache operation."); } return caches; } - protected CacheOperationContext getOperationContext(CacheOperation operation, Method method, Object[] args, - Object target, Class targetClass) { + protected CacheOperationContext getOperationContext( + CacheOperation operation, Method method, Object[] args, Object target, Class targetClass) { CacheOperationMetadata metadata = getCacheOperationMetadata(operation, method, targetClass); return new CacheOperationContext(metadata, args, target); @@ -230,10 +236,11 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker * @param targetClass the target type * @return the resolved metadata for the operation */ - protected CacheOperationMetadata getCacheOperationMetadata(CacheOperation operation, - Method method, Class targetClass) { - final CacheOperationCacheKey cacheKey = new CacheOperationCacheKey(operation, method, targetClass); - CacheOperationMetadata metadata = metadataCache.get(cacheKey); + protected CacheOperationMetadata getCacheOperationMetadata( + CacheOperation operation, Method method, Class targetClass) { + + CacheOperationCacheKey cacheKey = new CacheOperationCacheKey(operation, method, targetClass); + CacheOperationMetadata metadata = this.metadataCache.get(cacheKey); if (metadata == null) { KeyGenerator operationKeyGenerator; if (StringUtils.hasText(operation.getKeyGenerator())) { @@ -255,7 +262,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } metadata = new CacheOperationMetadata(operation, method, targetClass, operationKeyGenerator, operationCacheResolver); - metadataCache.put(cacheKey, metadata); + this.metadataCache.put(cacheKey, metadata); } return metadata; } @@ -263,7 +270,6 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker /** * Return a bean with the specified name and type. Used to resolve services that * are referenced by name in a {@link CacheOperation}. - * * @param beanName the name of the bean, as defined by the operation * @param expectedType type type for the bean * @return the bean matching that name @@ -273,15 +279,14 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker * @see CacheOperation#cacheResolver */ protected T getBean(String beanName, Class expectedType) { - return BeanFactoryAnnotationUtils.qualifiedBeanOfType( - applicationContext, expectedType, beanName); + return BeanFactoryAnnotationUtils.qualifiedBeanOfType(this.applicationContext, expectedType, beanName); } /** * Clear the cached metadata. */ protected void clearMetadataCache() { - metadataCache.clear(); + this.metadataCache.clear(); } protected Object execute(CacheOperationInvoker invoker, Object target, Method method, Object[] args) { @@ -486,8 +491,8 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker private final MultiValueMap, CacheOperationContext> contexts = new LinkedMultiValueMap, CacheOperationContext>(); - public CacheOperationContexts(Collection operations, - Method method, Object[] args, Object target, Class targetClass) { + 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)); @@ -508,14 +513,18 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker protected static class CacheOperationMetadata { private final CacheOperation operation; + private final Method method; + private final Class targetClass; + private final KeyGenerator keyGenerator; + private final CacheResolver cacheResolver; - public CacheOperationMetadata(CacheOperation operation, Method method, - Class targetClass, KeyGenerator keyGenerator, - CacheResolver cacheResolver) { + public CacheOperationMetadata(CacheOperation operation, Method method, Class targetClass, + KeyGenerator keyGenerator, CacheResolver cacheResolver) { + this.operation = operation; this.method = method; this.targetClass = targetClass; @@ -524,6 +533,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } } + protected class CacheOperationContext implements CacheOperationInvocationContext { private final CacheOperationMetadata metadata; @@ -536,8 +546,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker private final MethodCacheKey methodCacheKey; - public CacheOperationContext(CacheOperationMetadata metadata, - Object[] args, Object target) { + public CacheOperationContext(CacheOperationMetadata metadata, Object[] args, Object target) { this.metadata = metadata; this.args = extractArgs(metadata.method, args); this.target = target; @@ -547,22 +556,22 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker @Override public CacheOperation getOperation() { - return metadata.operation; + return this.metadata.operation; } @Override public Object getTarget() { - return target; + return this.target; } @Override public Method getMethod() { - return metadata.method; + return this.metadata.method; } @Override public Object[] getArgs() { - return args; + return this.args; } private Object[] extractArgs(Method method, Object[] args) { @@ -579,7 +588,8 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker protected boolean isConditionPassing(Object result) { if (StringUtils.hasText(this.metadata.operation.getCondition())) { EvaluationContext evaluationContext = createEvaluationContext(result); - return evaluator.condition(this.metadata.operation.getCondition(), this.methodCacheKey, evaluationContext); + return evaluator.condition(this.metadata.operation.getCondition(), + this.methodCacheKey, evaluationContext); } return true; } @@ -600,15 +610,15 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } /** - * Computes the key for the given caching operation. - * @return generated key (null if none can be generated) + * Compute the key for the given caching operation. + * @return the generated key, or {@code null} if none can be generated */ protected Object generateKey(Object result) { if (StringUtils.hasText(this.metadata.operation.getKey())) { EvaluationContext evaluationContext = createEvaluationContext(result); return evaluator.key(this.metadata.operation.getKey(), this.methodCacheKey, evaluationContext); } - return metadata.keyGenerator.generate(this.target, this.metadata.method, this.args); + return this.metadata.keyGenerator.generate(this.target, this.metadata.method, this.args); } private EvaluationContext createEvaluationContext(Object result) { @@ -642,9 +652,11 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } } + private static class CacheOperationCacheKey { private final CacheOperation cacheOperation; + private final MethodCacheKey methodCacheKey; private CacheOperationCacheKey(CacheOperation cacheOperation, Method method, Class targetClass) { @@ -653,20 +665,21 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - CacheOperationCacheKey that = (CacheOperationCacheKey) o; - return cacheOperation.equals(that.cacheOperation) - && methodCacheKey.equals(that.methodCacheKey); + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof CacheOperationCacheKey)) { + return false; + } + CacheOperationCacheKey otherKey = (CacheOperationCacheKey) other; + return (this.cacheOperation.equals(otherKey.cacheOperation) && + this.methodCacheKey.equals(otherKey.methodCacheKey)); } @Override public int hashCode() { - int result = cacheOperation.hashCode(); - result = 31 * result + methodCacheKey.hashCode(); - return result; + return (this.cacheOperation.hashCode() * 31 + this.methodCacheKey.hashCode()); } } } diff --git a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java index e3c1a6e01e..d14ac0ceda 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java @@ -19,8 +19,6 @@ package org.springframework.cache.config; import org.junit.Test; import org.springframework.beans.factory.BeanCreationException; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; -import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.cache.CacheManager; import org.springframework.cache.CacheTestUtils; import org.springframework.cache.annotation.CachingConfigurerSupport; @@ -75,13 +73,14 @@ public class EnableCachingTests extends AbstractAnnotationTests { ctx.refresh(); } - @Test(expected=NoUniqueBeanDefinitionException.class) + @Test(expected = IllegalStateException.class) public void multipleCacheManagerBeans() throws Throwable { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(MultiCacheManagerConfig.class); try { ctx.refresh(); - } catch (BeanCreationException ex) { + } + catch (BeanCreationException ex) { Throwable root = ex.getRootCause(); assertTrue(root.getMessage().contains("beans of type CacheManager")); throw root; @@ -95,26 +94,28 @@ public class EnableCachingTests extends AbstractAnnotationTests { ctx.refresh(); // does not throw } - @Test(expected=IllegalStateException.class) + @Test(expected = IllegalStateException.class) public void multipleCachingConfigurers() throws Throwable { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(MultiCacheManagerConfigurer.class, EnableCachingConfig.class); try { ctx.refresh(); - } catch (BeanCreationException ex) { + } + catch (BeanCreationException ex) { Throwable root = ex.getRootCause(); assertTrue(root.getMessage().contains("implementations of CachingConfigurer")); throw root; } } - @Test(expected=NoSuchBeanDefinitionException.class) + @Test(expected = IllegalStateException.class) public void noCacheManagerBeans() throws Throwable { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(EmptyConfig.class); try { ctx.refresh(); - } catch (BeanCreationException ex) { + } + catch (BeanCreationException ex) { Throwable root = ex.getRootCause(); assertTrue(root.getMessage().contains("No bean of type CacheManager")); throw root; @@ -149,6 +150,7 @@ public class EnableCachingTests extends AbstractAnnotationTests { @Configuration @EnableCaching static class EnableCachingConfig extends CachingConfigurerSupport { + @Override @Bean public CacheManager cacheManager() { @@ -198,39 +200,56 @@ public class EnableCachingTests extends AbstractAnnotationTests { @Configuration @EnableCaching static class SingleCacheManagerConfig { + @Bean - public CacheManager cm1() { return new NoOpCacheManager(); } + public CacheManager cm1() { + return new NoOpCacheManager(); + } } @Configuration @EnableCaching static class MultiCacheManagerConfig { + @Bean - public CacheManager cm1() { return new NoOpCacheManager(); } + public CacheManager cm1() { + return new NoOpCacheManager(); + } + @Bean - public CacheManager cm2() { return new NoOpCacheManager(); } + public CacheManager cm2() { + return new NoOpCacheManager(); + } } @Configuration @EnableCaching static class MultiCacheManagerConfigurer extends CachingConfigurerSupport { + @Bean - public CacheManager cm1() { return new NoOpCacheManager(); } + public CacheManager cm1() { + return new NoOpCacheManager(); + } + @Bean - public CacheManager cm2() { return new NoOpCacheManager(); } + public CacheManager cm2() { + return new NoOpCacheManager(); + } @Override public CacheManager cacheManager() { return cm1(); } + @Override public KeyGenerator keyGenerator() { return null; } } + @Configuration @EnableCaching static class EmptyConfigSupportConfig extends CachingConfigurerSupport { @@ -241,6 +260,7 @@ public class EnableCachingTests extends AbstractAnnotationTests { } } + @Configuration @EnableCaching static class FullCachingConfig extends CachingConfigurerSupport { @@ -263,4 +283,5 @@ public class EnableCachingTests extends AbstractAnnotationTests { return new NamedCacheResolver(cacheManager(), "foo"); } } + }