diff --git a/docs/src/content/docs/commands/LPOP.md b/docs/src/content/docs/commands/LPOP.md index fcf6e9249..8596bf871 100644 --- a/docs/src/content/docs/commands/LPOP.md +++ b/docs/src/content/docs/commands/LPOP.md @@ -16,6 +16,8 @@ LPOP key | Parameter | Description | Type | Required | | --------- | ------------------------------------------------------------------------------ | ------ | -------- | | `key` | The key of the list from which the first element will be removed and returned. | String | Yes | +| `count` | The count of elemments to be removed. | Number | No | + ## Return Value @@ -24,14 +26,32 @@ LPOP key | Command is successful | `String` The value of the first element in the list. | | If the key does not exist | `nil` | | The key is of the wrong type | error | +| The count is invalid | error | + ## Behavior -- When the `LPOP` command is executed, DiceDB checks if the key exists and is associated with a list. -- If the list has elements, the first element is removed and returned. -- If the key does not exist, the command treats it as an empty list and returns `nil`. -- If the key exists but is not associated with a list, a `WRONGTYPE` error is returned. -- If more than one key is passed, an error is returned. +1. **Key Existence and Type**: + - If the key does not exist, the command treats it as an empty list and returns `nil`. + - If the key exists but is not associated with a list, a `WRONGTYPE` error is returned. + - If more than one key is provided, an error is returned. + +2. **Without `count` Parameter**: + - If the list has elements and the optional `count` parameter is not provided, the first element of the list is removed and returned. + +3. **With `count` Parameter**: + - If the `count` parameter is provided: + - If the list has elements, up to `count` elements are removed from the start of the list and returned in order (from the first to the last removed element). + - If `count` is greater than the number of elements in the list, all elements are removed and returned. + - If `count` is less than 0, the following error is returned: + ```bash + (error) value is out of range, must be positive + ``` + +4. **Empty List**: + - If the list is empty, the command returns `nil`. + + ## Errors @@ -104,7 +124,7 @@ Passing more than one key will result in an error: ```bash LPOP mylist secondlist -(error) ERR wrong number of arguments for 'lpop' command +(error) value is out of range, must be positive ``` ## Best Practices diff --git a/docs/src/content/docs/commands/RPOP.md b/docs/src/content/docs/commands/RPOP.md index 0fb5407cf..a58c2f570 100644 --- a/docs/src/content/docs/commands/RPOP.md +++ b/docs/src/content/docs/commands/RPOP.md @@ -1,14 +1,14 @@ --- title: RPOP -description: The `RPOP` command in DiceDB removes and returns the last element of a list. It is commonly used for processing elements in Last-In-First-Out (LIFO) order. +description: The `RPOP` command in DiceDB removes and returns elements from the end of a list. When provided with the optional `count` argument, the reply will consist of up to `count` elements, depending on the list's length. It is commonly used for processing elements in Last-In-First-Out (LIFO) order. --- -The `RPOP` command in DiceDB is used to remove and return the last element of a list. This command is useful when processing elements in a Last-In-First-Out (LIFO) order. +The `RPOP` command in DiceDB is used to remove and return elements from the end of a list. By default, it removes and returns a single element. When provided with the optional `count` argument, the reply will consist of up to `count` elements, depending on the list's length. This command is useful for processing elements in a Last-In-First-Out (LIFO) order. ## Syntax ```bash -RPOP key +RPOP key [count] ``` ## Parameters @@ -16,6 +16,7 @@ RPOP key | Parameter | Description | Type | Required | | --------- | ---------------------------------------------------------------- | ------ | -------- | | `key` | The key of the list from which the last element will be removed. | String | Yes | +| `count` | The count of elemments to be removed. | Number | No | ## Return values @@ -24,14 +25,32 @@ RPOP key | The command is successful | `String` The value of the last element in the list | | The key does not exist | `nil` | | The key is of the wrong type | error | +| The count is invalid | error | + + +#### Behavior: + +1. **Key Existence and Type**: + - If the key does not exist, the command treats it as an empty list and returns `nil`. + - If the key exists but is not associated with a list, a `WRONGTYPE` error is returned. + - If more than one key is provided, an error is returned. + +2. **Without `count` Parameter**: + - If the list has elements and the optional `count` parameter is not provided, the last element of the list is removed and returned. + +3. **With `count` Parameter**: + - If the `count` parameter is provided: + - If the list has elements, up to `count` elements are removed from the end of the list and returned in order (from the last element to the earliest removed). + - If `count` is greater than the number of elements in the list, all elements are removed and returned. + - If `count` is less than 0, the following error is returned: + ```bash + (error) value is out of range, must be positive + ``` + +4. **Empty List**: + - If the list is empty, the command returns `nil`. -## Behaviour -- When the `RPOP` command is executed, DiceDB checks if the key exists and is associated with a list. -- If the list has elements, the last element is removed and returned. -- If the key does not exist, the command treats it as an empty list and returns `nil`. -- If the key exists but is not associated with a list, a `WRONGTYPE` error is returned. -- If more than one key is passed, an error is returned. ## Errors @@ -40,19 +59,29 @@ RPOP key - Error Message: `(error) WRONGTYPE Operation against a key holding the wrong kind of value` - Occurs if the key exists but is not associated with a list. +```bash +127.0.0.1:7379> RPOP mystring +(error) WRONGTYPE Operation against a key holding the wrong kind of value +``` + 2. `Wrong number of arguments` - - Error Message: `(error) ERR wrong number of arguments for 'lpop' command` + - Error Message: `(error) ERR wrong number of arguments for 'rpop' command` - Occurs if command is executed without any arguments or with 2 or more arguments + 3. `Invalid Count` + + - Error Message: `(error) value is out of range, must be positive` + - Cause: Occurs if the count parameter is less than 0. + ## Example Usage ### Basic Usage ```bash -127.0.0.1:7379> LPUSH mylist "one" "two" "three" +LPUSH mylist "one" "two" "three" (integer) 3 -127.0.0.1:7379> RPOP mylist +RPOP mylist "one" ``` @@ -61,7 +90,7 @@ RPOP key Returns `(nil)` if the provided key is non-existent ```bash -127.0.0.1:7379> RPOP emptylist +RPOP emptylist (nil) ``` @@ -72,7 +101,7 @@ Trying to `LPOP` a key `mystring` which is holding wrong data type `string`. ```bash SET mystring "hello" OK -LPOP mystring +RPOP mystring (error) WRONGTYPE Operation against a key holding the wrong kind of value ``` @@ -82,13 +111,30 @@ Passing more than one key will result in an error: ```bash RPOP mylist secondlist -(error) ERR wrong number of arguments for 'lpop' command +(error) ERR wrong number of arguments for 'rpop' command ``` +### Invalid Count Parameter + +Passing a negative value for count will result in an error: + +```bash +RPOP mylist -1 +(error) value is out of range, must be positive +``` + +Passing a non integer value as the count paramater will result in an error: + +```bash +RPOP mylist secondlist +(error) value is out of range, must be positive +``` + + ## Best Practices - `Check Key Type`: Before using `RPOP`, ensure that the key is associated with a list to avoid errors. - `Handle Non-Existent Keys`: Be prepared to handle the case where the key does not exist, as `RPOP` will return `nil` in such scenarios. - `Use in Conjunction with Other List Commands`: The `RPOP` command is often used alongside other list commands like [`RPUSH`](/commands/rpush), [`LPUSH`](/commands/lpush), [`LLEN`](/commands/llen), and [`LPOP`](/commands/lpop) to manage and process lists effectively. -By understanding the `RPOP` command, you can effectively manage lists in DiceDB, ensuring that you can retrieve and process elements in a LIFO order. \ No newline at end of file +By understanding the `RPOP` command, you can effectively manage lists in DiceDB, ensuring that you can retrieve and process elements in a LIFO order. diff --git a/integration_tests/commands/http/deque_test.go b/integration_tests/commands/http/deque_test.go index cb1582ce7..05df37762 100644 --- a/integration_tests/commands/http/deque_test.go +++ b/integration_tests/commands/http/deque_test.go @@ -580,8 +580,8 @@ func TestLPOPCount(t *testing.T) { float64(4), []interface{}{"v1", "v2"}, []interface{}{"v3", "v4"}, - "ERR value is not an integer or out of range", - "ERR value is not an integer or a float", + "value is out of range, must be positive", + "value is out of range, must be positive", float64(0), }, }, @@ -599,3 +599,49 @@ func TestLPOPCount(t *testing.T) { exec.FireCommand(HTTPCommand{Command: "DEL", Body: map[string]interface{}{"keys": [...]string{"k"}}}) } + +func TestRPOPCount(t *testing.T) { + exec := NewHTTPCommandExecutor() + exec.FireCommand(HTTPCommand{Command: "FLUSHDB"}) + testCases := []struct { + name string + cmds []HTTPCommand + expect []any + }{ + { + name: "RPOP with count argument - valid, invalid, and edge cases", + cmds: []HTTPCommand{ + {Command: "RPUSH", Body: map[string]interface{}{"key": "k", "value": "v1"}}, + {Command: "RPUSH", Body: map[string]interface{}{"key": "k", "value": "v2"}}, + {Command: "RPUSH", Body: map[string]interface{}{"key": "k", "value": "v3"}}, + {Command: "RPUSH", Body: map[string]interface{}{"key": "k", "value": "v4"}}, + {Command: "RPOP", Body: map[string]interface{}{"key": "k", "value": 2}}, + {Command: "RPOP", Body: map[string]interface{}{"key": "k", "value": 2}}, + {Command: "RPOP", Body: map[string]interface{}{"key": "k", "value": -1}}, + {Command: "RPOP", Body: map[string]interface{}{"key": "k", "value": "abc"}}, + {Command: "LLEN", Body: map[string]interface{}{"key": "k"}}, + }, + expect: []any{ + float64(1), + float64(2), + float64(3), + float64(4), + []interface{}{"v4", "v3"}, + []interface{}{"v2", "v1"}, + "value is out of range, must be positive", + "value is out of range, must be positive", + float64(0), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + exec.FireCommand(HTTPCommand{Command: "DEL", Body: map[string]interface{}{"keys": []interface{}{"k"}}}) + for i, cmd := range tc.cmds { + result, _ := exec.FireCommand(cmd) + assert.Equal(t, tc.expect[i], result, "Value mismatch for cmd %v", cmd) + } + }) + } +} diff --git a/integration_tests/commands/resp/deque_test.go b/integration_tests/commands/resp/deque_test.go index db69db3ce..c8d55cbc9 100644 --- a/integration_tests/commands/resp/deque_test.go +++ b/integration_tests/commands/resp/deque_test.go @@ -236,12 +236,12 @@ func TestLPushRPop(t *testing.T) { { name: "LPUSH RPOP normal values", cmds: append([]string{"LPUSH k " + strings.Join(deqNormalValues, " ")}, getPops(deqNormalValues)...), - expect: append(append([]any{int64(14)}, getPopExpects(deqNormalValues)...), "(nil)"), + expect: append(append([]any{int64(len(deqNormalValues))}, getPopExpects(deqNormalValues)...), "(nil)"), }, { name: "LPUSH RPOP edge values", cmds: append([]string{"LPUSH k " + strings.Join(deqEdgeValues, " ")}, getPops(deqEdgeValues)...), - expect: append(append([]any{int64(17)}, getPopExpects(deqEdgeValues)...), "(nil)"), + expect: append(append([]any{int64(len(deqEdgeValues))}, getPopExpects(deqEdgeValues)...), "(nil)"), }, } @@ -560,8 +560,8 @@ func TestLPOPCount(t *testing.T) { int64(2), []interface{}{"v3", "v4"}, int64(0), - "ERR value is not an integer or out of range", - "ERR value is not an integer or a float", + "value is out of range, must be positive", + "value is out of range, must be positive", int64(0), }, }, diff --git a/integration_tests/commands/websocket/deque_test.go b/integration_tests/commands/websocket/deque_test.go index ff986e111..e1a2aad9a 100644 --- a/integration_tests/commands/websocket/deque_test.go +++ b/integration_tests/commands/websocket/deque_test.go @@ -485,8 +485,58 @@ func TestLPOPCount(t *testing.T) { float64(4), []interface{}{"v1", "v2"}, []interface{}{"v3", "v4"}, - "ERR value is not an integer or out of range", - "ERR value is not an integer or a float", + "value is out of range, must be positive", + "value is out of range, must be positive", + float64(0), + }, + cleanupKey: "k", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + conn := exec.ConnectToServer() + + for i, cmd := range tc.commands { + result, err := exec.FireCommandAndReadResponse(conn, cmd) + assert.NilError(t, err) + assert.DeepEqual(t, tc.expected[i], result) + } + DeleteKey(t, conn, exec, tc.cleanupKey) + }) + } +} + +func TestRPOPCount(t *testing.T) { + exec := NewWebsocketCommandExecutor() + + testCases := []struct { + name string + commands []string + expected []interface{} + cleanupKey string + }{ + { + name: "RPOP with count argument - valid, invalid, and edge cases", + commands: []string{ + "RPUSH k v1", + "RPUSH k v2", + "RPUSH k v3", + "RPUSH k v4", + "RPOP k 2", + "RPOP k 2", + "RPOP k -1", + "RPOP k abc", + "LLEN k", + }, + expected: []interface{}{ + float64(1), + float64(2), + float64(3), + float64(4), + []interface{}{"v4", "v3"}, + []interface{}{"v2", "v1"}, + "value is out of range, must be positive", + "value is out of range, must be positive", float64(0), }, cleanupKey: "k", diff --git a/internal/errors/migrated_errors.go b/internal/errors/migrated_errors.go index ea81b0c4e..0d730ca4f 100644 --- a/internal/errors/migrated_errors.go +++ b/internal/errors/migrated_errors.go @@ -13,8 +13,9 @@ import ( // Standard error variables for various DiceDB-related error conditions. var ( - ErrAuthFailed = errors.New("AUTH failed") // Indicates authentication failure. - ErrIntegerOutOfRange = errors.New("ERR value is not an integer or out of range") // Represents a value that is either not an integer or is out of allowed range. + ErrAuthFailed = errors.New("AUTH failed") // Indicates authentication failure. + ErrIntegerOutOfRange = errors.New("ERR value is not an integer or out of range") + ErrValueMustBePositive = errors.New("value is out of range, must be positive") ErrInvalidNumberFormat = errors.New("ERR value is not an integer or a float") // Signals that a value provided is not in a valid integer or float format. ErrValueOutOfRange = errors.New("ERR value is out of range") // Indicates that a value is beyond the permissible range. ErrOverflow = errors.New("ERR increment or decrement would overflow") // Signifies that an increment or decrement operation would exceed the limits. diff --git a/internal/eval/eval_test.go b/internal/eval/eval_test.go index 41b0088df..58a37ea04 100644 --- a/internal/eval/eval_test.go +++ b/internal/eval/eval_test.go @@ -3957,11 +3957,11 @@ func testEvalLPOP(t *testing.T, store *dstore.Store) { }, "non-integer count": { input: []string{"k", "abc"}, - migratedOutput: EvalResponse{Result: nil, Error: errors.New("ERR value is not an integer or a float")}, + migratedOutput: EvalResponse{Result: nil, Error: errors.New("value is out of range, must be positive")}, }, "negative count": { input: []string{"k", "-1"}, - migratedOutput: EvalResponse{Result: nil, Error: errors.New("ERR value is not an integer or out of range")}, + migratedOutput: EvalResponse{Result: nil, Error: errors.New("value is out of range, must be positive")}, }, "key with different type": { setup: func() { @@ -4031,10 +4031,18 @@ func testEvalRPOP(t *testing.T, store *dstore.Store) { input: []string{}, migratedOutput: EvalResponse{Result: nil, Error: errors.New("ERR wrong number of arguments for 'rpop' command")}, }, - "wrong number of args": { - input: []string{"KEY1", "KEY2"}, + "more than 2 args": { + input: []string{"k", "2", "3"}, migratedOutput: EvalResponse{Result: nil, Error: errors.New("ERR wrong number of arguments for 'rpop' command")}, }, + "non-integer count": { + input: []string{"k", "abc"}, + migratedOutput: EvalResponse{Result: nil, Error: errors.New("value is out of range, must be positive")}, + }, + "negative count": { + input: []string{"k", "-1"}, + migratedOutput: EvalResponse{Result: nil, Error: errors.New("value is out of range, must be positive")}, + }, "key with different type": { setup: func() { evalSET([]string{"EXISTING_KEY", "mock_value"}, store) @@ -4061,6 +4069,35 @@ func testEvalRPOP(t *testing.T, store *dstore.Store) { input: []string{"EXISTING_KEY"}, migratedOutput: EvalResponse{Result: "value_4", Error: nil}, }, + "pop one element": { + setup: func() { + evalRPUSH([]string{"k", "v1", "v2", "v3", "v4"}, store) + }, + input: []string{"k"}, + migratedOutput: EvalResponse{Result: "v4", Error: nil}, + }, + "pop two elements": { + setup: func() { + evalRPUSH([]string{"k", "v1", "v2", "v3", "v4"}, store) + }, + input: []string{"k", "2"}, + migratedOutput: EvalResponse{Result: []string{"v3", "v4"}, Error: nil}, + }, + "pop more elements than available": { + setup: func() { + evalRPUSH([]string{"k", "v1", "v2"}, store) + }, + input: []string{"k", "5"}, + migratedOutput: EvalResponse{Result: []string{"v1", "v2"}, Error: nil}, + }, + "pop 0 elements": { + + setup: func() { + evalRPUSH([]string{"k", "v1", "v2"}, store) + }, + input: []string{"k", "0"}, + migratedOutput: EvalResponse{Result: clientio.NIL, Error: nil}, + }, } runMigratedEvalTests(t, tests, evalRPOP, store) } diff --git a/internal/eval/store_eval.go b/internal/eval/store_eval.go index 015d20b00..ce415ef2e 100644 --- a/internal/eval/store_eval.go +++ b/internal/eval/store_eval.go @@ -3182,21 +3182,14 @@ func evalRPUSH(args []string, store *dstore.Store) *EvalResponse { } } -// evalLPOP pops the element at the head of the list and returns it -// -// # Returns the element at the head of the list -// -// Usage: LPOP key -func evalLPOP(args []string, store *dstore.Store) *EvalResponse { - // By default we pop only 1 +// Pop[s] elements either from the left or right based on direction +func popElements(args []string, store *dstore.Store, direction string) *EvalResponse { popNumber := 1 - // LPOP accepts 1 or 2 arguments only - LPOP key [count] - if len(args) < 1 || len(args) > 2 { return &EvalResponse{ Result: nil, - Error: diceerrors.ErrWrongArgumentCount("LPOP"), + Error: diceerrors.ErrWrongArgumentCount(direction), } } @@ -3205,21 +3198,19 @@ func evalLPOP(args []string, store *dstore.Store) *EvalResponse { if err != nil { return &EvalResponse{ Result: nil, - Error: diceerrors.ErrInvalidNumberFormat, + Error: diceerrors.ErrValueMustBePositive, } } if nos == 0 { - // returns empty string if count given is 0 return &EvalResponse{ Result: clientio.NIL, Error: nil, } } if nos < 0 { - // returns an out of range err if count is negetive return &EvalResponse{ Result: nil, - Error: diceerrors.ErrIntegerOutOfRange, + Error: diceerrors.ErrValueMustBePositive, } } popNumber = nos @@ -3242,10 +3233,15 @@ func evalLPOP(args []string, store *dstore.Store) *EvalResponse { deq := obj.Value.(*Deque) - // holds the elements popped var elements []string + var err error for iter := 0; iter < popNumber; iter++ { - x, err := deq.LPop() + var x string + if direction == "LPOP" { + x, err = deq.LPop() + } else { + x, err = deq.RPop() + } if err != nil { if errors.Is(err, ErrDequeEmpty) { break @@ -3274,49 +3270,22 @@ func evalLPOP(args []string, store *dstore.Store) *EvalResponse { } } +// evalLPOP pops the element at the head of the list and returns it +// +// # Returns the element at the head of the list +// +// Usage: LPOP key +func evalLPOP(args []string, store *dstore.Store) *EvalResponse { + return popElements(args, store, "LPOP") +} + // evalRPOP pops the element at the tail of the list and returns it // // # Returns the element at the tail of the list // // Usage: RPOP key func evalRPOP(args []string, store *dstore.Store) *EvalResponse { - if len(args) != 1 { - return &EvalResponse{ - Result: nil, - Error: diceerrors.ErrWrongArgumentCount("RPOP"), - } - } - - obj := store.Get(args[0]) - if obj == nil { - return &EvalResponse{ - Result: clientio.NIL, - Error: nil, - } - } - - if err := object.AssertType(obj.Type, object.ObjTypeDequeue); err != nil { - return &EvalResponse{ - Result: nil, - Error: diceerrors.ErrWrongTypeOperation, - } - } - - deq := obj.Value.(*Deque) - x, err := deq.RPop() - if err != nil { - if errors.Is(err, ErrDequeEmpty) { - return &EvalResponse{ - Result: clientio.NIL, - Error: nil, - } - } - } - - return &EvalResponse{ - Result: x, - Error: nil, - } + return popElements(args, store, "RPOP") } // evalLLEN returns the number of elements in the list