DATAREDIS-843 - Adapt compare-and-set to changed transaction rollback response.

We now consider a transactional rollback that returns an empty EXEC result as rollback for the CAS (compare-and-set) operation. Previously, we checked only that the response of EXEC is not null. A rollback returns an empty list which was previously considered a successful CAS operation.

The code for CAS is now extracted to CompareAndSet and is reused from RedisAtomic implementations.

Original Pull Request: #349

src/main/java/org/springframework/data/redis/support/atomic/CompareAndSet.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.redis.support.atomic;
+
+import lombok.RequiredArgsConstructor;
+
+import java.util.Collection;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+import org.springframework.dao.DataAccessException;
+import org.springframework.data.redis.core.RedisOperations;
+import org.springframework.data.redis.core.SessionCallback;
+import org.springframework.util.CollectionUtils;
+
+/**
+ * Compare-and-set (CAS) operation using Redis Transactions ({@literal WATCH} and {@literal MULTI}) to atomically update
+ * the value at {@code key}.
+ * <p>
+ * The CAS block registers a {@literal WATCH} on the key holding the expected value which guarantees that changes after
+ * watching and comparing the key will rollback the transaction. The {@literal WATCH} is reset if the comparison fails.
+ *
+ * @author Mark Paluch
+ * @since 2.0.8
+ * @see RedisAtomicDouble
+ * @see RedisAtomicInteger
+ * @see RedisAtomicLong
+ */
+@RequiredArgsConstructor
+class CompareAndSet<T> implements SessionCallback<Boolean> {
+
+	private final Supplier<T> getter;
+	private final Consumer<T> setter;
+	private final String key;
+	private final T expect;
+	private final T update;
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.core.SessionCallback#execute(org.springframework.data.redis.core.RedisOperations)
+	 */
+	@Override
+	@SuppressWarnings("unchecked")
+	public <K, V> Boolean execute(RedisOperations<K, V> operations) throws DataAccessException {
+
+		RedisOperations<String, T> ops = (RedisOperations<String, T>) operations;
+
+		ops.watch(key);
+
+		if (expect.equals(getter.get())) {
+
+			ops.multi();
+			setter.accept(update);
+
+			if (updateSuccessful(operations.exec())) {
+				return true;
+			}
+		}
+
+		ops.unwatch();
+		return false;
+	}
+
+	private static boolean updateSuccessful(Collection<?> exec) {
+		return !CollectionUtils.isEmpty(exec);
+	}
+}

src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java

@@ -16,7 +16,6 @@
 package org.springframework.data.redis.support.atomic;
 
 import java.io.Serializable;
-import java.util.Collections;
 import java.util.Date;
 import java.util.concurrent.TimeUnit;
 
@@ -26,11 +25,9 @@ import org.springframework.data.redis.connection.RedisConnectionFactory;
 import org.springframework.data.redis.core.BoundKeyOperations;
 import org.springframework.data.redis.core.RedisOperations;
 import org.springframework.data.redis.core.RedisTemplate;
-import org.springframework.data.redis.core.SessionCallback;
 import org.springframework.data.redis.core.ValueOperations;
 import org.springframework.data.redis.serializer.GenericToStringSerializer;
 import org.springframework.data.redis.serializer.RedisSerializer;
-import org.springframework.data.redis.serializer.StringRedisSerializer;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
@@ -190,27 +187,7 @@ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyO
 	 *         expected value.
 	 */
 	public boolean compareAndSet(final double expect, final double update) {
-
-		return generalOps.execute(new SessionCallback<Boolean>() {
-
-			@Override
-			@SuppressWarnings({ "unchecked", "rawtypes" })
-			public Boolean execute(RedisOperations operations) {
-				for (;;) {
-					operations.watch(Collections.singleton(key));
-					if (expect == get()) {
-						generalOps.multi();
-						set(update);
-						if (operations.exec() != null) {
-							return true;
-						}
-					}
-					{
-						return false;
-					}
-				}
-			}
-		});
+		return generalOps.execute(new CompareAndSet<>(this::get, this::set, key, expect, update));
 	}
 
 	/**

src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java

@@ -16,7 +16,6 @@
 package org.springframework.data.redis.support.atomic;
 
 import java.io.Serializable;
-import java.util.Collections;
 import java.util.Date;
 import java.util.concurrent.TimeUnit;
 
@@ -26,7 +25,6 @@ import org.springframework.data.redis.connection.RedisConnectionFactory;
 import org.springframework.data.redis.core.BoundKeyOperations;
 import org.springframework.data.redis.core.RedisOperations;
 import org.springframework.data.redis.core.RedisTemplate;
-import org.springframework.data.redis.core.SessionCallback;
 import org.springframework.data.redis.core.ValueOperations;
 import org.springframework.data.redis.serializer.GenericToStringSerializer;
 import org.springframework.data.redis.serializer.RedisSerializer;
@@ -187,27 +185,7 @@ public class RedisAtomicInteger extends Number implements Serializable, BoundKey
 	 *         expected value.
 	 */
 	public boolean compareAndSet(int expect, int update) {
-
-		return generalOps.execute(new SessionCallback<Boolean>() {
-
-			@Override
-			@SuppressWarnings("unchecked")
-			public Boolean execute(RedisOperations operations) {
-				for (;;) {
-					operations.watch(Collections.singleton(key));
-					if (expect == get()) {
-						generalOps.multi();
-						set(update);
-						if (operations.exec() != null) {
-							return true;
-						}
-					}
-					{
-						return false;
-					}
-				}
-			}
-		});
+		return generalOps.execute(new CompareAndSet<>(this::get, this::set, key, expect, update));
 	}
 
 	/**

src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java

@@ -16,7 +16,6 @@
 package org.springframework.data.redis.support.atomic;
 
 import java.io.Serializable;
-import java.util.Collections;
 import java.util.Date;
 import java.util.concurrent.TimeUnit;
 
@@ -26,7 +25,6 @@ import org.springframework.data.redis.connection.RedisConnectionFactory;
 import org.springframework.data.redis.core.BoundKeyOperations;
 import org.springframework.data.redis.core.RedisOperations;
 import org.springframework.data.redis.core.RedisTemplate;
-import org.springframework.data.redis.core.SessionCallback;
 import org.springframework.data.redis.core.ValueOperations;
 import org.springframework.data.redis.serializer.GenericToStringSerializer;
 import org.springframework.data.redis.serializer.RedisSerializer;
@@ -194,27 +192,7 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe
 	 *         expected value.
 	 */
 	public boolean compareAndSet(long expect, long update) {
-
-		return generalOps.execute(new SessionCallback<Boolean>() {
-
-			@Override
-			@SuppressWarnings("unchecked")
-			public Boolean execute(RedisOperations operations) {
-				for (;;) {
-					operations.watch(Collections.singleton(key));
-					if (expect == get()) {
-						generalOps.multi();
-						set(update);
-						if (operations.exec() != null) {
-							return true;
-						}
-					}
-					{
-						return false;
-					}
-				}
-			}
-		});
+		return generalOps.execute(new CompareAndSet<>(this::get, this::set, key, expect, update));
 	}
 
 	/**

src/test/java/org/springframework/data/redis/support/atomic/CompareAndSetIntegrationTests.java

@@ -0,0 +1,136 @@
+/*
+ * Copyright 2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.redis.support.atomic;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.Collection;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.springframework.data.redis.ConnectionFactoryTracker;
+import org.springframework.data.redis.connection.RedisConnection;
+import org.springframework.data.redis.connection.RedisConnectionFactory;
+import org.springframework.data.redis.core.RedisTemplate;
+import org.springframework.data.redis.core.ValueOperations;
+import org.springframework.data.redis.serializer.GenericToStringSerializer;
+import org.springframework.data.redis.serializer.StringRedisSerializer;
+
+/**
+ * Integration tests for {@link CompareAndSet}.
+ *
+ * @author Mark Paluch
+ */
+@RunWith(Parameterized.class)
+public class CompareAndSetIntegrationTests {
+
+	private static final String KEY = "key";
+
+	private final RedisConnectionFactory factory;
+	private final RedisTemplate<String, Long> template;
+	private final ValueOperations<String, Long> valueOps;
+
+	public CompareAndSetIntegrationTests(RedisConnectionFactory factory) {
+
+		this.factory = factory;
+
+		this.template = new RedisTemplate<>();
+		this.template.setConnectionFactory(factory);
+		this.template.setKeySerializer(StringRedisSerializer.UTF_8);
+		this.template.setValueSerializer(new GenericToStringSerializer<>(Long.class));
+		this.template.afterPropertiesSet();
+
+		this.valueOps = this.template.opsForValue();
+
+		ConnectionFactoryTracker.add(factory);
+	}
+
+	@After
+	public void stop() {
+		RedisConnection connection = factory.getConnection();
+		connection.flushDb();
+		connection.close();
+	}
+
+	@AfterClass
+	public static void cleanUp() {
+		ConnectionFactoryTracker.cleanUp();
+	}
+
+	@Parameters
+	public static Collection<Object[]> testParams() {
+		return AtomicCountersParam.testParams();
+	}
+
+	@Test // DATAREDIS-843
+	public void shouldUpdateCounter() {
+
+		long expected = 5;
+		long actual = 5;
+		long update = 6;
+
+		CompareAndSet<Long> cas = new CompareAndSet<>(() -> actual, newValue -> valueOps.set(KEY, newValue), KEY, expected,
+				update);
+
+		Boolean executed = this.template.execute(cas);
+
+		assertThat(executed).isTrue();
+		assertThat(valueOps.get(KEY)).isEqualTo(update);
+	}
+
+	@Test // DATAREDIS-843
+	public void expectationNotMet() {
+
+		long expected = 5;
+		long actual = 7;
+		long update = 6;
+
+		CompareAndSet<Long> cas = new CompareAndSet<>(() -> actual, newValue -> valueOps.set(KEY, newValue), KEY, expected,
+				update);
+
+		Boolean executed = this.template.execute(cas);
+
+		assertThat(executed).isFalse();
+		assertThat(valueOps.get(KEY)).isNull();
+	}
+
+	@Test // DATAREDIS-843
+	public void concurrentUpdate() {
+
+		long expected = 5;
+		long actual = 5;
+		long update = 6;
+		long concurrentlyUpdated = 7;
+
+		CompareAndSet<Long> cas = new CompareAndSet<>(() -> actual, newValue -> {
+
+			RedisConnection connection = this.factory.getConnection();
+			connection.set(KEY.getBytes(), Long.toString(concurrentlyUpdated).getBytes());
+			connection.close();
+
+			valueOps.set(KEY, newValue);
+		}, KEY, expected, update);
+
+		Boolean executed = this.template.execute(cas);
+
+		assertThat(executed).isFalse();
+		assertThat(valueOps.get(KEY)).isEqualTo(concurrentlyUpdated);
+	}
+}

src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicDoubleTests.java

@@ -69,12 +69,6 @@ public class RedisAtomicDoubleTests extends AbstractRedisAtomicsTests {
 		ConnectionFactoryTracker.add(factory);
 	}
 
-	@Before
-	public void setUp() {
-		// Most atomic Double ops involve incrByFloat, which is new as of 2.6
-		assumeTrue(RedisTestProfileValueSource.matches("redisVersion", "2.6"));
-	}
-
 	@After
 	public void stop() {
 		RedisConnection connection = factory.getConnection();