From cd32c88608bd255113acaab1777ae37f1ccff54b Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Thu, 23 May 2024 14:37:05 -0600 Subject: [PATCH] more bugfixes --- .../SortedSetGeo/SortedSetGeoObjectImpl.cs | 1 + libs/server/Resp/BasicCommands.cs | 2 +- .../Resp/Objects/SortedSetGeoCommands.cs | 25 ++++++++++++++++--- test/Garnet.test/RespSortedSetGeoTests.cs | 11 ++++---- test/Garnet.test/RespTests.cs | 2 +- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/libs/server/Objects/SortedSetGeo/SortedSetGeoObjectImpl.cs b/libs/server/Objects/SortedSetGeo/SortedSetGeoObjectImpl.cs index ce4ceec7d6..24dcc3ce2c 100644 --- a/libs/server/Objects/SortedSetGeo/SortedSetGeoObjectImpl.cs +++ b/libs/server/Objects/SortedSetGeo/SortedSetGeoObjectImpl.cs @@ -110,6 +110,7 @@ private void GeoAdd(byte* input, int length, byte* output) _output->opsDone++; this.UpdateSize(member); + elementsChanged++; } } else if (!nx && scoreStored != score) diff --git a/libs/server/Resp/BasicCommands.cs b/libs/server/Resp/BasicCommands.cs index d05d9cef14..8575dffe83 100644 --- a/libs/server/Resp/BasicCommands.cs +++ b/libs/server/Resp/BasicCommands.cs @@ -362,7 +362,7 @@ private bool NetworkGetRange(byte* ptr, ref TGarnetApi storageApi) { sessionMetrics?.incr_total_notfound(); Debug.Assert(o.IsSpanByte); - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_EMPTY, ref dcurr, dend)) SendAndReset(); } diff --git a/libs/server/Resp/Objects/SortedSetGeoCommands.cs b/libs/server/Resp/Objects/SortedSetGeoCommands.cs index 9638eccfb2..f6d35ef346 100644 --- a/libs/server/Resp/Objects/SortedSetGeoCommands.cs +++ b/libs/server/Resp/Objects/SortedSetGeoCommands.cs @@ -101,13 +101,11 @@ private unsafe bool GeoCommands(int count, byte* ptr, SortedSetOpera { int paramsRequiredInCommand = 0; string cmd = string.Empty; - var responseWhenNotFound = CmdStrings.RESP_EMPTYLIST; switch (op) { case SortedSetOperation.GEODIST: paramsRequiredInCommand = 3; cmd = "GEODIST"; - responseWhenNotFound = CmdStrings.RESP_ERRNOTFOUND; break; case SortedSetOperation.GEOHASH: paramsRequiredInCommand = 1; @@ -181,8 +179,27 @@ private unsafe bool GeoCommands(int count, byte* ptr, SortedSetOpera ptr += objOutputHeader.bytesDone; break; case GarnetStatus.NOTFOUND: - while (!RespWriteUtils.WriteDirect(responseWhenNotFound, ref dcurr, dend)) - SendAndReset(); + var tokens = ReadLeftToken(inputCount, ref ptr); + if (tokens < inputCount) + return false; + + switch (op) + { + case SortedSetOperation.GEODIST: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) + SendAndReset(); + break; + default: + while (!RespWriteUtils.WriteArrayLength(inputCount, ref dcurr, dend)) + SendAndReset(); + for (var i = 0; i < inputCount; i++) + { + while (!RespWriteUtils.WriteNullArray(ref dcurr, dend)) + SendAndReset(); + } + break; + } + break; } } diff --git a/test/Garnet.test/RespSortedSetGeoTests.cs b/test/Garnet.test/RespSortedSetGeoTests.cs index 56bea8d9b6..010e6129f6 100644 --- a/test/Garnet.test/RespSortedSetGeoTests.cs +++ b/test/Garnet.test/RespSortedSetGeoTests.cs @@ -289,12 +289,6 @@ public void CanDoGeoAddWhenInvalidPairLC(int bytesSent) var actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); - // Check GEOADD with insufficient parameters - //response = lightClientRequest.SendCommandChunks("GEOADD Sicily NX CH 13.361389 38.115556", bytesSent); - //expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR)}\r\n"; - //actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); - //Assert.AreEqual(expectedResponse, actualValue); - response = lightClientRequest.SendCommandChunks("GEOADD Sicily NX 13.361389 38.115556 Palermo 15.087269 37.502669 Catania", bytesSent); expectedResponse = ":2\r\n"; actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); @@ -433,6 +427,11 @@ public void CanUseGeoPosLC(int bytesSent) var response = lightClientRequest.Execute("GEOADD Sicily 13.361389 38.115556 Palermo 15.087269 37.502669 Catania", "PING", expectedResponse.Length, bytesSent); Assert.AreEqual(expectedResponse, response); + // GEOPOS with unknown key + response = lightClientRequest.Execute("GEOPOS Unknown Palermo Catania", expectedResponse.Length, bytesSent); + expectedResponse = "*2\r\n*-1\r\n*-1\r\n"; + Assert.AreEqual(expectedResponse, response); + expectedResponse = "*3\r\n*2\r\n$18\r\n13.361389338970184\r\n$17\r\n38.11555668711662\r\n*2\r\n$18\r\n15.087267458438873\r\n$18\r\n37.502669245004654\r\n*-1\r\n+PONG\r\n"; response = lightClientRequest.Execute("GEOPOS Sicily Palermo Catania Unknown", "PING", expectedResponse.Length, bytesSent); Assert.AreEqual(expectedResponse, response); diff --git a/test/Garnet.test/RespTests.cs b/test/Garnet.test/RespTests.cs index dc49d4ba62..95647cd1da 100644 --- a/test/Garnet.test/RespTests.cs +++ b/test/Garnet.test/RespTests.cs @@ -1523,7 +1523,7 @@ public void GetSliceTest() string value = "0123456789"; var resp = (string)db.StringGetRange(key, 2, 10); - Assert.AreEqual(null, resp); + Assert.AreEqual(string.Empty, resp); Assert.AreEqual(true, db.StringSet(key, value)); //0,0