Skip to content

Commit 0b1e9f7

Browse files
ofekshenawanic-gibsonndyakov
authored
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 <[email protected]> Co-authored-by: Nedyalko Dyakov <[email protected]>
1 parent b566dca commit 0b1e9f7

File tree

2 files changed

+74
-2
lines changed

2 files changed

+74
-2
lines changed

json.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ func (cmd *JSONCmd) SetVal(val string) {
8282
cmd.val = val
8383
}
8484

85+
// Val returns the result of the JSON.GET command as a string.
8586
func (cmd *JSONCmd) Val() string {
8687
if len(cmd.val) == 0 && cmd.expanded != nil {
8788
val, err := json.Marshal(cmd.expanded)
@@ -100,6 +101,7 @@ func (cmd *JSONCmd) Result() (string, error) {
100101
return cmd.Val(), cmd.Err()
101102
}
102103

104+
// Expanded returns the result of the JSON.GET command as unmarshalled JSON.
103105
func (cmd *JSONCmd) Expanded() (interface{}, error) {
104106
if len(cmd.val) != 0 && cmd.expanded == nil {
105107
err := json.Unmarshal([]byte(cmd.val), &cmd.expanded)
@@ -113,11 +115,17 @@ func (cmd *JSONCmd) Expanded() (interface{}, error) {
113115

114116
func (cmd *JSONCmd) readReply(rd *proto.Reader) error {
115117
// nil response from JSON.(M)GET (cmd.baseCmd.err will be "redis: nil")
118+
// This happens when the key doesn't exist
116119
if cmd.baseCmd.Err() == Nil {
117120
cmd.val = ""
118121
return Nil
119122
}
120123

124+
// Handle other base command errors
125+
if cmd.baseCmd.Err() != nil {
126+
return cmd.baseCmd.Err()
127+
}
128+
121129
if readType, err := rd.PeekReplyType(); err != nil {
122130
return err
123131
} else if readType == proto.RespArray {
@@ -127,6 +135,13 @@ func (cmd *JSONCmd) readReply(rd *proto.Reader) error {
127135
return err
128136
}
129137

138+
// Empty array means no results found for JSON path, but key exists
139+
// This should return "[]", not an error
140+
if size == 0 {
141+
cmd.val = "[]"
142+
return nil
143+
}
144+
130145
expanded := make([]interface{}, size)
131146

132147
for i := 0; i < size; i++ {
@@ -141,6 +156,7 @@ func (cmd *JSONCmd) readReply(rd *proto.Reader) error {
141156
return err
142157
} else if str == "" || err == Nil {
143158
cmd.val = ""
159+
return Nil
144160
} else {
145161
cmd.val = str
146162
}

json_test.go

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ var _ = Describe("JSON Commands", Label("json"), func() {
142142
resArr, err := client.JSONArrIndex(ctx, "doc1", "$.store.book[?(@.price<10)].size", 20).Result()
143143
Expect(err).NotTo(HaveOccurred())
144144
Expect(resArr).To(Equal([]int64{1, 2}))
145+
146+
_, err = client.JSONGet(ctx, "this-key-does-not-exist", "$").Result()
147+
Expect(err).To(HaveOccurred())
148+
Expect(err).To(BeIdenticalTo(redis.Nil))
145149
})
146150

147151
It("should JSONArrInsert", Label("json.arrinsert", "json"), func() {
@@ -402,8 +406,8 @@ var _ = Describe("JSON Commands", Label("json"), func() {
402406
Expect(cmd2.Val()).To(Equal(int64(1)))
403407

404408
cmd3 := client.JSONGet(ctx, "del1", "$")
405-
Expect(cmd3.Err()).NotTo(HaveOccurred())
406-
Expect(cmd3.Val()).To(HaveLen(0))
409+
Expect(cmd3.Err()).To(Equal(redis.Nil))
410+
Expect(cmd3.Val()).To(Equal(""))
407411
})
408412

409413
It("should JSONDel with $", Label("json.del", "json"), func() {
@@ -673,6 +677,58 @@ var _ = Describe("JSON Commands", Label("json"), func() {
673677
Expect(cmd2.Val()[0]).To(Or(Equal([]interface{}{"boolean"}), Equal("boolean")))
674678
})
675679
})
680+
681+
Describe("JSON Nil Handling", func() {
682+
It("should return redis.Nil for non-existent key", func() {
683+
_, err := client.JSONGet(ctx, "non-existent-key", "$").Result()
684+
Expect(err).To(Equal(redis.Nil))
685+
})
686+
687+
It("should return empty array for non-existent path in existing key", func() {
688+
err := client.JSONSet(ctx, "test-key", "$", `{"a": 1, "b": "hello"}`).Err()
689+
Expect(err).NotTo(HaveOccurred())
690+
691+
// Non-existent path should return empty array, not error
692+
val, err := client.JSONGet(ctx, "test-key", "$.nonexistent").Result()
693+
Expect(err).NotTo(HaveOccurred())
694+
Expect(val).To(Equal("[]"))
695+
})
696+
697+
It("should distinguish empty array from non-existent path", func() {
698+
err := client.JSONSet(ctx, "test-key", "$", `{"arr": [], "obj": {}}`).Err()
699+
Expect(err).NotTo(HaveOccurred())
700+
701+
// Empty array should return the array
702+
val, err := client.JSONGet(ctx, "test-key", "$.arr").Result()
703+
Expect(err).NotTo(HaveOccurred())
704+
Expect(val).To(Equal("[[]]"))
705+
706+
// Non-existent field should return empty array
707+
val, err = client.JSONGet(ctx, "test-key", "$.missing").Result()
708+
Expect(err).NotTo(HaveOccurred())
709+
Expect(val).To(Equal("[]"))
710+
})
711+
712+
It("should handle multiple paths with mixed results", func() {
713+
err := client.JSONSet(ctx, "test-key", "$", `{"a": 1, "b": 2}`).Err()
714+
Expect(err).NotTo(HaveOccurred())
715+
716+
// Path that exists
717+
val, err := client.JSONGet(ctx, "test-key", "$.a").Result()
718+
Expect(err).NotTo(HaveOccurred())
719+
Expect(val).To(Equal("[1]"))
720+
721+
// Path that doesn't exist should return empty array
722+
val, err = client.JSONGet(ctx, "test-key", "$.c").Result()
723+
Expect(err).NotTo(HaveOccurred())
724+
Expect(val).To(Equal("[]"))
725+
})
726+
727+
AfterEach(func() {
728+
// Clean up test keys
729+
client.Del(ctx, "test-key", "non-existent-key")
730+
})
731+
})
676732
}
677733
})
678734

0 commit comments

Comments
 (0)