This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new f7cc081 GEODE-9185: Add ZPOPMAX Radish command (#6726)
f7cc081 is described below
commit f7cc081745ff3c0c162b5073bf7eddd3da474573
Author: Jens Deppe <jde...@vmware.com>
AuthorDate: Wed Aug 4 06:23:40 2021 -0700
GEODE-9185: Add ZPOPMAX Radish command (#6726)
---
.../ZPopMaxNativeRedisAcceptanceTest.java | 36 +++++
.../server/AbstractHitsMissesIntegrationTest.java | 5 +
.../sortedset/AbstractZPopMaxIntegrationTest.java | 157 +++++++++++++++++++++
.../executor/sortedset/ZPopMaxIntegrationTest.java | 31 ++++
.../geode/redis/internal/RedisCommandType.java | 3 +
.../redis/internal/data/NullRedisSortedSet.java | 5 +
.../geode/redis/internal/data/RedisSortedSet.java | 32 ++++-
.../RedisSortedSetCommandsFunctionExecutor.java | 6 +
.../executor/sortedset/RedisSortedSetCommands.java | 2 +
.../executor/sortedset/ZPopMaxExecutor.java | 51 +++++++
.../redis/internal/SupportedCommandsJUnitTest.java | 1 +
.../redis/internal/data/RedisSortedSetTest.java | 125 +++++++++++++++-
12 files changed, 446 insertions(+), 8 deletions(-)
diff --git
a/geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxNativeRedisAcceptanceTest.java
b/geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxNativeRedisAcceptanceTest.java
new file mode 100755
index 0000000..12fb5b6
--- /dev/null
+++
b/geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxNativeRedisAcceptanceTest.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.sortedset;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.NativeRedisClusterTestRule;
+
+public class ZPopMaxNativeRedisAcceptanceTest extends
AbstractZPopMaxIntegrationTest {
+
+ @ClassRule
+ public static NativeRedisClusterTestRule server = new
NativeRedisClusterTestRule();
+
+ @Override
+ public int getPort() {
+ return server.getExposedPorts().get(0);
+ }
+
+ @Override
+ public void flushAll() {
+ server.flushAll();
+ }
+
+}
diff --git
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
index ceb0425..53d6a5f 100644
---
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
+++
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
@@ -253,6 +253,11 @@ public abstract class AbstractHitsMissesIntegrationTest
implements RedisIntegrat
}
@Test
+ public void testZPopMax() {
+ runCommandAndAssertNoStatUpdates(SORTED_SET_KEY, k -> jedis.zpopmax(k, 1));
+ }
+
+ @Test
public void testZrange() {
runCommandAndAssertHitsAndMisses(SORTED_SET_KEY, k -> jedis.zrange(k, 0,
1));
}
diff --git
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZPopMaxIntegrationTest.java
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZPopMaxIntegrationTest.java
new file mode 100755
index 0000000..4de663a
--- /dev/null
+++
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZPopMaxIntegrationTest.java
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.sortedset;
+
+import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.redis.internal.RedisConstants;
+
+public abstract class AbstractZPopMaxIntegrationTest implements
RedisIntegrationTest {
+ private JedisCluster jedis;
+
+ @Before
+ public void setUp() {
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()),
REDIS_CLIENT_TIMEOUT);
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ jedis.close();
+ }
+
+ @Test
+ public void shouldError_givenWrongNumberOfArguments() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand("key", Protocol.Command.ZPOPMAX, "key", "1",
"2"))
+ .hasMessageContaining(RedisConstants.ERROR_SYNTAX);
+ }
+
+ @Test
+ public void shouldError_givenWrongNumberFormat() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand("key", Protocol.Command.ZPOPMAX, "key", "wat"))
+ .hasMessageContaining(RedisConstants.ERROR_NOT_INTEGER);
+ }
+
+ @Test
+ public void shouldReturnEmpty_givenNonExistentSortedSet() {
+ assertThat(jedis.zpopmax("unknown", 1)).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnEmpty_givenNegativeCount() {
+ jedis.zadd("key", 1, "player1");
+
+ List<?> result = (List<?>) jedis.sendCommand("key",
Protocol.Command.ZPOPMAX, "key", "-1");
+ assertThat(result).isEmpty();
+ assertThat(jedis.zrange("key", 0, 10)).containsExactly("player1");
+ }
+
+ @Test
+ public void shouldReturn_highestLex_whenScoresAreEqual() {
+ jedis.zadd("key", 1, "player1");
+ jedis.zadd("key", 1, "player2");
+ jedis.zadd("key", 1, "player3");
+
+ assertThat(jedis.zpopmax("key").getElement()).isEqualTo("player3");
+ assertThat(jedis.zrange("key", 0, 10))
+ .containsExactlyInAnyOrder("player1", "player2");
+ }
+
+ @Test
+ public void shouldReturn_highestScore() {
+ jedis.zadd("key", 3, "player1");
+ jedis.zadd("key", 2, "player2");
+ jedis.zadd("key", 1, "player3");
+
+ assertThat(jedis.zpopmax("key").getElement()).isEqualTo("player1");
+ assertThat(jedis.zrange("key", 0, 10))
+ .containsExactlyInAnyOrder("player2", "player3");
+ }
+
+ @Test
+ public void
withCountShouldReturn_membersInScoreOrder_whenScoresAreDifferent() {
+ List<Tuple> tuples = new ArrayList<>();
+ int count = 10;
+ // Make sure that the results are not somehow dependent on the order of
insertion
+ List<Integer> shuffles = makeShuffledList(count);
+ for (int i = 0; i < count; i++) {
+ jedis.zadd("key", count - shuffles.get(i), "player" + shuffles.get(i));
+ tuples.add(new Tuple("player" + i, (double) (count - i)));
+ }
+
+ assertThat(jedis.zpopmax("key", count)).containsExactlyElementsOf(tuples);
+ }
+
+ @Test
+ public void
withCountShouldReturn_membersInReverseLexicalOrder_whenScoresAreTheSame() {
+ List<Tuple> tuples = new ArrayList<>();
+ int count = 10;
+ // Make sure that the results are not somehow dependent on the order of
insertion
+ List<Integer> shuffles = makeShuffledList(count);
+ for (int i = 0; i < count; i++) {
+ jedis.zadd("key", 1D, "player" + shuffles.get(i));
+ tuples.add(new Tuple("player" + (count - i - 1), 1D));
+ }
+
+ assertThat(jedis.zpopmax("key", count)).containsExactlyElementsOf(tuples);
+ }
+
+ @Test
+ public void shouldReturn_countHighestScores() {
+ for (int i = 0; i < 5; i++) {
+ jedis.zadd("key", i, "player" + i);
+ }
+
+ assertThat(jedis.zpopmax("key", 3))
+ .containsExactlyInAnyOrder(
+ new Tuple("player2", 2D),
+ new Tuple("player3", 3D),
+ new Tuple("player4", 4D));
+
+ assertThat(jedis.zrange("key", 0, 10))
+ .containsExactly("player0", "player1");
+ }
+
+ /**
+ * Create a shuffled list of a range of integers from 0..count (exclusive).
+ */
+ private List<Integer> makeShuffledList(int count) {
+ List<Integer> result = new ArrayList<>(count);
+ for (int i = 0; i < count; i++) {
+ result.add(i);
+ }
+
+ Collections.shuffle(result);
+ return result;
+ }
+}
diff --git
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxIntegrationTest.java
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxIntegrationTest.java
new file mode 100755
index 0000000..2e79e6f
--- /dev/null
+++
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxIntegrationTest.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.sortedset;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.GeodeRedisServerRule;
+
+public class ZPopMaxIntegrationTest extends AbstractZPopMaxIntegrationTest {
+
+ @ClassRule
+ public static GeodeRedisServerRule server = new GeodeRedisServerRule();
+
+ @Override
+ public int getPort() {
+ return server.getPort();
+ }
+
+}
diff --git
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
index 60f7a30..10fe5a7 100755
---
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
+++
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
@@ -100,6 +100,7 @@ import
org.apache.geode.redis.internal.executor.sortedset.ZAddExecutor;
import org.apache.geode.redis.internal.executor.sortedset.ZCardExecutor;
import org.apache.geode.redis.internal.executor.sortedset.ZCountExecutor;
import org.apache.geode.redis.internal.executor.sortedset.ZIncrByExecutor;
+import org.apache.geode.redis.internal.executor.sortedset.ZPopMaxExecutor;
import org.apache.geode.redis.internal.executor.sortedset.ZRangeByLexExecutor;
import
org.apache.geode.redis.internal.executor.sortedset.ZRangeByScoreExecutor;
import org.apache.geode.redis.internal.executor.sortedset.ZRangeExecutor;
@@ -213,6 +214,8 @@ public enum RedisCommandType {
ZCARD(new ZCardExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
ZCOUNT(new ZCountExecutor(), SUPPORTED, new ExactParameterRequirements(4)),
ZINCRBY(new ZIncrByExecutor(), SUPPORTED, new ExactParameterRequirements(4)),
+ ZPOPMAX(new ZPopMaxExecutor(), SUPPORTED, new MinimumParameterRequirements(2)
+ .and(new MaximumParameterRequirements(3, ERROR_SYNTAX))),
ZRANGE(new ZRangeExecutor(), SUPPORTED, new MinimumParameterRequirements(4)
.and(new MaximumParameterRequirements(5, ERROR_SYNTAX))),
ZRANGEBYLEX(new ZRangeByLexExecutor(), SUPPORTED, new
MinimumParameterRequirements(4)),
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 94ea183..f58c75c 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
@@ -87,6 +87,11 @@ class NullRedisSortedSet extends RedisSortedSet {
}
@Override
+ List<byte[]> zpopmax(Region<RedisKey, RedisData> region, RedisKey key, int
count) {
+ return Collections.emptyList();
+ }
+
+ @Override
List<byte[]> zrange(int min, int max, boolean withScores) {
return Collections.emptyList();
}
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 f92ebae..ebfeb76 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
@@ -205,12 +205,16 @@ public class RedisSortedSet extends AbstractRedisData {
scoreSet.remove(orderedSetEntry);
oldValue = orderedSetEntry.getScoreBytes();
// Adjust for removal.
- sizeInBytesAdjustment -= orderedSetEntry.getSizeInBytes() +
calculateByteArraySize(member);
+ adjustSizeForRemoval(orderedSetEntry);
}
return oldValue;
}
+ private void adjustSizeForRemoval(AbstractOrderedSetEntry entry) {
+ sizeInBytesAdjustment -= entry.getSizeInBytes() +
calculateByteArraySize(entry.getMember());
+ }
+
private synchronized void membersAddAll(AddsDeltaInfo addsDeltaInfo) {
Iterator<byte[]> iterator = addsDeltaInfo.getAdds().iterator();
while (iterator.hasNext()) {
@@ -397,6 +401,32 @@ public class RedisSortedSet extends AbstractRedisData {
return null;
}
+ List<byte[]> zpopmax(Region<RedisKey, RedisData> region, RedisKey key, int
count) {
+ Iterator<AbstractOrderedSetEntry> scoresIterator =
+ scoreSet.getIndexRange(scoreSet.size() - 1, count, true);
+
+ if (!scoresIterator.hasNext()) {
+ return Collections.emptyList();
+ }
+
+ List<byte[]> result = new ArrayList<>();
+ RemsDeltaInfo deltaInfo = new RemsDeltaInfo();
+ while (scoresIterator.hasNext()) {
+ AbstractOrderedSetEntry entry = scoresIterator.next();
+ scoresIterator.remove();
+ members.remove(entry.member);
+ adjustSizeForRemoval(entry);
+
+ result.add(entry.member);
+ result.add(entry.scoreBytes);
+ deltaInfo.add(entry.member);
+ }
+
+ storeChanges(region, key, deltaInfo);
+
+ return result;
+ }
+
private byte[] zaddIncr(Region<RedisKey, RedisData> region, RedisKey key,
List<byte[]> membersToAdd, ZAddOptions options) {
// for zadd incr option, only one incrementing element pair is allowed to
get here.
diff --git
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java
index f2405a0..910afa0 100644
---
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java
+++
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java
@@ -102,4 +102,10 @@ public class RedisSortedSetCommandsFunctionExecutor
extends RedisDataCommandsFun
public byte[] zscore(RedisKey key, byte[] member) {
return stripedExecute(key, () -> getRedisSortedSet(key,
true).zscore(member));
}
+
+ @Override
+ public List<byte[]> zpopmax(RedisKey key, int count) {
+ return stripedExecute(key,
+ () -> getRedisSortedSet(key, false).zpopmax(getRegion(), key, count));
+ }
}
diff --git
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/RedisSortedSetCommands.java
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/RedisSortedSetCommands.java
index 444d4e0..6bcd46a 100644
---
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/RedisSortedSetCommands.java
+++
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/RedisSortedSetCommands.java
@@ -45,4 +45,6 @@ public interface RedisSortedSetCommands {
long zrevrank(RedisKey key, byte[] member);
byte[] zscore(RedisKey key, byte[] member);
+
+ List<byte[]> zpopmax(RedisKey key, int count);
}
diff --git
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxExecutor.java
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxExecutor.java
new file mode 100644
index 0000000..9ccbc90
--- /dev/null
+++
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZPopMaxExecutor.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.sortedset;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.RedisConstants;
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Coder;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public class ZPopMaxExecutor extends AbstractExecutor {
+ @Override
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
+ RedisSortedSetCommands redisSortedSetCommands =
context.getSortedSetCommands();
+
+ List<byte[]> commandElements = command.getProcessedCommand();
+
+ int count = 1;
+ if (commandElements.size() > 2) {
+ try {
+ count =
Coder.narrowLongToInt(Coder.bytesToLong(commandElements.get(2)));
+ } catch (NumberFormatException nex) {
+ return RedisResponse.error(RedisConstants.ERROR_NOT_INTEGER);
+ }
+ }
+
+ if (count < 1) {
+ return RedisResponse.emptyArray();
+ }
+
+ List<byte[]> result = redisSortedSetCommands.zpopmax(command.getKey(),
count);
+
+ return RedisResponse.array(result);
+ }
+}
diff --git
a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
index 3ba9e19..fb29e59 100644
---
a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
+++
b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
@@ -91,6 +91,7 @@ public class SupportedCommandsJUnitTest {
"ZINCRBY",
"ZCARD",
"ZCOUNT",
+ "ZPOPMAX",
"ZRANGE",
"ZRANGEBYLEX",
"ZRANGEBYSCORE",
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 176db29..0e3bc3e 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
@@ -45,6 +45,7 @@ import java.util.stream.Collectors;
import it.unimi.dsi.fastutil.bytes.ByteArrays;
import org.junit.BeforeClass;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
import org.apache.geode.DataSerializer;
import org.apache.geode.cache.Region;
@@ -475,6 +476,70 @@ public class RedisSortedSetTest {
assertThat(inclusiveMax.compareTo(realEntry)).isEqualTo(1);
}
+ @Test
+ public void zpopmaxRemovesMemberWithHighestScore() {
+ int originalSize = rangeSortedSet.getSortedSetSize();
+ RedisSortedSet sortedSet = spy(rangeSortedSet);
+ Region<RedisKey, RedisData> region = uncheckedCast(mock(Region.class));
+ RedisKey key = new RedisKey();
+ int count = 1;
+
+ List<byte[]> result = sortedSet.zpopmax(region, key, count);
+ assertThat(result).containsExactly("member12".getBytes(),
"2.1".getBytes());
+
+ ArgumentCaptor<RemsDeltaInfo> argumentCaptor =
ArgumentCaptor.forClass(RemsDeltaInfo.class);
+ verify(sortedSet).storeChanges(eq(region), eq(key),
argumentCaptor.capture());
+
assertThat(argumentCaptor.getValue().getRemoves()).containsExactly("member12".getBytes());
+ assertThat(rangeSortedSet.getSortedSetSize()).isEqualTo(originalSize -
count);
+ }
+
+ @Test
+ public void
zpopmaxRemovesMembersWithHighestScores_whenCountIsGreaterThanOne() {
+ int originalSize = rangeSortedSet.getSortedSetSize();
+ RedisSortedSet sortedSet = spy(rangeSortedSet);
+ Region<RedisKey, RedisData> region = uncheckedCast(mock(Region.class));
+ RedisKey key = new RedisKey();
+ int count = 3;
+
+ List<byte[]> result = sortedSet.zpopmax(region, key, count);
+ assertThat(result).containsExactlyInAnyOrder("member10".getBytes(),
"1.9".getBytes(),
+ "member11".getBytes(), "2".getBytes(), "member12".getBytes(),
"2.1".getBytes());
+
+ ArgumentCaptor<RemsDeltaInfo> argumentCaptor =
ArgumentCaptor.forClass(RemsDeltaInfo.class);
+ verify(sortedSet).storeChanges(eq(region), eq(key),
argumentCaptor.capture());
+
assertThat(argumentCaptor.getValue().getRemoves()).containsExactlyInAnyOrder(
+ "member10".getBytes(), "member11".getBytes(), "member12".getBytes());
+ assertThat(rangeSortedSet.getSortedSetSize()).isEqualTo(originalSize -
count);
+ }
+
+ @Test
+ public void zpopmaxRemovesRegionEntryWhenSetBecomesEmpty() {
+ RedisSortedSet sortedSet = spy(createRedisSortedSet(score1, member1));
+ Region<RedisKey, RedisData> region = uncheckedCast(mock(Region.class));
+ RedisKey key = new RedisKey();
+
+ List<byte[]> result = sortedSet.zpopmax(region, key, 1);
+ assertThat(result).containsExactly(member1.getBytes(), score1.getBytes());
+
+ verify(sortedSet).storeChanges(eq(region), eq(key),
any(RemsDeltaInfo.class));
+ verify(region).remove(key);
+ }
+
+ @Test
+ public void zpopmaxRemovesHighestLexWhenScoresAreEqual() {
+ RedisSortedSet sortedSet = spy(createRedisSortedSet(
+ "1.1", "member5",
+ "1.1", "member4",
+ "1.1", "member3",
+ "1.1", "member2",
+ "1.1", "member1"));
+ Region<RedisKey, RedisData> region = uncheckedCast(mock(Region.class));
+ RedisKey key = new RedisKey();
+
+ List<byte[]> result = sortedSet.zpopmax(region, key, 1);
+ assertThat(result).containsExactly("member5".getBytes(), "1.1".getBytes());
+ }
+
/******** constants *******/
// These tests contain the math that is used to derive the constants in
RedisSortedSet and
// OrderedSetEntry. If these tests start failing, it is because the
overheads of RedisSortedSet or
@@ -507,7 +572,7 @@ public class RedisSortedSetTest {
/****************** Size ******************/
@Test
- public void
redisSortedSetGetSizeInBytes_isAccurateForAddsUpdatesAndRemoves() {
+ public void redisSortedSetGetSizeInBytes_isAccurateForAdds() {
Region<RedisKey, RedisData> mockRegion = uncheckedCast(mock(Region.class));
RedisKey mockKey = mock(RedisKey.class);
ZAddOptions options = new ZAddOptions(ZAddOptions.Exists.NONE, false,
false);
@@ -527,15 +592,41 @@ public class RedisSortedSetTest {
calculatedSize = sortedSet.getSizeInBytes();
assertThat(calculatedSize).isEqualTo(actualSize);
}
+ }
+
+ @Test
+ public void redisSortedSetGetSizeInBytes_isAccurateForUpdates() {
+ Region<RedisKey, RedisData> mockRegion = uncheckedCast(mock(Region.class));
+ RedisKey mockKey = mock(RedisKey.class);
+ ZAddOptions options = new ZAddOptions(ZAddOptions.Exists.NONE, false,
false);
+ RedisSortedSet sortedSet = new RedisSortedSet(Collections.emptyList());
+
+ int numberOfEntries = 100;
+ for (int i = 0; i < numberOfEntries; ++i) {
+ List<byte[]> scoreAndMember = Arrays.asList(doubleToBytes(i), new
byte[i]);
+ sortedSet.zadd(mockRegion, mockKey, scoreAndMember, options);
+ }
// Update half the scores and confirm that the calculated size is accurate
after each operation
for (int i = 0; i < numberOfEntries / 2; ++i) {
List<byte[]> scoreAndMember = Arrays.asList(doubleToBytes(i * 2), new
byte[i]);
sortedSet.zadd(mockRegion, mockKey, scoreAndMember, options);
sortedSet.clearDelta();
- actualSize = sizer.sizeof(sortedSet);
- calculatedSize = sortedSet.getSizeInBytes();
- assertThat(calculatedSize).isEqualTo(actualSize);
+
assertThat(sizer.sizeof(sortedSet)).isEqualTo(sortedSet.getSizeInBytes());
+ }
+ }
+
+ @Test
+ public void redisSortedSetGetSizeInBytes_isAccurateForRemoves() {
+ Region<RedisKey, RedisData> mockRegion = uncheckedCast(mock(Region.class));
+ RedisKey mockKey = mock(RedisKey.class);
+ ZAddOptions options = new ZAddOptions(ZAddOptions.Exists.NONE, false,
false);
+ RedisSortedSet sortedSet = new RedisSortedSet(Collections.emptyList());
+
+ int numberOfEntries = 100;
+ for (int i = 0; i < numberOfEntries; ++i) {
+ List<byte[]> scoreAndMember = Arrays.asList(doubleToBytes(i), new
byte[i]);
+ sortedSet.zadd(mockRegion, mockKey, scoreAndMember, options);
}
// Remove all members and confirm that the calculated size is accurate
after each operation
@@ -543,9 +634,29 @@ public class RedisSortedSetTest {
List<byte[]> memberToRemove = Collections.singletonList(new byte[i]);
sortedSet.zrem(mockRegion, mockKey, memberToRemove);
sortedSet.clearDelta();
- actualSize = sizer.sizeof(sortedSet);
- calculatedSize = sortedSet.getSizeInBytes();
- assertThat(calculatedSize).isEqualTo(actualSize);
+
assertThat(sizer.sizeof(sortedSet)).isEqualTo(sortedSet.getSizeInBytes());
+ }
+ }
+
+
+ @Test
+ public void redisSortedSetGetSizeInBytes_isAccurateForZpopmax() {
+ Region<RedisKey, RedisData> mockRegion = uncheckedCast(mock(Region.class));
+ RedisKey mockKey = mock(RedisKey.class);
+ ZAddOptions options = new ZAddOptions(ZAddOptions.Exists.NONE, false,
false);
+ RedisSortedSet sortedSet = new RedisSortedSet(Collections.emptyList());
+
+ int numberOfEntries = 100;
+ for (int i = 0; i < numberOfEntries; ++i) {
+ List<byte[]> scoreAndMember = Arrays.asList(doubleToBytes(i), new
byte[i]);
+ sortedSet.zadd(mockRegion, mockKey, scoreAndMember, options);
+ }
+
+ // Remove all members by zpopmax and ensure size is correct
+ for (int i = 0; i < numberOfEntries; ++i) {
+ sortedSet.zpopmax(mockRegion, mockKey, 1);
+ sortedSet.clearDelta();
+
assertThat(sizer.sizeof(sortedSet)).isEqualTo(sortedSet.getSizeInBytes());
}
}
