From b33b1be5347cd8e7aa9d3eb1c9101da0ee1a38ca Mon Sep 17 00:00:00 2001 From: Harmen Date: Fri, 8 Mar 2024 13:11:15 +0100 Subject: [PATCH 1/2] geosearch wip --- cmd_geo.go | 189 ++++++++++++++++++++++++++++++++++++++++ integration/geo_test.go | 60 +++++++++++++ opts.go | 11 +++ 3 files changed, 260 insertions(+) diff --git a/cmd_geo.go b/cmd_geo.go index 29a92d55..48a33fe9 100644 --- a/cmd_geo.go +++ b/cmd_geo.go @@ -20,6 +20,7 @@ func commandsGeo(m *Miniredis) { m.srv.Register("GEORADIUS_RO", m.cmdGeoradius) m.srv.Register("GEORADIUSBYMEMBER", m.cmdGeoradiusbymember) m.srv.Register("GEORADIUSBYMEMBER_RO", m.cmdGeoradiusbymember) + m.srv.Register("GEOSEARCH", m.cmdGeosearch) } // GEOADD @@ -607,3 +608,191 @@ func parseUnit(u string) float64 { return 0 } } + +type tuple struct { + a float64 + b float64 +} + +// GEOSEARCH +func (m *Miniredis) cmdGeosearch(c *server.Peer, cmd string, args []string) { + if len(args) < 2 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + if !m.handleAuth(c) { + return + } + if m.checkPubsub(c, cmd) { + return + } + + var opts struct { + key string + withFromMember bool + fromMember string + withFromLonLat bool + fromLonLat tuple + withByRadius bool + byRadius float64 + withByBox bool + byBox tuple + direction direction // unsorted + count int + withAny bool + withCoord bool + withDist bool + withHash bool + } + + opts.key, args = args[0], args[1:] + + fmt.Printf("args: %v\n", args) + + switch strings.ToUpper(args[0]) { + case "FROMMEMBER": + if len(args) < 2 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + opts.withFromMember = true + opts.fromMember = args[1] + args = args[2:] + case "FROMLONLAT": + if len(args) < 3 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + opts.withFromLonLat = true + if !optFloat(c, args[1], &opts.fromLonLat.a) { + return + } + if !optFloat(c, args[2], &opts.fromLonLat.b) { + return + } + args = args[3:] + default: + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + + if len(args) < 3 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + switch strings.ToUpper(args[0]) { + case "BYRADIUS": + if len(args) < 3 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + opts.withByRadius = true + if !optFloat(c, args[1], &opts.byRadius) { + return + } + toMeter := parseUnit(args[2]) + if toMeter == 0 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + opts.byRadius *= toMeter + args = args[3:] + case "BYBOX": + if len(args) < 4 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + opts.withByBox = true + if !optFloat(c, args[1], &opts.byBox.a) { + return + } + if !optFloat(c, args[2], &opts.byBox.b) { + return + } + toMeter := parseUnit(args[3]) + if toMeter == 0 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + opts.byBox.a *= toMeter + opts.byBox.b *= toMeter + args = args[4:] + default: + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + + // FIXME: ASC|DESC + // FIXME: COUNT n ANY + // FIXME: WITHCOORD + // FIXME: WITHDIST + // FIXME: WITHHASH + + withTx(m, c, func(c *server.Peer, ctx *connCtx) { + db := m.db(ctx.selectedDB) + members := db.ssetElements(opts.key) + + if !opts.withFromLonLat { + panic("wip") + } + if !opts.withByRadius { + panic("wip") + } + matches := withinRadius(members, opts.fromLonLat.a, opts.fromLonLat.b, opts.byRadius) + + /* + // deal with ASC/DESC + if opts.direction != unsorted { + sort.Slice(matches, func(i, j int) bool { + if opts.direction == desc { + return matches[i].Distance > matches[j].Distance + } + return matches[i].Distance < matches[j].Distance + }) + } + + // deal with COUNT + if opts.count > 0 && len(matches) > opts.count { + matches = matches[:opts.count] + } + */ + + c.WriteLen(len(matches)) + for _, member := range matches { + // if !opts.withDist && !opts.withCoord { + c.WriteBulk(member.Name) + continue + // } + + /* + len := 1 + if opts.withDist { + len++ + } + if opts.withCoord { + len++ + } + c.WriteLen(len) + c.WriteBulk(member.Name) + if opts.withDist { + c.WriteBulk(fmt.Sprintf("%.4f", member.Distance/toMeter)) + } + if opts.withCoord { + c.WriteLen(2) + c.WriteBulk(fmt.Sprintf("%f", member.Longitude)) + c.WriteBulk(fmt.Sprintf("%f", member.Latitude)) + } + */ + } + }) +} diff --git a/integration/geo_test.go b/integration/geo_test.go index 4ce87dd0..7da1a6e6 100644 --- a/integration/geo_test.go +++ b/integration/geo_test.go @@ -934,3 +934,63 @@ func TestGeo(t *testing.T) { c.DoLoosely("ZRANGE", "resbymemd", "0", "-1", "WITHSCORES") }) } + +func TestGeosearch(t *testing.T) { + skip(t) + t.Run("basic", func(t *testing.T) { + testRaw(t, func(c *client) { + c.Do("GEOADD", + "stations", + "-73.99106999861966", "40.73005400028978", "Astor Pl", + "-74.00019299927328", "40.71880300107709", "Canal St", + "-73.98384899986625", "40.76172799961419", "50th St", + ) + c.Do("GEOSEARCH", "stations", "FROMLONLAT", "-73.9718893", "40.7728773", "BYRADIUS", "4", "km") + c.Do("GEOSEARCH", "stations", "FROMLONLAT", "-73.9718893", "40.7728773", "BYRADIUS", "4", "KM") // case of KM + c.Do("GEOSEARCH", "stations", "FROMLONLAT", "1.0", "1.0", "BYRADIUS", "1", "km") + // c.Do("GEOSEARCH", "stations", "FROMLONLAT", "-73.9718893", "40.7728773", "BYRADIUS", "4", "ft", "WITHDIST") + // c.Do("GEORADIUS", "stations", "FROMLONLAT", "-73.9718893", "40.7728773", "BYRADIUS", "4", "m", "WITHDIST") + + /* + // redis has more precision in the coords + c.Do("GEORADIUS", "stations", "-73.9718893", "40.7728773", "4", "m", "WITHCOORD") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "WITHDIST", "WITHCOORD") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "WITHCOORD", "WITHDIST") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "WITHCOORD", "WITHCOORD", "WITHCOORD") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "WITHDIST", "WITHDIST", "WITHDIST") + // FIXME: the distances don't quite match for miles or km + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "mi", "WITHDIST") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "WITHDIST") + + // Sorting + c.Do("GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "DESC") + c.Do("GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "ASC") + c.Do("GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "ASC", "DESC", "ASC") + + // COUNT + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "ASC", "COUNT", "1") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "ASC", "COUNT", "2") + c.DoRounded(3, "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "ASC", "COUNT", "999") + c.Error("syntax error", "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "COUNT") + c.Error("COUNT must", "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "COUNT", "0") + c.Error("COUNT must", "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "COUNT", "-12") + c.Error("not an integer", "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "COUNT", "foobar") + + // non-existing key + c.Do("GEORADIUS", "foo", "-73.9718893", "40.7728773", "4", "km") + + // no error in redis, for some reason + // c.Do("GEORADIUS", "foo", "-73.9718893", "40.7728773", "4", "km", "FOOBAR") + c.Error("syntax error", "GEORADIUS", "stations", "-73.9718893", "40.7728773", "400", "km", "ASC", "FOOBAR") + + // GEORADIUS_RO + c.Do("GEORADIUS_RO", "stations", "-73.9718893", "40.7728773", "4", "km") + c.Do("GEORADIUS_RO", "stations", "1.0", "1.0", "1", "km") + c.Error("syntax error", "GEORADIUS_RO", "stations", "-73.9718893", "40.7728773", "4", "km", "STORE", "bar") + c.Error("syntax error", "GEORADIUS_RO", "stations", "-73.9718893", "40.7728773", "4", "km", "STOREDIST", "bar") + c.Error("syntax error", "GEORADIUS_RO", "stations", "-73.9718893", "40.7728773", "4", "km", "STORE") + c.Error("syntax error", "GEORADIUS_RO", "stations", "-73.9718893", "40.7728773", "4", "km", "STOREDIST") + */ + }) + }) +} diff --git a/opts.go b/opts.go index 666ace7f..8ce05f78 100644 --- a/opts.go +++ b/opts.go @@ -47,3 +47,14 @@ func optDuration(c *server.Peer, src string, dest *time.Duration) bool { *dest = time.Duration(n*1_000_000) * time.Microsecond return true } + +func optFloat(c *server.Peer, src string, dest *float64) bool { + n, err := strconv.ParseFloat(src, 64) + if err != nil { + setDirty(c) + c.WriteError(msgInvalidInt) // FIXME + return false + } + *dest = n + return true +} From 039c46968865b47b9464e30c528e2c1f7bd9d00e Mon Sep 17 00:00:00 2001 From: Harmen Date: Wed, 13 Mar 2024 09:40:55 +0100 Subject: [PATCH 2/2] split out the opt parsing --- cmd_geo.go | 130 ++++++++++++++++++++++++++--------------------------- opts.go | 9 ++-- 2 files changed, 67 insertions(+), 72 deletions(-) diff --git a/cmd_geo.go b/cmd_geo.go index ede0e816..8767b8a7 100644 --- a/cmd_geo.go +++ b/cmd_geo.go @@ -3,6 +3,7 @@ package miniredis import ( + "errors" "fmt" "sort" "strconv" @@ -614,37 +615,26 @@ type tuple struct { b float64 } -// GEOSEARCH -func (m *Miniredis) cmdGeosearch(c *server.Peer, cmd string, args []string) { - if len(args) < 2 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return - } - if !m.handleAuth(c) { - return - } - if m.checkPubsub(c, cmd) { - return - } +type geosearchOpts struct { + key string + withFromMember bool + fromMember string + withFromLonLat bool + fromLonLat tuple + withByRadius bool + byRadius float64 + withByBox bool + byBox tuple + direction direction // unsorted + count int + withAny bool + withCoord bool + withDist bool + withHash bool +} - var opts struct { - key string - withFromMember bool - fromMember string - withFromLonLat bool - fromLonLat tuple - withByRadius bool - byRadius float64 - withByBox bool - byBox tuple - direction direction // unsorted - count int - withAny bool - withCoord bool - withDist bool - withHash bool - } +func geosearchParse(cmd string, args []string) (*geosearchOpts, error) { + var opts geosearchOpts opts.key, args = args[0], args[1:] @@ -653,83 +643,65 @@ func (m *Miniredis) cmdGeosearch(c *server.Peer, cmd string, args []string) { switch strings.ToUpper(args[0]) { case "FROMMEMBER": if len(args) < 2 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } opts.withFromMember = true opts.fromMember = args[1] args = args[2:] case "FROMLONLAT": if len(args) < 3 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } opts.withFromLonLat = true - if !optFloat(c, args[1], &opts.fromLonLat.a) { - return + if err := optFloat(args[1], &opts.fromLonLat.a); err != nil { + return nil, err } - if !optFloat(c, args[2], &opts.fromLonLat.b) { - return + if err := optFloat(args[2], &opts.fromLonLat.b); err != nil { + return nil, err } args = args[3:] default: - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } if len(args) < 3 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } switch strings.ToUpper(args[0]) { case "BYRADIUS": if len(args) < 3 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } opts.withByRadius = true - if !optFloat(c, args[1], &opts.byRadius) { - return + if err := optFloat(args[1], &opts.byRadius); err != nil { + return nil, err } toMeter := parseUnit(args[2]) if toMeter == 0 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } opts.byRadius *= toMeter args = args[3:] case "BYBOX": if len(args) < 4 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } opts.withByBox = true - if !optFloat(c, args[1], &opts.byBox.a) { - return + if err := optFloat(args[1], &opts.byBox.a); err != nil { + return nil, err } - if !optFloat(c, args[2], &opts.byBox.b) { - return + if err := optFloat(args[2], &opts.byBox.b); err != nil { + return nil, err } toMeter := parseUnit(args[3]) if toMeter == 0 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } opts.byBox.a *= toMeter opts.byBox.b *= toMeter args = args[4:] default: - setDirty(c) - c.WriteError(errWrongNumber(cmd)) - return + return nil, errors.New(errWrongNumber(cmd)) } // FIXME: ASC|DESC @@ -738,6 +710,30 @@ func (m *Miniredis) cmdGeosearch(c *server.Peer, cmd string, args []string) { // FIXME: WITHDIST // FIXME: WITHHASH + return &opts, nil +} + +// GEOSEARCH +func (m *Miniredis) cmdGeosearch(c *server.Peer, cmd string, args []string) { + if len(args) < 2 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + if !m.handleAuth(c) { + return + } + if m.checkPubsub(c, cmd) { + return + } + + opts, err := geosearchParse(cmd, args) + if err != nil { + setDirty(c) + c.WriteError(err.Error()) + return + } + withTx(m, c, func(c *server.Peer, ctx *connCtx) { db := m.db(ctx.selectedDB) members := db.ssetElements(opts.key) diff --git a/opts.go b/opts.go index 8ce05f78..ee879168 100644 --- a/opts.go +++ b/opts.go @@ -1,6 +1,7 @@ package miniredis import ( + "errors" "math" "strconv" "time" @@ -48,13 +49,11 @@ func optDuration(c *server.Peer, src string, dest *time.Duration) bool { return true } -func optFloat(c *server.Peer, src string, dest *float64) bool { +func optFloat(src string, dest *float64) error { n, err := strconv.ParseFloat(src, 64) if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) // FIXME - return false + return errors.New(msgInvalidInt) // FIXME } *dest = n - return true + return nil }