Fix RCU Bug in metadata and value overlap of SETIFGREATER and SETIFMATCH (#1185)

* Fix Bug in SETIFGREATER and SETIFMATCH for no-exp to exp case

* retrigger tests

* fmt

---------

Co-authored-by: Hamdaan Khalid <[email protected]>

Author: hamdaankhalid

libs/server/Storage/Functions/MainStore/RMWMethods.cs

@@ -1058,22 +1058,22 @@ public bool CopyUpdater(ref SpanByte key, ref RawStringInput input, ref SpanByte
                     // By now the comparison for etag against existing etag has already been done in NeedCopyUpdate
                     shouldUpdateEtag = true;
                     // Copy input to value
-                    Span<byte> dest = newValue.AsSpan(EtagConstants.EtagSize);
                     ReadOnlySpan<byte> src = input.parseState.GetArgSliceByRef(0).ReadOnlySpan;
 
                     // retain metadata unless metadata sent
                     int metadataSize = input.arg1 != 0 ? sizeof(long) : oldValue.MetadataSize;
 
                     Debug.Assert(src.Length + EtagConstants.EtagSize + metadataSize == newValue.Length);
 
-                    src.CopyTo(dest);
-
                     newValue.ExtraMetadata = oldValue.ExtraMetadata;
                     if (input.arg1 != 0)
                     {
                         newValue.ExtraMetadata = input.arg1;
                     }
 
+                    Span<byte> dest = newValue.AsSpan(EtagConstants.EtagSize);
+                    src.CopyTo(dest);
+
                     etagFromClient = input.parseState.GetLong(1);
 
                     functionsState.etagState.etag = etagFromClient;

test/Garnet.test/RespEtagTests.cs

@@ -396,6 +396,7 @@ public void SetIfGreaterWorksWithoutInitialETag()
         #endregion
 
         #region ETAG DEL Happy Paths
+
         [Test]
         public void DelIfGreaterOnAnAlreadyExistingKeyWithEtagWorks()
         {
@@ -590,6 +591,60 @@ private void MakeReadOnly(long untilAddress, IServer server, IDatabase db)
         #endregion
 
         #region Edgecases
+
+        [Test]
+        [TestCase("m", "mo", null)] // RCU with no existing exp on noetag key
+        [TestCase("mexicanmochawithdoubleespresso", "c", null)] // IPU with no existing exp on noetag key
+        [TestCase("m", "mo", 30)] // RCU with existing exp on noetag key
+        [TestCase("mexicanmochawithdoubleespresso", "c", 30)] // IPU with existing exp on noetag key
+        public void SetIfGreaterWhenExpIsSentForExistingNonEtagKey(string initialValue, string newValue, double? exp)
+        {
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            IDatabase db = redis.GetDatabase(0);
+            var key = "meow-key";
+
+            if (exp != null)
+                db.StringSet(key, initialValue, TimeSpan.FromSeconds(exp.Value));
+            else
+                db.StringSet(key, initialValue);
+
+            RedisResult[] arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, newValue, 5, "EX", 90);
+
+            ClassicAssert.AreEqual(5, (long)arrRes[0]);
+            ClassicAssert.IsTrue(arrRes[1].IsNull);
+
+            var res = db.StringGetWithExpiry(key);
+            ClassicAssert.AreEqual(newValue, res.Value.ToString());
+            ClassicAssert.IsTrue(res.Expiry.HasValue);
+        }
+
+        [Test]
+        [TestCase("m", "mo", null)] // RCU with no existing exp on noetag key
+        [TestCase("mexicanmochawithdoubleespresso", "c", null)] // IPU with no existing exp on noetag key
+        [TestCase("m", "mo", 30)] // RCU with existing exp on noetag key
+        [TestCase("mexicanmochawithdoubleespresso", "c", 30)] // IPU with existing exp on noetag key
+        public void SetIfMatchWhenExpIsSentForExistingNonEtagKey(string initialValue, string newValue, int? exp)
+        {
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            IDatabase db = redis.GetDatabase(0);
+            var key = "meow-key";
+
+            if (exp != null)
+                db.StringSet(key, initialValue, TimeSpan.FromSeconds(exp.Value));
+            else
+                db.StringSet(key, initialValue);
+
+            RedisResult[] arrRes = (RedisResult[])db.Execute("SETIFMATCH", key, newValue, 0, "EX", 90);
+
+            ClassicAssert.AreEqual(1, (long)arrRes[0]);
+            ClassicAssert.IsTrue(arrRes[1].IsNull);
+
+            var res = db.StringGetWithExpiry(key);
+            ClassicAssert.AreEqual(newValue, res.Value.ToString());
+            ClassicAssert.IsTrue(res.Expiry.HasValue);
+        }
+
+
         [Test]
         public void SetIfMatchSetsKeyValueOnNonExistingKey()
         {