DiceDB#603: BugFix : Handle root path '.' correctly in JSON.OBJLEN co…

…mmand (DiceDB#976)
iamskp11 · Oct 20, 2024 · a18329e · a18329e
1 parent 219f18c
commit a18329e
Show file tree

Hide file tree

Showing 6 changed files with 193 additions and 1 deletion.
diff --git a/integration_tests/commands/async/json_test.go b/integration_tests/commands/async/json_test.go
@@ -1053,6 +1053,41 @@ func TestJsonObjLen(t *testing.T) {
 			commands: []string{"json.set obj $ " + c, "json.objlen"},
 			expected: []interface{}{"OK", "ERR wrong number of arguments for 'json.objlen' command"},
 		},
+		{
+			name:     "JSON.OBJLEN with legacy path - root",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj ."},
+			expected: []interface{}{"OK", int64(3)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .partner", "json.objlen obj .partner2",},
+			expected: []interface{}{"OK", int64(2), int64(2)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path v2",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj partner", "json.objlen obj partner2",},
+			expected: []interface{}{"OK", int64(2), int64(2)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .idonotexist",},
+			expected: []interface{}{"OK", "(nil)"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path v2",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj idonotexist",},
+			expected: []interface{}{"OK", "(nil)"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path with nonJSON object",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .name",},
+			expected: []interface{}{"OK", "WRONGTYPE Operation against a key holding the wrong kind of value"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path recursive object",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj ..partner",},
+			expected: []interface{}{"OK", int64(2)},
+		},
 	}
 
 	for _, tcase := range testCases {

diff --git a/integration_tests/commands/http/json_test.go b/integration_tests/commands/http/json_test.go
@@ -1069,6 +1069,55 @@ func TestJsonObjLen(t *testing.T) {
 			},
 			expected: []interface{}{"ERR Path '$[1' does not exist"},
 		},
+		{
+			name:     "JSON.OBJLEN with legacy path - root",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": "."}},
+			},
+			expected: []interface{}{3.0},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": ".partner2"}},
+			},
+			expected: []interface{}{2.0},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path v2",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": "partner"}},
+			},
+			expected: []interface{}{2.0},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": ".idonotexist"}},
+			},
+			expected: []interface{}{nil},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path v2",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": "idonotexist"}},
+			},
+			expected: []interface{}{nil},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path with nonJSON object",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": ".name"}},
+			},
+			expected: []interface{}{"WRONGTYPE Operation against a key holding the wrong kind of value"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path recursive object",
+			commands: []HTTPCommand{
+				{Command: "json.objlen", Body: map[string]interface{}{"key": "c", "path": "..partner"}},
+			},
+			expected: []interface{}{2.0},
+		},
 	}
 
 	for _, tc := range testCases {

diff --git a/integration_tests/commands/resp/json_test.go b/integration_tests/commands/resp/json_test.go
@@ -240,6 +240,41 @@ func TestJsonObjLen(t *testing.T) {
 			commands: []string{"json.set obj $ " + c, "json.objlen"},
 			expected: []interface{}{"OK", "ERR wrong number of arguments for 'json.objlen' command"},
 		},
+		{
+			name:     "JSON.OBJLEN with legacy path - root",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj ."},
+			expected: []interface{}{"OK", int64(3)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .partner", "json.objlen obj .partner2",},
+			expected: []interface{}{"OK", int64(2), int64(2)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path v2",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj partner", "json.objlen obj partner2",},
+			expected: []interface{}{"OK", int64(2), int64(2)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .idonotexist",},
+			expected: []interface{}{"OK", "(nil)"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path v2",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj idonotexist",},
+			expected: []interface{}{"OK", "(nil)"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path with nonJSON object",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .name",},
+			expected: []interface{}{"OK", "WRONGTYPE Operation against a key holding the wrong kind of value"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path recursive object",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj ..partner",},
+			expected: []interface{}{"OK", int64(2)},
+		},
 	}
 
 	for _, tcase := range testCases {

diff --git a/integration_tests/commands/websocket/json_test.go b/integration_tests/commands/websocket/json_test.go
@@ -262,6 +262,41 @@ func TestJsonObjLen(t *testing.T) {
 			commands: []string{"json.set obj $ " + c, "json.objlen"},
 			expected: []interface{}{"OK", "ERR wrong number of arguments for 'json.objlen' command"},
 		},
+		{
+			name:     "JSON.OBJLEN with legacy path - root",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj ."},
+			expected: []interface{}{"OK", float64(3)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .partner", "json.objlen obj .partner2",},
+			expected: []interface{}{"OK", float64(2), float64(2)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existing path v2",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj partner", "json.objlen obj partner2",},
+			expected: []interface{}{"OK", float64(2), float64(2)},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .idonotexist",},
+			expected: []interface{}{"OK", nil},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner non-existent path v2",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj idonotexist",},
+			expected: []interface{}{"OK", nil},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path with nonJSON object",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj .name",},
+			expected: []interface{}{"OK", "WRONGTYPE Operation against a key holding the wrong kind of value"},
+		},
+		{
+			name:     "JSON.OBJLEN with legacy path - inner existent path recursive object",
+			commands: []string{"json.set obj $ " + c, "json.objlen obj ..partner",},
+			expected: []interface{}{"OK", float64(2)},
+		},
 	}
 
 	for _, tcase := range testCases {

diff --git a/internal/eval/store_eval.go b/internal/eval/store_eval.go
@@ -884,7 +884,7 @@ func evalJSONOBJLEN(args []string, store *dstore.Store) *EvalResponse {
 		}
 	}
 
-	path := args[1]
+	path, isDefinitePath := utils.ParseInputJSONPath(args[1])
 
 	expr, err := jp.ParseString(path)
 	if err != nil {
@@ -908,9 +908,31 @@ func evalJSONOBJLEN(args []string, store *dstore.Store) *EvalResponse {
 				objectLen = append(objectLen, nil)
 			}
 		default:
+			// If it is a definitePath, and the only value is not JSON, throw wrong type error
+			if isDefinitePath {
+				return &EvalResponse{
+					Result: nil,
+					Error: diceerrors.ErrWrongTypeOperation,
+				}
+			}
 			objectLen = append(objectLen, nil)
 		}
 	}
+
+	// Must return a single integer if it is a definite Path
+	if isDefinitePath {
+		if len(objectLen) == 0 {
+			return &EvalResponse{
+				Result: nil,
+				Error:  nil,
+			}
+		}
+		return &EvalResponse{
+			Result: objectLen[0],
+			Error:  nil,
+		}
+	}
+
 	return &EvalResponse{
 		Result: objectLen,
 		Error:  nil,

diff --git a/internal/server/utils/json.go b/internal/server/utils/json.go
@@ -0,0 +1,16 @@
+package utils
+
+// This method returns the path, and a boolean value telling if the path provided follows Legacy Path Syntax or JSONPath syntax. 
+// JSON knows which syntax to use depending on the first character of the path query. 
+// If the query starts with the character $, it uses JSONPath syntax. Otherwise, it defaults to the legacy path syntax.
+// A JSONPath query can resolve to several locations in a JSON document. 
+// In this case, the JSON commands apply the operation to every possible location. This is a major improvement over legacy path queries, which only operate on the first path.
+func ParseInputJSONPath(path string) (string, bool) {
+	isLegacyPath := path[0] != '$'
+
+	// Handle . path error
+	if len(path) ==1 && path[0] == '.' {
+		path = "$"
+	}
+	return path, isLegacyPath
+}