This is an automated email from the ASF dual-hosted git repository.
nnag 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 c752491 GEODE-8934: add incrby to supported commands (#6022)
c752491 is described below
commit c7524917067b5db4648315fa3778e8925e1d440f
Author: Hale Bales <[email protected]>
AuthorDate: Mon Feb 22 07:12:35 2021 -0800
GEODE-8934: add incrby to supported commands (#6022)
- remove duplicate test and refactor existing tests
- add test for incrementing number greater than max int
(cherry picked from commit 4ffaa38951041e9e452d31a02760170de0c8f504)
---
.../string/AbstractIncrByIntegrationTest.java | 117 +++++++++++++++------
.../string/AbstractStringIntegrationTest.java | 61 ++++++-----
.../geode/redis/internal/RedisCommandType.java | 4 +-
.../redis/internal/SupportedCommandsJUnitTest.java | 4 +-
4 files changed, 121 insertions(+), 65 deletions(-)
diff --git
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrByIntegrationTest.java
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrByIntegrationTest.java
index 6b051c1..bad8fd8 100755
---
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrByIntegrationTest.java
+++
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrByIntegrationTest.java
@@ -15,9 +15,13 @@
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.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicInteger;
import org.junit.After;
import org.junit.Before;
@@ -25,12 +29,14 @@ import org.junit.Test;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.Protocol;
+import org.apache.geode.redis.ConcurrentLoopingThreads;
import org.apache.geode.test.awaitility.GeodeAwaitility;
import org.apache.geode.test.dunit.rules.RedisPortSupplier;
public abstract class AbstractIncrByIntegrationTest implements
RedisPortSupplier {
- private Jedis jedis;
+ private Jedis jedis1;
+ private Jedis jedis2;
private Random rand;
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@@ -38,49 +44,96 @@ public abstract class AbstractIncrByIntegrationTest
implements RedisPortSupplier
@Before
public void setUp() {
rand = new Random();
- jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ jedis1 = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ jedis2 = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
}
@After
public void tearDown() {
- jedis.flushAll();
- jedis.close();
+ jedis1.flushAll();
+ jedis1.close();
+ jedis2.flushAll();
+ jedis2.close();
}
@Test
public void errors_givenWrongNumberOfArguments() {
- assertExactNumberOfArgs(jedis, Protocol.Command.INCRBY, 2);
+ assertExactNumberOfArgs(jedis1, Protocol.Command.INCRBY, 2);
}
@Test
- public void testIncrBy() {
- String key1 = randString();
- String key2 = randString();
- String key3 = randString();
- int incr1 = rand.nextInt(100);
- int incr2 = rand.nextInt(100);
- Long incr3 = Long.MAX_VALUE / 2;
- int num1 = 100;
- int num2 = -100;
- jedis.set(key1, "" + num1);
- jedis.set(key2, "" + num2);
- jedis.set(key3, "" + Long.MAX_VALUE);
-
- jedis.incrBy(key1, incr1);
- jedis.incrBy(key2, incr2);
- assertThat(jedis.get(key1)).isEqualTo("" + (num1 + incr1 * 1));
- assertThat(jedis.get(key2)).isEqualTo("" + (num2 + incr2 * 1));
-
- Exception ex = null;
- try {
- jedis.incrBy(key3, incr3);
- } catch (Exception e) {
- ex = e;
- }
- assertThat(ex).isNotNull();
+ public void testIncrBy_failsWhenPerformedOnNonIntegerValue() {
+ jedis1.sadd("key", "member");
+ assertThatThrownBy(() -> jedis1.incrBy("key", 1))
+ .hasMessageContaining("WRONGTYPE Operation against a key holding the
wrong kind of value");
}
- private String randString() {
- return Long.toHexString(Double.doubleToLongBits(Math.random()));
+ @Test
+ public void testIncrBy_createsAndIncrementsNonExistentKey() {
+ assertThat(jedis1.incrBy("nonexistentkey", 1)).isEqualTo(1);
+ assertThat(jedis1.incrBy("otherNonexistentKey", -1)).isEqualTo(-1);
}
+
+ @Test
+ public void incrBy_incrementsPositiveIntegerValue() {
+ String key = "key";
+ int num = 100;
+ int increment = rand.nextInt(100);
+
+ jedis1.set(key, String.valueOf(num));
+ jedis1.incrBy(key, increment);
+ assertThat(jedis1.get(key)).isEqualTo(String.valueOf(num + increment));
+ }
+
+ @Test
+ public void incrBy_incrementsNegativeValue() {
+ String key = "key";
+ int num = -100;
+ int increment = rand.nextInt(100);
+
+ jedis1.set(key, "" + num);
+ jedis1.incrBy(key, increment);
+ assertThat(jedis1.get(key)).isEqualTo(String.valueOf(num + increment));
+ }
+
+ @Test
+ public void testIncrBy_IncrementingMaxValueThrowsError() {
+ String key = "key";
+ Long increment = Long.MAX_VALUE / 2;
+
+ jedis1.set(key, String.valueOf(Long.MAX_VALUE));
+ assertThatThrownBy(() -> jedis1.incrBy(key, increment))
+ .hasMessageContaining("ERR increment or decrement would overflow");
+ }
+
+ @Test
+ public void testConcurrentIncrBy_performsAllIncrBys() {
+ String key = "key";
+ AtomicInteger expectedValue = new AtomicInteger(0);
+
+ jedis1.set(key, "0");
+
+ new ConcurrentLoopingThreads(1000,
+ (i) -> {
+ int increment = ThreadLocalRandom.current().nextInt(-50, 50);
+ expectedValue.addAndGet(increment);
+ jedis1.incrBy(key, increment);
+ },
+ (i) -> {
+ int increment = ThreadLocalRandom.current().nextInt(-50, 50);
+ expectedValue.addAndGet(increment);
+ jedis2.incrBy(key, increment);
+ }).run();
+
+
assertThat(Integer.parseInt(jedis1.get(key))).isEqualTo(expectedValue.get());
+ }
+
+ @Test
+ public void testIncrByErrorsForValuesGreaterThatMaxInt() {
+ jedis1.set("key", "9223372036854775808");
+
+ assertThatThrownBy(() -> jedis1.incrBy("key",
1)).hasMessageContaining(ERROR_NOT_INTEGER);
+ }
+
}
diff --git
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStringIntegrationTest.java
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStringIntegrationTest.java
index ecc2144..73d4385 100755
---
a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStringIntegrationTest.java
+++
b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStringIntegrationTest.java
@@ -33,29 +33,32 @@ import org.apache.geode.test.dunit.rules.RedisPortSupplier;
public abstract class AbstractStringIntegrationTest implements
RedisPortSupplier {
- private Jedis jedis;
+ private Jedis jedis1;
+ private Jedis jedis2;
+
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@Before
public void setUp() {
- jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ jedis1 = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ jedis2 = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
}
@After
public void tearDown() {
- jedis.flushAll();
- jedis.close();
+ jedis1.flushAll();
+ jedis1.close();
}
@Test
public void strlen_errorsGivenWrongNumberOfArguments() {
- assertExactNumberOfArgs(jedis, Protocol.Command.STRLEN, 1);
+ assertExactNumberOfArgs(jedis1, Protocol.Command.STRLEN, 1);
}
@Test
public void testStrlen_requestNonexistentKey_returnsZero() {
- Long result = jedis.strlen("Nohbdy");
+ Long result = jedis1.strlen("Nohbdy");
assertThat(result).isEqualTo(0);
}
@@ -63,18 +66,18 @@ public abstract class AbstractStringIntegrationTest
implements RedisPortSupplier
public void testStrlen_requestKey_returnsLengthOfStringValue() {
String value = "byGoogle";
- jedis.set("golang", value);
+ jedis1.set("golang", value);
- Long result = jedis.strlen("golang");
+ Long result = jedis1.strlen("golang");
assertThat(result).isEqualTo(value.length());
}
@Test
public void testStrlen_requestWrongType_shouldReturnError() {
String key = "hashKey";
- jedis.hset(key, "field", "this value doesn't matter");
+ jedis1.hset(key, "field", "this value doesn't matter");
- assertThatThrownBy(() -> jedis.strlen(key))
+ assertThatThrownBy(() -> jedis1.strlen(key))
.isInstanceOf(JedisDataException.class)
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@@ -82,57 +85,57 @@ public abstract class AbstractStringIntegrationTest
implements RedisPortSupplier
@Test
public void testStrlen_withEmptyByte() {
byte[] key = new byte[] {0};
- jedis.set(key, new byte[] {});
+ jedis1.set(key, new byte[] {});
- assertThat(jedis.strlen(key)).isEqualTo(0);
+ assertThat(jedis1.strlen(key)).isEqualTo(0);
}
@Test
public void testStrlen_withBinaryData() {
byte[] zero = new byte[] {0};
- jedis.set(zero, zero);
+ jedis1.set(zero, zero);
- assertThat(jedis.strlen(zero)).isEqualTo(1);
+ assertThat(jedis1.strlen(zero)).isEqualTo(1);
}
@Test
public void testStrlen_withUTF16BinaryData() {
String test_utf16_string = "最𐐷𤭢";
byte[] testBytes = test_utf16_string.getBytes(StandardCharsets.UTF_16);
- jedis.set(testBytes, testBytes);
+ jedis1.set(testBytes, testBytes);
- assertThat(jedis.strlen(testBytes)).isEqualTo(12);
+ assertThat(jedis1.strlen(testBytes)).isEqualTo(12);
}
@Test
public void testStrlen_withIntData() {
byte[] key = new byte[] {0};
byte[] value = new byte[] {1, 0, 0};
- jedis.set(key, value);
+ jedis1.set(key, value);
- assertThat(jedis.strlen(key)).isEqualTo(value.length);
+ assertThat(jedis1.strlen(key)).isEqualTo(value.length);
}
@Test
public void testStrlen_withFloatData() {
byte[] key = new byte[] {0};
byte[] value = new byte[] {'0', '.', '9'};
- jedis.set(key, value);
+ jedis1.set(key, value);
- assertThat(jedis.strlen(key)).isEqualTo(value.length);
+ assertThat(jedis1.strlen(key)).isEqualTo(value.length);
}
@Test
public void testDecr_ErrorsWithWrongNumberOfArguments() {
- assertExactNumberOfArgs(jedis, Protocol.Command.DECR, 1);
+ assertExactNumberOfArgs(jedis1, Protocol.Command.DECR, 1);
}
@Test
public void testDecr_withWrongType_shouldError() {
String key = "hashKey";
- jedis.hset(key, "field", "non-int value");
+ jedis1.hset(key, "field", "non-int value");
- assertThatThrownBy(() -> jedis.decr(key))
+ assertThatThrownBy(() -> jedis1.decr(key))
.isInstanceOf(JedisDataException.class)
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@@ -140,18 +143,18 @@ public abstract class AbstractStringIntegrationTest
implements RedisPortSupplier
@Test
public void testDecr_decrementsPositiveIntegerValues() {
String key = "key";
- jedis.set(key, "10");
+ jedis1.set(key, "10");
- assertThat(jedis.decr(key)).isEqualTo(9L);
- assertThat(jedis.get(key)).isEqualTo("9");
+ assertThat(jedis1.decr(key)).isEqualTo(9L);
+ assertThat(jedis1.get(key)).isEqualTo("9");
}
@Test
public void testDecr_returnsValueWhenDecrementingResultsInNegativeNumber() {
String key = "key";
- jedis.set(key, "0");
+ jedis1.set(key, "0");
- assertThat(jedis.decr(key)).isEqualTo(-1L);
- assertThat(jedis.get(key)).isEqualTo("-1");
+ assertThat(jedis1.decr(key)).isEqualTo(-1L);
+ assertThat(jedis1.get(key)).isEqualTo("-1");
}
}
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 7a9b83a..3163b66 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
@@ -152,6 +152,8 @@ public enum RedisCommandType {
DECR(new DecrExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
DECRBY(new DecrByExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
GET(new GetExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
+ INCRBY(new IncrByExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
+ INCR(new IncrExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
GETRANGE(new GetRangeExecutor(), SUPPORTED, new
ExactParameterRequirements(4)),
INCRBYFLOAT(new IncrByFloatExecutor(), SUPPORTED, new
ExactParameterRequirements(3)),
MGET(new MGetExecutor(), SUPPORTED, new MinimumParameterRequirements(2)),
@@ -229,8 +231,6 @@ public enum RedisCommandType {
BITPOS(new BitPosExecutor(), UNSUPPORTED, new
MinimumParameterRequirements(3)),
GETBIT(new GetBitExecutor(), UNSUPPORTED, new ExactParameterRequirements(3)),
GETSET(new GetSetExecutor(), UNSUPPORTED, new ExactParameterRequirements(3)),
- INCR(new IncrExecutor(), UNSUPPORTED, new ExactParameterRequirements(2)),
- INCRBY(new IncrByExecutor(), UNSUPPORTED, new ExactParameterRequirements(3)),
MSET(new MSetExecutor(), UNSUPPORTED,
new MinimumParameterRequirements(3).and(new OddParameterRequirements())),
MSETNX(new MSetNXExecutor(), UNSUPPORTED,
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 1a1e526..ad86623 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
@@ -55,6 +55,8 @@ public class SupportedCommandsJUnitTest {
"HINCRBY",
"HVALS",
"HKEYS",
+ "INCRBY",
+ "INCR",
"INCRBYFLOAT",
"INFO",
"KEYS",
@@ -92,8 +94,6 @@ public class SupportedCommandsJUnitTest {
"FLUSHDB",
"GETBIT",
"GETSET",
- "INCR",
- "INCRBY",
"MSET",
"MSETNX",
"PSETEX",
