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 76071ae80..a34c34b30 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -17,7 +17,6 @@ package org.springframework.data.redis.cache; import static org.springframework.util.Assert.*; -import static org.springframework.util.ObjectUtils.*; import java.lang.reflect.Constructor; import java.util.Arrays; @@ -37,6 +36,7 @@ import org.springframework.data.redis.core.RedisOperations; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; /** * Cache implementation on top of Redis. @@ -720,20 +720,27 @@ public class RedisCache implements Cache { public byte[] doInRedis(BinaryRedisCacheElement element, RedisConnection connection) throws DataAccessException { waitForLock(connection); - byte[] resultValue = put(element, connection); - if (nullSafeEquals(element.get(), resultValue)) { + byte existingValue[] = null; + + boolean keyMaintenance; + + byte[] keyBytes = element.getKeyBytes(); + byte[] value = element.get(); + + if (connection.setNX(keyBytes, value)) { + keyMaintenance = true; + } else { + existingValue = connection.get(keyBytes); + keyMaintenance = ObjectUtils.nullSafeEquals(value, existingValue); + } + + if (keyMaintenance) { processKeyExpiration(element, connection); maintainKnownKeys(element, connection); } - return resultValue; - } - - private byte[] put(BinaryRedisCacheElement element, RedisConnection connection) { - - boolean valueWasSet = connection.setNX(element.getKeyBytes(), element.get()); - return valueWasSet ? null : connection.get(element.getKeyBytes()); + return existingValue; } } 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 5a9b7338b..8f1df5421 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java @@ -15,6 +15,9 @@ */ package org.springframework.data.redis.cache; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.IsEqual.*; import static org.junit.Assert.*; import static org.mockito.Matchers.*; @@ -30,6 +33,7 @@ import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cache.Cache; import org.springframework.data.redis.RedisSystemException; import org.springframework.data.redis.connection.RedisClusterConnection; import org.springframework.data.redis.connection.RedisConnection; @@ -40,6 +44,7 @@ import org.springframework.data.redis.serializer.RedisSerializer; /** * @author Christoph Strobl + * @author Mark Paluch */ @SuppressWarnings("rawtypes") @RunWith(MockitoJUnitRunner.class) @@ -154,6 +159,56 @@ public class RedisCacheUnitTests { verify(connectionMock, never()).expire(eq(KNOWN_KEYS_SET_NAME_BYTES), anyLong()); } + /** + * @see DATAREDIS-542 + */ + @Test + public void putIfAbsentShouldExpireWhenValueWasSet() { + + when(connectionMock.setNX(KEY_BYTES, VALUE_BYTES)).thenReturn(true); + + cache = new RedisCache(CACHE_NAME, NO_PREFIX_BYTES, templateSpy, 10L); + Cache.ValueWrapper valueWrapper = cache.putIfAbsent(KEY, VALUE); + + assertThat(valueWrapper, is(nullValue())); + verify(connectionMock).setNX(KEY_BYTES, VALUE_BYTES); + verify(connectionMock).expire(eq(KEY_BYTES), anyLong()); + } + + /** + * @see DATAREDIS-542 + */ + @Test + public void putIfAbsentShouldNotExpireWhenValueWasNotSetAndRedisContainsOtherData() { + + String other = "other"; + when(connectionMock.setNX(KEY_BYTES, VALUE_BYTES)).thenReturn(false); + when(connectionMock.get(KEY_BYTES)).thenReturn(other.getBytes()); + when(valueSerializerMock.deserialize(eq(other.getBytes()))).thenReturn(other); + + cache = new RedisCache(CACHE_NAME, NO_PREFIX_BYTES, templateSpy, 10L); + Cache.ValueWrapper valueWrapper = cache.putIfAbsent(KEY, VALUE); + + assertThat(valueWrapper, is(notNullValue())); + verify(connectionMock, never()).expire(eq(KEY_BYTES), anyLong()); + } + + /** + * @see DATAREDIS-542 + */ + @Test + public void putIfAbsentShouldExpireWhenValueWasNotSetAndRedisContainsSameData() { + + when(connectionMock.setNX(KEY_BYTES, VALUE_BYTES)).thenReturn(false); + when(connectionMock.get(KEY_BYTES)).thenReturn(VALUE_BYTES); + + cache = new RedisCache(CACHE_NAME, NO_PREFIX_BYTES, templateSpy, 10L); + Cache.ValueWrapper valueWrapper = cache.putIfAbsent(KEY, VALUE); + + assertThat(valueWrapper, is(notNullValue())); + verify(connectionMock).expire(eq(KEY_BYTES), anyLong()); + } + /** * @see DATAREDIS-443 */