Allow Cache annotations to not specify any cache name

Since Spring 4.1, a CacheResolver may be configured to customize the way
the cache(s) to use for a given cache operation are retrieved. Since a
CacheResolver implementation may not use the cache names information at
all, this attribute has been made optional.

However, a fix was still applied, preventing a Cache operation without a
cache name to be defined properly. We now allow this valid use case.

Issue: SPR-13081

Author: snicoll

spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java

@@ -236,11 +236,6 @@ private void validateCacheOperation(AnnotatedElement ae, CacheOperation operatio
 					"default cache resolver if none is set. If a cache resolver is set, the cache manager" +
 					"won't be used.");
 		}
-		if (operation.getCacheNames().isEmpty()) {
-			throw new IllegalStateException("No cache names could be detected on '" +
-					ae.toString() + "'. Make sure to set the value parameter on the annotation or " +
-					"declare a @CacheConfig at the class-level with the default cache name(s) to use.");
-		}
 	}
 
 	@Override

spring-context/src/test/java/org/springframework/cache/CacheReproTests.java

@@ -17,18 +17,24 @@
 package org.springframework.cache;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
+import org.junit.Rule;
 import org.junit.Test;
-
+import org.junit.rules.ExpectedException;
 import org.mockito.Mockito;
 
 import org.springframework.cache.annotation.Cacheable;
 import org.springframework.cache.annotation.Caching;
+import org.springframework.cache.annotation.CachingConfigurerSupport;
 import org.springframework.cache.annotation.EnableCaching;
 import org.springframework.cache.concurrent.ConcurrentMapCache;
 import org.springframework.cache.concurrent.ConcurrentMapCacheManager;
+import org.springframework.cache.interceptor.AbstractCacheResolver;
+import org.springframework.cache.interceptor.CacheOperationInvocationContext;
+import org.springframework.cache.interceptor.CacheResolver;
 import org.springframework.cache.support.SimpleCacheManager;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -46,6 +52,9 @@
  */
 public class CacheReproTests {
 
+	@Rule
+	public final ExpectedException thrown = ExpectedException.none();
+
 	@Test
 	public void spr11124() throws Exception {
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr11124Config.class);
@@ -84,7 +93,7 @@ public void spr11592GetSimple() {
 	}
 
 	@Test
-	public void spr11592GetNeverCache(){
+	public void spr11592GetNeverCache() {
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr11592Config.class);
 		Spr11592Service bean = context.getBean(Spr11592Service.class);
 		Cache cache = context.getBean("cache", Cache.class);
@@ -100,6 +109,27 @@ public void spr11592GetNeverCache(){
 		context.close();
 	}
 
+	@Test
+	public void spr13081ConfigNoCacheNameIsRequired() {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr13081Config.class);
+		MyCacheResolver cacheResolver = context.getBean(MyCacheResolver.class);
+		Spr13081Service bean = context.getBean(Spr13081Service.class);
+		assertNull(cacheResolver.getCache("foo").get("foo"));
+		Object result = bean.getSimple("foo"); // cache name = id
+		assertEquals(result, cacheResolver.getCache("foo").get("foo").get());
+	}
+
+	@Test
+	public void spr13081ConfigFailIfCacheResolverReturnsNullCacheName() {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr13081Config.class);
+		Spr13081Service bean = context.getBean(Spr13081Service.class);
+
+
+		thrown.expect(IllegalStateException.class);
+		thrown.expectMessage(MyCacheResolver.class.getName());
+		bean.getSimple(null);
+	}
+
 
 	@Configuration
 	@EnableCaching
@@ -119,9 +149,9 @@ public Spr11124Service service() {
 
 	public interface Spr11124Service {
 
-		public List<String> single(int id);
+		List<String> single(int id);
 
-		public List<String> multiple(int id);
+		List<String> multiple(int id);
 	}
 
 
@@ -142,7 +172,7 @@ public List<String> single(int id) {
 		@Override
 		@Caching(cacheable = {
 				@Cacheable(cacheNames = "bigCache", unless = "#result.size() < 4"),
-				@Cacheable(cacheNames = "smallCache", unless = "#result.size() > 3") })
+				@Cacheable(cacheNames = "smallCache", unless = "#result.size() > 3")})
 		public List<String> multiple(int id) {
 			if (this.multipleCount > 0) {
 				fail("Called too many times");
@@ -215,4 +245,49 @@ public Object getNeverCache(String key) {
 		}
 	}
 
+	@Configuration
+	@EnableCaching
+	public static class Spr13081Config extends CachingConfigurerSupport {
+
+		@Bean
+		@Override
+		public CacheResolver cacheResolver() {
+			return new MyCacheResolver();
+		}
+
+		@Bean
+		public Spr13081Service service() {
+			return new Spr13081Service();
+		}
+
+	}
+
+	public static class MyCacheResolver extends AbstractCacheResolver {
+
+		public MyCacheResolver() {
+			super(new ConcurrentMapCacheManager());
+		}
+
+		@Override
+		protected Collection<String> getCacheNames(CacheOperationInvocationContext<?> context) {
+			String cacheName = (String) context.getArgs()[0];
+			if (cacheName != null) {
+				return Collections.singleton(cacheName);
+			}
+			return null;
+		}
+
+		public Cache getCache(String name) {
+			return getCacheManager().getCache(name);
+		}
+	}
+
+	public static class Spr13081Service {
+
+		@Cacheable
+		public Object getSimple(String cacheName) {
+			return new Object();
+		}
+	}
+
 }

spring-context/src/test/java/org/springframework/cache/annotation/AnnotationCacheOperationSourceTests.java

@@ -191,9 +191,12 @@ public void fullClassLevelWithCustomCacheResolver() {
 	}
 
 	@Test
-	public void validateAtLeastOneCacheNameMustBeSet() {
-		thrown.expect(IllegalStateException.class);
-		getOps(AnnotatedClass.class, "noCacheNameSpecified");
+	public void validateNoCacheIsValid() {
+		// Valid as a CacheResolver might return the cache names to use with other info
+		Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "noCacheNameSpecified");
+		CacheOperation cacheOperation = ops.iterator().next();
+		assertNotNull("cache names set must not be null", cacheOperation.getCacheNames());
+		assertEquals("no cache names specified", 0, cacheOperation.getCacheNames().size());
 	}
 
 	@Test