KeyGenerators should not return a plain array parameter as raw key but rather always handle that case in a deepHashCode fashion

Issue: SPR-11505

spring-context/src/main/java/org/springframework/cache/interceptor/DefaultKeyGenerator.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.
@@ -17,6 +17,7 @@
 package org.springframework.cache.interceptor;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
 
 /**
  * Default key generator. Returns {@value #NO_PARAM_KEY} if no
@@ -33,6 +34,7 @@ import java.lang.reflect.Method;
  *
  * @author Costin Leau
  * @author Chris Beams
+ * @author Juergen Hoeller
  * @since 3.1
  * @deprecated as of Spring 4.0, in favor of {@link SimpleKeyGenerator}
  * or custom {@link KeyGenerator} implementations based on hash codes
@@ -47,17 +49,19 @@ public class DefaultKeyGenerator implements KeyGenerator {
 
 	@Override
 	public Object generate(Object target, Method method, Object... params) {
-		if (params.length == 1) {
-			return (params[0] == null ? NULL_PARAM_KEY : params[0]);
-		}
 		if (params.length == 0) {
 			return NO_PARAM_KEY;
 		}
-		int hashCode = 17;
-		for (Object object : params) {
-			hashCode = 31 * hashCode + (object == null ? NULL_PARAM_KEY : object.hashCode());
+		if (params.length == 1) {
+			Object param = params[0];
+			if (param == null) {
+				return NULL_PARAM_KEY;
+			}
+			if (!param.getClass().isArray()) {
+				return param;
+			}
 		}
-		return Integer.valueOf(hashCode);
+		return Arrays.deepHashCode(params);
 	}
 
 }

spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.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.
@@ -29,6 +29,7 @@ import java.lang.reflect.Method;
  * implementations.
  *
  * @author Phillip Webb
+ * @author Juergen Hoeller
  * @since 4.0
  * @see SimpleKey
  * @see DefaultKeyGenerator
@@ -41,8 +42,11 @@ public class SimpleKeyGenerator implements KeyGenerator {
 		if (params.length == 0) {
 			return SimpleKey.EMPTY;
 		}
-		if (params.length == 1 && params[0] != null) {
-			return params[0];
+		if (params.length == 1) {
+			Object param = params[0];
+			if (param != null && !param.getClass().isArray()) {
+				return param;
+			}
 		}
 		return new SimpleKey(params);
 	}

spring-context/src/test/java/org/springframework/cache/interceptor/DefaultKeyGeneratorTests.java

@@ -0,0 +1,119 @@
+/*
+ * 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.interceptor;
+
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+/**
+ * Tests for {@link DefaultKeyGenerator}.
+ *
+ * @author Juergen Hoeller
+ * @author Stephane Nicoll
+ */
+public class DefaultKeyGeneratorTests {
+
+	private final DefaultKeyGenerator generator = new DefaultKeyGenerator();
+
+
+	@Test
+	public void noValues() {
+		Object k1 = generateKey(new Object[] {});
+		Object k2 = generateKey(new Object[] {});
+		Object k3 = generateKey(new Object[] { "different" });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+	}
+
+	@Test
+	public void singleValue(){
+		Object k1 = generateKey(new Object[] { "a" });
+		Object k2 = generateKey(new Object[] { "a" });
+		Object k3 = generateKey(new Object[] { "different" });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+		assertThat(k1, equalTo((Object) "a"));
+	}
+
+	@Test
+	public void multipleValues()  {
+		Object k1 = generateKey(new Object[] { "a", 1, "b" });
+		Object k2 = generateKey(new Object[] { "a", 1, "b" });
+		Object k3 = generateKey(new Object[] { "b", 1, "a" });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+	}
+
+	@Test
+	public void singleNullValue() {
+		Object k1 = generateKey(new Object[] { null });
+		Object k2 = generateKey(new Object[] { null });
+		Object k3 = generateKey(new Object[] { "different" });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+		assertThat(k1, instanceOf(Integer.class));
+	}
+
+	@Test
+	public void multipleNullValues() {
+		Object k1 = generateKey(new Object[] { "a", null, "b", null });
+		Object k2 = generateKey(new Object[] { "a", null, "b", null });
+		Object k3 = generateKey(new Object[] { "a", null, "b" });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+	}
+
+	@Test
+	public void plainArray() {
+		Object k1 = generateKey(new Object[] { new String[]{"a", "b"} });
+		Object k2 = generateKey(new Object[] { new String[]{"a", "b"} });
+		Object k3 = generateKey(new Object[] { new String[]{"b", "a"} });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+	}
+
+	@Test
+	public void arrayWithExtraParameter() {
+		Object k1 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
+		Object k2 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
+		Object k3 = generateKey(new Object[] { new String[]{"b", "a"}, "c" });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+	}
+
+
+	private Object generateKey(Object[] arguments) {
+		return this.generator.generate(null, null, arguments);
+	}
+
+}

spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.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.
@@ -29,7 +29,8 @@ import static org.junit.Assert.*;
  */
 public class SimpleKeyGeneratorTests {
 
-	private SimpleKeyGenerator generator = new SimpleKeyGenerator();
+	private final SimpleKeyGenerator generator = new SimpleKeyGenerator();
+
 
 	@Test
 	public void noValues() {
@@ -89,9 +90,20 @@ public class SimpleKeyGeneratorTests {
 	}
 
 	@Test
-	public void arrays() {
+	public void plainArray() {
+		Object k1 = generateKey(new Object[] { new String[]{"a", "b"} });
+		Object k2 = generateKey(new Object[] { new String[]{"a", "b"} });
+		Object k3 = generateKey(new Object[] { new String[]{"b", "a"} });
+		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
+		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
+		assertThat(k1, equalTo(k2));
+		assertThat(k1, not(equalTo(k3)));
+	}
+
+	@Test
+	public void arrayWithExtraParameter() {
 		Object k1 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
-		Object k2 = generateKey(new Object[] { new String[]{"a", "b"}, "c"  });
+		Object k2 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
 		Object k3 = generateKey(new Object[] { new String[]{"b", "a"}, "c" });
 		assertThat(k1.hashCode(), equalTo(k2.hashCode()));
 		assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
@@ -99,7 +111,9 @@ public class SimpleKeyGeneratorTests {
 		assertThat(k1, not(equalTo(k3)));
 	}
 
+
 	private Object generateKey(Object[] arguments) {
-		return generator.generate(null, null, arguments);
+		return this.generator.generate(null, null, arguments);
 	}
+
 }