Cache key classes implement Comparable and consistently provide a toString representation

Issue: SPR-14017

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.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.
@@ -586,7 +586,7 @@ public String toString() {
 	 * Simple wrapper class around a Method. Used as the key when
 	 * caching methods, for efficient equals and hashCode comparisons.
 	 */
-	private static class MethodCacheKey {
+	private static final class MethodCacheKey implements Comparable<MethodCacheKey> {
 
 		private final Method method;
 
@@ -599,17 +599,28 @@ public MethodCacheKey(Method method) {
 
 		@Override
 		public boolean equals(Object other) {
-			if (other == this) {
-				return true;
-			}
-			MethodCacheKey otherKey = (MethodCacheKey) other;
-			return (this.method == otherKey.method);
+			return (this == other || (other instanceof MethodCacheKey &&
+					this.method == ((MethodCacheKey) other).method));
 		}
 
 		@Override
 		public int hashCode() {
 			return this.hashCode;
 		}
+
+		@Override
+		public String toString() {
+			return this.method.toString();
+		}
+
+		@Override
+		public int compareTo(MethodCacheKey other) {
+			int result = this.method.getName().compareTo(other.method.getName());
+			if (result == 0) {
+				result = this.method.toString().compareTo(other.method.toString());
+			}
+			return result;
+		}
 	}
 
 }

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

@@ -101,6 +101,7 @@ private <T extends Annotation> Collection<CacheOperation> lazyInit(Collection<Ca
 	CacheableOperation parseCacheableAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, Cacheable cacheable) {
 		CacheableOperation.Builder builder = new CacheableOperation.Builder();
 
+		builder.setName(ae.toString());
 		builder.setCacheNames(cacheable.cacheNames());
 		builder.setCondition(cacheable.condition());
 		builder.setUnless(cacheable.unless());
@@ -109,7 +110,6 @@ CacheableOperation parseCacheableAnnotation(AnnotatedElement ae, DefaultCacheCon
 		builder.setCacheManager(cacheable.cacheManager());
 		builder.setCacheResolver(cacheable.cacheResolver());
 		builder.setSync(cacheable.sync());
-		builder.setName(ae.toString());
 
 		defaultConfig.applyDefault(builder);
 		CacheableOperation op = builder.build();
@@ -121,6 +121,7 @@ CacheableOperation parseCacheableAnnotation(AnnotatedElement ae, DefaultCacheCon
 	CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, CacheEvict cacheEvict) {
 		CacheEvictOperation.Builder builder = new CacheEvictOperation.Builder();
 
+		builder.setName(ae.toString());
 		builder.setCacheNames(cacheEvict.cacheNames());
 		builder.setCondition(cacheEvict.condition());
 		builder.setKey(cacheEvict.key());
@@ -129,7 +130,6 @@ CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, DefaultCacheConfig
 		builder.setCacheResolver(cacheEvict.cacheResolver());
 		builder.setCacheWide(cacheEvict.allEntries());
 		builder.setBeforeInvocation(cacheEvict.beforeInvocation());
-		builder.setName(ae.toString());
 
 		defaultConfig.applyDefault(builder);
 		CacheEvictOperation op = builder.build();
@@ -141,14 +141,14 @@ CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, DefaultCacheConfig
 	CacheOperation parsePutAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, CachePut cachePut) {
 		CachePutOperation.Builder builder = new CachePutOperation.Builder();
 
+		builder.setName(ae.toString());
 		builder.setCacheNames(cachePut.cacheNames());
 		builder.setCondition(cachePut.condition());
 		builder.setUnless(cachePut.unless());
 		builder.setKey(cachePut.key());
 		builder.setKeyGenerator(cachePut.keyGenerator());
 		builder.setCacheManager(cachePut.cacheManager());
 		builder.setCacheResolver(cachePut.cacheResolver());
-		builder.setName(ae.toString());
 
 		defaultConfig.applyDefault(builder);
 		CachePutOperation op = builder.build();

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

@@ -746,7 +746,7 @@ public void apply(Object result) {
 	}
 
 
-	private static class CacheOperationCacheKey {
+	private static final class CacheOperationCacheKey implements Comparable<CacheOperationCacheKey> {
 
 		private final CacheOperation cacheOperation;
 
@@ -774,6 +774,20 @@ public boolean equals(Object other) {
 		public int hashCode() {
 			return (this.cacheOperation.hashCode() * 31 + this.methodCacheKey.hashCode());
 		}
+
+		@Override
+		public String toString() {
+			return this.cacheOperation + " on " + this.methodCacheKey;
+		}
+
+		@Override
+		public int compareTo(CacheOperationCacheKey other) {
+			int result = this.cacheOperation.getName().compareTo(other.cacheOperation.getName());
+			if (result == 0) {
+				result = this.methodCacheKey.compareTo(other.methodCacheKey);
+			}
+			return result;
+		}
 	}
 
 }

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

@@ -29,6 +29,13 @@ public class CacheEvictOperation extends CacheOperation {
 
 	private final boolean beforeInvocation;
 
+
+	public CacheEvictOperation(CacheEvictOperation.Builder b) {
+		super(b);
+		this.cacheWide = b.cacheWide;
+		this.beforeInvocation = b.beforeInvocation;
+	}
+
 	public boolean isCacheWide() {
 		return this.cacheWide;
 	}
@@ -37,12 +44,10 @@ public boolean isBeforeInvocation() {
 		return this.beforeInvocation;
 	}
 
-	public CacheEvictOperation(CacheEvictOperation.Builder b) {
-		super(b);
-		this.cacheWide = b.cacheWide;
-		this.beforeInvocation = b.beforeInvocation;
-	}
 
+	/**
+	 * @since 4.3
+	 */
 	public static class Builder extends CacheOperation.Builder {
 
 		private boolean cacheWide = false;
@@ -71,4 +76,5 @@ public CacheEvictOperation build() {
 			return new CacheEvictOperation(this);
 		}
 	}
+
 }

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

@@ -48,6 +48,7 @@ public abstract class CacheOperation implements BasicOperation {
 
 	private final String toString;
 
+
 	protected CacheOperation(Builder b) {
 		this.name = b.name;
 		this.cacheNames = b.cacheNames;
@@ -59,11 +60,11 @@ protected CacheOperation(Builder b) {
 		this.toString = b.getOperationDescription().toString();
 	}
 
+
 	public String getName() {
 		return this.name;
 	}
 
-
 	@Override
 	public Set<String> getCacheNames() {
 		return this.cacheNames;
@@ -96,7 +97,6 @@ public String getCondition() {
 
 	/**
 	 * This implementation compares the {@code toString()} results.
-	 *
 	 * @see #toString()
 	 */
 	@Override
@@ -106,7 +106,6 @@ public boolean equals(Object other) {
 
 	/**
 	 * This implementation returns {@code toString()}'s hash code.
-	 *
 	 * @see #toString()
 	 */
 	@Override
@@ -118,14 +117,17 @@ public int hashCode() {
 	 * Return an identifying description for this cache operation.
 	 * <p>Returned value is produced by calling {@link Builder#getOperationDescription()}
 	 * during object construction. This method is used in {#hashCode} and {#equals}.
-	 *
 	 * @see Builder#getOperationDescription()
 	 */
 	@Override
 	public final String toString() {
 		return this.toString;
 	}
 
+
+	/**
+	 * @since 4.3
+	 */
 	public abstract static class Builder {
 
 		private String name = "";

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

@@ -28,6 +28,7 @@ public class CachePutOperation extends CacheOperation {
 
 	private final String unless;
 
+
 	public CachePutOperation(CachePutOperation.Builder b) {
 		super(b);
 		this.unless = b.unless;
@@ -37,6 +38,10 @@ public String getUnless() {
 		return this.unless;
 	}
 
+
+	/**
+	 * @since 4.3
+	 */
 	public static class Builder extends CacheOperation.Builder {
 
 		private String unless;
@@ -58,4 +63,5 @@ public CachePutOperation build() {
 			return new CachePutOperation(this);
 		}
 	}
+
 }

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

@@ -30,6 +30,7 @@ public class CacheableOperation extends CacheOperation {
 
 	private boolean sync;
 
+
 	public CacheableOperation(CacheableOperation.Builder b) {
 		super(b);
 		this.unless = b.unless;
@@ -76,4 +77,5 @@ public CacheableOperation build() {
 			return new CacheableOperation(this);
 		}
 	}
+
 }

spring-context/src/main/java/org/springframework/context/event/AbstractApplicationEventMulticaster.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.
@@ -286,7 +286,7 @@ protected boolean supportsEvent(ApplicationListener<?> listener, ResolvableType
 	/**
 	 * Cache key for ListenerRetrievers, based on event type and source type.
 	 */
-	private static class ListenerCacheKey {
+	private static final class ListenerCacheKey implements Comparable<ListenerCacheKey> {
 
 		private final ResolvableType eventType;
 
@@ -311,6 +311,23 @@ public boolean equals(Object other) {
 		public int hashCode() {
 			return (ObjectUtils.nullSafeHashCode(this.eventType) * 29 + ObjectUtils.nullSafeHashCode(this.sourceType));
 		}
+
+		@Override
+		public String toString() {
+			return "ListenerCacheKey [eventType = " + this.eventType + ", sourceType = " + this.sourceType.getName() + "]";
+		}
+
+		@Override
+		public int compareTo(ListenerCacheKey other) {
+			int result = 0;
+			if (this.eventType != null) {
+				result = this.eventType.toString().compareTo(other.eventType.toString());
+			}
+			if (result == 0 && this.sourceType != null) {
+				result = this.sourceType.getName().compareTo(other.sourceType.getName());
+			}
+			return result;
+		}
 	}
 
 

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

@@ -30,7 +30,7 @@
  * @since 4.2
  * @see CachedExpressionEvaluator
  */
-public final class AnnotatedElementKey {
+public final class AnnotatedElementKey implements Comparable<AnnotatedElementKey> {
 
 	private final AnnotatedElement element;
 
@@ -66,4 +66,18 @@ public int hashCode() {
 		return this.element.hashCode() + (this.targetClass != null ? this.targetClass.hashCode() * 29 : 0);
 	}
 
+	@Override
+	public String toString() {
+		return this.element + (this.targetClass != null ? " on " + this.targetClass : "");
+	}
+
+	@Override
+	public int compareTo(AnnotatedElementKey other) {
+		int result = this.element.toString().compareTo(other.element.toString());
+		if (result == 0 && this.targetClass != null) {
+			result = this.targetClass.getName().compareTo(other.targetClass.getName());
+		}
+		return result;
+	}
+
 }

spring-context/src/main/java/org/springframework/jmx/access/MBeanClientInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -63,6 +63,7 @@
 import org.springframework.jmx.support.ObjectNameManager;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ReflectionUtils;
+import org.springframework.util.StringUtils;
 
 /**
  * {@link org.aopalliance.intercept.MethodInterceptor} that routes calls to an
@@ -609,7 +610,7 @@ public void destroy() {
 	 * Simple wrapper class around a method name and its signature.
 	 * Used as the key when caching methods.
 	 */
-	private static class MethodCacheKey {
+	private static final class MethodCacheKey implements Comparable<MethodCacheKey> {
 
 		private final String name;
 
@@ -628,7 +629,7 @@ public MethodCacheKey(String name, Class<?>[] parameterTypes) {
 
 		@Override
 		public boolean equals(Object other) {
-			if (other == this) {
+			if (this == other) {
 				return true;
 			}
 			MethodCacheKey otherKey = (MethodCacheKey) other;
@@ -639,6 +640,32 @@ public boolean equals(Object other) {
 		public int hashCode() {
 			return this.name.hashCode();
 		}
+
+		@Override
+		public String toString() {
+			return this.name + "(" + StringUtils.arrayToCommaDelimitedString(this.parameterTypes) + ")";
+		}
+
+		@Override
+		public int compareTo(MethodCacheKey other) {
+			int result = this.name.compareTo(other.name);
+			if (result != 0) {
+				return result;
+			}
+			if (this.parameterTypes.length < other.parameterTypes.length) {
+				return -1;
+			}
+			if (this.parameterTypes.length > other.parameterTypes.length) {
+				return 1;
+			}
+			for (int i = 0; i < this.parameterTypes.length; i++) {
+				result = this.parameterTypes[i].getName().compareTo(other.parameterTypes[i].getName());
+				if (result != 0) {
+					return result;
+				}
+			}
+			return 0;
+		}
 	}
 
 }