You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2020/06/19 13:46:33 UTC

[geode] branch develop updated: GEODE-8269: Improve test coverage (#5275)

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 18e5817  GEODE-8269: Improve test coverage (#5275)
18e5817 is described below

commit 18e5817b3a7c6f39984577d3790250cd5c900164
Author: Sarah Abbey <41...@users.noreply.github.com>
AuthorDate: Fri Jun 19 09:45:58 2020 -0400

    GEODE-8269: Improve test coverage (#5275)
---
 .../apache/geode/redis/GeodeRedisServerRule.java   |  5 --
 .../redis/GeodeRedisServerStartupDUnitTest.java    |  2 +-
 .../internal/executor/hash/HMsetDUnitTest.java     |  1 -
 ...rationTest.java => UnknownIntegrationTest.java} | 36 +++++------
 .../executor/connection/AuthIntegrationTest.java   | 70 +++++++---------------
 .../executor/key/ExpireIntegrationTest.java        | 34 +----------
 .../executor/key/PexpireIntegrationTest.java       | 36 +++++++++++
 .../geode/redis/internal/GeodeRedisServer.java     |  3 -
 .../geode/redis/internal/GeodeRedisService.java    | 15 ++---
 .../geode/redis/internal/RedisCommandType.java     |  2 +-
 .../redis/internal/executor/AbstractExecutor.java  | 47 ---------------
 .../apache/geode/redis/internal/netty/Coder.java   |  9 ---
 .../apache/geode/redis/internal/netty/Command.java | 34 -----------
 .../internal/netty/ExecutionHandlerContext.java    |  6 --
 14 files changed, 82 insertions(+), 218 deletions(-)

diff --git a/geode-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java b/geode-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java
index 5cc7969..54022a0 100644
--- a/geode-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java
+++ b/geode-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java
@@ -24,7 +24,6 @@ import static org.apache.geode.distributed.ConfigurationProperties.REDIS_PASSWOR
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.GemFireCache;
 import org.apache.geode.redis.internal.GeodeRedisServer;
-import org.apache.geode.redis.internal.RegionProvider;
 import org.apache.geode.test.junit.rules.serializable.SerializableExternalResource;
 
 public class GeodeRedisServerRule extends SerializableExternalResource {
@@ -73,8 +72,4 @@ public class GeodeRedisServerRule extends SerializableExternalResource {
   public int getPort() {
     return server.getPort();
   }
-
-  public RegionProvider getRegionProvider() {
-    return server.getRegionProvider();
-  }
 }
diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java
index 28a8ef3..c1c7c7a 100644
--- a/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java
+++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java
@@ -192,7 +192,7 @@ public class GeodeRedisServerStartupDUnitTest {
   public void whenUnsupportedCommandsEnabledDynamicallyWithGfsh_newGeodeRedisServersWillRetainConfig()
       throws Exception {
     MemberVM locator = cluster.startLocatorVM(0);
-    MemberVM server1 = cluster.startServerVM(1, s -> s
+    cluster.startServerVM(1, s -> s
         .withProperty(REDIS_PORT, "0")
         .withProperty(REDIS_BIND_ADDRESS, "localhost")
         .withProperty(REDIS_ENABLED, "true")
diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMsetDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMsetDUnitTest.java
index 0b2181d..76b3077 100644
--- a/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMsetDUnitTest.java
+++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HMsetDUnitTest.java
@@ -96,7 +96,6 @@ public class HMsetDUnitTest {
 
   @Test
   public void shouldDistributeDataAmongMultipleServers_givenMultipleClients() {
-
     String key = "key";
 
     Map<String, String> testMap = makeHashMap(HASH_SIZE, "field-", "value-");
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java
similarity index 64%
copy from geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java
copy to geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java
index 72f20f2..1f27107 100644
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java
@@ -13,10 +13,11 @@
  * the License.
  */
 
-package org.apache.geode.redis.internal.executor.key;
+package org.apache.geode.redis.internal.executor;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -24,11 +25,13 @@ import org.junit.Test;
 import redis.clients.jedis.Jedis;
 
 import org.apache.geode.redis.GeodeRedisServerRule;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
 
-public class PexpireIntegrationTest {
+public class UnknownIntegrationTest {
 
   public static Jedis jedis;
-  public static int REDIS_CLIENT_TIMEOUT = 10000000;
+  public static int REDIS_CLIENT_TIMEOUT =
+      Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
 
   @ClassRule
   public static GeodeRedisServerRule server = new GeodeRedisServerRule();
@@ -38,26 +41,19 @@ public class PexpireIntegrationTest {
     jedis = new Jedis("localhost", server.getPort(), REDIS_CLIENT_TIMEOUT);
   }
 
+  @After
+  public void flushAll() {
+    jedis.flushAll();
+  }
+
   @AfterClass
-  public static void classLevelTearDown() {
+  public static void tearDown() {
     jedis.close();
   }
 
   @Test
-  public void should_SetExpiration_givenKeyTo_StringValueInMilliSeconds() {
-
-    String key = "key";
-    String value = "value";
-    long millisecondsToLive = 20000L;
-
-    jedis.set(key, value);
-    Long timeToLive = jedis.ttl(key);
-    assertThat(timeToLive).isEqualTo(-1);
-
-    jedis.pexpire(key, millisecondsToLive);
-
-    timeToLive = jedis.ttl(key);
-    assertThat(timeToLive).isLessThanOrEqualTo(20);
-    assertThat(timeToLive).isGreaterThanOrEqualTo(15);
+  public void shouldReturnUnknownCommandError() {
+    assertThatThrownBy(() -> jedis.sendCommand(() -> "fhqwhgads".getBytes()))
+        .hasMessageContaining("Unable to process unknown command fhqwhgads");
   }
 }
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java
index d84d920..65bc800 100644
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java
@@ -28,7 +28,6 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import redis.clients.jedis.Jedis;
 import redis.clients.jedis.Protocol;
-import redis.clients.jedis.exceptions.JedisDataException;
 
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.GemFireCache;
@@ -48,8 +47,6 @@ public class AuthIntegrationTest {
   Random rand;
   int port;
 
-  int runs = 150;
-
   @Before
   public void setUp() {
     rand = new Random();
@@ -93,16 +90,11 @@ public class AuthIntegrationTest {
   @Test
   public void testAuthRejectAccept() {
     setupCacheWithPassword();
-    Exception ex = null;
-    try {
-      jedis.auth("wrongpwd");
-    } catch (JedisDataException e) {
-      ex = e;
-    }
-    assertThat(ex).isNotNull();
 
-    String res = jedis.auth(PASSWORD);
-    assertThat(res).isEqualTo("OK");
+    assertThatThrownBy(() -> jedis.auth("wrongpwd"))
+        .hasMessageContaining("Attemping to authenticate with an invalid password");
+
+    assertThat(jedis.auth(PASSWORD)).isEqualTo("OK");
   }
 
   @Test
@@ -115,25 +107,16 @@ public class AuthIntegrationTest {
     server = new GeodeRedisServer("localhost", port);
     server.start();
 
-    Exception ex = null;
-    try {
-      jedis.auth(PASSWORD);
-    } catch (JedisDataException e) {
-      ex = e;
-    }
-    assertThat(ex).isNotNull();
+    assertThatThrownBy(() -> jedis.auth(PASSWORD))
+        .hasMessageContaining("Attempting to authenticate when no password has been set");
   }
 
   @Test
   public void testAuthAcceptRequests() {
     setupCacheWithPassword();
-    Exception ex = null;
-    try {
-      jedis.set("foo", "bar");
-    } catch (JedisDataException e) {
-      ex = e;
-    }
-    assertThat(ex).isNotNull();
+
+    assertThatThrownBy(() -> jedis.set("foo", "bar"))
+        .hasMessageContaining("Must authenticate before sending any requests");
 
     String res = jedis.auth(PASSWORD);
     assertThat(res).isEqualTo("OK");
@@ -144,28 +127,17 @@ public class AuthIntegrationTest {
   @Test
   public void testSeparateClientRequests() {
     setupCacheWithPassword();
-    Jedis authorizedJedis = null;
-    Jedis nonAuthorizedJedis = null;
-    try {
-      authorizedJedis = new Jedis("localhost", port, 100000);
-      nonAuthorizedJedis = new Jedis("localhost", port, 100000);
-      String res = authorizedJedis.auth(PASSWORD);
-      assertThat(res).isEqualTo("OK");
-      authorizedJedis.set("foo", "bar"); // No exception for authorized client
-
-      authorizedJedis.auth(PASSWORD);
-      Exception ex = null;
-      try {
-        nonAuthorizedJedis.set("foo", "bar");
-      } catch (JedisDataException e) {
-        ex = e;
-      }
-      assertThat(ex).isNotNull();
-    } finally {
-      if (authorizedJedis != null)
-        authorizedJedis.close();
-      if (nonAuthorizedJedis != null)
-        nonAuthorizedJedis.close();
-    }
+    Jedis nonAuthorizedJedis = new Jedis("localhost", port, 100000);
+    Jedis authorizedJedis = new Jedis("localhost", port, 100000);
+
+    assertThat(authorizedJedis.auth(PASSWORD)).isEqualTo("OK");
+    authorizedJedis.set("foo", "bar"); // No exception for authorized client
+    authorizedJedis.auth(PASSWORD);
+
+    assertThatThrownBy(() -> nonAuthorizedJedis.set("foo", "bar"))
+        .hasMessageContaining("Must authenticate before sending any requests");
+
+    authorizedJedis.close();
+    nonAuthorizedJedis.close();
   }
 }
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/ExpireIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/ExpireIntegrationTest.java
index b7624bf..7202904 100644
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/ExpireIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/ExpireIntegrationTest.java
@@ -25,7 +25,6 @@ import org.junit.Test;
 import redis.clients.jedis.Jedis;
 
 import org.apache.geode.redis.GeodeRedisServerRule;
-import org.apache.geode.test.awaitility.GeodeAwaitility;
 
 public class ExpireIntegrationTest {
 
@@ -51,7 +50,7 @@ public class ExpireIntegrationTest {
   }
 
   @Test
-  public void Should_SetExipration_givenKeyTo_StringValue() {
+  public void Should_SetExpiration_givenKeyTo_StringValue() {
 
     String key = "key";
     String value = "value";
@@ -121,27 +120,6 @@ public class ExpireIntegrationTest {
   }
 
   @Test
-  public void should_removeKey_AfterExpirationPeriod() {
-    String key = "key";
-    String value = "value";
-    jedis.set(key, value);
-
-    jedis.pexpire(key, 10);
-
-    GeodeAwaitility.await().until(() -> jedis.get(key) == null);
-  }
-
-  @Test
-  public void should_removeSetKey_AfterExpirationPeriod() {
-    String key = "key";
-    String value = "value";
-    jedis.sadd(key, value);
-
-    jedis.pexpire(key, 10);
-    GeodeAwaitility.await().until(() -> !jedis.exists(key));
-  }
-
-  @Test
   public void settingAnExistingKeyToANewValue_ShouldClearExpirationTime() {
 
     String key = "key";
@@ -356,7 +334,7 @@ public class ExpireIntegrationTest {
 
 
   @Test
-  public void CallingExpireOnAKeyThatAlreadyHasAnExiprationTime_ShouldUpdateTheExpirationTime() {
+  public void CallingExpireOnAKeyThatAlreadyHasAnExpirationTime_ShouldUpdateTheExpirationTime() {
     String key = "key";
     String value = "value";
     jedis.set(key, value);
@@ -367,12 +345,4 @@ public class ExpireIntegrationTest {
     Long timeToLive = jedis.ttl(key);
     assertThat(timeToLive).isGreaterThan(21);
   }
-
-  @Test
-  public void should_passivelyExpireKeys() {
-    jedis.sadd("key", "value");
-    jedis.pexpire("key", 100);
-
-    GeodeAwaitility.await().until(() -> jedis.keys("key").isEmpty());
-  }
 }
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java
index 72f20f2..b702d8f 100644
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/PexpireIntegrationTest.java
@@ -17,6 +17,7 @@ package org.apache.geode.redis.internal.executor.key;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -24,6 +25,7 @@ import org.junit.Test;
 import redis.clients.jedis.Jedis;
 
 import org.apache.geode.redis.GeodeRedisServerRule;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
 
 public class PexpireIntegrationTest {
 
@@ -38,6 +40,11 @@ public class PexpireIntegrationTest {
     jedis = new Jedis("localhost", server.getPort(), REDIS_CLIENT_TIMEOUT);
   }
 
+  @After
+  public void flushAll() {
+    jedis.flushAll();
+  }
+
   @AfterClass
   public static void classLevelTearDown() {
     jedis.close();
@@ -60,4 +67,33 @@ public class PexpireIntegrationTest {
     assertThat(timeToLive).isLessThanOrEqualTo(20);
     assertThat(timeToLive).isGreaterThanOrEqualTo(15);
   }
+
+  @Test
+  public void should_removeKey_AfterExpirationPeriod() {
+    String key = "key";
+    String value = "value";
+    jedis.set(key, value);
+
+    jedis.pexpire(key, 10);
+
+    GeodeAwaitility.await().until(() -> jedis.get(key) == null);
+  }
+
+  @Test
+  public void should_removeSetKey_AfterExpirationPeriod() {
+    String key = "key";
+    String value = "value";
+    jedis.sadd(key, value);
+
+    jedis.pexpire(key, 10);
+    GeodeAwaitility.await().until(() -> !jedis.exists(key));
+  }
+
+  @Test
+  public void should_passivelyExpireKeys() {
+    jedis.sadd("key", "value");
+    jedis.pexpire("key", 100);
+
+    GeodeAwaitility.await().until(() -> jedis.keys("key").isEmpty());
+  }
 }
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
index e24bbcf..0b66d64 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
@@ -49,7 +49,6 @@ import io.netty.util.concurrent.Future;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.annotations.Experimental;
-import org.apache.geode.annotations.VisibleForTesting;
 import org.apache.geode.annotations.internal.MakeNotStatic;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheClosedException;
@@ -342,7 +341,6 @@ public class GeodeRedisServer {
     this.cache = cache;
   }
 
-  @VisibleForTesting
   public RegionProvider getRegionProvider() {
     return regionProvider;
   }
@@ -355,7 +353,6 @@ public class GeodeRedisServer {
     return subscriberGroup;
   }
 
-
   private void initializeRedis() {
     synchronized (cache) {
 
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java
index 5f947ed..d1e0716 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java
@@ -17,7 +17,6 @@ package org.apache.geode.redis.internal;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.cache.Cache;
-import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.ResourceEvent;
 import org.apache.geode.distributed.internal.ResourceEventsListener;
@@ -73,15 +72,11 @@ public class GeodeRedisService implements CacheService, ResourceEventsListener {
       int port = system.getConfig().getRedisPort();
       String bindAddress = system.getConfig().getRedisBindAddress();
       assert bindAddress != null;
-      if (bindAddress.equals(DistributionConfig.DEFAULT_REDIS_BIND_ADDRESS)) {
-        logger.info(
-            String.format("Starting GeodeRedisServer on port %s",
-                new Object[] {port}));
-      } else {
-        logger.info(
-            String.format("Starting GeodeRedisServer on bind address %s on port %s",
-                new Object[] {bindAddress, port}));
-      }
+
+      logger.info(
+          String.format("Starting GeodeRedisServer on bind address %s on port %s",
+              new Object[] {bindAddress, port}));
+
       this.redisServer = new GeodeRedisServer(bindAddress, port);
       this.redisServer.start();
     }
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 7753ba0..4ec0ac6 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
@@ -122,7 +122,7 @@ public enum RedisCommandType {
 
   /*************** Connection ****************/
   AUTH(new AuthExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
-  PING(new PingExecutor(), SUPPORTED),
+  PING(new PingExecutor(), SUPPORTED, new MaximumParameterRequirements(2)),
   QUIT(new QuitExecutor(), SUPPORTED),
 
   /*************** Keys ******************/
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/AbstractExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/AbstractExecutor.java
index ab73c65..263c155 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/AbstractExecutor.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/AbstractExecutor.java
@@ -16,18 +16,12 @@ package org.apache.geode.redis.internal.executor;
 
 import java.util.Collection;
 
-import io.netty.buffer.ByteBuf;
-
 import org.apache.geode.cache.Region;
 import org.apache.geode.redis.internal.GeodeRedisServer;
-import org.apache.geode.redis.internal.RedisConstants;
 import org.apache.geode.redis.internal.data.ByteArrayWrapper;
 import org.apache.geode.redis.internal.data.RedisData;
 import org.apache.geode.redis.internal.executor.key.RedisKeyCommands;
 import org.apache.geode.redis.internal.executor.key.RedisKeyCommandsFunctionExecutor;
-import org.apache.geode.redis.internal.netty.Coder;
-import org.apache.geode.redis.internal.netty.CoderException;
-import org.apache.geode.redis.internal.netty.Command;
 import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
 
 /**
@@ -35,47 +29,6 @@ import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
  */
 public abstract class AbstractExecutor implements Executor {
 
-  protected long getBoundedStartIndex(long index, long size) {
-    if (size < 0L) {
-      throw new IllegalArgumentException("Size < 0, really?");
-    }
-    if (index >= 0L) {
-      return Math.min(index, size);
-    } else {
-      return Math.max(index + size, 0);
-    }
-  }
-
-  protected long getBoundedEndIndex(long index, long size) {
-    if (size < 0L) {
-      throw new IllegalArgumentException("Size < 0, really?");
-    }
-    if (index >= 0L) {
-      return Math.min(index, size);
-    } else {
-      return Math.max(index + size, -1);
-    }
-  }
-
-  protected void respondBulkStrings(Command command, ExecutionHandlerContext context,
-      Object message) {
-    ByteBuf rsp;
-    try {
-      if (message instanceof Collection) {
-        rsp = Coder.getArrayResponse(context.getByteBufAllocator(),
-            (Collection<?>) message);
-      } else {
-        rsp = Coder.getBulkStringResponse(context.getByteBufAllocator(), message);
-      }
-    } catch (CoderException e) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(),
-          RedisConstants.SERVER_ERROR_MESSAGE));
-      return;
-    }
-
-    command.setResponse(rsp);
-  }
-
   protected RedisResponse respondBulkStrings(Object message) {
     if (message instanceof Collection) {
       return RedisResponse.array((Collection<?>) message);
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java
index 25104bd..f291f67 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java
@@ -57,11 +57,6 @@ public class Coder {
    * byte identifier of an integer
    */
   public static final byte INTEGER_ID = 58; // ':'
-
-  public static final byte OPEN_BRACE_ID = 0x28; // '('
-  public static final byte OPEN_BRACKET_ID = 0x5b; // '['
-  public static final byte HYPHEN_ID = 0x2d; // '-'
-  public static final byte PLUS_ID = 0x2b; // '+'
   public static final byte NUMBER_1_BYTE = 0x31; // '1'
   /**
    * byte identifier of a simple string
@@ -358,10 +353,6 @@ public class Coder {
     }
   }
 
-  public static ByteArrayWrapper stringToByteArrayWrapper(String s) {
-    return new ByteArrayWrapper(stringToBytes(s));
-  }
-
   /*
    * These toByte methods convert to byte arrays of the string representation of the input, not
    * literal to byte
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Command.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Command.java
index fca62da..14ccfb5 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Command.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Command.java
@@ -19,8 +19,6 @@ import java.nio.channels.SocketChannel;
 import java.util.List;
 import java.util.stream.Collectors;
 
-import io.netty.buffer.ByteBuf;
-
 import org.apache.geode.redis.internal.RedisCommandType;
 import org.apache.geode.redis.internal.data.ByteArrayWrapper;
 import org.apache.geode.redis.internal.executor.RedisResponse;
@@ -33,7 +31,6 @@ public class Command {
 
   private final List<byte[]> commandElems;
   private final RedisCommandType commandType;
-  private ByteBuf response;
   private String key;
   private ByteArrayWrapper bytes;
 
@@ -49,7 +46,6 @@ public class Command {
           "List of command elements cannot be empty -> List:" + commandElems);
     }
     this.commandElems = commandElems;
-    this.response = null;
 
     RedisCommandType type;
     try {
@@ -102,36 +98,6 @@ public class Command {
   }
 
   /**
-   * Getter method to get the response to be sent
-   *
-   * @return The response
-   */
-  public ByteBuf getResponse() {
-    return response;
-  }
-
-  /**
-   * Setter method to set the response to be sent
-   *
-   * @param response The response to be sent
-   */
-  public void setResponse(ByteBuf response) {
-    this.response = response;
-  }
-
-  public boolean hasError() {
-    if (response == null) {
-      return false;
-    }
-
-    if (response.getByte(0) == Coder.ERROR_ID) {
-      return true;
-    }
-
-    return false;
-  }
-
-  /**
    * Convenience method to get a String representation of the key in a Redis command, always at the
    * second position in the sent command array
    *
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
index f3ed225..494c9fa 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
@@ -84,12 +84,6 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter {
     this.isAuthenticated = password == null;
   }
 
-  private void flushChannel() {
-    while (needChannelFlush.getAndSet(false)) {
-      channel.flush();
-    }
-  }
-
   public ChannelFuture writeToChannel(ByteBuf message) {
     return channel.writeAndFlush(message, channel.newPromise());
   }