GEODE-8219: Apply RedisResponse to all redis string commands (apache#…

…5206)
shjgiser · Jun 4, 2020 · 80ad8dd · 80ad8dd
1 parent ac6c65f
commit 80ad8dd
Show file tree

Hide file tree

Showing 31 changed files with 258 additions and 289 deletions.
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisResponse.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisResponse.java
@@ -16,12 +16,14 @@
 
 package org.apache.geode.redis.internal;
 
+import java.nio.charset.Charset;
 import java.util.Collection;
 import java.util.List;
 import java.util.function.Function;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufAllocator;
+import io.netty.buffer.UnpooledByteBufAllocator;
 
 public class RedisResponse {
 
@@ -79,6 +81,10 @@ public static RedisResponse emptyArray() {
     return new RedisResponse(Coder::getEmptyArrayResponse);
   }
 
+  public static RedisResponse emptyString() {
+    return new RedisResponse(Coder::getEmptyStringResponse);
+  }
+
   public static RedisResponse error(String error) {
     return new RedisResponse((bba) -> Coder.getErrorResponse(bba, error));
   }
@@ -90,4 +96,11 @@ public static RedisResponse wrongType(String error) {
   public static RedisResponse scan(List<?> items) {
     return new RedisResponse((bba) -> Coder.getScanResponse(bba, items));
   }
+
+  /**
+   * Be aware that this implementation will create extra garbage since it allocates from the heap.
+   */
+  public String toString() {
+    return encode(new UnpooledByteBufAllocator(false)).toString(Charset.defaultCharset());
+  }
 }
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
@@ -80,7 +80,7 @@ protected RedisResponse respondBulkStrings(Object message) {
     if (message instanceof Collection) {
       return RedisResponse.array((Collection<?>) message);
     } else {
-      return RedisResponse.string((String) message);
+      return RedisResponse.bulkString(message);
     }
   }
 

diff --git a/...e-redis/src/main/java/org/apache/geode/redis/internal/executor/string/AppendExecutor.java b/...e-redis/src/main/java/org/apache/geode/redis/internal/executor/string/AppendExecutor.java
@@ -18,23 +18,24 @@
 import java.util.List;
 
 import org.apache.geode.redis.internal.ByteArrayWrapper;
-import org.apache.geode.redis.internal.Coder;
 import org.apache.geode.redis.internal.Command;
 import org.apache.geode.redis.internal.ExecutionHandlerContext;
+import org.apache.geode.redis.internal.RedisResponse;
 
 public class AppendExecutor extends StringExecutor {
 
   private final int VALUE_INDEX = 2;
 
   @Override
-  public void executeCommand(Command command, ExecutionHandlerContext context) {
+  public RedisResponse executeCommandWithResponse(Command command,
+      ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
     ByteArrayWrapper key = command.getKey();
     byte[] bytesToAppend = commandElems.get(VALUE_INDEX);
     ByteArrayWrapper valueToAppend = new ByteArrayWrapper(bytesToAppend);
 
     long returnValue = getRedisStringCommands(context).append(key, valueToAppend);
 
-    command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), returnValue));
+    return RedisResponse.integer(returnValue);
   }
 }
diff --git a/...redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitCountExecutor.java b/...redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitCountExecutor.java
@@ -22,26 +22,26 @@
 import org.apache.geode.redis.internal.ExecutionHandlerContext;
 import org.apache.geode.redis.internal.RedisConstants;
 import org.apache.geode.redis.internal.RedisConstants.ArityDef;
+import org.apache.geode.redis.internal.RedisResponse;
 
 public class BitCountExecutor extends StringExecutor {
 
   private final String ERROR_NOT_INT = "The indexes provided must be numeric values";
 
   @Override
-  public void executeCommand(Command command, ExecutionHandlerContext context) {
+  public RedisResponse executeCommandWithResponse(Command command,
+      ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
 
     if (commandElems.size() != 2 && commandElems.size() != 4) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ArityDef.BITCOUNT));
-      return;
+      return RedisResponse.error(ArityDef.BITCOUNT);
     }
 
     ByteArrayWrapper key = command.getKey();
     RedisStringCommands stringCommands = getRedisStringCommands(context);
     ByteArrayWrapper wrapper = stringCommands.get(key);
     if (wrapper == null) {
-      command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), 0));
-      return;
+      return RedisResponse.integer(0);
     }
     byte[] value = wrapper.toBytes();
 
@@ -53,14 +53,11 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
         startL = Coder.bytesToLong(commandElems.get(2));
         endL = Coder.bytesToLong(commandElems.get(3));
       } catch (NumberFormatException e) {
-        command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_NOT_INT));
-        return;
+        return RedisResponse.error(ERROR_NOT_INT);
       }
     }
     if (startL > Integer.MAX_VALUE || endL > Integer.MAX_VALUE) {
-      command.setResponse(
-          Coder.getErrorResponse(context.getByteBufAllocator(), RedisConstants.ERROR_OUT_OF_RANGE));
-      return;
+      return RedisResponse.error(RedisConstants.ERROR_OUT_OF_RANGE);
     }
 
     int start = (int) startL;
@@ -84,8 +81,7 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
     }
 
     if (end < start || start >= value.length) {
-      command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), 0));
-      return;
+      return RedisResponse.integer(0);
     }
 
     long setBits = 0;
@@ -94,7 +90,7 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
     }
     // opposed to keeping the same value
 
-    command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), setBits));
+    return RedisResponse.integer(setBits);
   }
 
 }
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitOpExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitOpExecutor.java
@@ -17,22 +17,22 @@
 import java.util.List;
 
 import org.apache.geode.redis.internal.ByteArrayWrapper;
-import org.apache.geode.redis.internal.Coder;
 import org.apache.geode.redis.internal.Command;
 import org.apache.geode.redis.internal.ExecutionHandlerContext;
 import org.apache.geode.redis.internal.RedisConstants.ArityDef;
+import org.apache.geode.redis.internal.RedisResponse;
 
 public class BitOpExecutor extends StringExecutor {
 
   private static final String ERROR_NO_SUCH_OP = "Please specify a legal operation";
 
   @Override
-  public void executeCommand(Command command, ExecutionHandlerContext context) {
+  public RedisResponse executeCommandWithResponse(Command command,
+      ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
 
     if (commandElems.size() < 4) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ArityDef.BITOP));
-      return;
+      return RedisResponse.error(ArityDef.BITOP);
     }
 
     String operation = command.getStringKey().toUpperCase();
@@ -70,11 +70,10 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
     } else if (operation.equals("NOT")) {
       not(context, destKey, values, maxLength);
     } else {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_NO_SUCH_OP));
-      return;
+      return RedisResponse.error(ERROR_NO_SUCH_OP);
     }
 
-    command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), maxLength));
+    return RedisResponse.integer(maxLength);
   }
 
   private void and(ExecutionHandlerContext context,

diff --git a/...e-redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitPosExecutor.java b/...e-redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitPosExecutor.java
@@ -21,6 +21,7 @@
 import org.apache.geode.redis.internal.Command;
 import org.apache.geode.redis.internal.ExecutionHandlerContext;
 import org.apache.geode.redis.internal.RedisConstants.ArityDef;
+import org.apache.geode.redis.internal.RedisResponse;
 
 public class BitPosExecutor extends StringExecutor {
 
@@ -29,11 +30,11 @@ public class BitPosExecutor extends StringExecutor {
   private final String ERROR_BIT = "The bit must either be a 0 or 1";
 
   @Override
-  public void executeCommand(Command command, ExecutionHandlerContext context) {
+  public RedisResponse executeCommandWithResponse(Command command,
+      ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
     if (commandElems.size() < 3) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ArityDef.BITPOS));
-      return;
+      return RedisResponse.error(ArityDef.BITPOS);
     }
 
     ByteArrayWrapper key = command.getKey();
@@ -47,27 +48,16 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
       byte[] bitAr = commandElems.get(2);
       bit = Coder.bytesToInt(bitAr);
     } catch (NumberFormatException e) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_NOT_INT));
-      return;
+      return RedisResponse.error(ERROR_NOT_INT);
     }
 
     if (bit != 0 && bit != 1) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_BIT));
-      return;
+      return RedisResponse.error(ERROR_BIT);
     }
 
     if (string == null || string.length() == 0) {
-      command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), -bit)); // Redis
-      // returns
-      // 0 when
-      // key
-      // does
-      // not
-      // exists
-      // for
-      // this
-      // command
-      return;
+      // Redis returns 0 when key does not exists for this command
+      return RedisResponse.integer(-bit);
     }
     byte[] bytes = string.toBytes();
     int start = 0;
@@ -77,8 +67,7 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
         byte[] startAr = commandElems.get(3);
         start = Coder.bytesToInt(startAr);
       } catch (NumberFormatException e) {
-        command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_NOT_INT));
-        return;
+        return RedisResponse.error(ERROR_NOT_INT);
       }
     }
 
@@ -88,8 +77,7 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
         end = Coder.bytesToInt(endAr);
         endSet = true;
       } catch (NumberFormatException e) {
-        command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_NOT_INT));
-        return;
+        return RedisResponse.error(ERROR_NOT_INT);
       }
     }
 
@@ -115,8 +103,7 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
     }
 
     if (end < start) {
-      command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), -1));
-      return;
+      return RedisResponse.integer(-1);
     }
 
     outerLoop: for (int i = start; i <= end; i++) {
@@ -135,7 +122,7 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
       bitPosition = bytes.length * 8;
     }
 
-    command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), bitPosition));
+    return RedisResponse.integer(bitPosition);
   }
 
 }
diff --git a/...e-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrByExecutor.java b/...e-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrByExecutor.java
@@ -22,6 +22,7 @@
 import org.apache.geode.redis.internal.Command;
 import org.apache.geode.redis.internal.ExecutionHandlerContext;
 import org.apache.geode.redis.internal.RedisConstants.ArityDef;
+import org.apache.geode.redis.internal.RedisResponse;
 
 public class DecrByExecutor extends StringExecutor {
 
@@ -31,12 +32,12 @@ public class DecrByExecutor extends StringExecutor {
   private final int DECREMENT_INDEX = 2;
 
   @Override
-  public void executeCommand(Command command, ExecutionHandlerContext context) {
+  public RedisResponse executeCommandWithResponse(Command command,
+      ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
 
     if (commandElems.size() < 3) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ArityDef.DECRBY));
-      return;
+      return RedisResponse.error(ArityDef.DECRBY);
     }
     ByteArrayWrapper key = command.getKey();
 
@@ -47,12 +48,10 @@ public void executeCommand(Command command, ExecutionHandlerContext context) {
     try {
       decrement = Long.parseLong(decrString);
     } catch (NumberFormatException e) {
-      command.setResponse(
-          Coder.getErrorResponse(context.getByteBufAllocator(), ERROR_DECREMENT_NOT_USABLE));
-      return;
+      return RedisResponse.error(ERROR_DECREMENT_NOT_USABLE);
     }
 
     long value = getRedisStringCommands(context).decrby(key, decrement);
-    command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), value));
+    return RedisResponse.integer(value);
   }
 }
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrExecutor.java
@@ -18,26 +18,26 @@
 import java.util.List;
 
 import org.apache.geode.redis.internal.ByteArrayWrapper;
-import org.apache.geode.redis.internal.Coder;
 import org.apache.geode.redis.internal.Command;
 import org.apache.geode.redis.internal.ExecutionHandlerContext;
 import org.apache.geode.redis.internal.RedisConstants.ArityDef;
+import org.apache.geode.redis.internal.RedisResponse;
 
 public class DecrExecutor extends StringExecutor {
 
   @Override
-  public void executeCommand(Command command, ExecutionHandlerContext context) {
+  public RedisResponse executeCommandWithResponse(Command command,
+      ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
 
     if (commandElems.size() < 2) {
-      command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ArityDef.DECR));
-      return;
+      return RedisResponse.error(ArityDef.DECR);
     }
 
     ByteArrayWrapper key = command.getKey();
     RedisStringCommands stringCommands = getRedisStringCommands(context);
 
     long value = stringCommands.decr(key);
-    command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), value));
+    return RedisResponse.integer(value);
   }
 }