Skip to content

Commit

Permalink
GEODE-9400: Do not use Coder conversion methods in tests (apache#6643)
Browse files Browse the repository at this point in the history 
- Replace uses of Coder conversion methods in DUnit and Integration
 tests with implementations that do not assume knowledge of the way we
 internally handle such conversions.

Authored-by: Donal Evans <doevans@vmware.com>
  • Loading branch information
@DonalEvans
DonalEvans committed Jun 25, 2021
1 parent 8fcf7f8 commit 396f213
Show file tree
Hide file tree
Showing 27 changed files with 158 additions and 191 deletions.
5 changes: 2 additions & 3 deletions ...e-apis-compatible-with-redis/src/commonTest/java/org/apache/geode/redis/ClusterNodes.java
View file
Expand Up @@ -15,7 +15,6 @@

package org.apache.geode.redis;

import static org.apache.geode.redis.internal.netty.Coder.bytesToString;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -42,9 +41,9 @@ public static ClusterNodes parseClusterSlots(List<Object> rawSlots) {
long slotEnd = (long) firstLevel.get(1);

List<Object> primary = (List<Object>) firstLevel.get(2);
String primaryIp = bytesToString((byte[]) primary.get(0));
String primaryIp = new String((byte[]) primary.get(0));
long primaryPort = (long) primary.get(1);
String primaryGUID = primary.size() > 2 ? bytesToString((byte[]) primary.get(2)) : "";
String primaryGUID = primary.size() > 2 ? new String((byte[]) primary.get(2)) : "";

result.addSlot(primaryGUID, primaryIp, primaryPort, slotStart, slotEnd);
}
Expand Down
5 changes: 2 additions & 3 deletions ...rc/distributedTest/java/org/apache/geode/redis/internal/executor/key/RenameDUnitTest.java
View file
Expand Up @@ -15,7 +15,6 @@
package org.apache.geode.redis.internal.executor.key;


import static org.apache.geode.redis.internal.netty.Coder.stringToBytes;

import java.util.ArrayList;
import java.util.HashSet;
Expand Down Expand Up @@ -120,15 +119,15 @@ public void testRenameWithKeysOnSameStripeDifferentServers()
private Set<String> getKeysOnSameRandomStripe(int numKeysNeeded) {
Random random = new Random();
String key1 = "{rename}keyz" + random.nextInt();
RedisKey key1RedisKey = new RedisKey(stringToBytes(key1));
RedisKey key1RedisKey = new RedisKey(key1.getBytes());
StripedExecutor stripedExecutor = new SynchronizedStripedExecutor();
Set<String> keys = new HashSet<>();
keys.add(key1);

do {
String key2 = "{rename}key" + random.nextInt();
if (stripedExecutor.compareStripes(key1RedisKey,
new RedisKey(stringToBytes(key2))) == 0) {
new RedisKey(key2.getBytes())) == 0) {
keys.add(key2);
}
} while (keys.size() < numKeysNeeded);
Expand Down
9 changes: 4 additions & 5 deletions ...Test/java/org/apache/geode/redis/internal/executor/sortedset/ZAddIncrOptionDUnitTest.java
View file
Expand Up @@ -42,7 +42,6 @@
import org.apache.geode.redis.internal.RegionProvider;
import org.apache.geode.redis.internal.data.RedisData;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.netty.Coder;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
Expand Down Expand Up @@ -101,11 +100,11 @@ private void verifyZScores() {
private void doZAddIncr(int i, double increment, double total, boolean isConcurrentExecution) {
Object result =
jedis.sendCommand(sortedSetKey, Protocol.Command.ZADD, sortedSetKey, "INCR",
Coder.doubleToString(increment), baseMemberName + i);
String.valueOf(increment), baseMemberName + i);
if (isConcurrentExecution) {
assertThat(Coder.bytesToDouble((byte[]) result)).isIn(increment, total);
assertThat(Double.parseDouble(new String((byte[]) result))).isIn(increment, total);
} else {
assertThat(Coder.bytesToDouble((byte[]) result)).isEqualTo(total);
assertThat(Double.parseDouble(new String((byte[]) result))).isEqualTo(total);
}
}

Expand Down Expand Up @@ -193,7 +192,7 @@ private void stopNodeWithPrimaryBucketOfTheKey(boolean isCrash) {
}

private static boolean isPrimaryForKey() {
int bucketId = getBucketId(new RedisKey(Coder.stringToBytes(sortedSetKey)));
int bucketId = getBucketId(new RedisKey(sortedSetKey.getBytes()));
return isPrimaryForBucket(bucketId);
}

Expand Down
3 changes: 1 addition & 2 deletions ...istributedTest/java/org/apache/geode/redis/internal/executor/sortedset/ZRemDUnitTest.java
View file
Expand Up @@ -14,7 +14,6 @@
*/
package org.apache.geode.redis.internal.executor.sortedset;

import static org.apache.geode.redis.internal.netty.Coder.stringToBytes;
import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -221,7 +220,7 @@ private void stopNodeWithPrimaryBucketOfTheKey(boolean isCrash) {
}

private static boolean isPrimaryForKey() {
int bucketId = getBucketId(new RedisKey(stringToBytes(sortedSetKey)));
int bucketId = getBucketId(new RedisKey(sortedSetKey.getBytes()));
return isPrimaryForBucket(bucketId);
}

Expand Down
3 changes: 1 addition & 2 deletions ...-with-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionDUnitTest.java
View file
Expand Up @@ -16,7 +16,6 @@
package org.apache.geode.redis.session;


import static org.apache.geode.redis.internal.netty.Coder.bytesToString;

import java.net.HttpCookie;
import java.time.Duration;
Expand Down Expand Up @@ -255,7 +254,7 @@ private Void addNoteToSession0(int sessionApp, String sessionCookie, String note
protected String getSessionId(String sessionCookie) {
List<HttpCookie> cookies = HttpCookie.parse(sessionCookie);
byte[] decodedCookie = Base64.getDecoder().decode(cookies.get(0).getValue());
return bytesToString(decodedCookie);
return new String(decodedCookie);
}

}
3 changes: 1 addition & 2 deletions ...s/src/distributedTest/java/org/apache/geode/redis/session/SessionExpirationDUnitTest.java
View file
Expand Up @@ -15,7 +15,6 @@

package org.apache.geode.redis.session;

import static org.apache.geode.redis.internal.netty.Coder.stringToBytes;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.ByteArrayInputStream;
Expand Down Expand Up @@ -166,7 +165,7 @@ private static int getIntFromBytes(RedisHash redisHash) {
return 0;
}
ObjectInputStream inputStream;
byte[] bytes = redisHash.hget(stringToBytes("maxInactiveInterval"));
byte[] bytes = redisHash.hget("maxInactiveInterval".getBytes());
ByteArrayInputStream byteStream = new ByteArrayInputStream(bytes);
try {
inputStream = new ObjectInputStream(byteStream);
Expand Down
13 changes: 5 additions & 8 deletions ...edis/src/integrationTest/java/org/apache/geode/redis/RedisCommandArgumentsTestHelper.java
View file
Expand Up @@ -15,9 +15,6 @@

package org.apache.geode.redis;

import static org.apache.geode.redis.internal.netty.Coder.bytesToString;
import static org.apache.geode.redis.internal.netty.Coder.intToBytes;
import static org.apache.geode.redis.internal.netty.Coder.stringToBytes;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.function.BiFunction;
Expand All @@ -35,7 +32,7 @@ public static void assertExactNumberOfArgs(Jedis jedis, Protocol.Command command
public static void assertExactNumberOfArgs(JedisCluster jedis,
Protocol.Command command, int numArgs) {
assertExactNumberOfArgs0(
(cmd, args) -> jedis.sendCommand(stringToBytes("key"), cmd, args), command,
(cmd, args) -> jedis.sendCommand("key".getBytes(), cmd, args), command,
numArgs);
}

Expand All @@ -59,7 +56,7 @@ public static void assertAtLeastNArgs(Jedis jedis, Protocol.Command command, int

public static void assertAtLeastNArgs(JedisCluster jedis, Protocol.Command command,
int minNumArgs) {
assertAtLeastNArgs0((cmd, args) -> jedis.sendCommand(stringToBytes("key"), cmd, args),
assertAtLeastNArgs0((cmd, args) -> jedis.sendCommand("key".getBytes(), cmd, args),
command,
minNumArgs);
}
Expand Down Expand Up @@ -87,7 +84,7 @@ public static void assertAtMostNArgsForSubCommand(Jedis jedis, Protocol.Command

public static void assertAtMostNArgs(JedisCluster jedis, Protocol.Command command,
int maxNumArgs) {
assertAtMostNArgs0((cmd, args) -> jedis.sendCommand(stringToBytes("key"), cmd, args),
assertAtMostNArgs0((cmd, args) -> jedis.sendCommand("key".getBytes(), cmd, args),
command,
maxNumArgs);
}
Expand All @@ -112,7 +109,7 @@ private static void assertAtMostNArgsForSubCommand0(

assertThatThrownBy(() -> runMe.apply(command, args))
.hasMessageContaining("ERR Unknown subcommand or wrong number of arguments for '"
+ bytesToString(firstParameter));
+ new String(firstParameter));
}
}

Expand All @@ -124,7 +121,7 @@ private static byte[][] buildArgs(int numArgs) {
}

for (int i = 0; i < numArgs; i++) {
args[i] = intToBytes(i);
args[i] = String.valueOf(i).getBytes();
}

return args;
Expand Down
15 changes: 7 additions & 8 deletions ...ionTest/java/org/apache/geode/redis/internal/executor/AbstractUnknownIntegrationTest.java
View file
Expand Up @@ -15,7 +15,6 @@

package org.apache.geode.redis.internal.executor;

import static org.apache.geode.redis.internal.netty.Coder.stringToBytes;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import org.junit.After;
Expand Down Expand Up @@ -45,13 +44,13 @@ public void tearDown() {

@Test
public void givenUnknownCommand_returnsUnknownCommandError() {
assertThatThrownBy(() -> jedis.sendCommand(() -> stringToBytes("fhqwhgads")))
assertThatThrownBy(() -> jedis.sendCommand(() -> "fhqwhgads".getBytes()))
.hasMessage("ERR unknown command `fhqwhgads`, with args beginning with: ");
}

@Test
public void givenUnknownCommand_withArguments_returnsUnknownCommandErrorWithArgumentsListed() {
assertThatThrownBy(() -> jedis.sendCommand(() -> stringToBytes("fhqwhgads"), "EVERYBODY",
assertThatThrownBy(() -> jedis.sendCommand(() -> "fhqwhgads".getBytes(), "EVERYBODY",
"TO THE LIMIT"))
.hasMessage(
"ERR unknown command `fhqwhgads`, with args beginning with: `EVERYBODY`, `TO THE LIMIT`, ");
Expand All @@ -60,21 +59,21 @@ public void givenUnknownCommand_withArguments_returnsUnknownCommandErrorWithArgu
@Test
public void givenUnknownCommand_withEmptyStringArgument_returnsUnknownCommandErrorWithArgumentsListed() {
assertThatThrownBy(
() -> jedis.sendCommand(() -> stringToBytes("fhqwhgads"), "EVERYBODY", ""))
() -> jedis.sendCommand(() -> "fhqwhgads".getBytes(), "EVERYBODY", ""))
.hasMessage(
"ERR unknown command `fhqwhgads`, with args beginning with: `EVERYBODY`, ``, ");
}

@Test // HELLO is not a recognized command until Redis 6.0.0
public void givenHelloCommand_returnsUnknownCommandErrorWithArgumentsListed() {
assertThatThrownBy(() -> jedis.sendCommand(() -> stringToBytes("HELLO")))
assertThatThrownBy(() -> jedis.sendCommand(() -> "HELLO".getBytes()))
.hasMessage("ERR unknown command `HELLO`, with args beginning with: ");
}

@Test
public void givenInternalSMembersCommand_returnsUnknownCommandErrorWithArgumentsListed() {
assertThatThrownBy(
() -> jedis.sendCommand(() -> stringToBytes("INTERNALSMEMBERS"), "something",
() -> jedis.sendCommand(() -> "INTERNALSMEMBERS".getBytes(), "something",
"somethingElse"))
.hasMessage(
"ERR unknown command `INTERNALSMEMBERS`, with args beginning with: `something`, `somethingElse`, ");
Expand All @@ -83,15 +82,15 @@ public void givenInternalSMembersCommand_returnsUnknownCommandErrorWithArguments
@Test
public void givenInternalPTTLCommand_returnsUnknownCommandErrorWithArgumentsListed() {
assertThatThrownBy(
() -> jedis.sendCommand(() -> stringToBytes("INTERNALPTTL"), "something"))
() -> jedis.sendCommand(() -> "INTERNALPTTL".getBytes(), "something"))
.hasMessage(
"ERR unknown command `INTERNALPTTL`, with args beginning with: `something`, ");
}

@Test
public void givenInternalTypeCommand_returnsUnknownCommandErrorWithArgumentsListed() {
assertThatThrownBy(
() -> jedis.sendCommand(() -> stringToBytes("INTERNALTYPE"), "something"))
() -> jedis.sendCommand(() -> "INTERNALTYPE".getBytes(), "something"))
.hasMessage(
"ERR unknown command `INTERNALTYPE`, with args beginning with: `something`, ");
}
Expand Down
76 changes: 37 additions & 39 deletions ...Test/java/org/apache/geode/redis/internal/executor/hash/AbstractHScanIntegrationTest.java
View file
Expand Up @@ -20,8 +20,6 @@
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE;
import static org.apache.geode.redis.internal.netty.Coder.bytesToString;
import static org.apache.geode.redis.internal.netty.Coder.stringToBytes;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

Expand Down Expand Up @@ -232,7 +230,7 @@ public void givenInvalidRegexSyntax_returnsEmptyArray() {
scanParams.match("\\p");

ScanResult<Map.Entry<byte[], byte[]>> result =
jedis.hscan(stringToBytes("a"), stringToBytes("0"), scanParams);
jedis.hscan("a".getBytes(), "0".getBytes(), scanParams);

assertThat(result.getResult()).isEmpty();
}
Expand Down Expand Up @@ -287,10 +285,10 @@ public void givenMultipleCounts_DoesNotFail() {
"COUNT", "1");

allEntries.addAll((List<byte[]>) result.get(1));
cursor = bytesToString((byte[]) result.get(0));
} while (!Arrays.equals((byte[]) result.get(0), stringToBytes("0")));
cursor = new String((byte[]) result.get(0));
} while (!Arrays.equals((byte[]) result.get(0), "0".getBytes()));

assertThat((byte[]) result.get(0)).isEqualTo(stringToBytes("0"));
assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
}

@Test
Expand All @@ -308,7 +306,7 @@ public void givenCompleteIteration_shouldReturnCursorWithValueOfZero() {
String cursor = "0";

do {
result = jedis.hscan(stringToBytes("colors"), stringToBytes(cursor), scanParams);
result = jedis.hscan("colors".getBytes(), cursor.getBytes(), scanParams);
allEntries.addAll(result.getResult());
cursor = result.getCursor();
} while (!result.isCompleteIteration());
Expand All @@ -319,17 +317,17 @@ public void givenCompleteIteration_shouldReturnCursorWithValueOfZero() {
@Test
public void givenMatch_returnsAllMatchingEntries() {
Map<byte[], byte[]> entryMap = new HashMap<>();
byte[] field3 = stringToBytes("3");
entryMap.put(stringToBytes("1"), stringToBytes("yellow"));
entryMap.put(stringToBytes("12"), stringToBytes("green"));
entryMap.put(field3, stringToBytes("grey"));
jedis.hmset(stringToBytes("colors"), entryMap);
byte[] field3 = "3".getBytes();
entryMap.put("1".getBytes(), "yellow".getBytes());
entryMap.put("12".getBytes(), "green".getBytes());
entryMap.put(field3, "grey".getBytes());
jedis.hmset("colors".getBytes(), entryMap);

ScanParams scanParams = new ScanParams();
scanParams.match("1*");

ScanResult<Map.Entry<byte[], byte[]>> result =
jedis.hscan(stringToBytes("colors"), stringToBytes("0"), scanParams);
jedis.hscan("colors".getBytes(), "0".getBytes(), scanParams);

entryMap.remove(field3);
assertThat(result.isCompleteIteration()).isTrue();
Expand Down Expand Up @@ -357,25 +355,25 @@ public void givenMultipleMatches_returnsEntriesMatchingLastMatchParameter() {
jedis.hmset("colors", entryMap);

List<Object> result =
(List<Object>) jedis.sendCommand(stringToBytes("colors"), Protocol.Command.HSCAN,
stringToBytes("colors"), stringToBytes("0"), stringToBytes("MATCH"),
stringToBytes("3*"),
stringToBytes("MATCH"), stringToBytes("1*"));
(List<Object>) jedis.sendCommand("colors".getBytes(), Protocol.Command.HSCAN,
"colors".getBytes(), "0".getBytes(), "MATCH".getBytes(),
"3*".getBytes(),
"MATCH".getBytes(), "1*".getBytes());

assertThat((byte[]) result.get(0)).isEqualTo(stringToBytes("0"));
assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
assertThat((List<Object>) result.get(1)).containsAll(
Arrays.asList(stringToBytes("1"), stringToBytes("yellow"),
stringToBytes("12"), stringToBytes("green")));
Arrays.asList("1".getBytes(), "yellow".getBytes(),
"12".getBytes(), "green".getBytes()));
}

@Test
public void givenMatchAndCount_returnsAllMatchingKeys() {
Map<byte[], byte[]> entryMap = new HashMap<>();
byte[] field3 = stringToBytes("3");
entryMap.put(stringToBytes("1"), stringToBytes("yellow"));
entryMap.put(stringToBytes("12"), stringToBytes("green"));
entryMap.put(field3, stringToBytes("orange"));
jedis.hmset(stringToBytes("colors"), entryMap);
byte[] field3 = "3".getBytes();
entryMap.put("1".getBytes(), "yellow".getBytes());
entryMap.put("12".getBytes(), "green".getBytes());
entryMap.put(field3, "orange".getBytes());
jedis.hmset("colors".getBytes(), entryMap);

ScanParams scanParams = new ScanParams();
scanParams.count(1);
Expand All @@ -385,7 +383,7 @@ public void givenMatchAndCount_returnsAllMatchingKeys() {
String cursor = "0";

do {
result = jedis.hscan(stringToBytes("colors"), stringToBytes(cursor), scanParams);
result = jedis.hscan("colors".getBytes(), cursor.getBytes(), scanParams);
allEntries.addAll(result.getResult());
cursor = result.getCursor();
} while (!result.isCompleteIteration());
Expand All @@ -412,21 +410,21 @@ public void givenMultipleCountsAndMatches_returnsEntriesMatchingLastMatchParamet

do {
result =
(List<Object>) jedis.sendCommand(stringToBytes("colors"), Protocol.Command.HSCAN,
stringToBytes("colors"), stringToBytes(cursor),
stringToBytes("COUNT"), stringToBytes("37"),
stringToBytes("MATCH"), stringToBytes("3*"), stringToBytes("COUNT"),
stringToBytes("2"),
stringToBytes("COUNT"), stringToBytes("1"), stringToBytes("MATCH"),
stringToBytes("1*"));
(List<Object>) jedis.sendCommand("colors".getBytes(), Protocol.Command.HSCAN,
"colors".getBytes(), cursor.getBytes(),
"COUNT".getBytes(), "37".getBytes(),
"MATCH".getBytes(), "3*".getBytes(), "COUNT".getBytes(),
"2".getBytes(),
"COUNT".getBytes(), "1".getBytes(), "MATCH".getBytes(),
"1*".getBytes());
allEntries.addAll((List<byte[]>) result.get(1));
cursor = bytesToString((byte[]) result.get(0));
} while (!Arrays.equals((byte[]) result.get(0), stringToBytes("0")));
cursor = new String((byte[]) result.get(0));
} while (!Arrays.equals((byte[]) result.get(0), "0".getBytes()));

assertThat((byte[]) result.get(0)).isEqualTo(stringToBytes("0"));
assertThat(allEntries).containsExactlyInAnyOrder(stringToBytes("1"),
stringToBytes("yellow"),
stringToBytes("12"), stringToBytes("green"));
assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
assertThat(allEntries).containsExactlyInAnyOrder("1".getBytes(),
"yellow".getBytes(),
"12".getBytes(), "green".getBytes());
}

@Test
Expand Down

0 comments on commit 396f213

Please sign in to comment.