You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ri...@apache.org on 2021/07/01 20:22:35 UTC
[geode] 03/03: Fix bug in NullRedisSet zadd,
fix updated entries leaving fossils in tree
This is an automated email from the ASF dual-hosted git repository.
ringles pushed a commit to branch GEODE-9375-Implement-ZRANGE-Radish-command
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 702a0e16feb22224c87d52db47313fadf5a77c66
Author: Ray Ingles <ri...@vmware.com>
AuthorDate: Thu Jul 1 16:19:11 2021 -0400
Fix bug in NullRedisSet zadd, fix updated entries leaving fossils in tree
---
.../executor/sortedset/AbstractZRangeIntegrationTest.java | 9 ++++++++-
.../apache/geode/redis/internal/data/NullRedisSortedSet.java | 2 +-
.../org/apache/geode/redis/internal/data/RedisSortedSet.java | 5 ++++-
.../apache/geode/redis/internal/data/RedisSortedSetTest.java | 11 ++++++++++-
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java
index d808d16..bcdfa60 100755
--- a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java
+++ b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java
@@ -82,7 +82,7 @@ public abstract class AbstractZRangeIntegrationTest implements RedisIntegrationT
}
@Test
- public void shouldReturnSyntaxError_givenWrongWithScoresFlag() {
+ public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
jedis.zadd(SORTED_SET_KEY, 1.0, "member");
assertThatThrownBy(
() -> jedis.sendCommand(SORTED_SET_KEY, Protocol.Command.ZRANGE, SORTED_SET_KEY, "1", "2",
@@ -91,6 +91,13 @@ public abstract class AbstractZRangeIntegrationTest implements RedisIntegrationT
}
@Test
+ public void shouldNotRetainOldEntries_whenEntryUpdated() {
+ jedis.zadd(SORTED_SET_KEY, 2.0, "mem4");
+ assertThat(jedis.zcard(SORTED_SET_KEY)).isEqualTo(members.size());
+ assertThat(jedis.zrange(SORTED_SET_KEY, 0, 100).size()).isEqualTo(members.size());
+ }
+
+ @Test
public void shouldReturnEmptyList_GivenInvalidRanges() {
assertThat(jedis.zrange(SORTED_SET_KEY, 5, 0)).isEmpty();
assertThat(jedis.zrange(SORTED_SET_KEY, 27, 30)).isEmpty();
diff --git a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
index 8028076..d74c89b 100644
--- a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
+++ b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
@@ -42,7 +42,7 @@ class NullRedisSortedSet extends RedisSortedSet {
if (options.isINCR()) {
return null;
}
- return 0L;
+ return 0;
}
if (options.isINCR()) {
diff --git a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
index 4f236af..a624248 100644
--- a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
+++ b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
@@ -171,11 +171,13 @@ public class RedisSortedSet extends AbstractRedisData {
byte[] oldScore = null;
OrderedSetEntry newEntry = new OrderedSetEntry(memberToAdd, scoreToAdd);
- scoreSet.add(newEntry);
OrderedSetEntry orderedSetEntry = members.put(memberToAdd, newEntry);
if (orderedSetEntry == null) {
+ scoreSet.add(newEntry);
sizeInBytes += calculateSizeOfFieldValuePair(memberToAdd, scoreToAdd);
} else {
+ scoreSet.remove(orderedSetEntry);
+ scoreSet.add(newEntry);
oldScore = orderedSetEntry.getScoreBytes();
sizeInBytes += scoreToAdd.length - oldScore.length;
}
@@ -350,6 +352,7 @@ public class RedisSortedSet extends AbstractRedisData {
byte[] oldValue = null;
OrderedSetEntry orderedSetEntry = members.remove(member);
if (orderedSetEntry != null) {
+ scoreSet.remove(orderedSetEntry);
oldValue = orderedSetEntry.getScoreBytes();
sizeInBytes -= calculateSizeOfFieldValuePair(member, oldValue);
}
diff --git a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java
index 5537cbe..f59058f 100644
--- a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java
+++ b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java
@@ -322,7 +322,6 @@ public class RedisSortedSetTest {
assertThat(rangeList).isEmpty();
}
-
@Test
public void zrange_ShouldReturnSimpleRanges() {
Collection<byte[]> rangeList = rangeSortedSet.zrange(0, 5, false);
@@ -378,6 +377,16 @@ public class RedisSortedSetTest {
"member6".getBytes(), "1.5".getBytes());
}
+ @Test
+ public void scoreSet_shouldNotRetainOldEntries_whenEntriesUpdated() {
+ Collection<byte[]> rangeList = rangeSortedSet.zrange(0, 100, false);
+ assertThat(rangeList).hasSize(12);
+ assertThat(rangeList).containsExactly("member1".getBytes(), "1.0".getBytes(),
+ "member2".getBytes(), "1.1".getBytes(), "member3".getBytes(), "1.2".getBytes(),
+ "member4".getBytes(), "1.3".getBytes(), "member5".getBytes(), "1.4".getBytes(),
+ "member6".getBytes(), "1.5".getBytes());
+ }
+
private RedisSortedSet createRedisSortedSet(String... membersAndScores) {
final List<byte[]> membersAndScoresList = Arrays
.stream(membersAndScores)