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 bba1935 GEODE-8865: Create additional dunit and integration tests for
Redis HMGET (#5945)
bba1935 is described below
commit bba1935d98328c7b767adebded577f433e49ba87
Author: Jens Deppe <[email protected]>
AuthorDate: Sat Jan 23 13:20:44 2021 -0800
GEODE-8865: Create additional dunit and integration tests for Redis HMGET
(#5945)
---
.../tools_modules/redis_api_for_geode.html.md.erb | 4 +-
geode-redis/README.md | 92 ++++++++---------
.../internal/executor/hash/HMgetDUnitTest.java | 112 +++++++++++++++++++++
.../hash/AbstractHashesIntegrationTest.java | 59 +++++++++--
.../geode/redis/internal/RedisCommandType.java | 2 +-
.../redis/internal/SupportedCommandsJUnitTest.java | 2 +-
6 files changed, 215 insertions(+), 56 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 2887109..c4327a4 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, HMSET, HSET, HVALS
+- **Hashes**: HGETALL, HMGET, HMSET, HSET, HVALS
- **Keys**: DEL, EXISTS, EXPIRE, EXPIREAT, KEYS, PERSIST, PEXPIRE,
PEXPIREAT, PTTL, RENAME, TTL,
TYPE
- **Publish/Subscribe**: PUBLISH, PSUBSCRIBE, PUNSUBSCRIBE, SUBSCRIBE,
UNSUBSCRIBE
@@ -80,7 +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,
HMGET, HSCAN, HSETNX,
+- **Hashes**: HDEL, HEXISTS, HGET, HINCRBY, HINCRBYFLOAT, HKEYS, HLEN,
HSCAN, HSETNX,
HSTRLEN
- **Keys**: SCAN, UNLINK
- **Server**: DBSIZE, FLUSHALL (no async option), FLUSHDB (no async option),
INFO, SHUTDOWN,
diff --git a/geode-redis/README.md b/geode-redis/README.md
index 72d4a36..f690001 100644
--- a/geode-redis/README.md
+++ b/geode-redis/README.md
@@ -165,52 +165,52 @@ start server \
| EXPIREAT | DECRBY
| ACL LIST |
| GET | ECHO
| ACL LOAD |
| HGETALL | FLUSHALL
| ACL LOG |
-| HMSET | FLUSHDB
| ACL SAVE |
-| HSET | GETBIT
| ACL SETUSER |
-| HVALS | GETRANGE
| ACL USERS |
-| KEYS | GETSET
| ACL WHOAMI |
-| PERSIST | HDEL
| BGREWRITEAOF |
-| PEXPIRE | HEXISTS
| BGSAVE |
-| PEXPIREAT | HGET
| BITFIELD |
-| PING | HINCRBY
| BLPOP |
-| PSUBSCRIBE | HINCRBYFLOAT
| BRPOP |
-| PTTL | HKEYS
| BRPOPLPUSH |
-| PUBLISH | HLEN
| BZPOPMAX |
-| PUNSUBSCRIBE | HMGET
| BZPOPMIN |
-| QUIT | HSCAN
| CLIENT CACHING |
-| RENAME | HSETNX
| CLIENT GETNAME |
-| SADD | HSTRLEN
| CLIENT GETREDIR |
-| SET | INCR
| CLIENT ID |
-| SMEMBERS | INCRBY
| CLIENT KILL |
-| SREM | INCRBYFLOAT
| CLIENT LIST |
-| SUBSCRIBE | INFO
| CLIENT PAUSE |
-| TTL | MGET
| CLIENT REPLY |
-| TYPE | MSET
| CLIENT SETNAME |
-| UNSUBSCRIBE | MSETNX
| CLIENT TRACKING |
-| | PSETEX
| CLIENT UNBLOCK |
-| | SCAN
| CLUSTER ADDSLOTS |
-| | SCARD
| CLUSTER BUMPEPOCH |
-| | SDIFF
| CLUSTER COUNT-FAILURE-REPORTS |
-| | SDIFFSTORE
| CLUSTER COUNTKEYSINSLOT |
-| | SELECT
| CLUSTER DELSLOTS |
-| | SETBIT
| CLUSTER FAILOVER |
-| | SETEX
| CLUSTER FLUSHSLOTS |
-| | SETNX
| CLUSTER FORGET |
-| | SETRANGE
| CLUSTER GETKEYSINSLOT |
-| | SHUTDOWN
| CLUSTER INFO |
-| | SINTER
| CLUSTER KEYSLOT |
-| | SINTERSTORE
| CLUSTER MEET |
-| | SISMEMBER
| CLUSTER MYID |
-| | SLOWLOG
| CLUSTER NODES |
-| | SMOVE
| CLUSTER REPLICAS |
-| | SPOP
| CLUSTER REPLICATE |
-| | SRANDMEMBER
| CLUSTER RESET |
-| | SSCAN
| CLUSTER SAVECONFIG |
-| | STRLEN
| CLUSTER SET-CONFIG-EPOCH |
-| | SUNION
| CLUSTER SETSLOT |
-| | SUNIONSTORE
| CLUSTER SLAVES |
-| | TIME
| CLUSTER SLOTS |
-| | UNLINK [1]
| COMMAND |
+| 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 |
+| |
| COMMAND |
| |
| COMMAND COUNT |
| |
| COMMAND GETKEYS |
| |
| COMMAND INFO |
diff --git
a/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMgetDUnitTest.java
b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMgetDUnitTest.java
new file mode 100644
index 0000000..c2d7422
--- /dev/null
+++
b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMgetDUnitTest.java
@@ -0,0 +1,112 @@
+/*
+ * 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.apache.geode.distributed.ConfigurationProperties.MAX_WAIT_TIME_RECONNECT;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+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 HMgetDUnitTest {
+
+ @ClassRule
+ public static RedisClusterStartupRule clusterStartUp = new
RedisClusterStartupRule(4);
+
+ private static final String LOCAL_HOST = "127.0.0.1";
+ private static final int HASH_SIZE = 1000;
+ private static final int JEDIS_TIMEOUT =
+ Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ private static Jedis jedis1;
+ private static Jedis jedis2;
+ private static Jedis jedis3;
+
+ private static Properties locatorProperties;
+
+ private static MemberVM locator;
+ private static MemberVM server1;
+ private static MemberVM server2;
+
+ private static int redisServerPort1;
+ private static int redisServerPort2;
+
+ @BeforeClass
+ public static void classSetup() {
+ locatorProperties = new Properties();
+ locatorProperties.setProperty(MAX_WAIT_TIME_RECONNECT, "15000");
+
+ locator = clusterStartUp.startLocatorVM(0, locatorProperties);
+ server1 = clusterStartUp.startRedisVM(1, locator.getPort());
+ server2 = clusterStartUp.startRedisVM(2, locator.getPort());
+
+ redisServerPort1 = clusterStartUp.getRedisPort(1);
+ 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();
+ }
+
+ @AfterClass
+ public static void tearDown() {
+ jedis1.disconnect();
+ jedis2.disconnect();
+
+ server1.stop();
+ server2.stop();
+ }
+
+ @Test
+ public void testConcurrentHMget_whileUpdatingValues() {
+ String key = "key";
+
+ Map<String, String> testMap = makeHashMap(HASH_SIZE, "field-", "value-");
+
+ jedis1.hset(key, testMap);
+
+ new ConcurrentLoopingThreads(HASH_SIZE,
+ (i) -> jedis1.hset(key, "field-" + i, "value-" + i),
+ (i) -> assertThat(jedis2.hmget(key, "field-" + i)).isNotNull(),
+ (i) -> assertThat(jedis3.hmget(key, "field-" + i)).isNotNull());
+ }
+
+ private Map<String, String> makeHashMap(int hashSize, String baseFieldName,
+ String baseValueName) {
+ Map<String, String> map = new HashMap<>();
+ for (int i = 0; i < hashSize; i++) {
+ map.put(baseFieldName + i, baseValueName + i);
+ }
+ return map;
+ }
+}
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 9454e1b..f0c3f2c 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
@@ -115,6 +115,22 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
}
@Test
+ public void testHMSetErrorMessage_givenIncorrectDataType() {
+ Map<String, String> animalMap = new HashMap<>();
+ animalMap.put("chicken", "eggs");
+
+ jedis.set("farm", "chicken");
+ assertThatThrownBy(() -> jedis.hmset("farm", animalMap))
+ .isInstanceOf(JedisDataException.class)
+ .hasMessageContaining("WRONGTYPE Operation against a key holding the
wrong kind of value");
+
+ jedis.sadd("zoo", "elephant");
+ assertThatThrownBy(() -> jedis.hmset("zoo", animalMap))
+ .isInstanceOf(JedisDataException.class)
+ .hasMessageContaining("WRONGTYPE Operation against a key holding the
wrong kind of value");
+ }
+
+ @Test
public void testHSet() {
String key = "key";
Map<String, String> hash = new HashMap<String, String>();
@@ -136,27 +152,26 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
}
@Test
- public void testHMGetHDelHGetAllHVals() {
+ public void testHMGet_HDel_HGetAll_HVals() {
String key = "key";
- Map<String, String> hash = new HashMap<String, String>();
+ Map<String, String> hash = new HashMap<>();
for (int i = 0; i < 10; i++) {
hash.put("field_" + i, "member_" + i);
}
jedis.hmset(key, hash);
+
Set<String> keys = hash.keySet();
String[] keyArray = keys.toArray(new String[keys.size()]);
List<String> retList = jedis.hmget(key, keyArray);
- for (int i = 0; i < keys.size(); i++) {
- assertThat(hash.get(keyArray[i])).isEqualTo(retList.get(i));
- }
+ assertThat(retList).containsExactlyInAnyOrderElementsOf(hash.values());
Map<String, String> retMap = jedis.hgetAll(key);
assertThat(retMap).containsExactlyInAnyOrderEntriesOf(hash);
List<String> retVals = jedis.hvals(key);
- Set<String> retSet = new HashSet<String>(retVals);
+ Set<String> retSet = new HashSet<>(retVals);
assertThat(retSet.containsAll(hash.values())).isTrue();
@@ -165,6 +180,38 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
}
@Test
+ public void testHMGet_returnNull_forUnknownFields() {
+ String key = "key";
+ jedis.hset(key, "rooster", "crows");
+ jedis.hset(key, "duck", "quacks");
+
+ List<String> result =
+ jedis.hmget(key, "unknown-1", "rooster", "unknown-2", "duck",
"unknown-3");
+ assertThat(result).containsExactly(null, "crows", null, "quacks", null);
+ }
+
+ @Test
+ public void testHMGet_givenWrongNumberOfArguments() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HMGET))
+ .hasMessage("ERR wrong number of arguments for 'hmget' command");
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HMGET, "1"))
+ .hasMessage("ERR wrong number of arguments for 'hmget' command");
+ }
+
+ @Test
+ public void testHMGetErrorMessage_givenIncorrectDataType() {
+ jedis.set("farm", "chicken");
+ assertThatThrownBy(() -> jedis.hmget("farm", "chicken"))
+ .isInstanceOf(JedisDataException.class)
+ .hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
+
+ jedis.sadd("zoo", "elephant");
+ assertThatThrownBy(() -> jedis.hmget("zoo", "chicken"))
+ .isInstanceOf(JedisDataException.class)
+ .hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
+ }
+
+ @Test
public void testHDelErrorMessage_givenIncorrectDataType() {
jedis.set("farm", "chicken");
assertThatThrownBy(() -> {
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 4efdb0c..97d90a1 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
@@ -156,6 +156,7 @@ public enum RedisCommandType {
/************* Hashes *****************/
HGETALL(new HGetAllExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
+ HMGET(new HMGetExecutor(), SUPPORTED, new MinimumParameterRequirements(3)),
HMSET(new HMSetExecutor(), SUPPORTED,
new MinimumParameterRequirements(4).and(new
EvenParameterRequirements())),
HSET(new HSetExecutor(), SUPPORTED,
@@ -238,7 +239,6 @@ public enum RedisCommandType {
HINCRBYFLOAT(new HIncrByFloatExecutor(), UNSUPPORTED, new
ExactParameterRequirements(4)),
HKEYS(new HKeysExecutor(), UNSUPPORTED, new ExactParameterRequirements(2)),
HLEN(new HLenExecutor(), UNSUPPORTED, new ExactParameterRequirements(2)),
- HMGET(new HMGetExecutor(), UNSUPPORTED, new MinimumParameterRequirements(3)),
HSCAN(new HScanExecutor(), UNSUPPORTED, new MinimumParameterRequirements(3),
new OddParameterRequirements(ERROR_SYNTAX)),
HSETNX(new HSetNXExecutor(), UNSUPPORTED, new ExactParameterRequirements(4)),
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 e0b05be..52deaff 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
@@ -38,6 +38,7 @@ public class SupportedCommandsJUnitTest {
"EXPIREAT",
"GET",
"HGETALL",
+ "HMGET",
"HMSET",
"HSET",
"HVALS",
@@ -82,7 +83,6 @@ public class SupportedCommandsJUnitTest {
"HINCRBYFLOAT",
"HKEYS",
"HLEN",
- "HMGET",
"HSCAN",
"HSETNX",
"HSTRLEN",
