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 3ddd9da Geode-8853: DUNIT tests for HSTRLEN command (#5939)
3ddd9da is described below
commit 3ddd9dae75b1cc5b0f3ed45148be225542eb4f57
Author: Ray Ingles <[email protected]>
AuthorDate: Tue Jan 26 09:11:59 2021 -0500
Geode-8853: DUNIT tests for HSTRLEN command (#5939)
- test the error messages for hgetall and hstrlen more specifically
- fix an hgetall test that was testing the wrong command
Co-authored-by: Ray Ingles <[email protected]>
Co-authored-by: Helena A. Bales <[email protected]>
---
.../tools_modules/redis_api_for_geode.html.md.erb | 5 +-
geode-redis/README.md | 88 ++++++++++----------
.../internal/executor/hash/HstrlenDUnitTest.java | 96 ++++++++++++++++++++++
.../hash/AbstractHashesIntegrationTest.java | 22 +++--
4 files changed, 158 insertions(+), 53 deletions(-)
diff --git a/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
b/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
index c4327a4..16a034f 100644
--- a/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
+++ b/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
@@ -68,7 +68,7 @@ The Redis API for <%=vars.product_name%> currently supports
the following comman
**Note**: These commands are supported for Redis 5.
- **Connection**: AUTH, PING, QUIT
-- **Hashes**: HGETALL, HMGET, HMSET, HSET, HVALS
+- **Hashes**: HGETALL, HMGET, HMSET, HSET, HSTRLEN, HVALS
- **Keys**: DEL, EXISTS, EXPIRE, EXPIREAT, KEYS, PERSIST, PEXPIRE,
PEXPIREAT, PTTL, RENAME, TTL,
TYPE
- **Publish/Subscribe**: PUBLISH, PSUBSCRIBE, PUNSUBSCRIBE, SUBSCRIBE,
UNSUBSCRIBE
@@ -80,8 +80,7 @@ commands are available to use, but have not been fully
tested. There is no guara
exactly as expected.
- **Connection**: ECHO, SELECT
-- **Hashes**: HDEL, HEXISTS, HGET, HINCRBY, HINCRBYFLOAT, HKEYS, HLEN,
HSCAN, HSETNX,
- HSTRLEN
+- **Hashes**: HDEL, HEXISTS, HGET, HINCRBY, HINCRBYFLOAT, HKEYS, HLEN,
HSCAN, HSETNX
- **Keys**: SCAN, UNLINK
- **Server**: DBSIZE, FLUSHALL (no async option), FLUSHDB (no async option),
INFO, SHUTDOWN,
SLOWLOG, TIME
diff --git a/geode-redis/README.md b/geode-redis/README.md
index f690001..b8fd770 100644
--- a/geode-redis/README.md
+++ b/geode-redis/README.md
@@ -165,51 +165,51 @@ start server \
| EXPIREAT | DECRBY
| ACL LIST |
| GET | ECHO
| ACL LOAD |
| HGETALL | FLUSHALL
| ACL LOG |
-| HMGET | FLUSHDB
| ACL SAVE |
+| HMGET | FLUSHDB
| ACL SAVE |
| HMSET | GETBIT
| ACL SETUSER |
-| HSET | GETRANGE
| ACL USERS |
-| HVALS | GETSET
| ACL WHOAMI |
-| KEYS | HDEL
| BGREWRITEAOF |
-| PERSIST | HEXISTS
| BGSAVE |
-| PEXPIRE | HGET
| BITFIELD |
-| PEXPIREAT | HINCRBY
| BLPOP |
-| PING | HINCRBYFLOAT
| BRPOP |
-| PSUBSCRIBE | HKEYS
| BRPOPLPUSH |
-| PTTL | HLEN
| BZPOPMAX |
-| PUBLISH | HSCAN
| BZPOPMIN |
-| PUNSUBSCRIBE | HSETNX
| CLIENT CACHING |
-| QUIT | HSTRLEN
| CLIENT GETNAME |
-| RENAME | INCR
| CLIENT GETREDIR |
-| SADD | INCRBY
| CLIENT ID |
-| SET | INCRBYFLOAT
| CLIENT KILL |
-| SMEMBERS | INFO
| CLIENT LIST |
-| SREM | MGET
| CLIENT PAUSE |
-| SUBSCRIBE | MSET
| CLIENT REPLY |
-| TTL | MSETNX
| CLIENT SETNAME |
-| TYPE | PSETEX
| CLIENT TRACKING |
-| UNSUBSCRIBE | SCAN
| CLIENT UNBLOCK |
-| | SCARD
| CLUSTER ADDSLOTS |
-| | SDIFF
| CLUSTER BUMPEPOCH |
-| | SDIFFSTORE
| CLUSTER COUNT-FAILURE-REPORTS |
-| | SELECT
| CLUSTER COUNTKEYSINSLOT |
-| | SETBIT
| CLUSTER DELSLOTS |
-| | SETEX
| CLUSTER FAILOVER |
-| | SETNX
| CLUSTER FLUSHSLOTS |
-| | SETRANGE
| CLUSTER FORGET |
-| | SHUTDOWN
| CLUSTER GETKEYSINSLOT |
-| | SINTER
| CLUSTER INFO |
-| | SINTERSTORE
| CLUSTER KEYSLOT |
-| | SISMEMBER
| CLUSTER MEET |
-| | SLOWLOG
| CLUSTER MYID |
-| | SMOVE
| CLUSTER NODES |
-| | SPOP
| CLUSTER REPLICAS |
-| | SRANDMEMBER
| CLUSTER REPLICATE |
-| | SSCAN
| CLUSTER RESET |
-| | STRLEN
| CLUSTER SAVECONFIG |
-| | SUNION
| CLUSTER SET-CONFIG-EPOCH |
-| | SUNIONSTORE
| CLUSTER SETSLOT |
-| | TIME
| CLUSTER SLAVES |
-| | UNLINK [1]
| CLUSTER SLOTS |
+| HSET | GETRANGE
| ACL USERS |
+| HSTRLEN | GETSET
| ACL WHOAMI |
+| HVALS | HDEL
| BGREWRITEAOF |
+| KEYS | HEXISTS
| BGSAVE |
+| PERSIST | HGET
| BITFIELD |
+| PEXPIRE | HINCRBY
| BLPOP |
+| PEXPIREAT | HINCRBYFLOAT
| BRPOP |
+| PING | HKEYS
| BRPOPLPUSH |
+| PSUBSCRIBE | HLEN
| BZPOPMAX |
+| PTTL | HSCAN
| BZPOPMIN |
+| PUBLISH | HSETNX
| CLIENT CACHING |
+| PUNSUBSCRIBE | INCR
| CLIENT GETNAME |
+| QUIT | INCRBY
| CLIENT GETREDIR |
+| RENAME | INCRBYFLOAT
| CLIENT ID |
+| SADD | INFO
| CLIENT KILL |
+| SET | MGET
| CLIENT LIST |
+| SMEMBERS | MSET
| CLIENT PAUSE |
+| SREM | MSETNX
| CLIENT REPLY |
+| SUBSCRIBE | PSETEX
| CLIENT SETNAME |
+| TTL | SCAN
| CLIENT TRACKING |
+| TYPE | SCARD
| CLIENT UNBLOCK |
+| UNSUBSCRIBE | SDIFF
| CLUSTER ADDSLOTS |
+| | SDIFFSTORE
| CLUSTER BUMPEPOCH |
+| | SELECT
| CLUSTER COUNT-FAILURE-REPORTS |
+| | SETBIT
| CLUSTER COUNTKEYSINSLOT |
+| | SETEX
| CLUSTER DELSLOTS |
+| | SETNX
| CLUSTER FAILOVER |
+| | SETRANGE
| CLUSTER FLUSHSLOTS |
+| | SHUTDOWN
| CLUSTER FORGET |
+| | SINTER
| CLUSTER GETKEYSINSLOT |
+| | SINTERSTORE
| CLUSTER INFO |
+| | SISMEMBER
| CLUSTER KEYSLOT |
+| | SLOWLOG
| CLUSTER MEET |
+| | SMOVE
| CLUSTER MYID |
+| | SPOP
| CLUSTER NODES |
+| | SRANDMEMBER
| CLUSTER REPLICAS |
+| | SSCAN
| CLUSTER REPLICATE |
+| | STRLEN
| CLUSTER RESET |
+| | SUNION
| CLUSTER SAVECONFIG |
+| | SUNIONSTORE
| CLUSTER SET-CONFIG-EPOCH |
+| | TIME
| CLUSTER SETSLOT |
+| | UNLINK [1]
| CLUSTER SLAVES |
+| |
| CLUSTER SLOTS |
| |
| COMMAND |
| |
| COMMAND COUNT |
| |
| COMMAND GETKEYS |
diff --git
a/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HstrlenDUnitTest.java
b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HstrlenDUnitTest.java
new file mode 100644
index 0000000..9fecd73
--- /dev/null
+++
b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HstrlenDUnitTest.java
@@ -0,0 +1,96 @@
+/*
+ * 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.hash;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Random;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.redis.ConcurrentLoopingThreads;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+
+public class HstrlenDUnitTest {
+
+ @ClassRule
+ public static RedisClusterStartupRule clusterStartUp = new
RedisClusterStartupRule(4);
+
+ private static final String LOCAL_HOST = "127.0.0.1";
+ private static final int JEDIS_TIMEOUT =
+ Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ private static Jedis jedis1;
+ private static Jedis jedis2;
+ private static Jedis jedis3;
+
+ @BeforeClass
+ public static void classSetup() {
+ MemberVM locator = clusterStartUp.startLocatorVM(0);
+ clusterStartUp.startRedisVM(1, locator.getPort());
+ clusterStartUp.startRedisVM(2, locator.getPort());
+
+ int redisServerPort1 = clusterStartUp.getRedisPort(1);
+ int redisServerPort2 = clusterStartUp.getRedisPort(2);
+
+ jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+ jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+ jedis3 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+ }
+
+ @Before
+ public void testSetup() {
+ jedis1.flushAll();
+ }
+
+ @Test
+ public void hstrlenDoesNotCorruptData_whileHashIsConcurrentlyUpdated() {
+ String key = "key";
+ String field = "field";
+ int iterations = 10000;
+ Random rand = new Random();
+
+ jedis1.hset(key, field, "22");
+
+ new ConcurrentLoopingThreads(iterations,
+ (i) -> {
+ int newLength = rand.nextInt(9) + 1;
+ String newVal = makeStringOfRepeatedDigits(newLength);
+ jedis1.hset(key, field, newVal);
+ },
+ (i) -> assertThat(jedis2.hstrlen(key, field)).isBetween(1L, 9L),
+ (i) -> assertThat(jedis3.hstrlen(key, field)).isBetween(1L, 9L))
+ .run();
+
+ String value = jedis1.hget(key, field);
+ String encodedStringLength = Character.toString(value.charAt(0));
+ int expectedLength = Integer.parseInt(encodedStringLength);
+ assertThat(jedis2.hstrlen(key, field)).isEqualTo(expectedLength);
+ }
+
+ private String makeStringOfRepeatedDigits(int newLength) {
+ String stringOfRepeatedDigits = "";
+ for (int i = 0; i < newLength; i++) {
+ stringOfRepeatedDigits += newLength;
+ }
+ return stringOfRepeatedDigits;
+ }
+}
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
index f0c3f2c..7c0de65 100755
---
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
@@ -97,8 +97,8 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
public void testHGetall_givenWrongNumberOfArguments() {
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HGETALL))
.hasMessage("ERR wrong number of arguments for 'hgetall' command");
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HMSET, "1",
"2"))
- .hasMessage("ERR wrong number of arguments for 'hmset' command");
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HGETALL, "1",
"2"))
+ .hasMessage("ERR wrong number of arguments for 'hgetall' command");
}
@Test
@@ -250,6 +250,16 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
}
@Test
+ public void testHStrlen_givenWrongNumberOfArguments() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSTRLEN))
+ .hasMessageContaining("ERR wrong number of arguments for 'hstrlen'
command");
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSTRLEN, "1"))
+ .hasMessageContaining("ERR wrong number of arguments for 'hstrlen'
command");
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSTRLEN, "1",
"2", "3"))
+ .hasMessageContaining("ERR wrong number of arguments for 'hstrlen'
command");
+ }
+
+ @Test
public void testHkeys() {
String key = "key";
Map<String, String> hash = new HashMap<String, String>();
@@ -440,13 +450,13 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
@Test
public void hsetnx_shouldThrowError_givenWrongNumberOfArguments() {
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX))
- .hasMessageContaining("wrong number of arguments");
+ .hasMessageContaining("ERR wrong number of arguments for 'hsetnx'
command");
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1"))
- .hasMessageContaining("wrong number of arguments");
+ .hasMessageContaining("ERR wrong number of arguments for 'hsetnx'
command");
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1",
"2"))
- .hasMessageContaining("wrong number of arguments");
+ .hasMessageContaining("ERR wrong number of arguments for 'hsetnx'
command");
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1",
"2", "3", "4"))
- .hasMessageContaining("wrong number of arguments");
+ .hasMessageContaining("ERR wrong number of arguments for 'hsetnx'
command");
}
/**
