From 01462e4d2c73bd1e84bf1f11bfc178130b5b4f59 Mon Sep 17 00:00:00 2001 From: Harmen Date: Wed, 2 Mar 2022 11:48:54 +0100 Subject: [PATCH 1/5] cleanup zrange option parsing --- cmd_sorted_set.go | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/cmd_sorted_set.go b/cmd_sorted_set.go index 1ce97bd1..2ab372e4 100644 --- a/cmd_sorted_set.go +++ b/cmd_sorted_set.go @@ -486,21 +486,30 @@ func (m *Miniredis) makeCmdZrange(reverse bool) server.Cmd { return } - key := args[0] + var opts struct { + Key string + Start int + End int + WithScores bool + } + + opts.Key = args[0] start, err := strconv.Atoi(args[1]) if err != nil { setDirty(c) c.WriteError(msgInvalidInt) return } + opts.Start = start + end, err := strconv.Atoi(args[2]) if err != nil { setDirty(c) c.WriteError(msgInvalidInt) return } + opts.End = end - withScores := false if len(args) > 4 { c.WriteError(msgSyntaxError) return @@ -511,36 +520,36 @@ func (m *Miniredis) makeCmdZrange(reverse bool) server.Cmd { c.WriteError(msgSyntaxError) return } - withScores = true + opts.WithScores = true } withTx(m, c, func(c *server.Peer, ctx *connCtx) { db := m.db(ctx.selectedDB) - if !db.exists(key) { + if !db.exists(opts.Key) { c.WriteLen(0) return } - if db.t(key) != "zset" { + if db.t(opts.Key) != "zset" { c.WriteError(ErrWrongType.Error()) return } - members := db.ssetMembers(key) + members := db.ssetMembers(opts.Key) if reverse { reverseSlice(members) } - rs, re := redisRange(len(members), start, end, false) - if withScores { + rs, re := redisRange(len(members), opts.Start, opts.End, false) + if opts.WithScores { c.WriteLen((re - rs) * 2) } else { c.WriteLen(re - rs) } for _, el := range members[rs:re] { c.WriteBulk(el) - if withScores { - c.WriteFloat(db.ssetScore(key, el)) + if opts.WithScores { + c.WriteFloat(db.ssetScore(opts.Key, el)) } } }) @@ -1075,7 +1084,7 @@ func parseFloatRange(s string) (float64, bool, error) { return f, inclusive, err } -// parseLexrange handles ZRANGEBYLEX ranges. They start with '[', '(', or are +// parseLexrange handles ZRANGE{,BYLEX} ranges. They start with '[', '(', or are // '+' or '-'. // Returns range, inclusive, error. // On '+' or '-' that's just returned. From a37a69710a8a5337268acb8c05f5a0ba72581806 Mon Sep 17 00:00:00 2001 From: Harmen Date: Wed, 2 Mar 2022 12:14:37 +0100 Subject: [PATCH 2/5] introduce optInt() to dedup the int parsing done everywhere --- cmd_sorted_set.go | 28 +++++++++------------------- cmd_string.go | 46 ++++++++++++++++++++++------------------------ opts.go | 21 +++++++++++++++++++++ 3 files changed, 52 insertions(+), 43 deletions(-) create mode 100644 opts.go diff --git a/cmd_sorted_set.go b/cmd_sorted_set.go index 2ab372e4..2739d9be 100644 --- a/cmd_sorted_set.go +++ b/cmd_sorted_set.go @@ -494,33 +494,23 @@ func (m *Miniredis) makeCmdZrange(reverse bool) server.Cmd { } opts.Key = args[0] - start, err := strconv.Atoi(args[1]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + if ok := optInt(c, args[1], &opts.Start); !ok { return } - opts.Start = start - - end, err := strconv.Atoi(args[2]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + if ok := optInt(c, args[2], &opts.End); !ok { return } - opts.End = end + args = args[3:] - if len(args) > 4 { - c.WriteError(msgSyntaxError) - return - } - if len(args) == 4 { - if strings.ToLower(args[3]) != "withscores" { - setDirty(c) + for len(args) > 0 { + switch strings.ToLower(args[0]) { + case "withscores": + opts.WithScores = true + args = args[1:] + default: c.WriteError(msgSyntaxError) return } - opts.WithScores = true } withTx(m, c, func(c *server.Peer, ctx *connCtx) { diff --git a/cmd_string.go b/cmd_string.go index ce2589fc..ddf76ed9 100644 --- a/cmd_string.go +++ b/cmd_string.go @@ -993,50 +993,48 @@ func (m *Miniredis) cmdBitpos(c *server.Peer, cmd string, args []string) { return } - key := args[0] - bit, err := strconv.Atoi(args[1]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + var opts struct { + Key string + Bit int + Start int + End int + WithEnd bool + } + + opts.Key = args[0] + if ok := optInt(c, args[1], &opts.Bit); !ok { return } - var start, end int - withEnd := false if len(args) > 2 { - start, err = strconv.Atoi(args[2]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + if ok := optInt(c, args[2], &opts.Start); !ok { return } } if len(args) > 3 { - end, err = strconv.Atoi(args[3]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + if ok := optInt(c, args[3], &opts.End); !ok { return } - withEnd = true + opts.WithEnd = true } withTx(m, c, func(c *server.Peer, ctx *connCtx) { db := m.db(ctx.selectedDB) - if t, ok := db.keys[key]; ok && t != "string" { + if t, ok := db.keys[opts.Key]; ok && t != "string" { c.WriteError(msgWrongType) return } else if !ok { // non-existing key behaves differently - if bit == 0 { + if opts.Bit == 0 { c.WriteInt(0) } else { c.WriteInt(-1) } return } - value := db.stringKeys[key] - + value := db.stringKeys[opts.Key] + start := opts.Start + end := opts.End if start < 0 { start += len(value) if start < 0 { @@ -1047,7 +1045,7 @@ func (m *Miniredis) cmdBitpos(c *server.Peer, cmd string, args []string) { start = len(value) } - if withEnd { + if opts.WithEnd { if end < 0 { end += len(value) } @@ -1062,20 +1060,20 @@ func (m *Miniredis) cmdBitpos(c *server.Peer, cmd string, args []string) { end = len(value) } - if start != 0 || withEnd { + if start != 0 || opts.WithEnd { if end < start { value = "" } else { value = value[start:end] } } - pos := bitPos([]byte(value), bit == 1) + pos := bitPos([]byte(value), opts.Bit == 1) if pos >= 0 { pos += start * 8 } // Special case when looking for 0, but not when start and end are // given. - if bit == 0 && pos == -1 && !withEnd && len(value) > 0 { + if opts.Bit == 0 && pos == -1 && !opts.WithEnd && len(value) > 0 { pos = start*8 + len(value)*8 } c.WriteInt(pos) diff --git a/opts.go b/opts.go new file mode 100644 index 00000000..016d2682 --- /dev/null +++ b/opts.go @@ -0,0 +1,21 @@ +package miniredis + +import ( + "strconv" + + "github.com/alicebob/miniredis/v2/server" +) + +// optInt parses an int option in a command. +// Writes "invalid integer" error to c if it's not a valid integer. Returns +// whether or not things were okay. +func optInt(c *server.Peer, src string, dest *int) bool { + n, err := strconv.Atoi(src) + if err != nil { + setDirty(c) + c.WriteError(msgInvalidInt) + return false + } + *dest = n + return true +} From baa6b88cda15efd0f36712fd155b4aba8ba3995b Mon Sep 17 00:00:00 2001 From: Harmen Date: Wed, 2 Mar 2022 12:27:06 +0100 Subject: [PATCH 3/5] cleanup of some sorted set related code Should be no functional change. --- cmd_sorted_set.go | 155 ++++++++++++++++++++-------------------------- opts.go | 33 ++++++++++ 2 files changed, 100 insertions(+), 88 deletions(-) diff --git a/cmd_sorted_set.go b/cmd_sorted_set.go index 2739d9be..c8933240 100644 --- a/cmd_sorted_set.go +++ b/cmd_sorted_set.go @@ -11,10 +11,6 @@ import ( "github.com/alicebob/miniredis/v2/server" ) -var ( - errInvalidRangeItem = errors.New(msgInvalidRangeItem) -) - // commandsSortedSet handles all sorted set operations. func commandsSortedSet(m *Miniredis) { m.srv.Register("ZADD", m.cmdZadd) @@ -435,37 +431,39 @@ func (m *Miniredis) cmdZlexcount(c *server.Peer, cmd string, args []string) { return } - key := args[0] - min, minIncl, err := parseLexrange(args[1]) - if err != nil { - setDirty(c) - c.WriteError(err.Error()) + var opts struct { + Key string + Min string + MinIncl bool + Max string + MaxIncl bool + } + + opts.Key = args[0] + if ok := optLexrange(c, args[1], &opts.Min, &opts.MinIncl); !ok { return } - max, maxIncl, err := parseLexrange(args[2]) - if err != nil { - setDirty(c) - c.WriteError(err.Error()) + if ok := optLexrange(c, args[2], &opts.Max, &opts.MaxIncl); !ok { return } withTx(m, c, func(c *server.Peer, ctx *connCtx) { db := m.db(ctx.selectedDB) - if !db.exists(key) { + if !db.exists(opts.Key) { c.WriteInt(0) return } - if db.t(key) != "zset" { + if db.t(opts.Key) != "zset" { c.WriteError(ErrWrongType.Error()) return } - members := db.ssetMembers(key) + members := db.ssetMembers(opts.Key) // Just key sort. If scores are not the same we don't care. sort.Strings(members) - members = withLexRange(members, min, minIncl, max, maxIncl) + members = withLexRange(members, opts.Min, opts.MinIncl, opts.Max, opts.MaxIncl) c.WriteInt(len(members)) }) @@ -561,69 +559,69 @@ func (m *Miniredis) makeCmdZrangebylex(reverse bool) server.Cmd { return } - key := args[0] - min, minIncl, err := parseLexrange(args[1]) - if err != nil { - setDirty(c) - c.WriteError(err.Error()) + var opts struct { + Key string + Min string + MinIncl bool + Max string + MaxIncl bool + WithLimit bool + LimitStart int + LimitEnd int + } + + opts.Key = args[0] + if ok := optLexrange(c, args[1], &opts.Min, &opts.MinIncl); !ok { return } - max, maxIncl, err := parseLexrange(args[2]) - if err != nil { - setDirty(c) - c.WriteError(err.Error()) + if ok := optLexrange(c, args[2], &opts.Max, &opts.MaxIncl); !ok { return } args = args[3:] - withLimit := false - limitStart := 0 - limitEnd := 0 for len(args) > 0 { - if strings.ToLower(args[0]) == "limit" { - withLimit = true + switch strings.ToLower(args[0]) { + case "limit": + opts.WithLimit = true args = args[1:] if len(args) < 2 { c.WriteError(msgSyntaxError) return } - limitStart, err = strconv.Atoi(args[0]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + if ok := optInt(c, args[0], &opts.LimitStart); !ok { return } - limitEnd, err = strconv.Atoi(args[1]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) + if ok := optInt(c, args[1], &opts.LimitEnd); !ok { return } args = args[2:] continue + default: + // Syntax error + setDirty(c) + c.WriteError(msgSyntaxError) + return } - // Syntax error - setDirty(c) - c.WriteError(msgSyntaxError) - return } withTx(m, c, func(c *server.Peer, ctx *connCtx) { db := m.db(ctx.selectedDB) - if !db.exists(key) { + if !db.exists(opts.Key) { c.WriteLen(0) return } - if db.t(key) != "zset" { + if db.t(opts.Key) != "zset" { c.WriteError(ErrWrongType.Error()) return } - members := db.ssetMembers(key) + members := db.ssetMembers(opts.Key) // Just key sort. If scores are not the same we don't care. sort.Strings(members) + min, max := opts.Min, opts.Max + minIncl, maxIncl := opts.MinIncl, opts.MaxIncl if reverse { min, max = max, min minIncl, maxIncl = maxIncl, minIncl @@ -634,19 +632,19 @@ func (m *Miniredis) makeCmdZrangebylex(reverse bool) server.Cmd { } // Apply LIMIT ranges. That's . Unlike RANGE. - if withLimit { - if limitStart < 0 { + if opts.WithLimit { + if opts.LimitStart < 0 { members = nil } else { - if limitStart < len(members) { - members = members[limitStart:] + if opts.LimitStart < len(members) { + members = members[opts.LimitStart:] } else { // out of range members = nil } - if limitEnd >= 0 { - if len(members) > limitEnd { - members = members[:limitEnd] + if opts.LimitEnd >= 0 { + if len(members) > opts.LimitEnd { + members = members[:opts.LimitEnd] } } } @@ -881,40 +879,42 @@ func (m *Miniredis) cmdZremrangebylex(c *server.Peer, cmd string, args []string) return } - key := args[0] - min, minIncl, err := parseLexrange(args[1]) - if err != nil { - setDirty(c) - c.WriteError(err.Error()) + var opts struct { + Key string + Min string + MinIncl bool + Max string + MaxIncl bool + } + + opts.Key = args[0] + if ok := optLexrange(c, args[1], &opts.Min, &opts.MinIncl); !ok { return } - max, maxIncl, err := parseLexrange(args[2]) - if err != nil { - setDirty(c) - c.WriteError(err.Error()) + if ok := optLexrange(c, args[2], &opts.Max, &opts.MaxIncl); !ok { return } withTx(m, c, func(c *server.Peer, ctx *connCtx) { db := m.db(ctx.selectedDB) - if !db.exists(key) { + if !db.exists(opts.Key) { c.WriteInt(0) return } - if db.t(key) != "zset" { + if db.t(opts.Key) != "zset" { c.WriteError(ErrWrongType.Error()) return } - members := db.ssetMembers(key) + members := db.ssetMembers(opts.Key) // Just key sort. If scores are not the same we don't care. sort.Strings(members) - members = withLexRange(members, min, minIncl, max, maxIncl) + members = withLexRange(members, opts.Min, opts.MinIncl, opts.Max, opts.MaxIncl) for _, el := range members { - db.ssetRem(key, el) + db.ssetRem(opts.Key, el) } c.WriteInt(len(members)) }) @@ -1074,27 +1074,6 @@ func parseFloatRange(s string) (float64, bool, error) { return f, inclusive, err } -// parseLexrange handles ZRANGE{,BYLEX} ranges. They start with '[', '(', or are -// '+' or '-'. -// Returns range, inclusive, error. -// On '+' or '-' that's just returned. -func parseLexrange(s string) (string, bool, error) { - if len(s) == 0 { - return "", false, errInvalidRangeItem - } - if s == "+" || s == "-" { - return s, false, nil - } - switch s[0] { - case '(': - return s[1:], false, nil - case '[': - return s[1:], true, nil - default: - return "", false, errInvalidRangeItem - } -} - // withSSRange limits a list of sorted set elements by the ZRANGEBYSCORE range // logic. func withSSRange(members ssElems, min float64, minIncl bool, max float64, maxIncl bool) ssElems { diff --git a/opts.go b/opts.go index 016d2682..86650818 100644 --- a/opts.go +++ b/opts.go @@ -19,3 +19,36 @@ func optInt(c *server.Peer, src string, dest *int) bool { *dest = n return true } + +// optLexrange handles ZRANGE{,BYLEX} ranges. They start with '[', '(', or are +// '+' or '-'. +// Sets destValue and destInclusive. destValue can be '+' or '-'. +// Returns whether or not things were okay. +func optLexrange(c *server.Peer, s string, destValue *string, destInclusive *bool) bool { + if len(s) == 0 { + setDirty(c) + c.WriteError(msgInvalidRangeItem) + return false + } + + if s == "+" || s == "-" { + *destValue = s + *destInclusive = false + return true + } + + switch s[0] { + case '(': + *destValue = s[1:] + *destInclusive = false + return true + case '[': + *destValue = s[1:] + *destInclusive = true + return true + default: + setDirty(c) + c.WriteError(msgInvalidRangeItem) + return false + } +} From a2b33f081c12c61ab3e0b0c8a34ed03c3da7e1e3 Mon Sep 17 00:00:00 2001 From: Harmen Date: Tue, 8 Mar 2022 13:01:31 +0100 Subject: [PATCH 4/5] split up zrange and zrevrange Zrevrange is "legacy" and shouldn't get the updates for zrange. --- cmd_sorted_set.go | 187 +++++++++++++++++++++++---------- cmd_sorted_set_test.go | 153 ++++++++++++++++++--------- integration/sorted_set_test.go | 3 +- 3 files changed, 234 insertions(+), 109 deletions(-) diff --git a/cmd_sorted_set.go b/cmd_sorted_set.go index c8933240..6a70b89f 100644 --- a/cmd_sorted_set.go +++ b/cmd_sorted_set.go @@ -19,7 +19,7 @@ func commandsSortedSet(m *Miniredis) { m.srv.Register("ZINCRBY", m.cmdZincrby) m.srv.Register("ZINTERSTORE", m.cmdZinterstore) m.srv.Register("ZLEXCOUNT", m.cmdZlexcount) - m.srv.Register("ZRANGE", m.makeCmdZrange(false)) + m.srv.Register("ZRANGE", m.cmdZrange) m.srv.Register("ZRANGEBYLEX", m.makeCmdZrangebylex(false)) m.srv.Register("ZRANGEBYSCORE", m.makeCmdZrangebyscore(false)) m.srv.Register("ZRANK", m.makeCmdZrank(false)) @@ -27,7 +27,7 @@ func commandsSortedSet(m *Miniredis) { m.srv.Register("ZREMRANGEBYLEX", m.cmdZremrangebylex) m.srv.Register("ZREMRANGEBYRANK", m.cmdZremrangebyrank) m.srv.Register("ZREMRANGEBYSCORE", m.cmdZremrangebyscore) - m.srv.Register("ZREVRANGE", m.makeCmdZrange(true)) + m.srv.Register("ZREVRANGE", m.cmdZrevrange) m.srv.Register("ZREVRANGEBYLEX", m.makeCmdZrangebylex(true)) m.srv.Register("ZREVRANGEBYSCORE", m.makeCmdZrangebyscore(true)) m.srv.Register("ZREVRANK", m.makeCmdZrank(true)) @@ -469,79 +469,152 @@ func (m *Miniredis) cmdZlexcount(c *server.Peer, cmd string, args []string) { }) } -// ZRANGE and ZREVRANGE -func (m *Miniredis) makeCmdZrange(reverse bool) server.Cmd { - return func(c *server.Peer, cmd string, args []string) { - if len(args) < 3 { - setDirty(c) - c.WriteError(errWrongNumber(cmd)) +// ZRANGE +func (m *Miniredis) cmdZrange(c *server.Peer, cmd string, args []string) { + if len(args) < 3 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + if !m.handleAuth(c) { + return + } + if m.checkPubsub(c, cmd) { + return + } + + var opts struct { + Key string + Start int + End int + WithScores bool + Reverse bool + } + + opts.Key = args[0] + if ok := optInt(c, args[1], &opts.Start); !ok { + return + } + if ok := optInt(c, args[2], &opts.End); !ok { + return + } + args = args[3:] + + for len(args) > 0 { + switch strings.ToLower(args[0]) { + case "withscores": + opts.WithScores = true + args = args[1:] + default: + c.WriteError(msgSyntaxError) return } - if !m.handleAuth(c) { + } + + withTx(m, c, func(c *server.Peer, ctx *connCtx) { + db := m.db(ctx.selectedDB) + + if !db.exists(opts.Key) { + c.WriteLen(0) return } - if m.checkPubsub(c, cmd) { + + if db.t(opts.Key) != "zset" { + c.WriteError(ErrWrongType.Error()) return } - var opts struct { - Key string - Start int - End int - WithScores bool + members := db.ssetMembers(opts.Key) + if opts.Reverse { + reverseSlice(members) } - - opts.Key = args[0] - if ok := optInt(c, args[1], &opts.Start); !ok { - return + rs, re := redisRange(len(members), opts.Start, opts.End, false) + if opts.WithScores { + c.WriteLen((re - rs) * 2) + } else { + c.WriteLen(re - rs) } - if ok := optInt(c, args[2], &opts.End); !ok { - return + for _, el := range members[rs:re] { + c.WriteBulk(el) + if opts.WithScores { + c.WriteFloat(db.ssetScore(opts.Key, el)) + } } - args = args[3:] + }) +} - for len(args) > 0 { - switch strings.ToLower(args[0]) { - case "withscores": - opts.WithScores = true - args = args[1:] - default: - c.WriteError(msgSyntaxError) - return - } +// ZREVRANGE +func (m *Miniredis) cmdZrevrange(c *server.Peer, cmd string, args []string) { + reverse := true + if len(args) < 3 { + setDirty(c) + c.WriteError(errWrongNumber(cmd)) + return + } + if !m.handleAuth(c) { + return + } + if m.checkPubsub(c, cmd) { + return + } + + var opts struct { + Key string + Start int + End int + WithScores bool + } + + opts.Key = args[0] + if ok := optInt(c, args[1], &opts.Start); !ok { + return + } + if ok := optInt(c, args[2], &opts.End); !ok { + return + } + args = args[3:] + + for len(args) > 0 { + switch strings.ToLower(args[0]) { + case "withscores": + opts.WithScores = true + args = args[1:] + default: + c.WriteError(msgSyntaxError) + return } + } - withTx(m, c, func(c *server.Peer, ctx *connCtx) { - db := m.db(ctx.selectedDB) + withTx(m, c, func(c *server.Peer, ctx *connCtx) { + db := m.db(ctx.selectedDB) - if !db.exists(opts.Key) { - c.WriteLen(0) - return - } + if !db.exists(opts.Key) { + c.WriteLen(0) + return + } - if db.t(opts.Key) != "zset" { - c.WriteError(ErrWrongType.Error()) - return - } + if db.t(opts.Key) != "zset" { + c.WriteError(ErrWrongType.Error()) + return + } - members := db.ssetMembers(opts.Key) - if reverse { - reverseSlice(members) - } - rs, re := redisRange(len(members), opts.Start, opts.End, false) + members := db.ssetMembers(opts.Key) + if reverse { + reverseSlice(members) + } + rs, re := redisRange(len(members), opts.Start, opts.End, false) + if opts.WithScores { + c.WriteLen((re - rs) * 2) + } else { + c.WriteLen(re - rs) + } + for _, el := range members[rs:re] { + c.WriteBulk(el) if opts.WithScores { - c.WriteLen((re - rs) * 2) - } else { - c.WriteLen(re - rs) + c.WriteFloat(db.ssetScore(opts.Key, el)) } - for _, el := range members[rs:re] { - c.WriteBulk(el) - if opts.WithScores { - c.WriteFloat(db.ssetScore(opts.Key, el)) - } - } - }) - } + } + }) } // ZRANGEBYLEX and ZREVRANGEBYLEX diff --git a/cmd_sorted_set_test.go b/cmd_sorted_set_test.go index 15963ae2..f3ca482a 100644 --- a/cmd_sorted_set_test.go +++ b/cmd_sorted_set_test.go @@ -282,9 +282,7 @@ func TestSortedSetAdd(t *testing.T) { }) } -// Test ZRANGE and ZREVRANGE func TestSortedSetRange(t *testing.T) { - // ZREVRANGE is the same code as ZRANGE s, err := Run() ok(t, err) defer s.Close() @@ -299,85 +297,53 @@ func TestSortedSetRange(t *testing.T) { s.ZAdd("z", 3, "drei") s.ZAdd("z", math.Inf(+1), "inf") - { + t.Run("basic", func(t *testing.T) { mustDo(t, c, "ZRANGE", "z", "0", "-1", proto.Strings("one", "two", "zwei", "drei", "three", "inf"), ) - - mustDo(t, c, - "ZREVRANGE", "z", "0", "-1", - proto.Strings("inf", "three", "drei", "zwei", "two", "one"), - ) - } - { mustDo(t, c, "ZRANGE", "z", "0", "1", proto.Strings("one", "two"), ) - - mustDo(t, c, - "ZREVRANGE", "z", "0", "1", - proto.Strings("inf", "three"), - ) - } - { mustDo(t, c, "ZRANGE", "z", "-1", "-1", proto.Strings("inf"), ) - + // weird cases. mustDo(t, c, - "ZREVRANGE", "z", "-1", "-1", - proto.Strings("one"), + "ZRANGE", "z", "-100", "-100", + proto.Strings(), + ) + mustDo(t, c, + "ZRANGE", "z", "100", "400", + proto.Strings(), ) - } - - // weird cases. - mustDo(t, c, - "ZRANGE", "z", "-100", "-100", - proto.Strings(), - ) - mustDo(t, c, - "ZRANGE", "z", "100", "400", - proto.Strings(), - ) - - // Nonexistent key - mustDo(t, c, - "ZRANGE", "nosuch", "1", "4", - proto.Strings(), - ) - // With scores - { + // Nonexistent key mustDo(t, c, - "ZRANGE", "z", "1", "2", "WITHSCORES", - proto.Strings("two", "2", "zwei", "2"), + "ZRANGE", "nosuch", "1", "4", + proto.Strings(), ) + }) + t.Run("withscores", func(t *testing.T) { mustDo(t, c, - "ZREVRANGE", "z", "1", "2", "WITHSCORES", - proto.Strings("three", "3", "drei", "3"), + "ZRANGE", "z", "1", "2", "WITHSCORES", + proto.Strings("two", "2", "zwei", "2"), ) - } - // INF in WITHSCORES - { + // INF in WITHSCORES mustDo(t, c, "ZRANGE", "z", "4", "-1", "WITHSCORES", proto.Strings("three", "3", "inf", "inf"), ) - } + }) t.Run("errors", func(t *testing.T) { mustDo(t, c, "ZRANGE", proto.Error(errWrongNumber("zrange")), ) - mustDo(t, c, - "ZREVRANGE", - proto.Error(errWrongNumber("zrevrange")), - ) mustDo(t, c, "ZRANGE", "set", proto.Error(errWrongNumber("zrange")), @@ -407,6 +373,91 @@ func TestSortedSetRange(t *testing.T) { }) } +// Test ZREVRANGE +func TestSortedSetRevRange(t *testing.T) { + s, err := Run() + ok(t, err) + defer s.Close() + c, err := proto.Dial(s.Addr()) + ok(t, err) + defer c.Close() + + s.ZAdd("z", 1, "one") + s.ZAdd("z", 2, "two") + s.ZAdd("z", 2, "zwei") + s.ZAdd("z", 3, "three") + s.ZAdd("z", 3, "drei") + s.ZAdd("z", math.Inf(+1), "inf") + + mustDo(t, c, + "ZREVRANGE", "z", "0", "-1", + proto.Strings("inf", "three", "drei", "zwei", "two", "one"), + ) + mustDo(t, c, + "ZREVRANGE", "z", "0", "1", + proto.Strings("inf", "three"), + ) + mustDo(t, c, + "ZREVRANGE", "z", "-1", "-1", + proto.Strings("one"), + ) + + // weird cases. + mustDo(t, c, + "ZREVRANGE", "z", "-100", "-100", + proto.Strings(), + ) + mustDo(t, c, + "ZREVRANGE", "z", "100", "400", + proto.Strings(), + ) + + // Nonexistent key + mustDo(t, c, + "ZREVRANGE", "nosuch", "1", "4", + proto.Strings(), + ) + + // With scores + mustDo(t, c, + "ZREVRANGE", "z", "1", "2", "WITHSCORES", + proto.Strings("three", "3", "drei", "3"), + ) + + t.Run("errors", func(t *testing.T) { + mustDo(t, c, + "ZREVRANGE", + proto.Error(errWrongNumber("zrevrange")), + ) + mustDo(t, c, + "ZREVRANGE", "set", + proto.Error(errWrongNumber("zrevrange")), + ) + mustDo(t, c, + "ZREVRANGE", "set", "1", + proto.Error(errWrongNumber("zrevrange")), + ) + mustDo(t, c, + "ZREVRANGE", "set", "noint", "1", + proto.Error(msgInvalidInt), + ) + mustDo(t, c, + "ZREVRANGE", "set", "1", "noint", + proto.Error(msgInvalidInt), + ) + mustDo(t, c, + "ZREVRANGE", "set", "1", "2", "toomany", + proto.Error(msgSyntaxError), + ) + // Wrong type of key + s.Set("str", "value") + mustDo(t, c, + "ZREVRANGE", "str", "1", "2", + proto.Error(msgWrongType), + ) + }) +} + // Test ZRANGEBYSCORE, ZREVRANGEBYSCORE, and ZCOUNT func TestSortedSetRangeByScore(t *testing.T) { s, err := Run() diff --git a/integration/sorted_set_test.go b/integration/sorted_set_test.go index 1c6c3730..44ea3ba3 100644 --- a/integration/sorted_set_test.go +++ b/integration/sorted_set_test.go @@ -171,7 +171,7 @@ func TestSortedSetRange(t *testing.T) { "-Inf", "big bang", ) c.Do("ZRANGE", "z", "0", "-1") - c.Do("ZRANGE", "z", "0", "-1", "WITHSCORES") + c.Do("ZRANGE", "z", "0", "-1", "WITHSCORES", "WITHSCORES") c.Do("ZRANGE", "z", "0", "-1", "WiThScOrEs") c.Do("ZRANGE", "z", "0", "-2") c.Do("ZRANGE", "z", "0", "-1000") @@ -180,6 +180,7 @@ func TestSortedSetRange(t *testing.T) { c.Do("ZRANGE", "z", "300", "-110") c.Do("ZREVRANGE", "z", "0", "-1") c.Do("ZREVRANGE", "z", "0", "-1", "WITHSCORES") + c.Do("ZREVRANGE", "z", "0", "-1", "WITHSCORES", "WITHSCORES", "WITHSCORES") c.Do("ZREVRANGE", "z", "0", "-1", "WiThScOrEs") c.Do("ZREVRANGE", "z", "0", "-2") c.Do("ZREVRANGE", "z", "0", "-1000") From 0c3f48f090606ab773d260fb82a2bf3d2e53a2d1 Mon Sep 17 00:00:00 2001 From: Harmen Date: Wed, 9 Mar 2022 09:19:42 +0100 Subject: [PATCH 5/5] update all zrange* commands --- cmd_sorted_set.go | 636 +++++++++++++++++++-------------- cmd_sorted_set_test.go | 34 +- integration/sorted_set_test.go | 98 ++++- integration/test.go | 3 +- opts.go | 33 -- redis.go | 1 + 6 files changed, 482 insertions(+), 323 deletions(-) diff --git a/cmd_sorted_set.go b/cmd_sorted_set.go index 6a70b89f..5b373d47 100644 --- a/cmd_sorted_set.go +++ b/cmd_sorted_set.go @@ -4,6 +4,7 @@ package miniredis import ( "errors" + "math" "sort" "strconv" "strings" @@ -431,23 +432,24 @@ func (m *Miniredis) cmdZlexcount(c *server.Peer, cmd string, args []string) { return } - var opts struct { - Key string - Min string - MinIncl bool - Max string - MaxIncl bool - } - - opts.Key = args[0] - if ok := optLexrange(c, args[1], &opts.Min, &opts.MinIncl); !ok { - return - } - if ok := optLexrange(c, args[2], &opts.Max, &opts.MaxIncl); !ok { - return + var opts = struct { + Key string + Min string + Max string + }{ + Key: args[0], + Min: args[1], + Max: args[2], } withTx(m, c, func(c *server.Peer, ctx *connCtx) { + min, minIncl, minErr := parseLexrange(opts.Min) + max, maxIncl, maxErr := parseLexrange(opts.Max) + if minErr != nil || maxErr != nil { + c.WriteError(msgInvalidRangeItem) + return + } + db := m.db(ctx.selectedDB) if !db.exists(opts.Key) { @@ -463,7 +465,7 @@ func (m *Miniredis) cmdZlexcount(c *server.Peer, cmd string, args []string) { members := db.ssetMembers(opts.Key) // Just key sort. If scores are not the same we don't care. sort.Strings(members) - members = withLexRange(members, opts.Min, opts.MinIncl, opts.Max, opts.MaxIncl) + members = withLexRange(members, min, minIncl, max, maxIncl) c.WriteInt(len(members)) }) @@ -485,23 +487,41 @@ func (m *Miniredis) cmdZrange(c *server.Peer, cmd string, args []string) { var opts struct { Key string - Start int - End int + Min string + Max string WithScores bool + ByScore bool + ByLex bool Reverse bool + WithLimit bool + Offset string + Count string } - opts.Key = args[0] - if ok := optInt(c, args[1], &opts.Start); !ok { - return - } - if ok := optInt(c, args[2], &opts.End); !ok { - return - } + opts.Key, opts.Min, opts.Max = args[0], args[1], args[2] args = args[3:] for len(args) > 0 { switch strings.ToLower(args[0]) { + case "byscore": + opts.ByScore = true + args = args[1:] + case "bylex": + opts.ByLex = true + args = args[1:] + case "rev": + opts.Reverse = true + args = args[1:] + case "limit": + opts.WithLimit = true + args = args[1:] + if len(args) < 2 { + c.WriteError(msgSyntaxError) + return + } + opts.Offset = args[0] + opts.Count = args[1] + args = args[2:] case "withscores": opts.WithScores = true args = args[1:] @@ -512,40 +532,49 @@ func (m *Miniredis) cmdZrange(c *server.Peer, cmd string, args []string) { } withTx(m, c, func(c *server.Peer, ctx *connCtx) { - db := m.db(ctx.selectedDB) - - if !db.exists(opts.Key) { - c.WriteLen(0) - return - } - - if db.t(opts.Key) != "zset" { - c.WriteError(ErrWrongType.Error()) - return - } - - members := db.ssetMembers(opts.Key) - if opts.Reverse { - reverseSlice(members) - } - rs, re := redisRange(len(members), opts.Start, opts.End, false) - if opts.WithScores { - c.WriteLen((re - rs) * 2) - } else { - c.WriteLen(re - rs) - } - for _, el := range members[rs:re] { - c.WriteBulk(el) - if opts.WithScores { - c.WriteFloat(db.ssetScore(opts.Key, el)) + switch { + case opts.ByScore && opts.ByLex: + c.WriteError(msgSyntaxError) + case opts.ByScore: + runRangeByScore(m, c, ctx, optsRangeByScore{ + Key: opts.Key, + Min: opts.Min, + Max: opts.Max, + Reverse: opts.Reverse, + WithLimit: opts.WithLimit, + Offset: opts.Offset, + Count: opts.Count, + WithScores: opts.WithScores, + }) + case opts.ByLex: + runRangeByLex(m, c, ctx, optsRangeByLex{ + Key: opts.Key, + Min: opts.Min, + Max: opts.Max, + Reverse: opts.Reverse, + WithLimit: opts.WithLimit, + Offset: opts.Offset, + Count: opts.Count, + WithScores: opts.WithScores, + }) + default: + if opts.WithLimit { + c.WriteError(msgLimitCombination) + return } + runRange(m, c, ctx, optsRange{ + Key: opts.Key, + Min: opts.Min, + Max: opts.Max, + Reverse: opts.Reverse, + WithScores: opts.WithScores, + }) } }) } // ZREVRANGE func (m *Miniredis) cmdZrevrange(c *server.Peer, cmd string, args []string) { - reverse := true if len(args) < 3 { setDirty(c) c.WriteError(errWrongNumber(cmd)) @@ -558,19 +587,11 @@ func (m *Miniredis) cmdZrevrange(c *server.Peer, cmd string, args []string) { return } - var opts struct { - Key string - Start int - End int - WithScores bool - } - - opts.Key = args[0] - if ok := optInt(c, args[1], &opts.Start); !ok { - return - } - if ok := optInt(c, args[2], &opts.End); !ok { - return + var opts = optsRange{ + Reverse: true, + Key: args[0], + Min: args[1], + Max: args[2], } args = args[3:] @@ -585,35 +606,8 @@ func (m *Miniredis) cmdZrevrange(c *server.Peer, cmd string, args []string) { } } - withTx(m, c, func(c *server.Peer, ctx *connCtx) { - db := m.db(ctx.selectedDB) - - if !db.exists(opts.Key) { - c.WriteLen(0) - return - } - - if db.t(opts.Key) != "zset" { - c.WriteError(ErrWrongType.Error()) - return - } - - members := db.ssetMembers(opts.Key) - if reverse { - reverseSlice(members) - } - rs, re := redisRange(len(members), opts.Start, opts.End, false) - if opts.WithScores { - c.WriteLen((re - rs) * 2) - } else { - c.WriteLen(re - rs) - } - for _, el := range members[rs:re] { - c.WriteBulk(el) - if opts.WithScores { - c.WriteFloat(db.ssetScore(opts.Key, el)) - } - } + withTx(m, c, func(c *server.Peer, cctx *connCtx) { + runRange(m, c, cctx, opts) }) } @@ -631,24 +625,11 @@ func (m *Miniredis) makeCmdZrangebylex(reverse bool) server.Cmd { if m.checkPubsub(c, cmd) { return } - - var opts struct { - Key string - Min string - MinIncl bool - Max string - MaxIncl bool - WithLimit bool - LimitStart int - LimitEnd int - } - - opts.Key = args[0] - if ok := optLexrange(c, args[1], &opts.Min, &opts.MinIncl); !ok { - return - } - if ok := optLexrange(c, args[2], &opts.Max, &opts.MaxIncl); !ok { - return + opts := optsRangeByLex{ + Reverse: reverse, + Key: args[0], + Min: args[1], + Max: args[2], } args = args[3:] @@ -661,12 +642,8 @@ func (m *Miniredis) makeCmdZrangebylex(reverse bool) server.Cmd { c.WriteError(msgSyntaxError) return } - if ok := optInt(c, args[0], &opts.LimitStart); !ok { - return - } - if ok := optInt(c, args[1], &opts.LimitEnd); !ok { - return - } + opts.Offset = args[0] + opts.Count = args[1] args = args[2:] continue default: @@ -677,56 +654,8 @@ func (m *Miniredis) makeCmdZrangebylex(reverse bool) server.Cmd { } } - withTx(m, c, func(c *server.Peer, ctx *connCtx) { - db := m.db(ctx.selectedDB) - - if !db.exists(opts.Key) { - c.WriteLen(0) - return - } - - if db.t(opts.Key) != "zset" { - c.WriteError(ErrWrongType.Error()) - return - } - - members := db.ssetMembers(opts.Key) - // Just key sort. If scores are not the same we don't care. - sort.Strings(members) - min, max := opts.Min, opts.Max - minIncl, maxIncl := opts.MinIncl, opts.MaxIncl - if reverse { - min, max = max, min - minIncl, maxIncl = maxIncl, minIncl - } - members = withLexRange(members, min, minIncl, max, maxIncl) - if reverse { - reverseSlice(members) - } - - // Apply LIMIT ranges. That's . Unlike RANGE. - if opts.WithLimit { - if opts.LimitStart < 0 { - members = nil - } else { - if opts.LimitStart < len(members) { - members = members[opts.LimitStart:] - } else { - // out of range - members = nil - } - if opts.LimitEnd >= 0 { - if len(members) > opts.LimitEnd { - members = members[:opts.LimitEnd] - } - } - } - } - - c.WriteLen(len(members)) - for _, el := range members { - c.WriteBulk(el) - } + withTx(m, c, func(c *server.Peer, cctx *connCtx) { + runRangeByLex(m, c, cctx, opts) }) } } @@ -746,50 +675,29 @@ func (m *Miniredis) makeCmdZrangebyscore(reverse bool) server.Cmd { return } - key := args[0] - min, minIncl, err := parseFloatRange(args[1]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidMinMax) - return - } - max, maxIncl, err := parseFloatRange(args[2]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidMinMax) - return + var opts = optsRangeByScore{ + Reverse: reverse, + Key: args[0], + Min: args[1], + Max: args[2], } args = args[3:] - withScores := false - withLimit := false - limitStart := 0 - limitEnd := 0 for len(args) > 0 { if strings.ToLower(args[0]) == "limit" { - withLimit = true + opts.WithLimit = true args = args[1:] if len(args) < 2 { c.WriteError(msgSyntaxError) return } - limitStart, err = strconv.Atoi(args[0]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) - return - } - limitEnd, err = strconv.Atoi(args[1]) - if err != nil { - setDirty(c) - c.WriteError(msgInvalidInt) - return - } + opts.Offset = args[0] + opts.Count = args[1] args = args[2:] continue } if strings.ToLower(args[0]) == "withscores" { - withScores = true + opts.WithScores = true args = args[1:] continue } @@ -798,59 +706,8 @@ func (m *Miniredis) makeCmdZrangebyscore(reverse bool) server.Cmd { return } - withTx(m, c, func(c *server.Peer, ctx *connCtx) { - db := m.db(ctx.selectedDB) - - if !db.exists(key) { - c.WriteLen(0) - return - } - - if db.t(key) != "zset" { - c.WriteError(ErrWrongType.Error()) - return - } - - members := db.ssetElements(key) - if reverse { - min, max = max, min - minIncl, maxIncl = maxIncl, minIncl - } - members = withSSRange(members, min, minIncl, max, maxIncl) - if reverse { - reverseElems(members) - } - - // Apply LIMIT ranges. That's . Unlike RANGE. - if withLimit { - if limitStart < 0 { - members = ssElems{} - } else { - if limitStart < len(members) { - members = members[limitStart:] - } else { - // out of range - members = ssElems{} - } - if limitEnd >= 0 { - if len(members) > limitEnd { - members = members[:limitEnd] - } - } - } - } - - if withScores { - c.WriteLen(len(members) * 2) - } else { - c.WriteLen(len(members)) - } - for _, el := range members { - c.WriteBulk(el.member) - if withScores { - c.WriteFloat(el.score) - } - } + withTx(m, c, func(c *server.Peer, cctx *connCtx) { + runRangeByScore(m, c, cctx, opts) }) } } @@ -952,23 +809,24 @@ func (m *Miniredis) cmdZremrangebylex(c *server.Peer, cmd string, args []string) return } - var opts struct { - Key string - Min string - MinIncl bool - Max string - MaxIncl bool - } - - opts.Key = args[0] - if ok := optLexrange(c, args[1], &opts.Min, &opts.MinIncl); !ok { - return - } - if ok := optLexrange(c, args[2], &opts.Max, &opts.MaxIncl); !ok { - return + var opts = struct { + Key string + Min string + Max string + }{ + Key: args[0], + Min: args[1], + Max: args[2], } withTx(m, c, func(c *server.Peer, ctx *connCtx) { + min, minIncl, minErr := parseLexrange(opts.Min) + max, maxIncl, maxErr := parseLexrange(opts.Max) + if minErr != nil || maxErr != nil { + c.WriteError(msgInvalidRangeItem) + return + } + db := m.db(ctx.selectedDB) if !db.exists(opts.Key) { @@ -984,7 +842,7 @@ func (m *Miniredis) cmdZremrangebylex(c *server.Peer, cmd string, args []string) members := db.ssetMembers(opts.Key) // Just key sort. If scores are not the same we don't care. sort.Strings(members) - members = withLexRange(members, opts.Min, opts.MinIncl, opts.Max, opts.MaxIncl) + members = withLexRange(members, min, minIncl, max, maxIncl) for _, el := range members { db.ssetRem(opts.Key, el) @@ -1143,8 +1001,15 @@ func parseFloatRange(s string) (float64, bool, error) { s = s[1:] inclusive = false } - f, err := strconv.ParseFloat(s, 64) - return f, inclusive, err + switch strings.ToLower(s) { + case "+inf": + return math.Inf(+1), true, nil + case "-inf": + return math.Inf(-1), true, nil + default: + f, err := strconv.ParseFloat(s, 64) + return f, inclusive, err + } } // withSSRange limits a list of sorted set elements by the ZRANGEBYSCORE range @@ -1726,3 +1591,240 @@ func (m *Miniredis) cmdZrandmember(c *server.Peer, cmd string, args []string) { c.WriteStrings(members) }) } + +type optsRange struct { + Key string + Min string + Max string + Reverse bool + WithScores bool +} + +func runRange(m *Miniredis, c *server.Peer, cctx *connCtx, opts optsRange) { + min, minErr := strconv.Atoi(opts.Min) + max, maxErr := strconv.Atoi(opts.Max) + if minErr != nil || maxErr != nil { + c.WriteError(msgInvalidInt) + return + } + + db := m.db(cctx.selectedDB) + + if !db.exists(opts.Key) { + c.WriteLen(0) + return + } + + if db.t(opts.Key) != "zset" { + c.WriteError(ErrWrongType.Error()) + return + } + + members := db.ssetMembers(opts.Key) + if opts.Reverse { + reverseSlice(members) + } + rs, re := redisRange(len(members), min, max, false) + if opts.WithScores { + c.WriteLen((re - rs) * 2) + } else { + c.WriteLen(re - rs) + } + for _, el := range members[rs:re] { + c.WriteBulk(el) + if opts.WithScores { + c.WriteFloat(db.ssetScore(opts.Key, el)) + } + } +} + +type optsRangeByScore struct { + Key string + Min string + Max string + Reverse bool + WithLimit bool + Offset string + Count string + WithScores bool +} + +func runRangeByScore(m *Miniredis, c *server.Peer, cctx *connCtx, opts optsRangeByScore) { + var limitOffset, limitCount int + var err error + if opts.WithLimit { + limitOffset, err = strconv.Atoi(opts.Offset) + if err != nil { + c.WriteError(msgInvalidInt) + return + } + limitCount, err = strconv.Atoi(opts.Count) + if err != nil { + c.WriteError(msgInvalidInt) + return + } + } + min, minIncl, minErr := parseFloatRange(opts.Min) + max, maxIncl, maxErr := parseFloatRange(opts.Max) + if minErr != nil || maxErr != nil { + c.WriteError(msgInvalidMinMax) + return + } + + db := m.db(cctx.selectedDB) + + if !db.exists(opts.Key) { + c.WriteLen(0) + return + } + + if db.t(opts.Key) != "zset" { + c.WriteError(ErrWrongType.Error()) + return + } + + members := db.ssetElements(opts.Key) + if opts.Reverse { + min, max = max, min + minIncl, maxIncl = maxIncl, minIncl + } + members = withSSRange(members, min, minIncl, max, maxIncl) + if opts.Reverse { + reverseElems(members) + } + + // Apply LIMIT ranges. That's . Unlike RANGE. + if opts.WithLimit { + if limitOffset < 0 { + members = ssElems{} + } else { + if limitOffset < len(members) { + members = members[limitOffset:] + } else { + // out of range + members = ssElems{} + } + if limitCount >= 0 { + if len(members) > limitCount { + members = members[:limitCount] + } + } + } + } + + if opts.WithScores { + c.WriteLen(len(members) * 2) + } else { + c.WriteLen(len(members)) + } + for _, el := range members { + c.WriteBulk(el.member) + if opts.WithScores { + c.WriteFloat(el.score) + } + } +} + +type optsRangeByLex struct { + Key string + Min string + Max string + Reverse bool + WithLimit bool + Offset string + Count string + WithScores bool +} + +func runRangeByLex(m *Miniredis, c *server.Peer, cctx *connCtx, opts optsRangeByLex) { + var limitOffset, limitCount int + var err error + if opts.WithLimit { + limitOffset, err = strconv.Atoi(opts.Offset) + if err != nil { + c.WriteError(msgInvalidInt) + return + } + limitCount, err = strconv.Atoi(opts.Count) + if err != nil { + c.WriteError(msgInvalidInt) + return + } + } + min, minIncl, minErr := parseLexrange(opts.Min) + max, maxIncl, maxErr := parseLexrange(opts.Max) + if minErr != nil || maxErr != nil { + c.WriteError(msgInvalidRangeItem) + return + } + + db := m.db(cctx.selectedDB) + + if !db.exists(opts.Key) { + c.WriteLen(0) + return + } + + if db.t(opts.Key) != "zset" { + c.WriteError(ErrWrongType.Error()) + return + } + + members := db.ssetMembers(opts.Key) + // Just key sort. If scores are not the same we don't care. + sort.Strings(members) + if opts.Reverse { + min, max = max, min + minIncl, maxIncl = maxIncl, minIncl + } + members = withLexRange(members, min, minIncl, max, maxIncl) + if opts.Reverse { + reverseSlice(members) + } + + // Apply LIMIT ranges. That's . Unlike RANGE. + if opts.WithLimit { + if limitOffset < 0 { + members = nil + } else { + if limitOffset < len(members) { + members = members[limitOffset:] + } else { + // out of range + members = nil + } + if limitCount >= 0 { + if len(members) > limitCount { + members = members[:limitCount] + } + } + } + } + + c.WriteLen(len(members)) + for _, el := range members { + c.WriteBulk(el) + } +} + +// optLexrange handles ZRANGE{,BYLEX} ranges. They start with '[', '(', or are +// '+' or '-'. +// Sets destValue and destInclusive. destValue can be '+' or '-'. +func parseLexrange(s string) (string, bool, error) { + if len(s) == 0 { + return "", false, errors.New(msgInvalidRangeItem) + } + + if s == "+" || s == "-" { + return s, false, nil + } + + switch s[0] { + case '(': + return s[1:], false, nil + case '[': + return s[1:], true, nil + default: + return "", false, errors.New(msgInvalidRangeItem) + } +} diff --git a/cmd_sorted_set_test.go b/cmd_sorted_set_test.go index f3ca482a..de0d4ba5 100644 --- a/cmd_sorted_set_test.go +++ b/cmd_sorted_set_test.go @@ -339,6 +339,28 @@ func TestSortedSetRange(t *testing.T) { ) }) + t.Run("reverse", func(t *testing.T) { + mustDo(t, c, + "ZRANGE", "z", "0", "-1", "REV", + proto.Strings("inf", "three", "drei", "zwei", "two", "one"), + ) + }) + + t.Run("limit", func(t *testing.T) { + mustDo(t, c, + "ZRANGE", "z", "0", "+inf", "BYSCORE", "LIMIT", "1", "2", + proto.Strings("two", "zwei"), + ) + mustDo(t, c, + "ZRANGE", "z", "0", "+inf", "BYSCORE", "LIMIT", "1", "-1", + proto.Strings("two", "zwei", "drei", "three", "inf"), + ) + mustDo(t, c, + "ZRANGE", "z", "0", "+inf", "BYSCORE", "LIMIT", "1", "9999", + proto.Strings("two", "zwei", "drei", "three", "inf"), + ) + }) + t.Run("errors", func(t *testing.T) { mustDo(t, c, "ZRANGE", @@ -364,6 +386,10 @@ func TestSortedSetRange(t *testing.T) { "ZRANGE", "set", "1", "2", "toomany", proto.Error(msgSyntaxError), ) + mustDo(t, c, + "ZRANGE", "set", "1", "2", "LIMIT", "1", "2", + proto.Error(msgLimitCombination), + ) // Wrong type of key s.Set("str", "value") mustDo(t, c, @@ -642,20 +668,20 @@ func TestSortedSetRangeByScore(t *testing.T) { proto.Error(msgSyntaxError), ) mustDo(t, c, - "ZRANGEBYSCORE", "set", "[1", "2", "toomany", + "ZRANGEBYSCORE", "set", "[1", "2", proto.Error("ERR min or max is not a float"), ) mustDo(t, c, - "ZRANGEBYSCORE", "set", "1", "[2", "toomany", + "ZRANGEBYSCORE", "set", "1", "[2", proto.Error("ERR min or max is not a float"), ) mustDo(t, c, "ZRANGEBYSCORE", "set", "[1", "2", "LIMIT", "noint", "1", - proto.Error("ERR min or max is not a float"), + proto.Error(msgInvalidInt), ) mustDo(t, c, "ZRANGEBYSCORE", "set", "[1", "2", "LIMIT", "1", "noint", - proto.Error("ERR min or max is not a float"), + proto.Error(msgInvalidInt), ) // Wrong type of key s.Set("str", "value") diff --git a/integration/sorted_set_test.go b/integration/sorted_set_test.go index 44ea3ba3..3cd11de0 100644 --- a/integration/sorted_set_test.go +++ b/integration/sorted_set_test.go @@ -170,23 +170,61 @@ func TestSortedSetRange(t *testing.T) { "+Inf", "more stars", "-Inf", "big bang", ) - c.Do("ZRANGE", "z", "0", "-1") - c.Do("ZRANGE", "z", "0", "-1", "WITHSCORES", "WITHSCORES") - c.Do("ZRANGE", "z", "0", "-1", "WiThScOrEs") - c.Do("ZRANGE", "z", "0", "-2") - c.Do("ZRANGE", "z", "0", "-1000") - c.Do("ZRANGE", "z", "2", "-2") - c.Do("ZRANGE", "z", "400", "-1") - c.Do("ZRANGE", "z", "300", "-110") - c.Do("ZREVRANGE", "z", "0", "-1") - c.Do("ZREVRANGE", "z", "0", "-1", "WITHSCORES") - c.Do("ZREVRANGE", "z", "0", "-1", "WITHSCORES", "WITHSCORES", "WITHSCORES") - c.Do("ZREVRANGE", "z", "0", "-1", "WiThScOrEs") - c.Do("ZREVRANGE", "z", "0", "-2") - c.Do("ZREVRANGE", "z", "0", "-1000") - c.Do("ZREVRANGE", "z", "2", "-2") - c.Do("ZREVRANGE", "z", "400", "-1") - c.Do("ZREVRANGE", "z", "300", "-110") + c.Do("ZADD", "zs", + "5", "berlin", + "5", "lisbon", + "5", "manila", + "5", "budapest", + "5", "london", + "5", "singapore", + "5", "amsterdam", + ) + + t.Run("plain", func(t *testing.T) { + c.Do("ZRANGE", "z", "0", "-1") + c.Do("ZRANGE", "z", "0", "10", "WITHSCORES", "WITHSCORES") + c.Do("ZRANGE", "z", "0", "-1", "WiThScOrEs") + c.Do("ZRANGE", "z", "0", "10") + c.Do("ZRANGE", "z", "0", "2") + c.Do("ZRANGE", "z", "2", "20") + c.Do("ZRANGE", "z", "0", "-4") + c.Do("ZRANGE", "z", "2", "-4") + c.Do("ZRANGE", "z", "400", "-1") + c.Do("ZRANGE", "z", "300", "-110") + c.Do("ZRANGE", "z", "0", "-1", "REV") + c.Error("not an integer", "ZRANGE", "z", "(0", "-1") + c.Error("not an integer", "ZRANGE", "z", "0", "(-1") + c.Error("combination", "ZRANGE", "z", "0", "-1", "LIMIT", "1", "2") + }) + + t.Run("byscore", func(t *testing.T) { + c.Do("ZRANGE", "z", "0", "-1", "BYSCORE") + c.Do("ZRANGE", "z", "0", "1000", "BYSCORE") + c.Do("ZRANGE", "z", "1", "2", "BYSCORE") + c.Do("ZRANGE", "z", "1", "(2", "BYSCORE") + c.Do("ZRANGE", "z", "-inf", "+inf", "BYSCORE") + c.Do("ZRANGE", "z", "-inf", "+inf", "BYSCORE", "REV") + c.Do("ZRANGE", "z", "-inf", "+inf", "BYSCORE", "LIMIT", "0", "1") + c.Do("ZRANGE", "z", "-inf", "+inf", "BYSCORE", "LIMIT", "1", "2") + c.Do("ZRANGE", "z", "-inf", "+inf", "BYSCORE", "LIMIT", "0", "-1") + c.Do("ZRANGE", "z", "-inf", "+inf", "BYSCORE", "REV", "LIMIT", "0", "1") + c.Error("not a float", "ZRANGE", "z", "[1", "2", "BYSCORE") + }) + + t.Run("bylex", func(t *testing.T) { + c.Do("ZRANGE", "zs", "[be", "(ma", "BYLEX") + c.Do("ZRANGE", "zs", "[be", "+", "BYLEX") + c.Do("ZRANGE", "zs", "-", "(ma", "BYLEX") + c.Do("ZRANGE", "zs", "-", "+", "BYLEX") + c.Do("ZRANGE", "zs", "[be", "(ma", "BYLEX", "REV") + c.Do("ZRANGE", "zs", "-", "+", "BYLEX", "LIMIT", "0", "1") + c.Do("ZRANGE", "zs", "-", "+", "BYLEX", "LIMIT", "1", "3") + c.Do("ZRANGE", "zs", "-", "+", "BYLEX", "LIMIT", "1", "-1") + c.Do("ZRANGE", "zs", "-", "+", "BYLEX", "LIMIT", "1", "-1", "REV") + c.Error("syntax error", "ZRANGE", "z", "[be", "[ma", "BYSCORE", "BYLEX") + c.Error("range item", "ZRANGE", "z", "be", "(ma", "BYLEX") + c.Error("range item", "ZRANGE", "z", "(be", "ma", "BYLEX") + }) c.Do("ZADD", "zz", "0", "aap", @@ -207,8 +245,34 @@ func TestSortedSetRange(t *testing.T) { c.Error("not an integer", "ZRANGE", "foo", "2", "noint") c.Do("SET", "str", "I am a string") c.Error("wrong kind", "ZRANGE", "str", "300", "-110") + }) +} +func TestSortedSetRevRange(t *testing.T) { + testRaw(t, func(c *client) { + c.Do("ZADD", "z", + "1", "aap", + "2", "noot", + "3", "mies", + "2", "nootagain", + "3", "miesagain", + "+Inf", "the stars", + "+Inf", "more stars", + "-Inf", "big bang", + ) + c.Do("ZREVRANGE", "z", "0", "-1") + c.Do("ZREVRANGE", "z", "0", "-1", "WITHSCORES") + c.Do("ZREVRANGE", "z", "0", "-1", "WITHSCORES", "WITHSCORES", "WITHSCORES") + c.Do("ZREVRANGE", "z", "0", "-1", "WiThScOrEs") + c.Do("ZREVRANGE", "z", "0", "-2") + c.Do("ZREVRANGE", "z", "0", "-1000") + c.Do("ZREVRANGE", "z", "2", "-2") + c.Do("ZREVRANGE", "z", "400", "-1") + c.Do("ZREVRANGE", "z", "300", "-110") + c.Error("syntax", "ZREVRANGE", "z", "300", "-110", "REV") + // failure cases c.Error("wrong number", "ZREVRANGE") + c.Do("SET", "str", "I am a string") c.Error("wrong kind", "ZREVRANGE", "str", "300", "-110") }) } diff --git a/integration/test.go b/integration/test.go index 27159b95..1fece403 100644 --- a/integration/test.go +++ b/integration/test.go @@ -508,10 +508,9 @@ func (c *client) Error(msg string, cmd string, args ...string) { return } - // c.t.Logf("real:%q mini:%q", string(resReal), string(resMini)) - mini, err := proto.ReadError(resMini) if err != nil { + c.t.Logf("real:%q mini:%q", string(resReal), string(resMini)) c.t.Errorf("parse error miniredis: %s", err) return } diff --git a/opts.go b/opts.go index 86650818..016d2682 100644 --- a/opts.go +++ b/opts.go @@ -19,36 +19,3 @@ func optInt(c *server.Peer, src string, dest *int) bool { *dest = n return true } - -// optLexrange handles ZRANGE{,BYLEX} ranges. They start with '[', '(', or are -// '+' or '-'. -// Sets destValue and destInclusive. destValue can be '+' or '-'. -// Returns whether or not things were okay. -func optLexrange(c *server.Peer, s string, destValue *string, destInclusive *bool) bool { - if len(s) == 0 { - setDirty(c) - c.WriteError(msgInvalidRangeItem) - return false - } - - if s == "+" || s == "-" { - *destValue = s - *destInclusive = false - return true - } - - switch s[0] { - case '(': - *destValue = s[1:] - *destInclusive = false - return true - case '[': - *destValue = s[1:] - *destInclusive = true - return true - default: - setDirty(c) - c.WriteError(msgInvalidRangeItem) - return false - } -} diff --git a/redis.go b/redis.go index b1ccb30e..0f4c049c 100644 --- a/redis.go +++ b/redis.go @@ -47,6 +47,7 @@ const ( msgXtrimInvalidMaxLen = "ERR value is not an integer or out of range" msgXtrimInvalidLimit = "ERR syntax error, LIMIT cannot be used without the special ~ option" msgDBIndexOutOfRange = "ERR DB index is out of range" + msgLimitCombination = "ERR syntax error, LIMIT is only supported in combination with either BYSCORE or BYLEX" ) func errWrongNumber(cmd string) string {