[Compatibility] Added BRPOPLPUSH, ZREVRANGEBYLEX deprecated commands by mapping to existing commands #769
Conversation
Vijay-Nirmal
changed the title
| var rightOption = ArgSlice.FromPinnedSpan(CmdStrings.RIGHT); | ||
| var leftOption = ArgSlice.FromPinnedSpan(CmdStrings.LEFT); | ||
| var timeout = parseState.GetArgSliceByRef(2); | ||
| parseState.InitializeWithArguments(srcKey, dstKey, rightOption, leftOption, timeout); |
There was a problem hiding this comment.
Why create a new parse state with the direction arguments and then call ListBlockingMove to reparse them once again? Just create the cmdArgs array and call the itemBroker directly. Alternatively, you can create a common method that takes the 5 arguments.
There was a problem hiding this comment.
Ok I will do it, I did it in this way because there are deprecated commands and the only purpose of adding these is additioning compatibility still there commands are removed as discussed in this GitHub command #684 (comment)
There was a problem hiding this comment.
I realize these are deprecated commands. Still, it's better to have good practices throughout so that other contributors can use them as reference :)
| // ZREVRANGEBYLEX key start stop | ||
| if (parseState.Count == 3) | ||
| { | ||
| parseState.InitializeWithArguments(sbKey, start, stop, byLenOption, revOption); |
There was a problem hiding this comment.
Again, no need to initialize a new parse state, actually no need for this method at all, just create a new SortedSetOperation - ZREVRANGEBYLEX and call SortedSetRange directy, then handle that specific op in SortedSetObjectImpl.
| @@ -231,5 +231,25 @@ public void BlockingClientEventsTests(string blockingCmd) | |||
| ClassicAssert.IsTrue(blockingTask.IsCompletedSuccessfully); | |||
| ClassicAssert.IsTrue(releasingTask.IsCompletedSuccessfully); | |||
| } | |||
|
|
|||
| [Test] | |||
| public void BasicBlockingListPopPushTest() | |||
There was a problem hiding this comment.
This doesn't really test the blocking functionality, you can piggyback on the BasicBlockingListMoveTest and just add a test case for BRPOPLPUSH.
| #region ZREVRANGEBYLEX | ||
|
|
||
| [Test] | ||
| [TestCase("(a", "(a", new string[] { })] |
There was a problem hiding this comment.
Perhaps add some more test cases?
There was a problem hiding this comment.
I have mentioned the reason in the PR description
Note: ZRANGE with REV command has a lot of issues. Also, all the causes are failing. I will have a separate PR to fix it. This is the reason I couldn't write test causes for ZREVRANGEBYLEX command with any of the valid cases.
Adding the BRPOPLPUSH and ZREVRANGEBYLEX commands to garnet and added doc to SLAVEOF command
This PR closes #684
Note: ZRANGE with REV command has a lot of issues. Also, all the causes are failing. I will have a separate PR to fix it. This is the reason I couldn't write test causes for ZREVRANGEBYLEX command with any of the valid cases.