From 0a611aa77649ad6859a0b8dbb66048ae72670998 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Wed, 30 Nov 2011 15:21:09 +0000 Subject: [PATCH] SPR-8082 + fix internal cache causing the multiple annotations key/conditions to overlap --- .../cache/interceptor/CacheAspectSupport.java | 14 +++++----- .../cache/interceptor/CacheOperation.java | 3 ++- .../interceptor/ExpressionEvaluator.java | 27 +++++++++++++------ .../LazyParamAwareEvaluationContext.java | 18 +++++++++---- .../cache/config/AbstractAnnotationTests.java | 4 +++ .../AnnotatedClassCacheableService.java | 2 +- .../cache/config/DefaultCacheableService.java | 2 +- .../cache/config/cache-advice.xml | 2 ++ 8 files changed, 48 insertions(+), 24 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index b833f80af5..ea0cc2b4ed 100644 --- a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -225,14 +225,14 @@ public abstract class CacheAspectSupport implements InitializingBean { } private void inspectBeforeCacheEvicts(Collection evictions) { - inspectAfterCacheEvicts(evictions, false); + inspectCacheEvicts(evictions, false); } private void inspectAfterCacheEvicts(Collection evictions) { - inspectAfterCacheEvicts(evictions, true); + inspectCacheEvicts(evictions, true); } - private void inspectAfterCacheEvicts(Collection evictions, boolean afterInvocation) { + private void inspectCacheEvicts(Collection evictions, boolean afterInvocation) { if (!evictions.isEmpty()) { @@ -276,8 +276,7 @@ public abstract class CacheAspectSupport implements InitializingBean { } private CacheStatus inspectCacheables(Collection cacheables) { - Map cUpdates = new LinkedHashMap( - cacheables.size()); + Map cUpdates = new LinkedHashMap(cacheables.size()); boolean updateRequire = false; Object retVal = null; @@ -439,8 +438,7 @@ public abstract class CacheAspectSupport implements InitializingBean { // context passed around to avoid multiple creations private final EvaluationContext evalContext; - public CacheOperationContext(CacheOperation operation, Method method, Object[] args, Object target, - Class targetClass) { + public CacheOperationContext(CacheOperation operation, Method method, Object[] args, Object target, Class targetClass) { this.operation = operation; this.caches = CacheAspectSupport.this.getCaches(operation); this.target = target; @@ -472,4 +470,4 @@ public abstract class CacheAspectSupport implements InitializingBean { return this.caches; } } -} +} \ No newline at end of file diff --git a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java index 13da45282e..cd8a40ceb2 100644 --- a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java +++ b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java @@ -114,7 +114,8 @@ public abstract class CacheOperation { */ protected StringBuilder getOperationDescription() { StringBuilder result = new StringBuilder(); - result.append("CacheOperation["); + result.append(getClass().getSimpleName()); + result.append("["); result.append(this.name); result.append("] caches="); result.append(this.cacheNames); diff --git a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java index 4db94bb1b0..3e989408d4 100644 --- a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java +++ b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/ExpressionEvaluator.java @@ -44,11 +44,11 @@ class ExpressionEvaluator { // shared param discoverer since it caches data internally private ParameterNameDiscoverer paramNameDiscoverer = new LocalVariableTableParameterNameDiscoverer(); - private Map conditionCache = new ConcurrentHashMap(); + private Map conditionCache = new ConcurrentHashMap(); - private Map keyCache = new ConcurrentHashMap(); + private Map keyCache = new ConcurrentHashMap(); - private Map targetMethodCache = new ConcurrentHashMap(); + private Map targetMethodCache = new ConcurrentHashMap(); public EvaluationContext createEvaluationContext( @@ -61,21 +61,32 @@ class ExpressionEvaluator { } public boolean condition(String conditionExpression, Method method, EvaluationContext evalContext) { - Expression condExp = this.conditionCache.get(method); + String key = toString(method, conditionExpression); + Expression condExp = this.conditionCache.get(key); if (condExp == null) { condExp = this.parser.parseExpression(conditionExpression); - this.conditionCache.put(method, condExp); + this.conditionCache.put(key, condExp); } return condExp.getValue(evalContext, boolean.class); } public Object key(String keyExpression, Method method, EvaluationContext evalContext) { - Expression keyExp = this.keyCache.get(method); + String key = toString(method, keyExpression); + Expression keyExp = this.keyCache.get(key); if (keyExp == null) { keyExp = this.parser.parseExpression(keyExpression); - this.keyCache.put(method, keyExp); + this.keyCache.put(key, keyExp); } return keyExp.getValue(evalContext); } -} + 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(); + } +} \ No newline at end of file diff --git a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java index f8a5b00f9c..5227e43b4c 100644 --- a/org.springframework.context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java +++ b/org.springframework.context/src/main/java/org/springframework/cache/interceptor/LazyParamAwareEvaluationContext.java @@ -45,13 +45,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,13 +85,14 @@ class LazyParamAwareEvaluationContext extends StandardEvaluationContext { return; } - Method targetMethod = this.methodCache.get(this.method); + String mKey = toString(this.method); + Method targetMethod = this.methodCache.get(mKey); if (targetMethod == null) { targetMethod = AopUtils.getMostSpecificMethod(this.method, this.targetClass); if (targetMethod == null) { targetMethod = this.method; } - this.methodCache.put(this.method, targetMethod); + this.methodCache.put(mKey, targetMethod); } // save arguments as indexed variables @@ -108,4 +109,11 @@ 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(); + } +} \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java b/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java index 15429fd77d..cfc661281f 100644 --- a/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java +++ b/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java @@ -307,6 +307,8 @@ public abstract class AbstractAnnotationTests { public void testMultiEvict(CacheableService service) { Object o1 = new Object(); + Object o2 = o1.toString() + "A"; + Object r1 = service.multiCache(o1); Object r2 = service.multiCache(o1); @@ -314,6 +316,7 @@ public abstract class AbstractAnnotationTests { Cache primary = cm.getCache("primary"); Cache secondary = cm.getCache("secondary"); + primary.put(o2, o2); assertSame(r1, r2); assertSame(r1, primary.get(o1).get()); assertSame(r1, secondary.get(o1).get()); @@ -321,6 +324,7 @@ public abstract class AbstractAnnotationTests { service.multiEvict(o1); assertNull(primary.get(o1)); assertNull(secondary.get(o1)); + assertNull(primary.get(o2)); Object r3 = service.multiCache(o1); Object r4 = service.multiCache(o1); diff --git a/org.springframework.context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java b/org.springframework.context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java index 2db724f6f1..94d8f9c4c8 100644 --- a/org.springframework.context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java +++ b/org.springframework.context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java @@ -116,7 +116,7 @@ public class AnnotatedClassCacheableService implements CacheableService return counter.getAndIncrement(); } - @Caching(evict = { @CacheEvict("primary"), @CacheEvict(value = "secondary", key = "#p0") }) + @Caching(evict = { @CacheEvict("primary"), @CacheEvict(value = "secondary", key = "#p0"), @CacheEvict(value = "primary", key = "#p0 + 'A'") }) public Object multiEvict(Object arg1) { return counter.getAndIncrement(); } diff --git a/org.springframework.context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java b/org.springframework.context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java index aeabc9a046..034df1143a 100644 --- a/org.springframework.context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java +++ b/org.springframework.context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java @@ -122,7 +122,7 @@ public class DefaultCacheableService implements CacheableService { return counter.getAndIncrement(); } - @Caching(evict = { @CacheEvict("primary"), @CacheEvict(value = "secondary", key = "#p0") }) + @Caching(evict = { @CacheEvict("primary"), @CacheEvict(value = "secondary", key = "#p0"), @CacheEvict(value = "primary", key = "#p0 + 'A'") }) public Long multiEvict(Object arg1) { return counter.getAndIncrement(); } diff --git a/org.springframework.context/src/test/resources/org/springframework/cache/config/cache-advice.xml b/org.springframework.context/src/test/resources/org/springframework/cache/config/cache-advice.xml index 3f5e85b849..ec36d11ffa 100644 --- a/org.springframework.context/src/test/resources/org/springframework/cache/config/cache-advice.xml +++ b/org.springframework.context/src/test/resources/org/springframework/cache/config/cache-advice.xml @@ -34,6 +34,7 @@ + @@ -72,6 +73,7 @@ +