From 70155e9ff91a712e5df2b95de2d05fdf1022b54a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 5 Mar 2014 17:37:09 +0100 Subject: [PATCH] KeyGenerators should not return a plain array parameter as raw key but rather always handle that case in a deepHashCode fashion Issue: SPR-11505 (cherry picked from commit e50cff4) --- .../interceptor/DefaultKeyGenerator.java | 23 ++-- .../interceptor/DefaultKeyGeneratorTests.java | 119 ++++++++++++++++++ 2 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/cache/interceptor/DefaultKeyGeneratorTests.java diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/DefaultKeyGenerator.java b/spring-context/src/main/java/org/springframework/cache/interceptor/DefaultKeyGenerator.java index cb0dcc95d8c..ed6c0e96cbf 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/DefaultKeyGenerator.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/DefaultKeyGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 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,8 +17,7 @@ package org.springframework.cache.interceptor; import java.lang.reflect.Method; - -import org.springframework.cache.interceptor.KeyGenerator; +import java.util.Arrays; /** * Default key generator. Returns {@value #NO_PARAM_KEY} if no @@ -29,25 +28,29 @@ import org.springframework.cache.interceptor.KeyGenerator; * * @author Costin Leau * @author Chris Beams + * @author Juergen Hoeller * @since 3.1 */ public class DefaultKeyGenerator implements KeyGenerator { public static final int NO_PARAM_KEY = 0; + public static final int NULL_PARAM_KEY = 53; 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); } } diff --git a/spring-context/src/test/java/org/springframework/cache/interceptor/DefaultKeyGeneratorTests.java b/spring-context/src/test/java/org/springframework/cache/interceptor/DefaultKeyGeneratorTests.java new file mode 100644 index 00000000000..3376a617dcf --- /dev/null +++ b/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); + } + +}