From 72d6df00c81082f906d3a7669b3565e90fefbddb Mon Sep 17 00:00:00 2001
From: Christoph Strobl <cstrobl@pivotal.io>
Date: Fri, 7 Oct 2016 09:07:28 +0200
Subject: [PATCH] DATAREDIS-542 - Polishing.

Prevent putIfAbsent from prolonging expiration time of existing keys having same value.
Remove some trailing white spaces.

Original Pull Request: #224
---
 .../data/redis/cache/RedisCache.java          | 41 ++++++++-----------
 .../data/redis/cache/RedisCacheUnitTests.java |  4 +-
 2 files changed, 18 insertions(+), 27 deletions(-)

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 a34c34b30..935290040 100644
--- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java
+++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java
@@ -76,7 +76,7 @@ public class RedisCache implements Cache {
 	/**
 	 * Return the value to which this cache maps the specified key, generically specifying a type that return value will
 	 * be cast to.
-	 * 
+	 *
 	 * @param key
 	 * @param type
 	 * @return
@@ -126,7 +126,7 @@ public class RedisCache implements Cache {
 
 	/**
 	 * Return the value to which this cache maps the specified key.
-	 * 
+	 *
 	 * @param cacheKey the key whose associated value is to be returned via its binary representation.
 	 * @return the {@link RedisCacheElement} stored at given key or {@literal null} if no value found for key.
 	 * @since 1.5
@@ -162,7 +162,7 @@ public class RedisCache implements Cache {
 	 * Add the element by adding {@link RedisCacheElement#get()} at {@link RedisCacheElement#getKeyBytes()}. If the cache
 	 * previously contained a mapping for this {@link RedisCacheElement#getKeyBytes()}, the old value is replaced by
 	 * {@link RedisCacheElement#get()}.
-	 * 
+	 *
 	 * @param element must not be {@literal null}.
 	 * @since 1.5
 	 */
@@ -188,7 +188,7 @@ public class RedisCache implements Cache {
 	/**
 	 * Add the element as long as no element exists at {@link RedisCacheElement#getKeyBytes()}. If a value is present for
 	 * {@link RedisCacheElement#getKeyBytes()} this one is returned.
-	 * 
+	 *
 	 * @param element must not be {@literal null}.
 	 * @return
 	 * @since 1.5
@@ -256,7 +256,7 @@ public class RedisCache implements Cache {
 	/**
 	 * Metadata required to maintain {@link RedisCache}. Keeps track of additional data structures required for processing
 	 * cache operations.
-	 * 
+	 *
 	 * @author Christoph Strobl
 	 * @since 1.5
 	 */
@@ -294,7 +294,7 @@ public class RedisCache implements Cache {
 
 		/**
 		 * Get the binary representation of the key prefix.
-		 * 
+		 *
 		 * @return never {@literal null}.
 		 */
 		public byte[] getKeyPrefix() {
@@ -303,7 +303,7 @@ public class RedisCache implements Cache {
 
 		/**
 		 * Get the binary representation of the key identifying the data structure used to maintain known keys.
-		 * 
+		 *
 		 * @return never {@literal null}.
 		 */
 		public byte[] getSetOfKnownKeysKey() {
@@ -312,7 +312,7 @@ public class RedisCache implements Cache {
 
 		/**
 		 * Get the binary representation of the key identifying the data structure used to lock the cache.
-		 * 
+		 *
 		 * @return never {@literal null}.
 		 */
 		public byte[] getCacheLockKey() {
@@ -321,7 +321,7 @@ public class RedisCache implements Cache {
 
 		/**
 		 * Get the name of the cache.
-		 * 
+		 *
 		 * @return
 		 */
 		public String getCacheName() {
@@ -330,7 +330,7 @@ public class RedisCache implements Cache {
 
 		/**
 		 * Set the default expiration time in seconds
-		 * 
+		 *
 		 * @param defaultExpiration
 		 */
 		public void setDefaultExpiration(long seconds) {
@@ -339,7 +339,7 @@ public class RedisCache implements Cache {
 
 		/**
 		 * Get the default expiration time in seconds.
-		 * 
+		 *
 		 * @return
 		 */
 		public long getDefaultExpiration() {
@@ -721,26 +721,17 @@ public class RedisCache implements Cache {
 
 			waitForLock(connection);
 
-			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 (!connection.setNX(keyBytes, value)) {
+				return connection.get(keyBytes);
 			}
 
-			if (keyMaintenance) {
-				processKeyExpiration(element, connection);
-				maintainKnownKeys(element, connection);
-			}
+			maintainKnownKeys(element, connection);
+			processKeyExpiration(element, connection);
 
-			return existingValue;
+			return null;
 		}
 	}
 
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 8f1df5421..bb5dace38 100644
--- a/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java
+++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheUnitTests.java
@@ -197,7 +197,7 @@ public class RedisCacheUnitTests {
 	 * @see DATAREDIS-542
 	 */
 	@Test
-	public void putIfAbsentShouldExpireWhenValueWasNotSetAndRedisContainsSameData() {
+	public void putIfAbsentShouldNotSetExpireWhenValueWasNotSetAndRedisContainsSameData() {
 
 		when(connectionMock.setNX(KEY_BYTES, VALUE_BYTES)).thenReturn(false);
 		when(connectionMock.get(KEY_BYTES)).thenReturn(VALUE_BYTES);
@@ -206,7 +206,7 @@ public class RedisCacheUnitTests {
 		Cache.ValueWrapper valueWrapper = cache.putIfAbsent(KEY, VALUE);
 
 		assertThat(valueWrapper, is(notNullValue()));
-		verify(connectionMock).expire(eq(KEY_BYTES), anyLong());
+		verify(connectionMock, never()).expire(eq(KEY_BYTES), anyLong());
 	}
 
 	/**