From d33f88a7d82a6d1d8fd1b0381781e07f5a07622f Mon Sep 17 00:00:00 2001 From: Manuel McLure Date: Wed, 18 Sep 2024 16:19:32 -0700 Subject: [PATCH] Add more tests and logic fixes --- libs/server/Objects/Hash/HashObjectImpl.cs | 31 +++++--- test/Garnet.test/RespHashTests.cs | 90 +++++++++++++++++++--- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/libs/server/Objects/Hash/HashObjectImpl.cs b/libs/server/Objects/Hash/HashObjectImpl.cs index 785f05696aa..5d0d532174f 100644 --- a/libs/server/Objects/Hash/HashObjectImpl.cs +++ b/libs/server/Objects/Hash/HashObjectImpl.cs @@ -519,7 +519,8 @@ private void HashExpire(ref ObjectInput input, ref SpanByteAndMemory output) } while (!RespWriteUtils.WriteArrayLength(fieldCount, ref curr, end)) ObjectUtils.ReallocateOutput(ref output, ref isMemory, ref ptr, ref ptrHandle, ref curr, ref end); - for (var fieldIdx = 0; fieldIdx < fieldCount; fieldIdx++) + + for (var fieldIdx = 0; fieldIdx < fieldCount; fieldIdx++, currIdx++) { var key = parseState.GetArgSliceByRef(currIdx).SpanByte.ToByteArray(); @@ -528,17 +529,9 @@ private void HashExpire(ref ObjectInput input, ref SpanByteAndMemory output) { result = -2; } - else if (DateTimeOffset.UtcNow >= expiryTime) - { - // If provided expiration time is before or equal to now, delete key - if (hash.Remove(key)) - { - this.UpdateSize(key, hashValue.Value, false); - } - result = 2; - } else { + Debug.Print($"HashExpire: old: {hashValue.Expiration} new: {expiryTime.Ticks}"); switch (expireOption) { case ExpireOption.NX: // Only set if not already set @@ -562,7 +555,23 @@ private void HashExpire(ref ObjectInput input, ref SpanByteAndMemory output) } if (result != 0) // Option did not reject the operation { - hashValue.Expiration = expiryTime.Ticks; // Update the expiration time + if (DateTimeOffset.UtcNow >= expiryTime) + { + // If provided expiration time is before or equal to now, delete key + if (hash.Remove(key)) + { + this.UpdateSize(key, hashValue.Value, false); + } + result = 2; + Debug.Print($"Deleted value"); + } + else + { + hashValue.Expiration = expiryTime.Ticks; // Update the expiration time + hash[key] = hashValue; + Debug.Print($"Set expiration to {expiryTime.Ticks}"); + } + } } while (!RespWriteUtils.WriteInteger(result, ref curr, end)) diff --git a/test/Garnet.test/RespHashTests.cs b/test/Garnet.test/RespHashTests.cs index fa91c2e982f..185ec4432b9 100644 --- a/test/Garnet.test/RespHashTests.cs +++ b/test/Garnet.test/RespHashTests.cs @@ -1184,26 +1184,26 @@ public void HExpireCommandParameters(string command) var lightClientRequest = TestUtils.CreateRequest(); var middleTtl = command switch { - "HEXPIRE" => 20, - "HPEXPIRE" => 20000, - "HEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(20).ToUnixTimeSeconds(), - "HPEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(20).ToUnixTimeMilliseconds(), + "HEXPIRE" => 120, + "HPEXPIRE" => 120000, + "HEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(120).ToUnixTimeSeconds(), + "HPEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(120).ToUnixTimeMilliseconds(), _ => 10 }; var largerTtl = command switch { - "HEXPIRE" => 30, - "HPEXPIRE" => 30000, - "HEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(30).ToUnixTimeSeconds(), - "HPEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(30).ToUnixTimeMilliseconds(), + "HEXPIRE" => 240, + "HPEXPIRE" => 240000, + "HEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(240).ToUnixTimeSeconds(), + "HPEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(240).ToUnixTimeMilliseconds(), _ => 10 }; var smallerTtl = command switch { - "HEXPIRE" => 10, - "HPEXPIRE" => 10000, - "HEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(10).ToUnixTimeSeconds(), - "HPEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(10).ToUnixTimeMilliseconds(), + "HEXPIRE" => 60, + "HPEXPIRE" => 60000, + "HEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(60).ToUnixTimeSeconds(), + "HPEXPIREAT" => DateTimeOffset.UtcNow.AddSeconds(60).ToUnixTimeMilliseconds(), _ => 10 }; // This value should ensure the key is deleted @@ -1288,6 +1288,72 @@ public void HExpireCommandParameters(string command) expectedResponse = "$-1\r\n"; actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // Add three fields (this also clears their TTLs) + res = lightClientRequest.SendCommand($"HSET {key} field1 1 field2 1 field3 1"); + expectedResponse = ":3\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // Set TTL on the first two fields + res = lightClientRequest.SendCommand($"{command} {key} {middleTtl} FIELDS 2 field1 field2", 3); + expectedResponse = "*2\r\n:1\r\n:1\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // XX on a field with no TTL will fail + res = lightClientRequest.SendCommand($"{command} {key} {middleTtl} XX FIELDS 1 field3", 2); + expectedResponse = "*1\r\n:0\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // XX on a field with TTL will succeed + res = lightClientRequest.SendCommand($"{command} {key} {middleTtl} XX FIELDS 1 field1", 2); + expectedResponse = "*1\r\n:1\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // NX on a field with TTL will fail + res = lightClientRequest.SendCommand($"{command} {key} {middleTtl} NX FIELDS 1 field1", 2); + expectedResponse = "*1\r\n:0\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // NX on a field with no TTL will succeed + res = lightClientRequest.SendCommand($"{command} {key} {middleTtl} NX FIELDS 1 field3", 2); + expectedResponse = "*1\r\n:1\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // Set TTL on both fields again + res = lightClientRequest.SendCommand($"{command} {key} {middleTtl} FIELDS 2 field1 field2", 3); + expectedResponse = "*2\r\n:1\r\n:1\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // LT when new TTL is larger will fail + res = lightClientRequest.SendCommand($"{command} {key} {largerTtl} LT FIELDS 1 field1", 2); + expectedResponse = "*1\r\n:0\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // LT when new TTL is smaller will succeed + res = lightClientRequest.SendCommand($"{command} {key} {smallerTtl} LT FIELDS 1 field1", 2); + expectedResponse = "*1\r\n:1\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // GT when new TTL is smaller will fail + res = lightClientRequest.SendCommand($"{command} {key} {smallerTtl} GT FIELDS 1 field2", 2); + expectedResponse = "*1\r\n:0\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); + + // GT when new TTL is larger will succeed + res = lightClientRequest.SendCommand($"{command} {key} {largerTtl} GT FIELDS 1 field2", 2); + expectedResponse = "*1\r\n:1\r\n"; + actualResponse = Encoding.ASCII.GetString(res).Substring(0, expectedResponse.Length); + ClassicAssert.AreEqual(expectedResponse, actualResponse); } #endregion