Consistent cache key implementation across transaction and cache attribute sources

Includes consistent applicability of class-level metadata to user-level methods only.

Issue: SPR-14017
Issue: SPR-14095

spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.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.
@@ -26,27 +26,24 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.context.expression.AnnotatedElementKey;
 import org.springframework.core.BridgeMethodResolver;
+import org.springframework.core.MethodClassKey;
 import org.springframework.util.ClassUtils;
 
 /**
- * Abstract implementation of {@link CacheOperation} that caches
- * attributes for methods and implements a fallback policy: 1. specific
- * target method; 2. target class; 3. declaring method; 4. declaring
- * class/interface.
+ * Abstract implementation of {@link CacheOperation} that caches attributes
+ * for methods and implements a fallback policy: 1. specific target method;
+ * 2. target class; 3. declaring method; 4. declaring class/interface.
  *
  * <p>Defaults to using the target class's caching attribute if none is
- * associated with the target method. Any caching attribute associated
- * with the target method completely overrides a class caching attribute.
- * If none found on the target class, the interface that the invoked
- * method has been called through (in case of a JDK proxy) will be
- * checked.
+ * associated with the target method. Any caching attribute associated with
+ * the target method completely overrides a class caching attribute.
+ * If none found on the target class, the interface that the invoked method
+ * has been called through (in case of a JDK proxy) will be checked.
  *
- * <p>This implementation caches attributes by method after they are
- * first used. If it is ever desirable to allow dynamic changing of
- * cacheable attributes (which is very unlikely), caching could be made
- * configurable.
+ * <p>This implementation caches attributes by method after they are first
+ * used. If it is ever desirable to allow dynamic changing of cacheable
+ * attributes (which is very unlikely), caching could be made configurable.
  *
  * @author Costin Leau
  * @author Juergen Hoeller
@@ -69,7 +66,7 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 	protected final Log logger = LogFactory.getLog(getClass());
 
 	/**
-	 * Cache of CacheOperations, keyed by {@link AnnotatedElementKey}.
+	 * Cache of CacheOperations, keyed by method on a specific target class.
 	 * <p>As this base class is not marked Serializable, the cache will be recreated
 	 * after serialization - provided that the concrete subclass is Serializable.
 	 */
@@ -117,7 +114,7 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 	 * @return the cache key (never {@code null})
 	 */
 	protected Object getCacheKey(Method method, Class<?> targetClass) {
-		return new AnnotatedElementKey(method, targetClass);
+		return new MethodClassKey(method, targetClass);
 	}
 
 	private Collection<CacheOperation> computeCacheOperations(Method method, Class<?> targetClass) {
@@ -140,19 +137,23 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 
 		// Second try is the caching operation on the target class.
 		opDef = findCacheOperations(specificMethod.getDeclaringClass());
-		if (opDef != null) {
+		if (opDef != null && ClassUtils.isUserLevelMethod(method)) {
 			return opDef;
 		}
 
 		if (specificMethod != method) {
-			// Fall back is to look at the original method.
+			// Fallback is to look at the original method.
 			opDef = findCacheOperations(method);
 			if (opDef != null) {
 				return opDef;
 			}
-			// Last fall back is the class of the original method.
-			return findCacheOperations(method.getDeclaringClass());
+			// Last fallback is the class of the original method.
+			opDef = findCacheOperations(method.getDeclaringClass());
+			if (opDef != null && ClassUtils.isUserLevelMethod(method)) {
+				return opDef;
+			}
 		}
+
 		return null;
 	}
 

spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java

@@ -84,14 +84,14 @@ public abstract class CachedExpressionEvaluator {
 	}
 
 
-	protected static class ExpressionKey {
+	protected static class ExpressionKey implements Comparable<ExpressionKey> {
 
-		private final AnnotatedElementKey key;
+		private final AnnotatedElementKey element;
 
 		private final String expression;
 
-		protected ExpressionKey(AnnotatedElementKey key, String expression) {
-			this.key = key;
+		protected ExpressionKey(AnnotatedElementKey element, String expression) {
+			this.element = element;
 			this.expression = expression;
 		}
 
@@ -104,13 +104,27 @@ public abstract class CachedExpressionEvaluator {
 				return false;
 			}
 			ExpressionKey otherKey = (ExpressionKey) other;
-			return (this.key.equals(otherKey.key) &&
+			return (this.element.equals(otherKey.element) &&
 					ObjectUtils.nullSafeEquals(this.expression, otherKey.expression));
 		}
 
 		@Override
 		public int hashCode() {
-			return this.key.hashCode() + (this.expression != null ? this.expression.hashCode() * 29 : 0);
+			return this.element.hashCode() + (this.expression != null ? this.expression.hashCode() * 29 : 0);
+		}
+
+		@Override
+		public String toString() {
+			return this.element + (this.expression != null ? " with expression \"" + this.expression : "\"");
+		}
+
+		@Override
+		public int compareTo(ExpressionKey other) {
+			int result = this.element.toString().compareTo(other.element.toString());
+			if (result == 0 && this.expression != null) {
+				result = this.expression.compareTo(other.expression);
+			}
+			return result;
 		}
 	}