From 1eb54b700da7747be5a4808af1fa1e59796ccad9 Mon Sep 17 00:00:00 2001
From: Costin Leau <cleau@vmware.com>
Date: Sun, 6 Mar 2011 17:13:24 +0000
Subject: [PATCH] SPR-8015 + update default key generator strategy to improve
 compatibility for implicit declaration on one arg method + updated docs

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4066 50f2f4bb-b051-0410-bef5-90022cba6387
---
 .../cache/config/annotation-cache-aspectj.xml | 38 +++++++++++++++++++
 .../springframework/cache/KeyGenerator.java   |  2 +-
 .../cache/support/DefaultKeyGenerator.java    | 12 +++++-
 .../cache/config/AbstractAnnotationTest.java  | 25 ++++++++++++
 .../AnnotatedClassCacheableService.java       |  4 ++
 .../cache/config/CacheableService.java        |  2 +
 .../cache/config/DefaultCacheableService.java |  4 ++
 spring-framework-reference/src/cache.xml      | 10 +++--
 8 files changed, 91 insertions(+), 6 deletions(-)
 create mode 100644 org.springframework.aspects/src/test/resources/org/springframework/cache/config/annotation-cache-aspectj.xml

diff --git a/org.springframework.aspects/src/test/resources/org/springframework/cache/config/annotation-cache-aspectj.xml b/org.springframework.aspects/src/test/resources/org/springframework/cache/config/annotation-cache-aspectj.xml
new file mode 100644
index 00000000000..9877eba813c
--- /dev/null
+++ b/org.springframework.aspects/src/test/resources/org/springframework/cache/config/annotation-cache-aspectj.xml
@@ -0,0 +1,38 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns:aop="http://www.springframework.org/schema/aop"
+       xmlns:p="http://www.springframework.org/schema/p"
+       xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd 
+       		http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd">
+
+	<bean id="annotationSource" class="org.springframework.cache.annotation.AnnotationCacheDefinitionSource"/>
+	
+	<aop:config>
+		<aop:advisor advice-ref="debugInterceptor" pointcut="execution(* *..CacheableService.*(..))" order="1"/>
+	</aop:config>
+	
+	<bean id="cacheAspect" class="org.springframework.cache.aspectj.AnnotationCacheAspect" factory-method="aspectOf">
+		<property name="cacheManager" ref="cacheManager"/>
+		<property name="cacheDefinitionSources" ref="annotationSource"/>
+	</bean>
+	
+	<bean id="advisor" class="org.springframework.cache.interceptor.BeanFactoryCacheDefinitionSourceAdvisor">
+		<property name="cacheDefinitionSource" ref="annotationSource"/>
+		<property name="adviceBeanName" value="cacheInterceptor"/>
+	</bean>
+	
+
+	<bean id="cacheManager" class="org.springframework.cache.support.MapCacheManager">
+		<property name="caches">
+			<set>
+				<bean class="org.springframework.cache.concurrent.ConcurrentCacheFactoryBean" p:name="default"/>
+			</set>
+		</property>
+	</bean>
+
+	<bean id="debugInterceptor" class="org.springframework.aop.interceptor.DebugInterceptor"/>
+	
+	<bean id="service" class="org.springframework.cache.config.DefaultCacheableService"/>
+	<bean id="classService" class="org.springframework.cache.config.AnnotatedClassCacheableService"/>
+
+</beans>
diff --git a/org.springframework.context/src/main/java/org/springframework/cache/KeyGenerator.java b/org.springframework.context/src/main/java/org/springframework/cache/KeyGenerator.java
index 704bf35d9b5..ae5dd9b89a7 100644
--- a/org.springframework.context/src/main/java/org/springframework/cache/KeyGenerator.java
+++ b/org.springframework.context/src/main/java/org/springframework/cache/KeyGenerator.java
@@ -20,7 +20,7 @@ import java.lang.reflect.Method;
 
 /**
  * Cache 'key' extractor. Used for creating a key based on the given method
- * (used as context) and its parameter.
+ * (used as context) and its parameters.
  * 
  * @author Costin Leau
  */
diff --git a/org.springframework.context/src/main/java/org/springframework/cache/support/DefaultKeyGenerator.java b/org.springframework.context/src/main/java/org/springframework/cache/support/DefaultKeyGenerator.java
index 9b2151010f8..72aab7e884e 100644
--- a/org.springframework.context/src/main/java/org/springframework/cache/support/DefaultKeyGenerator.java
+++ b/org.springframework.context/src/main/java/org/springframework/cache/support/DefaultKeyGenerator.java
@@ -21,14 +21,22 @@ import java.lang.reflect.Method;
 import org.springframework.cache.KeyGenerator;
 
 /**
- * Default key generator. Computes a resulting key based on the hashcode of the
- * given parameters.
+ * Default key generator. Returns 0 if no param is given, the param itself if only one is given or a hash code computed
+ * from all given params hash code.
  * 
  * @author Costin Leau
  */
 public class DefaultKeyGenerator implements KeyGenerator<Object> {
 
 	public Object extract(Method method, Object... params) {
+		if (params.length == 1) {
+			return params[0];
+		}
+
+		if (params.length == 0) {
+			return 0;
+		}
+
 		int hashCode = 17;
 
 		for (Object object : params) {
diff --git a/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java b/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java
index c2a3124fe9d..5709d5c1070 100644
--- a/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java
+++ b/org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java
@@ -77,6 +77,21 @@ public abstract class AbstractAnnotationTest {
 		assertSame(r3, r4);
 	}
 
+	public void testInvalidateWKey(CacheableService service) throws Exception {
+		Object o1 = new Object();
+		Object o2 = new Object();
+
+		Object r1 = service.cache(o1);
+		Object r2 = service.cache(o1);
+
+		assertSame(r1, r2);
+		service.invalidate(o1, null);
+		Object r3 = service.cache(o1);
+		Object r4 = service.cache(o1);
+		assertNotSame(r1, r3);
+		assertSame(r3, r4);
+	}
+
 	public void testConditionalExpression(CacheableService service)
 			throws Exception {
 		Object r1 = service.conditional(4);
@@ -132,6 +147,11 @@ public abstract class AbstractAnnotationTest {
 		testInvalidate(cs);
 	}
 
+	@Test
+	public void testInvalidateWithKey() throws Exception {
+		testInvalidateWKey(cs);
+	}
+
 	@Test
 	public void testConditionalExpression() throws Exception {
 		testConditionalExpression(cs);
@@ -152,6 +172,11 @@ public abstract class AbstractAnnotationTest {
 		testInvalidate(ccs);
 	}
 
+	@Test
+	public void testClassCacheInvalidateWKey() throws Exception {
+		testInvalidateWKey(ccs);
+	}
+
 	@Test
 	public void testNullValue() throws Exception {
 		testNullValue(cs);
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 6104c195dbc..d3e2cbabc89 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
@@ -42,6 +42,10 @@ public class AnnotatedClassCacheableService implements CacheableService {
 	public void invalidate(Object arg1) {
 	}
 
+	@CacheEvict(value = "default", key = "#p0")
+	public void invalidate(Object arg1, Object arg2) {
+	}
+
 	@Cacheable(value = "default", key = "#p0")
 	public Object key(Object arg1, Object arg2) {
 		return counter.getAndIncrement();
diff --git a/org.springframework.context/src/test/java/org/springframework/cache/config/CacheableService.java b/org.springframework.context/src/test/java/org/springframework/cache/config/CacheableService.java
index 8e7d8fe794b..34f5e164ea8 100644
--- a/org.springframework.context/src/test/java/org/springframework/cache/config/CacheableService.java
+++ b/org.springframework.context/src/test/java/org/springframework/cache/config/CacheableService.java
@@ -28,6 +28,8 @@ public interface CacheableService<T> {
 
 	void invalidate(Object arg1);
 
+	void invalidate(Object arg1, Object arg2);
+
 	T conditional(int field);
 
 	T key(Object arg1, Object arg2);
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 b96493ac55b..4c9134a6ed3 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
@@ -40,6 +40,10 @@ public class DefaultCacheableService implements CacheableService<Long> {
 	public void invalidate(Object arg1) {
 	}
 
+	@CacheEvict(value = "default", key = "#p0")
+	public void invalidate(Object arg1, Object arg2) {
+	}
+
 	@Cacheable(value = "default", condition = "#classField == 3")
 	public Long conditional(int classField) {
 		return counter.getAndIncrement();
diff --git a/spring-framework-reference/src/cache.xml b/spring-framework-reference/src/cache.xml
index 261e5293110..0e35e9ec662 100644
--- a/spring-framework-reference/src/cache.xml
+++ b/spring-framework-reference/src/cache.xml
@@ -86,9 +86,13 @@ public Book findBook(ISBN isbn) {...}]]></programlisting>
 			<title>Default Key Generation</title>
 		
 			<para>Since caches are essentially key-value stores, each invocation of a cached method needs to be translated into a suitable key for cache access.
-			Out of the box, the caching abstraction uses a simple <literal>hash-code</literal> based <interfacename>KeyGenerator</interfacename> that computes the key based on the 
-			hashes of all objects used for method invocation. This approach works well for objects with <emphasis>natural keys</emphasis> as long as 
-			the <literal>hashCode()</literal> reflects that. If that is not the case then
+			Out of the box, the caching abstraction uses a simple <interfacename>KeyGenerator</interfacename> based on the following algorithm:</para>
+			<itemizedlist>
+			   <listitem>If no params are given, return 0.</listitem>
+			   <listitem>If only one param is given, return that instance.</listitem>
+			   <listitem>If more the one param is given, return a key computed from the hashes of all parameters.</listitem>
+			</itemizedlist>
+			This approach works well for objects with <emphasis>natural keys</emphasis> as long as the <literal>hashCode()</literal> reflects that. If that is not the case then
 			for distributed or persistent environments, the strategy needs to be changed as the objects hashCode is not preserved. 
 			In fact, depending on the JVM implementation or running conditions, the same hashCode can be reused for different objects, in the same VM instance.</para>