diff --git a/org.springframework.context.support/src/main/java/org/springframework/cache/Cache.java b/org.springframework.context.support/src/main/java/org/springframework/cache/Cache.java index ba9ce6b4a0a..47bf595b3a0 100644 --- a/org.springframework.context.support/src/main/java/org/springframework/cache/Cache.java +++ b/org.springframework.context.support/src/main/java/org/springframework/cache/Cache.java @@ -18,7 +18,11 @@ package org.springframework.cache; /** - * Interface that defines the common cache operations. + * Interface that defines the common cache operations. + * + * Note: Due to the generic use of caching, it is recommended that + * implementations allow storage of null values (for example to + * cache methods that return null). * * @author Costin Leau */ diff --git a/org.springframework.context.support/src/main/java/org/springframework/cache/concurrent/ConcurrentCache.java b/org.springframework.context.support/src/main/java/org/springframework/cache/concurrent/ConcurrentCache.java index c853aba5b5b..2c2f4709c88 100644 --- a/org.springframework.context.support/src/main/java/org/springframework/cache/concurrent/ConcurrentCache.java +++ b/org.springframework.context.support/src/main/java/org/springframework/cache/concurrent/ConcurrentCache.java @@ -22,9 +22,9 @@ import java.util.concurrent.ConcurrentMap; import org.springframework.cache.Cache; import org.springframework.cache.support.AbstractDelegatingCache; -/** - * Simple {@link Cache} implementation based on the JDK 1.5+ java.util.concurrent package. - * Useful for testing or simple caching scenarios. +/** + * Simple {@link Cache} implementation based on the JDK 1.5+ + * java.util.concurrent package. Useful for testing or simple caching scenarios. * * @author Costin Leau */ @@ -42,7 +42,7 @@ public class ConcurrentCache extends AbstractDelegatingCache { } public ConcurrentCache(ConcurrentMap delegate, String name) { - super(delegate); + super(delegate, true); this.store = delegate; this.name = name; } @@ -55,19 +55,51 @@ public class ConcurrentCache extends AbstractDelegatingCache { return store; } + @SuppressWarnings("unchecked") public V putIfAbsent(K key, V value) { - return store.putIfAbsent(key, value); + if (getAllowNullValues()) { + if (value == null) { + ConcurrentMap raw = store; + return filterNull((V) raw.putIfAbsent(key, NULL_HOLDER)); + } + } + return filterNull(store.putIfAbsent(key, value)); } + @SuppressWarnings("unchecked") public boolean remove(Object key, Object value) { + if (getAllowNullValues()) { + if (value == null) { + ConcurrentMap raw = store; + return raw.remove(key, NULL_HOLDER); + } + } + return store.remove(key, value); } + @SuppressWarnings("unchecked") public boolean replace(K key, V oldValue, V newValue) { + if (getAllowNullValues()) { + Object rawOldValue = (oldValue == null ? NULL_HOLDER : oldValue); + Object rawNewValue = (newValue == null ? NULL_HOLDER : newValue); + + ConcurrentMap raw = store; + return raw.replace(key, rawOldValue, rawNewValue); + } + return store.replace(key, oldValue, newValue); } + @SuppressWarnings("unchecked") public V replace(K key, V value) { - return store.replace(key, value); + if (getAllowNullValues()) { + if (value == null) { + ConcurrentMap raw = store; + return filterNull((V) raw.replace(key, NULL_HOLDER)); + } + } + + return filterNull(store.replace(key, value)); } } \ No newline at end of file diff --git a/org.springframework.context.support/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/org.springframework.context.support/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index 2d6ad3f5a61..7b96f5269d0 100644 --- a/org.springframework.context.support/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/org.springframework.context.support/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -16,7 +16,6 @@ package org.springframework.cache.interceptor; -import java.io.Serializable; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; @@ -61,14 +60,6 @@ import org.springframework.util.StringUtils; */ public abstract class CacheAspectSupport implements InitializingBean { - private static class EmptyHolder implements Serializable { - - private static final long serialVersionUID = 1L; - } - - // TODO: can null values be properly stored into user caches? - private static final Object NULL_RETURN = new EmptyHolder(); - protected final Log logger = LogFactory.getLog(getClass()); private CacheManager cacheManager; @@ -124,7 +115,6 @@ public abstract class CacheAspectSupport implements InitializingBean { } protected Collection> getCaches(CacheDefinition definition) { - // TODO: add behaviour for the default cache Set cacheNames = definition.getCacheNames(); Collection> caches = new ArrayList>(cacheNames.size()); diff --git a/org.springframework.context.support/src/main/java/org/springframework/cache/support/AbstractDelegatingCache.java b/org.springframework.context.support/src/main/java/org/springframework/cache/support/AbstractDelegatingCache.java index 354798ea769..7d9d733af51 100644 --- a/org.springframework.context.support/src/main/java/org/springframework/cache/support/AbstractDelegatingCache.java +++ b/org.springframework.context.support/src/main/java/org/springframework/cache/support/AbstractDelegatingCache.java @@ -16,6 +16,7 @@ package org.springframework.cache.support; +import java.io.Serializable; import java.util.Map; import org.springframework.cache.Cache; @@ -25,15 +26,47 @@ import org.springframework.util.Assert; * Abstract base class delegating most of the {@link Map}-like methods * to the underlying cache. * + * Note:Allows null values to be stored, even if the underlying map + * does not support them. + * * @author Costin Leau */ public abstract class AbstractDelegatingCache implements Cache { - private final Map delegate; + private static class NullHolder implements Serializable { + private static final long serialVersionUID = 1L; + } + public static final Object NULL_HOLDER = new NullHolder(); + + private final Map delegate; + private final boolean allowNullValues; + + /** + * Creates a new instance using the given delegate. + * + * @param map type + * @param delegate map delegate + */ public > AbstractDelegatingCache(D delegate) { + this(delegate, false); + } + + /** + * Creates a new instance using the given delegate. + * + * @param map type + * @param delegate map delegate + * @param allowNullValues flag indicating whether null values are allowed or not + */ + public > AbstractDelegatingCache(D delegate, boolean allowNullValues) { Assert.notNull(delegate); this.delegate = delegate; + this.allowNullValues = allowNullValues; + } + + public boolean getAllowNullValues() { + return allowNullValues; } public void clear() { @@ -45,14 +78,31 @@ public abstract class AbstractDelegatingCache implements Cache { } public V get(Object key) { - return delegate.get(key); + return filterNull(delegate.get(key)); } + @SuppressWarnings("unchecked") public V put(K key, V value) { - return delegate.put(key, value); + if (allowNullValues && value == null) { + Map map = delegate; + Object val = map.put(key, NULL_HOLDER); + if (val == NULL_HOLDER) { + return null; + } + return (V) val; + } + + return filterNull(delegate.put(key, value)); } public V remove(Object key) { - return delegate.remove(key); + return filterNull(delegate.remove(key)); + } + + protected V filterNull(V val) { + if (allowNullValues && val == NULL_HOLDER) { + return null; + } + return val; } } \ No newline at end of file diff --git a/org.springframework.context.support/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java b/org.springframework.context.support/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java index f9a833e4a8e..f0956745162 100644 --- a/org.springframework.context.support/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java +++ b/org.springframework.context.support/src/test/java/org/springframework/cache/config/AbstractAnnotationTest.java @@ -16,8 +16,7 @@ package org.springframework.cache.config; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertSame; +import static org.junit.Assert.*; import org.junit.Before; import org.junit.Test; @@ -26,6 +25,7 @@ import org.springframework.context.support.ClassPathXmlApplicationContext; /** * Abstract annotation test (containing several reusable methods). + * * @author Costin Leau */ public abstract class AbstractAnnotationTest { @@ -72,7 +72,8 @@ public abstract class AbstractAnnotationTest { assertSame(r3, r4); } - public void testConditionalExpression(CacheableService service) throws Exception { + public void testConditionalExpression(CacheableService service) + throws Exception { Object r1 = service.conditional(4); Object r2 = service.conditional(4); @@ -96,6 +97,16 @@ public abstract class AbstractAnnotationTest { assertNotSame(r3, r4); } + public void testNullValue(CacheableService service) throws Exception { + Object key = new Object(); + assertNull(service.nullValue(key)); + int nr = service.nullInvocations().intValue(); + assertNull(service.nullValue(key)); + assertEquals(nr, service.nullInvocations().intValue()); + assertNull(service.nullValue(new Object())); + assertEquals(nr + 1, service.nullInvocations().intValue()); + } + @Test public void testCacheable() throws Exception { testCacheable(cs); @@ -126,4 +137,22 @@ public abstract class AbstractAnnotationTest { testInvalidate(ccs); } + @Test + public void testNullValue() throws Exception { + testNullValue(cs); + } + + @Test + public void testClassNullValue() throws Exception { + Object key = new Object(); + assertNull(ccs.nullValue(key)); + int nr = ccs.nullInvocations().intValue(); + assertNull(ccs.nullValue(key)); + assertEquals(nr, ccs.nullInvocations().intValue()); + assertNull(ccs.nullValue(new Object())); + // the check method is also cached + assertEquals(nr, ccs.nullInvocations().intValue()); + assertEquals(nr + 1, AnnotatedClassCacheableService.nullInvocations + .intValue()); + } } \ No newline at end of file diff --git a/org.springframework.context.support/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java b/org.springframework.context.support/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java index 431fada2201..16510428d6c 100644 --- a/org.springframework.context.support/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java +++ b/org.springframework.context.support/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java @@ -27,7 +27,8 @@ import org.springframework.cache.annotation.Cacheable; @Cacheable("default") public class AnnotatedClassCacheableService implements CacheableService { - private AtomicLong counter = new AtomicLong(); + private final AtomicLong counter = new AtomicLong(); + public static final AtomicLong nullInvocations = new AtomicLong(); public Object cache(Object arg1) { return counter.getAndIncrement(); @@ -45,4 +46,13 @@ public class AnnotatedClassCacheableService implements CacheableService { public Object key(Object arg1, Object arg2) { return counter.getAndIncrement(); } + + public Object nullValue(Object arg1) { + nullInvocations.incrementAndGet(); + return null; + } + + public Number nullInvocations() { + return nullInvocations.get(); + } } diff --git a/org.springframework.context.support/src/test/java/org/springframework/cache/config/CacheableService.java b/org.springframework.context.support/src/test/java/org/springframework/cache/config/CacheableService.java index 5ec9b0c155b..ebb8834da4e 100644 --- a/org.springframework.context.support/src/test/java/org/springframework/cache/config/CacheableService.java +++ b/org.springframework.context.support/src/test/java/org/springframework/cache/config/CacheableService.java @@ -16,6 +16,7 @@ package org.springframework.cache.config; + /** * Basic service interface. * @@ -31,4 +32,8 @@ public interface CacheableService { T key(Object arg1, Object arg2); + T nullValue(Object arg1); + + Number nullInvocations(); + } \ No newline at end of file diff --git a/org.springframework.context.support/src/test/java/org/springframework/cache/config/DefaultCacheableService.java b/org.springframework.context.support/src/test/java/org/springframework/cache/config/DefaultCacheableService.java index 5a1e1490579..903e07d76e6 100644 --- a/org.springframework.context.support/src/test/java/org/springframework/cache/config/DefaultCacheableService.java +++ b/org.springframework.context.support/src/test/java/org/springframework/cache/config/DefaultCacheableService.java @@ -21,7 +21,6 @@ import java.util.concurrent.atomic.AtomicLong; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; - /** * Simple cacheable service * @@ -29,7 +28,8 @@ import org.springframework.cache.annotation.Cacheable; */ public class DefaultCacheableService implements CacheableService { - private AtomicLong counter = new AtomicLong(); + private final AtomicLong counter = new AtomicLong(); + private final AtomicLong nullInvocations = new AtomicLong(); @Cacheable("default") public Long cache(Object arg1) { @@ -49,4 +49,14 @@ public class DefaultCacheableService implements CacheableService { public Long key(Object arg1, Object arg2) { return counter.getAndIncrement(); } + + @Cacheable("default") + public Long nullValue(Object arg1) { + nullInvocations.incrementAndGet(); + return null; + } + + public Number nullInvocations() { + return nullInvocations.get(); + } } \ No newline at end of file