#1100: Fix LPOP to support multiple arguments (#1231)

integration_tests/commands/async/deque_test.go

@@ -121,17 +121,17 @@ func TestRPush(t *testing.T) {
 	}{
 		{
 			name:   "RPUSH",
-			cmds:   []string{"LPUSH k v", "LPUSH k v1 1 v2 2", "LPUSH k 3 3 3 v3 v3 v3"},
+			cmds:   []string{"RPUSH k v", "RPUSH k v1 1 v2 2", "RPUSH k 3 3 3 v3 v3 v3"},
 			expect: []any{int64(1), int64(5), int64(11)},
 		},
 		{
 			name:   "RPUSH normal values",
-			cmds:   []string{"LPUSH k " + strings.Join(deqNormalValues, " ")},
+			cmds:   []string{"RPUSH k " + strings.Join(deqNormalValues, " ")},
 			expect: []any{int64(25)},
 		},
 		{
 			name:   "RPUSH edge values",
-			cmds:   []string{"LPUSH k " + strings.Join(deqEdgeValues, " ")},
+			cmds:   []string{"RPUSH k " + strings.Join(deqEdgeValues, " ")},
 			expect: []any{int64(42)},
 		},
 	}
@@ -442,6 +442,58 @@ func TestLLEN(t *testing.T) {
 	deqCleanUp(conn, "k")
 }
 
+func TestLPOPCount(t *testing.T) {
+	deqTestInit()
+	conn := getLocalConnection()
+	defer conn.Close()
+
+	testCases := []struct {
+		name   string
+		cmds   []string
+		expect []interface{}
+	}{
+		{
+			name: "LPOP with count argument - valid, invalid, and edge cases",
+			cmds: []string{
+				"RPUSH k v1 v2 v3 v4", 
+				"LPOP k 2",            
+                "LLEN k",
+				"LPOP k 0",            
+                "LLEN k",
+				"LPOP k 5",            
+                "LLEN k",
+				"LPOP k -1",           
+				"LPOP k abc",          
+				"LLEN k",              
+			},
+			expect: []any{
+				int64(4),                  
+				[]interface{}{"v1", "v2"}, 
+                int64(2),
+				[]interface{}{},           
+                int64(2),
+				[]interface{}{"v3", "v4"}, 
+                int64(0),
+				"ERR value is out of range",                   
+				"ERR value is not an integer or out of range", 
+				int64(0),                                      
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			for i, cmd := range tc.cmds {
+				result := FireCommand(conn, cmd)
+				assert.Equal(t, tc.expect[i], result)
+			}
+		})
+	}
+
+	deqCleanUp(conn, "k")
+
+}
+
 func deqCleanUp(conn net.Conn, key string) {
 	for {
 		result := FireCommand(conn, "LPOP "+key)

integration_tests/commands/http/deque_test.go

@@ -556,3 +556,51 @@ func TestLLEN(t *testing.T) {
 
 	exec.FireCommand(HTTPCommand{Command: "DEL", Body: map[string]interface{}{"keys": [...]string{"k"}}})
 }
+
+func TestLPOPCount(t *testing.T) {
+	deqTestInit()
+	exec := NewHTTPCommandExecutor()
+
+	testCases := []struct {
+		name   string
+		cmds   []HTTPCommand
+		expect []any
+	}{
+		{
+			name: "LPOP 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: "LPOP", Body: map[string]interface{}{"key": "k", "value": 2}}, 
+				{Command: "LPOP", Body: map[string]interface{}{"key": "k", "value": 2}}, 
+				{Command: "LPOP", Body: map[string]interface{}{"key": "k", "value": -1}}, 
+				{Command: "LPOP", 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{}{"v1", "v2"},
+				[]interface{}{"v3", "v4"},
+				"ERR value is out of range",                   
+				"ERR value is not an integer or out of range", 
+                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)
+			}
+		})
+	}
+}
+

integration_tests/commands/websocket/deque_test.go

@@ -0,0 +1,58 @@
+package websocket
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLPOPCount(t *testing.T) {
+	exec := NewWebsocketCommandExecutor()
+
+	testCases := []struct {
+		name       string
+		commands   []string
+		expected   []interface{}
+		cleanupKey string
+	}{
+		{
+			name: "LPOP with count argument - valid, invalid, and edge cases",
+			commands: []string{
+				"RPUSH k v1",
+				"RPUSH k v2",
+				"RPUSH k v3",
+				"RPUSH k v4",
+				"LPOP k 2",
+				"LPOP k 2",
+				"LPOP k -1",
+				"LPOP k abc",
+				"LLEN k",
+			},
+			expected: []any{
+				float64(1),
+				float64(2),
+				float64(3),
+				float64(4),
+				[]interface{}{"v1", "v2"},
+				[]interface{}{"v3", "v4"},
+				"ERR value is out of range",
+				"ERR value is not an integer or out of range",
+				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.Nil(t, err)
+				assert.Equal(t, tc.expected[i], result)
+			}
+			DeleteKey(t, conn, exec, tc.cleanupKey)
+		})
+	}
+}
+

internal/eval/eval.go

@@ -2157,10 +2157,31 @@ func evalRPOP(args []string, store *dstore.Store) []byte {
 }
 
 func evalLPOP(args []string, store *dstore.Store) []byte {
-	if len(args) != 1 {
+	// By default we pop only 1
+	popNumber := 1
+
+	// LPOP accepts 1 or 2 arguments only - LPOP key [count]
+	if len(args) < 1 || len(args) > 2 {
 		return diceerrors.NewErrArity("LPOP")
 	}
 
+	// to updated the number of pops
+	if len(args) == 2 {
+		nos, err := strconv.Atoi(args[1])
+		if err != nil {
+			return diceerrors.NewErrWithFormattedMessage(diceerrors.IntOrOutOfRangeErr)
+		}
+		if nos == 0 {
+			// returns empty string if count given is 0
+			return clientio.Encode([]string{}, false)
+		}
+		if nos < 0 {
+			// returns an out of range err if count is negetive
+			return diceerrors.NewErrWithFormattedMessage(diceerrors.ValOutOfRangeErr)
+		}
+		popNumber = nos
+	}
+
 	obj := store.Get(args[0])
 	if obj == nil {
 		return clientio.RespNIL
@@ -2180,15 +2201,29 @@ func evalLPOP(args []string, store *dstore.Store) []byte {
 	}
 
 	deq := obj.Value.(*Deque)
-	x, err := deq.LPop()
-	if err != nil {
-		if errors.Is(err, ErrDequeEmpty) {
-			return clientio.RespNIL
+
+	// holds the elements popped
+	var elements []string
+	for iter := 0; iter < popNumber; iter++ {
+		x, err := deq.LPop()
+		if err != nil {
+			if errors.Is(err, ErrDequeEmpty) {
+				break
+			}
+			panic(fmt.Sprintf("unknown error: %v", err))
 		}
-		panic(fmt.Sprintf("unknown error: %v", err))
+		elements = append(elements, x)
 	}
 
-	return clientio.Encode(x, false)
+	if len(elements) == 0 {
+		return clientio.RespNIL
+	}
+
+	if len(elements) == 1 {
+		return clientio.Encode(elements[0], false)
+	}
+
+	return clientio.Encode(elements, false)
 }
 
 func evalLLEN(args []string, store *dstore.Store) []byte {

internal/eval/eval_test.go

@@ -86,12 +86,14 @@ func TestEval(t *testing.T) {
 	testEvalHEXISTS(t, store)
 	testEvalHDEL(t, store)
 	testEvalHSCAN(t, store)
+	testEvalPFMERGE(t, store)
 	testEvalJSONSTRLEN(t, store)
 	testEvalJSONOBJLEN(t, store)
 	testEvalHLEN(t, store)
 	testEvalSELECT(t, store)
 	testEvalLLEN(t, store)
 	testEvalGETDEL(t, store)
+	testEvalGETEX(t, store)
 	testEvalJSONNUMINCRBY(t, store)
 	testEvalDUMP(t, store)
 	testEvalTYPE(t, store)
@@ -118,6 +120,8 @@ func TestEval(t *testing.T) {
 	testEvalZRANK(t, store)
 	testEvalZCARD(t, store)
 	testEvalZREM(t, store)
+	testEvalZADD(t, store)
+	testEvalZRANGE(t, store)
 	testEvalHVALS(t, store)
 	testEvalBitField(t, store)
 	testEvalHINCRBYFLOAT(t, store)
@@ -137,6 +141,7 @@ func TestEval(t *testing.T) {
 	testEvalBFADD(t, store)
 	testEvalLINSERT(t, store)
 	testEvalLRANGE(t, store)
+	testEvalLPOP(t, store)
 }
 
 func testEvalPING(t *testing.T, store *dstore.Store) {
@@ -3779,6 +3784,57 @@ func testEvalLLEN(t *testing.T, store *dstore.Store) {
 	runEvalTests(t, tests, evalLLEN, store)
 }
 
+func testEvalLPOP(t *testing.T, store *dstore.Store) {
+	tests := map[string]evalTestCase{
+		"empty args": {
+			input:  nil,
+			output: []byte("-ERR wrong number of arguments for 'lpop' command\r\n"),
+		},
+		"more than 2 args": {
+			input:  []string{"k", "2", "3"},
+			output: []byte("-ERR wrong number of arguments for 'lpop' command\r\n"),
+		},
+		"pop one element": {
+			setup: func() {
+				evalRPUSH([]string{"k", "v1", "v2", "v3", "v4"}, store)
+			},
+			input:  []string{"k"},
+			output: []byte("$2\r\nv1\r\n"),
+		},
+		"pop two elements": {
+			setup: func() {
+				evalRPUSH([]string{"k", "v1", "v2", "v3", "v4"}, store)
+			},
+			input:  []string{"k", "2"},
+			output: []byte("*2\r\n$2\r\nv1\r\n$2\r\nv2\r\n")},
+		"pop more elements than available": {
+			setup: func() {
+				evalRPUSH([]string{"k", "v1", "v2"}, store)
+			},
+			input:  []string{"k", "5"},
+			output: []byte("*2\r\n$2\r\nv1\r\n$2\r\nv2\r\n")},
+		"pop 0 elements": {
+			setup: func() {
+				evalRPUSH([]string{"k", "v1", "v2"}, store)
+			},
+			input:  []string{"k", "0"},
+			output: []byte("*0\r\n")},
+		"negative count": {
+			input:  []string{"k", "-1"},
+			output: []byte("-ERR value is out of range\r\n"),
+		},
+		"non-integer count": {
+			input:  []string{"k", "abc"},
+			output: []byte("-ERR value is not an integer or out of range\r\n"),
+		},
+		"key does not exist": {
+			input:  []string{"nonexistent_key"},
+			output: []byte("$-1\r\n"),
+		},
+	}
+	runEvalTests(t, tests, evalLPOP, store)
+}
+
 func testEvalJSONNUMINCRBY(t *testing.T, store *dstore.Store) {
 	tests := map[string]evalTestCase{
 		"incr on numeric field": {