Skip to content

Commit 109a2b4

Browse files
jhoellerjhoeller
committed
Consistently skip unnecessary search on superclasses and empty elements
Includes caching of declared annotation arrays and combined searching for several annotation types (used in SpringCacheAnnotationParser). Issue: SPR-16933
1 parent 50dc8c2 commit 109a2b4

File tree

6 files changed

+292
-270
lines changed

6 files changed

+292
-270
lines changed

‎spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean
120120

121121
protected final Log logger = LogFactory.getLog(getClass());
122122

123-
private final Set<Class<? extends Annotation>> autowiredAnnotationTypes = new LinkedHashSet<>();
123+
private final Set<Class<? extends Annotation>> autowiredAnnotationTypes = new LinkedHashSet<>(4);
124124

125125
private String requiredParameterName = "required";
126126

@@ -490,7 +490,7 @@ private InjectionMetadata buildAutowiringMetadata(final Class<?> clazz) {
490490

491491
@Nullable
492492
private AnnotationAttributes findAutowiredAnnotation(AccessibleObject ao) {
493-
if (ao.getAnnotations().length > 0) {
493+
if (ao.getAnnotations().length > 0) { // autowiring annotations have to be local
494494
for (Class<? extends Annotation> type : this.autowiredAnnotationTypes) {
495495
AnnotationAttributes attributes = AnnotatedElementUtils.getMergedAnnotationAttributes(ao, type);
496496
if (attributes != null) {

‎spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -347,10 +347,12 @@ public Object getSuggestedValue(DependencyDescriptor descriptor) {
347347
*/
348348
@Nullable
349349
protected Object findValue(Annotation[] annotationsToSearch) {
350-
AnnotationAttributes attr = AnnotatedElementUtils.getMergedAnnotationAttributes(
351-
AnnotatedElementUtils.forAnnotations(annotationsToSearch), this.valueAnnotationType);
352-
if (attr != null) {
353-
return extractValue(attr);
350+
if (annotationsToSearch.length > 0) { // qualifier annotations have to be local
351+
AnnotationAttributes attr = AnnotatedElementUtils.getMergedAnnotationAttributes(
352+
AnnotatedElementUtils.forAnnotations(annotationsToSearch), this.valueAnnotationType);
353+
if (attr != null) {
354+
return extractValue(attr);
355+
}
354356
}
355357
return null;
356358
}

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

Lines changed: 66 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@
2222
import java.lang.reflect.Method;
2323
import java.util.ArrayList;
2424
import java.util.Collection;
25+
import java.util.LinkedHashSet;
26+
import java.util.Set;
2527

2628
import org.springframework.cache.interceptor.CacheEvictOperation;
2729
import org.springframework.cache.interceptor.CacheOperation;
2830
import org.springframework.cache.interceptor.CachePutOperation;
2931
import org.springframework.cache.interceptor.CacheableOperation;
3032
import org.springframework.core.annotation.AnnotatedElementUtils;
3133
import org.springframework.lang.Nullable;
32-
import org.springframework.util.ObjectUtils;
3334
import org.springframework.util.StringUtils;
3435

3536
/**
@@ -47,24 +48,34 @@
4748
@SuppressWarnings("serial")
4849
public class SpringCacheAnnotationParser implements CacheAnnotationParser, Serializable {
4950

51+
private static final Set<Class<? extends Annotation>> CACHE_OPERATION_ANNOTATIONS = new LinkedHashSet<>(8);
52+
53+
static {
54+
CACHE_OPERATION_ANNOTATIONS.add(Cacheable.class);
55+
CACHE_OPERATION_ANNOTATIONS.add(CacheEvict.class);
56+
CACHE_OPERATION_ANNOTATIONS.add(CachePut.class);
57+
CACHE_OPERATION_ANNOTATIONS.add(Caching.class);
58+
}
59+
60+
5061
@Override
5162
@Nullable
5263
public Collection<CacheOperation> parseCacheAnnotations(Class<?> type) {
53-
DefaultCacheConfig defaultConfig = getDefaultCacheConfig(type);
64+
DefaultCacheConfig defaultConfig = new DefaultCacheConfig(type);
5465
return parseCacheAnnotations(defaultConfig, type);
5566
}
5667

5768
@Override
5869
@Nullable
5970
public Collection<CacheOperation> parseCacheAnnotations(Method method) {
60-
DefaultCacheConfig defaultConfig = getDefaultCacheConfig(method.getDeclaringClass());
71+
DefaultCacheConfig defaultConfig = new DefaultCacheConfig(method.getDeclaringClass());
6172
return parseCacheAnnotations(defaultConfig, method);
6273
}
6374

6475
@Nullable
6576
private Collection<CacheOperation> parseCacheAnnotations(DefaultCacheConfig cachingConfig, AnnotatedElement ae) {
6677
Collection<CacheOperation> ops = parseCacheAnnotations(cachingConfig, ae, false);
67-
if (ops != null && ops.size() > 1 && ae.getAnnotations().length > 0) {
78+
if (ops != null && ops.size() > 1) {
6879
// More than one operation found -> local declarations override interface-declared ones...
6980
Collection<CacheOperation> localOps = parseCacheAnnotations(cachingConfig, ae, true);
7081
if (localOps != null) {
@@ -78,55 +89,28 @@ private Collection<CacheOperation> parseCacheAnnotations(DefaultCacheConfig cach
7889
private Collection<CacheOperation> parseCacheAnnotations(
7990
DefaultCacheConfig cachingConfig, AnnotatedElement ae, boolean localOnly) {
8091

81-
Collection<CacheOperation> ops = null;
82-
83-
Collection<Cacheable> cacheables = (localOnly ? AnnotatedElementUtils.getAllMergedAnnotations(ae, Cacheable.class) :
84-
AnnotatedElementUtils.findAllMergedAnnotations(ae, Cacheable.class));
85-
if (!cacheables.isEmpty()) {
86-
ops = lazyInit(null);
87-
for (Cacheable cacheable : cacheables) {
88-
ops.add(parseCacheableAnnotation(ae, cachingConfig, cacheable));
89-
}
90-
}
91-
92-
Collection<CacheEvict> evicts = (localOnly ? AnnotatedElementUtils.getAllMergedAnnotations(ae, CacheEvict.class) :
93-
AnnotatedElementUtils.findAllMergedAnnotations(ae, CacheEvict.class));
94-
if (!evicts.isEmpty()) {
95-
ops = lazyInit(ops);
96-
for (CacheEvict evict : evicts) {
97-
ops.add(parseEvictAnnotation(ae, cachingConfig, evict));
98-
}
99-
}
100-
101-
Collection<CachePut> puts = (localOnly ? AnnotatedElementUtils.getAllMergedAnnotations(ae, CachePut.class) :
102-
AnnotatedElementUtils.findAllMergedAnnotations(ae, CachePut.class));
103-
if (!puts.isEmpty()) {
104-
ops = lazyInit(ops);
105-
for (CachePut put : puts) {
106-
ops.add(parsePutAnnotation(ae, cachingConfig, put));
107-
}
108-
}
109-
110-
Collection<Caching> cachings = (localOnly ? AnnotatedElementUtils.getAllMergedAnnotations(ae, Caching.class) :
111-
AnnotatedElementUtils.findAllMergedAnnotations(ae, Caching.class));
112-
if (!cachings.isEmpty()) {
113-
ops = lazyInit(ops);
114-
for (Caching caching : cachings) {
115-
Collection<CacheOperation> cachingOps = parseCachingAnnotation(ae, cachingConfig, caching);
116-
if (cachingOps != null) {
117-
ops.addAll(cachingOps);
118-
}
119-
}
92+
Collection<? extends Annotation> anns = (localOnly ?
93+
AnnotatedElementUtils.getAllMergedAnnotations(ae, CACHE_OPERATION_ANNOTATIONS) :
94+
AnnotatedElementUtils.findAllMergedAnnotations(ae, CACHE_OPERATION_ANNOTATIONS));
95+
if (anns.isEmpty()) {
96+
return null;
12097
}
12198

99+
final Collection<CacheOperation> ops = new ArrayList<>(1);
100+
anns.stream().filter(ann -> ann instanceof Cacheable).forEach(
101+
ann -> ops.add(parseCacheableAnnotation(ae, cachingConfig, (Cacheable) ann)));
102+
anns.stream().filter(ann -> ann instanceof CacheEvict).forEach(
103+
ann -> ops.add(parseEvictAnnotation(ae, cachingConfig, (CacheEvict) ann)));
104+
anns.stream().filter(ann -> ann instanceof CachePut).forEach(
105+
ann -> ops.add(parsePutAnnotation(ae, cachingConfig, (CachePut) ann)));
106+
anns.stream().filter(ann -> ann instanceof Caching).forEach(
107+
ann -> parseCachingAnnotation(ae, cachingConfig, (Caching) ann, ops));
122108
return ops;
123109
}
124110

125-
private <T extends Annotation> Collection<CacheOperation> lazyInit(@Nullable Collection<CacheOperation> ops) {
126-
return (ops != null ? ops : new ArrayList<>(1));
127-
}
111+
private CacheableOperation parseCacheableAnnotation(
112+
AnnotatedElement ae, DefaultCacheConfig defaultConfig, Cacheable cacheable) {
128113

129-
CacheableOperation parseCacheableAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, Cacheable cacheable) {
130114
CacheableOperation.Builder builder = new CacheableOperation.Builder();
131115

132116
builder.setName(ae.toString());
@@ -146,7 +130,9 @@ CacheableOperation parseCacheableAnnotation(AnnotatedElement ae, DefaultCacheCon
146130
return op;
147131
}
148132

149-
CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, CacheEvict cacheEvict) {
133+
private CacheEvictOperation parseEvictAnnotation(
134+
AnnotatedElement ae, DefaultCacheConfig defaultConfig, CacheEvict cacheEvict) {
135+
150136
CacheEvictOperation.Builder builder = new CacheEvictOperation.Builder();
151137

152138
builder.setName(ae.toString());
@@ -166,7 +152,9 @@ CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, DefaultCacheConfig
166152
return op;
167153
}
168154

169-
CacheOperation parsePutAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, CachePut cachePut) {
155+
private CacheOperation parsePutAnnotation(
156+
AnnotatedElement ae, DefaultCacheConfig defaultConfig, CachePut cachePut) {
157+
170158
CachePutOperation.Builder builder = new CachePutOperation.Builder();
171159

172160
builder.setName(ae.toString());
@@ -185,48 +173,23 @@ CacheOperation parsePutAnnotation(AnnotatedElement ae, DefaultCacheConfig defaul
185173
return op;
186174
}
187175

188-
@Nullable
189-
Collection<CacheOperation> parseCachingAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, Caching caching) {
190-
Collection<CacheOperation> ops = null;
176+
private void parseCachingAnnotation(
177+
AnnotatedElement ae, DefaultCacheConfig defaultConfig, Caching caching, Collection<CacheOperation> ops) {
191178

192179
Cacheable[] cacheables = caching.cacheable();
193-
if (!ObjectUtils.isEmpty(cacheables)) {
194-
ops = lazyInit(null);
195-
for (Cacheable cacheable : cacheables) {
196-
ops.add(parseCacheableAnnotation(ae, defaultConfig, cacheable));
197-
}
180+
for (Cacheable cacheable : cacheables) {
181+
ops.add(parseCacheableAnnotation(ae, defaultConfig, cacheable));
198182
}
199183
CacheEvict[] cacheEvicts = caching.evict();
200-
if (!ObjectUtils.isEmpty(cacheEvicts)) {
201-
ops = lazyInit(ops);
202-
for (CacheEvict cacheEvict : cacheEvicts) {
203-
ops.add(parseEvictAnnotation(ae, defaultConfig, cacheEvict));
204-
}
184+
for (CacheEvict cacheEvict : cacheEvicts) {
185+
ops.add(parseEvictAnnotation(ae, defaultConfig, cacheEvict));
205186
}
206187
CachePut[] cachePuts = caching.put();
207-
if (!ObjectUtils.isEmpty(cachePuts)) {
208-
ops = lazyInit(ops);
209-
for (CachePut cachePut : cachePuts) {
210-
ops.add(parsePutAnnotation(ae, defaultConfig, cachePut));
211-
}
188+
for (CachePut cachePut : cachePuts) {
189+
ops.add(parsePutAnnotation(ae, defaultConfig, cachePut));
212190
}
213-
214-
return ops;
215191
}
216192

217-
/**
218-
* Provides the {@link DefaultCacheConfig} instance for the specified {@link Class}.
219-
* @param target the class-level to handle
220-
* @return the default config (never {@code null})
221-
*/
222-
DefaultCacheConfig getDefaultCacheConfig(Class<?> target) {
223-
CacheConfig annotation = AnnotatedElementUtils.findMergedAnnotation(target, CacheConfig.class);
224-
if (annotation != null) {
225-
return new DefaultCacheConfig(annotation.cacheNames(), annotation.keyGenerator(),
226-
annotation.cacheManager(), annotation.cacheResolver());
227-
}
228-
return new DefaultCacheConfig();
229-
}
230193

231194
/**
232195
* Validates the specified {@link CacheOperation}.
@@ -266,38 +229,44 @@ public int hashCode() {
266229
/**
267230
* Provides default settings for a given set of cache operations.
268231
*/
269-
static class DefaultCacheConfig {
232+
private static class DefaultCacheConfig {
270233

271-
@Nullable
272-
private final String[] cacheNames;
234+
private final Class<?> target;
273235

274236
@Nullable
275-
private final String keyGenerator;
237+
private String[] cacheNames;
276238

277239
@Nullable
278-
private final String cacheManager;
240+
private String keyGenerator;
279241

280242
@Nullable
281-
private final String cacheResolver;
243+
private String cacheManager;
282244

283-
public DefaultCacheConfig() {
284-
this(null, null, null, null);
285-
}
245+
@Nullable
246+
private String cacheResolver;
286247

287-
private DefaultCacheConfig(@Nullable String[] cacheNames, @Nullable String keyGenerator,
288-
@Nullable String cacheManager, @Nullable String cacheResolver) {
248+
private boolean initialized = false;
289249

290-
this.cacheNames = cacheNames;
291-
this.keyGenerator = keyGenerator;
292-
this.cacheManager = cacheManager;
293-
this.cacheResolver = cacheResolver;
250+
public DefaultCacheConfig(Class<?> target) {
251+
this.target = target;
294252
}
295253

296254
/**
297255
* Apply the defaults to the specified {@link CacheOperation.Builder}.
298256
* @param builder the operation builder to update
299257
*/
300258
public void applyDefault(CacheOperation.Builder builder) {
259+
if (!this.initialized) {
260+
CacheConfig annotation = AnnotatedElementUtils.findMergedAnnotation(this.target, CacheConfig.class);
261+
if (annotation != null) {
262+
this.cacheNames = annotation.cacheNames();
263+
this.keyGenerator = annotation.keyGenerator();
264+
this.cacheManager = annotation.cacheManager();
265+
this.cacheResolver = annotation.cacheResolver();
266+
}
267+
this.initialized = true;
268+
}
269+
301270
if (builder.getCacheNames().isEmpty() && this.cacheNames != null) {
302271
builder.setCacheNames(this.cacheNames);
303272
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -52,40 +52,40 @@ public class AnnotationCacheOperationSourceTests {
5252

5353

5454
@Test
55-
public void singularAnnotation() throws Exception {
55+
public void singularAnnotation() {
5656
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "singular", 1);
5757
assertTrue(ops.iterator().next() instanceof CacheableOperation);
5858
}
5959

6060
@Test
61-
public void multipleAnnotation() throws Exception {
61+
public void multipleAnnotation() {
6262
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "multiple", 2);
6363
Iterator<CacheOperation> it = ops.iterator();
6464
assertTrue(it.next() instanceof CacheableOperation);
6565
assertTrue(it.next() instanceof CacheEvictOperation);
6666
}
6767

6868
@Test
69-
public void caching() throws Exception {
69+
public void caching() {
7070
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "caching", 2);
7171
Iterator<CacheOperation> it = ops.iterator();
7272
assertTrue(it.next() instanceof CacheableOperation);
7373
assertTrue(it.next() instanceof CacheEvictOperation);
7474
}
7575

7676
@Test
77-
public void emptyCaching() throws Exception {
77+
public void emptyCaching() {
7878
getOps(AnnotatedClass.class, "emptyCaching", 0);
7979
}
8080

8181
@Test
82-
public void singularStereotype() throws Exception {
82+
public void singularStereotype() {
8383
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "singleStereotype", 1);
8484
assertTrue(ops.iterator().next() instanceof CacheEvictOperation);
8585
}
8686

8787
@Test
88-
public void multipleStereotypes() throws Exception {
88+
public void multipleStereotypes() {
8989
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "multipleStereotype", 3);
9090
Iterator<CacheOperation> it = ops.iterator();
9191
assertTrue(it.next() instanceof CacheableOperation);
@@ -98,7 +98,7 @@ public void multipleStereotypes() throws Exception {
9898
}
9999

100100
@Test
101-
public void singleComposedAnnotation() throws Exception {
101+
public void singleComposedAnnotation() {
102102
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "singleComposed", 2);
103103
Iterator<CacheOperation> it = ops.iterator();
104104

@@ -114,7 +114,7 @@ public void singleComposedAnnotation() throws Exception {
114114
}
115115

116116
@Test
117-
public void multipleComposedAnnotations() throws Exception {
117+
public void multipleComposedAnnotations() {
118118
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "multipleComposed", 4);
119119
Iterator<CacheOperation> it = ops.iterator();
120120

0 commit comments

Comments
 (0)