Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix expression cache

Browse Source 
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
This commit is contained in:
Stephane Nicoll
2014-04-14 21:01:01 +02:00
parent 4e1781ae8c
commit 397aa82984
6 changed files with 192 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java vendored
View File

@@ -483,12 +483,15 @@ public abstract class CacheAspectSupport implements InitializingBean, Applicatio
private final Collection<? extends Cache> 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);
}

74
spring-context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java vendored
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);
}
}
}

16
spring-context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java vendored
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();
}
}

2
spring-context/src/main/java/org/springframework/cache/interceptor/MethodCacheKey.java vendored
View File

@@ -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 {

122
spring-context/src/test/java/org/springframework/cache/config/ExpressionCachingIntegrationTests.java vendored Normal file
View File

@@ -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<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 extends CachingConfigurerSupport {
@Override
@Bean
public CacheManager cacheManager() {
return new ConcurrentMapCacheManager();
}
}
}

9
spring-context/src/test/java/org/springframework/cache/interceptor/ExpressionEvaluatorTests.java vendored
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);