diff --git a/command.go b/command.go index 22d77478..5e0dd5e4 100644 --- a/command.go +++ b/command.go @@ -5015,29 +5015,31 @@ type GeoRadiusQuery struct { Sort string Store string StoreDist string - - // WithCoord+WithDist+WithGeoHash - withLen int } type GeoLocationCmd struct { baseCmd - q *GeoRadiusQuery + // q is a copy of the caller's query, taken when the command is built, so + // the reply is parsed with the flags that were sent even if the caller + // changes or reuses its query before the reply arrives (pipelines, + // autopipelining). + q GeoRadiusQuery locations []GeoLocation } var _ Cmder = (*GeoLocationCmd)(nil) func NewGeoLocationCmd(ctx context.Context, q *GeoRadiusQuery, args ...interface{}) *GeoLocationCmd { - return &GeoLocationCmd{ + cmd := &GeoLocationCmd{ baseCmd: baseCmd{ ctx: ctx, - args: geoLocationArgs(q, args...), cmdType: CmdTypeGeoLocation, }, - q: q, + q: *q, } + cmd.args = geoLocationArgs(&cmd.q, args...) + return cmd } func geoLocationArgs(q *GeoRadiusQuery, args ...interface{}) []interface{} { @@ -5049,15 +5051,12 @@ func geoLocationArgs(q *GeoRadiusQuery, args ...interface{}) []interface{} { } if q.WithCoord { args = append(args, "withcoord") - q.withLen++ } if q.WithDist { args = append(args, "withdist") - q.withLen++ } if q.WithGeoHash { args = append(args, "withhash") - q.withLen++ } if q.Count > 0 { args = append(args, "count", q.Count) @@ -5102,9 +5101,21 @@ func (cmd *GeoLocationCmd) readReply(rd *proto.Reader) error { } cmd.locations = make([]GeoLocation, n) + // Count the WITH flags of the query this command was built with. + withLen := 0 + if cmd.q.WithCoord { + withLen++ + } + if cmd.q.WithDist { + withLen++ + } + if cmd.q.WithGeoHash { + withLen++ + } + for i := 0; i < len(cmd.locations); i++ { // only name - if cmd.q.withLen == 0 { + if withLen == 0 { if cmd.locations[i].Name, err = rd.ReadString(); err != nil { return err } @@ -5112,7 +5123,7 @@ func (cmd *GeoLocationCmd) readReply(rd *proto.Reader) error { } // +name - if err = rd.ReadFixedArrayLen(cmd.q.withLen + 1); err != nil { + if err = rd.ReadFixedArrayLen(withLen + 1); err != nil { return err } @@ -5146,21 +5157,6 @@ func (cmd *GeoLocationCmd) readReply(rd *proto.Reader) error { } func (cmd *GeoLocationCmd) Clone() Cmder { - var q *GeoRadiusQuery - if cmd.q != nil { - q = &GeoRadiusQuery{ - Radius: cmd.q.Radius, - Unit: cmd.q.Unit, - WithCoord: cmd.q.WithCoord, - WithDist: cmd.q.WithDist, - WithGeoHash: cmd.q.WithGeoHash, - Count: cmd.q.Count, - Sort: cmd.q.Sort, - Store: cmd.q.Store, - StoreDist: cmd.q.StoreDist, - withLen: cmd.q.withLen, - } - } var locations []GeoLocation if cmd.locations != nil { locations = make([]GeoLocation, len(cmd.locations)) @@ -5168,7 +5164,7 @@ func (cmd *GeoLocationCmd) Clone() Cmder { } return &GeoLocationCmd{ baseCmd: cmd.cloneBaseCmd(), - q: q, + q: cmd.q, locations: locations, } } @@ -5241,15 +5237,17 @@ func geoSearchArgs(q *GeoSearchQuery, args []interface{}) []interface{} { } if q.Radius > 0 { - if q.RadiusUnit == "" { - q.RadiusUnit = "km" + unit := q.RadiusUnit + if unit == "" { + unit = "km" } - args = append(args, "byradius", q.Radius, q.RadiusUnit) + args = append(args, "byradius", q.Radius, unit) } else { - if q.BoxUnit == "" { - q.BoxUnit = "km" + unit := q.BoxUnit + if unit == "" { + unit = "km" } - args = append(args, "bybox", q.BoxWidth, q.BoxHeight, q.BoxUnit) + args = append(args, "bybox", q.BoxWidth, q.BoxHeight, unit) } if q.Sort != "" { @@ -5269,7 +5267,8 @@ func geoSearchArgs(q *GeoSearchQuery, args []interface{}) []interface{} { type GeoSearchLocationCmd struct { baseCmd - opt *GeoSearchLocationQuery + // opt is a copy of the caller's query; see GeoLocationCmd.q. + opt GeoSearchLocationQuery val []GeoLocation } @@ -5278,14 +5277,15 @@ var _ Cmder = (*GeoSearchLocationCmd)(nil) func NewGeoSearchLocationCmd( ctx context.Context, opt *GeoSearchLocationQuery, args ...interface{}, ) *GeoSearchLocationCmd { - return &GeoSearchLocationCmd{ + cmd := &GeoSearchLocationCmd{ baseCmd: baseCmd{ ctx: ctx, - args: geoSearchLocationArgs(opt, args), cmdType: CmdTypeGeoSearchLocation, }, - opt: opt, + opt: *opt, } + cmd.args = geoSearchLocationArgs(&cmd.opt, args) + return cmd } func (cmd *GeoSearchLocationCmd) SetVal(val []GeoLocation) { @@ -5381,27 +5381,6 @@ func (cmd *GeoSearchLocationCmd) readReply(rd *proto.Reader) error { } func (cmd *GeoSearchLocationCmd) Clone() Cmder { - var opt *GeoSearchLocationQuery - if cmd.opt != nil { - opt = &GeoSearchLocationQuery{ - GeoSearchQuery: GeoSearchQuery{ - Member: cmd.opt.Member, - Longitude: cmd.opt.Longitude, - Latitude: cmd.opt.Latitude, - Radius: cmd.opt.Radius, - RadiusUnit: cmd.opt.RadiusUnit, - BoxWidth: cmd.opt.BoxWidth, - BoxHeight: cmd.opt.BoxHeight, - BoxUnit: cmd.opt.BoxUnit, - Sort: cmd.opt.Sort, - Count: cmd.opt.Count, - CountAny: cmd.opt.CountAny, - }, - WithCoord: cmd.opt.WithCoord, - WithDist: cmd.opt.WithDist, - WithHash: cmd.opt.WithHash, - } - } var val []GeoLocation if cmd.val != nil { val = make([]GeoLocation, len(cmd.val)) @@ -5409,7 +5388,7 @@ func (cmd *GeoSearchLocationCmd) Clone() Cmder { } return &GeoSearchLocationCmd{ baseCmd: cmd.cloneBaseCmd(), - opt: opt, + opt: cmd.opt, val: val, } } diff --git a/commands_test.go b/commands_test.go index bd7bfcb6..7620eb29 100644 --- a/commands_test.go +++ b/commands_test.go @@ -9280,6 +9280,24 @@ var _ = Describe("Commands", func() { Expect(res[0].Latitude).To(Equal(37.50266842333162)) }) + It("should search geo radius twice with the same query", func() { + q := &redis.GeoRadiusQuery{ + Radius: 200, + Unit: "km", + WithDist: true, + Sort: "ASC", + } + for i := 0; i < 2; i++ { + res, err := client.GeoRadius(ctx, "Sicily", 15, 37, q).Result() + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(HaveLen(2)) + Expect(res[0].Name).To(Equal("Catania")) + Expect(res[0].Dist).To(Equal(56.4413)) + Expect(res[1].Name).To(Equal("Palermo")) + Expect(res[1].Dist).To(Equal(190.4424)) + } + }) + It("should search geo radius by member with options", func() { res, err := client.GeoRadiusByMember(ctx, "Sicily", "Catania", &redis.GeoRadiusQuery{ Radius: 200, @@ -9462,6 +9480,18 @@ var _ = Describe("Commands", func() { Expect(val).To(Equal([]string{"Palermo"})) }) + It("should geo search without changing the query", func() { + q := &redis.GeoSearchQuery{ + Member: "Catania", + Radius: 200, + Sort: "asc", + } + val, err := client.GeoSearch(ctx, "Sicily", q).Result() + Expect(err).NotTo(HaveOccurred()) + Expect(val).To(Equal([]string{"Catania", "Palermo"})) + Expect(q.RadiusUnit).To(BeEmpty()) + }) + It("should geo search with options", func() { q := &redis.GeoSearchLocationQuery{ GeoSearchQuery: redis.GeoSearchQuery{ diff --git a/geo_commands_unit_test.go b/geo_commands_unit_test.go new file mode 100644 index 00000000..98d34b98 --- /dev/null +++ b/geo_commands_unit_test.go @@ -0,0 +1,48 @@ +package redis + +import ( + "context" + "strings" + "testing" + + "github.com/redis/go-redis/v9/internal/proto" +) + +// A pipelined or autopipelined command is built when it is queued and its +// reply is read later. Changing the caller's query in between must not change +// how the reply is parsed. +func TestGeoCmdsParseReplyWithQueuedFlags(t *testing.T) { + // Two entries, each [name, dist]: the reply to a WITHDIST query. + const withDistReply = "*2\r\n" + + "*2\r\n$7\r\nCatania\r\n$7\r\n56.4413\r\n" + + "*2\r\n$7\r\nPalermo\r\n$8\r\n190.4424\r\n" + + t.Run("GeoLocationCmd", func(t *testing.T) { + q := &GeoRadiusQuery{Radius: 200, Unit: "km", WithDist: true} + cmd := NewGeoLocationCmd(context.Background(), q, "georadius", "Sicily", 15, 37) + q.WithDist, q.WithCoord = false, true + + if err := cmd.readReply(proto.NewReader(strings.NewReader(withDistReply))); err != nil { + t.Fatalf("readReply: %v", err) + } + if got := cmd.Val(); len(got) != 2 || got[0].Dist != 56.4413 || got[1].Dist != 190.4424 { + t.Fatalf("got %+v", got) + } + }) + + t.Run("GeoSearchLocationCmd", func(t *testing.T) { + q := &GeoSearchLocationQuery{ + GeoSearchQuery: GeoSearchQuery{Member: "Catania", Radius: 200}, + WithDist: true, + } + cmd := NewGeoSearchLocationCmd(context.Background(), q, "geosearch", "Sicily") + q.WithDist, q.WithCoord = false, true + + if err := cmd.readReply(proto.NewReader(strings.NewReader(withDistReply))); err != nil { + t.Fatalf("readReply: %v", err) + } + if got := cmd.Val(); len(got) != 2 || got[0].Dist != 56.4413 || got[1].Dist != 190.4424 { + t.Fatalf("got %+v", got) + } + }) +}