Fix SET KEEPTTL return string (#1039)

* SETKEEPTTL (no other flags) should always return ok. It doesn't care about NX or XX

* Simplify GETSET

---------

Co-authored-by: prvyk <[email protected]>
Co-authored-by: Tal Zaccai <[email protected]>

libs/server/Resp/BasicCommands.cs

@@ -303,12 +303,10 @@ private bool NetworkGETSET<TGarnetApi>(ref TGarnetApi storageApi)
             where TGarnetApi : IGarnetApi
         {
             Debug.Assert(parseState.Count == 2);
-            var key = parseState.GetArgSliceByRef(0);
-            var value = parseState.GetArgSliceByRef(1);
-            var getOption = ArgSlice.FromPinnedSpan(CmdStrings.GET);
-            parseState.InitializeWithArguments(key, value, getOption);
+            var key = parseState.GetArgSliceByRef(0).SpanByte;
 
-            return NetworkSETEXNX(ref storageApi);
+            return NetworkSET_Conditional(RespCommand.SET, 0, ref key, true,
+                                          false, false, ref storageApi);
         }
 
         /// <summary>
@@ -431,7 +429,11 @@ private bool NetworkSETEX<TGarnetApi>(bool highPrecision, ref TGarnetApi storage
         private bool NetworkSETNX<TGarnetApi>(bool highPrecision, ref TGarnetApi storageApi)
             where TGarnetApi : IGarnetApi
         {
-            Debug.Assert(parseState.Count == 2);
+            if (parseState.Count != 2)
+            {
+                return AbortWithWrongNumberOfArguments(nameof(RespCommand.SETNX));
+            }
+
             var key = parseState.GetArgSliceByRef(0);
             var sbKey = key.SpanByte;
 
@@ -660,24 +662,34 @@ private bool NetworkSET_Conditional<TGarnetApi>(RespCommand cmd, int expiry, ref
                 // the following debug assertion is the catch any edge case leading to SETIFMATCH, or SETIFGREATER skipping the above block
                 Debug.Assert(cmd is not (RespCommand.SETIFMATCH or RespCommand.SETIFGREATER), "SETIFMATCH should have gone though pointing to right output variable");
 
-                GarnetStatus status = storageApi.SET_Conditional(ref key, ref input);
+                var status = storageApi.SET_Conditional(ref key, ref input);
 
-                bool ok = status != GarnetStatus.NOTFOUND;
-
-                // the status returned for SETEXNX as NOTFOUND is the expected status in the happy path, so flip the ok flag
-                if (cmd == RespCommand.SETEXNX)
-                    ok = !ok;
-
-                if (ok)
+                // KEEPTTL without flags doesn't care whether it was found or not.
+                if (cmd == RespCommand.SETKEEPTTL)
                 {
                     while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
                         SendAndReset();
                 }
                 else
                 {
-                    while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend))
-                        SendAndReset();
+                    var ok = status != GarnetStatus.NOTFOUND;
+
+                    // the status returned for SETEXNX as NOTFOUND is the expected status in the happy path, so flip the ok flag
+                    if (cmd == RespCommand.SETEXNX)
+                        ok = !ok;
+
+                    if (ok)
+                    {
+                        while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
+                            SendAndReset();
+                    }
+                    else
+                    {
+                        while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend))
+                            SendAndReset();
+                    }
                 }
+
                 return true;
             }
             else

test/Garnet.test/RespTests.cs

@@ -3633,6 +3633,8 @@ public void KeepTtlTest()
             string value = db.StringGet(keyA);
             ClassicAssert.AreEqual(null, value);
 
+            ClassicAssert.AreEqual("OK", db.Execute("SET", keyA, keyA, "KEEPTTL")?.ToString());
+
             value = db.StringGet(keyB);
             ClassicAssert.AreEqual(keyB, value);
         }