Revised retrieval of cache strategy beans

Issue: SPR-12336

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<O extends AbstractJCacheOperation<A>, A
 
 	protected final Log logger = LogFactory.getLog(getClass());
 
+
 	protected AbstractCacheInterceptor(CacheErrorHandler errorHandler) {
 		super(errorHandler);
 	}
 
-	protected abstract Object invoke(CacheOperationInvocationContext<O> context,
+
-			CacheOperationInvoker invoker) throws Throwable;
+	protected abstract Object invoke(CacheOperationInvocationContext<O> context, CacheOperationInvoker invoker)
+			throws Throwable;
+
 
 	/**
 	 * Resolve the cache to use.
@@ -68,15 +72,15 @@ abstract class AbstractCacheInterceptor<O extends AbstractJCacheOperation<A>, A
 	 * @return the singe element or {@code null} if the collection is empty
 	 */
 	static Cache extractFrom(Collection<? extends Cache> 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 "
+			throw new IllegalStateException("Unsupported cache resolution result " + caches +
-					+ caches + " JSR-107 only supports a single cache.");
+					": JSR-107 only supports a single cache.");
 		}
 	}
 

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> T getBean(Class<T> 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,
+	protected CachePutOperation createCachePutOperation(Method method, CacheDefaults defaults, CachePut ann) {
-			CachePut ann) {
 		String cacheName = determineCacheName(method, defaults, ann.cacheName());
 		CacheResolverFactory cacheResolverFactory =
 				determineCacheResolverFactory(defaults, ann.cacheResolverFactory());
 		KeyGenerator keyGenerator = determineKeyGenerator(defaults, ann.cacheKeyGenerator());
 
 		CacheMethodDetails<CachePut> methodDetails = createMethodDetails(method, ann, cacheName);
-
 		CacheResolver cacheResolver = getCacheResolver(cacheResolverFactory, methodDetails);
-
 		return new CachePutOperation(methodDetails, cacheResolver, keyGenerator);
 	}
 
-	protected CacheRemoveOperation createCacheRemoveOperation(Method method, CacheDefaults defaults,
+	protected CacheRemoveOperation createCacheRemoveOperation(Method method, CacheDefaults defaults, CacheRemove ann) {
-			CacheRemove ann) {
 		String cacheName = determineCacheName(method, defaults, ann.cacheName());
 		CacheResolverFactory cacheResolverFactory =
 				determineCacheResolverFactory(defaults, ann.cacheResolverFactory());
 		KeyGenerator keyGenerator = determineKeyGenerator(defaults, ann.cacheKeyGenerator());
 
 		CacheMethodDetails<CacheRemove> methodDetails = createMethodDetails(method, ann, cacheName);
-
 		CacheResolver cacheResolver = getCacheResolver(cacheResolverFactory, methodDetails);
-
 		return new CacheRemoveOperation(methodDetails, cacheResolver, keyGenerator);
 	}
 
-	protected CacheRemoveAllOperation createCacheRemoveAllOperation(Method method, CacheDefaults defaults,
+	protected CacheRemoveAllOperation createCacheRemoveAllOperation(Method method, CacheDefaults defaults, CacheRemoveAll ann) {
-			CacheRemoveAll ann) {
 		String cacheName = determineCacheName(method, defaults, ann.cacheName());
 		CacheResolverFactory cacheResolverFactory =
 				determineCacheResolverFactory(defaults, ann.cacheResolverFactory());
 
 		CacheMethodDetails<CacheRemoveAll> methodDetails = createMethodDetails(method, ann, cacheName);
-
 		CacheResolver cacheResolver = getCacheResolver(cacheResolverFactory, methodDetails);
-
 		return new CacheRemoveAllOperation(methodDetails, cacheResolver);
 	}
 
-	private <A extends Annotation> CacheMethodDetails<A> createMethodDetails(
+	private <A extends Annotation> CacheMethodDetails<A> createMethodDetails(Method method, A annotation, String cacheName) {
-			Method method, A annotation, String cacheName) {
 		return new DefaultCacheMethodDetails<A>(method, annotation, cacheName);
 	}
 
@@ -181,6 +147,7 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC
 
 	protected CacheResolver getExceptionCacheResolver(CacheResolverFactory factory,
 			CacheMethodDetails<CacheResult> 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<? extends CacheResolverFactory> candidate) {
+
 		if (!CacheResolverFactory.class.equals(candidate)) {
 			return getBean(candidate);
 		}
@@ -203,8 +171,7 @@ public abstract class AnnotationJCacheOperationSource extends AbstractFallbackJC
 		}
 	}
 
-	protected KeyGenerator determineKeyGenerator(CacheDefaults defaults,
+	protected KeyGenerator determineKeyGenerator(CacheDefaults defaults, Class<? extends CacheKeyGenerator> candidate) {
-			Class<? extends CacheKeyGenerator> 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<String> parameters = new ArrayList<String>();
+		List<String> parameters = new ArrayList<String>(parameterTypes.length);
 		for (Class<?> parameterType : parameterTypes) {
 			parameters.add(parameterType.getName());
 		}
 
-		StringBuilder sb = new StringBuilder();
+		StringBuilder sb = new StringBuilder(method.getDeclaringClass().getName());
-		sb.append(method.getDeclaringClass().getName())
+		sb.append(".").append(method.getName());
-				.append(".")
+		sb.append("(").append(StringUtils.collectionToCommaDelimitedString(parameters)).append(")");
-				.append(method.getName())
-				.append("(")
-				.append(StringUtils.collectionToCommaDelimitedString(parameters))
-				.append(")");
 		return sb.toString();
 	}
 
 	private int countNonNull(Object... instances) {
 		int result = 0;
-		for (Object o : instances) {
+		for (Object instance : instances) {
-			if (o != null) {
+			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> T getBean(Class<T> 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();
+
 }

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
+	public void afterSingletonsInstantiated() {
-		Assert.notNull(getDefaultCacheResolver(), "Cache resolver should have been initialized.");
+		// Make sure those are initialized on startup...
-		Assert.notNull(getDefaultExceptionCacheResolver(), "Exception cache resolver should have been initialized.");
+		Assert.notNull(getDefaultCacheResolver(), "Cache resolver should have been initialized");
+		Assert.notNull(getDefaultExceptionCacheResolver(), "Exception cache resolver should have been initialized");
 	}
 
+
 	@Override
 	protected <T> T getBean(Class<T> type) {
-		Map<String, T> map = BeanFactoryUtils.beansOfTypeIncludingAncestors(this.applicationContext, type);
+		try {
-		if (map.size() == 1) {
+			return this.applicationContext.getBean(type);
-			return map.values().iterator().next();
 		}
-		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);
+			try {
-			if (this.cacheManager == null) {
+				this.cacheManager = this.applicationContext.getBean(CacheManager.class);
-				throw new IllegalStateException("No bean of type CacheManager could be found. " +
+			}
-						"Register a CacheManager bean or remove the @EnableCaching annotation " +
+			catch (NoUniqueBeanDefinitionException ex) {
-						"from your configuration.");
+				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;

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
+		// Check whether aspect is enabled to cope with cases where the AJ is pulled in automatically
-		// 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,
+	private CacheOperationInvocationContext<?> createCacheOperationInvocationContext(
-			Object[] args,
+			Object target, Object[] args, JCacheOperation<?> operation) {
-			JCacheOperation<?> operation) {
+
 		return new DefaultCacheInvocationContext<Annotation>(
 				(JCacheOperation<Annotation>) operation, target, args);
 	}
@@ -124,12 +123,10 @@ public class JCacheAspectSupport extends AbstractCacheInvoker implements Initial
 	}
 
 	@SuppressWarnings("unchecked")
-	private Object execute(CacheOperationInvocationContext<?> context,
+	private Object execute(CacheOperationInvocationContext<?> context, CacheOperationInvoker invoker) {
-			CacheOperationInvoker invoker) {
-
 		CacheOperationInvoker adapter = new CacheOperationInvokerAdapter(invoker);
-
 		BasicOperation operation = context.getOperation();
+
 		if (operation instanceof CacheResultOperation) {
 			return cacheResultInterceptor.invoke(
 					(CacheOperationInvocationContext<CacheResultOperation>) context, adapter);

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) {
+		catch (IllegalStateException ex) {
-			assertTrue(e.getMessage().contains("JSR-107 only supports a single cache."));
+			assertTrue(ex.getMessage().contains("JSR-107 only supports a single cache"));
 		}
-		catch (Throwable t) {
+		catch (Throwable ex) {
-			fail("Unexpected: " + t);
+			fail("Unexpected: " + ex);
 		}
 	}
 
@@ -71,17 +70,17 @@ public class JCacheInterceptorTests extends AbstractJCacheTests {
 		try {
 			interceptor.execute(dummyInvoker, service, m, new Object[] {"myId"});
 		}
-		catch (IllegalStateException e) {
+		catch (IllegalStateException ex) {
-			assertTrue(e.getMessage().contains("Cache could not have been resolved for"));
+			assertTrue(ex.getMessage().contains("Cache could not have been resolved for"));
 		}
-		catch (Throwable t) {
+		catch (Throwable ex) {
-			fail("Unexpected: " + t);
+			fail("Unexpected: " + ex);
 		}
 	}
 
 	@Test
-	public void cacheManagerMandatoryIfCacheResolverNotSetSet() {
+	public void cacheManagerMandatoryIfCacheResolverNotSet() {
-		thrown.expect(NoSuchBeanDefinitionException.class);
+		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,
+				cacheManager, defaultCacheResolver, defaultExceptionCacheResolver, defaultKeyGenerator));
-				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 cacheResolver, CacheResolver exceptionCacheResolver, KeyGenerator keyGenerator) {
-			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;

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
+	 * attributes. If more than one source is provided, they will be aggregated
-	 * {@link CompositeCacheOperationSource}.
+	 * using a {@link CompositeCacheOperationSource}.
-	 * @param cacheOperationSources must not be {@code null}
 	 */
 	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
+	 * Set the {@link CacheManager} to use to create a default {@link CacheResolver}.
-	 * the current {@link CacheResolver}, if any.
+	 * 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
+		if (getCacheResolver() == null) {
-			CacheManager cacheManager = this.applicationContext.getBean(CacheManager.class);
+			// Lazily initialize cache resolver via default cache manager...
-			if (cacheManager == null) {
+			try {
-				throw new IllegalStateException("No bean of type CacheManager could be found. " +
+				setCacheManager(this.applicationContext.getBean(CacheManager.class));
-						"Register a CacheManager bean or remove the @EnableCaching annotation " +
+			}
-						"from your configuration.");
+			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<? extends Cache> getCaches(CacheOperationInvocationContext<CacheOperation> context,
+	protected Collection<? extends Cache> getCaches(
-													CacheResolver cacheResolver) {
+			CacheOperationInvocationContext<CacheOperation> context, CacheResolver cacheResolver) {
+
 		Collection<? extends Cache> caches = cacheResolver.resolveCaches(context);
 		if (caches.isEmpty()) {
-			throw new IllegalStateException("No cache could be resolved for '" + context.getOperation()
+			throw new IllegalStateException("No cache could be resolved for '" +
-					+ "' using  resolver '" + cacheResolver
+					context.getOperation() + "' using resolver '" + cacheResolver +
-					+ "'. At least one cache should be provided per cache operation.");
+					"'. At least one cache should be provided per cache operation.");
 		}
 		return caches;
 	}
 
-	protected CacheOperationContext getOperationContext(CacheOperation operation, Method method, Object[] args,
+	protected CacheOperationContext getOperationContext(
-														Object target, Class<?> targetClass) {
+			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,
+	protected CacheOperationMetadata getCacheOperationMetadata(
-															   Method method, Class<?> targetClass) {
+			CacheOperation operation, Method method, Class<?> targetClass) {
-		final CacheOperationCacheKey cacheKey = new CacheOperationCacheKey(operation, method, targetClass);
+
-		CacheOperationMetadata metadata = metadataCache.get(cacheKey);
+		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> T getBean(String beanName, Class<T> expectedType) {
-		return BeanFactoryAnnotationUtils.qualifiedBeanOfType(
+		return BeanFactoryAnnotationUtils.qualifiedBeanOfType(this.applicationContext, expectedType, beanName);
-				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<Class<? extends CacheOperation>, CacheOperationContext> contexts =
 				new LinkedMultiValueMap<Class<? extends CacheOperation>, CacheOperationContext>();
 
-		public CacheOperationContexts(Collection<? extends CacheOperation> operations,
+		public CacheOperationContexts(Collection<? extends CacheOperation> operations, Method method,
-									  Method method, Object[] args, Object target, Class<?> targetClass) {
+				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,
+		public CacheOperationMetadata(CacheOperation operation, Method method, Class<?> targetClass,
-									  Class<?> targetClass, KeyGenerator keyGenerator,
+				KeyGenerator keyGenerator, CacheResolver cacheResolver) {
-									  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<CacheOperation> {
 
 		private final CacheOperationMetadata metadata;
@@ -536,8 +546,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
 
 		private final MethodCacheKey methodCacheKey;
 
-		public CacheOperationContext(CacheOperationMetadata metadata,
+		public CacheOperationContext(CacheOperationMetadata metadata, Object[] args, Object target) {
-									 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.
+		 * Compute the key for the given caching operation.
-		 * @return generated key (null if none can be generated)
+		 * @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) {
+		public boolean equals(Object other) {
-			if (this == o) return true;
+			if (this == other) {
-			if (o == null || getClass() != o.getClass()) return false;
+				return true;
-
+			}
-			CacheOperationCacheKey that = (CacheOperationCacheKey) o;
+			if (!(other instanceof CacheOperationCacheKey)) {
-			return cacheOperation.equals(that.cacheOperation)
+				return false;
-					&& methodCacheKey.equals(that.methodCacheKey);
+			}
+			CacheOperationCacheKey otherKey = (CacheOperationCacheKey) other;
+			return (this.cacheOperation.equals(otherKey.cacheOperation) &&
+					this.methodCacheKey.equals(otherKey.methodCacheKey));
 		}
 
 		@Override
 		public int hashCode() {
-			int result = cacheOperation.hashCode();
+			return (this.cacheOperation.hashCode() * 31 + this.methodCacheKey.hashCode());
-			result = 31 * result + methodCacheKey.hashCode();
-			return result;
 		}
 	}
 }

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");
 		}
 	}
+
 }