[
https://issues.apache.org/jira/browse/GEODE-8219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17125570#comment-17125570
]
ASF GitHub Bot commented on GEODE-8219:
---------------------------------------
dschneider-pivotal commented on a change in pull request #5206:
URL: https://github.com/apache/geode/pull/5206#discussion_r435011635
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetBitExecutor.java
##########
@@ -34,12 +35,13 @@
"The offset is out of range, must be greater than or equal to 0 and at
most 4294967295 (512MB)";
@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.SETBIT));
- return;
+ return RedisResponse.error(ArityDef.SETBIT);
Review comment:
remove this setResponse calls
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/BitPosExecutor.java
##########
@@ -47,27 +48,17 @@ 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
+
command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(),
-bit));
Review comment:
Should this setResponse be removed?
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetBitExecutor.java
##########
@@ -81,31 +79,30 @@ public void executeCommand(Command command,
ExecutionHandlerContext context) {
}
stringCommands.set(key, new ByteArrayWrapper(bytes), null);
command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), 0));
Review comment:
I think this setReponse call should have been deleted
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetExecutor.java
##########
@@ -42,23 +44,24 @@ public void executeCommand(Command command,
ExecutionHandlerContext context) {
try {
setOptions = parseCommandElems(commandElems);
} catch (IllegalArgumentException ex) {
-
command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(),
ex.getMessage()));
- return;
+ return RedisResponse.error(ex.getMessage());
}
- doSet(command, context, keyToSet, valueToSet, redisStringCommands,
setOptions);
+ return doSet(command, context, keyToSet, valueToSet, redisStringCommands,
setOptions);
}
- private void doSet(Command command, ExecutionHandlerContext context,
ByteArrayWrapper key,
+ private RedisResponse doSet(Command command, ExecutionHandlerContext context,
+ ByteArrayWrapper key,
ByteArrayWrapper value, RedisStringCommands redisStringCommands,
SetOptions setOptions) {
boolean result = redisStringCommands.set(key, value, setOptions);
if (result) {
command.setResponse(Coder.getSimpleStringResponse(context.getByteBufAllocator(),
SUCCESS));
- } else {
- command.setResponse(Coder.getNilResponse(context.getByteBufAllocator()));
+ return RedisResponse.string(SUCCESS);
Review comment:
remove this setResponse call
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Apply RedisResponse to all redis string commands
> ------------------------------------------------
>
> Key: GEODE-8219
> URL: https://issues.apache.org/jira/browse/GEODE-8219
> Project: Geode
> Issue Type: Improvement
> Components: redis
> Reporter: Jens Deppe
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
