Align caching AspectJ configuration

The `CacheResolver` and `ErrorHandler` features introduced in 4.1 were
not properly enabled in AspectJ mode. This commit adds more tests from
the regular proxy-based mode and align the AspectJ caching configuration.

Issue: SPR-14413

spring-aspects/src/main/java/org/springframework/cache/aspectj/AspectJCachingConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -39,12 +39,18 @@ public class AspectJCachingConfiguration extends AbstractCachingConfiguration {
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
 	public AnnotationCacheAspect cacheAspect() {
 		AnnotationCacheAspect cacheAspect = AnnotationCacheAspect.aspectOf();
-		if (this.cacheManager != null) {
+		if (this.cacheResolver != null) {
+			cacheAspect.setCacheResolver(this.cacheResolver);
+		}
+		else if (this.cacheManager != null) {
 			cacheAspect.setCacheManager(this.cacheManager);
 		}
 		if (this.keyGenerator != null) {
 			cacheAspect.setKeyGenerator(this.keyGenerator);
 		}
+		if (this.errorHandler != null) {
+			cacheAspect.setErrorHandler(this.errorHandler);
+		}
 		return cacheAspect;
 	}
 

spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java

@@ -0,0 +1,288 @@
+/*
+ * Copyright 2002-2016 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;
+
+import org.junit.After;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import org.springframework.beans.factory.BeanCreationException;
+import org.springframework.cache.CacheManager;
+import org.springframework.cache.CacheTestUtils;
+import org.springframework.cache.annotation.CachingConfigurerSupport;
+import org.springframework.cache.annotation.EnableCaching;
+import org.springframework.cache.config.AnnotatedClassCacheableService;
+import org.springframework.cache.config.CacheableService;
+import org.springframework.cache.config.DefaultCacheableService;
+import org.springframework.cache.config.SomeCustomKeyGenerator;
+import org.springframework.cache.config.SomeKeyGenerator;
+import org.springframework.cache.interceptor.CacheErrorHandler;
+import org.springframework.cache.interceptor.CacheInterceptor;
+import org.springframework.cache.interceptor.CacheResolver;
+import org.springframework.cache.interceptor.KeyGenerator;
+import org.springframework.cache.interceptor.NamedCacheResolver;
+import org.springframework.cache.interceptor.SimpleCacheErrorHandler;
+import org.springframework.cache.interceptor.SimpleCacheResolver;
+import org.springframework.cache.support.NoOpCacheManager;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.annotation.AdviceMode;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+import static org.junit.Assert.*;
+
+/**
+ *
+ * @author Stephane Nicoll
+ */
+public class AspectJEnableCachingIsolatedTests {
+
+	private ConfigurableApplicationContext ctx;
+
+	@After
+	public void closeContext() {
+		if (this.ctx != null) {
+			this.ctx.close();
+		}
+	}
+
+	@Test
+	public void testKeyStrategy() {
+		load(EnableCachingConfig.class);
+		AnnotationCacheAspect aspect = this.ctx.getBean(AnnotationCacheAspect.class);
+		assertSame(this.ctx.getBean("keyGenerator", KeyGenerator.class), aspect.getKeyGenerator());
+	}
+
+	@Test
+	public void testCacheErrorHandler() {
+		load(EnableCachingConfig.class);
+		AnnotationCacheAspect aspect = this.ctx.getBean(AnnotationCacheAspect.class);
+		assertSame(this.ctx.getBean("errorHandler", CacheErrorHandler.class), aspect.getErrorHandler());
+	}
+
+	// --- local tests -------
+
+	@Test
+	public void singleCacheManagerBean() throws Throwable {
+		load(SingleCacheManagerConfig.class);
+	}
+
+	@Test(expected = IllegalStateException.class)
+	public void multipleCacheManagerBeans() throws Throwable {
+		try {
+			load(MultiCacheManagerConfig.class);
+		}
+		catch (BeanCreationException ex) {
+			Throwable root = ex.getRootCause();
+			assertTrue(root.getMessage().contains("beans of type CacheManager"));
+			throw root;
+		}
+	}
+
+	@Test
+	public void multipleCacheManagerBeans_implementsCachingConfigurer() {
+		load(MultiCacheManagerConfigurer.class); // does not throw
+	}
+
+	@Test(expected = IllegalStateException.class)
+	public void multipleCachingConfigurers() throws Throwable {
+		try {
+			load(MultiCacheManagerConfigurer.class, EnableCachingConfig.class);
+		}
+		catch (BeanCreationException ex) {
+			Throwable root = ex.getRootCause();
+			assertTrue(root.getMessage().contains("implementations of CachingConfigurer"));
+			throw root;
+		}
+	}
+
+	@Test(expected = IllegalStateException.class)
+	public void noCacheManagerBeans() throws Throwable {
+		try {
+			load(EmptyConfig.class);
+		}
+		catch (BeanCreationException ex) {
+			Throwable root = ex.getRootCause();
+			assertTrue(root.getMessage().contains("No bean of type CacheManager"));
+			throw root;
+		}
+	}
+
+	@Test
+	@Ignore("AspectJ has some sort of caching that makes this one fail")
+	public void emptyConfigSupport() {
+		load(EmptyConfigSupportConfig.class);
+		AnnotationCacheAspect aspect = this.ctx.getBean(AnnotationCacheAspect.class);
+		assertNotNull(aspect.getCacheResolver());
+		assertEquals(SimpleCacheResolver.class, aspect.getCacheResolver().getClass());
+		assertSame(this.ctx.getBean(CacheManager.class),
+				((SimpleCacheResolver) aspect.getCacheResolver()).getCacheManager());
+	}
+
+	@Test
+	public void bothSetOnlyResolverIsUsed() {
+		load(FullCachingConfig.class);
+
+		AnnotationCacheAspect aspect = this.ctx.getBean(AnnotationCacheAspect.class);
+		assertSame(this.ctx.getBean("cacheResolver"), aspect.getCacheResolver());
+		assertSame(this.ctx.getBean("keyGenerator"), aspect.getKeyGenerator());
+	}
+
+	private void load(Class<?>... config) {
+		this.ctx = new AnnotationConfigApplicationContext(config);
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class EnableCachingConfig extends CachingConfigurerSupport {
+
+		@Override
+		@Bean
+		public CacheManager cacheManager() {
+			return CacheTestUtils.createSimpleCacheManager("testCache", "primary", "secondary");
+		}
+
+		@Bean
+		public CacheableService<?> service() {
+			return new DefaultCacheableService();
+		}
+
+		@Bean
+		public CacheableService<?> classService() {
+			return new AnnotatedClassCacheableService();
+		}
+
+		@Override
+		@Bean
+		public KeyGenerator keyGenerator() {
+			return new SomeKeyGenerator();
+		}
+
+		@Override
+		@Bean
+		public CacheErrorHandler errorHandler() {
+			return new SimpleCacheErrorHandler();
+		}
+
+		@Bean
+		public KeyGenerator customKeyGenerator() {
+			return new SomeCustomKeyGenerator();
+		}
+
+		@Bean
+		public CacheManager customCacheManager() {
+			return CacheTestUtils.createSimpleCacheManager("testCache");
+		}
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class EmptyConfig {
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class SingleCacheManagerConfig {
+
+		@Bean
+		public CacheManager cm1() {
+			return new NoOpCacheManager();
+		}
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class MultiCacheManagerConfig {
+
+		@Bean
+		public CacheManager cm1() {
+			return new NoOpCacheManager();
+		}
+
+		@Bean
+		public CacheManager cm2() {
+			return new NoOpCacheManager();
+		}
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class MultiCacheManagerConfigurer extends CachingConfigurerSupport {
+
+		@Bean
+		public CacheManager cm1() {
+			return new NoOpCacheManager();
+		}
+
+		@Bean
+		public CacheManager cm2() {
+			return new NoOpCacheManager();
+		}
+
+		@Override
+		public CacheManager cacheManager() {
+			return cm1();
+		}
+
+		@Override
+		public KeyGenerator keyGenerator() {
+			return null;
+		}
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class EmptyConfigSupportConfig extends CachingConfigurerSupport {
+
+		@Bean
+		public CacheManager cm() {
+			return new NoOpCacheManager();
+		}
+
+	}
+
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class FullCachingConfig extends CachingConfigurerSupport {
+
+		@Override
+		@Bean
+		public CacheManager cacheManager() {
+			return new NoOpCacheManager();
+		}
+
+		@Override
+		@Bean
+		public KeyGenerator keyGenerator() {
+			return new SomeKeyGenerator();
+		}
+
+		@Override
+		@Bean
+		public CacheResolver cacheResolver() {
+			return new NamedCacheResolver(cacheManager(), "foo");
+		}
+	}
+}

spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingTests.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2002-2016 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;
+
+import org.springframework.cache.CacheManager;
+import org.springframework.cache.CacheTestUtils;
+import org.springframework.cache.annotation.CachingConfigurerSupport;
+import org.springframework.cache.annotation.EnableCaching;
+import org.springframework.cache.config.AbstractCacheAnnotationTests;
+import org.springframework.cache.config.AnnotatedClassCacheableService;
+import org.springframework.cache.config.CacheableService;
+import org.springframework.cache.config.DefaultCacheableService;
+import org.springframework.cache.config.SomeCustomKeyGenerator;
+import org.springframework.cache.config.SomeKeyGenerator;
+import org.springframework.cache.interceptor.CacheErrorHandler;
+import org.springframework.cache.interceptor.KeyGenerator;
+import org.springframework.cache.interceptor.SimpleCacheErrorHandler;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.annotation.AdviceMode;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ *
+ * @author Stephane Nicoll
+ */
+public class AspectJEnableCachingTests extends AbstractCacheAnnotationTests {
+
+	/** hook into superclass suite of tests */
+	@Override
+	protected ConfigurableApplicationContext getApplicationContext() {
+		return new AnnotationConfigApplicationContext(EnableCachingConfig.class);
+	}
+
+	@Configuration
+	@EnableCaching(mode = AdviceMode.ASPECTJ)
+	static class EnableCachingConfig extends CachingConfigurerSupport {
+
+		@Override
+		@Bean
+		public CacheManager cacheManager() {
+			return CacheTestUtils.createSimpleCacheManager("testCache", "primary", "secondary");
+		}
+
+		@Bean
+		public CacheableService<?> service() {
+			return new DefaultCacheableService();
+		}
+
+		@Bean
+		public CacheableService<?> classService() {
+			return new AnnotatedClassCacheableService();
+		}
+
+		@Override
+		@Bean
+		public KeyGenerator keyGenerator() {
+			return new SomeKeyGenerator();
+		}
+
+		@Override
+		@Bean
+		public CacheErrorHandler errorHandler() {
+			return new SimpleCacheErrorHandler();
+		}
+
+		@Bean
+		public KeyGenerator customKeyGenerator() {
+			return new SomeCustomKeyGenerator();
+		}
+
+		@Bean
+		public CacheManager customCacheManager() {
+			return CacheTestUtils.createSimpleCacheManager("testCache");
+		}
+	}
+
+}

spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -207,7 +207,7 @@ public class AnnotatedClassCacheableService implements CacheableService<Object>
 	}
 
 	@Override
-	@Caching(evict = { @CacheEvict("primary"), @CacheEvict(cacheNames = "secondary", key = "#p0") })
+	@Caching(evict = { @CacheEvict("primary"), @CacheEvict(cacheNames = "secondary", key = "#a0"),  @CacheEvict(cacheNames = "primary", key = "#p0 + 'A'") })
 	public Object multiEvict(Object arg1) {
 		return counter.getAndIncrement();
 	}
@@ -219,7 +219,7 @@ public class AnnotatedClassCacheableService implements CacheableService<Object>
 	}
 
 	@Override
-	@Caching(cacheable = { @Cacheable(cacheNames = "primary", condition = "#p0 == 3") }, evict = { @CacheEvict("secondary") })
+	@Caching(cacheable = { @Cacheable(cacheNames = "primary", condition = "#a0 == 3") }, evict = { @CacheEvict("secondary") })
 	public Object multiConditionalCacheAndEvict(Object arg1) {
 		return counter.getAndIncrement();
 	}

spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -216,8 +216,7 @@ public class DefaultCacheableService implements CacheableService<Long> {
 	}
 
 	@Override
-//FIXME	@Caching(evict = { @CacheEvict("primary"), @CacheEvict(cacheNames = "secondary", key = "#p0"), @CacheEvict(cacheNames = "primary", key = "#p0 + 'A'") })
-	@Caching(evict = { @CacheEvict("primary"), @CacheEvict(cacheNames = "secondary", key = "#p0") })
+	@Caching(evict = { @CacheEvict("primary"), @CacheEvict(cacheNames = "secondary", key = "#p0"), @CacheEvict(cacheNames = "primary", key = "#p0 + 'A'") })
 	public Long multiEvict(Object arg1) {
 		return counter.getAndIncrement();
 	}

spring-aspects/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -26,7 +26,7 @@ import org.springframework.cache.interceptor.KeyGenerator;
  *
  * @author Stephane Nicoll
  */
-final class SomeCustomKeyGenerator implements KeyGenerator {
+public final class SomeCustomKeyGenerator implements KeyGenerator {
 
     @Override
     public Object generate(Object target, Method method, Object... params) {