SimpleKeyGenerator to replace DefaultKeyGenerator

Introduce new SimpleKeyGenerator class to supersede DefaultKeyGenerator.
Unlike DefaultKeyGenerator, no collisions will occur with the keys
generated by SimpleKeyGenerator as the full parameter values are
considered within the SimpleKey.equals(...) method.

The SimpleKeyGenerator is now the default class used when no explicit
generator is configured.

Issue: SPR-10237

spring-context/src/main/java/org/springframework/cache/annotation/EnableCaching.java

@@ -120,9 +120,9 @@ import org.springframework.core.Ordered;
  * customizing the strategy for cache key generation, per Spring's {@link
  * org.springframework.cache.interceptor.KeyGenerator KeyGenerator} SPI. Normally,
  * {@code @EnableCaching} will configure Spring's
- * {@link org.springframework.cache.interceptor.DefaultKeyGenerator DefaultKeyGenerator}
+ * {@link org.springframework.cache.interceptor.SimpleKeyGenerator SimpleKeyGenerator}
  * for this purpose, but when implementing {@code CachingConfigurer}, a key generator
- * must be provided explicitly. Return {@code new DefaultKeyGenerator()} from this method
+ * must be provided explicitly. Return {@code new SimpleKeyGenerator()} from this method
  * if no customization is necessary. See {@link CachingConfigurer} Javadoc for further
  * details.
  *

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

@@ -73,7 +73,7 @@ public abstract class CacheAspectSupport implements InitializingBean {
 
 	private final ExpressionEvaluator evaluator = new ExpressionEvaluator();
 
-	private KeyGenerator keyGenerator = new DefaultKeyGenerator();
+	private KeyGenerator keyGenerator = new SimpleKeyGenerator();
 
 	private boolean initialized = false;
 
@@ -116,7 +116,7 @@ public abstract class CacheAspectSupport implements InitializingBean {
 
 	/**
 	 * Set the KeyGenerator for this cache aspect.
-	 * Default is {@link DefaultKeyGenerator}.
+	 * Default is {@link SimpleKeyGenerator}.
 	 */
 	public void setKeyGenerator(KeyGenerator keyGenerator) {
 		this.keyGenerator = keyGenerator;

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

@@ -27,9 +27,17 @@ import org.springframework.cache.interceptor.KeyGenerator;
  * Uses the constant value {@value #NULL_PARAM_KEY} for any
  * {@code null} parameters given.
  *
+ * <p>NOTE: As this implementation returns only a hash of the parameters
+ * it is possible for key collisions to occur. Since Spring 4.0 the
+ * {@link SimpleKeyGenerator} is used when no explicit key generator
+ * has been defined. This class remains for applications that do not
+ * wish to migrate to the {@link SimpleKeyGenerator}.
+ *
  * @author Costin Leau
  * @author Chris Beams
  * @since 3.1
+ * @see SimpleKeyGenerator
+ * @see org.springframework.cache.annotation.CachingConfigurer
  */
 public class DefaultKeyGenerator implements KeyGenerator {
 

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

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2002-2013 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 java.io.Serializable;
+import java.util.Arrays;
+
+import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
+
+/**
+ * A simple key as returned from the {@link SimpleKeyGenerator}.
+ *
+ * @author Phillip Webb
+ * @see SimpleKeyGenerator
+ */
+public final class SimpleKey implements Serializable {
+
+	private static final long serialVersionUID = 1;
+
+	public static final SimpleKey EMPTY = new SimpleKey(new Object[] {});
+
+
+	private final Object[] params;
+
+
+	/**
+	 * Create a new {@link SimpleKey} instance.
+	 * @param elements the elements of the key
+	 */
+	public SimpleKey(Object[] elements) {
+		Assert.notNull(elements, "Elements must not be null");
+		this.params = new Object[elements.length];
+		System.arraycopy(elements, 0, this.params, 0, elements.length);
+	}
+
+
+	@Override
+	public boolean equals(Object obj) {
+		if (obj == this) {
+			return true;
+		}
+		if (obj != null && getClass() == obj.getClass()) {
+			return Arrays.equals(this.params, ((SimpleKey) obj).params);
+		}
+		return false;
+	}
+
+	@Override
+	public int hashCode() {
+		return Arrays.hashCode(params);
+	}
+
+	@Override
+	public String toString() {
+		return "SimpleKey [" + StringUtils.arrayToCommaDelimitedString(this.params) + "]";
+	}
+}

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

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2002-2013 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 java.lang.reflect.Method;
+
+/**
+ * Simple key generator. Returns the parameter itself if a single non-null value
+ * is given, otherwise returns a {@link SimpleKey} of the parameters.
+ *
+ * <p>Unlike {@link DefaultKeyGenerator}, no collisions will occur with the keys
+ * generated by this class. The returned {@link SimpleKey} object can be safely
+ * used with a {@link org.springframework.cache.concurrent.ConcurrentMapCache},
+ * however, might not be suitable for all {@link org.springframework.cache.Cache}
+ * implementations.
+ *
+ * @author Phillip Webb
+ * @since 4.0
+ * @see SimpleKey
+ * @see DefaultKeyGenerator
+ * @see org.springframework.cache.annotation.CachingConfigurer
+ */
+public class SimpleKeyGenerator implements KeyGenerator {
+
+	@Override
+	public Object generate(Object target, Method method, Object... params) {
+		if(params.length == 0) {
+			return SimpleKey.EMPTY;
+		}
+		if(params.length == 1 && params[0] != null) {
+			return params[0];
+		}
+		return new SimpleKey(params);
+	}
+
+}

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

@@ -0,0 +1,89 @@
+/*
+ * Copyright 2002-2013 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 SimpleKeyGenerator} and {@link SimpleKey}.
+ *
+ * @author Phillip Webb
+ */
+public class SimpleKeyGeneratorTests {
+
+	private SimpleKeyGenerator generator = new SimpleKeyGenerator();
+
+	@Test
+	public void noValues() throws Exception {
+		Object k1 = generator.generate(null, null, new Object[] {});
+		Object k2 = generator.generate(null, null, new Object[] {});
+		Object k3 = generator.generate(null, null, 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() throws Exception {
+		Object k1 = generator.generate(null, null, new Object[] { "a" });
+		Object k2 = generator.generate(null, null, new Object[] { "a" });
+		Object k3 = generator.generate(null, null, 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() throws Exception {
+		Object k1 = generator.generate(null, null, new Object[] { "a", 1, "b" });
+		Object k2 = generator.generate(null, null, new Object[] { "a", 1, "b" });
+		Object k3 = generator.generate(null, null, 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() throws Exception {
+		Object k1 = generator.generate(null, null, new Object[] { null });
+		Object k2 = generator.generate(null, null, new Object[] { null });
+		Object k3 = generator.generate(null, null, 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(SimpleKey.class));
+	}
+
+	@Test
+	public void multiplNullValues() throws Exception {
+		Object k1 = generator.generate(null, null, new Object[] { "a", null, "b", null });
+		Object k2 = generator.generate(null, null, new Object[] { "a", null, "b", null });
+		Object k3 = generator.generate(null, null, 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)));
+	}
+}