From 7fb0f8f3af404df126a36249f9c76c50d0fdec03 Mon Sep 17 00:00:00 2001
From: prvyk <52283326+prvyk@users.noreply.github.com>
Date: Sat, 25 Jan 2025 02:13:19 +0200
Subject: [PATCH] SETNX command should return integer value (#951)

* Fix SETNX return value, fix test and add new test for this.

* dotnet format

* Clearer comment

* Move new SETNX test to right file

* Remove unneeded code

* Nit

* No need for this optional argument

---------

Co-authored-by: prvyk <github@privatemail.fastmailbox.net>
Co-authored-by: Badrish Chandramouli <badrishc@microsoft.com>
---
 libs/server/Resp/BasicCommands.cs             | 14 +++++++++----
 test/Garnet.test/Resp/ACL/RespCommandTests.cs |  4 ++--
 test/Garnet.test/RespSetTest.cs               |  1 -
 test/Garnet.test/RespTests.cs                 | 20 +++++++++++++++++++
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/libs/server/Resp/BasicCommands.cs b/libs/server/Resp/BasicCommands.cs
index a5c0867c53..6c351be832 100644
--- a/libs/server/Resp/BasicCommands.cs
+++ b/libs/server/Resp/BasicCommands.cs
@@ -434,11 +434,17 @@ private bool NetworkSETNX<TGarnetApi>(bool highPrecision, ref TGarnetApi storage
         {
             Debug.Assert(parseState.Count == 2);
             var key = parseState.GetArgSliceByRef(0);
-            var value = parseState.GetArgSliceByRef(1);
-            var getOption = ArgSlice.FromPinnedSpan(CmdStrings.NX);
-            parseState.InitializeWithArguments(key, value, getOption);
+            var sbKey = key.SpanByte;
 
-            return NetworkSETEXNX(ref storageApi);
+            var input = new RawStringInput(RespCommand.SETEXNX, ref parseState, startIdx: 1);
+            var status = storageApi.SET_Conditional(ref sbKey, ref input);
+
+            // The status returned for SETNX as NOTFOUND is the expected status in the happy path
+            var retVal = status == GarnetStatus.NOTFOUND ? 1 : 0;
+            while (!RespWriteUtils.TryWriteInt32(retVal, ref dcurr, dend))
+                SendAndReset();
+
+            return true;
         }
 
         /// <summary>
diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs
index f9a72c56fc..91ea4fb2e8 100644
--- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs
+++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs
@@ -5369,8 +5369,8 @@ await CheckCommandsAsync(
 
             async Task DoSetIfNotExistAsync(GarnetClient client)
             {
-                string val = await client.ExecuteForStringResultAsync("SETNX", [$"foo-{keyIx++}", "bar"]);
-                ClassicAssert.AreEqual(val, "OK");
+                var val = await client.ExecuteForLongResultAsync("SETNX", [$"foo-{keyIx++}", "bar"]);
+                ClassicAssert.AreEqual(1, val);
             }
         }
 
diff --git a/test/Garnet.test/RespSetTest.cs b/test/Garnet.test/RespSetTest.cs
index 59344b462d..fbb1eae1ec 100644
--- a/test/Garnet.test/RespSetTest.cs
+++ b/test/Garnet.test/RespSetTest.cs
@@ -1389,7 +1389,6 @@ public void CanDoSinterCardLC(string values1, string values2, string expectedCou
             expectedResponse = ":0\r\n";
             ClassicAssert.AreEqual(expectedResponse, response.AsSpan().Slice(0, expectedResponse.Length).ToArray());
         }
-
         #endregion
 
 
diff --git a/test/Garnet.test/RespTests.cs b/test/Garnet.test/RespTests.cs
index 670b0f4f69..36f599e8eb 100644
--- a/test/Garnet.test/RespTests.cs
+++ b/test/Garnet.test/RespTests.cs
@@ -4581,6 +4581,26 @@ public void SetIfNotExistWithNewKey()
             ClassicAssert.AreEqual(newValue, result);
         }
 
+        [Test]
+        public void SetNXCorrectResponse()
+        {
+            var lightClientRequest = TestUtils.CreateRequest();
+
+            // Set key
+            var response = lightClientRequest.SendCommand("SETNX key1 2");
+            var expectedResponse = ":1\r\n";
+            ClassicAssert.AreEqual(expectedResponse, response.AsSpan().Slice(0, expectedResponse.Length).ToArray());
+
+            // Setnx command should fail since key exists
+            response = lightClientRequest.SendCommand("SETNX key1 3");
+            expectedResponse = ":0\r\n";
+            ClassicAssert.AreEqual(expectedResponse, response.AsSpan().Slice(0, expectedResponse.Length).ToArray());
+
+            // Be sure key wasn't modified
+            response = lightClientRequest.SendCommand("GET key1");
+            expectedResponse = "$1\r\n2\r\n";
+            ClassicAssert.AreEqual(expectedResponse, response.AsSpan().Slice(0, expectedResponse.Length).ToArray());
+        }
         #endregion
 
         #region SUBSTR