From 33dcef358305865b54e40e48199174ce34081034 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 12 Apr 2016 23:19:51 +0200 Subject: [PATCH] SpringCacheAnnotationParser properly accepts empty @Caching annotation Issue: SPR-14162 (cherry picked from commit da11261) --- .../SpringCacheAnnotationParser.java | 7 +- .../AnnotationCacheOperationSourceTests.java | 95 +++++++++---------- 2 files changed, 50 insertions(+), 52 deletions(-) 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 3fd43c091d..d6992bd263 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 @@ -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. @@ -86,7 +86,10 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria if (cachings != null) { ops = lazyInit(ops); for (Caching caching : cachings) { - ops.addAll(parseCachingAnnotation(ae, cachingConfig, caching)); + Collection cachingOps = parseCachingAnnotation(ae, cachingConfig, caching); + if (cachingOps != null) { + ops.addAll(cachingOps); + } } } 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 35091c83a3..42430474c8 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 @@ -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. @@ -23,10 +23,8 @@ import java.lang.annotation.Target; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Iterator; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -37,7 +35,6 @@ import org.springframework.cache.interceptor.CacheableOperation; import org.springframework.core.annotation.AliasFor; import org.springframework.util.ReflectionUtils; -import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; /** @@ -50,7 +47,7 @@ public class AnnotationCacheOperationSourceTests { @Rule public final ExpectedException exception = ExpectedException.none(); - private AnnotationCacheOperationSource source = new AnnotationCacheOperationSource(); + private final AnnotationCacheOperationSource source = new AnnotationCacheOperationSource(); private Collection getOps(Class target, String name, int expectedNumberOfOperations) { @@ -86,6 +83,11 @@ public class AnnotationCacheOperationSourceTests { assertTrue(it.next() instanceof CacheEvictOperation); } + @Test + public void emptyCaching() throws Exception { + Collection ops = getOps(AnnotatedClass.class, "emptyCaching", 0); + } + @Test public void singularStereotype() throws Exception { Collection ops = getOps(AnnotatedClass.class, "singleStereotype", 1); @@ -105,36 +107,6 @@ public class AnnotationCacheOperationSourceTests { assertTrue(next.getCacheNames().contains("bar")); } - // TODO [SPR-13475] Enable test once @Cache* is supported as a composed annotation. - @Ignore("Disabled until SPR-13475 is resolved") - @Test - public void singleComposedAnnotation() throws Exception { - Collection ops = getOps(AnnotatedClass.class, "singleComposed", 1); - CacheOperation cacheOperation = ops.iterator().next(); - assertThat(cacheOperation, instanceOf(CacheableOperation.class)); - assertThat(cacheOperation.getCacheNames(), equalTo(Collections.singleton("composed"))); - } - - // TODO [SPR-13475] Enable test once @Cache* is supported as a composed annotation. - @Ignore("Disabled until SPR-13475 is resolved") - @Test - public void multipleComposedAnnotations() throws Exception { - Collection ops = getOps(AnnotatedClass.class, "multipleComposed", 3); - Iterator it = ops.iterator(); - - CacheOperation cacheOperation = it.next(); - assertThat(cacheOperation, instanceOf(CacheableOperation.class)); - assertThat(cacheOperation.getCacheNames(), equalTo(Collections.singleton("composedCache"))); - - cacheOperation = it.next(); - assertThat(cacheOperation, instanceOf(CacheableOperation.class)); - assertThat(cacheOperation.getCacheNames(), equalTo(Collections.singleton("foo"))); - - cacheOperation = it.next(); - assertThat(cacheOperation, instanceOf(CacheEvictOperation.class)); - assertThat(cacheOperation.getCacheNames(), equalTo(Collections.singleton("composedCache"))); - } - @Test public void customKeyGenerator() { Collection ops = getOps(AnnotatedClass.class, "customKeyGenerator", 1); @@ -261,17 +233,20 @@ public class AnnotationCacheOperationSourceTests { assertSharedConfig(cacheOperation, "classKeyGenerator", "classCacheManager", "", "classCacheName"); } + private void assertSharedConfig(CacheOperation actual, String keyGenerator, String cacheManager, - String cacheResolver, String... cacheNames) { + String cacheResolver, String... cacheNames) { + assertEquals("Wrong key manager", keyGenerator, actual.getKeyGenerator()); assertEquals("Wrong cache manager", cacheManager, actual.getCacheManager()); assertEquals("Wrong cache resolver", cacheResolver, actual.getCacheResolver()); assertEquals("Wrong number of cache names", cacheNames.length, actual.getCacheNames().size()); Arrays.stream(cacheNames).forEach( - cacheName -> assertTrue("Cache '" + cacheName + "' not found in " + actual.getCacheNames(), - actual.getCacheNames().contains(cacheName))); + cacheName -> assertTrue("Cache '" + cacheName + "' not found in " + actual.getCacheNames(), + actual.getCacheNames().contains(cacheName))); } + private static class AnnotatedClass { @Cacheable("test") @@ -287,6 +262,10 @@ public class AnnotationCacheOperationSourceTests { public void caching() { } + @Caching + public void emptyCaching() { + } + @Cacheable(cacheNames = "test", keyGenerator = "custom") public void customKeyGenerator() { } @@ -309,13 +288,15 @@ public class AnnotationCacheOperationSourceTests { public void multipleStereotype() { } - @ComposedCacheable("composed") + @Cacheable("directly declared") + @ComposedCacheable(cacheNames = "composedCache", key = "composedKey") public void singleComposed() { } + @Cacheable("directly declared") @ComposedCacheable(cacheNames = "composedCache", key = "composedKey") @CacheableFoo - @ComposedCacheEvict(cacheNames = "composedCache", key = "composedKey") + @ComposedCacheEvict(cacheNames = "composedCacheEvict", key = "composedEvictionKey") public void multipleComposed() { } @@ -348,6 +329,7 @@ public class AnnotationCacheOperationSourceTests { } } + @CacheConfig(cacheNames = "classCacheName", keyGenerator = "classKeyGenerator", cacheManager = "classCacheManager", cacheResolver = "classCacheResolver") @@ -370,6 +352,7 @@ public class AnnotationCacheOperationSourceTests { } } + @CacheConfigFoo private static class AnnotatedClassWithCustomDefault { @@ -378,6 +361,7 @@ public class AnnotationCacheOperationSourceTests { } } + @CacheConfig(cacheNames = "classCacheName", keyGenerator = "classKeyGenerator", cacheManager = "classCacheManager") @@ -396,6 +380,7 @@ public class AnnotationCacheOperationSourceTests { } } + @CacheConfigFoo @CacheConfig(cacheNames = "myCache") // multiple sources private static class MultipleCacheConfig { @@ -405,77 +390,87 @@ public class AnnotationCacheOperationSourceTests { } } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @Cacheable("foo") public @interface CacheableFoo { } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @Cacheable(cacheNames = "foo", keyGenerator = "custom") public @interface CacheableFooCustomKeyGenerator { } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @Cacheable(cacheNames = "foo", cacheManager = "custom") public @interface CacheableFooCustomCacheManager { } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @Cacheable(cacheNames = "foo", cacheResolver = "custom") public @interface CacheableFooCustomCacheResolver { } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @CacheEvict("foo") public @interface EvictFoo { } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @CacheEvict("bar") public @interface EvictBar { } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) @CacheConfig(keyGenerator = "classKeyGenerator", - cacheManager = "classCacheManager", cacheResolver = "classCacheResolver") + cacheManager = "classCacheManager", + cacheResolver = "classCacheResolver") public @interface CacheConfigFoo { } + @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.METHOD, ElementType.TYPE }) @Cacheable(cacheNames = "shadowed cache name", key = "shadowed key") - public @interface ComposedCacheable { + @interface ComposedCacheable { - @AliasFor(annotation = Cacheable.class, attribute = "cacheNames") + @AliasFor(annotation = Cacheable.class) String[] value() default {}; - @AliasFor(annotation = Cacheable.class, attribute = "cacheNames") + @AliasFor(annotation = Cacheable.class) String[] cacheNames() default {}; - @AliasFor(annotation = Cacheable.class, attribute = "key") + @AliasFor(annotation = Cacheable.class) String key() default ""; } + @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.METHOD, ElementType.TYPE }) @CacheEvict(cacheNames = "shadowed cache name", key = "shadowed key") - public @interface ComposedCacheEvict { + @interface ComposedCacheEvict { - @AliasFor(annotation = Cacheable.class, attribute = "cacheNames") + @AliasFor(annotation = CacheEvict.class) String[] value() default {}; - @AliasFor(annotation = Cacheable.class, attribute = "cacheNames") + @AliasFor(annotation = CacheEvict.class) String[] cacheNames() default {}; - @AliasFor(annotation = Cacheable.class, attribute = "key") + @AliasFor(annotation = CacheEvict.class) String key() default ""; } -} \ No newline at end of file +}