Redis doesn't do the BLMOVE check when the initial key does not exist.

Author: prvyk

libs/server/Objects/ItemBroker/CollectionItemBroker.cs

@@ -436,6 +436,10 @@ private static unsafe bool TryGetNextSetObjects(byte[] key, SortedSetObject sort
             }
         }
 
+        /// <summary>
+        /// Try to get available item(s) from sorted set object based on command type and arguments
+        /// When run from initializer (initial = true), can return WRONGTYPE errors
+        /// </summary>
         private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                                          RespCommand command, ArgSlice[] cmdArgs, bool initial,
                                          out int currCount, out CollectionItemResult result)
@@ -451,13 +455,13 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                 _ => throw new NotSupportedException()
             };
 
+            var asKey = storageSession.scratchBufferManager.CreateArgSlice(key);
             ArgSlice dstKey = default;
             if (command == RespCommand.BLMOVE)
             {
                 dstKey = cmdArgs[0];
             }
 
-            var asKey = storageSession.scratchBufferManager.CreateArgSlice(key);
             // Create a transaction if not currently in a running transaction
             if (storageSession.txnManager.state != TxnState.Running)
             {
@@ -479,8 +483,6 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
 
             var lockableContext = storageSession.txnManager.LockableContext;
             var objectLockableContext = storageSession.txnManager.ObjectStoreLockableContext;
-            IGarnetObject dstObj = null;
-            byte[] arrDstKey = default;
 
             try
             {
@@ -491,42 +493,18 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                     if (!initial)
                         return false;
 
+                    // Check the string store as well to see if WRONGTYPE should be returned.
                     statusOp = storageSession.GET(asKey, out ArgSlice _, ref lockableContext);
 
                     if (statusOp != GarnetStatus.NOTFOUND)
                     {
                         result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
-                        return true;
-                    }
-
-                    if (command == RespCommand.BLMOVE)
-                    {
-                        arrDstKey = dstKey.ToArray();
-                        var dstStatusOp = storageSession.GET(arrDstKey, out var osDstObject, ref objectLockableContext);
-                        if (dstStatusOp != GarnetStatus.NOTFOUND && osDstObject.GarnetObject is not ListObject)
-                        {
-                            result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
-                            return true;
-                        }
-
-                        dstStatusOp = storageSession.GET(dstKey, out ArgSlice _, ref lockableContext);
-                        if (dstStatusOp != GarnetStatus.NOTFOUND)
-                        {
-                            result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
-                            return true;
-                        }
+                        return initial;
                     }
 
                     return false;
                 }
 
-                if (command == RespCommand.BLMOVE)
-                {
-                    arrDstKey = dstKey.ToArray();
-                    var dstStatusOp = storageSession.GET(arrDstKey, out var osDstObject, ref objectLockableContext);
-                    if (dstStatusOp != GarnetStatus.NOTFOUND) dstObj = osDstObject.GarnetObject;
-                }
-
                 // Check for type match between the observer and the actual object type
                 // If types match, get next item based on item type
                 switch (osObject.GarnetObject)
@@ -550,21 +528,46 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                                 result = new CollectionItemResult(key, nextItem);
                                 break;
                             case RespCommand.BLMOVE:
+                                var arrDstKey = dstKey.ToArray();
+                                var dstStatusOp = storageSession.GET(arrDstKey, out var osDstObject, ref objectLockableContext);
+
                                 ListObject dstList;
                                 var newObj = false;
-                                if (dstObj == null)
-                                {
-                                    dstList = new ListObject();
-                                    newObj = true;
-                                }
-                                else if (dstObj is ListObject tmpDstList)
+
+                                if (dstStatusOp != GarnetStatus.NOTFOUND)
                                 {
-                                    dstList = tmpDstList;
+                                    var dstObj = osDstObject.GarnetObject;
+
+                                    if (dstObj == null)
+                                    {
+                                        dstList = new ListObject();
+                                        newObj = true;
+                                    }
+                                    else if (dstObj is ListObject tmpDstList)
+                                    {
+                                        dstList = tmpDstList;
+                                    }
+                                    else
+                                    {
+                                        result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
+                                        return initial;
+                                    }
                                 }
                                 else
                                 {
-                                    result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
-                                    return initial;
+                                    if (initial)
+                                    {
+                                        // Check string store for wrongtype errors on initial run.
+                                        dstStatusOp = storageSession.GET(dstKey, out ArgSlice _, ref lockableContext);
+                                        if (dstStatusOp != GarnetStatus.NOTFOUND)
+                                        {
+                                            result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
+                                            return initial;
+                                        }
+                                    }
+
+                                    dstList = new ListObject();
+                                    newObj = true;
                                 }
 
                                 isSuccessful = TryMoveNextListItem(listObj, dstList, (OperationDirection)cmdArgs[1].ReadOnlySpan[0],

test/Garnet.test/RespBlockingCollectionTests.cs

@@ -656,16 +656,16 @@ public void PopWrongTypeTest()
             response = lightClientRequest.SendCommand("BLMOVE set foo RIGHT RIGHT 0");
             TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
 
-            response = lightClientRequest.SendCommand("BLMOVE foo key LEFT LEFT 0");
+            response = lightClientRequest.SendCommand("BLMOVE list set LEFT LEFT 0");
             TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
 
-            response = lightClientRequest.SendCommand("BLMOVE foo set LEFT LEFT 0");
+            response = lightClientRequest.SendCommand("BLMOVE list key LEFT LEFT 0");
             TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
 
             response = lightClientRequest.SendCommand("BRPOPLPUSH key foo 0");
             TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
 
-            response = lightClientRequest.SendCommand("BRPOPLPUSH foo key 0");
+            response = lightClientRequest.SendCommand("BRPOPLPUSH list key 0");
             TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
 
             response = lightClientRequest.SendCommand("BLPOP key 0");