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)
This commit is contained in:
Stephane Nicoll
2014-04-14 21:01:01 +02:00
parent 03ae8eeb95
commit 2d8e0c8f87
6 changed files with 249 additions and 39 deletions

View File

@@ -372,6 +372,8 @@ public abstract class CacheAspectSupport implements InitializingBean {
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 abstract class CacheAspectSupport implements InitializingBean {
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 @@ public abstract class CacheAspectSupport implements InitializingBean {
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 @@ public abstract class CacheAspectSupport implements InitializingBean {
}
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 @@ public abstract class CacheAspectSupport implements InitializingBean {
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);
}

View File

@@ -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.
*
* <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 @@ 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<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 @@ 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);
}
}
}

View File

@@ -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<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 @@ 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();
}
}

View File

@@ -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);
}
}

View File

@@ -0,0 +1,118 @@
/*
* 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.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<User> userDao = (BaseDao<User>) context.getBean("userDao");
BaseDao<Order> orderDao = (BaseDao<Order>) context.getBean("orderDao");
userDao.persist(new User("1"));
orderDao.persist(new Order("2"));
context.close();
}
@Configuration
static class Spr11692Config {
@Bean
public BaseDao<User> userDao() {
return new UserDaoImpl();
}
@Bean
public BaseDao<Order> orderDao() {
return new OrderDaoImpl();
}
}
private static interface BaseDao<T> {
T persist(T t);
}
private static class UserDaoImpl implements BaseDao<User> {
@Override
@CachePut(value = "users", key = "#user.id")
public User persist(User user) {
return user;
}
}
private static class OrderDaoImpl implements BaseDao<Order> {
@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 {
@Bean
public CacheManager cacheManager() {
return new ConcurrentMapCacheManager();
}
}
}

View File

@@ -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<CacheOperation> 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);