From b056c4b53cfc3e483900f96bd3226c9d89a4745e Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Fri, 31 Mar 2017 15:42:33 +0100 Subject: [PATCH 1/7] fix the #827 fixed by extending addAndGet method and made it packed content aware --- .../java/org/redisson/RedissonMapCache.java | 41 ++++++++++++++++++- .../org/redisson/RedissonMapCacheTest.java | 12 ++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/redisson/src/main/java/org/redisson/RedissonMapCache.java b/redisson/src/main/java/org/redisson/RedissonMapCache.java index b0005bca3..cf4a8f0e2 100644 --- a/redisson/src/main/java/org/redisson/RedissonMapCache.java +++ b/redisson/src/main/java/org/redisson/RedissonMapCache.java @@ -50,6 +50,9 @@ import org.redisson.eviction.EvictionScheduler; import io.netty.util.concurrent.Future; import io.netty.util.concurrent.FutureListener; +import java.math.BigDecimal; +import org.redisson.client.codec.StringCodec; +import org.redisson.client.protocol.convertor.NumberConvertor; /** *

Map-based cache with ability to set TTL for each entry via @@ -362,7 +365,7 @@ public class RedissonMapCache extends RedissonMap implements RMapCac return commandExecutor.evalWriteAsync(getName(key), codec, EVAL_PUT, "local value = struct.pack('dLc0', 0, string.len(ARGV[2]), ARGV[2]); " + "if redis.call('hsetnx', KEYS[1], ARGV[1], value) == 1 then " - + "return nil " + + "return nil;" + "else " + "local v = redis.call('hget', KEYS[1], ARGV[1]); " + "if v == false then " @@ -374,6 +377,42 @@ public class RedissonMapCache extends RedissonMap implements RMapCac Collections.singletonList(getName(key)), key, value); } + @Override + public RFuture addAndGetAsync(K key, Number value) { + byte[] keyState = encodeMapKey(key); + byte[] valueState = encodeMapValue(new BigDecimal(value.toString()).toPlainString()); + return commandExecutor.evalWriteAsync(getName(key), StringCodec.INSTANCE, + new RedisCommand("EVAL", new NumberConvertor(value.getClass())), + "local value = redis.call('hget', KEYS[1], ARGV[2]); " + + "if value ~= false then " + + "local t, val = struct.unpack('dLc0', value); " + + "local expireDate = 92233720368547758; " + + "local expireDateScore = redis.call('zscore', KEYS[2], ARGV[2]); " + + "if expireDateScore ~= false then " + + "expireDate = tonumber(expireDateScore) " + + "end; " + + "if t ~= 0 then " + + "local expireIdle = redis.call('zscore', KEYS[3], ARGV[2]); " + + "if expireIdle ~= false then " + + "if tonumber(expireIdle) > tonumber(ARGV[1]) then " + + "local value = struct.pack('dLc0', t, string.len(val), val); " + + "redis.call('hset', KEYS[1], ARGV[2], value); " + + "redis.call('zadd', KEYS[3], t + tonumber(ARGV[1]), ARGV[2]); " + + "end; " + + "expireDate = math.min(expireDate, tonumber(expireIdle)) " + + "end; " + + "end; " + + "end; " + + "local newValue = tonumber(ARGV[3]); " + + "if expireDate <= tonumber(ARGV[1]) then " + + "newValue = tonumber(value) + newValue; " + + "end; " + + "local newValuePack = struct.pack('dLc0', t + tonumber(ARGV[1]), string.len(newValue), newValue); " + + "redis.call('hset', KEYS[1], ARGV[2], newValuePack);" + + "return newValue; ", + Arrays.asList(getName(key), getTimeoutSetNameByKey(key), getIdleSetNameByKey(key)), System.currentTimeMillis(), keyState, valueState); + } + @Override public boolean fastPut(K key, V value, long ttl, TimeUnit ttlUnit) { return get(fastPutAsync(key, value, ttl, ttlUnit)); diff --git a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java index 3c3d95904..ad1d4c642 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java @@ -20,6 +20,8 @@ import org.junit.Test; import org.redisson.api.RFuture; import org.redisson.api.RMap; import org.redisson.api.RMapCache; +import org.redisson.client.codec.LongCodec; +import org.redisson.client.codec.StringCodec; import org.redisson.codec.JsonJacksonCodec; import org.redisson.codec.MsgPackJacksonCodec; @@ -838,4 +840,14 @@ public class RedissonMapCacheTest extends BaseTest { this.testField = testField; } } + + @Test + public void testIssue827() { + RMap mapCache = redisson.getMap("test_put_if_absent", StringCodec.INSTANCE); + mapCache.putIfAbsent("4", 0L); + mapCache.addAndGet("4", 1L); + mapCache.putIfAbsent("4", 0L); + System.out.println(mapCache.get("4")); + } + } From 38c8ae6a78822f87c102f6566f39692a869748ab Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Tue, 4 Apr 2017 16:53:47 +0100 Subject: [PATCH 2/7] checks and fixed a few more different usage --- .../main/java/org/redisson/RedissonMapCache.java | 15 +++++++++++---- .../java/org/redisson/RedissonMapCacheTest.java | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonMapCache.java b/redisson/src/main/java/org/redisson/RedissonMapCache.java index cf4a8f0e2..80ffc962d 100644 --- a/redisson/src/main/java/org/redisson/RedissonMapCache.java +++ b/redisson/src/main/java/org/redisson/RedissonMapCache.java @@ -377,6 +377,11 @@ public class RedissonMapCache extends RedissonMap implements RMapCac Collections.singletonList(getName(key)), key, value); } + @Override + public V addAndGet(K key, Number value) { + return get(addAndGetAsync(key, value)); + } + @Override public RFuture addAndGetAsync(K key, Number value) { byte[] keyState = encodeMapKey(key); @@ -384,13 +389,15 @@ public class RedissonMapCache extends RedissonMap implements RMapCac return commandExecutor.evalWriteAsync(getName(key), StringCodec.INSTANCE, new RedisCommand("EVAL", new NumberConvertor(value.getClass())), "local value = redis.call('hget', KEYS[1], ARGV[2]); " + + "local expireDate = 92233720368547758; " + + "local idleTtl = 0; " + "if value ~= false then " + "local t, val = struct.unpack('dLc0', value); " - + "local expireDate = 92233720368547758; " + - "local expireDateScore = redis.call('zscore', KEYS[2], ARGV[2]); " + + "local expireDateScore = redis.call('zscore', KEYS[2], ARGV[2]); " + "if expireDateScore ~= false then " + "expireDate = tonumber(expireDateScore) " + "end; " + + "idleTtl = t; " + "if t ~= 0 then " + "local expireIdle = redis.call('zscore', KEYS[3], ARGV[2]); " + "if expireIdle ~= false then " @@ -407,9 +414,9 @@ public class RedissonMapCache extends RedissonMap implements RMapCac + "if expireDate <= tonumber(ARGV[1]) then " + "newValue = tonumber(value) + newValue; " + "end; " - + "local newValuePack = struct.pack('dLc0', t + tonumber(ARGV[1]), string.len(newValue), newValue); " + + "local newValuePack = struct.pack('dLc0', idleTtl + tonumber(ARGV[1]), string.len(newValue), newValue); " + "redis.call('hset', KEYS[1], ARGV[2], newValuePack);" - + "return newValue; ", + + "return tostring(newValue); ", Arrays.asList(getName(key), getTimeoutSetNameByKey(key), getIdleSetNameByKey(key)), System.currentTimeMillis(), keyState, valueState); } diff --git a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java index ad1d4c642..577c6cf4a 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java @@ -843,11 +843,21 @@ public class RedissonMapCacheTest extends BaseTest { @Test public void testIssue827() { - RMap mapCache = redisson.getMap("test_put_if_absent", StringCodec.INSTANCE); + RMapCache mapCache = redisson.getMapCache("test_put_if_absent", StringCodec.INSTANCE); + mapCache.putIfAbsent("4", 0L, 10000L, TimeUnit.SECONDS); + mapCache.addAndGet("4", 1L); + mapCache.putIfAbsent("4", 0L); + Assert.assertEquals("1", mapCache.get("4")); + mapCache = redisson.getMapCache("test_put_if_absent_1", StringCodec.INSTANCE); mapCache.putIfAbsent("4", 0L); mapCache.addAndGet("4", 1L); mapCache.putIfAbsent("4", 0L); - System.out.println(mapCache.get("4")); + Assert.assertEquals("1", mapCache.get("4")); + RMap map = redisson.getMap("test_put_if_absent_2", StringCodec.INSTANCE); + map.putIfAbsent("4", 0L); + map.addAndGet("4", 1L); + map.putIfAbsent("4", 0L); + Assert.assertEquals("1", map.get("4")); } } From 6a48ef0c4f167eb594826c60bed6715b42287718 Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Tue, 4 Apr 2017 23:19:41 +0100 Subject: [PATCH 3/7] one more case and fix added value of the addAndGet method shall always be encoded using StringCodec --- .../java/org/redisson/RedissonMapCache.java | 22 +++++++++++++------ .../org/redisson/RedissonMapCacheTest.java | 4 ++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonMapCache.java b/redisson/src/main/java/org/redisson/RedissonMapCache.java index 80ffc962d..13c472351 100644 --- a/redisson/src/main/java/org/redisson/RedissonMapCache.java +++ b/redisson/src/main/java/org/redisson/RedissonMapCache.java @@ -50,6 +50,7 @@ import org.redisson.eviction.EvictionScheduler; import io.netty.util.concurrent.Future; import io.netty.util.concurrent.FutureListener; +import java.io.IOException; import java.math.BigDecimal; import org.redisson.client.codec.StringCodec; import org.redisson.client.protocol.convertor.NumberConvertor; @@ -385,19 +386,26 @@ public class RedissonMapCache extends RedissonMap implements RMapCac @Override public RFuture addAndGetAsync(K key, Number value) { byte[] keyState = encodeMapKey(key); - byte[] valueState = encodeMapValue(new BigDecimal(value.toString()).toPlainString()); + byte[] valueState; + try { + valueState = StringCodec.INSTANCE + .getMapValueEncoder() + .encode(new BigDecimal(value.toString()).toPlainString()); + } catch (IOException e) { + throw new IllegalArgumentException(e); + } return commandExecutor.evalWriteAsync(getName(key), StringCodec.INSTANCE, new RedisCommand("EVAL", new NumberConvertor(value.getClass())), "local value = redis.call('hget', KEYS[1], ARGV[2]); " + "local expireDate = 92233720368547758; " - + "local idleTtl = 0; " + + "local t = 0; " + + "local val = 0; " + "if value ~= false then " - + "local t, val = struct.unpack('dLc0', value); " + + "t, val = struct.unpack('dLc0', value); " + "local expireDateScore = redis.call('zscore', KEYS[2], ARGV[2]); " + "if expireDateScore ~= false then " + "expireDate = tonumber(expireDateScore) " + "end; " - + "idleTtl = t; " + "if t ~= 0 then " + "local expireIdle = redis.call('zscore', KEYS[3], ARGV[2]); " + "if expireIdle ~= false then " @@ -412,10 +420,10 @@ public class RedissonMapCache extends RedissonMap implements RMapCac + "end; " + "local newValue = tonumber(ARGV[3]); " + "if expireDate <= tonumber(ARGV[1]) then " - + "newValue = tonumber(value) + newValue; " + + "newValue = tonumber(val) + newValue; " + "end; " - + "local newValuePack = struct.pack('dLc0', idleTtl + tonumber(ARGV[1]), string.len(newValue), newValue); " - + "redis.call('hset', KEYS[1], ARGV[2], newValuePack);" + + "local newValuePack = struct.pack('dLc0', t + tonumber(ARGV[1]), string.len(newValue), newValue); " + + "redis.call('hset', KEYS[1], ARGV[2], newValuePack); " + "return tostring(newValue); ", Arrays.asList(getName(key), getTimeoutSetNameByKey(key), getIdleSetNameByKey(key)), System.currentTimeMillis(), keyState, valueState); } diff --git a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java index 577c6cf4a..2f0354ea4 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java @@ -858,6 +858,10 @@ public class RedissonMapCacheTest extends BaseTest { map.addAndGet("4", 1L); map.putIfAbsent("4", 0L); Assert.assertEquals("1", map.get("4")); + RMapCache mapCache1 = redisson.getMapCache("test_put_if_absent_3"); + Object currValue = mapCache1.putIfAbsent("4", 1.23, 10000L, TimeUnit.SECONDS); + Object updatedValue = mapCache1.addAndGet("4", 1L); + System.out.println("updatedValue: " + updatedValue); } } From 09077c7ab72cd545926568402dbba5f1db6202f4 Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Tue, 4 Apr 2017 23:52:35 +0100 Subject: [PATCH 4/7] fixed wrong angle direction --- redisson/src/main/java/org/redisson/RedissonMapCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisson/src/main/java/org/redisson/RedissonMapCache.java b/redisson/src/main/java/org/redisson/RedissonMapCache.java index 13c472351..d67591df3 100644 --- a/redisson/src/main/java/org/redisson/RedissonMapCache.java +++ b/redisson/src/main/java/org/redisson/RedissonMapCache.java @@ -419,7 +419,7 @@ public class RedissonMapCache extends RedissonMap implements RMapCac + "end; " + "end; " + "local newValue = tonumber(ARGV[3]); " - + "if expireDate <= tonumber(ARGV[1]) then " + + "if expireDate >= tonumber(ARGV[1]) then " + "newValue = tonumber(val) + newValue; " + "end; " + "local newValuePack = struct.pack('dLc0', t + tonumber(ARGV[1]), string.len(newValue), newValue); " From c896e049473f8b1c43ccc5abe19befab3bec5fcb Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Tue, 4 Apr 2017 23:53:12 +0100 Subject: [PATCH 5/7] should use double type for test --- redisson/src/test/java/org/redisson/RedissonMapCacheTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java index 2f0354ea4..7336ff4eb 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java @@ -860,7 +860,7 @@ public class RedissonMapCacheTest extends BaseTest { Assert.assertEquals("1", map.get("4")); RMapCache mapCache1 = redisson.getMapCache("test_put_if_absent_3"); Object currValue = mapCache1.putIfAbsent("4", 1.23, 10000L, TimeUnit.SECONDS); - Object updatedValue = mapCache1.addAndGet("4", 1L); + Object updatedValue = mapCache1.addAndGet("4", 1D); System.out.println("updatedValue: " + updatedValue); } From 93d8e7f13985e9003fd6bd5f64c3808c102a9aac Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Tue, 4 Apr 2017 23:53:25 +0100 Subject: [PATCH 6/7] added assertion --- redisson/src/test/java/org/redisson/RedissonMapCacheTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java index 7336ff4eb..feb28fa78 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java @@ -862,6 +862,7 @@ public class RedissonMapCacheTest extends BaseTest { Object currValue = mapCache1.putIfAbsent("4", 1.23, 10000L, TimeUnit.SECONDS); Object updatedValue = mapCache1.addAndGet("4", 1D); System.out.println("updatedValue: " + updatedValue); + Assert.assertEquals(2.23, mapCache1.get("4")); } } From 32e8c1bb98477e52da7d83662b4e0c005ffabbb0 Mon Sep 17 00:00:00 2001 From: Rui Gu Date: Thu, 20 Apr 2017 14:40:40 +0100 Subject: [PATCH 7/7] renamed test method and removed standard out output --- .../src/test/java/org/redisson/RedissonMapCacheTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java index ea87bd75d..5ed285660 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapCacheTest.java @@ -842,7 +842,7 @@ public class RedissonMapCacheTest extends BaseTest { } @Test - public void testIssue827() { + public void testAddAndGet() { RMapCache mapCache = redisson.getMapCache("test_put_if_absent", StringCodec.INSTANCE); mapCache.putIfAbsent("4", 0L, 10000L, TimeUnit.SECONDS); mapCache.addAndGet("4", 1L); @@ -859,9 +859,8 @@ public class RedissonMapCacheTest extends BaseTest { map.putIfAbsent("4", 0L); Assert.assertEquals("1", map.get("4")); RMapCache mapCache1 = redisson.getMapCache("test_put_if_absent_3"); - Object currValue = mapCache1.putIfAbsent("4", 1.23, 10000L, TimeUnit.SECONDS); - Object updatedValue = mapCache1.addAndGet("4", 1D); - System.out.println("updatedValue: " + updatedValue); + mapCache1.putIfAbsent("4", 1.23, 10000L, TimeUnit.SECONDS); + mapCache1.addAndGet("4", 1D); Assert.assertEquals(2.23, mapCache1.get("4")); }