DonalEvans commented on a change in pull request #6700:
URL: https://github.com/apache/geode/pull/6700#discussion_r671431145
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/netty/StringBytesGlossary.java
##########
@@ -238,6 +238,12 @@
@MakeImmutable
public static final byte[] bRADISH_DUMP_HEADER = stringToBytes("RADISH");
+ @MakeImmutable
+ public static final byte[] bRADISH_WITHSCORES = stringToBytes("WITHSCORES");
Review comment:
There is already a byte array constant for `WITHSCORES` further up in
this class (line 180), so this one is not necessary.
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,269 @@
+/*
+ * 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.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+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.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
+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;
+
+public abstract class AbstractZRangeByScoreIntegrationTest implements
RedisIntegrationTest {
+ public static final String KEY = "key";
+ 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() {
+ assertAtLeastNArgs(jedis, Protocol.Command.ZRANGEBYSCORE, 3);
+ }
+
+ @Test
+ public void shouldError_givenInvalidMinOrMax() {
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "notANumber", "1"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "1", "notANumber"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "notANumber",
"notANumber"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "((", "1"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "1", "(("))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "(a", "(b"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ }
+
+ @Test
+ public void shouldReturnZero_givenNonExistentKey() {
+ assertThat(jedis.zrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnZero_givenMinGreaterThanMax() {
+ jedis.zadd(KEY, 1, "member");
+
+ // Count +inf <= score <= -inf
+ assertThat(jedis.zrangeByScore(KEY, "+inf", "-inf")).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnCount_givenRangeIncludingScore() {
Review comment:
This test name should probably be
"shouldReturnMember_givenRangeIncludingScore" or something similar.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZRangeByScoreExecutor.java
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.redis.internal.netty.Coder.bytesToLong;
+import static
org.apache.geode.redis.internal.netty.Coder.equalsIgnoreCaseBytes;
+import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
+import static
org.apache.geode.redis.internal.netty.StringBytesGlossary.bRADISH_LIMIT;
+import static
org.apache.geode.redis.internal.netty.StringBytesGlossary.bRADISH_WITHSCORES;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public class ZRangeByScoreExecutor extends AbstractExecutor {
+ @Override
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
+ RedisSortedSetCommands redisSortedSetCommands =
context.getSortedSetCommands();
+
+ List<byte[]> commandElements = command.getProcessedCommand();
+
+ SortedSetRangeOptions rangeOptions;
+ boolean withScores = false;
+
+ try {
+ byte[] minBytes = commandElements.get(2);
+ byte[] maxBytes = commandElements.get(3);
+ rangeOptions = new SortedSetRangeOptions(minBytes, maxBytes);
+ } catch (NumberFormatException ex) {
+ return RedisResponse.error(ERROR_MIN_MAX_NOT_A_FLOAT);
+ }
+
+ // Native redis allows multiple "withscores" and "limit ? ?" clauses; the
last "limit"
+ // clause overrides any previous ones
+ if (commandElements.size() >= 5) {
+ int currentCommandElement = 4;
+
+ while (currentCommandElement < commandElements.size()) {
+ try {
+ if (equalsIgnoreCaseBytes(commandElements.get(currentCommandElement),
+ bRADISH_WITHSCORES)) {
+ withScores = true;
+ currentCommandElement++;
+ } else {
+ parseLimitArguments(rangeOptions, commandElements,
currentCommandElement);
+ currentCommandElement += 3;
+ }
+ } catch (NumberFormatException nfex) {
+ return RedisResponse.error(ERROR_NOT_INTEGER);
+ } catch (IllegalArgumentException iex) {
+ return RedisResponse.error(ERROR_SYNTAX);
+ }
+ }
+ }
+
+ // If the range is empty (min > max or min == max and both are exclusive),
or
+ // limit specified but count is zero, return early
+ if ((rangeOptions.hasLimit() && (rangeOptions.getCount() == 0 ||
rangeOptions.getOffset() < 0))
+ ||
+ rangeOptions.getMinDouble() > rangeOptions.getMaxDouble() ||
+ (rangeOptions.getMinDouble() == rangeOptions.getMaxDouble())
+ && rangeOptions.isMinExclusive() && rangeOptions.isMaxExclusive())
{
+ return RedisResponse.emptyArray();
+ }
+
+ List<byte[]> result =
+ redisSortedSetCommands.zrangebyscore(command.getKey(), rangeOptions,
withScores);
+
+ return RedisResponse.array(result);
+ }
+
+ void parseLimitArguments(SortedSetRangeOptions rangeOptions, List<byte[]>
commandElements,
+ int commandIndex) {
+ int offset;
+ int count;
+ if (equalsIgnoreCaseBytes(commandElements.get(commandIndex),
bRADISH_LIMIT)) {
+ offset = narrowLongToInt(bytesToLong(commandElements.get(commandIndex +
1)));
+ count = narrowLongToInt(bytesToLong(commandElements.get(commandIndex +
2)));
Review comment:
With an improperly provided `LIMIT` argument (one or both of `offset` or
`count` not supplied) this code will throw an `ArrayIndexOutOfBoundsException`
which is not handled, so no syntax error will be returned to the client.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
##########
@@ -213,10 +214,10 @@
ZINCRBY(new ZIncrByExecutor(), SUPPORTED, new ExactParameterRequirements(4)),
ZRANGE(new ZRangeExecutor(), SUPPORTED, new MinimumParameterRequirements(4)
.and(new MaximumParameterRequirements(5, ERROR_SYNTAX))),
+ ZRANGEBYSCORE(new ZRangeByScoreExecutor(), SUPPORTED, new
MinimumParameterRequirements(4)),
ZRANK(new ZRankExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
ZREM(new ZRemExecutor(), SUPPORTED, new MinimumParameterRequirements(3)),
- ZREVRANGE(new ZRevRangeExecutor(), SUPPORTED, new
MinimumParameterRequirements(4)
- .and(new MaximumParameterRequirements(5, ERROR_SYNTAX))),
Review comment:
This change should be reverted as ZREVRANGE should return a syntax error
if more than 5 arguments are passed to it.
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,269 @@
+/*
+ * 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.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+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.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
+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;
+
+public abstract class AbstractZRangeByScoreIntegrationTest implements
RedisIntegrationTest {
+ public static final String KEY = "key";
+ 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() {
+ assertAtLeastNArgs(jedis, Protocol.Command.ZRANGEBYSCORE, 3);
+ }
+
+ @Test
+ public void shouldError_givenInvalidMinOrMax() {
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "notANumber", "1"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "1", "notANumber"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "notANumber",
"notANumber"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "((", "1"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "1", "(("))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "(a", "(b"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ }
+
+ @Test
+ public void shouldReturnZero_givenNonExistentKey() {
+ assertThat(jedis.zrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnZero_givenMinGreaterThanMax() {
+ jedis.zadd(KEY, 1, "member");
+
+ // Count +inf <= score <= -inf
+ assertThat(jedis.zrangeByScore(KEY, "+inf", "-inf")).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnCount_givenRangeIncludingScore() {
+ jedis.zadd(KEY, 1, "member");
+
+ // Count -inf <= score <= +inf
+ assertThat(jedis.zrangeByScore(KEY, "-inf", "inf"))
+ .containsExactly("member");
+ }
+
+ @Test
+ public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+ int score = 1;
+ jedis.zadd(KEY, score, "member");
+
+ // Count 2 <= score <= 3
+ assertThat(jedis.zrangeByScore(KEY, score + 1, score + 2)).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+ int score = 1;
+ jedis.zadd(KEY, score, "member");
+
+ // Count 1 <= score <= 1
+ assertThat(jedis.zrangeByScore(KEY, score, score))
+ .containsExactly("member");
+ }
+
+ @Test
+ public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+ Map<String, Double> map = new HashMap<>();
+
+ map.put("member1", -10.0);
+ map.put("member2", 1.0);
+ map.put("member3", 10.0);
+
+ jedis.zadd(KEY, map);
+
+ // Count -5 <= score <= 15
+ assertThat(jedis.zrangeByScore(KEY, "-5", "15"))
+ .containsExactlyElementsOf(Arrays.asList("member2", "member3"));
+ }
+
+ @Test
+ public void
shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore()
{
+ Map<String, Double> map = new HashMap<>();
+ double score = 1;
+ map.put("member1", score);
+ map.put("member2", score);
+ map.put("member3", score);
+
+ jedis.zadd(KEY, map);
+
+ // Count 1 <= score <= 1
+ assertThat(jedis.zrangeByScore(KEY, score, score))
+ .containsExactlyInAnyOrderElementsOf(map.keySet());
+ }
+
+ @Test
+ public void shouldReturnRange_givenExclusiveMin() {
+ Map<String, Double> map = new HashMap<>();
+
+ map.put("member1", Double.NEGATIVE_INFINITY);
+ map.put("member2", 1.0);
+ map.put("member3", Double.POSITIVE_INFINITY);
+
+ jedis.zadd(KEY, map);
+
+ // Count -inf < score <= +inf
+ assertThat(jedis.zrangeByScore(KEY, "(-inf", "+inf"))
+ .containsExactlyElementsOf(Arrays.asList("member2", "member3"));
Review comment:
This can be simplified to `.containsExactly("member2", "member3")`. This
also applies to other places in this test where `Arrays.asList()` and
`containsExactlyElementsOf()` are used together.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/netty/StringBytesGlossary.java
##########
@@ -238,6 +238,12 @@
@MakeImmutable
public static final byte[] bRADISH_DUMP_HEADER = stringToBytes("RADISH");
+ @MakeImmutable
+ public static final byte[] bRADISH_WITHSCORES = stringToBytes("WITHSCORES");
+
+ @MakeImmutable
+ public static final byte[] bRADISH_LIMIT = stringToBytes("LIMIT");
Review comment:
Could this be renamed "bLIMIT" to match the naming of other byte array
constants used in executors, and moved further up in this class to be with the
other executor-related constants?
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -307,6 +308,58 @@ long zcount(SortedSetRangeOptions rangeOptions) {
return getRange(min, max, withScores, false);
}
+
+ List<byte[]> zrangebyscore(SortedSetRangeOptions rangeOptions, boolean
withScores) {
+ List<byte[]> result = new ArrayList<>();
+ AbstractOrderedSetEntry minEntry =
+ new DummyOrderedSetEntry(rangeOptions.getMinDouble(),
rangeOptions.isMinExclusive(), true);
+ int minIndex = scoreSet.indexOf(minEntry);
+ if (minIndex >= scoreSet.size()) {
+ return Collections.emptyList();
+ }
+
+ AbstractOrderedSetEntry maxEntry =
+ new DummyOrderedSetEntry(rangeOptions.getMaxDouble(),
rangeOptions.isMaxExclusive(), false);
+ int maxIndex = scoreSet.indexOf(maxEntry);
+ if (minIndex == maxIndex) {
+ return Collections.emptyList();
+ }
+
+ // Okay, if we make it this far there's a potential range of things to
return.
+ int offset = 0;
+ int count = Integer.MAX_VALUE;
+ Iterator<AbstractOrderedSetEntry> entryIterator =
+ scoreSet.getIndexRange(minIndex, maxIndex, false);
+ if (rangeOptions.hasLimit()) {
+ count = rangeOptions.getCount();
+ offset = rangeOptions.getOffset();
+ }
Review comment:
Rather than getting the range iterator and then skipping to the offset,
might it be neater to add the offset to `minIndex` and then retrieve the range
iterator? Similarly, I think you should be able to pass `Math.min(count,
maxIndex - minIndex)` as the second argument of `getIndexRange()` so that you
get an iterator that's automatically the right size. Then you can just iterate
the entire length and return that with no further checks needed.
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,269 @@
+/*
+ * 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.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+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.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
+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;
+
+public abstract class AbstractZRangeByScoreIntegrationTest implements
RedisIntegrationTest {
+ public static final String KEY = "key";
+ 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() {
+ assertAtLeastNArgs(jedis, Protocol.Command.ZRANGEBYSCORE, 3);
+ }
+
+ @Test
+ public void shouldError_givenInvalidMinOrMax() {
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "notANumber", "1"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "1", "notANumber"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "notANumber",
"notANumber"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "((", "1"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "1", "(("))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ assertThatThrownBy(() -> jedis.zrangeByScore("fakeKey", "(a", "(b"))
+ .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+ }
+
+ @Test
+ public void shouldReturnZero_givenNonExistentKey() {
+ assertThat(jedis.zrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnZero_givenMinGreaterThanMax() {
+ jedis.zadd(KEY, 1, "member");
+
+ // Count +inf <= score <= -inf
Review comment:
This comment (and others in this class) is a little misleading, as these
tests aren't returning a count, but rather a range.
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,269 @@
+/*
+ * 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.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+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.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
+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;
+
+public abstract class AbstractZRangeByScoreIntegrationTest implements
RedisIntegrationTest {
Review comment:
Could you also add tests to confirm the behaviour of ZRANGEBYSCORE with
multiple instances of `WITHSCORES` and `LIMIT`, and with incorrectly formatted
`LIMIT` arguments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
