From 14bf6509ec7d3da279d0ffb9ca38884421c335cd Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 30 Mar 2016 14:13:04 +0200 Subject: [PATCH] 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 --- ...AbstractFallbackJCacheOperationSource.java | 10 +-- .../AbstractFallbackCacheOperationSource.java | 43 +++++----- .../expression/CachedExpressionEvaluator.java | 26 ++++-- .../springframework/core/MethodClassKey.java | 85 +++++++++++++++++++ ...actFallbackTransactionAttributeSource.java | 57 +------------ 5 files changed, 135 insertions(+), 86 deletions(-) create mode 100644 spring-core/src/main/java/org/springframework/core/MethodClassKey.java diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java index 3f18c3c878..73313d22c5 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.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. @@ -24,8 +24,8 @@ 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; /** @@ -51,12 +51,12 @@ public abstract class AbstractFallbackJCacheOperationSource implements JCacheOpe protected final Log logger = LogFactory.getLog(getClass()); - private final Map cache = new ConcurrentHashMap(1024); + private final Map cache = new ConcurrentHashMap(1024); @Override public JCacheOperation getCacheOperation(Method method, Class targetClass) { - Object cacheKey = new AnnotatedElementKey(method, targetClass); + MethodClassKey cacheKey = new MethodClassKey(method, targetClass); Object cached = this.cache.get(cacheKey); if (cached != null) { @@ -95,7 +95,7 @@ public abstract class AbstractFallbackJCacheOperationSource implements JCacheOpe return operation; } if (specificMethod != method) { - // Fall back is to look at the original method. + // Fallback is to look at the original method. operation = findCacheOperation(method, targetClass); if (operation != null) { return operation; diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java b/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java index d383b28111..88d0aec5f7 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java +++ b/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. * *

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. * - *

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. + *

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. *

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 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; } diff --git a/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java b/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java index 4bb20b2894..6ff1c89876 100644 --- a/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java +++ b/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 { - 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; } } diff --git a/spring-core/src/main/java/org/springframework/core/MethodClassKey.java b/spring-core/src/main/java/org/springframework/core/MethodClassKey.java new file mode 100644 index 0000000000..b837fc6486 --- /dev/null +++ b/spring-core/src/main/java/org/springframework/core/MethodClassKey.java @@ -0,0 +1,85 @@ +/* + * 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. + * 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.core; + +import java.lang.reflect.Method; + +import org.springframework.util.ObjectUtils; + +/** + * A common key class for a method against a specific target class, + * including {@link #toString()} representation and {@link Comparable} + * support (as suggested for custom {@code HashMap} keys as of Java 8). + * + * @author Juergen Hoeller + * @since 4.3 + */ +public final class MethodClassKey implements Comparable { + + private final Method method; + + private final Class targetClass; + + + /** + * Create a key object for the given method and target class. + * @param method the method to wrap (must not be {@code null}) + * @param targetClass the target class that the method will be invoked + * on (may be {@code null} if identical to the declaring class) + */ + public MethodClassKey(Method method, Class targetClass) { + this.method = method; + this.targetClass = targetClass; + } + + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof MethodClassKey)) { + return false; + } + MethodClassKey otherKey = (MethodClassKey) other; + return (this.method.equals(otherKey.method) && + ObjectUtils.nullSafeEquals(this.targetClass, otherKey.targetClass)); + } + + @Override + public int hashCode() { + return this.method.hashCode() + (this.targetClass != null ? this.targetClass.hashCode() * 29 : 0); + } + + @Override + public String toString() { + return this.method + (this.targetClass != null ? " on " + this.targetClass : ""); + } + + @Override + public int compareTo(MethodClassKey other) { + int result = this.method.getName().compareTo(other.method.getName()); + if (result == 0) { + result = this.method.toString().compareTo(other.method.toString()); + if (result == 0 && this.targetClass != null) { + result = this.targetClass.getName().compareTo(other.targetClass.getName()); + } + } + return result; + } + +} diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java index e96dcd3644..206f3c8240 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java @@ -25,8 +25,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.core.BridgeMethodResolver; +import org.springframework.core.MethodClassKey; import org.springframework.util.ClassUtils; -import org.springframework.util.ObjectUtils; /** * Abstract implementation of {@link TransactionAttributeSource} that caches @@ -65,7 +65,7 @@ public abstract class AbstractFallbackTransactionAttributeSource implements Tran protected final Log logger = LogFactory.getLog(getClass()); /** - * Cache of TransactionAttributes, keyed by DefaultCacheKey (Method + target Class). + * Cache of TransactionAttributes, keyed by method on a specific target class. *

As this base class is not marked Serializable, the cache will be recreated * after serialization - provided that the concrete subclass is Serializable. */ @@ -123,7 +123,7 @@ public abstract class AbstractFallbackTransactionAttributeSource implements Tran * @return the cache key (never {@code null}) */ protected Object getCacheKey(Method method, Class targetClass) { - return new DefaultCacheKey(method, targetClass); + return new MethodClassKey(method, targetClass); } /** @@ -203,55 +203,4 @@ public abstract class AbstractFallbackTransactionAttributeSource implements Tran return false; } - - /** - * Default cache key for the TransactionAttribute cache. - */ - private static final class DefaultCacheKey implements Comparable { - - private final Method method; - - private final Class targetClass; - - public DefaultCacheKey(Method method, Class targetClass) { - this.method = method; - this.targetClass = targetClass; - } - - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (!(other instanceof DefaultCacheKey)) { - return false; - } - DefaultCacheKey otherKey = (DefaultCacheKey) other; - return (this.method.equals(otherKey.method) && - ObjectUtils.nullSafeEquals(this.targetClass, otherKey.targetClass)); - } - - @Override - public int hashCode() { - return this.method.hashCode() + (this.targetClass != null ? this.targetClass.hashCode() * 29 : 0); - } - - @Override - public String toString() { - return this.method + (this.targetClass != null ? " on " + this.targetClass : ""); - } - - @Override - public int compareTo(DefaultCacheKey other) { - int result = this.method.getName().compareTo(other.method.getName()); - if (result == 0) { - result = this.method.toString().compareTo(other.method.toString()); - if (result == 0 && this.targetClass != null) { - result = this.targetClass.getName().compareTo(other.targetClass.getName()); - } - } - return result; - } - } - }