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 22ac2170c..fc14fb16f 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -142,8 +142,9 @@ public class RedisCache extends AbstractValueAdaptingCache { */ public T get(final Object key, final Callable valueLoader) { - BinaryRedisCacheElement rce = new BinaryRedisCacheElement(new RedisCacheElement(new RedisCacheKey(key).usePrefix( - cacheMetadata.getKeyPrefix()).withKeySerializer(redisOperations.getKeySerializer()), valueLoader), + BinaryRedisCacheElement rce = new BinaryRedisCacheElement( + new RedisCacheElement(new RedisCacheKey(key).usePrefix(cacheMetadata.getKeyPrefix()) + .withKeySerializer(redisOperations.getKeySerializer()), new StoreTranslatingCallable(valueLoader)), cacheValueAccessor); ValueWrapper val = get(key); @@ -155,11 +156,10 @@ public class RedisCache extends AbstractValueAdaptingCache { try { byte[] result = (byte[]) redisOperations.execute(callback); - return (T) (result == null ? null : cacheValueAccessor.deserializeIfNecessary(result)); + return (T) (result == null ? null : fromStoreValue(cacheValueAccessor.deserializeIfNecessary(result))); } catch (RuntimeException e) { throw CacheValueRetrievalExceptionFactory.INSTANCE.create(key, valueLoader, e); } - } /** @@ -333,6 +333,27 @@ public class RedisCache extends AbstractValueAdaptingCache { return bytes == null ? null : cacheValueAccessor.deserializeIfNecessary(bytes); } + /** + * {@link Callable} to transform a value obtained from another {@link Callable} to its store value. + * + * @author Mark Paluch + * @since 1.8 + * @see #toStoreValue(Object) + */ + private class StoreTranslatingCallable implements Callable { + + private Callable valueLoader; + + public StoreTranslatingCallable(Callable valueLoader) { + this.valueLoader = valueLoader; + } + + @Override + public Object call() throws Exception { + return toStoreValue(valueLoader.call()); + } + } + /** * Metadata required to maintain {@link RedisCache}. Keeps track of additional data structures required for processing * cache operations. @@ -847,10 +868,14 @@ public class RedisCache extends AbstractValueAdaptingCache { } value = element.get(); - connection.set(element.getKeyBytes(), value); - processKeyExpiration(element, connection); - maintainKnownKeys(element, connection); + if (value.length == 0) { + connection.del(element.getKeyBytes()); + } else { + connection.set(element.getKeyBytes(), value); + processKeyExpiration(element, connection); + maintainKnownKeys(element, connection); + } if (!isClusterConnection(connection)) { connection.exec(); 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 d9ea0ca9e..09278043c 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java @@ -51,6 +51,7 @@ 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 org.springframework.data.redis.serializer.StringRedisSerializer; import edu.umd.cs.mtc.MultithreadedTestCase; @@ -381,6 +382,71 @@ public class RedisCacheTest extends AbstractNativeCacheTest { assertThat(cache.get(key, String.class), is(nullValue())); } + /** + * @see DATAREDIS-553 + */ + @Test + public void testCacheGetSynchronizedNullAllowingNull() { + + assumeThat(getAllowCacheNullValues(), is(true)); + assumeThat(cache, instanceOf(RedisCache.class)); + + Object key = getKey(); + Object value = cache.get(key, new Callable() { + @Override + public Object call() throws Exception { + return null; + } + }); + + assertThat(value, is(nullValue())); + assertThat(cache.get(key).get(), is(nullValue())); + } + + /** + * @see DATAREDIS-553 + */ + @Test + public void testCacheGetSynchronizedNullNotAllowingNull() { + + assumeThat(getAllowCacheNullValues(), is(false)); + assumeThat(cache, instanceOf(RedisCache.class)); + assumeThat(template.getValueSerializer(), not(instanceOf(StringRedisSerializer.class))); + + Object key = getKey(); + Object value = cache.get(key, new Callable() { + @Override + public Object call() throws Exception { + return null; + } + }); + + assertThat(value, is(nullValue())); + assertThat(cache.get(key), is(nullValue())); + } + + /** + * @see DATAREDIS-553 + */ + @Test + public void testCacheGetSynchronizedNullWithStoredNull() { + + assumeThat(getAllowCacheNullValues(), is(true)); + assumeThat(cache, instanceOf(RedisCache.class)); + + Object key = getKey(); + cache.put(key, null); + + Object cachedValue = cache.get(key, new Callable() { + @Override + public Object call() throws Exception { + return null; + } + }); + + assertThat(cachedValue, 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 f50e342a5..e8ac69a15 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java @@ -34,6 +34,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.cache.Cache; +import org.springframework.cache.support.NullValue; import org.springframework.data.redis.RedisSystemException; import org.springframework.data.redis.connection.RedisClusterConnection; import org.springframework.data.redis.connection.RedisConnection; @@ -138,7 +139,7 @@ public class RedisCacheUnitTests { * @see DATAREDIS-369 */ @Test - public void clearShouldCallLuaScritpToRemoveKeysWhenPrefixIsSet() { + public void clearShouldCallLuaScriptToRemoveKeysWhenPrefixIsSet() { cache = new RedisCache(CACHE_NAME, PREFIX_BYTES, templateSpy, EXPIRATION); cache.clear(); @@ -214,7 +215,7 @@ public class RedisCacheUnitTests { */ @Test @SuppressWarnings("unchecked") - public void getWithCallable() throws ClassNotFoundException, LinkageError { + public void getWithCallable() throws ClassNotFoundException { if (isPresent("org.springframework.cache.Cache$ValueRetrievalException", getDefaultClassLoader())) { exception.expect((Class) forName("org.springframework.cache.Cache$ValueRetrievalException", @@ -235,6 +236,51 @@ public class RedisCacheUnitTests { }); } + /** + * @see DATAREDIS-553 + */ + @Test + @SuppressWarnings("unchecked") + public void getWithCallableShouldStoreNullNotAllowingNull() throws ClassNotFoundException { + + cache = new RedisCache(CACHE_NAME, NO_PREFIX_BYTES, templateSpy, 0L, false); + + cache.get(KEY, new Callable() { + @Override + public Object call() throws Exception { + return null; + } + }); + + verify(connectionMock, times(1)).get(eq(KEY_BYTES)); + verify(connectionMock, times(1)).multi(); + verify(connectionMock, times(1)).del(eq(KEY_BYTES)); + verify(connectionMock, times(1)).exec(); + } + + /** + * @see DATAREDIS-553 + */ + @Test + @SuppressWarnings("unchecked") + public void getWithCallableShouldStoreNullAllowingNull() throws ClassNotFoundException { + + cache = new RedisCache(CACHE_NAME, NO_PREFIX_BYTES, templateSpy, 0L, true); + + cache.get(KEY, new Callable() { + @Override + public Object call() throws Exception { + return null; + } + }); + + verify(valueSerializerMock).serialize(isA(NullValue.class)); + 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(); + } + /** * @see DATAREDIS-443 */