Remove GetOperationDirection entirely and use sessionparsestate extension.

Author: prvyk

libs/server/Resp/Objects/ListCommands.cs

@@ -226,10 +226,7 @@ private unsafe bool ListPopMultiple<TGarnetApi>(ref TGarnetApi storageApi)
             }
 
             // Get the direction
-            var dir = parseState.GetArgSliceByRef(currTokenId++);
-            var popDirection = GetOperationDirection(dir);
-
-            if (popDirection == OperationDirection.Unknown)
+            if (!parseState.TryGetOperationDirection(currTokenId++, out var popDirection))
             {
                 return AbortWithErrorMessage(CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
             }
@@ -355,8 +352,13 @@ private unsafe bool ListBlockingMove()
 
             var srcKey = parseState.GetArgSliceByRef(0);
             var dstKey = parseState.GetArgSliceByRef(1);
-            var srcDir = parseState.GetArgSliceByRef(2);
-            var dstDir = parseState.GetArgSliceByRef(3);
+
+            if (!parseState.TryGetOperationDirection(2, out var srcDir) ||
+                !parseState.TryGetOperationDirection(3, out var dstDir)
+                )
+            {
+                return AbortWithErrorMessage(CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
+            }
 
             if (!parseState.TryGetTimeout(4, out var timeout, out var error))
             {
@@ -379,27 +381,26 @@ private bool ListBlockingPopPush()
 
             var srcKey = parseState.GetArgSliceByRef(0);
             var dstKey = parseState.GetArgSliceByRef(1);
-            var rightOption = ArgSlice.FromPinnedSpan(CmdStrings.RIGHT);
-            var leftOption = ArgSlice.FromPinnedSpan(CmdStrings.LEFT);
 
             if (!parseState.TryGetTimeout(2, out var timeout, out var error))
             {
                 return AbortWithErrorMessage(error);
             }
 
-            return ListBlockingMove(srcKey, dstKey, rightOption, leftOption, timeout);
+            return ListBlockingMove(srcKey, dstKey, OperationDirection.Right,
+                                    OperationDirection.Left, timeout);
         }
 
-        private bool ListBlockingMove(ArgSlice srcKey, ArgSlice dstKey, ArgSlice srcDir, ArgSlice dstDir, double timeout)
+        private bool ListBlockingMove(ArgSlice srcKey, ArgSlice dstKey,
+                                      OperationDirection sourceDirection,
+                                      OperationDirection destinationDirection,
+                                      double timeout)
         {
             var cmdArgs = new ArgSlice[] { default, default, default };
 
             // Read destination key
             cmdArgs[0] = dstKey;
 
-            var sourceDirection = GetOperationDirection(srcDir);
-            var destinationDirection = GetOperationDirection(dstDir);
-
             if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown)
             {
                 return AbortWithErrorMessage(CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
@@ -770,13 +771,9 @@ private bool ListMove<TGarnetApi>(ref TGarnetApi storageApi)
             var srcKey = parseState.GetArgSliceByRef(0);
             var dstKey = parseState.GetArgSliceByRef(1);
 
-            var srcDirSlice = parseState.GetArgSliceByRef(2);
-            var dstDirSlice = parseState.GetArgSliceByRef(3);
-
-            var sourceDirection = GetOperationDirection(srcDirSlice);
-            var destinationDirection = GetOperationDirection(dstDirSlice);
-
-            if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown)
+            if (!parseState.TryGetOperationDirection(2, out var sourceDirection) ||
+                !parseState.TryGetOperationDirection(3, out var destinationDirection)
+                )
             {
                 return AbortWithErrorMessage(CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
             }
@@ -964,10 +961,7 @@ private unsafe bool ListBlockingPopMultiple()
             var cmdArgs = new ArgSlice[2];
 
             // Get the direction
-            var dir = parseState.GetArgSliceByRef(currTokenId++);
-            var popDirection = GetOperationDirection(dir);
-
-            if (popDirection == OperationDirection.Unknown)
+            if (!parseState.TryGetOperationDirection(currTokenId++, out var popDirection))
             {
                 return AbortWithErrorMessage(CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
             }

libs/server/Resp/Objects/ObjectStoreUtils.cs

@@ -99,27 +99,5 @@ private bool AbortWithErrorMessage(string format, params object[] args)
         {
             return AbortWithErrorMessage(Encoding.ASCII.GetBytes(string.Format(format, args)));
         }
-
-        /// <summary>
-        /// Tries to parse the input as "LEFT" or "RIGHT" and returns the corresponding OperationDirection.
-        /// If parsing fails, returns OperationDirection.Unknown.
-        /// </summary>
-        /// <param name="input">The input to parse.</param>
-        /// <returns>The parsed OperationDirection, or OperationDirection.Unknown if parsing fails.</returns>
-        public OperationDirection GetOperationDirection(ArgSlice input)
-        {
-            // Optimize for the common case
-            if (input.ReadOnlySpan.SequenceEqual("LEFT"u8))
-                return OperationDirection.Left;
-            if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8))
-                return OperationDirection.Right;
-            // Rare case: try making upper case and retry
-            MakeUpperCase(input.ptr, input.length);
-            if (input.ReadOnlySpan.SequenceEqual("LEFT"u8))
-                return OperationDirection.Left;
-            if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8))
-                return OperationDirection.Right;
-            return OperationDirection.Unknown;
-        }
     }
 }

libs/server/SessionParseStateExtensions.cs

@@ -614,6 +614,21 @@ internal static bool TryGetManagerType(this SessionParseState parseState, int id
             return true;
         }
 
+        internal static bool TryGetOperationDirection(this SessionParseState parseState, int idx, out OperationDirection value)
+        {
+            value = OperationDirection.Unknown;
+            var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan;
+
+            if (sbArg.EqualsUpperCaseSpanIgnoringCase("LEFT"u8))
+                value = OperationDirection.Left;
+            else if (sbArg.EqualsUpperCaseSpanIgnoringCase("RIGHT"u8))
+                value = OperationDirection.Right;
+            else
+                return false;
+
+            return true;
+        }
+
         /// <summary>
         /// Parse sorted set add option from parse state at specified index
         /// </summary>

libs/server/Transaction/TxnKeyManager.cs

@@ -378,6 +378,7 @@ private int ListObjectKeys(RespCommand command)
                 RespCommand.LINSERT => SingleKey(StoreType.Object, LockType.Exclusive),
                 RespCommand.LLEN => SingleKey(StoreType.Object, LockType.Shared),
                 RespCommand.LMOVE => ListKeys(2, StoreType.Object, LockType.Exclusive),
+                // Account for mandatory LEFT/RIGHT argument
                 RespCommand.LMPOP => ListReadKeysWithCount(LockType.Exclusive, mandatoryArgs: 1),
                 RespCommand.LPOP => SingleKey(StoreType.Object, LockType.Exclusive),
                 RespCommand.LPOS => SingleKey(StoreType.Object, LockType.Shared),