You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by sa...@apache.org on 2021/03/16 14:58:58 UTC
[geode] branch support/1.14 updated: GEODE-9009: Finish support for
functionality compatible with Redis DECRBY command
This is an automated email from the ASF dual-hosted git repository.
sabbey37 pushed a commit to branch support/1.14
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.14 by this push:
new 7053443 GEODE-9009: Finish support for functionality compatible with Redis DECRBY command
7053443 is described below
commit 70534438cc68bebfbdda75b7f567c1000f9d04db
Author: John Hutchison <jh...@gmail.com>
AuthorDate: Tue Mar 16 10:57:53 2021 -0400
GEODE-9009: Finish support for functionality compatible with Redis DECRBY command
(cherry-picked from <39c0abf96ae93a2a49fd0ab28ad2e96c1fa9e7a8>)
---
.../internal/executor/string/StringsDUnitTest.java | 18 ++-
.../string/AbstractDecrByIntegrationTest.java | 135 +++++++++++++++------
.../geode/redis/internal/RedisCommandType.java | 2 +-
.../redis/internal/SupportedCommandsJUnitTest.java | 2 +-
4 files changed, 118 insertions(+), 39 deletions(-)
diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/string/StringsDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/string/StringsDUnitTest.java
index 2b18a13..b21fbed 100644
--- a/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/string/StringsDUnitTest.java
+++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/string/StringsDUnitTest.java
@@ -98,7 +98,7 @@ public class StringsDUnitTest {
}
@Test
- public void set_shoulddistributeDataAmongMultipleServers_givenMultipleClients() {
+ public void set_shouldDistributeDataAmongMultipleServers_givenMultipleClients() {
List<String> keys = makeStringList(LIST_SIZE, "key1-");
List<String> values = makeStringList(LIST_SIZE, "values1-");
@@ -220,7 +220,7 @@ public class StringsDUnitTest {
}
@Test
- public void decr_shouldDecrementWhileDoingConcurrentDecrs() {
+ public void decr_shouldDecrementWhileDoingConcurrentDecr() {
String key = "key";
int initialValue = NUM_ITERATIONS * 2;
jedis1.set(key, String.valueOf(initialValue));
@@ -234,6 +234,19 @@ public class StringsDUnitTest {
}
@Test
+ public void decrby_shouldDecrementReliably_givenConcurrentThreadsPerformingDecrby() {
+ String key = "key";
+ int initialValue = NUM_ITERATIONS * 6;
+ jedis1.set(key, String.valueOf(initialValue));
+
+ new ConcurrentLoopingThreads(NUM_ITERATIONS,
+ (i) -> jedis1.decrBy(key, 4),
+ (i) -> jedis2.decrBy(key, 2)).run();
+
+ assertThat(jedis1.get(key)).isEqualTo("0");
+ }
+
+ @Test
public void strLen_returnsStringLengthWhileUpdatingValues() {
for (int i = 0; i < LIST_SIZE; i++) {
jedis1.set("key-" + i, "value-" + i);
@@ -257,6 +270,7 @@ public class StringsDUnitTest {
}
}
+
private List<String> makeStringList(int setSize, String baseString) {
List<String> strings = new ArrayList<>();
for (int i = 0; i < setSize; i++) {
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java
index af7d3d0..8e7fb88 100755
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java
@@ -16,10 +16,12 @@ package org.apache.geode.redis.internal.executor.string;
import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_OVERFLOW;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import java.util.Random;
+import java.math.BigInteger;
import org.junit.After;
import org.junit.Before;
@@ -33,13 +35,12 @@ import org.apache.geode.test.dunit.rules.RedisPortSupplier;
public abstract class AbstractDecrByIntegrationTest implements RedisPortSupplier {
private Jedis jedis;
- private Random rand;
+
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@Before
public void setUp() {
- rand = new Random();
jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
}
@@ -50,48 +51,112 @@ public abstract class AbstractDecrByIntegrationTest implements RedisPortSupplier
}
@Test
- public void decrBy_errors_givenWrongNumberOfArguments() {
+ public void shouldDecrementByGivenAmount_givenValidInputsAndKey() {
+ String key1 = "key1";
+ jedis.set(key1, "100");
+ jedis.decrBy(key1, 10);
+ String result = jedis.get(key1);
+ assertThat(Integer.parseInt(result)).isEqualTo(90);
+
+ jedis.set(key1, "100");
+ jedis.decrBy(key1, -10);
+ result = jedis.get(key1);
+ assertThat(Integer.parseInt(result)).isEqualTo(110);
+
+ String key2 = "key2";
+ jedis.set(key2, "-100");
+ jedis.decrBy(key2, 10);
+ result = jedis.get(key2);
+ assertThat(Integer.parseInt(result)).isEqualTo(-110);
+
+ jedis.set(key2, "-100");
+ jedis.decrBy(key2, -10);
+ result = jedis.get(key2);
+ assertThat(Integer.parseInt(result)).isEqualTo(-90);
+ }
+
+ @Test
+ public void should_returnNewValue_givenSuccessfulDecrby() {
+ jedis.set("key", "100");
+ Long decrByresult = jedis.decrBy("key", 50);
+
+ String getResult = jedis.get("key");
+
+ assertThat(decrByresult).isEqualTo(Long.valueOf(getResult)).isEqualTo(50l);
+ }
+
+
+ @Test
+ public void should_setKeyToZeroAndThenDecrement_givenKeyThatDoesNotExist() {
+
+ Long returnValue = jedis.decrBy("noneSuch", 10);
+ assertThat(returnValue).isEqualTo(-10);
+
+ }
+
+ @Test
+ public void shouldThrowError_givenWrongNumberOfArguments() {
assertExactNumberOfArgs(jedis, Protocol.Command.DECRBY, 2);
}
@Test
- public void testDecrBy() {
- String key1 = randString();
- String key2 = randString();
- String key3 = randString();
- int decr1 = rand.nextInt(100);
- int decr2 = rand.nextInt(100);
- Long decr3 = Long.MAX_VALUE / 2;
- int num1 = 100;
- int num2 = -100;
- jedis.set(key1, "" + num1);
- jedis.set(key2, "" + num2);
- jedis.set(key3, "" + Long.MIN_VALUE);
-
- jedis.decrBy(key1, decr1);
- jedis.decrBy(key2, decr2);
-
- assertThat(jedis.get(key1)).isEqualTo("" + (num1 - decr1 * 1));
- assertThat(jedis.get(key2)).isEqualTo("" + (num2 - decr2 * 1));
-
- Exception ex = null;
- try {
- jedis.decrBy(key3, decr3);
- } catch (Exception e) {
- ex = e;
- }
- assertThat(ex).isNotNull();
+ public void shouldThrowArithmeticException_givenDecrbyMoreThanMaxLong() {
+ jedis.set("somekey", "1");
+
+ BigInteger maxLongValue = new BigInteger(String.valueOf(Long.MAX_VALUE));
+ BigInteger biggerThanMaxLongValue = maxLongValue.add(new BigInteger("1"));
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.DECRBY,
+ "somekey", String.valueOf(biggerThanMaxLongValue)))
+ .hasMessageContaining(ERROR_NOT_INTEGER);
+
+ jedis.set("key", String.valueOf((Long.MIN_VALUE)));
}
@Test
- public void decrbyMoreThanMaxLongThrowsArithmeticException() {
+ public void shouldReturnArithmeticError_givenDecrbyLessThanMinLong() {
+
jedis.set("somekey", "1");
+
+ BigInteger minLongValue = new BigInteger(String.valueOf(Long.MIN_VALUE));
+ BigInteger smallerThanMinLongValue = minLongValue.subtract(new BigInteger("1"));
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(
+ Protocol.Command.DECRBY, "somekey",
+ smallerThanMinLongValue.toString()))
+ .hasMessageContaining(ERROR_NOT_INTEGER);
+ }
+
+ @Test
+ public void shouldReturnOverflowError_givenDecrbyThatWouldResultInValueLessThanMinLong() {
+
+ BigInteger minLongValue = new BigInteger(String.valueOf(Long.MIN_VALUE));
+ jedis.set("somekey", String.valueOf(minLongValue));
+
assertThatThrownBy(
- () -> jedis.sendCommand(Protocol.Command.DECRBY, "somekey", "9223372036854775809"))
- .hasMessageContaining(ERROR_NOT_INTEGER);
+ () -> jedis.sendCommand(
+ Protocol.Command.DECRBY, "somekey",
+ "1"))
+ .hasMessageContaining(ERROR_OVERFLOW);
}
- private String randString() {
- return Long.toHexString(Double.doubleToLongBits(Math.random()));
+
+ @Test
+ public void should_returnWrongTypeError_givenKeyContainsNonStringValue() {
+
+ jedis.hset("key", "1", "1");
+ assertThatThrownBy(
+ () -> jedis.decrBy("key", 1)).hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+
+ @Test
+ public void should_returnNotAnIntegerError_givenKeyContainsNonNumericStringValue() {
+
+ jedis.set("key", "walrus");
+
+ assertThatThrownBy(
+ () -> jedis.decrBy("key", 1)).hasMessageContaining(ERROR_NOT_INTEGER);
}
}
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
index b634567..5d847bd 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
@@ -151,6 +151,7 @@ public enum RedisCommandType {
APPEND(new AppendExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
DECR(new DecrExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
+ DECRBY(new DecrByExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
GET(new GetExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
INCRBYFLOAT(new IncrByFloatExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
MGET(new MGetExecutor(), SUPPORTED, new MinimumParameterRequirements(2)),
@@ -224,7 +225,6 @@ public enum RedisCommandType {
BITCOUNT(new BitCountExecutor(), UNSUPPORTED, new MinimumParameterRequirements(2)),
BITOP(new BitOpExecutor(), UNSUPPORTED, new MinimumParameterRequirements(4)),
BITPOS(new BitPosExecutor(), UNSUPPORTED, new MinimumParameterRequirements(3)),
- DECRBY(new DecrByExecutor(), UNSUPPORTED, new ExactParameterRequirements(3)),
GETBIT(new GetBitExecutor(), UNSUPPORTED, new ExactParameterRequirements(3)),
GETRANGE(new GetRangeExecutor(), UNSUPPORTED, new ExactParameterRequirements(4)),
GETSET(new GetSetExecutor(), UNSUPPORTED, new ExactParameterRequirements(3)),
diff --git a/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java b/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
index c1e9da9..f8bb15c 100644
--- a/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
+++ b/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
@@ -33,6 +33,7 @@ public class SupportedCommandsJUnitTest {
"APPEND",
"AUTH",
"DECR",
+ "DECRBY",
"DEL",
"EXISTS",
"EXPIRE",
@@ -82,7 +83,6 @@ public class SupportedCommandsJUnitTest {
"BITOP",
"BITPOS",
"DBSIZE",
- "DECRBY",
"ECHO",
"FLUSHALL",
"FLUSHDB",