From bfba53f958640a5ffa3c2603188106c4a2505ea8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 19 Dec 2013 19:15:57 +0100 Subject: [PATCH] Fixed handling of primitive vararg array in CacheOperationContext Issue: SPR-11249 --- .../cache/interceptor/CacheAspectSupport.java | 23 +++++----- .../cache/CacheReproTests.java | 45 ++++++++++++++++--- 2 files changed, 53 insertions(+), 15 deletions(-) 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 4547673011..2c2bdb4f6a 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 @@ -25,6 +25,7 @@ import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.aop.framework.AopProxyUtils; import org.springframework.beans.factory.InitializingBean; import org.springframework.cache.Cache; @@ -37,6 +38,7 @@ import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -114,7 +116,7 @@ public abstract class CacheAspectSupport implements InitializingBean { /** * Set the KeyGenerator for this cache aspect. - * Default is {@link DefaultKeyGenerator}. + * The default is a {@link SimpleKeyGenerator}. */ public void setKeyGenerator(KeyGenerator keyGenerator) { this.keyGenerator = keyGenerator; @@ -249,8 +251,8 @@ public abstract class CacheAspectSupport implements InitializingBean { } private void logInvalidating(CacheOperationContext context, CacheEvictOperation operation, Object key) { - if (this.logger.isTraceEnabled()) { - this.logger.trace("Invalidating " + (key == null ? "entire cache" : "cache key " + key) + + if (logger.isTraceEnabled()) { + logger.trace("Invalidating " + (key != null ? "cache key [" + key + "]" : "entire cache") + " for operation " + operation + " on method " + context.method); } } @@ -302,8 +304,8 @@ public abstract class CacheAspectSupport implements InitializingBean { private boolean isConditionPassing(CacheOperationContext context, Object result) { boolean passing = context.isConditionPassing(result); - if (!passing && this.logger.isTraceEnabled()) { - this.logger.trace("Cache condition failed on method " + context.method + " for operation " + context.operation); + if (!passing && logger.isTraceEnabled()) { + logger.trace("Cache condition failed on method " + context.method + " for operation " + context.operation); } return passing; } @@ -312,8 +314,8 @@ public abstract class CacheAspectSupport implements InitializingBean { Object key = context.generateKey(result); Assert.notNull(key, "Null key returned for cache operation (maybe you are using named params " + "on classes without debug info?) " + context.operation); - if (this.logger.isTraceEnabled()) { - this.logger.trace("Computed cache key " + key + " for operation " + context.operation); + if (logger.isTraceEnabled()) { + logger.trace("Computed cache key " + key + " for operation " + context.operation); } return key; } @@ -334,7 +336,7 @@ public abstract class CacheAspectSupport implements InitializingBean { Method method, Object[] args, Object target, Class targetClass) { for (CacheOperation operation : operations) { - this.contexts.add(operation.getClass(), new CacheOperationContext(operation, method, args, target, targetClass)); + this.contexts.add(operation.getClass(), getOperationContext(operation, method, args, target, targetClass)); } } @@ -373,7 +375,7 @@ public abstract class CacheAspectSupport implements InitializingBean { if (!method.isVarArgs()) { return args; } - Object[] varArgs = (Object[]) args[args.length - 1]; + Object[] varArgs = ObjectUtils.toObjectArray(args[args.length - 1]); Object[] combinedArgs = new Object[args.length - 1 + varArgs.length]; System.arraycopy(args, 0, combinedArgs, 0, args.length - 1); System.arraycopy(varArgs, 0, combinedArgs, args.length - 1, varArgs.length); @@ -416,7 +418,8 @@ public abstract class CacheAspectSupport implements InitializingBean { } private EvaluationContext createEvaluationContext(Object result) { - return evaluator.createEvaluationContext(this.caches, this.method, this.args, this.target, this.targetClass, result); + return evaluator.createEvaluationContext( + this.caches, this.method, this.args, this.target, this.targetClass, result); } protected Collection getCaches() { diff --git a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java index c89520a0a4..d7242c4e76 100644 --- a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java +++ b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.List; import org.junit.Test; + import org.springframework.cache.annotation.Cacheable; import org.springframework.cache.annotation.Caching; import org.springframework.cache.annotation.EnableCaching; @@ -34,13 +35,13 @@ import static org.junit.Assert.*; * Tests to reproduce raised caching issues. * * @author Phillip Webb + * @author Juergen Hoeller */ public class CacheReproTests { @Test public void spr11124() throws Exception { - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( - Spr11124Config.class); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr11124Config.class); Spr11124Service bean = context.getBean(Spr11124Service.class); bean.single(2); bean.single(2); @@ -49,6 +50,16 @@ public class CacheReproTests { context.close(); } + @Test + public void spr11249() throws Exception { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Spr11249Config.class); + Spr11249Service bean = context.getBean(Spr11249Service.class); + Object result = bean.doSomething("op", 2, 3); + assertSame(result, bean.doSomething("op", 2, 3)); + context.close(); + } + + @Configuration @EnableCaching public static class Spr11124Config { @@ -62,23 +73,23 @@ public class CacheReproTests { public Spr11124Service service() { return new Spr11124ServiceImpl(); } - } + public interface Spr11124Service { public List single(int id); public List multiple(int id); - } + public static class Spr11124ServiceImpl implements Spr11124Service { private int multipleCount = 0; @Override - @Cacheable(value = "smallCache") + @Cacheable("smallCache") public List single(int id) { if (this.multipleCount > 0) { fail("Called too many times"); @@ -98,7 +109,31 @@ public class CacheReproTests { this.multipleCount++; return Collections.emptyList(); } + } + + @Configuration + @EnableCaching + public static class Spr11249Config { + + @Bean + public CacheManager cacheManager() { + return new ConcurrentMapCacheManager(); + } + + @Bean + public Spr11249Service service() { + return new Spr11249Service(); + } + } + + + public static class Spr11249Service { + + @Cacheable("smallCache") + public Object doSomething(String name, int... values) { + return new Object(); + } } }