1
0
mirror of https://github.com/redis/go-redis.git synced 2025-09-05 20:24:00 +03:00

fix(json): Ensure that JSON.GET returns Nil response (#3470)

* fix conflicts

* Fix JSON nil response handling

Ensure that JSON.GET returns redis.Nil for missing keys/paths,
making it consistent with other Redis commands.

- Restore proper nil detection logic in JSONCmd.readReply
- Add comprehensive test coverage for JSON nil scenarios
- Handle both non-existent keys and non-existent paths consistently
- Distinguish between empty arrays and nil responses
- Add documentation for Val() and Expanded() methods

Original work and problem identification by Nic Gibson.
Enhanced implementation with comprehensive testing and fixes
for the broken nil detection logic.

Fixes #2987

* Fix JSON nil response handling - align with Redis behavior

- Non-existent keys return redis.Nil (consistent with other Redis commands)
- Non-existent paths in existing keys return empty array '[]'
- Fix broken test that was using wrong doc1 reference
- Add comprehensive test coverage for JSON nil scenarios

This aligns with official Redis JSON.GET behavior:
- Missing keys should return nil error like other Redis commands
- Missing paths should return empty JSON array, not error

* Fix JSONDel tests

---------

Co-authored-by: Nic Gibson <nic.gibson@redis.com>
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
This commit is contained in:
ofekshenawa
2025-08-12 14:28:07 +03:00
committed by GitHub
parent b566dcacd6
commit 0b1e9f77ef
2 changed files with 74 additions and 2 deletions

16
json.go
View File

@@ -82,6 +82,7 @@ func (cmd *JSONCmd) SetVal(val string) {
cmd.val = val
}
// Val returns the result of the JSON.GET command as a string.
func (cmd *JSONCmd) Val() string {
if len(cmd.val) == 0 && cmd.expanded != nil {
val, err := json.Marshal(cmd.expanded)
@@ -100,6 +101,7 @@ func (cmd *JSONCmd) Result() (string, error) {
return cmd.Val(), cmd.Err()
}
// Expanded returns the result of the JSON.GET command as unmarshalled JSON.
func (cmd *JSONCmd) Expanded() (interface{}, error) {
if len(cmd.val) != 0 && cmd.expanded == nil {
err := json.Unmarshal([]byte(cmd.val), &cmd.expanded)
@@ -113,11 +115,17 @@ func (cmd *JSONCmd) Expanded() (interface{}, error) {
func (cmd *JSONCmd) readReply(rd *proto.Reader) error {
// nil response from JSON.(M)GET (cmd.baseCmd.err will be "redis: nil")
// This happens when the key doesn't exist
if cmd.baseCmd.Err() == Nil {
cmd.val = ""
return Nil
}
// Handle other base command errors
if cmd.baseCmd.Err() != nil {
return cmd.baseCmd.Err()
}
if readType, err := rd.PeekReplyType(); err != nil {
return err
} else if readType == proto.RespArray {
@@ -127,6 +135,13 @@ func (cmd *JSONCmd) readReply(rd *proto.Reader) error {
return err
}
// Empty array means no results found for JSON path, but key exists
// This should return "[]", not an error
if size == 0 {
cmd.val = "[]"
return nil
}
expanded := make([]interface{}, size)
for i := 0; i < size; i++ {
@@ -141,6 +156,7 @@ func (cmd *JSONCmd) readReply(rd *proto.Reader) error {
return err
} else if str == "" || err == Nil {
cmd.val = ""
return Nil
} else {
cmd.val = str
}

View File

@@ -142,6 +142,10 @@ var _ = Describe("JSON Commands", Label("json"), func() {
resArr, err := client.JSONArrIndex(ctx, "doc1", "$.store.book[?(@.price<10)].size", 20).Result()
Expect(err).NotTo(HaveOccurred())
Expect(resArr).To(Equal([]int64{1, 2}))
_, err = client.JSONGet(ctx, "this-key-does-not-exist", "$").Result()
Expect(err).To(HaveOccurred())
Expect(err).To(BeIdenticalTo(redis.Nil))
})
It("should JSONArrInsert", Label("json.arrinsert", "json"), func() {
@@ -402,8 +406,8 @@ var _ = Describe("JSON Commands", Label("json"), func() {
Expect(cmd2.Val()).To(Equal(int64(1)))
cmd3 := client.JSONGet(ctx, "del1", "$")
Expect(cmd3.Err()).NotTo(HaveOccurred())
Expect(cmd3.Val()).To(HaveLen(0))
Expect(cmd3.Err()).To(Equal(redis.Nil))
Expect(cmd3.Val()).To(Equal(""))
})
It("should JSONDel with $", Label("json.del", "json"), func() {
@@ -673,6 +677,58 @@ var _ = Describe("JSON Commands", Label("json"), func() {
Expect(cmd2.Val()[0]).To(Or(Equal([]interface{}{"boolean"}), Equal("boolean")))
})
})
Describe("JSON Nil Handling", func() {
It("should return redis.Nil for non-existent key", func() {
_, err := client.JSONGet(ctx, "non-existent-key", "$").Result()
Expect(err).To(Equal(redis.Nil))
})
It("should return empty array for non-existent path in existing key", func() {
err := client.JSONSet(ctx, "test-key", "$", `{"a": 1, "b": "hello"}`).Err()
Expect(err).NotTo(HaveOccurred())
// Non-existent path should return empty array, not error
val, err := client.JSONGet(ctx, "test-key", "$.nonexistent").Result()
Expect(err).NotTo(HaveOccurred())
Expect(val).To(Equal("[]"))
})
It("should distinguish empty array from non-existent path", func() {
err := client.JSONSet(ctx, "test-key", "$", `{"arr": [], "obj": {}}`).Err()
Expect(err).NotTo(HaveOccurred())
// Empty array should return the array
val, err := client.JSONGet(ctx, "test-key", "$.arr").Result()
Expect(err).NotTo(HaveOccurred())
Expect(val).To(Equal("[[]]"))
// Non-existent field should return empty array
val, err = client.JSONGet(ctx, "test-key", "$.missing").Result()
Expect(err).NotTo(HaveOccurred())
Expect(val).To(Equal("[]"))
})
It("should handle multiple paths with mixed results", func() {
err := client.JSONSet(ctx, "test-key", "$", `{"a": 1, "b": 2}`).Err()
Expect(err).NotTo(HaveOccurred())
// Path that exists
val, err := client.JSONGet(ctx, "test-key", "$.a").Result()
Expect(err).NotTo(HaveOccurred())
Expect(val).To(Equal("[1]"))
// Path that doesn't exist should return empty array
val, err = client.JSONGet(ctx, "test-key", "$.c").Result()
Expect(err).NotTo(HaveOccurred())
Expect(val).To(Equal("[]"))
})
AfterEach(func() {
// Clean up test keys
client.Del(ctx, "test-key", "non-existent-key")
})
})
}
})