diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index 6fa24bf338..1828e3febc 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -483,12 +483,15 @@ public abstract class CacheAspectSupport implements InitializingBean, Applicatio private final Collection caches; + private final MethodCacheKey methodCacheKey; + public CacheOperationContext(CacheOperationMetadata metadata, Object[] args, Object target) { this.metadata = metadata; this.args = extractArgs(metadata.method, args); this.target = target; this.caches = CacheAspectSupport.this.getCaches(this, metadata.cacheResolver); + this.methodCacheKey = new MethodCacheKey(metadata.method, metadata.targetClass); } @Override @@ -525,7 +528,7 @@ public abstract class CacheAspectSupport implements InitializingBean, Applicatio protected boolean isConditionPassing(Object result) { if (StringUtils.hasText(this.metadata.operation.getCondition())) { EvaluationContext evaluationContext = createEvaluationContext(result); - return evaluator.condition(this.metadata.operation.getCondition(), this.metadata.method, evaluationContext); + return evaluator.condition(this.metadata.operation.getCondition(), this.methodCacheKey, evaluationContext); } return true; } @@ -540,7 +543,7 @@ public abstract class CacheAspectSupport implements InitializingBean, Applicatio } if (StringUtils.hasText(unless)) { EvaluationContext evaluationContext = createEvaluationContext(value); - return !evaluator.unless(unless, this.metadata.method, evaluationContext); + return !evaluator.unless(unless, this.methodCacheKey, evaluationContext); } return true; } @@ -552,7 +555,7 @@ public abstract class CacheAspectSupport implements InitializingBean, Applicatio protected Object generateKey(Object result) { if (StringUtils.hasText(this.metadata.operation.getKey())) { EvaluationContext evaluationContext = createEvaluationContext(result); - return evaluator.key(this.metadata.operation.getKey(), this.metadata.method, evaluationContext); + return evaluator.key(this.metadata.operation.getKey(), this.methodCacheKey, evaluationContext); } return metadata.keyGenerator.generate(this.target, this.metadata.method, this.args); } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java b/spring-context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java index 9405c40346..84c9a962b0 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java +++ b/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.core.ParameterNameDiscoverer; 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. * - *

Performs internal caching for performance reasons. + *

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 keyCache = new ConcurrentHashMap(64); + private final Map keyCache + = new ConcurrentHashMap(64); - private final Map conditionCache = new ConcurrentHashMap(64); + private final Map conditionCache + = new ConcurrentHashMap(64); - private final Map unlessCache = new ConcurrentHashMap(64); + private final Map unlessCache + = new ConcurrentHashMap(64); - private final Map targetMethodCache = new ConcurrentHashMap(64); + private final Map targetMethodCache = new ConcurrentHashMap(64); /** @@ -93,22 +99,22 @@ class ExpressionEvaluator { 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 cache, String expression, Method method) { - String key = toString(method, expression); + private Expression getExpression(Map 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 @@ class ExpressionEvaluator { 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); + } + } + } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java b/spring-context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java index 631ea6e442..143d9f5f52 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java +++ b/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 @@ import org.springframework.util.ObjectUtils; * (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 methodCache; + private final Map methodCache; private boolean paramLoaded = false; LazyParamAwareEvaluationContext(Object rootObject, ParameterNameDiscoverer paramDiscoverer, Method method, - Object[] args, Class targetClass, Map methodCache) { + Object[] args, Class targetClass, Map methodCache) { super(rootObject); this.paramDiscoverer = paramDiscoverer; @@ -85,7 +86,7 @@ class LazyParamAwareEvaluationContext extends StandardEvaluationContext { 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 @@ class LazyParamAwareEvaluationContext extends StandardEvaluationContext { } } - private String toString(Method m) { - StringBuilder sb = new StringBuilder(); - sb.append(m.getDeclaringClass().getName()); - sb.append("#"); - sb.append(m.toString()); - return sb.toString(); - } } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/MethodCacheKey.java b/spring-context/src/main/java/org/springframework/cache/interceptor/MethodCacheKey.java index 9de1671211..c164c5e3bb 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/MethodCacheKey.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/MethodCacheKey.java @@ -27,7 +27,7 @@ import org.springframework.util.ObjectUtils; * * @author Costin Leau * @author Stephane Nicoll - * @since 4.1 + * @since 4.0.4 */ public final class MethodCacheKey { diff --git a/spring-context/src/test/java/org/springframework/cache/config/ExpressionCachingIntegrationTests.java b/spring-context/src/test/java/org/springframework/cache/config/ExpressionCachingIntegrationTests.java new file mode 100644 index 0000000000..3f1f5a9b46 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/cache/config/ExpressionCachingIntegrationTests.java @@ -0,0 +1,122 @@ +/* + * 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.config; + +import org.junit.Test; + +import org.springframework.cache.CacheManager; +import org.springframework.cache.annotation.CachePut; +import org.springframework.cache.annotation.CachingConfigurerSupport; +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.cache.concurrent.ConcurrentMapCacheManager; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * + * @author Stephane Nicoll + */ +public class ExpressionCachingIntegrationTests { + + @SuppressWarnings("unchecked") + @Test // SPR-11692 + public void expressionIsCacheBasedOnActualMethod() { + ConfigurableApplicationContext context = + new AnnotationConfigApplicationContext(SharedConfig.class, Spr11692Config.class); + + BaseDao userDao = (BaseDao) context.getBean("userDao"); + BaseDao orderDao = (BaseDao) context.getBean("orderDao"); + + userDao.persist(new User("1")); + orderDao.persist(new Order("2")); + + context.close(); + } + + + + @Configuration + static class Spr11692Config { + @Bean + public BaseDao userDao() { + return new UserDaoImpl(); + } + + @Bean + public BaseDao orderDao() { + return new OrderDaoImpl(); + } + } + + + private static interface BaseDao { + T persist(T t); + } + + private static class UserDaoImpl implements BaseDao { + @Override + @CachePut(value = "users", key = "#user.id") + public User persist(User user) { + return user; + } + } + + private static class OrderDaoImpl implements BaseDao { + @Override + @CachePut(value = "orders", key = "#order.id") + public Order persist(Order order) { + return order; + } + } + + private static class User { + private final String id; + + private User(String id) { + this.id = id; + } + + public String getId() { + return id; + } + } + + private static class Order { + private final String id; + + private Order(String id) { + this.id = id; + } + + public String getId() { + return id; + } + } + + @Configuration + @EnableCaching + static class SharedConfig extends CachingConfigurerSupport { + @Override + @Bean + public CacheManager cacheManager() { + return new ConcurrentMapCacheManager(); + } + } + +} diff --git a/spring-context/src/test/java/org/springframework/cache/interceptor/ExpressionEvaluatorTests.java b/spring-context/src/test/java/org/springframework/cache/interceptor/ExpressionEvaluatorTests.java index 05f0043e51..090fc7be92 100644 --- a/spring-context/src/test/java/org/springframework/cache/interceptor/ExpressionEvaluatorTests.java +++ b/spring-context/src/test/java/org/springframework/cache/interceptor/ExpressionEvaluatorTests.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. @@ -39,6 +39,7 @@ import static org.junit.Assert.*; * @author Costin Leau * @author Phillip Webb * @author Sam Brannen + * @author Stephane Nicoll */ public class ExpressionEvaluatorTests { @@ -82,8 +83,10 @@ public class ExpressionEvaluatorTests { Iterator it = ops.iterator(); - Object keyA = eval.key(it.next().getKey(), method, evalCtx); - Object keyB = eval.key(it.next().getKey(), method, evalCtx); + MethodCacheKey key = new MethodCacheKey(method, AnnotatedClass.class); + + Object keyA = eval.key(it.next().getKey(), key, evalCtx); + Object keyB = eval.key(it.next().getKey(), key, evalCtx); assertEquals(args[0], keyA); assertEquals(args[1], keyB);