Skip to content

Commit

Permalink
Geode-8853: DUNIT tests for HSTRLEN command (apache#5939)
Browse files Browse the repository at this point in the history
- 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]>
  • Loading branch information
3 people authored Jan 26, 2021
1 parent b46a0a2 commit 3ddd9da
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 53 deletions.
5 changes: 2 additions & 3 deletions geode-docs/tools_modules/redis_api_for_geode.html.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
88 changes: 44 additions & 44 deletions geode-redis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ public void testHSet_givenWrongNumberOfArguments() {
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
Expand Down Expand Up @@ -249,6 +249,16 @@ public void testHStrLen_failsForNonHashes() {
.hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
}

@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";
Expand Down Expand Up @@ -440,13 +450,13 @@ public void hsetNX_shouldThrowErrorIfKeyIsWrongType() {
@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");
}

/**
Expand Down

0 comments on commit 3ddd9da

Please sign in to comment.