Skip to content

Commit eaff2c2

Browse files
jhoellerjhoeller
committed
Consistent target method resolution for event and caching expressions
Issue: SPR-16779
1 parent aa11721 commit eaff2c2

File tree

6 files changed

+72
-119
lines changed

6 files changed

+72
-119
lines changed

‎spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.cache.interceptor;
1818

1919
import java.lang.reflect.Method;
20+
import java.lang.reflect.Proxy;
2021
import java.util.ArrayList;
2122
import java.util.Collection;
2223
import java.util.Collections;
@@ -30,6 +31,7 @@
3031
import org.apache.commons.logging.LogFactory;
3132

3233
import org.springframework.aop.framework.AopProxyUtils;
34+
import org.springframework.aop.support.AopUtils;
3335
import org.springframework.beans.factory.BeanFactory;
3436
import org.springframework.beans.factory.BeanFactoryAware;
3537
import org.springframework.beans.factory.InitializingBean;
@@ -40,6 +42,7 @@
4042
import org.springframework.cache.Cache;
4143
import org.springframework.cache.CacheManager;
4244
import org.springframework.context.expression.AnnotatedElementKey;
45+
import org.springframework.core.BridgeMethodResolver;
4346
import org.springframework.expression.EvaluationContext;
4447
import org.springframework.lang.Nullable;
4548
import org.springframework.util.Assert;
@@ -624,6 +627,10 @@ protected static class CacheOperationMetadata {
624627

625628
private final Class<?> targetClass;
626629

630+
private final Method targetMethod;
631+
632+
private final AnnotatedElementKey methodKey;
633+
627634
private final KeyGenerator keyGenerator;
628635

629636
private final CacheResolver cacheResolver;
@@ -632,8 +639,11 @@ public CacheOperationMetadata(CacheOperation operation, Method method, Class<?>
632639
KeyGenerator keyGenerator, CacheResolver cacheResolver) {
633640

634641
this.operation = operation;
635-
this.method = method;
642+
this.method = BridgeMethodResolver.findBridgedMethod(method);
636643
this.targetClass = targetClass;
644+
this.targetMethod = (!Proxy.isProxyClass(targetClass) ?
645+
AopUtils.getMostSpecificMethod(method, targetClass) : this.method);
646+
this.methodKey = new AnnotatedElementKey(this.targetMethod, targetClass);
637647
this.keyGenerator = keyGenerator;
638648
this.cacheResolver = cacheResolver;
639649
}
@@ -652,15 +662,12 @@ protected class CacheOperationContext implements CacheOperationInvocationContext
652662

653663
private final Collection<String> cacheNames;
654664

655-
private final AnnotatedElementKey methodCacheKey;
656-
657665
public CacheOperationContext(CacheOperationMetadata metadata, Object[] args, Object target) {
658666
this.metadata = metadata;
659667
this.args = extractArgs(metadata.method, args);
660668
this.target = target;
661669
this.caches = CacheAspectSupport.this.getCaches(this, metadata.cacheResolver);
662670
this.cacheNames = createCacheNames(this.caches);
663-
this.methodCacheKey = new AnnotatedElementKey(metadata.method, metadata.targetClass);
664671
}
665672

666673
@Override
@@ -698,7 +705,7 @@ protected boolean isConditionPassing(@Nullable Object result) {
698705
if (StringUtils.hasText(this.metadata.operation.getCondition())) {
699706
EvaluationContext evaluationContext = createEvaluationContext(result);
700707
return evaluator.condition(this.metadata.operation.getCondition(),
701-
this.methodCacheKey, evaluationContext);
708+
this.metadata.methodKey, evaluationContext);
702709
}
703710
return true;
704711
}
@@ -713,7 +720,7 @@ else if (this.metadata.operation instanceof CachePutOperation) {
713720
}
714721
if (StringUtils.hasText(unless)) {
715722
EvaluationContext evaluationContext = createEvaluationContext(value);
716-
return !evaluator.unless(unless, this.methodCacheKey, evaluationContext);
723+
return !evaluator.unless(unless, this.metadata.methodKey, evaluationContext);
717724
}
718725
return true;
719726
}
@@ -725,14 +732,14 @@ else if (this.metadata.operation instanceof CachePutOperation) {
725732
protected Object generateKey(@Nullable Object result) {
726733
if (StringUtils.hasText(this.metadata.operation.getKey())) {
727734
EvaluationContext evaluationContext = createEvaluationContext(result);
728-
return evaluator.key(this.metadata.operation.getKey(), this.methodCacheKey, evaluationContext);
735+
return evaluator.key(this.metadata.operation.getKey(), this.metadata.methodKey, evaluationContext);
729736
}
730737
return this.metadata.keyGenerator.generate(this.target, this.metadata.method, this.args);
731738
}
732739

733740
private EvaluationContext createEvaluationContext(@Nullable Object result) {
734741
return evaluator.createEvaluationContext(this.caches, this.metadata.method, this.args,
735-
this.target, this.metadata.targetClass, result, beanFactory);
742+
this.target, this.metadata.targetClass, this.metadata.targetMethod, result, beanFactory);
736743
}
737744

738745
protected Collection<? extends Cache> getCaches() {

‎spring-context/src/main/java/org/springframework/cache/interceptor/CacheExpressionRootObject.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 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.
@@ -20,7 +20,6 @@
2020
import java.util.Collection;
2121

2222
import org.springframework.cache.Cache;
23-
import org.springframework.util.Assert;
2423

2524
/**
2625
* Class describing the root object used during the expression evaluation.
@@ -45,8 +44,6 @@ class CacheExpressionRootObject {
4544
public CacheExpressionRootObject(
4645
Collection<? extends Cache> caches, Method method, Object[] args, Object target, Class<?> targetClass) {
4746

48-
Assert.notNull(method, "Method is required");
49-
Assert.notNull(targetClass, "targetClass is required");
5047
this.method = method;
5148
this.target = target;
5249
this.targetClass = targetClass;

‎spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationExpressionEvaluator.java

Lines changed: 3 additions & 29 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.
@@ -21,7 +21,6 @@
2121
import java.util.Map;
2222
import java.util.concurrent.ConcurrentHashMap;
2323

24-
import org.springframework.aop.support.AopUtils;
2524
import org.springframework.beans.factory.BeanFactory;
2625
import org.springframework.cache.Cache;
2726
import org.springframework.context.expression.AnnotatedElementKey;
@@ -68,18 +67,6 @@ class CacheOperationExpressionEvaluator extends CachedExpressionEvaluator {
6867

6968
private final Map<ExpressionKey, Expression> unlessCache = new ConcurrentHashMap<>(64);
7069

71-
private final Map<AnnotatedElementKey, Method> targetMethodCache = new ConcurrentHashMap<>(64);
72-
73-
74-
/**
75-
* Create an {@link EvaluationContext} without a return value.
76-
* @see #createEvaluationContext(Collection, Method, Object[], Object, Class, Object, BeanFactory)
77-
*/
78-
public EvaluationContext createEvaluationContext(Collection<? extends Cache> caches,
79-
Method method, Object[] args, Object target, Class<?> targetClass, BeanFactory beanFactory) {
80-
81-
return createEvaluationContext(caches, method, args, target, targetClass, NO_RESULT, beanFactory);
82-
}
8370

8471
/**
8572
* Create an {@link EvaluationContext}.
@@ -93,12 +80,11 @@ public EvaluationContext createEvaluationContext(Collection<? extends Cache> cac
9380
* @return the evaluation context
9481
*/
9582
public EvaluationContext createEvaluationContext(Collection<? extends Cache> caches,
96-
Method method, Object[] args, Object target, Class<?> targetClass, @Nullable Object result,
97-
@Nullable BeanFactory beanFactory) {
83+
Method method, Object[] args, Object target, Class<?> targetClass, Method targetMethod,
84+
@Nullable Object result, @Nullable BeanFactory beanFactory) {
9885

9986
CacheExpressionRootObject rootObject = new CacheExpressionRootObject(
10087
caches, method, args, target, targetClass);
101-
Method targetMethod = getTargetMethod(targetClass, method);
10288
CacheEvaluationContext evaluationContext = new CacheEvaluationContext(
10389
rootObject, targetMethod, args, getParameterNameDiscoverer());
10490
if (result == RESULT_UNAVAILABLE) {
@@ -135,18 +121,6 @@ void clear() {
135121
this.keyCache.clear();
136122
this.conditionCache.clear();
137123
this.unlessCache.clear();
138-
this.targetMethodCache.clear();
139124
}
140125

141-
private Method getTargetMethod(Class<?> targetClass, Method method) {
142-
AnnotatedElementKey methodKey = new AnnotatedElementKey(method, targetClass);
143-
Method targetMethod = this.targetMethodCache.get(methodKey);
144-
if (targetMethod == null) {
145-
targetMethod = AopUtils.getMostSpecificMethod(method, targetClass);
146-
this.targetMethodCache.put(methodKey, targetMethod);
147-
}
148-
return targetMethod;
149-
}
150-
151-
152126
}

‎spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.lang.reflect.InvocationTargetException;
2020
import java.lang.reflect.Method;
21+
import java.lang.reflect.Proxy;
2122
import java.lang.reflect.UndeclaredThrowableException;
2223
import java.util.ArrayList;
2324
import java.util.Collection;
@@ -27,6 +28,7 @@
2728
import org.apache.commons.logging.Log;
2829
import org.apache.commons.logging.LogFactory;
2930

31+
import org.springframework.aop.support.AopUtils;
3032
import org.springframework.context.ApplicationContext;
3133
import org.springframework.context.ApplicationEvent;
3234
import org.springframework.context.PayloadApplicationEvent;
@@ -35,10 +37,8 @@
3537
import org.springframework.core.ResolvableType;
3638
import org.springframework.core.annotation.AnnotatedElementUtils;
3739
import org.springframework.core.annotation.Order;
38-
import org.springframework.expression.EvaluationContext;
3940
import org.springframework.lang.Nullable;
4041
import org.springframework.util.Assert;
41-
import org.springframework.util.ClassUtils;
4242
import org.springframework.util.ObjectUtils;
4343
import org.springframework.util.ReflectionUtils;
4444
import org.springframework.util.StringUtils;
@@ -66,9 +66,9 @@ public class ApplicationListenerMethodAdapter implements GenericApplicationListe
6666

6767
private final Method method;
6868

69-
private final Class<?> targetClass;
69+
private final Method targetMethod;
7070

71-
private final Method bridgedMethod;
71+
private final AnnotatedElementKey methodKey;
7272

7373
private final List<ResolvableType> declaredEventTypes;
7474

@@ -77,8 +77,6 @@ public class ApplicationListenerMethodAdapter implements GenericApplicationListe
7777

7878
private final int order;
7979

80-
private final AnnotatedElementKey methodKey;
81-
8280
@Nullable
8381
private ApplicationContext applicationContext;
8482

@@ -88,18 +86,15 @@ public class ApplicationListenerMethodAdapter implements GenericApplicationListe
8886

8987
public ApplicationListenerMethodAdapter(String beanName, Class<?> targetClass, Method method) {
9088
this.beanName = beanName;
91-
this.method = method;
92-
this.targetClass = targetClass;
93-
this.bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
94-
95-
Method targetMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
96-
EventListener ann = AnnotatedElementUtils.findMergedAnnotation(targetMethod, EventListener.class);
89+
this.method = BridgeMethodResolver.findBridgedMethod(method);
90+
this.targetMethod = (!Proxy.isProxyClass(targetClass) ?
91+
AopUtils.getMostSpecificMethod(method, targetClass) : this.method);
92+
this.methodKey = new AnnotatedElementKey(this.targetMethod, targetClass);
9793

94+
EventListener ann = AnnotatedElementUtils.findMergedAnnotation(this.targetMethod, EventListener.class);
9895
this.declaredEventTypes = resolveDeclaredEventTypes(method, ann);
9996
this.condition = (ann != null ? ann.condition() : null);
10097
this.order = resolveOrder(method);
101-
102-
this.methodKey = new AnnotatedElementKey(method, targetClass);
10398
}
10499

105100

@@ -109,20 +104,23 @@ private List<ResolvableType> resolveDeclaredEventTypes(Method method, @Nullable
109104
throw new IllegalStateException(
110105
"Maximum one parameter is allowed for event listener method: " + method);
111106
}
112-
if (ann != null && ann.classes().length > 0) {
113-
List<ResolvableType> types = new ArrayList<>(ann.classes().length);
114-
for (Class<?> eventType : ann.classes()) {
115-
types.add(ResolvableType.forClass(eventType));
107+
108+
if (ann != null) {
109+
Class<?>[] classes = ann.classes();
110+
if (classes.length > 0) {
111+
List<ResolvableType> types = new ArrayList<>(classes.length);
112+
for (Class<?> eventType : classes) {
113+
types.add(ResolvableType.forClass(eventType));
114+
}
115+
return types;
116116
}
117-
return types;
118117
}
119-
else {
120-
if (count == 0) {
121-
throw new IllegalStateException(
122-
"Event parameter is mandatory for event listener method: " + method);
123-
}
124-
return Collections.singletonList(ResolvableType.forMethodParameter(method, 0));
118+
119+
if (count == 0) {
120+
throw new IllegalStateException(
121+
"Event parameter is mandatory for event listener method: " + method);
125122
}
123+
return Collections.singletonList(ResolvableType.forMethodParameter(method, 0));
126124
}
127125

128126
private int resolveOrder(Method method) {
@@ -245,10 +243,9 @@ private boolean shouldHandle(ApplicationEvent event, @Nullable Object[] args) {
245243
}
246244
String condition = getCondition();
247245
if (StringUtils.hasText(condition)) {
248-
Assert.notNull(this.evaluator, "EventExpressionEvaluator must no be null");
249-
EvaluationContext evaluationContext = this.evaluator.createEvaluationContext(
250-
event, this.targetClass, this.method, args, this.applicationContext);
251-
return this.evaluator.condition(condition, this.methodKey, evaluationContext);
246+
Assert.notNull(this.evaluator, "EventExpressionEvaluator must not be null");
247+
return this.evaluator.condition(
248+
condition, event, this.targetMethod, this.methodKey, args, this.applicationContext);
252249
}
253250
return true;
254251
}
@@ -259,12 +256,12 @@ private boolean shouldHandle(ApplicationEvent event, @Nullable Object[] args) {
259256
@Nullable
260257
protected Object doInvoke(Object... args) {
261258
Object bean = getTargetBean();
262-
ReflectionUtils.makeAccessible(this.bridgedMethod);
259+
ReflectionUtils.makeAccessible(this.method);
263260
try {
264-
return this.bridgedMethod.invoke(bean, args);
261+
return this.method.invoke(bean, args);
265262
}
266263
catch (IllegalArgumentException ex) {
267-
assertTargetBean(this.bridgedMethod, bean, args);
264+
assertTargetBean(this.method, bean, args);
268265
throw new IllegalStateException(getInvocationErrorMessage(bean, ex.getMessage(), args), ex);
269266
}
270267
catch (IllegalAccessException ex) {
@@ -311,7 +308,7 @@ protected String getDetailedErrorMessage(Object bean, String message) {
311308
StringBuilder sb = new StringBuilder(message).append("\n");
312309
sb.append("HandlerMethod details: \n");
313310
sb.append("Bean [").append(bean.getClass().getName()).append("]\n");
314-
sb.append("Method [").append(this.bridgedMethod.toGenericString()).append("]\n");
311+
sb.append("Method [").append(this.method.toGenericString()).append("]\n");
315312
return sb.toString();
316313
}
317314

0 commit comments

Comments
 (0)