From a2b33f081c12c61ab3e0b0c8a34ed03c3da7e1e3 Mon Sep 17 00:00:00 2001 From: Harmen Date: Tue, 8 Mar 2022 13:01:31 +0100 Subject: [PATCH] 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")