From 114c9facf60a2c070f3f208a05f14e4aab670783 Mon Sep 17 00:00:00 2001 From: prvyk Date: Sat, 8 Mar 2025 12:37:44 +0200 Subject: [PATCH] Fix BZMPOP return values, also add more argument checking. Add test. --- libs/server/Resp/Objects/SortedSetCommands.cs | 8 +++--- .../RespBlockingCollectionTests.cs | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/libs/server/Resp/Objects/SortedSetCommands.cs b/libs/server/Resp/Objects/SortedSetCommands.cs index e42100eb95..569672b7c5 100644 --- a/libs/server/Resp/Objects/SortedSetCommands.cs +++ b/libs/server/Resp/Objects/SortedSetCommands.cs @@ -1552,7 +1552,7 @@ private unsafe bool SortedSetBlockingPop(RespCommand command) return AbortWithWrongNumberOfArguments(command.ToString()); } - if (!parseState.TryGetDouble(parseState.Count - 1, out var timeout)) + if (!parseState.TryGetDouble(parseState.Count - 1, out var timeout) || (timeout < 0)) { return AbortWithErrorMessage(CmdStrings.RESP_ERR_TIMEOUT_NOT_VALID_FLOAT); } @@ -1605,13 +1605,13 @@ private unsafe bool SortedSetBlockingMPop() var currTokenId = 0; // Read timeout - if (!parseState.TryGetDouble(currTokenId++, out var timeout)) + if (!parseState.TryGetDouble(currTokenId++, out var timeout) || (timeout < 0)) { return AbortWithErrorMessage(CmdStrings.RESP_ERR_TIMEOUT_NOT_VALID_FLOAT); } // Read count of keys - if (!parseState.TryGetInt(currTokenId++, out var numKeys)) + if (!parseState.TryGetInt(currTokenId++, out var numKeys) || (numKeys <= 0)) { var err = string.Format(CmdStrings.GenericParamShouldBeGreaterThanZero, "numkeys"); return AbortWithErrorMessage(Encoding.ASCII.GetBytes(err)); @@ -1685,7 +1685,7 @@ private unsafe bool SortedSetBlockingMPop() while (!RespWriteUtils.TryWriteArrayLength(result.Items.Length, ref dcurr, dend)) SendAndReset(); - for (var i = 0; i < result.Items.Length; i += 2) + for (var i = 0; i < result.Items.Length; ++i) { while (!RespWriteUtils.TryWriteArrayLength(2, ref dcurr, dend)) SendAndReset(); diff --git a/test/Garnet.test/RespBlockingCollectionTests.cs b/test/Garnet.test/RespBlockingCollectionTests.cs index 6292038e48..a300115e8e 100644 --- a/test/Garnet.test/RespBlockingCollectionTests.cs +++ b/test/Garnet.test/RespBlockingCollectionTests.cs @@ -7,6 +7,7 @@ using Garnet.server; using NUnit.Framework; using NUnit.Framework.Legacy; +using StackExchange.Redis; namespace Garnet.test { @@ -413,6 +414,30 @@ public void BasicBzmpopTest(string mode, string expectedValue, double expectedSc ClassicAssert.AreEqual(expectedResponse, actualValue); } + [Test] + public void BzmpopReturnTest() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var res = db.SortedSetAdd("a", [new SortedSetEntry("one", 1)]); + ClassicAssert.AreEqual(1, res); + res = db.SortedSetAdd("a", [new SortedSetEntry("two", 2)]); + ClassicAssert.AreEqual(1, res); + res = db.SortedSetAdd("b", [new SortedSetEntry("three", 3)]); + ClassicAssert.AreEqual(1, res); + var pop = db.Execute("BZMPOP", "10", "2", "a", "b", "MIN", "COUNT", "2"); + ClassicAssert.AreEqual(2, pop.Length); + ClassicAssert.AreEqual("a", pop[0].ToString()); + ClassicAssert.AreEqual(2, pop[1].Length); + ClassicAssert.AreEqual(2, pop[1][0].Length); + ClassicAssert.AreEqual("one", pop[1][0][0].ToString()); + ClassicAssert.AreEqual(1, (int)(RedisValue)pop[1][0][1]); + ClassicAssert.AreEqual(2, pop[1][1].Length); + ClassicAssert.AreEqual("two", pop[1][1][0].ToString()); + ClassicAssert.AreEqual(2, (int)(RedisValue)pop[1][1][1]); + } + [Test] [TestCase(1, "key1", "value1", 1.5, Description = "First key has minimum value")] [TestCase(2, "key2", "value2", 2.5, Description = "Second key has minimum value")]