Skip to content

Commit e50cff4

Browse files
jhoellerjhoeller
committed
KeyGenerators should not return a plain array parameter as raw key but rather always handle that case in a deepHashCode fashion
Issue: SPR-11505
1 parent a434903 commit e50cff4

File tree

4 files changed

+157
-16
lines changed

4 files changed

+157
-16
lines changed

‎spring-context/src/main/java/org/springframework/cache/interceptor/DefaultKeyGenerator.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package org.springframework.cache.interceptor;
1818

1919
import java.lang.reflect.Method;
20+
import java.util.Arrays;
2021

2122
/**
2223
* Default key generator. Returns {@value #NO_PARAM_KEY} if no
@@ -33,6 +34,7 @@
3334
*
3435
* @author Costin Leau
3536
* @author Chris Beams
37+
* @author Juergen Hoeller
3638
* @since 3.1
3739
* @deprecated as of Spring 4.0, in favor of {@link SimpleKeyGenerator}
3840
* or custom {@link KeyGenerator} implementations based on hash codes
@@ -47,17 +49,19 @@ public class DefaultKeyGenerator implements KeyGenerator {
4749

4850
@Override
4951
public Object generate(Object target, Method method, Object... params) {
50-
if (params.length == 1) {
51-
return (params[0] == null ? NULL_PARAM_KEY : params[0]);
52-
}
5352
if (params.length == 0) {
5453
return NO_PARAM_KEY;
5554
}
56-
int hashCode = 17;
57-
for (Object object : params) {
58-
hashCode = 31 * hashCode + (object == null ? NULL_PARAM_KEY : object.hashCode());
55+
if (params.length == 1) {
56+
Object param = params[0];
57+
if (param == null) {
58+
return NULL_PARAM_KEY;
59+
}
60+
if (!param.getClass().isArray()) {
61+
return param;
62+
}
5963
}
60-
return Integer.valueOf(hashCode);
64+
return Arrays.deepHashCode(params);
6165
}
6266

6367
}

‎spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
2929
* implementations.
3030
*
3131
* @author Phillip Webb
32+
* @author Juergen Hoeller
3233
* @since 4.0
3334
* @see SimpleKey
3435
* @see DefaultKeyGenerator
@@ -41,8 +42,11 @@ public Object generate(Object target, Method method, Object... params) {
4142
if (params.length == 0) {
4243
return SimpleKey.EMPTY;
4344
}
44-
if (params.length == 1 && params[0] != null) {
45-
return params[0];
45+
if (params.length == 1) {
46+
Object param = params[0];
47+
if (param != null && !param.getClass().isArray()) {
48+
return param;
49+
}
4650
}
4751
return new SimpleKey(params);
4852
}

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* Copyright 2002-2014 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cache.interceptor;
18+
19+
import org.junit.Test;
20+
21+
import static org.hamcrest.Matchers.*;
22+
import static org.junit.Assert.*;
23+
24+
/**
25+
* Tests for {@link DefaultKeyGenerator}.
26+
*
27+
* @author Juergen Hoeller
28+
* @author Stephane Nicoll
29+
*/
30+
public class DefaultKeyGeneratorTests {
31+
32+
private final DefaultKeyGenerator generator = new DefaultKeyGenerator();
33+
34+
35+
@Test
36+
public void noValues() {
37+
Object k1 = generateKey(new Object[] {});
38+
Object k2 = generateKey(new Object[] {});
39+
Object k3 = generateKey(new Object[] { "different" });
40+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
41+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
42+
assertThat(k1, equalTo(k2));
43+
assertThat(k1, not(equalTo(k3)));
44+
}
45+
46+
@Test
47+
public void singleValue(){
48+
Object k1 = generateKey(new Object[] { "a" });
49+
Object k2 = generateKey(new Object[] { "a" });
50+
Object k3 = generateKey(new Object[] { "different" });
51+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
52+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
53+
assertThat(k1, equalTo(k2));
54+
assertThat(k1, not(equalTo(k3)));
55+
assertThat(k1, equalTo((Object) "a"));
56+
}
57+
58+
@Test
59+
public void multipleValues() {
60+
Object k1 = generateKey(new Object[] { "a", 1, "b" });
61+
Object k2 = generateKey(new Object[] { "a", 1, "b" });
62+
Object k3 = generateKey(new Object[] { "b", 1, "a" });
63+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
64+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
65+
assertThat(k1, equalTo(k2));
66+
assertThat(k1, not(equalTo(k3)));
67+
}
68+
69+
@Test
70+
public void singleNullValue() {
71+
Object k1 = generateKey(new Object[] { null });
72+
Object k2 = generateKey(new Object[] { null });
73+
Object k3 = generateKey(new Object[] { "different" });
74+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
75+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
76+
assertThat(k1, equalTo(k2));
77+
assertThat(k1, not(equalTo(k3)));
78+
assertThat(k1, instanceOf(Integer.class));
79+
}
80+
81+
@Test
82+
public void multipleNullValues() {
83+
Object k1 = generateKey(new Object[] { "a", null, "b", null });
84+
Object k2 = generateKey(new Object[] { "a", null, "b", null });
85+
Object k3 = generateKey(new Object[] { "a", null, "b" });
86+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
87+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
88+
assertThat(k1, equalTo(k2));
89+
assertThat(k1, not(equalTo(k3)));
90+
}
91+
92+
@Test
93+
public void plainArray() {
94+
Object k1 = generateKey(new Object[] { new String[]{"a", "b"} });
95+
Object k2 = generateKey(new Object[] { new String[]{"a", "b"} });
96+
Object k3 = generateKey(new Object[] { new String[]{"b", "a"} });
97+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
98+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
99+
assertThat(k1, equalTo(k2));
100+
assertThat(k1, not(equalTo(k3)));
101+
}
102+
103+
@Test
104+
public void arrayWithExtraParameter() {
105+
Object k1 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
106+
Object k2 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
107+
Object k3 = generateKey(new Object[] { new String[]{"b", "a"}, "c" });
108+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
109+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
110+
assertThat(k1, equalTo(k2));
111+
assertThat(k1, not(equalTo(k3)));
112+
}
113+
114+
115+
private Object generateKey(Object[] arguments) {
116+
return this.generator.generate(null, null, arguments);
117+
}
118+
119+
}

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,7 +29,8 @@
2929
*/
3030
public class SimpleKeyGeneratorTests {
3131

32-
private SimpleKeyGenerator generator = new SimpleKeyGenerator();
32+
private final SimpleKeyGenerator generator = new SimpleKeyGenerator();
33+
3334

3435
@Test
3536
public void noValues() {
@@ -89,17 +90,30 @@ public void multipleNullValues() {
8990
}
9091

9192
@Test
92-
public void arrays() {
93+
public void plainArray() {
94+
Object k1 = generateKey(new Object[] { new String[]{"a", "b"} });
95+
Object k2 = generateKey(new Object[] { new String[]{"a", "b"} });
96+
Object k3 = generateKey(new Object[] { new String[]{"b", "a"} });
97+
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
98+
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
99+
assertThat(k1, equalTo(k2));
100+
assertThat(k1, not(equalTo(k3)));
101+
}
102+
103+
@Test
104+
public void arrayWithExtraParameter() {
93105
Object k1 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
94-
Object k2 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
106+
Object k2 = generateKey(new Object[] { new String[]{"a", "b"}, "c" });
95107
Object k3 = generateKey(new Object[] { new String[]{"b", "a"}, "c" });
96108
assertThat(k1.hashCode(), equalTo(k2.hashCode()));
97109
assertThat(k1.hashCode(), not(equalTo(k3.hashCode())));
98110
assertThat(k1, equalTo(k2));
99111
assertThat(k1, not(equalTo(k3)));
100112
}
101113

114+
102115
private Object generateKey(Object[] arguments) {
103-
return generator.generate(null, null, arguments);
116+
return this.generator.generate(null, null, arguments);
104117
}
118+
105119
}

0 commit comments

Comments
 (0)