From 8f46a57aefb6c7cde85570b48453e001d07af87b Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Wed, 12 Jun 2024 15:03:19 -0700 Subject: [PATCH] Adding type validation logic to object operations (#450) * Initial support for WRONGTYPE * nit * nit * Added wrong type checks for list & set objects * Added hash commands * Added sorted set commands * Fixing some tests * Some fixes * Added sorted set tests * Adding geo commands + custom object commands * Moving helper methods to RespTestsUtils * Fixing comment --------- Co-authored-by: Badrish Chandramouli --- libs/server/API/GarnetApiObjectCommands.cs | 2 +- libs/server/API/GarnetStatus.cs | 6 +- libs/server/API/IGarnetApi.cs | 4 +- libs/server/Custom/CustomObjectBase.cs | 6 + libs/server/Custom/CustomRespCommands.cs | 47 ++- libs/server/Objects/List/ListObject.cs | 2 +- libs/server/Objects/Set/SetObject.cs | 10 +- .../Objects/SortedSet/SortedSetObject.cs | 14 +- libs/server/Resp/CmdStrings.cs | 2 + libs/server/Resp/Objects/HashCommands.cs | 141 ++++++-- libs/server/Resp/Objects/ListCommands.cs | 280 +++++++++----- libs/server/Resp/Objects/SetCommands.cs | 288 ++++++++++----- .../Resp/Objects/SharedObjectCommands.cs | 13 +- libs/server/Resp/Objects/SortedSetCommands.cs | 341 +++++++++++------- .../Resp/Objects/SortedSetGeoCommands.cs | 46 ++- .../Session/ObjectStore/AdvancedOps.cs | 18 +- .../Storage/Session/ObjectStore/Common.cs | 12 +- .../Storage/Session/ObjectStore/HashOps.cs | 14 +- .../Storage/Session/ObjectStore/ListOps.cs | 56 +-- .../Storage/Session/ObjectStore/SetOps.cs | 224 +++++++----- .../Session/ObjectStore/SortedSetOps.cs | 63 ++-- main/GarnetServer/Extensions/MyDictObject.cs | 3 +- test/Garnet.test/RespCustomCommandTests.cs | 30 ++ test/Garnet.test/RespHashTests.cs | 57 +++ test/Garnet.test/RespListTests.cs | 51 ++- test/Garnet.test/RespSetTest.cs | 46 +++ test/Garnet.test/RespSortedSetGeoTests.cs | 37 ++ test/Garnet.test/RespSortedSetTests.cs | 114 +++++- test/Garnet.test/RespTests.cs | 32 ++ test/Garnet.test/RespTestsUtils.cs | 68 ++++ test/Garnet.test/TestProcedureLists.cs | 4 +- 31 files changed, 1482 insertions(+), 549 deletions(-) create mode 100644 test/Garnet.test/RespTestsUtils.cs diff --git a/libs/server/API/GarnetApiObjectCommands.cs b/libs/server/API/GarnetApiObjectCommands.cs index 6ab965bd2d..ba643acdde 100644 --- a/libs/server/API/GarnetApiObjectCommands.cs +++ b/libs/server/API/GarnetApiObjectCommands.cs @@ -213,7 +213,7 @@ public GarnetStatus ListLength(byte[] key, ArgSlice input, out ObjectOutputHeade => storageSession.ListLength(key, input, out output, ref objectContext); /// - public bool ListMove(ArgSlice source, ArgSlice destination, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element) + public GarnetStatus ListMove(ArgSlice source, ArgSlice destination, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element) => storageSession.ListMove(source, destination, sourceDirection, destinationDirection, out element); /// diff --git a/libs/server/API/GarnetStatus.cs b/libs/server/API/GarnetStatus.cs index 8045697e6d..2277461ad4 100644 --- a/libs/server/API/GarnetStatus.cs +++ b/libs/server/API/GarnetStatus.cs @@ -19,6 +19,10 @@ public enum GarnetStatus : byte /// /// Moved /// - MOVED + MOVED, + /// + /// Wrong type + /// + WRONGTYPE } } \ No newline at end of file diff --git a/libs/server/API/IGarnetApi.cs b/libs/server/API/IGarnetApi.cs index 756715205b..cc1e145628 100644 --- a/libs/server/API/IGarnetApi.cs +++ b/libs/server/API/IGarnetApi.cs @@ -723,8 +723,8 @@ public interface IGarnetApi : IGarnetReadApi, IGarnetAdvancedApi /// /// /// The element being popped and pushed - /// true when success - public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element); + /// GarnetStatus + public GarnetStatus ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element); /// /// Trim an existing list so it only contains the specified range of elements. diff --git a/libs/server/Custom/CustomObjectBase.cs b/libs/server/Custom/CustomObjectBase.cs index ab934d7139..896bd85089 100644 --- a/libs/server/Custom/CustomObjectBase.cs +++ b/libs/server/Custom/CustomObjectBase.cs @@ -208,6 +208,12 @@ public sealed override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMe } break; default: + if ((byte)header->type != this.type) + { + // Indicates an incorrect type of key + output.Length = 0; + return true; + } (IMemoryOwner Memory, int Length) outp = (output.Memory, 0); Operate(header->SubId, input.AsReadOnlySpan().Slice(RespInputHeader.Size), ref outp, out removeKey); output.Memory = outp.Memory; diff --git a/libs/server/Custom/CustomRespCommands.cs b/libs/server/Custom/CustomRespCommands.cs index b3f6f686f1..5e0c4f3499 100644 --- a/libs/server/Custom/CustomRespCommands.cs +++ b/libs/server/Custom/CustomRespCommands.cs @@ -165,31 +165,44 @@ private bool TryCustomObjectCommand(byte* ptr, byte* end, RespComman status = storageApi.RMW_ObjectStore(ref key, ref Unsafe.AsRef(inputPtr), ref output); Debug.Assert(!output.spanByteAndMemory.IsSpanByte); - if (output.spanByteAndMemory.Memory != null) - SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length); - else - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) - SendAndReset(); + switch (status) + { + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + if (output.spanByteAndMemory.Memory != null) + SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length); + else + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) + SendAndReset(); + break; + } } else { status = storageApi.Read_ObjectStore(ref key, ref Unsafe.AsRef(inputPtr), ref output); Debug.Assert(!output.spanByteAndMemory.IsSpanByte); - if (status == GarnetStatus.OK) + switch (status) { - if (output.spanByteAndMemory.Memory != null) - SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length); - else - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) + case GarnetStatus.OK: + if (output.spanByteAndMemory.Memory != null) + SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length); + else + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.NOTFOUND: + Debug.Assert(output.spanByteAndMemory.Memory == null); + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) SendAndReset(); - - } - else - { - Debug.Assert(output.spanByteAndMemory.Memory == null); - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) - SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } diff --git a/libs/server/Objects/List/ListObject.cs b/libs/server/Objects/List/ListObject.cs index d50823322a..aab0ef0d18 100644 --- a/libs/server/Objects/List/ListObject.cs +++ b/libs/server/Objects/List/ListObject.cs @@ -136,7 +136,7 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou var header = (RespInputHeader*)_input; if (header->type != GarnetObjectType.List) { - //Indicates an incorrect type of key + // Indicates an incorrect type of key output.Length = 0; sizeChange = 0; return true; diff --git a/libs/server/Objects/Set/SetObject.cs b/libs/server/Objects/Set/SetObject.cs index d7db65c3d6..333efb60db 100644 --- a/libs/server/Objects/Set/SetObject.cs +++ b/libs/server/Objects/Set/SetObject.cs @@ -110,7 +110,15 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou fixed (byte* _output = output.SpanByte.AsSpan()) { var header = (RespInputHeader*)_input; - Debug.Assert(header->type == GarnetObjectType.Set); + if (header->type != GarnetObjectType.Set) + { + // Indicates an incorrect type of key + output.Length = 0; + sizeChange = 0; + removeKey = false; + return true; + } + long prevSize = this.Size; switch (header->SetOp) { diff --git a/libs/server/Objects/SortedSet/SortedSetObject.cs b/libs/server/Objects/SortedSet/SortedSetObject.cs index 49c3aef50d..6e283b82bb 100644 --- a/libs/server/Objects/SortedSet/SortedSetObject.cs +++ b/libs/server/Objects/SortedSet/SortedSetObject.cs @@ -183,8 +183,16 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou fixed (byte* _output = output.SpanByte.AsSpan()) { var header = (RespInputHeader*)_input; - Debug.Assert(header->type == GarnetObjectType.SortedSet); - long previouseSize = this.Size; + if (header->type != GarnetObjectType.SortedSet) + { + // Indicates an incorrect type of key + output.Length = 0; + sizeChange = 0; + removeKey = false; + return true; + } + + long prevSize = this.Size; switch (header->SortedSetOp) { case SortedSetOperation.ZADD: @@ -269,7 +277,7 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou default: throw new GarnetException($"Unsupported operation {(SortedSetOperation)_input[0]} in SortedSetObject.Operate"); } - sizeChange = this.Size - previouseSize; + sizeChange = this.Size - prevSize; } removeKey = sortedSetDict.Count == 0; diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index 6f468340db..048fc3b1f4 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -142,6 +142,8 @@ static partial class CmdStrings public static ReadOnlySpan RESP_ERR_UNSUPPORTED_PROTOCOL_VERSION => "ERR Unsupported protocol version"u8; public static ReadOnlySpan RESP_ERR_ASYNC_PROTOCOL_CHANGE => "ERR protocol change is not allowed with pending async operations"u8; public static ReadOnlySpan RESP_ERR_NOT_VALID_FLOAT => "ERR value is not a valid float"u8; + public static ReadOnlySpan RESP_ERR_MIN_MAX_NOT_VALID_FLOAT => "ERR min or max is not a float"u8; + public static ReadOnlySpan RESP_ERR_MIN_MAX_NOT_VALID_STRING => "ERR min or max not valid string range item"u8; public static ReadOnlySpan RESP_WRONGPASS_INVALID_PASSWORD => "WRONGPASS Invalid password"u8; public static ReadOnlySpan RESP_WRONGPASS_INVALID_USERNAME_PASSWORD => "WRONGPASS Invalid username/password combination"u8; public static ReadOnlySpan RESP_SYNTAX_ERROR => "ERR syntax error"u8; diff --git a/libs/server/Resp/Objects/HashCommands.cs b/libs/server/Resp/Objects/HashCommands.cs index 68e36955e0..fae7655dc0 100644 --- a/libs/server/Resp/Objects/HashCommands.cs +++ b/libs/server/Resp/Objects/HashCommands.cs @@ -81,33 +81,45 @@ private unsafe bool HashSet(RespCommand command, int count, byte* pt inputPtr->count = inputCount; inputPtr->done = hashOpsCount; - storageApi.HashSet(key, new ArgSlice((byte*)inputPtr, inputLength), out ObjectOutputHeader output); + var status = storageApi.HashSet(key, new ArgSlice((byte*)inputPtr, inputLength), out ObjectOutputHeader output); *inputPtr = save; // reset input buffer - hashItemsDoneCount += output.countDone; - hashOpsCount += output.opsDone; + switch (status) + { + case GarnetStatus.WRONGTYPE: + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; - // Reset buffer and return if HSET did not process the entire command tokens - if (hashItemsDoneCount < inputCount) - return false; + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + hashItemsDoneCount += output.countDone; + hashOpsCount += output.opsDone; - // Move head, write result to output, reset session counters - ptr += output.bytesDone; - readHead = (int)(ptr - recvBufferPtr); + // Reset buffer and return if HSET did not process the entire command tokens + if (hashItemsDoneCount < inputCount) + return false; - if (command == RespCommand.HMSET) - { - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) - SendAndReset(); - } - else - { - while (!RespWriteUtils.WriteInteger(hashOpsCount, ref dcurr, dend)) - SendAndReset(); + // Move head, write result to output, reset session counters + ptr += output.bytesDone; + if (command == RespCommand.HMSET) + { + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) + SendAndReset(); + } + else + { + while (!RespWriteUtils.WriteInteger(hashOpsCount, ref dcurr, dend)) + SendAndReset(); + } + break; } } + readHead = (int)(ptr - recvBufferPtr); hashItemsDoneCount = hashOpsCount = 0; return true; } @@ -176,7 +188,7 @@ private unsafe bool HashGet(RespCommand command, int count, byte* pt // Prepare GarnetObjectStore output var outputFooter = new GarnetObjectStoreOutput { spanByteAndMemory = new SpanByteAndMemory(dcurr, (int)(dend - dcurr)) }; - var status = GarnetStatus.NOTFOUND; + GarnetStatus status; var includeCountParameter = false; if (command == RespCommand.HRANDFIELD) @@ -190,6 +202,13 @@ private unsafe bool HashGet(RespCommand command, int count, byte* pt // Reset input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -215,7 +234,10 @@ private unsafe bool HashGet(RespCommand command, int count, byte* pt while (!RespWriteUtils.WriteDirect(respBytes, ref dcurr, dend)) SendAndReset(); } - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } } @@ -290,6 +312,10 @@ private unsafe bool HashLength(int count, byte* ptr, ref TGarnetApi while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -348,6 +374,13 @@ private unsafe bool HashStrLength(int count, byte* ptr, ref TGarnetA // Restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -359,7 +392,10 @@ private unsafe bool HashStrLength(int count, byte* ptr, ref TGarnetA case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } } @@ -422,6 +458,13 @@ private unsafe bool HashDelete(int count, byte* ptr, ref TGarnetApi // Restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -437,8 +480,10 @@ private unsafe bool HashDelete(int count, byte* ptr, ref TGarnetApi case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); - hashItemsDoneCount = hashOpsCount = 0; - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } } @@ -499,6 +544,13 @@ private unsafe bool HashExists(int count, byte* ptr, ref TGarnetApi // Restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -510,7 +562,10 @@ private unsafe bool HashExists(int count, byte* ptr, ref TGarnetApi case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } } @@ -586,6 +641,13 @@ private unsafe bool HashKeys(RespCommand command, int count, byte* p // Restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -601,7 +663,10 @@ private unsafe bool HashKeys(RespCommand command, int count, byte* p case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } @@ -676,15 +741,27 @@ private unsafe bool HashIncrement(RespCommand command, int count, by // Restore input *inputPtr = save; - // Process output - var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); - if (objOutputHeader.opsDone == Int32.MinValue) + switch (status) { - // Command was partially done - return false; - } - ptr += objOutputHeader.bytesDone; + case GarnetStatus.WRONGTYPE: + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + // Process output + var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); + if (objOutputHeader.opsDone == int.MinValue) + { + // Command was partially done + return false; + } + ptr += objOutputHeader.bytesDone; + break; + } } // Reset counters hashItemsDoneCount = hashOpsCount = 0; diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index af3eec1db6..6d2329320f 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -92,7 +92,7 @@ private unsafe bool ListPush(RespCommand command, int count, byte* p listItemsDoneCount += output.countDone; listOpsCount += output.opsDone; - //return if command is only partially done + // Return if command is only partially done if (output.countDone == Int32.MinValue && listOpsCount < inputCount) return false; @@ -103,11 +103,19 @@ private unsafe bool ListPush(RespCommand command, int count, byte* p if (tokens < count - 1) return false; - //write result to output - while (!RespWriteUtils.WriteInteger(listItemsDoneCount, ref dcurr, dend)) - SendAndReset(); + if (status == GarnetStatus.WRONGTYPE) + { + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + } + else + { + // Write result to output + while (!RespWriteUtils.WriteInteger(listItemsDoneCount, ref dcurr, dend)) + SendAndReset(); + } - //reset session counters + // Reset session counters listItemsDoneCount = listOpsCount = 0; // Move head @@ -198,6 +206,10 @@ private unsafe bool ListPop(RespCommand command, int count, byte* pt while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Move input head @@ -214,7 +226,7 @@ private unsafe bool ListPop(RespCommand command, int count, byte* pt /// /// /// - private unsafe bool ListLength(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListLength(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { if (count != 1) @@ -255,16 +267,21 @@ private unsafe bool ListLength(int count, byte* ptr, ref TGarnetApi //restore input buffer *inputPtr = save; - if (status == GarnetStatus.NOTFOUND) + switch (status) { - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) - SendAndReset(); - } - else - { - // Process output - while (!RespWriteUtils.WriteInteger(output.countDone, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.NOTFOUND: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + // Process output + while (!RespWriteUtils.WriteInteger(output.countDone, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -283,7 +300,7 @@ private unsafe bool ListLength(int count, byte* ptr, ref TGarnetApi /// /// /// - private unsafe bool ListTrim(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListTrim(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { if (count != 3) @@ -327,15 +344,24 @@ private unsafe bool ListTrim(int count, byte* ptr, ref TGarnetApi st inputPtr->count = start; inputPtr->done = stop; - var statusOp = storageApi.ListTrim(key, new ArgSlice((byte*)inputPtr, inputLength)); + var status = storageApi.ListTrim(key, new ArgSlice((byte*)inputPtr, inputLength)); //restore input buffer *inputPtr = save; - //GarnetStatus.OK or NOTFOUND have same result - // no need to process output, just send OK - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) - SendAndReset(); + switch (status) + { + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + //GarnetStatus.OK or NOTFOUND have same result + // no need to process output, just send OK + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend)) + SendAndReset(); + break; + } } // Move input head, write result to output readHead = (int)(ptr - recvBufferPtr); @@ -351,7 +377,7 @@ private unsafe bool ListTrim(int count, byte* ptr, ref TGarnetApi st /// /// /// - private unsafe bool ListRange(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListRange(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { if (count != 3) @@ -411,6 +437,10 @@ private unsafe bool ListRange(int count, byte* ptr, ref TGarnetApi s while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_EMPTYLIST, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } // Move input head, write result to output @@ -427,7 +457,7 @@ private unsafe bool ListRange(int count, byte* ptr, ref TGarnetApi s /// /// /// - private unsafe bool ListIndex(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListIndex(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { if (count != 2) @@ -474,7 +504,7 @@ private unsafe bool ListIndex(int count, byte* ptr, ref TGarnetApi s //restore input *inputPtr = save; - var error = CmdStrings.RESP_ERRNOTFOUND; + ReadOnlySpan error = default; switch (statusOp) { @@ -482,8 +512,15 @@ private unsafe bool ListIndex(int count, byte* ptr, ref TGarnetApi s //process output var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); ptr += objOutputHeader.bytesDone; - if (objOutputHeader.opsDone != -1) - error = default; + if (objOutputHeader.opsDone == -1) + error = CmdStrings.RESP_ERRNOTFOUND; + break; + case GarnetStatus.NOTFOUND: + error = CmdStrings.RESP_ERRNOTFOUND; + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } @@ -508,7 +545,7 @@ private unsafe bool ListIndex(int count, byte* ptr, ref TGarnetApi s /// /// /// - private unsafe bool ListInsert(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListInsert(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { if (count != 4) @@ -549,15 +586,16 @@ private unsafe bool ListInsert(int count, byte* ptr, ref TGarnetApi //restore input buffer *inputPtr = save; + if (statusOp != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (statusOp) { case GarnetStatus.OK: - //TODO: validation for different object type, pending to review - if (output.countDone == 0 && output.countDone == 0 && output.bytesDone == 0) - { - while (!RespWriteUtils.WriteError("ERR wrong key type used in LINSERT command."u8, ref dcurr, dend)) - SendAndReset(); - } //check for partial execution if (output.countDone == int.MinValue) return false; @@ -567,12 +605,13 @@ private unsafe bool ListInsert(int count, byte* ptr, ref TGarnetApi SendAndReset(); break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - return false; while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -589,7 +628,7 @@ private unsafe bool ListInsert(int count, byte* ptr, ref TGarnetApi /// /// /// - private unsafe bool ListRemove(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListRemove(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { // if params are missing return error @@ -634,6 +673,13 @@ private unsafe bool ListRemove(int count, byte* ptr, ref TGarnetApi //restore input buffer *inputPtr = save; + if (statusOp != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 2, ref ptr); + if (tokens < count - 2) + return false; + } + switch (statusOp) { case GarnetStatus.OK: @@ -646,12 +692,13 @@ private unsafe bool ListRemove(int count, byte* ptr, ref TGarnetApi SendAndReset(); break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(count - 2, ref ptr); - if (tokens < count - 2) - return false; while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } // Move input head, write result to output @@ -668,54 +715,64 @@ private unsafe bool ListRemove(int count, byte* ptr, ref TGarnetApi /// /// /// - private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListMove(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { - bool result = false; - if (count != 4) { return AbortWithWrongNumberOfArguments("LMOVE", count); } - else - { - ArgSlice sourceKey = default, destinationKey = default, param1 = default, param2 = default; - if (!RespReadUtils.ReadPtrWithLengthHeader(ref sourceKey.ptr, ref sourceKey.length, ref ptr, recvBufferPtr + bytesRead)) - return false; + ArgSlice sourceKey = default, destinationKey = default, param1 = default, param2 = default; - if (!RespReadUtils.ReadPtrWithLengthHeader(ref destinationKey.ptr, ref destinationKey.length, ref ptr, recvBufferPtr + bytesRead)) - return false; + if (!RespReadUtils.ReadPtrWithLengthHeader(ref sourceKey.ptr, ref sourceKey.length, ref ptr, recvBufferPtr + bytesRead)) + return false; - if (!RespReadUtils.ReadPtrWithLengthHeader(ref param1.ptr, ref param1.length, ref ptr, recvBufferPtr + bytesRead)) - return false; + if (!RespReadUtils.ReadPtrWithLengthHeader(ref destinationKey.ptr, ref destinationKey.length, ref ptr, recvBufferPtr + bytesRead)) + return false; - if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead)) - return false; + if (!RespReadUtils.ReadPtrWithLengthHeader(ref param1.ptr, ref param1.length, ref ptr, recvBufferPtr + bytesRead)) + return false; - OperationDirection sourceDirection = GetOperationDirection(param1); - OperationDirection destinationDirection = GetOperationDirection(param2); - if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) - { - return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); - } + if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead)) + return false; - result = ListMove(count, sourceKey, destinationKey, sourceDirection, destinationDirection, out var node, ref storageApi); - if (node != null) - { - while (!RespWriteUtils.WriteBulkString(node, ref dcurr, dend)) - SendAndReset(); - } - else - { - while (!RespWriteUtils.WriteNull(ref dcurr, dend)) + var sourceDirection = GetOperationDirection(param1); + var destinationDirection = GetOperationDirection(param2); + + if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) + { + return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); + } + + if (!ListMove(count, sourceKey, destinationKey, sourceDirection, destinationDirection, out var node, + ref storageApi, out var garnetStatus)) + return false; + + switch (garnetStatus) + { + case GarnetStatus.OK: + if (node != null) + { + while (!RespWriteUtils.WriteBulkString(node, ref dcurr, dend)) + SendAndReset(); + } + else + { + while (!RespWriteUtils.WriteNull(ref dcurr, dend)) + SendAndReset(); + } + + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) SendAndReset(); - } + break; } // Move input head, write result to output readHead = (int)(ptr - recvBufferPtr); - return result; + return true; } /// @@ -725,42 +782,50 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st /// /// /// - private unsafe bool ListRightPopLeftPush(int count, byte* ptr, ref TGarnetApi storageApi) + private bool ListRightPopLeftPush(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { - bool result = false; - if (count != 2) { return AbortWithWrongNumberOfArguments("RPOPLPUSH", count); } - else - { - ArgSlice sourceKey = default, destinationKey = default; - if (!RespReadUtils.ReadPtrWithLengthHeader(ref sourceKey.ptr, ref sourceKey.length, ref ptr, recvBufferPtr + bytesRead)) - return false; + ArgSlice sourceKey = default, destinationKey = default; - if (!RespReadUtils.ReadPtrWithLengthHeader(ref destinationKey.ptr, ref destinationKey.length, ref ptr, recvBufferPtr + bytesRead)) - return false; + if (!RespReadUtils.ReadPtrWithLengthHeader(ref sourceKey.ptr, ref sourceKey.length, ref ptr, recvBufferPtr + bytesRead)) + return false; - result = ListMove(count, sourceKey, destinationKey, OperationDirection.Right, OperationDirection.Left, out var node, ref storageApi); + if (!RespReadUtils.ReadPtrWithLengthHeader(ref destinationKey.ptr, ref destinationKey.length, ref ptr, recvBufferPtr + bytesRead)) + return false; - if (node != null) - { - while (!RespWriteUtils.WriteBulkString(node, ref dcurr, dend)) - SendAndReset(); - } - else - { - while (!RespWriteUtils.WriteNull(ref dcurr, dend)) + if (!ListMove(count, sourceKey, destinationKey, OperationDirection.Right, OperationDirection.Left, + out var node, ref storageApi, out var garnetStatus)) + return false; + + switch (garnetStatus) + { + case GarnetStatus.OK: + if (node != null) + { + while (!RespWriteUtils.WriteBulkString(node, ref dcurr, dend)) + SendAndReset(); + } + else + { + while (!RespWriteUtils.WriteNull(ref dcurr, dend)) + SendAndReset(); + } + + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) SendAndReset(); - } + break; } // update read pointers readHead = (int)(ptr - recvBufferPtr); - return result; + return true; } /// @@ -774,12 +839,17 @@ private unsafe bool ListRightPopLeftPush(int count, byte* ptr, ref T /// /// /// + /// /// - private unsafe bool ListMove(int count, ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] node, ref TGarnetApi storageApi) - where TGarnetApi : IGarnetApi + private bool ListMove(int count, ArgSlice sourceKey, ArgSlice destinationKey, + OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] node, + ref TGarnetApi storageApi, out GarnetStatus garnetStatus) + where TGarnetApi : IGarnetApi { - ArgSlice[] keys = new ArgSlice[2] { sourceKey, destinationKey }; + garnetStatus = GarnetStatus.OK; + var keys = new[] { sourceKey, destinationKey }; node = null; + if (NetworkKeyArraySlotVerify(ref keys, false)) { // check for non crosslot error @@ -790,7 +860,9 @@ private unsafe bool ListMove(int count, ArgSlice sourceKey, ArgSlice return true; } - return storageApi.ListMove(sourceKey, destinationKey, sourceDirection, destinationDirection, out node); + garnetStatus = + storageApi.ListMove(sourceKey, destinationKey, sourceDirection, destinationDirection, out node); + return true; } /// @@ -802,7 +874,7 @@ private unsafe bool ListMove(int count, ArgSlice sourceKey, ArgSlice /// /// /// - public unsafe bool ListSet(int count, byte* ptr, ref TGarnetApi storageApi) + public bool ListSet(int count, byte* ptr, ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { if (count != 3) @@ -845,6 +917,13 @@ public unsafe bool ListSet(int count, byte* ptr, ref TGarnetApi stor //restore input *inputPtr = save; + if (statusOp != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (statusOp) { case GarnetStatus.OK: @@ -853,12 +932,13 @@ public unsafe bool ListSet(int count, byte* ptr, ref TGarnetApi stor ptr += objOutputHeader.bytesDone; break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - return false; while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_NOSUCHKEY, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } diff --git a/libs/server/Resp/Objects/SetCommands.cs b/libs/server/Resp/Objects/SetCommands.cs index c0f9d3fb26..36f237776f 100644 --- a/libs/server/Resp/Objects/SetCommands.cs +++ b/libs/server/Resp/Objects/SetCommands.cs @@ -69,26 +69,39 @@ private unsafe bool SetAdd(int count, byte* ptr, ref TGarnetApi stor inputPtr->count = inputCount; inputPtr->done = setOpsCount; - storageApi.SetAdd(key, new ArgSlice((byte*)inputPtr, inputLength), out ObjectOutputHeader output); + var status = storageApi.SetAdd(key, new ArgSlice((byte*)inputPtr, inputLength), out ObjectOutputHeader output); // Restore input buffer *inputPtr = save; - setItemsDoneCount += output.countDone; - setOpsCount += output.opsDone; + switch (status) + { + case GarnetStatus.WRONGTYPE: + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; - // Reset buffer and return if SADD is only partially done - if (setOpsCount < inputCount) - return false; + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + setItemsDoneCount += output.countDone; + setOpsCount += output.opsDone; + + // Reset buffer and return if SADD is only partially done + if (setOpsCount < inputCount) + return false; - // Move head, write result to output, reset session counters - ptr += output.bytesDone; - readHead = (int)(ptr - recvBufferPtr); + // Move head, write result to output, reset session counters + ptr += output.bytesDone; - while (!RespWriteUtils.WriteInteger(setItemsDoneCount, ref dcurr, dend)) - SendAndReset(); + while (!RespWriteUtils.WriteInteger(setItemsDoneCount, ref dcurr, dend)) + SendAndReset(); + break; + } } + readHead = (int)(ptr - recvBufferPtr); setItemsDoneCount = setOpsCount = 0; return true; } @@ -128,27 +141,34 @@ private bool SetIntersect(int count, byte* ptr, ref TGarnetApi stora var status = storageApi.SetIntersect(keys, out var result); - if (status == GarnetStatus.OK) + switch (status) { - // write the size of result - int resultCount = 0; - if (result != null) - { - resultCount = result.Count; - while (!RespWriteUtils.WriteArrayLength(resultCount, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + // write the size of result + int resultCount = 0; + if (result != null) + { + resultCount = result.Count; + while (!RespWriteUtils.WriteArrayLength(resultCount, ref dcurr, dend)) + SendAndReset(); - foreach (var item in result) + foreach (var item in result) + { + while (!RespWriteUtils.WriteBulkString(item, ref dcurr, dend)) + SendAndReset(); + } + } + else { - while (!RespWriteUtils.WriteBulkString(item, ref dcurr, dend)) + while (!RespWriteUtils.WriteArrayLength(resultCount, ref dcurr, dend)) SendAndReset(); } - } - else - { - while (!RespWriteUtils.WriteArrayLength(resultCount, ref dcurr, dend)) + + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) SendAndReset(); - } + break; } // update read pointers @@ -200,10 +220,16 @@ private bool SetIntersectStore(int count, byte* ptr, ref TGarnetApi var status = storageApi.SetIntersectStore(key.ToArray(), keys, out var output); - if (status == GarnetStatus.OK) + switch (status) { - while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Move input head @@ -246,17 +272,26 @@ private bool SetUnion(int count, byte* ptr, ref TGarnetApi storageAp return true; } - storageApi.SetUnion(keys, out var result); + var status = storageApi.SetUnion(keys, out var result); - // write the size of result - var resultCount = result.Count; - while (!RespWriteUtils.WriteArrayLength(resultCount, ref dcurr, dend)) - SendAndReset(); - - foreach (var item in result) + switch (status) { - while (!RespWriteUtils.WriteBulkString(item, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + // write the size of result + var resultCount = result.Count; + while (!RespWriteUtils.WriteArrayLength(resultCount, ref dcurr, dend)) + SendAndReset(); + + foreach (var item in result) + { + while (!RespWriteUtils.WriteBulkString(item, ref dcurr, dend)) + SendAndReset(); + } + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // update read pointers @@ -308,10 +343,16 @@ private bool SetUnionStore(int count, byte* ptr, ref TGarnetApi stor var status = storageApi.SetUnionStore(key, keys, out var output); - if (status == GarnetStatus.OK) + switch (status) { - while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Move input head @@ -373,28 +414,39 @@ private unsafe bool SetRemove(int count, byte* ptr, ref TGarnetApi s // Restore input buffer *inputPtr = save; - if (status == GarnetStatus.NOTFOUND) + if (status != GarnetStatus.OK) { - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) - SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; } - else + + switch (status) { - setItemsDoneCount += output.countDone; - setOpsCount += output.opsDone; + case GarnetStatus.OK: + setItemsDoneCount += output.countDone; + setOpsCount += output.opsDone; - // Reset buffer and return if command is only partially done - if (setOpsCount < inputCount) - return false; + // Reset buffer and return if command is only partially done + if (setOpsCount < inputCount) + return false; - // Move head, write result to output, reset session counters - ptr += output.bytesDone; + // Move head, write result to output, reset session counters + ptr += output.bytesDone; - while (!RespWriteUtils.WriteInteger(setItemsDoneCount, ref dcurr, dend)) - SendAndReset(); + while (!RespWriteUtils.WriteInteger(setItemsDoneCount, ref dcurr, dend)) + SendAndReset(); - setOpsCount = setItemsDoneCount = 0; + setOpsCount = setItemsDoneCount = 0; + break; + case GarnetStatus.NOTFOUND: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -453,16 +505,21 @@ private unsafe bool SetLength(int count, byte* ptr, ref TGarnetApi s // Restore input buffer *inputPtr = save; - if (status == GarnetStatus.NOTFOUND) - { - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) - SendAndReset(); - } - else + switch (status) { - // Process output - while (!RespWriteUtils.WriteInteger(output.countDone, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + // Process output + while (!RespWriteUtils.WriteInteger(output.countDone, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.NOTFOUND: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } // Move input head @@ -523,6 +580,13 @@ private unsafe bool SetMembers(int count, byte* ptr, ref TGarnetApi // Restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -536,7 +600,10 @@ private unsafe bool SetMembers(int count, byte* ptr, ref TGarnetApi case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } } @@ -593,6 +660,13 @@ private unsafe bool SetIsMember(int count, byte* ptr, ref TGarnetApi // Restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -606,7 +680,10 @@ private unsafe bool SetIsMember(int count, byte* ptr, ref TGarnetApi case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } @@ -671,7 +748,7 @@ private unsafe bool SetPop(int count, byte* ptr, ref TGarnetApi stor // Prepare response if (!NumUtils.TryParse(countParameterBytes, out countParameter) || countParameter < 0) { - while (!RespWriteUtils.WriteError("ERR value is not an integer or out of range"u8, ref dcurr, dend)) + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_VALUE_IS_NOT_INTEGER, ref dcurr, dend)) SendAndReset(); // Restore input buffer @@ -720,6 +797,10 @@ private unsafe bool SetPop(int count, byte* ptr, ref TGarnetApi stor while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Reset session counters @@ -775,15 +856,20 @@ private unsafe bool SetMove(int count, byte* ptr, ref TGarnetApi sto var status = storageApi.SetMove(sourceKey, destinationKey, sourceMember, out var output); - if (status == GarnetStatus.NOTFOUND) - { - while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) - SendAndReset(); - } - else + switch (status) { - while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.NOTFOUND: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Reset session counters @@ -851,7 +937,7 @@ private unsafe bool SetRandomMember(int count, byte* ptr, ref TGarne // Prepare response if (!NumUtils.TryParse(countParameterBytes, out countParameter)) { - while (!RespWriteUtils.WriteError("ERR value is not an integer or out of range\r\n"u8, ref dcurr, dend)) + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_VALUE_IS_NOT_INTEGER, ref dcurr, dend)) SendAndReset(); // Restore input buffer @@ -908,6 +994,10 @@ private unsafe bool SetRandomMember(int count, byte* ptr, ref TGarne SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Reset session counters @@ -950,23 +1040,29 @@ private bool SetDiff(int count, byte* ptr, ref TGarnetApi storageApi var status = storageApi.SetDiff(keys, out var output); - if (status == GarnetStatus.OK) + switch (status) { - if (output == null || output.Count == 0) - { - while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) - SendAndReset(); - } - else - { - while (!RespWriteUtils.WriteArrayLength(output.Count, ref dcurr, dend)) - SendAndReset(); - foreach (var item in output) + case GarnetStatus.OK: + if (output == null || output.Count == 0) { - while (!RespWriteUtils.WriteBulkString(item, ref dcurr, dend)) + while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) SendAndReset(); } - } + else + { + while (!RespWriteUtils.WriteArrayLength(output.Count, ref dcurr, dend)) + SendAndReset(); + foreach (var item in output) + { + while (!RespWriteUtils.WriteBulkString(item, ref dcurr, dend)) + SendAndReset(); + } + } + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Move input head @@ -1010,10 +1106,16 @@ private bool SetDiffStore(int count, byte* ptr, ref TGarnetApi stora var status = storageApi.SetDiffStore(key.ToArray(), keys, out var output); - if (status == GarnetStatus.OK) + switch (status) { - while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) - SendAndReset(); + case GarnetStatus.OK: + while (!RespWriteUtils.WriteInteger(output, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // Move input head diff --git a/libs/server/Resp/Objects/SharedObjectCommands.cs b/libs/server/Resp/Objects/SharedObjectCommands.cs index a9c95ce529..da7e651ae1 100644 --- a/libs/server/Resp/Objects/SharedObjectCommands.cs +++ b/libs/server/Resp/Objects/SharedObjectCommands.cs @@ -114,6 +114,13 @@ private unsafe bool ObjectScan(int count, byte* ptr, GarnetObjectTyp *inputPtr = save; *ptrToInt = savePtrToInt; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 2, ref ptr); + if (tokens < count - 2) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -129,8 +136,10 @@ private unsafe bool ObjectScan(int count, byte* ptr, GarnetObjectTyp SendAndReset(); while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) SendAndReset(); - // Fast forward left of the input - ReadLeftToken(count - 2, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } diff --git a/libs/server/Resp/Objects/SortedSetCommands.cs b/libs/server/Resp/Objects/SortedSetCommands.cs index 4a2ea6b111..da67c1e6f7 100644 --- a/libs/server/Resp/Objects/SortedSetCommands.cs +++ b/libs/server/Resp/Objects/SortedSetCommands.cs @@ -76,22 +76,36 @@ private unsafe bool SortedSetAdd(int count, byte* ptr, ref TGarnetAp inputPtr->count = inputCount; inputPtr->done = zaddDoneCount; - storageApi.SortedSetAdd(key, new ArgSlice((byte*)inputPtr, inputLength), out ObjectOutputHeader output); + var status = storageApi.SortedSetAdd(key, new ArgSlice((byte*)inputPtr, inputLength), out ObjectOutputHeader output); // Reset input buffer *inputPtr = save; - zaddDoneCount += output.countDone; - zaddAddCount += output.opsDone; + switch (status) + { + case GarnetStatus.WRONGTYPE: + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; - // Reset buffer and return if command is only partially done - if (zaddDoneCount < inputCount) - return false; - while (!RespWriteUtils.WriteInteger(zaddAddCount, ref dcurr, dend)) - SendAndReset(); + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + zaddDoneCount += output.countDone; + zaddAddCount += output.opsDone; + + // Reset buffer and return if command is only partially done + if (zaddDoneCount < inputCount) + return false; + while (!RespWriteUtils.WriteInteger(zaddAddCount, ref dcurr, dend)) + SendAndReset(); + + // Move head, write result to output, reset session counters + ptr += output.bytesDone; + break; + } - // Move head, write result to output, reset session counters - ptr += output.bytesDone; readHead = (int)(ptr - recvBufferPtr); zaddDoneCount = zaddAddCount = 0; @@ -150,6 +164,14 @@ private unsafe bool SortedSetRemove(int count, byte* ptr, ref TGarne // Reset input buffer *rmwInput = save; + if (status != GarnetStatus.OK) + { + // This checks if we get the whole request, + // Otherwise it needs to return false + if (ReadLeftToken(count - 1, ref ptr) < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -166,13 +188,13 @@ private unsafe bool SortedSetRemove(int count, byte* ptr, ref TGarne SendAndReset(); break; case GarnetStatus.NOTFOUND: - // This checks if we get the whole request, - // Otherwise it needs to return false - if (ReadLeftToken(count - 1, ref ptr) < count - 1) - return false; while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -244,6 +266,10 @@ private unsafe bool SortedSetLength(int count, byte* ptr, ref TGarne while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -266,6 +292,11 @@ private unsafe bool SortedSetRange(RespCommand command, int count, b where TGarnetApi : IGarnetApi { //ZRANGE key min max [BYSCORE|BYLEX] [REV] [LIMIT offset count] [WITHSCORES] + if (count < 3) + { + zaddDoneCount = zaddAddCount = 0; + return AbortWithWrongNumberOfArguments(nameof(RespCommand.ZRANGE), count); + } // Get the key for the Sorted Set if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var key, @@ -278,72 +309,62 @@ private unsafe bool SortedSetRange(RespCommand command, int count, b return true; } - // at least we need 4 args cmd + params - if (count < 3) - { - //reset counters and fast forward the rest of the input - zaddDoneCount = zaddAddCount = 0; - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - { - //command partially executed - return false; - } - else - { - while (!RespWriteUtils.WriteNull(ref dcurr, dend)) - SendAndReset(); - } - } - else - { - // Prepare input - var inputPtr = (ObjectInputHeader*)(ptr - sizeof(ObjectInputHeader)); + // Prepare input + var inputPtr = (ObjectInputHeader*)(ptr - sizeof(ObjectInputHeader)); - // Save old values - var save = *inputPtr; + // Save old values + var save = *inputPtr; - // Prepare length of header in input buffer - var inputLength = (int)(recvBufferPtr + bytesRead - (byte*)inputPtr); + // Prepare length of header in input buffer + var inputLength = (int)(recvBufferPtr + bytesRead - (byte*)inputPtr); - SortedSetOperation op = - command switch - { - RespCommand.ZRANGE => SortedSetOperation.ZRANGE, - RespCommand.ZREVRANGE => SortedSetOperation.ZREVRANGE, - RespCommand.ZRANGEBYSCORE => SortedSetOperation.ZRANGEBYSCORE, - _ => throw new Exception($"Unexpected {nameof(SortedSetOperation)}: {command}") - }; + SortedSetOperation op = + command switch + { + RespCommand.ZRANGE => SortedSetOperation.ZRANGE, + RespCommand.ZREVRANGE => SortedSetOperation.ZREVRANGE, + RespCommand.ZRANGEBYSCORE => SortedSetOperation.ZRANGEBYSCORE, + _ => throw new Exception($"Unexpected {nameof(SortedSetOperation)}: {command}") + }; - // Prepare header in input buffer - inputPtr->header.type = GarnetObjectType.SortedSet; - inputPtr->header.flags = 0; - inputPtr->header.SortedSetOp = op; - inputPtr->count = count - 1; - inputPtr->done = 0; + // Prepare header in input buffer + inputPtr->header.type = GarnetObjectType.SortedSet; + inputPtr->header.flags = 0; + inputPtr->header.SortedSetOp = op; + inputPtr->count = count - 1; + inputPtr->done = 0; - var outputFooter = new GarnetObjectStoreOutput { spanByteAndMemory = new SpanByteAndMemory(dcurr, (int)(dend - dcurr)) }; + var outputFooter = new GarnetObjectStoreOutput { spanByteAndMemory = new SpanByteAndMemory(dcurr, (int)(dend - dcurr)) }; - var status = storageApi.SortedSetRange(key, new ArgSlice((byte*)inputPtr, inputLength), ref outputFooter); + var status = storageApi.SortedSetRange(key, new ArgSlice((byte*)inputPtr, inputLength), ref outputFooter); - // Reset input buffer - *inputPtr = save; + // Reset input buffer + *inputPtr = save; - switch (status) - { - case GarnetStatus.OK: - var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); - ptr += objOutputHeader.bytesDone; - // Return if ZRANGE is only partially done - if (objOutputHeader.bytesDone == 0) - return false; - break; - case GarnetStatus.NOTFOUND: - while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) - SendAndReset(); - ReadLeftToken(count - 1, ref ptr); - break; - } + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + + switch (status) + { + case GarnetStatus.OK: + var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); + ptr += objOutputHeader.bytesDone; + // Return if ZRANGE is only partially done + if (objOutputHeader.bytesDone == 0) + return false; + break; + case GarnetStatus.NOTFOUND: + while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } // reset session counters @@ -424,6 +445,10 @@ private unsafe bool SortedSetScore(int count, byte* ptr, ref TGarnet while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -487,6 +512,13 @@ private unsafe bool SortedSetScores(int count, byte* ptr, ref TGarne //restore input *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -495,14 +527,12 @@ private unsafe bool SortedSetScores(int count, byte* ptr, ref TGarne ptr += objOutputHeader.bytesDone; break; case GarnetStatus.NOTFOUND: - while (!RespWriteUtils.WriteArrayLength(inputCount, ref dcurr, dend)) + while (!RespWriteUtils.WriteArrayWithNullElements(inputCount, ref dcurr, dend)) + SendAndReset(); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) SendAndReset(); - - for (var c = 0; c < inputCount; c++) - while (!RespWriteUtils.WriteNull(ref dcurr, dend)) - SendAndReset(); - - ReadLeftToken(inputCount, ref ptr); break; } } @@ -596,6 +626,10 @@ private unsafe bool SortedSetPop(RespCommand command, int count, byt while (!RespWriteUtils.WriteEmptyArray(ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -655,17 +689,27 @@ private unsafe bool SortedSetCount(int count, byte* ptr, ref TGarnet //restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + { + //command partially executed + return false; + } + } + switch (status) { case GarnetStatus.OK: // Process response - if (output.countDone == Int32.MaxValue) + if (output.countDone == int.MaxValue) { // Error in arguments - while (!RespWriteUtils.WriteError("ERR max or min value is not a float value."u8, ref dcurr, dend)) + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_FLOAT, ref dcurr, dend)) SendAndReset(); } - else if (output.countDone == Int32.MinValue) // command partially executed + else if (output.countDone == int.MinValue) // command partially executed return false; else while (!RespWriteUtils.WriteInteger(output.opsDone, ref dcurr, dend)) @@ -673,12 +717,13 @@ private unsafe bool SortedSetCount(int count, byte* ptr, ref TGarnet ptr += output.bytesDone; break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - return false; while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } //reset session counters @@ -754,6 +799,13 @@ private unsafe bool SortedSetLengthByValue(RespCommand command, int //restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -761,7 +813,7 @@ private unsafe bool SortedSetLengthByValue(RespCommand command, int if (output.countDone == Int32.MaxValue) { // Error in arguments - while (!RespWriteUtils.WriteError("ERR max or min value not in a valid range."u8, ref dcurr, dend)) + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_STRING, ref dcurr, dend)) SendAndReset(); } else if (output.countDone == Int32.MinValue) // command partially executed @@ -772,12 +824,13 @@ private unsafe bool SortedSetLengthByValue(RespCommand command, int ptr += output.bytesDone; break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - return false; while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -839,30 +892,29 @@ private unsafe bool SortedSetIncrement(int count, byte* ptr, ref TGa //restore input *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + ReadOnlySpan errorMessage = default; switch (status) { case GarnetStatus.NOTFOUND: case GarnetStatus.OK: - //verifying length of outputFooter - if (outputFooter.spanByteAndMemory.Length == 0) - { - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - return false; - errorMessage = "ERR wrong key type used in ZINCRBY command."u8; - } - else - { - //process output - var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); - //check for partial execution - if (objOutputHeader.countDone == int.MinValue) - return false; - else if (objOutputHeader.countDone == int.MaxValue) - errorMessage = "ERR increment value is not valid."u8; - ptr += objOutputHeader.bytesDone; - } + //process output + var objOutputHeader = ProcessOutputWithHeader(outputFooter.spanByteAndMemory); + //check for partial execution + if (objOutputHeader.countDone == int.MinValue) + return false; + if (objOutputHeader.countDone == int.MaxValue) + errorMessage = CmdStrings.RESP_ERR_NOT_VALID_FLOAT; + ptr += objOutputHeader.bytesDone; + break; + case GarnetStatus.WRONGTYPE: + errorMessage = CmdStrings.RESP_ERR_WRONG_TYPE; break; } @@ -935,6 +987,13 @@ private unsafe bool SortedSetRank(RespCommand command, int count, by var status = storageApi.SortedSetRank(key, new ArgSlice((byte*)inputPtr, inputLength), ref outputFooter); + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + // Reset input buffer *inputPtr = save; switch (status) @@ -950,7 +1009,10 @@ private unsafe bool SortedSetRank(RespCommand command, int count, by case GarnetStatus.NOTFOUND: while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend)) SendAndReset(); - ReadLeftToken(count - 1, ref ptr); + break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); break; } } @@ -1017,14 +1079,21 @@ private unsafe bool SortedSetRemoveRange(RespCommand command, int co //restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; + } + switch (status) { case GarnetStatus.OK: if (output.countDone == int.MaxValue) { var errorMessage = command == RespCommand.ZREMRANGEBYRANK ? - "ERR start or stop value is not in an integer or out of range."u8 : - "ERR max or min value is not a float value."u8; + CmdStrings.RESP_ERR_GENERIC_VALUE_IS_NOT_INTEGER : + CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_FLOAT; // Error in arguments while (!RespWriteUtils.WriteError(errorMessage, ref dcurr, dend)) @@ -1038,12 +1107,13 @@ private unsafe bool SortedSetRemoveRange(RespCommand command, int co ptr += output.bytesDone; break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(count - 1, ref ptr); - if (tokens < count - 1) - return false; while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -1142,6 +1212,10 @@ private unsafe bool SortedSetRandomMember(int count, byte* ptr, ref while (!RespWriteUtils.WriteDirect(respBytes, ref dcurr, dend)) SendAndReset(); break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } @@ -1168,7 +1242,6 @@ private unsafe bool SortedSetDifference(int count, byte* ptr, ref TG } else { - //number of keys if (!RespReadUtils.ReadIntWithLengthHeader(out var nKeys, ref ptr, recvBufferPtr + bytesRead)) return false; @@ -1218,26 +1291,35 @@ private unsafe bool SortedSetDifference(int count, byte* ptr, ref TG return true; } - storageApi.SortedSetDifference(keys, out var result); - - // write the size of the array reply - int resultCount = result == null ? 0 : result.Count; - while (!RespWriteUtils.WriteArrayLength(withscoresInclude ? resultCount * 2 : resultCount, ref dcurr, dend)) - SendAndReset(); + var status = storageApi.SortedSetDifference(keys, out var result); - if (result != null) + switch (status) { - foreach (var (element, score) in result) - { - while (!RespWriteUtils.WriteBulkString(element, ref dcurr, dend)) + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + // write the size of the array reply + int resultCount = result == null ? 0 : result.Count; + while (!RespWriteUtils.WriteArrayLength(withscoresInclude ? resultCount * 2 : resultCount, ref dcurr, dend)) SendAndReset(); - if (withscoresInclude) + if (result != null) { - while (!RespWriteUtils.TryWriteDoubleBulkString(score, ref dcurr, dend)) - SendAndReset(); + foreach (var (element, score) in result) + { + while (!RespWriteUtils.WriteBulkString(element, ref dcurr, dend)) + SendAndReset(); + + if (withscoresInclude) + { + while (!RespWriteUtils.TryWriteDoubleBulkString(score, ref dcurr, dend)) + SendAndReset(); + } + } } - } + break; } } } @@ -1245,6 +1327,5 @@ private unsafe bool SortedSetDifference(int count, byte* ptr, ref TG readHead = (int)(ptr - recvBufferPtr); return true; } - } } \ No newline at end of file diff --git a/libs/server/Resp/Objects/SortedSetGeoCommands.cs b/libs/server/Resp/Objects/SortedSetGeoCommands.cs index 3af18f83f9..c08ebf7e67 100644 --- a/libs/server/Resp/Objects/SortedSetGeoCommands.cs +++ b/libs/server/Resp/Objects/SortedSetGeoCommands.cs @@ -62,17 +62,30 @@ private unsafe bool GeoAdd(int count, byte* ptr, ref TGarnetApi stor //restore input buffer *inputPtr = save; - zaddDoneCount += output.countDone; - zaddAddCount += output.opsDone; + switch (status) + { + case GarnetStatus.WRONGTYPE: + var tokens = ReadLeftToken(count - 1, ref ptr); + if (tokens < count - 1) + return false; - // return if command is only partially done - if (zaddDoneCount < (inputCount / 3)) - return false; + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; + default: + zaddDoneCount += output.countDone; + zaddAddCount += output.opsDone; + + // return if command is only partially done + if (zaddDoneCount < (inputCount / 3)) + return false; - //update pointers - ptr += output.bytesDone; - while (!RespWriteUtils.WriteInteger(zaddAddCount, ref dcurr, dend)) - SendAndReset(); + //update pointers + ptr += output.bytesDone; + while (!RespWriteUtils.WriteInteger(zaddAddCount, ref dcurr, dend)) + SendAndReset(); + break; + } } //reset sesion counters @@ -175,6 +188,13 @@ private unsafe bool GeoCommands(RespCommand command, int count, byte //restore input buffer *inputPtr = save; + if (status != GarnetStatus.OK) + { + var tokens = ReadLeftToken(inputCount, ref ptr); + if (tokens < inputCount) + return false; + } + switch (status) { case GarnetStatus.OK: @@ -187,10 +207,6 @@ private unsafe bool GeoCommands(RespCommand command, int count, byte ptr += objOutputHeader.bytesDone; break; case GarnetStatus.NOTFOUND: - var tokens = ReadLeftToken(inputCount, ref ptr); - if (tokens < inputCount) - return false; - switch (op) { case SortedSetOperation.GEODIST: @@ -209,6 +225,10 @@ private unsafe bool GeoCommands(RespCommand command, int count, byte } break; + case GarnetStatus.WRONGTYPE: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend)) + SendAndReset(); + break; } } diff --git a/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs b/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs index 60cff8b259..62e8717417 100644 --- a/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs +++ b/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs @@ -17,9 +17,14 @@ public GarnetStatus RMW_ObjectStore(ref byte[] key, ref SpanByte CompletePendingForObjectStoreSession(ref status, ref output, ref objectStoreContext); if (status.Found) + { + if (output.spanByteAndMemory.Length == 0) + return GarnetStatus.WRONGTYPE; + return GarnetStatus.OK; - else - return GarnetStatus.NOTFOUND; + } + + return GarnetStatus.NOTFOUND; } public GarnetStatus Read_ObjectStore(ref byte[] key, ref SpanByte input, ref GarnetObjectStoreOutput output, ref TObjectContext objectStoreContext) @@ -31,9 +36,14 @@ public GarnetStatus Read_ObjectStore(ref byte[] key, ref SpanByt CompletePendingForObjectStoreSession(ref status, ref output, ref objectStoreContext); if (status.Found) + { + if (output.spanByteAndMemory.Length == 0) + return GarnetStatus.WRONGTYPE; + return GarnetStatus.OK; - else - return GarnetStatus.NOTFOUND; + } + + return GarnetStatus.NOTFOUND; } } } \ No newline at end of file diff --git a/libs/server/Storage/Session/ObjectStore/Common.cs b/libs/server/Storage/Session/ObjectStore/Common.cs index d954b46adc..908c69f54e 100644 --- a/libs/server/Storage/Session/ObjectStore/Common.cs +++ b/libs/server/Storage/Session/ObjectStore/Common.cs @@ -27,8 +27,10 @@ unsafe GarnetStatus RMWObjectStoreOperation(byte[] key, ArgSlice if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext); - Debug.Assert(_output.spanByteAndMemory.IsSpanByte); + if (_output.spanByteAndMemory.Length == 0) + return GarnetStatus.WRONGTYPE; + Debug.Assert(_output.spanByteAndMemory.IsSpanByte); return status.Found || status.Record.Created ? GarnetStatus.OK : GarnetStatus.NOTFOUND; } @@ -54,6 +56,9 @@ GarnetStatus RMWObjectStoreOperationWithOutput(byte[] key, ArgSl if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref outputFooter, ref objectStoreContext); + if (outputFooter.spanByteAndMemory.Length == 0) + return GarnetStatus.WRONGTYPE; + return status.Found || status.Record.Created ? GarnetStatus.OK : GarnetStatus.NOTFOUND; } @@ -78,6 +83,9 @@ GarnetStatus ReadObjectStoreOperationWithOutput(byte[] key, ArgS if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref outputFooter, ref objectStoreContext); + if (outputFooter.spanByteAndMemory.Length == 0) + return GarnetStatus.WRONGTYPE; + if (status.NotFound) return GarnetStatus.NOTFOUND; @@ -192,6 +200,8 @@ unsafe GarnetStatus ReadObjectStoreOperation(byte[] key, ArgSlic if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext); + if (_output.spanByteAndMemory.Length == 0) + return GarnetStatus.WRONGTYPE; Debug.Assert(_output.spanByteAndMemory.IsSpanByte); if (status.Found && (!status.Record.Created && !status.Record.CopyUpdated && !status.Record.InPlaceUpdated)) diff --git a/libs/server/Storage/Session/ObjectStore/HashOps.cs b/libs/server/Storage/Session/ObjectStore/HashOps.cs index dd0dcf167e..cade2a38e8 100644 --- a/libs/server/Storage/Session/ObjectStore/HashOps.cs +++ b/libs/server/Storage/Session/ObjectStore/HashOps.cs @@ -46,10 +46,11 @@ public unsafe GarnetStatus HashSet(ArgSlice key, ArgSlice field, rmwInput->count = 1; rmwInput->done = 0; - RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); itemsDoneCount = output.opsDone; - return GarnetStatus.OK; + + return status; } /// @@ -256,10 +257,11 @@ public unsafe GarnetStatus HashLength(ArgSlice key, out int item rmwInput->count = 1; rmwInput->done = 0; - ReadObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = ReadObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); items = output.countDone; - return GarnetStatus.OK; + + return status; } /// @@ -288,11 +290,11 @@ public unsafe GarnetStatus HashExists(ArgSlice key, ArgSlice fie rmwInput->count = 1; rmwInput->done = 0; - ReadObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = ReadObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); exists = output.countDone == 1; - return GarnetStatus.OK; + return status; } /// diff --git a/libs/server/Storage/Session/ObjectStore/ListOps.cs b/libs/server/Storage/Session/ObjectStore/ListOps.cs index 30d64e6ad4..3c88ae039b 100644 --- a/libs/server/Storage/Session/ObjectStore/ListOps.cs +++ b/libs/server/Storage/Session/ObjectStore/ListOps.cs @@ -47,10 +47,10 @@ public unsafe GarnetStatus ListPush(ArgSlice key, ArgSlice[] ele } var input = scratchBufferManager.GetSliceFromTail(inputLength); - RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); itemsDoneCount = output.countDone; - return GarnetStatus.OK; + return status; } /// @@ -142,7 +142,7 @@ public unsafe GarnetStatus ListPop(ArgSlice key, int count, List if (status == GarnetStatus.OK) elements = ProcessRespArrayOutput(outputFooter, out var error); - return GarnetStatus.OK; + return status; } /// @@ -186,8 +186,8 @@ public unsafe GarnetStatus ListLength(ArgSlice key, ref TObjectC /// /// /// out parameter, The element being popped and pushed - /// true when success - public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element) + /// GarnetStatus + public GarnetStatus ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element) { element = default; var objectLockableContext = txnManager.ObjectStoreLockableContext; @@ -212,13 +212,35 @@ public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirec // get the source key var statusOp = GET(sourceKey.ToArray(), out var sourceList, ref objectLockableContext); - if (statusOp == GarnetStatus.NOTFOUND || ((ListObject)sourceList.garnetObject).LnkList.Count == 0) + if (statusOp == GarnetStatus.NOTFOUND) { - return true; + return GarnetStatus.OK; } else if (statusOp == GarnetStatus.OK) { - var srcListObject = (ListObject)sourceList.garnetObject; + if (sourceList.garnetObject is not ListObject srcListObject) + return GarnetStatus.WRONGTYPE; + + if (srcListObject.LnkList.Count == 0) + return GarnetStatus.OK; + + ListObject dstListObject = default; + if (!sameKey) + { + // read destination key + var arrDestKey = destinationKey.ToArray(); + statusOp = GET(arrDestKey, out var destinationList, ref objectStoreLockableContext); + + if (statusOp == GarnetStatus.NOTFOUND) + { + destinationList.garnetObject = new ListObject(); + } + + if (destinationList.garnetObject is not ListObject listObject) + return GarnetStatus.WRONGTYPE; + + dstListObject = listObject; + } // right pop (removelast) from source if (sourceDirection == OperationDirection.Right) @@ -235,22 +257,11 @@ public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirec srcListObject.UpdateSize(element, false); //update sourcelist - SET(sourceKey.ToArray(), sourceList.garnetObject, ref objectStoreLockableContext); + SET(sourceKey.ToArray(), srcListObject, ref objectStoreLockableContext); IGarnetObject newListValue = null; if (!sameKey) { - // read destination key - var _destinationKey = destinationKey.ToArray(); - statusOp = GET(_destinationKey, out var destinationList, ref objectStoreLockableContext); - - if (statusOp == GarnetStatus.NOTFOUND) - { - destinationList.garnetObject = new ListObject(); - } - - var dstListObject = (ListObject)destinationList.garnetObject; - //left push (addfirst) to destination if (destinationDirection == OperationDirection.Left) dstListObject.LnkList.AddFirst(element); @@ -267,7 +278,7 @@ public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirec srcListObject.LnkList.AddFirst(element); else if (sourceDirection == OperationDirection.Left && destinationDirection == OperationDirection.Right) srcListObject.LnkList.AddLast(element); - newListValue = sourceList.garnetObject; + newListValue = srcListObject; ((ListObject)newListValue).UpdateSize(element); } @@ -281,8 +292,7 @@ public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirec txnManager.Commit(true); } - return true; - + return GarnetStatus.OK; } /// diff --git a/libs/server/Storage/Session/ObjectStore/SetOps.cs b/libs/server/Storage/Session/ObjectStore/SetOps.cs index b46cb3a501..8a24951a75 100644 --- a/libs/server/Storage/Session/ObjectStore/SetOps.cs +++ b/libs/server/Storage/Session/ObjectStore/SetOps.cs @@ -41,10 +41,10 @@ internal unsafe GarnetStatus SetAdd(ArgSlice key, ArgSlice membe rmwInput->count = 1; rmwInput->done = 0; - _ = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); saddCount = output.opsDone; - return GarnetStatus.OK; + return status; } /// @@ -158,10 +158,10 @@ internal unsafe GarnetStatus SetRemove(ArgSlice key, ArgSlice[] var input = scratchBufferManager.GetSliceFromTail(inputLength); - RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); sremCount = output.countDone; - return GarnetStatus.OK; + return status; } /// @@ -192,7 +192,7 @@ internal unsafe GarnetStatus SetLength(ArgSlice key, out int cou var status = ReadObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); count = output.countDone; - return GarnetStatus.OK; + return status; } /// @@ -378,13 +378,6 @@ internal unsafe GarnetStatus SetMove(ArgSlice sourceKey, ArgSlice destinationKey { smoveResult = 0; - // If the keys are the same, no operation is performed. - var sameKey = sourceKey.ReadOnlySpan.SequenceEqual(destinationKey.ReadOnlySpan); - if (sameKey) - { - return GarnetStatus.OK; - } - var createTransaction = false; if (txnManager.state != TxnState.Running) { @@ -398,19 +391,57 @@ internal unsafe GarnetStatus SetMove(ArgSlice sourceKey, ArgSlice destinationKey try { - var sremStatus = SetRemove(sourceKey, member, out var sremOps, ref objectLockableContext); + var arrDstKey = destinationKey.ToArray(); + var arrSrcKey = sourceKey.ToArray(); - if (sremStatus == GarnetStatus.NOTFOUND) - { + var srcGetStatus = GET(arrSrcKey, out var srcObject, ref objectLockableContext); + + if (srcGetStatus == GarnetStatus.NOTFOUND) return GarnetStatus.NOTFOUND; - } - if (sremOps != 1) - { + if (srcObject.garnetObject is not SetObject srcSetObject) + return GarnetStatus.WRONGTYPE; + + // If the keys are the same, no operation is performed. + var sameKey = sourceKey.ReadOnlySpan.SequenceEqual(destinationKey.ReadOnlySpan); + if (sameKey) return GarnetStatus.OK; + + var dstGetStatus = GET(arrDstKey, out var dstObject, ref objectLockableContext); + + SetObject dstSetObject; + if (dstGetStatus == GarnetStatus.OK) + { + if (dstObject.garnetObject is not SetObject tmpDstSetObject) + return GarnetStatus.WRONGTYPE; + + dstSetObject = tmpDstSetObject; } + else + { + dstSetObject = new SetObject(); + } + + var arrMember = member.ToArray(); + + var removed = srcSetObject.Set.Remove(arrMember); + if (!removed) return GarnetStatus.OK; + + srcSetObject.UpdateSize(arrMember, false); + + dstSetObject.Set.Add(arrMember); + dstSetObject.UpdateSize(arrMember); - _ = SetAdd(destinationKey, member, out smoveResult, ref objectLockableContext); + if (dstGetStatus == GarnetStatus.NOTFOUND) + { + var setStatus = SET(arrDstKey, dstSetObject, ref objectLockableContext); + if (setStatus == GarnetStatus.OK) + smoveResult = 1; + } + else + { + smoveResult = 1; + } } finally { @@ -452,15 +483,13 @@ public GarnetStatus SetIntersect(ArgSlice[] keys, out HashSet output) try { - output = SetIntersect(keys, ref setObjectStoreLockableContext); + return SetIntersect(keys, ref setObjectStoreLockableContext, out output); } finally { if (createTransaction) txnManager.Commit(true); } - - return GarnetStatus.OK; } /// @@ -499,67 +528,85 @@ public GarnetStatus SetIntersectStore(byte[] key, ArgSlice[] keys, out int count try { - var members = SetIntersect(keys, ref setObjectStoreLockableContext); + var status = SetIntersect(keys, ref setObjectStoreLockableContext, out var members); - var newSetObject = new SetObject(); - foreach (var item in members) + if (status == GarnetStatus.OK) { - _ = newSetObject.Set.Add(item); - newSetObject.UpdateSize(item); + var newSetObject = new SetObject(); + foreach (var item in members) + { + _ = newSetObject.Set.Add(item); + newSetObject.UpdateSize(item); + } + _ = SET(key, newSetObject, ref setObjectStoreLockableContext); + count = members.Count; } - _ = SET(key, newSetObject, ref setObjectStoreLockableContext); - count = members.Count; + + return status; } finally { if (createTransaction) txnManager.Commit(true); } - - return GarnetStatus.OK; } - private HashSet SetIntersect(ArgSlice[] keys, ref TObjectContext objectContext) + private GarnetStatus SetIntersect(ArgSlice[] keys, ref TObjectContext objectContext, out HashSet output) where TObjectContext : ITsavoriteContext { + output = new HashSet(ByteArrayComparer.Instance); + if (keys.Length == 0) { - return new HashSet(ByteArrayComparer.Instance); + return GarnetStatus.OK; } - HashSet result; var status = GET(keys[0].ToArray(), out var first, ref objectContext); - if (status == GarnetStatus.OK && first.garnetObject is SetObject firstObject) + if (status == GarnetStatus.OK) { - result = new HashSet(firstObject.Set, ByteArrayComparer.Instance); + if (first.garnetObject is not SetObject firstObject) + { + output = default; + return GarnetStatus.WRONGTYPE; + } + + output = new HashSet(firstObject.Set, ByteArrayComparer.Instance); } else { - return new HashSet(ByteArrayComparer.Instance); + return GarnetStatus.OK; } for (var i = 1; i < keys.Length; i++) { // intersection of anything with empty set is empty set - if (result.Count == 0) + if (output.Count == 0) { - return result; + output.Clear(); + return GarnetStatus.OK; } status = GET(keys[i].ToArray(), out var next, ref objectContext); - if (status == GarnetStatus.OK && next.garnetObject is SetObject nextObject) + if (status == GarnetStatus.OK) { - result.IntersectWith(nextObject.Set); + if (next.garnetObject is not SetObject nextObject) + { + output = default; + return GarnetStatus.WRONGTYPE; + } + + output.IntersectWith(nextObject.Set); } else { - return new HashSet(ByteArrayComparer.Instance); + output.Clear(); + return GarnetStatus.OK; } } - return result; + return GarnetStatus.OK; } /// @@ -592,15 +639,13 @@ public GarnetStatus SetUnion(ArgSlice[] keys, out HashSet output) try { - output = SetUnion(keys, ref setObjectStoreLockableContext); + return SetUnion(keys, ref setObjectStoreLockableContext, out output); } finally { if (createTransaction) txnManager.Commit(true); } - - return GarnetStatus.OK; } /// @@ -637,45 +682,53 @@ public GarnetStatus SetUnionStore(byte[] key, ArgSlice[] keys, out int count) try { - var members = SetUnion(keys, ref setObjectStoreLockableContext); + var status = SetUnion(keys, ref setObjectStoreLockableContext, out var members); - var newSetObject = new SetObject(); - foreach (var item in members) + if (status == GarnetStatus.OK) { - _ = newSetObject.Set.Add(item); - newSetObject.UpdateSize(item); + var newSetObject = new SetObject(); + foreach (var item in members) + { + _ = newSetObject.Set.Add(item); + newSetObject.UpdateSize(item); + } + _ = SET(key, newSetObject, ref setObjectStoreLockableContext); + count = members.Count; } - _ = SET(key, newSetObject, ref setObjectStoreLockableContext); - count = members.Count; + + return status; } finally { if (createTransaction) txnManager.Commit(true); } - - return GarnetStatus.OK; } - private HashSet SetUnion(ArgSlice[] keys, ref TObjectContext objectContext) + private GarnetStatus SetUnion(ArgSlice[] keys, ref TObjectContext objectContext, out HashSet output) where TObjectContext : ITsavoriteContext { - var result = new HashSet(ByteArrayComparer.Instance); + output = new HashSet(ByteArrayComparer.Instance); if (keys.Length == 0) { - return result; + return GarnetStatus.OK; } foreach (var item in keys) { if (GET(item.ToArray(), out var currObject, ref objectContext) == GarnetStatus.OK) { - var currSet = ((SetObject)currObject.garnetObject).Set; - result.UnionWith(currSet); + if (currObject.garnetObject is not SetObject setObject) + { + output = default; + return GarnetStatus.WRONGTYPE; + } + + output.UnionWith(setObject.Set); } } - return result; + return GarnetStatus.OK; } /// @@ -806,15 +859,13 @@ public GarnetStatus SetDiff(ArgSlice[] keys, out HashSet members) try { - members = SetDiff(keys, ref setObjectStoreLockableContext); + return SetDiff(keys, ref setObjectStoreLockableContext, out members); } finally { if (createTransaction) txnManager.Commit(true); } - - return GarnetStatus.OK; } /// @@ -851,47 +902,53 @@ public GarnetStatus SetDiffStore(byte[] key, ArgSlice[] keys, out int count) try { - var diffSet = SetDiff(keys, ref setObjectStoreLockableContext); + var status = SetDiff(keys, ref setObjectStoreLockableContext, out var diffSet); - var newSetObject = new SetObject(); - foreach (var item in diffSet) + if (status == GarnetStatus.OK) { - _ = newSetObject.Set.Add(item); - newSetObject.UpdateSize(item); + var newSetObject = new SetObject(); + foreach (var item in diffSet) + { + _ = newSetObject.Set.Add(item); + newSetObject.UpdateSize(item); + } + _ = SET(key, newSetObject, ref setObjectStoreLockableContext); + count = diffSet.Count; } - _ = SET(key, newSetObject, ref setObjectStoreLockableContext); - count = diffSet.Count; + + return status; } finally { if (createTransaction) txnManager.Commit(true); } - - return GarnetStatus.OK; } - private HashSet SetDiff(ArgSlice[] keys, ref TObjectContext objectContext) + private GarnetStatus SetDiff(ArgSlice[] keys, ref TObjectContext objectContext, out HashSet output) where TObjectContext : ITsavoriteContext { - var result = new HashSet(); + output = new HashSet(); if (keys.Length == 0) { - return result; + return GarnetStatus.OK; } // first SetObject var status = GET(keys[0].ToArray(), out var first, ref objectContext); if (status == GarnetStatus.OK) { - if (first.garnetObject is SetObject firstObject) + if (first.garnetObject is not SetObject firstObject) { - result = new HashSet(firstObject.Set, ByteArrayComparer.Instance); + output = default; + return GarnetStatus.WRONGTYPE; } + + output = new HashSet(firstObject.Set, ByteArrayComparer.Instance); } else { - return result; + return GarnetStatus.OK; } // after SetObjects @@ -900,14 +957,17 @@ private HashSet SetDiff(ArgSlice[] keys, ref TObjectCont status = GET(keys[i].ToArray(), out var next, ref objectContext); if (status == GarnetStatus.OK) { - if (next.garnetObject is SetObject nextObject) + if (next.garnetObject is not SetObject nextObject) { - result.ExceptWith(nextObject.Set); + output = default; + return GarnetStatus.WRONGTYPE; } + + output.ExceptWith(nextObject.Set); } } - return result; + return GarnetStatus.OK; } } } \ No newline at end of file diff --git a/libs/server/Storage/Session/ObjectStore/SortedSetOps.cs b/libs/server/Storage/Session/ObjectStore/SortedSetOps.cs index 3ede0bba31..1bfb8cf69c 100644 --- a/libs/server/Storage/Session/ObjectStore/SortedSetOps.cs +++ b/libs/server/Storage/Session/ObjectStore/SortedSetOps.cs @@ -41,10 +41,10 @@ public unsafe GarnetStatus SortedSetAdd(ArgSlice key, ArgSlice s rmwInput->count = 1; rmwInput->done = 0; - RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); zaddCount = output.opsDone; - return GarnetStatus.OK; + return status; } /// @@ -81,10 +81,10 @@ public unsafe GarnetStatus SortedSetAdd(ArgSlice key, (ArgSlice } var input = scratchBufferManager.GetSliceFromTail(inputLength); - RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key.ToArray(), input, out var output, ref objectStoreContext); zaddCount = output.opsDone; - return GarnetStatus.OK; + return status; } /// @@ -115,10 +115,10 @@ public unsafe GarnetStatus SortedSetRemove(byte[] key, ArgSlice rmwInput->count = 1; rmwInput->done = 0; - RMWObjectStoreOperation(key, _inputSlice, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key, _inputSlice, out var output, ref objectStoreContext); zremCount = output.opsDone; - return GarnetStatus.OK; + return status; } /// @@ -155,10 +155,10 @@ public unsafe GarnetStatus SortedSetRemove(byte[] key, ArgSlice[ } var input = scratchBufferManager.GetSliceFromTail(inputLength); - RMWObjectStoreOperation(key, input, out var output, ref objectStoreContext); + var status = RMWObjectStoreOperation(key, input, out var output, ref objectStoreContext); zremCount = output.opsDone; - return GarnetStatus.OK; + return status; } /// @@ -182,6 +182,7 @@ public unsafe GarnetStatus SortedSetRemoveRangeByLex(ArgSlice ke var minBytes = Encoding.ASCII.GetBytes(min); var maxBytes = Encoding.ASCII.GetBytes(max); + GarnetStatus status; fixed (byte* ptr = minBytes) { fixed (byte* ptr2 = maxBytes) @@ -198,12 +199,12 @@ public unsafe GarnetStatus SortedSetRemoveRangeByLex(ArgSlice ke rmwInput->count = 3; rmwInput->done = 0; - RMWObjectStoreOperation(key.ToArray(), _inputSlice, out var output, ref objectStoreContext); + status = RMWObjectStoreOperation(key.ToArray(), _inputSlice, out var output, ref objectStoreContext); countRemoved = output.opsDone; } } - return GarnetStatus.OK; + return status; } /// @@ -227,6 +228,7 @@ public unsafe GarnetStatus SortedSetRemoveRangeByScore(ArgSlice var minBytes = Encoding.ASCII.GetBytes(min); var maxBytes = Encoding.ASCII.GetBytes(max); + GarnetStatus status; fixed (byte* ptr = minBytes) { fixed (byte* ptr2 = maxBytes) @@ -243,12 +245,12 @@ public unsafe GarnetStatus SortedSetRemoveRangeByScore(ArgSlice rmwInput->count = 3; rmwInput->done = 0; - RMWObjectStoreOperation(key.ToArray(), _inputSlice, out var output, ref objectStoreContext); + status = RMWObjectStoreOperation(key.ToArray(), _inputSlice, out var output, ref objectStoreContext); countRemoved = output.opsDone; } } - return GarnetStatus.OK; + return status; } /// @@ -272,6 +274,7 @@ public unsafe GarnetStatus SortedSetRemoveRangeByRank(ArgSlice k var startBytes = Encoding.ASCII.GetBytes(start.ToString()); var stopBytes = Encoding.ASCII.GetBytes(stop.ToString()); + GarnetStatus status; fixed (byte* ptr = startBytes) { fixed (byte* ptr2 = stopBytes) @@ -288,12 +291,12 @@ public unsafe GarnetStatus SortedSetRemoveRangeByRank(ArgSlice k rmwInput->count = 3; rmwInput->done = 0; - RMWObjectStoreOperation(key.ToArray(), _inputSlice, out var output, ref objectStoreContext); + status = RMWObjectStoreOperation(key.ToArray(), _inputSlice, out var output, ref objectStoreContext); countRemoved = output.opsDone; } } - return GarnetStatus.OK; + return status; } /// @@ -329,7 +332,7 @@ public unsafe GarnetStatus SortedSetPop(ArgSlice key, int count, //process output //if (status == GarnetStatus.OK) - var npairs = ProcessRespArrayOutput(outputFooter, out string error); + ProcessRespArrayOutput(outputFooter, out _); return status; } @@ -356,6 +359,7 @@ public unsafe GarnetStatus SortedSetIncrement(ArgSlice key, doub var incrementBytes = Encoding.ASCII.GetBytes(increment.ToString(CultureInfo.InvariantCulture)); + GarnetStatus status; fixed (byte* ptr = incrementBytes) { var incrementArgSlice = new ArgSlice(ptr, incrementBytes.Length); @@ -370,7 +374,7 @@ public unsafe GarnetStatus SortedSetIncrement(ArgSlice key, doub rmwInput->done = 0; var outputFooter = new GarnetObjectStoreOutput { spanByteAndMemory = new SpanByteAndMemory(null) }; - var status = RMWObjectStoreOperationWithOutput(key.ToArray(), _inputSlice, ref objectStoreContext, ref outputFooter); + status = RMWObjectStoreOperationWithOutput(key.ToArray(), _inputSlice, ref objectStoreContext, ref outputFooter); //Process output string error = default; @@ -385,7 +389,7 @@ public unsafe GarnetStatus SortedSetIncrement(ArgSlice key, doub } } - return GarnetStatus.OK; + return status; } /// @@ -413,10 +417,10 @@ public unsafe GarnetStatus SortedSetLength(ArgSlice key, out int rmwInput->count = 1; rmwInput->done = 0; - ReadObjectStoreOperation(key.ToArray(), input, out ObjectOutputHeader output, ref objectStoreContext); + var status = ReadObjectStoreOperation(key.ToArray(), input, out ObjectOutputHeader output, ref objectStoreContext); zcardCount = output.opsDone; - return GarnetStatus.OK; + return status; } /// @@ -573,21 +577,30 @@ public unsafe GarnetStatus SortedSetDifference(ArgSlice[] keys, out Dictionary input, ref (IMe dict[key] = value; UpdateSize(key, value); - break; // +OK is sent as response, by default + WriteSimpleString(ref output, "OK"); + break; } case 1: // MYDICTGET { diff --git a/test/Garnet.test/RespCustomCommandTests.cs b/test/Garnet.test/RespCustomCommandTests.cs index 1878ef76b6..e20f6cfe38 100644 --- a/test/Garnet.test/RespCustomCommandTests.cs +++ b/test/Garnet.test/RespCustomCommandTests.cs @@ -436,6 +436,36 @@ public void CustomObjectCommandTest2() Assert.AreEqual(value2, (string)retValue); } + [Test] + public void CustomObjectCommandTest3() + { + // Register sample custom command on object + var factory = new MyDictFactory(); + server.Register.NewCommand("MYDICTSET", 2, CommandType.ReadModifyWrite, factory); + server.Register.NewCommand("MYDICTGET", 1, CommandType.Read, factory); + + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var mainkey = "key"; + + var key1 = "mykey1"; + var value1 = "foovalue1"; + db.ListLeftPush(mainkey, value1); + + var ex = Assert.Throws(() => db.Execute("MYDICTGET", mainkey, key1)); + var expectedError = Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE); + Assert.IsNotNull(ex); + Assert.AreEqual(expectedError, ex.Message); + + var deleted = db.KeyDelete(mainkey); + Assert.IsTrue(deleted); + db.Execute("MYDICTSET", mainkey, key1, value1); + + ex = Assert.Throws(() => db.ListLeftPush(mainkey, value1)); + Assert.IsNotNull(ex); + Assert.AreEqual(expectedError, ex.Message); + } [Test] public async Task CustomCommandSetFollowedByTtlTestAsync() diff --git a/test/Garnet.test/RespHashTests.cs b/test/Garnet.test/RespHashTests.cs index 45b1882b74..13ef3667fa 100644 --- a/test/Garnet.test/RespHashTests.cs +++ b/test/Garnet.test/RespHashTests.cs @@ -1,4 +1,6 @@ // Copyright (c) Microsoft Corporation. +// // Copyright (c) Microsoft Corporation. +// // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. using System; @@ -533,6 +535,61 @@ public void CheckEmptyHashKeyRemoved() Assert.IsFalse(keyExists); } + [Test] + public void CheckHashOperationsOnWrongTypeObjectSE() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var keys = new[] { new RedisKey("user1:obj1"), new RedisKey("user1:obj2") }; + var key1Values = new[] { new RedisValue("Hello"), new RedisValue("World") }; + var key2Values = new[] { new RedisValue("Hola"), new RedisValue("Mundo") }; + var values = new[] { key1Values, key2Values }; + var hashFields = new[] + { + new[] { new RedisValue("K1_H1"), new RedisValue("K1_H2") }, + new[] { new RedisValue("K2_H1"), new RedisValue("K2_H2") } + }; + var hashEntries = hashFields.Select((h, idx) => h + .Zip(values[idx], (n, v) => new HashEntry(n, v)).ToArray()).ToArray(); + + // Set up different type objects + RespTestsUtils.SetUpTestObjects(db, GarnetObjectType.List, keys, values); + + // HGET + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashGet(keys[0], hashFields[0][0])); + // HMGET + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashGet(keys[0], hashFields[0])); + // HSET + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashSet(keys[0], hashFields[0][0], values[0][0])); + // HMSET + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashSet(keys[0], hashEntries[0])); + // HSETNX + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashSet(keys[0], hashFields[0][0], values[0][0], When.NotExists)); + // HLEN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashLength(keys[0])); + // HDEL + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashDelete(keys[0], hashFields[0])); + // HEXISTS + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashExists(keys[0], hashFields[0][0])); + // HGETALL + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashGetAll(keys[0])); + // HKEYS + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashKeys(keys[0])); + // HVALS + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashValues(keys[0])); + // HINCRBY + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashIncrement(keys[0], hashFields[0][0], 2L)); + // HINCRBYFLOAT + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashIncrement(keys[0], hashFields[0][0], 2.2)); + // HRANDFIELD + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashRandomField(keys[0])); + // HSCAN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashScan(keys[0], new RedisValue("*")).FirstOrDefault()); + //HSTRLEN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.HashStringLength(keys[0], hashFields[0][0])); + } + #endregion #region LightClientTests diff --git a/test/Garnet.test/RespListTests.cs b/test/Garnet.test/RespListTests.cs index b7aabe27d6..f70385751e 100644 --- a/test/Garnet.test/RespListTests.cs +++ b/test/Garnet.test/RespListTests.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using Garnet.server; +using Newtonsoft.Json.Linq; using NUnit.Framework; using StackExchange.Redis; @@ -925,7 +926,7 @@ public void CanSendErrorInWrongTypeLC() using var lightClientRequest = TestUtils.CreateRequest(); var response = lightClientRequest.SendCommand("HSET myhash onekey onepair"); lightClientRequest.SendCommand("LINSERT myhash BEFORE one two"); - var expectedResponse = "-ERR wrong key type used in LINSERT command.\r\n"; + var expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE)}\r\n"; var actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); } @@ -1223,7 +1224,7 @@ public void CanDoLPushxRPushx() lightClientRequest.SendCommand("RPUSHX mylist value-one"); var len = lightClientRequest.SendCommand("LLEN mylist"); - var expectedResponse = ":0\r\n"; + var expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE)}\r\n"; var actualValue = Encoding.ASCII.GetString(len).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); } @@ -1244,5 +1245,51 @@ public void CheckEmptyListKeyRemoved() var keyExists = db.KeyExists(key); Assert.IsFalse(keyExists); } + + [Test] + public void CheckListOperationsOnWrongTypeObjectSE() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var keys = new[] { new RedisKey("user1:obj1"), new RedisKey("user1:obj2") }; + var key1Values = new[] { new RedisValue("Hello"), new RedisValue("World") }; + var key2Values = new[] { new RedisValue("Hola"), new RedisValue("Mundo") }; + var values = new[] { key1Values, key2Values }; + + // Set up different type objects + RespTestsUtils.SetUpTestObjects(db, GarnetObjectType.Set, keys, values); + + // LPOP + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListLeftPop(keys[0])); + // LPUSH + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListLeftPush(keys[0], values[0])); + // LPUSHX + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListLeftPush(keys[0], values[0], When.Exists)); + // RPOP + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListRightPop(keys[0])); + // RPUSH + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListRightPush(keys[0], values[0])); + // RPUSHX + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListRightPush(keys[0], values[0], When.Exists)); + // LLEN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListLength(keys[0])); + // LTRIM + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListTrim(keys[0], 2, 5)); + // LRANGE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListRange(keys[0], 2, 5)); + // LINDEX + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListGetByIndex(keys[0], 2)); + // LINSERT + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListInsertAfter(keys[0], values[0][0], values[0][1])); + // LREM + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListRemove(keys[0], values[0][0])); + // RPOPLPUSH + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListRightPopLeftPush(keys[0], keys[1])); + // LMOVE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListMove(keys[0], keys[1], ListSide.Left, ListSide.Right)); + // LSET + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.ListSetByIndex(keys[0], 2, values[0][1])); + } } } \ No newline at end of file diff --git a/test/Garnet.test/RespSetTest.cs b/test/Garnet.test/RespSetTest.cs index 2e3351b6ac..a601bafbfc 100644 --- a/test/Garnet.test/RespSetTest.cs +++ b/test/Garnet.test/RespSetTest.cs @@ -1277,6 +1277,52 @@ public void CanDoSinterStoreWhenMemberKeysNotExisting() Assert.AreEqual(expectedResponse, strResponse); } + [Test] + public void CheckSetOperationsOnWrongTypeObjectSE() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var keys = new[] { new RedisKey("user1:obj1"), new RedisKey("user1:obj2") }; + var key1Values = new[] { new RedisValue("Hello"), new RedisValue("World") }; + var key2Values = new[] { new RedisValue("Hola"), new RedisValue("Mundo") }; + var values = new[] { key1Values, key2Values }; + + // Set up different type objects + RespTestsUtils.SetUpTestObjects(db, GarnetObjectType.List, keys, values); + + // SADD + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetAdd(keys[0], values[0])); + // SREM + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetRemove(keys[0], values[0])); + // SPOP + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetPop(keys[0], 2)); + // SMEMBERS + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetMembers(keys[0])); + // SCARD + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetLength(keys[0])); + // SSCAN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetScan(keys[0], new RedisValue("*")).FirstOrDefault()); + // SMOVE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetMove(keys[0], keys[1], values[0][0])); + // SRANDMEMBER + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetRandomMember(keys[0])); + // SISMEMBER + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetContains(keys[0], values[0][0])); + // SUNION + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetCombine(SetOperation.Union, keys[0], keys[1])); + // SUNIONSTORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.Execute("SUNIONSTORE", keys[0], keys[1])); + // SDIFF + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetCombine(SetOperation.Difference, keys[0], keys[1])); + // SDIFFSTORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.Execute("SDIFFSTORE", keys[0], keys[1])); + // SINTER + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SetCombine(SetOperation.Intersect, keys[0], keys[1])); + // SINTERSTORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.Execute("SINTERSTORE", keys[0], keys[1])); + } + #endregion diff --git a/test/Garnet.test/RespSortedSetGeoTests.cs b/test/Garnet.test/RespSortedSetGeoTests.cs index d4a181c97c..cebc53f5fb 100644 --- a/test/Garnet.test/RespSortedSetGeoTests.cs +++ b/test/Garnet.test/RespSortedSetGeoTests.cs @@ -3,6 +3,7 @@ using System; using System.Globalization; +using System.Linq; using System.Text; using Garnet.common; using Garnet.server; @@ -236,6 +237,42 @@ public void CanValidateUnknownWithNotSupportedOptions() Assert.Throws(() => db.GeoSearch(key, 73.9262, 40.8296, box, count: 2)); } + [Test] + public void CheckGeoSortedSetOperationsOnWrongTypeObjectSE() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var keys = new[] { new RedisKey("user1:obj1"), new RedisKey("user1:obj2") }; + var values = new[] + { + new[] { new RedisValue("Tel Aviv"), new RedisValue("Haifa") }, + new[] { new RedisValue("Athens"), new RedisValue("Thessaloniki") } + }; + var coords = new[] + { + new[] { new[] { 2.0853, 34.7818 }, new[] { 32.7940, 34.9896 } }, + new[] { new[] { 7.9838, 23.7275 }, new[] { 40.6401, 22.9444 } } + }; + + var geoEntries = values.Select((h, idx) => h + .Zip(coords[idx], (v, c) => new GeoEntry(c[0], c[1], v)).ToArray()).ToArray(); + + // Set up different type objects + RespTestsUtils.SetUpTestObjects(db, GarnetObjectType.Set, keys, values); + + // GEOADD + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.GeoAdd(keys[0], geoEntries[0])); + // GEOHASH + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.GeoHash(keys[0], values[0])); + // GEODIST + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.GeoDistance(keys[0], values[0][1], values[0][1])); + // GEOPOS + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.GeoPosition(keys[0], values[0])); + // GEOSEARCH + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.GeoSearch(keys[0], values[0][1], new GeoSearchBox(800, 800, GeoUnit.Kilometers))); + } + //end region of SE tests #endregion diff --git a/test/Garnet.test/RespSortedSetTests.cs b/test/Garnet.test/RespSortedSetTests.cs index 8b90270300..7dcdcb2ba6 100644 --- a/test/Garnet.test/RespSortedSetTests.cs +++ b/test/Garnet.test/RespSortedSetTests.cs @@ -10,6 +10,7 @@ using Garnet.server; using NUnit.Framework; using StackExchange.Redis; +using SetOperation = StackExchange.Redis.SetOperation; namespace Garnet.test { @@ -709,6 +710,68 @@ public void CheckEmptySortedSetKeyRemoved() Assert.IsFalse(keyExists); } + [Test] + public void CheckSortedSetOperationsOnWrongTypeObjectSE() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var keys = new[] { new RedisKey("user1:obj1"), new RedisKey("user1:obj2") }; + var key1Values = new[] { new RedisValue("Hello"), new RedisValue("World") }; + var key2Values = new[] { new RedisValue("Hola"), new RedisValue("Mundo") }; + var values = new[] { key1Values, key2Values }; + var scores = new[] { new[] { 1.1, 1.2 }, new[] { 2.1, 2.2 } }; + var sortedSetEntries = values.Select((h, idx) => h + .Zip(scores[idx], (n, v) => new SortedSetEntry(n, v)).ToArray()).ToArray(); + + + // Set up different type objects + RespTestsUtils.SetUpTestObjects(db, GarnetObjectType.Set, keys, values); + + // ZADD + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetAdd(keys[0], sortedSetEntries[0])); + // ZCARD + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetLength(keys[0])); + // ZPOPMAX + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetPop(keys[0])); + // ZSCORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetScore(keys[0], values[0][0])); + // ZREM + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRemove(keys[0], values[0])); + // ZCOUNT + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetLength(keys[1], 1, 2)); + // ZINCRBY + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetIncrement(keys[1], values[1][0], 2.2)); + // ZRANK + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRank(keys[1], values[1][0])); + // ZRANGE + //RespTests.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRangeByValueAsync(keys[1]).Wait()); + // ZRANGEBYSCORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRangeByScore(keys[1])); + // ZREVRANGE + //RespTests.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRangeByScore(keys[1], 1, 2, Exclude.None, Order.Descending)); + // ZREVRANK + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRangeByRank(keys[1], 1, 2, Order.Descending)); + // ZREMRANGEBYLEX + //RespTests.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRemoveRangeByValue(keys[1], values[1][0], values[1][1])); + // ZREMRANGEBYRANK + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRemoveRangeByRank(keys[1], 0, 1)); + // ZREMRANGEBYSCORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRemoveRangeByScore(keys[1], 1, 2)); + // ZLEXCOUNT + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetLengthByValue(keys[1], values[1][0], values[1][1])); + // ZPOPMIN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetPop(keys[1], Order.Descending)); + // ZRANDMEMBER + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetRandomMember(keys[1])); + // ZDIFF + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetCombine(SetOperation.Difference, keys)); + // ZSCAN + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetScan(keys[1], new RedisValue("*")).FirstOrDefault()); + //ZMSCORE + RespTestsUtils.CheckCommandOnWrongTypeObjectSE(() => db.SortedSetScores(keys[1], values[1])); + } + #endregion #region LightClientTests @@ -977,7 +1040,7 @@ public void CanValidateInvalidParamentersZCountLC(int bytesSent) response = lightClientRequest.Execute("ZADD board 560 Tom", expectedResponse.Length, bytesSent); Assert.AreEqual(expectedResponse, response); - expectedResponse = "-ERR max or min value is not a float value.\r\n"; + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_FLOAT)}\r\n"; response = lightClientRequest.Execute("ZCOUNT board 5 b", expectedResponse.Length, bytesSent); Assert.AreEqual(expectedResponse, response); } @@ -1148,7 +1211,7 @@ public void CanManageExistingKeyButOtherTypeLC(int bytesSent) //do zincrby var response = lightClientRequest.SendCommandChunks("ZINCRBY myboard 1 field1", bytesSent); - var expectedResponse = "-ERR wrong key type used in ZINCRBY command.\r\n"; + var expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE)}\r\n"; var actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); } @@ -1312,7 +1375,7 @@ public void CanDoZRemRangeByRank(int bytesSent) Assert.AreEqual(expectedResponse, actualValue); response = lightClientRequest.SendCommandChunks("ZREMRANGEBYLEX myzset =a .", bytesSent); - expectedResponse = "-ERR max or min value not in a valid range.\r\n"; + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_STRING)}\r\n"; actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); @@ -1322,7 +1385,7 @@ public void CanDoZRemRangeByRank(int bytesSent) Assert.AreEqual(expectedResponse, actualValue); response = lightClientRequest.SendCommandChunks("ZREMRANGEBYRANK board a b", bytesSent); - expectedResponse = "-ERR start or stop value is not in an integer or out of range.\r\n"; + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_GENERIC_VALUE_IS_NOT_INTEGER)}\r\n"; actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); @@ -1362,7 +1425,7 @@ public void CanDoZRemRangeByScore(int bytesSent) Assert.AreEqual(expectedResponse, actualValue); response = lightClientRequest.SendCommandChunks("ZREMRANGEBYSCORE board a b", bytesSent); - expectedResponse = "-ERR max or min value is not a float value.\r\n"; + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_FLOAT)}\r\n"; actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); } @@ -1405,7 +1468,7 @@ public void CanUseZLexCount(int bytesSent) Assert.AreEqual(expectedResponse, actualValue); response = lightClientRequest.SendCommandChunks("ZLEXCOUNT board *d 8", bytesSent); - expectedResponse = "-ERR max or min value not in a valid range.\r\n"; + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_STRING)}\r\n"; actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); } @@ -1704,7 +1767,7 @@ public void CanManageNotExistingKeySortedSetCommandsRMWOps() lightClientRequest.SendCommand("ZADD zset1 1 uno 2 due 3 tre 4 quattro 5 cinque 6 sei"); result = lightClientRequest.SendCommand("ZREMRANGEBYLEX zset1 0 1"); - expectedResponse = "-ERR max or min value not in a valid range.\r\n"; + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_MIN_MAX_NOT_VALID_STRING)}\r\n"; ; actualValue = Encoding.ASCII.GetString(result).Substring(0, expectedResponse.Length); Assert.AreEqual(expectedResponse, actualValue); @@ -1913,6 +1976,43 @@ public void CanContinueOnInvalidInput() } + [Test] + public void CheckSortedSetOperationsOnWrongTypeObjectLC() + { + // This is to test remaining commands that were not covered in CheckSortedSetOperationsOnWrongTypeObjectLC + // since they are not supported in SE.Redis + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + using var lightClientRequest = TestUtils.CreateRequest(); + + var keys = new[] { new RedisKey("user1:obj1"), new RedisKey("user1:obj2") }; + var key1Values = new[] { new RedisValue("Hello"), new RedisValue("World") }; + var key2Values = new[] { new RedisValue("Hola"), new RedisValue("Mundo") }; + var values = new[] { key1Values, key2Values }; + + // Set up different type objects + RespTestsUtils.SetUpTestObjects(db, GarnetObjectType.Set, keys, values); + + // ZRANGE + var response = lightClientRequest.SendCommand($"ZRANGE {keys[0]} 0 -1"); + var expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE)}\r\n"; + var actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); + Assert.AreEqual(expectedResponse, actualValue); + + + // ZREVRANGE + response = lightClientRequest.SendCommand($"ZREVRANGE {keys[0]} 0 -1"); + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE)}\r\n"; + actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); + Assert.AreEqual(expectedResponse, actualValue); + + // ZREMRANGEBYLEX + response = lightClientRequest.SendCommand($"ZREMRANGEBYLEX {keys[0]} {values[1][0]} {values[1][1]}"); + expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE)}\r\n"; + actualValue = Encoding.ASCII.GetString(response).Substring(0, expectedResponse.Length); + Assert.AreEqual(expectedResponse, actualValue); + } + #endregion private static void SendCommandWithoutKey(string command, LightClientRequest lightClientRequest) diff --git a/test/Garnet.test/RespTests.cs b/test/Garnet.test/RespTests.cs index 383ad7a8b5..3e1d3eb7e6 100644 --- a/test/Garnet.test/RespTests.cs +++ b/test/Garnet.test/RespTests.cs @@ -3,14 +3,17 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; using Garnet.common; using Garnet.server; +using Newtonsoft.Json.Linq; using NUnit.Framework; using StackExchange.Redis; +using AggregateException = System.AggregateException; namespace Garnet.test { @@ -1602,6 +1605,35 @@ public async Task ReAddExpiredKey() } } + [Test] + public void MainObjectKey() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var server = redis.GetServers()[0]; + var db = redis.GetDatabase(0); + + const string key = "test:1"; + + // Do StringSet + Assert.IsTrue(db.StringSet(key, "v1")); + + // Do SetAdd using the same key + Assert.IsTrue(db.SetAdd(key, "v2")); + + // Two keys "test:1" - this is expected as of now + // because Garnet has a separate main and object store + var keys = server.Keys(db.Database, key).ToList(); + Assert.AreEqual(2, keys.Count); + Assert.AreEqual(key, (string)keys[0]); + Assert.AreEqual(key, (string)keys[1]); + + // do ListRightPush using the same key, expected error + var ex = Assert.Throws(() => db.ListRightPush(key, "v3")); + var expectedError = Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE); + Assert.IsNotNull(ex); + Assert.AreEqual(expectedError, ex.Message); + } + [Test] public void GetSliceTest() { diff --git a/test/Garnet.test/RespTestsUtils.cs b/test/Garnet.test/RespTestsUtils.cs new file mode 100644 index 0000000000..e3c60f0eb1 --- /dev/null +++ b/test/Garnet.test/RespTestsUtils.cs @@ -0,0 +1,68 @@ +using System; +using System.Linq; +using System.Text; +using Garnet.server; +using NUnit.Framework; +using StackExchange.Redis; + +namespace Garnet.test; + +public static class RespTestsUtils +{ + /// + /// Add object(s) of specified type to database using SE.Redis + /// + /// SE.Redis database wrapper + /// Object type to add + /// Array of keys to add + /// Array of arrays of values to add to each object at key + /// Throws NotSupportedException if objectType is not supported + public static void SetUpTestObjects(IDatabase db, GarnetObjectType objectType, RedisKey[] keys, RedisValue[][] values) + { + Assert.AreEqual(keys.Length, values.Length); + + switch (objectType) + { + case GarnetObjectType.Set: + for (var i = 0; i < keys.Length; i++) + { + var added = db.SetAdd(keys[i], values[i]); + Assert.AreEqual(values[i].Select(v => v.ToString()).Distinct().Count(), added); + } + break; + case GarnetObjectType.List: + for (var i = 0; i < keys.Length; i++) + { + var added = db.ListRightPush(keys[i], values[i]); + Assert.AreEqual(values[i].Length, added); + } + break; + default: + throw new NotSupportedException(); + } + } + + /// + /// Assert that calling testAction results in a wrong type error + /// + /// Action to test + public static void CheckCommandOnWrongTypeObjectSE(Action testAction) + { + var expectedError = Encoding.ASCII.GetString(CmdStrings.RESP_ERR_WRONG_TYPE); + try + { + testAction(); + Assert.Fail(); + } + catch (RedisServerException e) + { + Assert.AreEqual(expectedError, e.Message); + } + catch (AggregateException ae) + { + var rse = ae.InnerExceptions.FirstOrDefault(e => e is RedisServerException); + Assert.IsNotNull(rse); + Assert.AreEqual(expectedError, rse.Message); + } + } +} \ No newline at end of file diff --git a/test/Garnet.test/TestProcedureLists.cs b/test/Garnet.test/TestProcedureLists.cs index b190967e7a..ed1bbe8526 100644 --- a/test/Garnet.test/TestProcedureLists.cs +++ b/test/Garnet.test/TestProcedureLists.cs @@ -70,8 +70,8 @@ public override void Main(TGarnetApi api, ArgSlice input, ref Memory api.ListLength(lstKeyB, out count); if (elementPopped.Length == 0 || count != 7) result = false; - result = api.ListMove(lstKey, lstKeyB, OperationDirection.Left, OperationDirection.Right, out _); - if (result) + var status = api.ListMove(lstKey, lstKeyB, OperationDirection.Left, OperationDirection.Right, out _); + if (status == GarnetStatus.OK) { result = api.ListTrim(lstKeyB, 1, 3); }