dschneider-pivotal commented on a change in pull request #5165: URL: https://github.com/apache/geode/pull/5165#discussion_r431238457
########## File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetNXExecutor.java ########## @@ -36,27 +36,23 @@ public void executeCommand(Command command, ExecutionHandlerContext context) { List<byte[]> commandElems = command.getProcessedCommand(); - Region<ByteArrayWrapper, RedisData> region = - context.getRegionProvider().getStringsRegion(); - if (commandElems.size() < 3) { command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), ArityDef.SETNX)); return; } ByteArrayWrapper key = command.getKey(); - checkAndSetDataType(key, context); - byte[] value = commandElems.get(VALUE_INDEX); + ByteArrayWrapper value = new ByteArrayWrapper(commandElems.get(VALUE_INDEX)); - Object oldValue = - region.putIfAbsent(key, (RedisData) new RedisString(new ByteArrayWrapper(value))); + RedisStringCommands stringCommands = getRedisStringCommands(context); + SetOptions setOptions = new SetOptions(NX, 0L, null, false); Review comment: Thanks for pointing this out. At this time, all that is used on the SetOptions when we call RedisStringCommands.set is the first parameter (NX in this case) so I just went with defaults on all the others. Some of these apply to expiration so they will be reviewed when the string expiration work is done. ---------------------------------------------------------------- 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