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


Reply via email to