diff --git a/src/main/asciidoc/reference/redis.adoc b/src/main/asciidoc/reference/redis.adoc index 56cdc8cc7..acdfd83c2 100644 --- a/src/main/asciidoc/reference/redis.adoc +++ b/src/main/asciidoc/reference/redis.adoc @@ -546,4 +546,5 @@ NOTE: By default `RedisCacheManager` will not participate in any ongoing transac NOTE: By default `RedisCacheManager` does not prefix keys for cache regions, which can lead to an unexpected growth of a `ZSET` used to maintain known keys. It's highly recommended to enable the usage of prefixes in order to avoid this unexpected growth and potential key clashes using more than one cache region. +NOTE: By default `RedisCache` will not cache any `null` values as keys without a value get dropped by Redis itself. However you can explicitly enable `null` value caching via `RedisCacheManager` which will store `org.springframework.cache.support.NullValue` as a placeholder. diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCache.java b/src/main/java/org/springframework/data/redis/cache/RedisCache.java index 935290040..22ac2170c 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -23,7 +23,8 @@ import java.util.Arrays; import java.util.Set; import java.util.concurrent.Callable; -import org.springframework.cache.Cache; +import org.springframework.cache.support.AbstractValueAdaptingCache; +import org.springframework.cache.support.NullValue; import org.springframework.cache.support.SimpleValueWrapper; import org.springframework.dao.DataAccessException; import org.springframework.data.redis.RedisSystemException; @@ -33,6 +34,10 @@ import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.ReturnType; import org.springframework.data.redis.core.RedisCallback; import org.springframework.data.redis.core.RedisOperations; +import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.Jackson2JsonRedisSerializer; +import org.springframework.data.redis.serializer.JacksonJsonRedisSerializer; +import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.springframework.util.ClassUtils; @@ -47,7 +52,7 @@ import org.springframework.util.ObjectUtils; * @author Mark Paluch */ @SuppressWarnings("unchecked") -public class RedisCache implements Cache { +public class RedisCache extends AbstractValueAdaptingCache { @SuppressWarnings("rawtypes")// private final RedisOperations redisOperations; @@ -64,13 +69,46 @@ public class RedisCache implements Cache { */ public RedisCache(String name, byte[] prefix, RedisOperations redisOperations, long expiration) { + this(name, prefix, redisOperations, expiration, false); + } + + /** + * Constructs a new RedisCache instance. + * + * @param name cache name + * @param prefix + * @param redisOperations + * @param expiration + * @param allowNullValues + * @since 1.8 + */ + public RedisCache(String name, byte[] prefix, RedisOperations redisOperations, + long expiration, boolean allowNullValues) { + + super(allowNullValues); hasText(name, "non-empty cache name is required"); this.cacheMetadata = new RedisCacheMetadata(name, prefix); this.cacheMetadata.setDefaultExpiration(expiration); this.redisOperations = redisOperations; - this.cacheValueAccessor = new CacheValueAccessor(redisOperations.getValueSerializer()); + + RedisSerializer serializer = redisOperations.getValueSerializer() != null ? redisOperations.getValueSerializer() + : (RedisSerializer) new JdkSerializationRedisSerializer(); + + this.cacheValueAccessor = new CacheValueAccessor(serializer); + + if (allowNullValues) { + + if (redisOperations.getValueSerializer() instanceof StringRedisSerializer + || redisOperations.getValueSerializer() instanceof GenericToStringSerializer + || redisOperations.getValueSerializer() instanceof JacksonJsonRedisSerializer + || redisOperations.getValueSerializer() instanceof Jackson2JsonRedisSerializer) { + throw new IllegalArgumentException(String.format( + "Redis does not allow keys with null value ¯\\_(ツ)_/¯. The chosen %s does not support generic type handling and therefore cannot be used with allowNullValues enabled. Please use a different RedisSerializer or disable null value support.", + ClassUtils.getShortName(redisOperations.getValueSerializer().getClass()))); + } + } } /** @@ -135,16 +173,19 @@ public class RedisCache implements Cache { notNull(cacheKey, "CacheKey must not be null!"); - byte[] bytes = (byte[]) redisOperations.execute(new AbstractRedisCacheCallback(new BinaryRedisCacheElement( - new RedisCacheElement(cacheKey, null), cacheValueAccessor), cacheMetadata) { + Boolean exists = (Boolean) redisOperations.execute(new RedisCallback() { @Override - public byte[] doInRedis(BinaryRedisCacheElement element, RedisConnection connection) throws DataAccessException { - return connection.get(element.getKeyBytes()); + public Boolean doInRedis(RedisConnection connection) throws DataAccessException { + return connection.exists(cacheKey.getKeyBytes()); } }); - return (bytes == null ? null : new RedisCacheElement(cacheKey, cacheValueAccessor.deserializeIfNecessary(bytes))); + if (!exists.booleanValue()) { + return null; + } + + return new RedisCacheElement(cacheKey, fromStoreValue(lookup(cacheKey))); } /* @@ -154,8 +195,24 @@ public class RedisCache implements Cache { @Override public void put(final Object key, final Object value) { - put(new RedisCacheElement(new RedisCacheKey(key).usePrefix(cacheMetadata.getKeyPrefix()).withKeySerializer( - redisOperations.getKeySerializer()), value).expireAfter(cacheMetadata.getDefaultExpiration())); + put(new RedisCacheElement(new RedisCacheKey(key).usePrefix(cacheMetadata.getKeyPrefix()) + .withKeySerializer(redisOperations.getKeySerializer()), toStoreValue(value)) + .expireAfter(cacheMetadata.getDefaultExpiration())); + } + + /* + * (non-Javadoc) + * @see org.springframework.cache.support.AbstractValueAdaptingCache#fromStoreValue(java.lang.Object) + */ + @Override + protected Object fromStoreValue(Object storeValue) { + + // we need this override for the GenericJackson2JsonRedisSerializer support. + if (isAllowNullValues() && storeValue instanceof NullValue) { + return null; + } + + return super.fromStoreValue(storeValue); } /** @@ -181,8 +238,8 @@ public class RedisCache implements Cache { public ValueWrapper putIfAbsent(Object key, final Object value) { return putIfAbsent(new RedisCacheElement(new RedisCacheKey(key).usePrefix(cacheMetadata.getKeyPrefix()) - .withKeySerializer(redisOperations.getKeySerializer()), value) - .expireAfter(cacheMetadata.getDefaultExpiration())); + .withKeySerializer(redisOperations.getKeySerializer()), toStoreValue(value)) + .expireAfter(cacheMetadata.getDefaultExpiration())); } /** @@ -253,6 +310,29 @@ public class RedisCache implements Cache { return (value != null ? new SimpleValueWrapper(value) : null); } + /* + * (non-Javadoc) + * @see org.springframework.cache.support.AbstractValueAdaptingCache#lookup(java.lang.Object) + */ + @Override + protected Object lookup(Object key) { + + RedisCacheKey cacheKey = key instanceof RedisCacheKey ? (RedisCacheKey) key + : new RedisCacheKey(key).usePrefix(this.cacheMetadata.getKeyPrefix()) + .withKeySerializer(redisOperations.getKeySerializer()); + + byte[] bytes = (byte[]) redisOperations.execute(new AbstractRedisCacheCallback( + new BinaryRedisCacheElement(new RedisCacheElement(cacheKey, null), cacheValueAccessor), cacheMetadata) { + + @Override + public byte[] doInRedis(BinaryRedisCacheElement element, RedisConnection connection) throws DataAccessException { + return connection.get(element.getKeyBytes()); + } + }); + + return bytes == null ? null : cacheValueAccessor.deserializeIfNecessary(bytes); + } + /** * Metadata required to maintain {@link RedisCache}. Keeps track of additional data structures required for processing * cache operations. diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java index 7f1b903f6..df5feaf5a 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java @@ -29,12 +29,14 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; +import org.springframework.cache.support.NullValue; import org.springframework.cache.transaction.AbstractTransactionSupportingCacheManager; import org.springframework.cache.transaction.TransactionAwareCacheDecorator; import org.springframework.dao.DataAccessException; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.core.RedisCallback; import org.springframework.data.redis.core.RedisOperations; +import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -70,6 +72,8 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager private Set configuredCacheNames; + private final boolean cacheNullValues; + /** * Construct a {@link RedisCacheManager}. * @@ -89,7 +93,25 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager */ @SuppressWarnings("rawtypes") public RedisCacheManager(RedisOperations redisOperations, Collection cacheNames) { + this(redisOperations, cacheNames, false); + } + + /** + * Construct a static {@link RedisCacheManager}, managing caches for the specified cache names only.
+ *
+ * NOTE When enabling {@code cacheNullValues} please make sure the {@link RedisSerializer} used by + * {@link RedisOperations} is capable of serializing {@link NullValue}. + * + * @param redisOperations {@link RedisOperations} to work upon. + * @param cacheNames {@link Collection} of known cache names. + * @param cacheNullValues set to {@literal true} to allow caching {@literal null}. + * @since 1.8 + */ + @SuppressWarnings("rawtypes") + public RedisCacheManager(RedisOperations redisOperations, Collection cacheNames, boolean cacheNullValues) { + this.redisOperations = redisOperations; + this.cacheNullValues = cacheNullValues; setCacheNames(cacheNames); } @@ -235,7 +257,8 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager @SuppressWarnings("unchecked") protected RedisCache createCache(String cacheName) { long expiration = computeExpiration(cacheName); - return new RedisCache(cacheName, (usePrefix ? cachePrefix.prefix(cacheName) : null), redisOperations, expiration); + return new RedisCache(cacheName, (usePrefix ? cachePrefix.prefix(cacheName) : null), redisOperations, expiration, + cacheNullValues); } protected long computeExpiration(String name) { diff --git a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java index 07f10b1ae..b2ac245ea 100644 --- a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java @@ -15,15 +15,22 @@ */ package org.springframework.data.redis.serializer; +import java.io.IOException; + +import org.springframework.cache.support.NullValue; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonTypeInfo.As; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.ser.SerializerFactory; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; /** * @author Christoph Strobl @@ -51,6 +58,10 @@ public class GenericJackson2JsonRedisSerializer implements RedisSerializer { + + private static final long serialVersionUID = 1999052150548658808L; + private final String classIdentifyer; + + /** + * @param classIdentifyer can be {@literal null} and will be defaulted to {@code @class}. + */ + NullValueSerializer(String classIdentifyer) { + + super(NullValue.class); + this.classIdentifyer = StringUtils.hasText(classIdentifyer) ? classIdentifyer : "@class"; + } + + /* + * (non-Javadoc) + * @see com.fasterxml.jackson.databind.ser.std.StdSerializer#serialize(java.lang.Object, com.fasterxml.jackson.core.JsonGenerator, com.fasterxml.jackson.databind.SerializerProvider) + */ + @Override + public void serialize(NullValue value, JsonGenerator jgen, SerializerProvider provider) + throws IOException, JsonProcessingException { + + jgen.writeStartObject(); + jgen.writeStringField(classIdentifyer, NullValue.class.getName()); + jgen.writeEndObject(); + } + + } } diff --git a/src/test/java/org/springframework/data/redis/cache/AbstractNativeCacheTest.java b/src/test/java/org/springframework/data/redis/cache/AbstractNativeCacheTest.java index 184843fc7..57a73db26 100644 --- a/src/test/java/org/springframework/data/redis/cache/AbstractNativeCacheTest.java +++ b/src/test/java/org/springframework/data/redis/cache/AbstractNativeCacheTest.java @@ -16,12 +16,8 @@ package org.springframework.data.redis.cache; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThat; -import static org.springframework.data.redis.matcher.RedisTestMatchers.isEqual; +import static org.junit.Assert.*; +import static org.springframework.data.redis.matcher.RedisTestMatchers.*; import org.junit.Before; import org.junit.Test; @@ -38,22 +34,31 @@ public abstract class AbstractNativeCacheTest { private T nativeCache; protected Cache cache; protected final static String CACHE_NAME = "testCache"; + private final boolean allowCacheNullValues; + + protected AbstractNativeCacheTest(boolean allowCacheNullValues) { + this.allowCacheNullValues = allowCacheNullValues; + } @Before public void setUp() throws Exception { nativeCache = createNativeCache(); - cache = createCache(nativeCache); + cache = createCache(nativeCache, allowCacheNullValues); cache.clear(); } protected abstract T createNativeCache() throws Exception; - protected abstract Cache createCache(T nativeCache); + protected abstract Cache createCache(T nativeCache, boolean allowCacheNullValues); protected abstract Object getKey(); protected abstract Object getValue(); + protected boolean getAllowCacheNullValues() { + return allowCacheNullValues; + } + @Test public void testCacheName() throws Exception { assertEquals(CACHE_NAME, cache.getName()); diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java index 5c6dffed3..d9ea0ca9e 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java @@ -27,6 +27,8 @@ import static org.junit.Assert.*; import static org.junit.Assume.*; import static org.springframework.data.redis.matcher.RedisTestMatchers.*; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; @@ -34,6 +36,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.junit.AfterClass; +import org.junit.AssumptionViolatedException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -46,6 +49,8 @@ import org.springframework.data.redis.ObjectFactory; import org.springframework.data.redis.StringObjectFactory; import org.springframework.data.redis.core.AbstractOperationsTestParams; import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.serializer.GenericJackson2JsonRedisSerializer; +import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer; import edu.umd.cs.mtc.MultithreadedTestCase; @@ -63,7 +68,11 @@ public class RedisCacheTest extends AbstractNativeCacheTest { private ObjectFactory valueFactory; private RedisTemplate template; - public RedisCacheTest(RedisTemplate template, ObjectFactory keyFactory, ObjectFactory valueFactory) { + public RedisCacheTest(RedisTemplate template, ObjectFactory keyFactory, ObjectFactory valueFactory, + boolean allowCacheNullValues) { + + super(allowCacheNullValues); + this.keyFactory = keyFactory; this.valueFactory = valueFactory; this.template = template; @@ -72,12 +81,30 @@ public class RedisCacheTest extends AbstractNativeCacheTest { @Parameters public static Collection testParams() { - return AbstractOperationsTestParams.testParams(); + + Collection params = AbstractOperationsTestParams.testParams(); + + Collection target = new ArrayList(); + for (Object[] source : params) { + + Object[] cacheNullDisabled = Arrays.copyOf(source, source.length + 1); + Object[] cacheNullEnabled = Arrays.copyOf(source, source.length + 1); + + cacheNullDisabled[source.length] = false; + cacheNullEnabled[source.length] = true; + + target.add(cacheNullDisabled); + target.add(cacheNullEnabled); + } + + return target; } @SuppressWarnings("unchecked") - protected RedisCache createCache(RedisTemplate nativeCache) { - return new RedisCache(CACHE_NAME, CACHE_NAME.concat(":").getBytes(), nativeCache, TimeUnit.MINUTES.toSeconds(10)); + protected RedisCache createCache(RedisTemplate nativeCache, boolean allowCacheNullValues) { + + return new RedisCache(CACHE_NAME, CACHE_NAME.concat(":").getBytes(), nativeCache, TimeUnit.MINUTES.toSeconds(10), + allowCacheNullValues); } protected RedisTemplate createNativeCache() throws Exception { @@ -86,6 +113,13 @@ public class RedisCacheTest extends AbstractNativeCacheTest { @Before public void setUp() throws Exception { + + if (!(template.getValueSerializer() instanceof JdkSerializationRedisSerializer + || template.getValueSerializer() instanceof GenericJackson2JsonRedisSerializer + || template.getValueSerializer() == null) && getAllowCacheNullValues()) { + throw new AssumptionViolatedException( + "Null values can only be cachend with the Jdk or GenericJackson2 serialization"); + } ConnectionFactoryTracker.add(template.getConnectionFactory()); super.setUp(); } @@ -288,6 +322,8 @@ public class RedisCacheTest extends AbstractNativeCacheTest { @Test public void cachePutWithNullShouldNotAddStuffToRedis() { + assumeThat(getAllowCacheNullValues(), is(false)); + Object key = getKey(); Object value = getValue(); @@ -302,6 +338,8 @@ public class RedisCacheTest extends AbstractNativeCacheTest { @Test public void cachePutWithNullShouldRemoveKeyIfExists() { + assumeThat(getAllowCacheNullValues(), is(false)); + Object key = getKey(); Object value = getValue(); @@ -327,6 +365,22 @@ public class RedisCacheTest extends AbstractNativeCacheTest { runOnce(new CacheGetWithValueLoaderIsThreadSafe((RedisCache) cache)); } + /** + * @see DATAREDIS-553 + */ + @Test + public void cachePutWithNullShouldAddStuffToRedisWhenCachingNullIsEnabled() { + + assumeThat(getAllowCacheNullValues(), is(true)); + + Object key = getKey(); + Object value = getValue(); + + cache.put(key, null); + + assertThat(cache.get(key, String.class), is(nullValue())); + } + @SuppressWarnings("unused") private static class CacheGetWithValueLoaderIsThreadSafe extends MultithreadedTestCase { diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java index bb5dace38..f50e342a5 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java @@ -250,7 +250,7 @@ public class RedisCacheUnitTests { } }); - verify(connectionMock, times(2)).get(eq(KEY_BYTES)); + verify(connectionMock, times(1)).get(eq(KEY_BYTES)); verify(connectionMock, times(1)).multi(); verify(connectionMock, times(1)).set(eq(KEY_BYTES), eq(VALUE_BYTES)); verify(connectionMock, times(1)).exec(); @@ -267,7 +267,7 @@ public class RedisCacheUnitTests { Callable callableMock = mock(Callable.class); when(connectionMock.exists(KEY_BYTES)).thenReturn(true); - when(connectionMock.get(KEY_BYTES)).thenReturn(null).thenReturn(VALUE_BYTES); + when(connectionMock.get(KEY_BYTES)).thenReturn(VALUE_BYTES); assertThat((String) cache.get(KEY, callableMock), equalTo(VALUE)); verifyZeroInteractions(callableMock); @@ -313,7 +313,7 @@ public class RedisCacheUnitTests { } }); - verify(clusterConnectionMock, times(2)).get(eq(KEY_BYTES)); + verify(clusterConnectionMock, times(1)).get(eq(KEY_BYTES)); verify(clusterConnectionMock, times(1)).set(eq(KEY_BYTES), eq(VALUE_BYTES)); verify(clusterConnectionMock, never()).multi(); diff --git a/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java b/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java index e61651d06..c00b3f582 100644 --- a/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java @@ -16,9 +16,11 @@ package org.springframework.data.redis.serializer; import static org.hamcrest.core.Is.*; +import static org.hamcrest.core.IsInstanceOf.*; import static org.hamcrest.core.IsNull.*; import static org.junit.Assert.*; import static org.mockito.Matchers.*; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; import static org.springframework.test.util.ReflectionTestUtils.*; import static org.springframework.util.ObjectUtils.*; @@ -26,6 +28,8 @@ import static org.springframework.util.ObjectUtils.*; import java.io.IOException; import org.junit.Test; +import org.springframework.beans.BeanUtils; +import org.springframework.cache.support.NullValue; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.core.JsonGenerationException; @@ -152,6 +156,23 @@ public class GenericJackson2JsonRedisSerializerUnitTests { new GenericJackson2JsonRedisSerializer(objectMapperMock).deserialize(new byte[] { 1 }); } + /** + * @see DATAREDIS-553 + */ + @Test + public void shouldSerializeNullValueSoThatItCanBeDeserializedWithDefaultTypingEnabled() { + + GenericJackson2JsonRedisSerializer serializer = new GenericJackson2JsonRedisSerializer(); + + NullValue nv = BeanUtils.instantiateClass(NullValue.class); + + byte[] serializedValue = serializer.serialize(nv); + assertThat(serializedValue, is(notNullValue())); + + Object deserializedValue = serializer.deserialize(serializedValue); + assertThat(deserializedValue, is(instanceOf(NullValue.class))); + } + private TypeResolverBuilder extractTypeResolver(GenericJackson2JsonRedisSerializer serializer) { ObjectMapper mapper = (ObjectMapper) getField(serializer, "mapper");