From e1f54d6c68f4cbdfd67b616c8733d52af66a7059 Mon Sep 17 00:00:00 2001 From: Jennifer Hickey Date: Mon, 19 Aug 2013 12:14:20 -0700 Subject: [PATCH] Implement RedisMap concurrent remove and replace DATAREDIS-37 --- .../data/redis/core/RedisOperations.java | 4 + .../support/collections/DefaultRedisMap.java | 153 ++++++++---------- .../support/collections/RedisProperties.java | 6 +- .../collections/AbstractRedisMapTests.java | 62 ++++--- 4 files changed, 118 insertions(+), 107 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/RedisOperations.java b/src/main/java/org/springframework/data/redis/core/RedisOperations.java index 75cb2190a..851e246ae 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisOperations.java +++ b/src/main/java/org/springframework/data/redis/core/RedisOperations.java @@ -299,4 +299,8 @@ public interface RedisOperations { RedisSerializer getValueSerializer(); RedisSerializer getKeySerializer(); + + RedisSerializer getHashKeySerializer(); + + RedisSerializer getHashValueSerializer(); } \ No newline at end of file diff --git a/src/main/java/org/springframework/data/redis/support/collections/DefaultRedisMap.java b/src/main/java/org/springframework/data/redis/support/collections/DefaultRedisMap.java index 8c5cef921..d0288ac3d 100644 --- a/src/main/java/org/springframework/data/redis/support/collections/DefaultRedisMap.java +++ b/src/main/java/org/springframework/data/redis/support/collections/DefaultRedisMap.java @@ -27,6 +27,7 @@ import java.util.concurrent.TimeUnit; import org.springframework.data.redis.connection.DataType; import org.springframework.data.redis.core.BoundHashOperations; import org.springframework.data.redis.core.RedisOperations; +import org.springframework.data.redis.core.SessionCallback; /** * Default implementation for {@link RedisMap}. @@ -193,101 +194,81 @@ public class DefaultRedisMap implements RedisMap { public V putIfAbsent(K key, V value) { return (hashOps.putIfAbsent(key, value) ? null : get(key)); - - // RedisOperations ops = hashOps.getOperations(); - // - // for (;;) { - // ops.watch(getKey()); - // V v = get(key); - // if (v == null) { - // ops.multi(); - // put(key, value); - // if (ops.exec() != null) { - // return null; - // } - // } - // else { - // return v; - // } - // } } - public boolean remove(Object key, Object value) { - throw new UnsupportedOperationException(); - - // if (value == null){ - // throw new NullPointerException(); - // } - // - // RedisOperations ops = hashOps.getOperations(); - // - // for (;;) { - // ops.watch(getKey()); - // V v = get(key); - // if (value.equals(v)) { - // ops.multi(); - // remove(key); - // if (ops.exec() != null) { - // return true; - // } - // } - // else { - // return false; - // } - // } + public boolean remove(final Object key, final Object value) { + if (value == null){ + throw new NullPointerException(); + } + return hashOps.getOperations().execute(new SessionCallback() { + @SuppressWarnings({ "unchecked", "rawtypes" }) + public Boolean execute(RedisOperations ops) { + for (;;) { + ops.watch(Collections.singleton(getKey())); + V v = get(key); + if (value.equals(v)) { + ops.multi(); + remove(key); + if (ops.exec(ops.getHashValueSerializer()) != null) { + return true; + } + } else { + return false; + } + } + } + }); } - public boolean replace(K key, V oldValue, V newValue) { - throw new UnsupportedOperationException(); - - // if (newValue == null || oldValue == null) { - // throw new NullPointerException(); - // } - // - // RedisOperations ops = hashOps.getOperations(); - // - // for (;;) { - // ops.watch(getKey()); - // V v = get(key); - // if (oldValue.equals(v)) { - // ops.multi(); - // put(key, newValue); - // if (ops.exec() != null) { - // return true; - // } - // } - // else { - // return false; - // } - // } + public boolean replace(final K key, final V oldValue, final V newValue) { + if(oldValue == null || newValue == null) { + throw new NullPointerException(); + } + return hashOps.getOperations().execute(new SessionCallback() { + @SuppressWarnings({ "unchecked", "rawtypes" }) + public Boolean execute(RedisOperations ops) { + for (;;) { + ops.watch(Collections.singleton(getKey())); + V v = get(key); + if (oldValue.equals(v)) { + ops.multi(); + put(key, newValue); + if (ops.exec(ops.getHashValueSerializer()) != null) { + return true; + } + } else { + return false; + } + } + } + }); } - public V replace(K key, V value) { - throw new UnsupportedOperationException(); - - - // if (value == null) { - // throw new NullPointerException(); - // } - // - // RedisOperations ops = hashOps.getOperations(); - // - // for (;;) { - // ops.watch(getKey()); - // if (containsKey(key)) { - // ops.multi(); - // V oldValue = put(key, value); - // if (ops.exec() != null) { - // return oldValue; - // } - // } - // else { - // return null; - // } - // } + public V replace(final K key, final V value) { + if(value == null) { + throw new NullPointerException(); + } + return hashOps.getOperations().execute(new SessionCallback() { + @SuppressWarnings({ "unchecked", "rawtypes" }) + public V execute(RedisOperations ops) { + for (;;) { + ops.watch(Collections.singleton(getKey())); + V v = get(key); + if (v != null) { + ops.multi(); + put(key, value); + if (ops.exec(ops.getHashValueSerializer()) != null) { + return v; + } + } else { + return null; + } + } + } + }); } diff --git a/src/main/java/org/springframework/data/redis/support/collections/RedisProperties.java b/src/main/java/org/springframework/data/redis/support/collections/RedisProperties.java index 277bb0ec0..622127b08 100644 --- a/src/main/java/org/springframework/data/redis/support/collections/RedisProperties.java +++ b/src/main/java/org/springframework/data/redis/support/collections/RedisProperties.java @@ -259,17 +259,17 @@ public class RedisProperties extends Properties implements RedisMap { assertThat(map.get(k2), isEqual(v2)); } - @Test(expected = UnsupportedOperationException.class) + @Test public void testConcurrentRemove() { - K k1 = getKey(); - V v1 = getValue(); - - map.put(k1, v1); - assertFalse(map.remove(k1, v1)); - assertEquals(v1, map.get(k1)); - assertTrue(map.remove(k1, v1)); - assertNull(map.get(k1)); - } - - @Test(expected = UnsupportedOperationException.class) - public void testConcurrentReplaceTwoArgs() { + assumeTrue(!ConnectionUtils.isJredis(template.getConnectionFactory())); K k1 = getKey(); V v1 = getValue(); V v2 = getValue(); + // No point testing this with byte[], they will never be equal + assumeTrue(!(v1 instanceof byte[])); + map.put(k1, v2); + assertFalse(map.remove(k1, v1)); + assertThat(map.get(k1), isEqual(v2)); + assertTrue(map.remove(k1, v2)); + assertNull(map.get(k1)); + } + + @Test(expected=NullPointerException.class) + public void testRemoveNullValue() { + map.remove(getKey(), null); + } + + @Test + public void testConcurrentReplaceTwoArgs() { + assumeTrue(!ConnectionUtils.isJredis(template.getConnectionFactory())); + K k1 = getKey(); + V v1 = getValue(); + V v2 = getValue(); + // No point testing binary data here, as equals will always be false + assumeTrue(!(v1 instanceof byte[])); map.put(k1, v1); assertFalse(map.replace(k1, v2, v1)); - assertEquals(v1, map.get(k1)); + assertThat(map.get(k1), isEqual(v1)); assertTrue(map.replace(k1, v1, v2)); - assertEquals(v2, map.get(k1)); + assertThat(map.get(k1), isEqual(v2)); } - @Test(expected = UnsupportedOperationException.class) + @Test(expected=NullPointerException.class) + public void testReplaceNullOldValue() { + map.replace(getKey(), null, getValue()); + } + + @Test(expected=NullPointerException.class) + public void testReplaceNullNewValue() { + map.replace(getKey(), getValue(), null); + } + + @Test public void testConcurrentReplaceOneArg() { + assumeTrue(!ConnectionUtils.isJredis(template.getConnectionFactory())); K k1 = getKey(); V v1 = getValue(); V v2 = getValue(); @@ -450,8 +472,12 @@ public abstract class AbstractRedisMapTests { assertNull(map.replace(k1, v1)); map.put(k1, v1); assertNull(map.replace(getKey(), v1)); - assertEquals(v1, map.replace(k1, v2)); - assertEquals(v2, map.get(k1)); + assertThat(map.replace(k1, v2), isEqual(v1)); + assertThat(map.get(k1), isEqual(v2)); + } + @Test(expected=NullPointerException.class) + public void testReplaceNullValue() { + map.replace(getKey(), null); } } \ No newline at end of file