From f1a18d29ba83345b91439da7e884ff47a9f037b0 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 21 Jun 2013 12:21:29 -0700 Subject: [PATCH] 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 --- .../cache/annotation/EnableCaching.java | 4 +- .../cache/interceptor/CacheAspectSupport.java | 4 +- .../interceptor/DefaultKeyGenerator.java | 8 ++ .../cache/interceptor/SimpleKey.java | 72 +++++++++++++++ .../cache/interceptor/SimpleKeyGenerator.java | 50 +++++++++++ .../interceptor/SimpleKeyGeneratorTests.java | 89 +++++++++++++++++++ 6 files changed, 223 insertions(+), 4 deletions(-) create mode 100644 spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKey.java create mode 100644 spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java create mode 100644 spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java diff --git a/spring-context/src/main/java/org/springframework/cache/annotation/EnableCaching.java b/spring-context/src/main/java/org/springframework/cache/annotation/EnableCaching.java index 771487a56ff..2a477b9e8b5 100644 --- a/spring-context/src/main/java/org/springframework/cache/annotation/EnableCaching.java +++ b/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. * diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index 8f8fc76fd2c..d910623a630 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/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; 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 a37218804a8..8759e4586f1 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 @@ -27,9 +27,17 @@ import org.springframework.cache.interceptor.KeyGenerator; * Uses the constant value {@value #NULL_PARAM_KEY} for any * {@code null} parameters given. * + *

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 { diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKey.java b/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKey.java new file mode 100644 index 00000000000..cc3b0e59048 --- /dev/null +++ b/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) + "]"; + } +} diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java b/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java new file mode 100644 index 00000000000..7d6fc6f16d3 --- /dev/null +++ b/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. + * + *

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); + } + +} diff --git a/spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java b/spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java new file mode 100644 index 00000000000..2d5305dd610 --- /dev/null +++ b/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))); + } +}