From 81c208098fb5ffb82abb88597710663da67fd764 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 10 Jan 2014 16:24:51 +0100 Subject: [PATCH] Custom KeyGenerator This commit adds an extra parameter to the base @Cache method annotations: keyGenerator. This parameter holds the name of the KeyGenerator bean to use to compute the key for that specific caching endpoint. This gives therefore a third way to customize the key. These are: 1. Default KeyGenerator (global for all endpoints) 2. The 'key' attribute of the annotation, giving the SpEL expression to use 3. The 'keyGenerator' attribute of the annotation The annotation attributes are therefore exclusive. Trying to specify them both will result in an IllegalStateException. The KeyGenerator to use for a given operation is cached on startup so that multiple calls to it does not resolve the instance to use over and over again. Issue: SPR-10629 --- .../AnnotatedClassCacheableService.java | 14 ++++- .../cache/config/CacheableService.java | 6 ++- .../cache/config/DefaultCacheableService.java | 14 ++++- .../cache/config/SomeCustomKeyGenerator.java | 46 +++++++++++++++++ .../cache/config/annotation-cache-aspectj.xml | 2 + .../cache/annotation/CacheEvict.java | 11 +++- .../cache/annotation/CachePut.java | 11 +++- .../cache/annotation/Cacheable.java | 11 +++- .../SpringCacheAnnotationParser.java | 21 +++++++- .../cache/config/CacheAdviceParser.java | 15 +++++- .../cache/interceptor/CacheAspectSupport.java | 35 ++++++++++--- .../cache/interceptor/CacheOperation.java | 15 +++++- .../cache/config/spring-cache-4.0.xsd | 8 ++- .../AnnotationCacheOperationSourceTests.java | 51 +++++++++++++++++-- .../cache/config/AbstractAnnotationTests.java | 28 ++++++++-- .../AnnotatedClassCacheableService.java | 14 ++++- .../cache/config/CacheAdviceParserTests.java | 40 +++++++++++++++ .../cache/config/CacheableService.java | 6 ++- .../cache/config/DefaultCacheableService.java | 14 ++++- .../cache/config/EnableCachingTests.java | 9 +++- .../cache/config/SomeCustomKeyGenerator.java | 46 +++++++++++++++++ .../config/annotationDrivenCacheConfig.xml | 6 +-- .../config/annotationDrivenCacheNamespace.xml | 4 +- .../cache/config/cache-advice-invalid.xml | 14 +++++ .../cache/config/cache-advice.xml | 4 ++ 25 files changed, 407 insertions(+), 38 deletions(-) create mode 100644 spring-aspects/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java create mode 100644 spring-context/src/test/java/org/springframework/cache/config/CacheAdviceParserTests.java create mode 100644 spring-context/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java create mode 100644 spring-context/src/test/resources/org/springframework/cache/config/cache-advice-invalid.xml diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java index dbfd801716..9a00b17c2b 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.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. @@ -105,6 +105,18 @@ public class AnnotatedClassCacheableService implements CacheableService return counter.getAndIncrement(); } + @Override + @Cacheable(value = "default", keyGenerator = "customKyeGenerator") + public Object customKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(value = "default", keyGenerator = "unknownBeanName") + public Object unknownCustomKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + @Override @CachePut("default") public Object update(Object arg1) { diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java index 3405d5f8c4..fe523574e5 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/CacheableService.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. @@ -58,6 +58,10 @@ public interface CacheableService { T rootVars(Object arg1); + T customKeyGenerator(Object arg1); + + T unknownCustomKeyGenerator(Object arg1); + T throwChecked(Object arg1) throws Exception; T throwUnchecked(Object arg1); diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java index cabeabd7aa..4ef5fa1eb3 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/DefaultCacheableService.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. @@ -109,6 +109,18 @@ public class DefaultCacheableService implements CacheableService { return counter.getAndIncrement(); } + @Override + @Cacheable(value = "default", keyGenerator = "customKeyGenerator") + public Long customKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(value = "default", keyGenerator = "unknownBeanName") + public Long unknownCustomKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + @Override @CachePut("default") public Long update(Object arg1) { diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java b/spring-aspects/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java new file mode 100644 index 0000000000..8a2b160ffa --- /dev/null +++ b/spring-aspects/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java @@ -0,0 +1,46 @@ +/* + * 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.springframework.cache.interceptor.KeyGenerator; + +import java.lang.reflect.Method; + +/** + * A custom {@link KeyGenerator} that exposes the algorithm used to compute the key + * for convenience in tests scenario. + * + * @author Stephane Nicoll + */ +final class SomeCustomKeyGenerator implements KeyGenerator { + + @Override + public Object generate(Object target, Method method, Object... params) { + return generateKey(method.getName(), params); + } + + /** + * @see #generate(Object, java.lang.reflect.Method, Object...) + */ + static Object generateKey(String methodName, Object... params) { + final StringBuilder sb = new StringBuilder(methodName); + for (Object param : params) { + sb.append(param); + } + return sb.toString(); + } +} diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/annotation-cache-aspectj.xml b/spring-aspects/src/test/java/org/springframework/cache/config/annotation-cache-aspectj.xml index 5650a370d2..f556250129 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/annotation-cache-aspectj.xml +++ b/spring-aspects/src/test/java/org/springframework/cache/config/annotation-cache-aspectj.xml @@ -43,6 +43,8 @@ + + diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/CacheEvict.java b/spring-context/src/main/java/org/springframework/cache/annotation/CacheEvict.java index 6017ca6df2..1d7b6e6483 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/CacheEvict.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/CacheEvict.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -45,10 +45,17 @@ public @interface CacheEvict { /** * Spring Expression Language (SpEL) attribute for computing the key dynamically. - *

Default is "", meaning all method parameters are considered as a key. + *

Default is "", meaning all method parameters are considered as a key, unless + * a custom {@link #keyGenerator()} has been set. */ String key() default ""; + /** + * The bean name of the custom {@link org.springframework.cache.interceptor.KeyGenerator} to use. + *

Mutually exclusive with the {@link #key()} attribute. + */ + String keyGenerator() default ""; + /** * Spring Expression Language (SpEL) attribute used for conditioning the method caching. *

Default is "", meaning the method is always cached. diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/CachePut.java b/spring-context/src/main/java/org/springframework/cache/annotation/CachePut.java index 3ab2f52b0a..3f12b53204 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/CachePut.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/CachePut.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. @@ -50,10 +50,17 @@ public @interface CachePut { /** * Spring Expression Language (SpEL) attribute for computing the key dynamically. - *

Default is "", meaning all method parameters are considered as a key. + *

Default is "", meaning all method parameters are considered as a key, unless + * a custom {@link #keyGenerator()} has been set. */ String key() default ""; + /** + * The bean name of the custom {@link org.springframework.cache.interceptor.KeyGenerator} to use. + *

Mutually exclusive with the {@link #key()} attribute. + */ + String keyGenerator() default ""; + /** * Spring Expression Language (SpEL) attribute used for conditioning the cache update. *

Default is "", meaning the method result is always cached. diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java b/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java index 1ab8df5762..2e5259171f 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/Cacheable.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. @@ -48,10 +48,17 @@ public @interface Cacheable { /** * Spring Expression Language (SpEL) attribute for computing the key dynamically. - *

Default is "", meaning all method parameters are considered as a key. + *

Default is "", meaning all method parameters are considered as a key, unless + * a custom {@link #keyGenerator()} has been set. */ String key() default ""; + /** + * The bean name of the custom {@link org.springframework.cache.interceptor.KeyGenerator} to use. + *

Mutually exclusive with the {@link #key()} attribute. + */ + String keyGenerator() default ""; + /** * Spring Expression Language (SpEL) attribute used for conditioning the method caching. *

Default is "", meaning the method is always cached. diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java b/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java index 100c380d34..dbadb9a834 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java +++ b/spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.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,6 +27,7 @@ import org.springframework.cache.interceptor.CacheOperation; import org.springframework.cache.interceptor.CachePutOperation; import org.springframework.cache.interceptor.CacheableOperation; import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; /** * Strategy implementation for parsing Spring's {@link Caching}, {@link Cacheable}, @@ -86,7 +87,10 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria cuo.setCondition(caching.condition()); cuo.setUnless(caching.unless()); cuo.setKey(caching.key()); + cuo.setKeyGenerator(caching.keyGenerator()); cuo.setName(ae.toString()); + + checkKeySourceConsistency(ae, caching.key(), caching.keyGenerator()); return cuo; } @@ -95,9 +99,12 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria ceo.setCacheNames(caching.value()); ceo.setCondition(caching.condition()); ceo.setKey(caching.key()); + ceo.setKeyGenerator(caching.keyGenerator()); ceo.setCacheWide(caching.allEntries()); ceo.setBeforeInvocation(caching.beforeInvocation()); ceo.setName(ae.toString()); + + checkKeySourceConsistency(ae, caching.key(), caching.keyGenerator()); return ceo; } @@ -107,7 +114,10 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria cuo.setCondition(caching.condition()); cuo.setUnless(caching.unless()); cuo.setKey(caching.key()); + cuo.setKeyGenerator(caching.keyGenerator()); cuo.setName(ae.toString()); + + checkKeySourceConsistency(ae, caching.key(), caching.keyGenerator()); return cuo; } @@ -159,6 +169,15 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria return (anns.isEmpty() ? null : anns); } + private void checkKeySourceConsistency(AnnotatedElement ae, String key, String keyGenerator) { + if (StringUtils.hasText(key) && StringUtils.hasText(keyGenerator)) { + throw new IllegalStateException("Invalid cache annotation configuration on '" + + ae.toString() + "'. Both 'key' and 'keyGenerator' attributes have been set. " + + "These attributes are mutually exclusive: either set the SpEL expression used to" + + "compute the key at runtime or set the name of the KeyGenerator bean to use."); + } + } + @Override public boolean equals(Object other) { return (this == other || other instanceof SpringCacheAnnotationParser); diff --git a/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java b/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java index b966fc232d..c44ee26d36 100644 --- a/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java +++ b/spring-context/src/main/java/org/springframework/cache/config/CacheAdviceParser.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. @@ -168,7 +168,7 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser { private static String getAttributeValue(Element element, String attributeName, String defaultValue) { String attribute = element.getAttribute(attributeName); - if(StringUtils.hasText(attribute)) { + if (StringUtils.hasText(attribute)) { return attribute.trim(); } return defaultValue; @@ -184,6 +184,8 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser { private String key; + private String keyGenerator; + private String condition; private String method; @@ -194,6 +196,7 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser { Props(Element root) { String defaultCache = root.getAttribute("cache"); key = root.getAttribute("key"); + keyGenerator = root.getAttribute("key-generator"); condition = root.getAttribute("condition"); method = root.getAttribute(METHOD_ATTRIBUTE); @@ -218,8 +221,16 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser { op.setCacheNames(localCaches); op.setKey(getAttributeValue(element, "key", this.key)); + op.setKeyGenerator(getAttributeValue(element, "key-generator", this.keyGenerator)); op.setCondition(getAttributeValue(element, "condition", this.condition)); + if (StringUtils.hasText(op.getKey()) && StringUtils.hasText(op.getKeyGenerator())) { + throw new IllegalStateException("Invalid cache advice configuration on '" + + element.toString() + "'. Both 'key' and 'keyGenerator' attributes have been set. " + + "These attributes are mutually exclusive: either set the SpEL expression used to" + + "compute the key at runtime or set the name of the KeyGenerator bean to use."); + } + return op; } 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 fac0d74305..21125779a8 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 @@ -29,9 +29,12 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.framework.AopProxyUtils; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; import org.springframework.cache.support.SimpleValueWrapper; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.expression.EvaluationContext; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -67,7 +70,7 @@ import org.springframework.util.StringUtils; * @author Stephane Nicoll * @since 3.1 */ -public abstract class CacheAspectSupport implements InitializingBean { +public abstract class CacheAspectSupport implements InitializingBean, ApplicationContextAware { protected final Log logger = LogFactory.getLog(getClass()); @@ -79,6 +82,8 @@ public abstract class CacheAspectSupport implements InitializingBean { private KeyGenerator keyGenerator = new SimpleKeyGenerator(); + private ApplicationContext applicationContext; + private boolean initialized = false; @@ -116,24 +121,31 @@ public abstract class CacheAspectSupport implements InitializingBean { } /** - * Set the KeyGenerator for this cache aspect. - * The default is a {@link SimpleKeyGenerator}. + * Set the default {@link KeyGenerator} that this cache aspect should delegate to + * if no specific key generator has been set for the operation. + *

The default is a {@link SimpleKeyGenerator} */ public void setKeyGenerator(KeyGenerator keyGenerator) { this.keyGenerator = keyGenerator; } /** - * Return the KeyGenerator for this cache aspect, + * Return the default {@link KeyGenerator} that this cache aspect delegates to. */ public KeyGenerator getKeyGenerator() { return this.keyGenerator; } + @Override + public void setApplicationContext(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + public void afterPropertiesSet() { Assert.state(this.cacheManager != null, "Property 'cacheManager' is required"); Assert.state(this.cacheOperationSource != null, "Property 'cacheOperationSources' is required: " + "If there are no cacheable methods, then don't use a cache aspect."); + Assert.state(this.applicationContext != null, "The application context was not injected as it should."); this.initialized = true; } @@ -372,14 +384,22 @@ public abstract class CacheAspectSupport implements InitializingBean { private final Collection caches; + private final KeyGenerator operationKeyGenerator; + public CacheOperationContext(CacheOperation operation, Method method, - Object[] args, Object target, Class targetClass) { + Object[] args, Object target, Class targetClass) { this.operation = operation; this.method = method; this.args = extractArgs(method, args); this.target = target; this.targetClass = targetClass; this.caches = CacheAspectSupport.this.getCaches(operation); + if (StringUtils.hasText(operation.getKeyGenerator())) { // TODO: exception mgt? + this.operationKeyGenerator = BeanFactoryAnnotationUtils.qualifiedBeanOfType( + applicationContext, KeyGenerator.class, operation.getKeyGenerator()); + } else { + this.operationKeyGenerator = keyGenerator; + } } private Object[] extractArgs(Method method, Object[] args) { @@ -405,8 +425,7 @@ public abstract class CacheAspectSupport implements InitializingBean { String unless = ""; if (this.operation instanceof CacheableOperation) { unless = ((CacheableOperation) this.operation).getUnless(); - } - else if (this.operation instanceof CachePutOperation) { + } else if (this.operation instanceof CachePutOperation) { unless = ((CachePutOperation) this.operation).getUnless(); } if (StringUtils.hasText(unless)) { @@ -425,7 +444,7 @@ public abstract class CacheAspectSupport implements InitializingBean { EvaluationContext evaluationContext = createEvaluationContext(result); return evaluator.key(this.operation.getKey(), this.method, evaluationContext); } - return keyGenerator.generate(this.target, this.method, this.args); + return operationKeyGenerator.generate(this.target, this.method, this.args); } private EvaluationContext createEvaluationContext(Object result) { diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java index bff9ff175c..8992d0ed4e 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperation.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperation.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. @@ -35,6 +35,8 @@ public abstract class CacheOperation { private String key = ""; + private String keyGenerator = ""; + private String name = ""; @@ -50,6 +52,10 @@ public abstract class CacheOperation { return key; } + public String getKeyGenerator() { + return keyGenerator; + } + public String getName() { return name; } @@ -77,6 +83,11 @@ public abstract class CacheOperation { this.key = key; } + public void setKeyGenerator(String keyGenerator) { + Assert.notNull(keyGenerator); + this.keyGenerator = keyGenerator; + } + public void setName(String name) { Assert.hasText(name); this.name = name; @@ -124,6 +135,8 @@ public abstract class CacheOperation { result.append(this.cacheNames); result.append(" | key='"); result.append(this.key); + result.append("' | keyGenerator='"); + result.append(this.keyGenerator); result.append("' | condition='"); result.append(this.condition); result.append("'"); diff --git a/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.0.xsd b/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.0.xsd index 96115aa624..b63601c36f 100644 --- a/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.0.xsd +++ b/spring-context/src/main/resources/org/springframework/cache/config/spring-cache-4.0.xsd @@ -176,9 +176,15 @@ + The SpEL expression used for computing the cache key, mutually exclusive with the key-generator parameter.]]> + + + + + ops = getOps("customKeyGenerator"); + assertEquals(1, ops.size()); + CacheOperation cacheOperation = ops.iterator().next(); + assertEquals("Custom key generator not set", "custom", cacheOperation.getKeyGenerator()); + } + + @Test + public void testCustomKeyGeneratorInherited() { + Collection ops = getOps("customKeyGeneratorInherited"); + assertEquals(1, ops.size()); + CacheOperation cacheOperation = ops.iterator().next(); + assertEquals("Custom key generator not set", "custom", cacheOperation.getKeyGenerator()); + } + + @Test + public void testKeyAndKeyGeneratorCannotBeSetTogether() { + try { + getOps("invalidKeyAndKeyGeneratorSet"); + fail("Should have failed to parse @Cacheable annotation"); + } catch (IllegalStateException e) { + // expected + } + } + private static class AnnotatedClass { @Cacheable("test") public void singular() { @@ -100,13 +126,16 @@ public class AnnotationCacheOperationSourceTests { public void multiple() { } - @Caching(cacheable = { @Cacheable("test") }, evict = { @CacheEvict("test") }) + @Caching(cacheable = {@Cacheable("test")}, evict = {@CacheEvict("test")}) public void caching() { } + @Cacheable(value = "test", keyGenerator = "custom") + public void customKeyGenerator() { + } + @EvictFoo public void singleStereotype() { - } @EvictFoo @@ -115,9 +144,17 @@ public class AnnotationCacheOperationSourceTests { public void multipleStereotype() { } - @Caching(cacheable = { @Cacheable(value = "test", key = "a"), @Cacheable(value = "test", key = "b") }) + @Caching(cacheable = {@Cacheable(value = "test", key = "a"), @Cacheable(value = "test", key = "b")}) public void multipleCaching() { } + + @CacheableFooCustomKeyGenerator + public void customKeyGeneratorInherited() { + } + + @Cacheable(value = "test", key = "#root.methodName", keyGenerator = "custom") + public void invalidKeyAndKeyGeneratorSet() { + } } @Retention(RetentionPolicy.RUNTIME) @@ -126,6 +163,12 @@ public class AnnotationCacheOperationSourceTests { public @interface CacheableFoo { } + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.METHOD) + @Cacheable(value = "foo", keyGenerator = "custom") + public @interface CacheableFooCustomKeyGenerator { + } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @CacheEvict(value = "foo") diff --git a/spring-context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java b/spring-context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java index 2aa1109cce..a402e3ba0b 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.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. @@ -16,8 +16,7 @@ package org.springframework.cache.config; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import java.util.Collection; @@ -26,6 +25,7 @@ import java.util.UUID; import org.junit.Before; import org.junit.Test; import org.springframework.aop.framework.AopProxyUtils; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; import org.springframework.context.ApplicationContext; @@ -569,6 +569,28 @@ public abstract class AbstractAnnotationTests { testRootVars(ccs); } + @Test + public void testCustomKeyGenerator() { + Object param = new Object(); + Object r1 = cs.customKeyGenerator(param); + assertSame(r1, cs.customKeyGenerator(param)); + Cache cache = cm.getCache("default"); + // Checks that the custom keyGenerator was used + Object expectedKey = SomeCustomKeyGenerator.generateKey("customKeyGenerator", param); + assertNotNull(cache.get(expectedKey)); + } + + @Test + public void testUnknownCustomKeyGenerator() { + try { + Object param = new Object(); + cs.unknownCustomKeyGenerator(param); + fail("should have failed with NoSuchBeanDefinitionException"); + } catch (NoSuchBeanDefinitionException e) { + // expected + } + } + @Test public void testNullArg() throws Exception { testNullArg(cs); diff --git a/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java b/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java index 2e15d90748..899c23d0b6 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java +++ b/spring-context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.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. @@ -107,6 +107,18 @@ public class AnnotatedClassCacheableService implements CacheableService return counter.getAndIncrement(); } + @Override + @Cacheable(value = "default", keyGenerator = "customKyeGenerator") + public Object customKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(value = "default", keyGenerator = "unknownBeanName") + public Object unknownCustomKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + @Override @CachePut("default") public Object update(Object arg1) { diff --git a/spring-context/src/test/java/org/springframework/cache/config/CacheAdviceParserTests.java b/spring-context/src/test/java/org/springframework/cache/config/CacheAdviceParserTests.java new file mode 100644 index 0000000000..d13891a0f3 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/cache/config/CacheAdviceParserTests.java @@ -0,0 +1,40 @@ +/* + * 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 static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.beans.factory.BeanDefinitionStoreException; +import org.springframework.context.support.GenericXmlApplicationContext; + +/** + * AOP advice specific parsing tests. + * + * @author Stephane Nicoll + */ +public class CacheAdviceParserTests { + + @Test + public void keyAndKeyGeneratorCannotBeSetTogether() { + try { + new GenericXmlApplicationContext("/org/springframework/cache/config/cache-advice-invalid.xml"); + fail("Should have failed to load context, one advise define both a key and a key generator"); + } catch (BeanDefinitionStoreException e) { // TODO better exception handling + } + } +} diff --git a/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java b/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java index 3405d5f8c4..fe523574e5 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/CacheableService.java +++ b/spring-context/src/test/java/org/springframework/cache/config/CacheableService.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. @@ -58,6 +58,10 @@ public interface CacheableService { T rootVars(Object arg1); + T customKeyGenerator(Object arg1); + + T unknownCustomKeyGenerator(Object arg1); + T throwChecked(Object arg1) throws Exception; T throwUnchecked(Object arg1); diff --git a/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java b/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java index 1dfbe93928..0c1881e795 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java +++ b/spring-context/src/test/java/org/springframework/cache/config/DefaultCacheableService.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. @@ -109,6 +109,18 @@ public class DefaultCacheableService implements CacheableService { return counter.getAndIncrement(); } + @Override + @Cacheable(value = "default", keyGenerator = "customKeyGenerator") + public Long customKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + + @Override + @Cacheable(value = "default", keyGenerator = "unknownBeanName") + public Long unknownCustomKeyGenerator(Object arg1) { + return counter.getAndIncrement(); + } + @Override @CachePut("default") public Long update(Object arg1) { diff --git a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java index 0a611c0bbf..2006ebd1a1 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -52,7 +52,7 @@ public class EnableCachingTests extends AbstractAnnotationTests { @Test public void testKeyStrategy() throws Exception { CacheInterceptor ci = ctx.getBean(CacheInterceptor.class); - assertSame(ctx.getBean(KeyGenerator.class), ci.getKeyGenerator()); + assertSame(ctx.getBean("keyGenerator", KeyGenerator.class), ci.getKeyGenerator()); } // --- local tests ------- @@ -140,6 +140,11 @@ public class EnableCachingTests extends AbstractAnnotationTests { public KeyGenerator keyGenerator() { return new SomeKeyGenerator(); } + + @Bean + public KeyGenerator customKeyGenerator() { + return new SomeCustomKeyGenerator(); + } } diff --git a/spring-context/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java b/spring-context/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java new file mode 100644 index 0000000000..8a2b160ffa --- /dev/null +++ b/spring-context/src/test/java/org/springframework/cache/config/SomeCustomKeyGenerator.java @@ -0,0 +1,46 @@ +/* + * 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.springframework.cache.interceptor.KeyGenerator; + +import java.lang.reflect.Method; + +/** + * A custom {@link KeyGenerator} that exposes the algorithm used to compute the key + * for convenience in tests scenario. + * + * @author Stephane Nicoll + */ +final class SomeCustomKeyGenerator implements KeyGenerator { + + @Override + public Object generate(Object target, Method method, Object... params) { + return generateKey(method.getName(), params); + } + + /** + * @see #generate(Object, java.lang.reflect.Method, Object...) + */ + static Object generateKey(String methodName, Object... params) { + final StringBuilder sb = new StringBuilder(methodName); + for (Object param : params) { + sb.append(param); + } + return sb.toString(); + } +} diff --git a/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheConfig.xml b/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheConfig.xml index 104a3fbb17..d2dd67bb4c 100644 --- a/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheConfig.xml +++ b/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheConfig.xml @@ -1,11 +1,9 @@ + http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd"> @@ -44,4 +42,6 @@ + + diff --git a/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheNamespace.xml b/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheNamespace.xml index 105a003504..83d34deff4 100644 --- a/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheNamespace.xml +++ b/spring-context/src/test/resources/org/springframework/cache/config/annotationDrivenCacheNamespace.xml @@ -30,5 +30,7 @@ - + + + diff --git a/spring-context/src/test/resources/org/springframework/cache/config/cache-advice-invalid.xml b/spring-context/src/test/resources/org/springframework/cache/config/cache-advice-invalid.xml new file mode 100644 index 0000000000..b156bdb3cc --- /dev/null +++ b/spring-context/src/test/resources/org/springframework/cache/config/cache-advice-invalid.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + diff --git a/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml b/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml index 78788afe02..1a87ea20c4 100644 --- a/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml +++ b/spring-context/src/test/resources/org/springframework/cache/config/cache-advice.xml @@ -16,6 +16,8 @@ + + @@ -107,6 +109,8 @@ + +