diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java b/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java index 7717e2af49..d3709d211a 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java @@ -236,11 +236,6 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria "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 diff --git a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java index b90f438c5d..84d6bc7696 100644 --- a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java +++ b/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 @@ import static org.mockito.Mockito.*; */ 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 class CacheReproTests { } @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 class CacheReproTests { 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 class CacheReproTests { public interface Spr11124Service { - public List single(int id); + List single(int id); - public List multiple(int id); + List multiple(int id); } @@ -142,7 +172,7 @@ public class CacheReproTests { @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 multiple(int id) { if (this.multipleCount > 0) { fail("Called too many times"); @@ -215,4 +245,49 @@ public class CacheReproTests { } } + @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 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(); + } + } + } diff --git a/spring-context/src/test/java/org/springframework/cache/annotation/AnnotationCacheOperationSourceTests.java b/spring-context/src/test/java/org/springframework/cache/annotation/AnnotationCacheOperationSourceTests.java index 5a6424302c..ee51e13ea1 100644 --- a/spring-context/src/test/java/org/springframework/cache/annotation/AnnotationCacheOperationSourceTests.java +++ b/spring-context/src/test/java/org/springframework/cache/annotation/AnnotationCacheOperationSourceTests.java @@ -191,9 +191,12 @@ public class AnnotationCacheOperationSourceTests { } @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 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