Fix expression cache

Prior to this commit, only the java.lang.reflect.Method was used to
identify an annotated method. As a result, if different annotations
were placed on different methods (method overriding, interface
implementation) only the first one (cached) was used.

LazyParamAwareEvaluationContext was affected by the exact
same problem and has been also fixed.

Issue: SPR-11692
(cherry picked from commit df34bab)

Author: snicoll

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

@@ -372,6 +372,8 @@ protected class CacheOperationContext {
 
 		private final Collection<? extends Cache> caches;
 
+		private final MethodCacheKey methodCacheKey;
+
 		public CacheOperationContext(CacheOperation operation, Method method,
 				Object[] args, Object target, Class<?> targetClass) {
 			this.operation = operation;
@@ -380,6 +382,7 @@ public CacheOperationContext(CacheOperation operation, Method method,
 			this.target = target;
 			this.targetClass = targetClass;
 			this.caches = CacheAspectSupport.this.getCaches(operation);
+			this.methodCacheKey = new MethodCacheKey(method, targetClass);
 		}
 
 		private Object[] extractArgs(Method method, Object[] args) {
@@ -396,7 +399,7 @@ private Object[] extractArgs(Method method, Object[] args) {
 		protected boolean isConditionPassing(Object result) {
 			if (StringUtils.hasText(this.operation.getCondition())) {
 				EvaluationContext evaluationContext = createEvaluationContext(result);
-				return evaluator.condition(this.operation.getCondition(), this.method, evaluationContext);
+				return evaluator.condition(this.operation.getCondition(), this.methodCacheKey, evaluationContext);
 			}
 			return true;
 		}
@@ -411,7 +414,7 @@ else if (this.operation instanceof CachePutOperation) {
 			}
 			if (StringUtils.hasText(unless)) {
 				EvaluationContext evaluationContext = createEvaluationContext(value);
-				return !evaluator.unless(unless, this.method, evaluationContext);
+				return !evaluator.unless(unless, this.methodCacheKey, evaluationContext);
 			}
 			return true;
 		}
@@ -423,7 +426,7 @@ else if (this.operation instanceof CachePutOperation) {
 		protected Object generateKey(Object result) {
 			if (StringUtils.hasText(this.operation.getKey())) {
 				EvaluationContext evaluationContext = createEvaluationContext(result);
-				return evaluator.key(this.operation.getKey(), this.method, evaluationContext);
+				return evaluator.key(this.operation.getKey(), this.methodCacheKey, evaluationContext);
 			}
 			return keyGenerator.generate(this.target, this.method, this.args);
 		}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -27,16 +27,19 @@
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.Expression;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Utility class handling the SpEL expression parsing.
  * Meant to be used as a reusable, thread-safe component.
  *
- * <p>Performs internal caching for performance reasons.
+ * <p>Performs internal caching for performance reasons
+ * using {@link MethodCacheKey}.
  *
  * @author Costin Leau
  * @author Phillip Webb
  * @author Sam Brannen
+ * @author Stephane Nicoll
  * @since 3.1
  */
 class ExpressionEvaluator {
@@ -49,13 +52,16 @@ class ExpressionEvaluator {
 	// shared param discoverer since it caches data internally
 	private final ParameterNameDiscoverer paramNameDiscoverer = new DefaultParameterNameDiscoverer();
 
-	private final Map<String, Expression> keyCache = new ConcurrentHashMap<String, Expression>(64);
+	private final Map<ExpressionKey, Expression> keyCache
+			= new ConcurrentHashMap<ExpressionKey, Expression>(64);
 
-	private final Map<String, Expression> conditionCache = new ConcurrentHashMap<String, Expression>(64);
+	private final Map<ExpressionKey, Expression> conditionCache
+			= new ConcurrentHashMap<ExpressionKey, Expression>(64);
 
-	private final Map<String, Expression> unlessCache = new ConcurrentHashMap<String, Expression>(64);
+	private final Map<ExpressionKey, Expression> unlessCache
+			= new ConcurrentHashMap<ExpressionKey, Expression>(64);
 
-	private final Map<String, Method> targetMethodCache = new ConcurrentHashMap<String, Method>(64);
+	private final Map<MethodCacheKey, Method> targetMethodCache = new ConcurrentHashMap<MethodCacheKey, Method>(64);
 
 
 	/**
@@ -93,22 +99,22 @@ public EvaluationContext createEvaluationContext(Collection<? extends Cache> cac
 		return evaluationContext;
 	}
 
-	public Object key(String keyExpression, Method method, EvaluationContext evalContext) {
-		return getExpression(this.keyCache, keyExpression, method).getValue(evalContext);
+	public Object key(String keyExpression, MethodCacheKey methodKey, EvaluationContext evalContext) {
+		return getExpression(this.keyCache, keyExpression, methodKey).getValue(evalContext);
 	}
 
-	public boolean condition(String conditionExpression, Method method, EvaluationContext evalContext) {
-		return getExpression(this.conditionCache, conditionExpression, method).getValue(
+	public boolean condition(String conditionExpression, MethodCacheKey methodKey, EvaluationContext evalContext) {
+		return getExpression(this.conditionCache, conditionExpression, methodKey).getValue(
 				evalContext, boolean.class);
 	}
 
-	public boolean unless(String unlessExpression, Method method, EvaluationContext evalContext) {
-		return getExpression(this.unlessCache, unlessExpression, method).getValue(
+	public boolean unless(String unlessExpression, MethodCacheKey methodKey, EvaluationContext evalContext) {
+		return getExpression(this.unlessCache, unlessExpression, methodKey).getValue(
 				evalContext, boolean.class);
 	}
 
-	private Expression getExpression(Map<String, Expression> cache, String expression, Method method) {
-		String key = toString(method, expression);
+	private Expression getExpression(Map<ExpressionKey, Expression> cache, String expression, MethodCacheKey methodKey) {
+		ExpressionKey key = createKey(methodKey, expression);
 		Expression rtn = cache.get(key);
 		if (rtn == null) {
 			rtn = this.parser.parseExpression(expression);
@@ -117,13 +123,37 @@ private Expression getExpression(Map<String, Expression> cache, String expressio
 		return rtn;
 	}
 
-	private String toString(Method method, String expression) {
-		StringBuilder sb = new StringBuilder();
-		sb.append(method.getDeclaringClass().getName());
-		sb.append("#");
-		sb.append(method.toString());
-		sb.append("#");
-		sb.append(expression);
-		return sb.toString();
+	private ExpressionKey createKey(MethodCacheKey methodCacheKey, String expression) {
+		return new ExpressionKey(methodCacheKey, expression);
 	}
+
+
+	private static class ExpressionKey {
+		private final MethodCacheKey methodCacheKey;
+		private final String expression;
+
+		private ExpressionKey(MethodCacheKey methodCacheKey, String expression) {
+			this.methodCacheKey = methodCacheKey;
+			this.expression = expression;
+		}
+
+		@Override
+		public boolean equals(Object other) {
+			if (this == other) {
+				return true;
+			}
+			if (!(other instanceof ExpressionKey)) {
+				return false;
+			}
+			ExpressionKey otherKey = (ExpressionKey) other;
+			return (this.methodCacheKey.equals(otherKey.methodCacheKey)
+					&& ObjectUtils.nullSafeEquals(this.expression, otherKey.expression));
+		}
+
+		@Override
+		public int hashCode() {
+			return this.methodCacheKey.hashCode() * 29 + (this.expression != null ? this.expression.hashCode() : 0);
+		}
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -33,6 +33,7 @@
  * (rather then a dedicated 'closure'-like class for deferred execution).
  *
  * @author Costin Leau
+ * @author Stephane Nicoll
  * @since 3.1
  */
 class LazyParamAwareEvaluationContext extends StandardEvaluationContext {
@@ -45,13 +46,13 @@ class LazyParamAwareEvaluationContext extends StandardEvaluationContext {
 
 	private final Class<?> targetClass;
 
-	private final Map<String, Method> methodCache;
+	private final Map<MethodCacheKey, Method> methodCache;
 
 	private boolean paramLoaded = false;
 
 
 	LazyParamAwareEvaluationContext(Object rootObject, ParameterNameDiscoverer paramDiscoverer, Method method,
-			Object[] args, Class<?> targetClass, Map<String, Method> methodCache) {
+			Object[] args, Class<?> targetClass, Map<MethodCacheKey, Method> methodCache) {
 		super(rootObject);
 
 		this.paramDiscoverer = paramDiscoverer;
@@ -85,7 +86,7 @@ private void loadArgsAsVariables() {
 			return;
 		}
 
-		String methodKey = toString(this.method);
+		MethodCacheKey methodKey = new MethodCacheKey(this.method, this.targetClass);
 		Method targetMethod = this.methodCache.get(methodKey);
 		if (targetMethod == null) {
 			targetMethod = AopUtils.getMostSpecificMethod(this.method, this.targetClass);
@@ -110,11 +111,4 @@ private void loadArgsAsVariables() {
 		}
 	}
 
-	private String toString(Method m) {
-		StringBuilder sb = new StringBuilder();
-		sb.append(m.getDeclaringClass().getName());
-		sb.append("#");
-		sb.append(m.toString());
-		return sb.toString();
-	}
 }

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

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2002-2014 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cache.interceptor;
+
+import java.lang.reflect.Method;
+
+import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
+
+/**
+ * Represent a method on a particular {@link Class} and  is suitable as a key.
+ * <p>Mainly for internal use within the framework.
+ *
+ * @author Costin Leau
+ * @author Stephane Nicoll
+ * @since 4.0.4
+ */
+public final class MethodCacheKey {
+
+	private final Method method;
+
+	private final Class<?> targetClass;
+
+	public MethodCacheKey(Method method, Class<?> targetClass) {
+		Assert.notNull(method, "method must be set.");
+		this.method = method;
+		this.targetClass = targetClass;
+	}
+
+	@Override
+	public boolean equals(Object other) {
+		if (this == other) {
+			return true;
+		}
+		if (!(other instanceof MethodCacheKey)) {
+			return false;
+		}
+		MethodCacheKey otherKey = (MethodCacheKey) other;
+		return (this.method.equals(otherKey.method) && ObjectUtils.nullSafeEquals(this.targetClass,
+				otherKey.targetClass));
+	}
+
+	@Override
+	public int hashCode() {
+		return this.method.hashCode() * 29 + (this.targetClass != null ? this.targetClass.hashCode() : 0);
+	}
+
+}