From b0e3eb8364ae6eea6ded05c04a36d3c9cf077c80 Mon Sep 17 00:00:00 2001 From: Nikita Date: Thu, 4 Feb 2016 11:34:51 +0300 Subject: [PATCH] RMap iterator fixed. #394 --- .../org/redisson/RedissonMapIterator.java | 24 ++++- .../org/redisson/client/codec/ScanCodec.java | 8 +- .../java/org/redisson/RedissonMapTest.java | 99 ++++++++++++------- 3 files changed, 93 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/redisson/RedissonMapIterator.java b/src/main/java/org/redisson/RedissonMapIterator.java index 7ed002fb1..5b6c17446 100644 --- a/src/main/java/org/redisson/RedissonMapIterator.java +++ b/src/main/java/org/redisson/RedissonMapIterator.java @@ -27,6 +27,7 @@ import org.redisson.client.protocol.decoder.MapScanResult; import org.redisson.client.protocol.decoder.ScanObjectEntry; import io.netty.buffer.ByteBuf; +import io.netty.util.CharsetUtil; public class RedissonMapIterator implements Iterator { @@ -35,6 +36,7 @@ public class RedissonMapIterator implements Iterator { private long iterPos = 0; private InetSocketAddress client; + private boolean finished; private boolean removeExecuted; private Map.Entry entry; @@ -46,13 +48,24 @@ public class RedissonMapIterator implements Iterator { @Override public boolean hasNext() { + if (finished) { + return false; + } if (iter == null || !iter.hasNext()) { MapScanResult res = map.scanIterator(client, iterPos); client = res.getRedisClient(); if (iterPos == 0 && firstValues == null) { firstValues = convert(res.getMap()); - } else if (convert(res.getMap()).equals(firstValues)) { - return false; + } else { + Map newValues = convert(res.getMap()); + if (newValues.equals(firstValues)) { + finished = true; + free(firstValues); + free(newValues); + firstValues = null; + return false; + } + free(newValues); } iter = res.getMap().entrySet().iterator(); iterPos = res.getPos(); @@ -60,6 +73,13 @@ public class RedissonMapIterator implements Iterator { return iter.hasNext(); } + private void free(Map map) { + for (Entry entry : map.entrySet()) { + entry.getKey().release(); + entry.getValue().release(); + } + } + private Map convert(Map map) { Map result = new HashMap(map.size()); for (Entry entry : map.entrySet()) { diff --git a/src/main/java/org/redisson/client/codec/ScanCodec.java b/src/main/java/org/redisson/client/codec/ScanCodec.java index f38ef9679..e1ef9ccd5 100644 --- a/src/main/java/org/redisson/client/codec/ScanCodec.java +++ b/src/main/java/org/redisson/client/codec/ScanCodec.java @@ -23,6 +23,8 @@ import org.redisson.client.protocol.Encoder; import org.redisson.client.protocol.decoder.ScanObjectEntry; import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.util.CharsetUtil; public class ScanCodec implements Codec { @@ -48,8 +50,9 @@ public class ScanCodec implements Codec { return new Decoder() { @Override public Object decode(ByteBuf buf, State state) throws IOException { + ByteBuf b = Unpooled.copiedBuffer(buf); Object val = delegate.getMapValueDecoder().decode(buf, state); - return new ScanObjectEntry(buf, val); + return new ScanObjectEntry(b, val); } }; } @@ -64,8 +67,9 @@ public class ScanCodec implements Codec { return new Decoder() { @Override public Object decode(ByteBuf buf, State state) throws IOException { + ByteBuf b = Unpooled.copiedBuffer(buf); Object val = delegate.getMapKeyDecoder().decode(buf, state); - return new ScanObjectEntry(buf, val); + return new ScanObjectEntry(b, val); } }; } diff --git a/src/test/java/org/redisson/RedissonMapTest.java b/src/test/java/org/redisson/RedissonMapTest.java index af9d9ea4f..9c1c4f4c1 100644 --- a/src/test/java/org/redisson/RedissonMapTest.java +++ b/src/test/java/org/redisson/RedissonMapTest.java @@ -1,5 +1,7 @@ package org.redisson; +import static org.assertj.core.api.Assertions.assertThat; + import java.io.Serializable; import java.util.Arrays; import java.util.Collection; @@ -7,10 +9,10 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; -import org.assertj.core.data.MapEntry; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.Assert; @@ -18,7 +20,6 @@ import org.junit.Test; import org.redisson.codec.JsonJacksonCodec; import org.redisson.core.Predicate; import org.redisson.core.RMap; -import static org.assertj.core.api.Assertions.*; import io.netty.util.concurrent.Future; @@ -132,17 +133,17 @@ public class RedissonMapTest extends BaseTest { map.put(1, 100); Integer res = map.addAndGet(1, 12); - Assert.assertEquals(112, (int)res); + assertThat(res).isEqualTo(112); res = map.get(1); - Assert.assertEquals(112, (int)res); + assertThat(res).isEqualTo(112); RMap map2 = redisson.getMap("getAll2"); map2.put(1, new Double(100.2)); Double res2 = map2.addAndGet(1, new Double(12.1)); - Assert.assertTrue(new Double(112.3).compareTo(res2) == 0); + assertThat(res2).isEqualTo(112.3); res2 = map2.get(1); - Assert.assertTrue(new Double(112.3).compareTo(res2) == 0); + assertThat(res2).isEqualTo(112.3); RMap mapStr = redisson.getMap("mapStr"); assertThat(mapStr.put("1", 100)).isNull(); @@ -164,7 +165,7 @@ public class RedissonMapTest extends BaseTest { Map expectedMap = new HashMap(); expectedMap.put(2, 200); expectedMap.put(3, 300); - Assert.assertEquals(expectedMap, filtered); + assertThat(filtered).isEqualTo(expectedMap); } @Test @@ -180,7 +181,7 @@ public class RedissonMapTest extends BaseTest { Map expectedMap = new HashMap(); expectedMap.put("B", 200); expectedMap.put("C", 300); - Assert.assertEquals(expectedMap, filtered); + assertThat(filtered).isEqualTo(expectedMap); } @Test @@ -201,7 +202,7 @@ public class RedissonMapTest extends BaseTest { Map expectedMap = new HashMap(); expectedMap.put(2, 200); expectedMap.put(3, 300); - Assert.assertEquals(expectedMap, filtered); + assertThat(filtered).isEqualTo(expectedMap); } @Test @@ -210,12 +211,13 @@ public class RedissonMapTest extends BaseTest { map.put(1, 2); map.put(3, 4); - Assert.assertEquals(2, map.size()); + assertThat(map.size()).isEqualTo(2); Integer val = map.get(1); - Assert.assertEquals(2, val.intValue()); + assertThat(val).isEqualTo(2); + Integer val2 = map.get(3); - Assert.assertEquals(4, val2.intValue()); + assertThat(val2).isEqualTo(4); } @Test @@ -224,14 +226,42 @@ public class RedissonMapTest extends BaseTest { map.put(1L, 2L); map.put(3L, 4L); - Assert.assertEquals(2, map.size()); + assertThat(map.size()).isEqualTo(2); Long val = map.get(1L); - Assert.assertEquals(2L, val.longValue()); + assertThat(val).isEqualTo(2); + Long val2 = map.get(3L); - Assert.assertEquals(4L, val2.longValue()); + assertThat(val2).isEqualTo(4); } + @Test + public void testIterator() { + RMap rMap = redisson.getMap("123"); + + int size = 1000; + for (int i = 0; i < size; i++) { + rMap.put(i,i); + } + assertThat(rMap.size()).isEqualTo(1000); + + int counter = 0; + for (Integer key : rMap.keySet()) { + counter++; + } + assertThat(counter).isEqualTo(size); + counter = 0; + for (Integer value : rMap.values()) { + counter++; + } + assertThat(counter).isEqualTo(size); + counter = 0; + for (Entry entry : rMap.entrySet()) { + counter++; + } + assertThat(counter).isEqualTo(size); + } + @Test public void testNull() { Map map = redisson.getMap("simple12"); @@ -239,14 +269,14 @@ public class RedissonMapTest extends BaseTest { map.put(2, null); map.put(3, "43"); - Assert.assertEquals(3, map.size()); + assertThat(map.size()).isEqualTo(3); String val = map.get(2); - Assert.assertNull(val); + assertThat(val).isNull(); String val2 = map.get(1); - Assert.assertNull(val2); + assertThat(val2).isNull(); String val3 = map.get(3); - Assert.assertEquals("43", val3); + assertThat(val3).isEqualTo("43"); } @Test @@ -281,7 +311,7 @@ public class RedissonMapTest extends BaseTest { map.put(3, "43"); String val = map.get(2); - Assert.assertEquals("33", val); + assertThat(val).isEqualTo("33"); } @Test @@ -294,7 +324,7 @@ public class RedissonMapTest extends BaseTest { map.remove(new SimpleKey("33")); map.remove(new SimpleKey("5")); - Assert.assertEquals(1, map.size()); + assertThat(map.size()).isEqualTo(1); } @Test @@ -310,7 +340,7 @@ public class RedissonMapTest extends BaseTest { joinMap.put(6, "6"); map.putAll(joinMap); - MatcherAssert.assertThat(map.keySet(), Matchers.containsInAnyOrder(1, 2, 3, 4, 5, 6)); + assertThat(map.keySet()).containsOnly(1, 2, 3, 4, 5, 6); } @Test @@ -553,10 +583,10 @@ public class RedissonMapTest extends BaseTest { map.put(3, 5); map.put(7, 8); - Assert.assertEquals((Integer) 3, map.removeAsync(1).get()); - Assert.assertEquals((Integer) 5, map.removeAsync(3).get()); - Assert.assertNull(map.removeAsync(10).get()); - Assert.assertEquals((Integer) 8, map.removeAsync(7).get()); + assertThat(map.removeAsync(1).get()).isEqualTo(3); + assertThat(map.removeAsync(3).get()).isEqualTo(5); + assertThat(map.removeAsync(10).get()).isNull(); + assertThat(map.removeAsync(7).get()).isEqualTo(8); } @Test @@ -567,9 +597,9 @@ public class RedissonMapTest extends BaseTest { map.put(4, 6); map.put(7, 8); - Assert.assertEquals((Long) 3L, map.fastRemoveAsync(1, 3, 7).get()); + assertThat(map.fastRemoveAsync(1, 3, 7).get()).isEqualTo(3); Thread.sleep(1); - Assert.assertEquals(1, map.size()); + assertThat(map.size()).isEqualTo(1); } @Test @@ -589,7 +619,7 @@ public class RedissonMapTest extends BaseTest { } } - Assert.assertEquals(0, keys.size()); + assertThat(keys.size()).isEqualTo(0); } @Test @@ -609,7 +639,7 @@ public class RedissonMapTest extends BaseTest { } } - Assert.assertEquals(0, values.size()); + assertThat(values.size()).isEqualTo(0); } @Test @@ -632,16 +662,17 @@ public class RedissonMapTest extends BaseTest { testMap.put("2", "4"); testMap.put("3", "5"); - Assert.assertEquals(testMap, map); - Assert.assertEquals(testMap.hashCode(), map.hashCode()); + assertThat(map).isEqualTo(testMap); + assertThat(testMap.hashCode()).isEqualTo(map.hashCode()); } @Test public void testFastRemoveEmpty() throws Exception { RMap map = redisson.getMap("simple"); map.put(1, 3); - Assert.assertEquals(0, map.fastRemove()); - Assert.assertEquals(1, map.size()); + + assertThat(map.fastRemove()).isZero(); + assertThat(map.size()).isEqualTo(1); } @Test(timeout = 5000)