From 8a8dbfb3ecbdc663a6280b87f9a5fcd412139ab2 Mon Sep 17 00:00:00 2001 From: xyb Date: Sat, 19 Dec 2020 00:09:09 +0800 Subject: [PATCH 01/36] chore: remove redundant type conversion (#541) Remove redundant []byte to string conversion --- redis/conn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/conn.go b/redis/conn.go index 5d7841c6..bc1735c2 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -615,7 +615,7 @@ func (c *conn) readReply() (interface{}, error) { return string(line[1:]), nil } case '-': - return Error(string(line[1:])), nil + return Error(line[1:]), nil case ':': return parseInt(line[1:]) case '$': From e2bc6dba0d438f2fc09a838f11838f978dbb39bb Mon Sep 17 00:00:00 2001 From: Shubhendra Singh Chauhan Date: Wed, 17 Feb 2021 03:37:33 +0530 Subject: [PATCH 02/36] chore: Remove unnecessary blank (_) identifier (#551) Remove unnecessary blank (_) identifier from redisx/muxConn.Close --- redisx/connmux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisx/connmux.go b/redisx/connmux.go index 6ae1f9d1..41c7cd5a 100644 --- a/redisx/connmux.go +++ b/redisx/connmux.go @@ -133,7 +133,7 @@ func (c *muxConn) Close() error { return nil } c.Flush() - for _ = range c.ids { + for range c.ids { _, err = c.Receive() } return err From dbd3ec6d786fdf1d49ddf173c2bd402993c3454e Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Thu, 18 Feb 2021 13:19:09 +0000 Subject: [PATCH 03/36] chore: Enable golangci-lint and fix errors (#554) Enable golangci-lint checking in travis and fix errors identified by it. This mainly involves removing dead code and improving error checking such as: * activeConn.Close now returns any error from operations it performs instead of always returning nil. --- .github/workflows/golangci-lint.yml | 31 +++++++ redis/conn.go | 40 ++++++--- redis/conn_test.go | 65 +++++++++------ redis/log.go | 4 +- redis/pool.go | 48 +++++++---- redis/pool_test.go | 124 ++++++++++++++++++---------- redis/pubsub.go | 20 +++-- redis/pubsub_example_test.go | 19 ++++- redis/pubsub_test.go | 20 +++-- redis/reply_test.go | 52 ++++++++++-- redis/scan.go | 5 +- redis/scan_test.go | 66 +++++++++++---- redis/script.go | 2 +- redis/script_test.go | 6 ++ redis/test_test.go | 12 ++- redis/zpop_example_test.go | 16 +++- redisx/commandinfo.go | 7 -- redisx/connmux_test.go | 24 +++--- redisx/db_test.go | 5 +- 19 files changed, 393 insertions(+), 173 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000..e59fd3c7 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,31 @@ +name: golangci-lint +on: + push: + tags: + - v* + branches: + - master + pull_request: +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. + version: v1.36 + + # Optional: working directory, useful for monorepos + # working-directory: somedir + + # Optional: golangci-lint command line arguments. + # args: --issues-exit-code=0 + + # Optional: show only new issues if it's a pull request. The default value is `false`. + # only-new-issues: true + + # Optional: if set to true then the action will use pre-installed Go. + # skip-go-installation: true diff --git a/redis/conn.go b/redis/conn.go index bc1735c2..1398b4d1 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -432,15 +432,23 @@ func (c *conn) writeLen(prefix byte, n int) error { } func (c *conn) writeString(s string) error { - c.writeLen('$', len(s)) - c.bw.WriteString(s) + if err := c.writeLen('$', len(s)); err != nil { + return err + } + if _, err := c.bw.WriteString(s); err != nil { + return err + } _, err := c.bw.WriteString("\r\n") return err } func (c *conn) writeBytes(p []byte) error { - c.writeLen('$', len(p)) - c.bw.Write(p) + if err := c.writeLen('$', len(p)); err != nil { + return err + } + if _, err := c.bw.Write(p); err != nil { + return err + } _, err := c.bw.WriteString("\r\n") return err } @@ -454,7 +462,9 @@ func (c *conn) writeFloat64(n float64) error { } func (c *conn) writeCommand(cmd string, args []interface{}) error { - c.writeLen('*', 1+len(args)) + if err := c.writeLen('*', 1+len(args)); err != nil { + return err + } if err := c.writeString(cmd); err != nil { return err } @@ -656,7 +666,9 @@ func (c *conn) Send(cmd string, args ...interface{}) error { c.pending += 1 c.mu.Unlock() if c.writeTimeout != 0 { - c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)) + if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)); err != nil { + return c.fatal(err) + } } if err := c.writeCommand(cmd, args); err != nil { return c.fatal(err) @@ -666,7 +678,9 @@ func (c *conn) Send(cmd string, args ...interface{}) error { func (c *conn) Flush() error { if c.writeTimeout != 0 { - c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)) + if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)); err != nil { + return c.fatal(err) + } } if err := c.bw.Flush(); err != nil { return c.fatal(err) @@ -683,7 +697,9 @@ func (c *conn) ReceiveWithTimeout(timeout time.Duration) (reply interface{}, err if timeout != 0 { deadline = time.Now().Add(timeout) } - c.conn.SetReadDeadline(deadline) + if err := c.conn.SetReadDeadline(deadline); err != nil { + return nil, c.fatal(err) + } if reply, err = c.readReply(); err != nil { return nil, c.fatal(err) @@ -721,7 +737,9 @@ func (c *conn) DoWithTimeout(readTimeout time.Duration, cmd string, args ...inte } if c.writeTimeout != 0 { - c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)) + if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)); err != nil { + return nil, c.fatal(err) + } } if cmd != "" { @@ -738,7 +756,9 @@ func (c *conn) DoWithTimeout(readTimeout time.Duration, cmd string, args ...inte if readTimeout != 0 { deadline = time.Now().Add(readTimeout) } - c.conn.SetReadDeadline(deadline) + if err := c.conn.SetReadDeadline(deadline); err != nil { + return nil, c.fatal(err) + } if cmd == "" { reply := make([]interface{}, pending) diff --git a/redis/conn_test.go b/redis/conn_test.go index 97d7bec1..425c4f09 100644 --- a/redis/conn_test.go +++ b/redis/conn_test.go @@ -30,6 +30,7 @@ import ( "time" "github.com/gomodule/redigo/redis" + "github.com/stretchr/testify/require" ) type testConn struct { @@ -71,10 +72,10 @@ func dialTestConnTLS(r string, w io.Writer) redis.DialOption { return redis.DialNetDial(func(network, addr string) (net.Conn, error) { client, server := net.Pipe() tlsServer := tls.Server(server, &serverTLSConfig) - go io.Copy(tlsServer, strings.NewReader(r)) + go io.Copy(tlsServer, strings.NewReader(r)) // nolint: errcheck done := make(chan struct{}) go func() { - io.Copy(w, tlsServer) + io.Copy(w, tlsServer) // nolint: errcheck close(done) }() return &tlsTestConn{Conn: client, done: done}, nil @@ -442,12 +443,12 @@ func TestRecvBeforeSend(t *testing.T) { defer c.Close() done := make(chan struct{}) go func() { - c.Receive() + c.Receive() // nolint: errcheck close(done) }() time.Sleep(time.Millisecond) - c.Send("PING") - c.Flush() + require.NoError(t, c.Send("PING")) + require.NoError(t, c.Flush()) <-done _, err = c.Do("") if err != nil { @@ -462,7 +463,8 @@ func TestError(t *testing.T) { } defer c.Close() - c.Do("SET", "key", "val") + _, err = c.Do("SET", "key", "val") + require.NoError(t, err) _, err = c.Do("HSET", "key", "fld", "val") if err == nil { t.Errorf("Expected err for HSET on string key.") @@ -491,7 +493,8 @@ func TestReadTimeout(t *testing.T) { } go func() { time.Sleep(time.Second) - c.Write([]byte("+OK\r\n")) + _, err := c.Write([]byte("+OK\r\n")) + require.NoError(t, err) c.Close() }() } @@ -521,8 +524,8 @@ func TestReadTimeout(t *testing.T) { } defer c2.Close() - c2.Send("PING") - c2.Flush() + require.NoError(t, c2.Send("PING")) + require.NoError(t, c2.Flush()) _, err = c2.Receive() if err == nil { t.Fatalf("c2.Receive() returned nil, expect error") @@ -859,11 +862,13 @@ func TestExecError(t *testing.T) { // Execute commands that fail before EXEC is called. - c.Do("DEL", "k0") - c.Do("ZADD", "k0", 0, 0) - c.Send("MULTI") - c.Send("NOTACOMMAND", "k0", 0, 0) - c.Send("ZINCRBY", "k0", 0, 0) + _, err = c.Do("DEL", "k0") + require.NoError(t, err) + _, err = c.Do("ZADD", "k0", 0, 0) + require.NoError(t, err) + require.NoError(t, c.Send("MULTI")) + require.NoError(t, c.Send("NOTACOMMAND", "k0", 0, 0)) + require.NoError(t, c.Send("ZINCRBY", "k0", 0, 0)) v, err := c.Do("EXEC") if err == nil { t.Fatalf("EXEC returned values %v, expected error", v) @@ -872,11 +877,13 @@ func TestExecError(t *testing.T) { // Execute commands that fail after EXEC is called. The first command // returns an error. - c.Do("DEL", "k1") - c.Do("ZADD", "k1", 0, 0) - c.Send("MULTI") - c.Send("HSET", "k1", 0, 0) - c.Send("ZINCRBY", "k1", 0, 0) + _, err = c.Do("DEL", "k1") + require.NoError(t, err) + _, err = c.Do("ZADD", "k1", 0, 0) + require.NoError(t, err) + require.NoError(t, c.Send("MULTI")) + require.NoError(t, c.Send("HSET", "k1", 0, 0)) + require.NoError(t, c.Send("ZINCRBY", "k1", 0, 0)) v, err = c.Do("EXEC") if err != nil { t.Fatalf("EXEC returned error %v", err) @@ -902,10 +909,11 @@ func TestExecError(t *testing.T) { // Execute commands that fail after EXEC is called. The second command // returns an error. - c.Do("ZADD", "k2", 0, 0) - c.Send("MULTI") - c.Send("ZINCRBY", "k2", 0, 0) - c.Send("HSET", "k2", 0, 0) + _, err = c.Do("ZADD", "k2", 0, 0) + require.NoError(t, err) + require.NoError(t, c.Send("MULTI")) + require.NoError(t, c.Send("ZINCRBY", "k2", 0, 0)) + require.NoError(t, c.Send("HSET", "k2", 0, 0)) v, err = c.Do("EXEC") if err != nil { t.Fatalf("EXEC returned error %v", err) @@ -1030,15 +1038,17 @@ func TestWithTimeout(t *testing.T) { var minDeadline, maxDeadline time.Time // Alternate between default and specified timeout. + var err error if i%2 == 0 { if defaultTimout != 0 { minDeadline = time.Now().Add(defaultTimout) } if recv { - c.Receive() + _, err = c.Receive() } else { - c.Do("PING") + _, err = c.Do("PING") } + require.NoError(t, err) if defaultTimout != 0 { maxDeadline = time.Now().Add(defaultTimout) } @@ -1046,10 +1056,11 @@ func TestWithTimeout(t *testing.T) { timeout := 10 * time.Minute minDeadline = time.Now().Add(timeout) if recv { - redis.ReceiveWithTimeout(c, timeout) + _, err = redis.ReceiveWithTimeout(c, timeout) } else { - redis.DoWithTimeout(c, timeout, "PING") + _, err = redis.DoWithTimeout(c, timeout, "PING") } + require.NoError(t, err) maxDeadline = time.Now().Add(timeout) } diff --git a/redis/log.go b/redis/log.go index a06db9d6..ef8cd7a0 100644 --- a/redis/log.go +++ b/redis/log.go @@ -52,7 +52,7 @@ func (c *loggingConn) Close() error { err := c.Conn.Close() var buf bytes.Buffer fmt.Fprintf(&buf, "%sClose() -> (%v)", c.prefix, err) - c.logger.Output(2, buf.String()) + c.logger.Output(2, buf.String()) // nolint: errcheck return err } @@ -112,7 +112,7 @@ func (c *loggingConn) print(method, commandName string, args []interface{}, repl buf.WriteString(", ") } fmt.Fprintf(&buf, "%v)", err) - c.logger.Output(3, buf.String()) + c.logger.Output(3, buf.String()) // nolint: errcheck } func (c *loggingConn) Do(commandName string, args ...interface{}) (interface{}, error) { diff --git a/redis/pool.go b/redis/pool.go index b74e469d..c7a2f194 100644 --- a/redis/pool.go +++ b/redis/pool.go @@ -39,7 +39,6 @@ var nowFunc = time.Now // for testing var ErrPoolExhausted = errors.New("redigo: connection pool exhausted") var ( - errPoolClosed = errors.New("redigo: connection pool closed") errConnClosed = errors.New("redigo: connection closed") ) @@ -254,7 +253,6 @@ func (p *Pool) GetContext(ctx context.Context) (Conn, error) { p.mu.Unlock() c, err := p.dial(ctx) if err != nil { - c = nil p.mu.Lock() p.active-- if p.ch != nil && !p.closed { @@ -445,13 +443,22 @@ func initSentinel() { sentinel = p } else { h := sha1.New() - io.WriteString(h, "Oops, rand failed. Use time instead.") - io.WriteString(h, strconv.FormatInt(time.Now().UnixNano(), 10)) + io.WriteString(h, "Oops, rand failed. Use time instead.") // nolint: errcheck + io.WriteString(h, strconv.FormatInt(time.Now().UnixNano(), 10)) // nolint: errcheck sentinel = h.Sum(nil) } } -func (ac *activeConn) Close() error { +func (ac *activeConn) firstError(errs ...error) error { + for _, err := range errs[:len(errs)-1] { + if err != nil { + return err + } + } + return errs[len(errs)-1] +} + +func (ac *activeConn) Close() (err error) { pc := ac.pc if pc == nil { return nil @@ -459,23 +466,28 @@ func (ac *activeConn) Close() error { ac.pc = nil if ac.state&connectionMultiState != 0 { - pc.c.Send("DISCARD") + err = pc.c.Send("DISCARD") ac.state &^= (connectionMultiState | connectionWatchState) } else if ac.state&connectionWatchState != 0 { - pc.c.Send("UNWATCH") + err = pc.c.Send("UNWATCH") ac.state &^= connectionWatchState } if ac.state&connectionSubscribeState != 0 { - pc.c.Send("UNSUBSCRIBE") - pc.c.Send("PUNSUBSCRIBE") + err = ac.firstError(err, + pc.c.Send("UNSUBSCRIBE"), + pc.c.Send("PUNSUBSCRIBE"), + ) // To detect the end of the message stream, ask the server to echo // a sentinel value and read until we see that value. sentinelOnce.Do(initSentinel) - pc.c.Send("ECHO", sentinel) - pc.c.Flush() + err = ac.firstError(err, + pc.c.Send("ECHO", sentinel), + pc.c.Flush(), + ) for { - p, err := pc.c.Receive() - if err != nil { + p, err2 := pc.c.Receive() + if err2 != nil { + err = ac.firstError(err, err2) break } if p, ok := p.([]byte); ok && bytes.Equal(p, sentinel) { @@ -484,9 +496,12 @@ func (ac *activeConn) Close() error { } } } - pc.c.Do("") - ac.p.put(pc, ac.state != 0 || pc.c.Err() != nil) - return nil + _, err2 := pc.c.Do("") + return ac.firstError( + err, + err2, + ac.p.put(pc, ac.state != 0 || pc.c.Err() != nil), + ) } func (ac *activeConn) Err() error { @@ -594,7 +609,6 @@ func (l *idleList) pushFront(pc *poolConn) { } l.front = pc l.count++ - return } func (l *idleList) popFront() { diff --git a/redis/pool_test.go b/redis/pool_test.go index b8cd3583..f420b124 100644 --- a/redis/pool_test.go +++ b/redis/pool_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/gomodule/redigo/redis" + "github.com/stretchr/testify/require" ) const ( @@ -142,11 +143,13 @@ func TestPoolReuse(t *testing.T) { for i := 0; i < 10; i++ { c1 := p.Get() - c1.Do("PING") + _, err := c1.Do("PING") + require.NoError(t, err) c2 := p.Get() - c2.Do("PING") - c1.Close() - c2.Close() + _, err = c2.Do("PING") + require.NoError(t, err) + require.NoError(t, c1.Close()) + require.NoError(t, c2.Close()) } d.check("before close", p, 2, 2, 0) @@ -164,14 +167,17 @@ func TestPoolMaxIdle(t *testing.T) { for i := 0; i < 10; i++ { c1 := p.Get() - c1.Do("PING") + _, err = c1.Do("PING") + require.NoError(t, err) c2 := p.Get() - c2.Do("PING") + _, err = c2.Do("PING") + require.NoError(t, err) c3 := p.Get() - c3.Do("PING") - c1.Close() - c2.Close() - c3.Close() + _, err = c3.Do("PING") + require.NoError(t, err) + require.NoError(t, c1.Close()) + require.NoError(t, c2.Close()) + require.NoError(t, c3.Close()) } d.check("before close", p, 12, 2, 0) p.Close() @@ -187,14 +193,16 @@ func TestPoolError(t *testing.T) { defer p.Close() c := p.Get() - c.Do("ERR", io.EOF) + _, err := c.Do("ERR", io.EOF) + require.NoError(t, err) if c.Err() == nil { t.Errorf("expected c.Err() != nil") } c.Close() c = p.Get() - c.Do("ERR", io.EOF) + _, err = c.Do("ERR", io.EOF) + require.NoError(t, err) c.Close() d.check(".", p, 2, 0, 0) @@ -209,11 +217,14 @@ func TestPoolClose(t *testing.T) { defer p.Close() c1 := p.Get() - c1.Do("PING") + _, err := c1.Do("PING") + require.NoError(t, err) c2 := p.Get() - c2.Do("PING") + _, err = c2.Do("PING") + require.NoError(t, err) c3 := p.Get() - c3.Do("PING") + _, err = c3.Do("PING") + require.NoError(t, err) c1.Close() if _, err := c1.Do("PING"); err == nil { @@ -285,7 +296,8 @@ func TestPoolIdleTimeout(t *testing.T) { defer redis.SetNowFunc(time.Now) c := p.Get() - c.Do("PING") + _, err := c.Do("PING") + require.NoError(t, err) c.Close() d.check("1", p, 1, 1, 0) @@ -293,7 +305,8 @@ func TestPoolIdleTimeout(t *testing.T) { now = now.Add(p.IdleTimeout + 1) c = p.Get() - c.Do("PING") + _, err = c.Do("PING") + require.NoError(t, err) c.Close() d.check("2", p, 2, 1, 0) @@ -313,7 +326,8 @@ func TestPoolMaxLifetime(t *testing.T) { defer redis.SetNowFunc(time.Now) c := p.Get() - c.Do("PING") + _, err := c.Do("PING") + require.NoError(t, err) c.Close() d.check("1", p, 1, 1, 0) @@ -321,7 +335,8 @@ func TestPoolMaxLifetime(t *testing.T) { now = now.Add(p.MaxConnLifetime + 1) c = p.Get() - c.Do("PING") + _, err = c.Do("PING") + require.NoError(t, err) c.Close() d.check("2", p, 2, 1, 0) @@ -339,7 +354,7 @@ func TestPoolConcurrenSendReceive(t *testing.T) { _, err := c.Receive() done <- err }() - c.Send("PING") + require.NoError(t, c.Send("PING")) c.Flush() err := <-done if err != nil { @@ -363,7 +378,8 @@ func TestPoolBorrowCheck(t *testing.T) { for i := 0; i < 10; i++ { c := p.Get() - c.Do("PING") + _, err := c.Do("PING") + require.NoError(t, err) c.Close() } d.check("1", p, 10, 1, 0) @@ -379,9 +395,11 @@ func TestPoolMaxActive(t *testing.T) { defer p.Close() c1 := p.Get() - c1.Do("PING") + _, err := c1.Do("PING") + require.NoError(t, err) c2 := p.Get() - c2.Do("PING") + _, err = c2.Do("PING") + require.NoError(t, err) d.check("1", p, 2, 2, 2) @@ -415,9 +433,11 @@ func TestPoolWaitStats(t *testing.T) { defer p.Close() c1 := p.Get() - c1.Do("PING") + _, err := c1.Do("PING") + require.NoError(t, err) c2 := p.Get() - c2.Do("PING") + _, err = c2.Do("PING") + require.NoError(t, err) d.checkAll("1", p, 2, 2, 2, 0, 0) @@ -445,7 +465,7 @@ func TestPoolMonitorCleanup(t *testing.T) { defer p.Close() c := p.Get() - c.Send("MONITOR") + require.NoError(t, c.Send("MONITOR")) c.Close() d.check("", p, 1, 0, 0) @@ -461,7 +481,7 @@ func TestPoolPubSubCleanup(t *testing.T) { defer p.Close() c := p.Get() - c.Send("SUBSCRIBE", "x") + require.NoError(t, c.Send("SUBSCRIBE", "x")) c.Close() want := []string{"SUBSCRIBE", "UNSUBSCRIBE", "PUNSUBSCRIBE", "ECHO"} @@ -471,7 +491,7 @@ func TestPoolPubSubCleanup(t *testing.T) { d.commands = nil c = p.Get() - c.Send("PSUBSCRIBE", "x*") + require.NoError(t, c.Send("PSUBSCRIBE", "x*")) c.Close() want = []string{"PSUBSCRIBE", "UNSUBSCRIBE", "PUNSUBSCRIBE", "ECHO"} @@ -491,8 +511,10 @@ func TestPoolTransactionCleanup(t *testing.T) { defer p.Close() c := p.Get() - c.Do("WATCH", "key") - c.Do("PING") + _, err := c.Do("WATCH", "key") + require.NoError(t, err) + _, err = c.Do("PING") + require.NoError(t, err) c.Close() want := []string{"WATCH", "PING", "UNWATCH"} @@ -502,9 +524,12 @@ func TestPoolTransactionCleanup(t *testing.T) { d.commands = nil c = p.Get() - c.Do("WATCH", "key") - c.Do("UNWATCH") - c.Do("PING") + _, err = c.Do("WATCH", "key") + require.NoError(t, err) + _, err = c.Do("UNWATCH") + require.NoError(t, err) + _, err = c.Do("PING") + require.NoError(t, err) c.Close() want = []string{"WATCH", "UNWATCH", "PING"} @@ -514,9 +539,12 @@ func TestPoolTransactionCleanup(t *testing.T) { d.commands = nil c = p.Get() - c.Do("WATCH", "key") - c.Do("MULTI") - c.Do("PING") + _, err = c.Do("WATCH", "key") + require.NoError(t, err) + _, err = c.Do("MULTI") + require.NoError(t, err) + _, err = c.Do("PING") + require.NoError(t, err) c.Close() want = []string{"WATCH", "MULTI", "PING", "DISCARD"} @@ -526,10 +554,14 @@ func TestPoolTransactionCleanup(t *testing.T) { d.commands = nil c = p.Get() - c.Do("WATCH", "key") - c.Do("MULTI") - c.Do("DISCARD") - c.Do("PING") + _, err = c.Do("WATCH", "key") + require.NoError(t, err) + _, err = c.Do("MULTI") + require.NoError(t, err) + _, err = c.Do("DISCARD") + require.NoError(t, err) + _, err = c.Do("PING") + require.NoError(t, err) c.Close() want = []string{"WATCH", "MULTI", "DISCARD", "PING"} @@ -539,10 +571,14 @@ func TestPoolTransactionCleanup(t *testing.T) { d.commands = nil c = p.Get() - c.Do("WATCH", "key") - c.Do("MULTI") - c.Do("EXEC") - c.Do("PING") + _, err = c.Do("WATCH", "key") + require.NoError(t, err) + _, err = c.Do("MULTI") + require.NoError(t, err) + _, err = c.Do("EXEC") + require.NoError(t, err) + _, err = c.Do("PING") + require.NoError(t, err) c.Close() want = []string{"WATCH", "MULTI", "EXEC", "PING"} diff --git a/redis/pubsub.go b/redis/pubsub.go index 2da60211..cc585757 100644 --- a/redis/pubsub.go +++ b/redis/pubsub.go @@ -60,27 +60,35 @@ func (c PubSubConn) Close() error { // Subscribe subscribes the connection to the specified channels. func (c PubSubConn) Subscribe(channel ...interface{}) error { - c.Conn.Send("SUBSCRIBE", channel...) + if err := c.Conn.Send("SUBSCRIBE", channel...); err != nil { + return err + } return c.Conn.Flush() } // PSubscribe subscribes the connection to the given patterns. func (c PubSubConn) PSubscribe(channel ...interface{}) error { - c.Conn.Send("PSUBSCRIBE", channel...) + if err := c.Conn.Send("PSUBSCRIBE", channel...); err != nil { + return err + } return c.Conn.Flush() } // Unsubscribe unsubscribes the connection from the given channels, or from all // of them if none is given. func (c PubSubConn) Unsubscribe(channel ...interface{}) error { - c.Conn.Send("UNSUBSCRIBE", channel...) + if err := c.Conn.Send("UNSUBSCRIBE", channel...); err != nil { + return err + } return c.Conn.Flush() } // PUnsubscribe unsubscribes the connection from the given patterns, or from all // of them if none is given. func (c PubSubConn) PUnsubscribe(channel ...interface{}) error { - c.Conn.Send("PUNSUBSCRIBE", channel...) + if err := c.Conn.Send("PUNSUBSCRIBE", channel...); err != nil { + return err + } return c.Conn.Flush() } @@ -89,7 +97,9 @@ func (c PubSubConn) PUnsubscribe(channel ...interface{}) error { // The connection must be subscribed to at least one channel or pattern when // calling this method. func (c PubSubConn) Ping(data string) error { - c.Conn.Send("PING", data) + if err := c.Conn.Send("PING", data); err != nil { + return err + } return c.Conn.Flush() } diff --git a/redis/pubsub_example_test.go b/redis/pubsub_example_test.go index 3e0cf078..4a5ec91a 100644 --- a/redis/pubsub_example_test.go +++ b/redis/pubsub_example_test.go @@ -102,7 +102,9 @@ loop: } // Signal the receiving goroutine to exit by unsubscribing from all channels. - psc.Unsubscribe() + if err := psc.Unsubscribe(); err != nil { + return err + } // Wait for goroutine to complete. return <-done @@ -116,9 +118,18 @@ func publish() { } defer c.Close() - c.Do("PUBLISH", "c1", "hello") - c.Do("PUBLISH", "c2", "world") - c.Do("PUBLISH", "c1", "goodbye") + if _, err = c.Do("PUBLISH", "c1", "hello"); err != nil { + fmt.Println(err) + return + } + if _, err = c.Do("PUBLISH", "c2", "world"); err != nil { + fmt.Println(err) + return + } + if _, err = c.Do("PUBLISH", "c1", "goodbye"); err != nil { + fmt.Println(err) + return + } } // This example shows how receive pubsub notifications with cancelation and diff --git a/redis/pubsub_test.go b/redis/pubsub_test.go index 13f3f797..83b91580 100644 --- a/redis/pubsub_test.go +++ b/redis/pubsub_test.go @@ -20,6 +20,7 @@ import ( "time" "github.com/gomodule/redigo/redis" + "github.com/stretchr/testify/require" ) func expectPushed(t *testing.T, c redis.PubSubConn, message string, expected interface{}) { @@ -44,29 +45,30 @@ func TestPushed(t *testing.T) { c := redis.PubSubConn{Conn: sc} - c.Subscribe("c1") + require.NoError(t, c.Subscribe("c1")) expectPushed(t, c, "Subscribe(c1)", redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}) - c.Subscribe("c2") + require.NoError(t, c.Subscribe("c2")) expectPushed(t, c, "Subscribe(c2)", redis.Subscription{Kind: "subscribe", Channel: "c2", Count: 2}) - c.PSubscribe("p1") + require.NoError(t, c.PSubscribe("p1")) expectPushed(t, c, "PSubscribe(p1)", redis.Subscription{Kind: "psubscribe", Channel: "p1", Count: 3}) - c.PSubscribe("p2") + require.NoError(t, c.PSubscribe("p2")) expectPushed(t, c, "PSubscribe(p2)", redis.Subscription{Kind: "psubscribe", Channel: "p2", Count: 4}) - c.PUnsubscribe() + require.NoError(t, c.PUnsubscribe()) expectPushed(t, c, "Punsubscribe(p1)", redis.Subscription{Kind: "punsubscribe", Channel: "p1", Count: 3}) expectPushed(t, c, "Punsubscribe()", redis.Subscription{Kind: "punsubscribe", Channel: "p2", Count: 2}) - pc.Do("PUBLISH", "c1", "hello") + _, err = pc.Do("PUBLISH", "c1", "hello") + require.NoError(t, err) expectPushed(t, c, "PUBLISH c1 hello", redis.Message{Channel: "c1", Data: []byte("hello")}) - c.Ping("hello") + require.NoError(t, c.Ping("hello")) expectPushed(t, c, `Ping("hello")`, redis.Pong{Data: "hello"}) - c.Conn.Send("PING") + require.NoError(t, c.Conn.Send("PING")) c.Conn.Flush() expectPushed(t, c, `Send("PING")`, redis.Pong{}) - c.Ping("timeout") + require.NoError(t, c.Ping("timeout")) got := c.ReceiveWithTimeout(time.Minute) if want := (redis.Pong{Data: "timeout"}); want != got { t.Errorf("recv /w timeout got %v, want %v", got, want) diff --git a/redis/reply_test.go b/redis/reply_test.go index 464045ae..8993bad1 100644 --- a/redis/reply_test.go +++ b/redis/reply_test.go @@ -238,8 +238,16 @@ func ExampleBool() { } defer c.Close() - c.Do("SET", "foo", 1) - exists, _ := redis.Bool(c.Do("EXISTS", "foo")) + if _, err = c.Do("SET", "foo", 1); err != nil { + fmt.Println(err) + return + } + exists, err := redis.Bool(c.Do("EXISTS", "foo")) + if err != nil { + fmt.Println(err) + return + } + fmt.Printf("%#v\n", exists) // Output: // true @@ -253,10 +261,22 @@ func ExampleInt() { } defer c.Close() - c.Do("SET", "k1", 1) - n, _ := redis.Int(c.Do("GET", "k1")) + _, err = c.Do("SET", "k1", 1) + if err != nil { + fmt.Println(err) + return + } + n, err := redis.Int(c.Do("GET", "k1")) + if err != nil { + fmt.Println(err) + return + } fmt.Printf("%#v\n", n) - n, _ = redis.Int(c.Do("INCR", "k1")) + n, err = redis.Int(c.Do("INCR", "k1")) + if err != nil { + fmt.Println(err) + return + } fmt.Printf("%#v\n", n) // Output: // 1 @@ -271,8 +291,16 @@ func ExampleInts() { } defer c.Close() - c.Do("SADD", "set_with_integers", 4, 5, 6) - ints, _ := redis.Ints(c.Do("SMEMBERS", "set_with_integers")) + _, err = c.Do("SADD", "set_with_integers", 4, 5, 6) + if err != nil { + fmt.Println(err) + return + } + ints, err := redis.Ints(c.Do("SMEMBERS", "set_with_integers")) + if err != nil { + fmt.Println(err) + return + } fmt.Printf("%#v\n", ints) // Output: // []int{4, 5, 6} @@ -286,8 +314,16 @@ func ExampleString() { } defer c.Close() - c.Do("SET", "hello", "world") + _, err = c.Do("SET", "hello", "world") + if err != nil { + fmt.Println(err) + return + } s, err := redis.String(c.Do("GET", "hello")) + if err != nil { + fmt.Println(err) + return + } fmt.Printf("%#v\n", s) // Output: // "world" diff --git a/redis/scan.go b/redis/scan.go index a4cf2015..379206ed 100644 --- a/redis/scan.go +++ b/redis/scan.go @@ -431,9 +431,8 @@ LOOP: } var ( - structSpecMutex sync.RWMutex - structSpecCache = make(map[reflect.Type]*structSpec) - defaultFieldSpec = &fieldSpec{} + structSpecMutex sync.RWMutex + structSpecCache = make(map[reflect.Type]*structSpec) ) func structSpecForType(t reflect.Type) *structSpec { diff --git a/redis/scan_test.go b/redis/scan_test.go index 445e0710..d43c372e 100644 --- a/redis/scan_test.go +++ b/redis/scan_test.go @@ -115,7 +115,7 @@ func TestScanConversion(t *testing.T) { for _, tt := range scanConversionTests { values := []interface{}{tt.src} dest := reflect.New(reflect.TypeOf(tt.dest)) - values, err := redis.Scan(values, dest.Interface()) + _, err := redis.Scan(values, dest.Interface()) if err != nil { t.Errorf("Scan(%v) returned error %v", tt, err) continue @@ -144,7 +144,7 @@ func TestScanConversionError(t *testing.T) { for _, tt := range scanConversionErrorTests { values := []interface{}{tt.src} dest := reflect.New(reflect.TypeOf(tt.dest)) - values, err := redis.Scan(values, dest.Interface()) + _, err := redis.Scan(values, dest.Interface()) if err == nil { t.Errorf("Scan(%v) did not return error", tt) } @@ -159,12 +159,30 @@ func ExampleScan() { } defer c.Close() - c.Send("HMSET", "album:1", "title", "Red", "rating", 5) - c.Send("HMSET", "album:2", "title", "Earthbound", "rating", 1) - c.Send("HMSET", "album:3", "title", "Beat") - c.Send("LPUSH", "albums", "1") - c.Send("LPUSH", "albums", "2") - c.Send("LPUSH", "albums", "3") + if err = c.Send("HMSET", "album:1", "title", "Red", "rating", 5); err != nil { + fmt.Println(err) + return + } + if err = c.Send("HMSET", "album:2", "title", "Earthbound", "rating", 1); err != nil { + fmt.Println(err) + return + } + if err = c.Send("HMSET", "album:3", "title", "Beat"); err != nil { + fmt.Println(err) + return + } + if err = c.Send("LPUSH", "albums", "1"); err != nil { + fmt.Println(err) + return + } + if err = c.Send("LPUSH", "albums", "2"); err != nil { + fmt.Println(err) + return + } + if err = c.Send("LPUSH", "albums", "3"); err != nil { + fmt.Println(err) + return + } values, err := redis.Values(c.Do("SORT", "albums", "BY", "album:*->rating", "GET", "album:*->title", @@ -414,12 +432,30 @@ func ExampleScanSlice() { } defer c.Close() - c.Send("HMSET", "album:1", "title", "Red", "rating", 5) - c.Send("HMSET", "album:2", "title", "Earthbound", "rating", 1) - c.Send("HMSET", "album:3", "title", "Beat", "rating", 4) - c.Send("LPUSH", "albums", "1") - c.Send("LPUSH", "albums", "2") - c.Send("LPUSH", "albums", "3") + if err = c.Send("HMSET", "album:1", "title", "Red", "rating", 5); err != nil { + fmt.Println(err) + return + } + if err = c.Send("HMSET", "album:2", "title", "Earthbound", "rating", 1); err != nil { + fmt.Println(err) + return + } + if err = c.Send("HMSET", "album:3", "title", "Beat", "rating", 4); err != nil { + fmt.Println(err) + return + } + if err = c.Send("LPUSH", "albums", "1"); err != nil { + fmt.Println(err) + return + } + if err = c.Send("LPUSH", "albums", "2"); err != nil { + fmt.Println(err) + return + } + if err = c.Send("LPUSH", "albums", "3"); err != nil { + fmt.Println(err) + return + } values, err := redis.Values(c.Do("SORT", "albums", "BY", "album:*->rating", "GET", "album:*->title", @@ -442,8 +478,6 @@ func ExampleScanSlice() { // [{Earthbound 1} {Beat 4} {Red 5}] } -var now = time.Now() - type Ed struct { EdI int `redis:"edi"` } diff --git a/redis/script.go b/redis/script.go index 0ef1c821..d0cec1ed 100644 --- a/redis/script.go +++ b/redis/script.go @@ -36,7 +36,7 @@ type Script struct { // SendHash methods. func NewScript(keyCount int, src string) *Script { h := sha1.New() - io.WriteString(h, src) + io.WriteString(h, src) // nolint: errcheck return &Script{keyCount, src, hex.EncodeToString(h.Sum(nil))} } diff --git a/redis/script_test.go b/redis/script_test.go index 388e167f..71424c30 100644 --- a/redis/script_test.go +++ b/redis/script_test.go @@ -78,6 +78,9 @@ func TestScript(t *testing.T) { } v, err = c.Receive() + if err != nil { + t.Errorf("s.Recieve() returned %v", err) + } if !reflect.DeepEqual(v, reply) { t.Errorf("s.SendHash(c, ..); c.Receive() = %v, want %v", v, reply) } @@ -93,6 +96,9 @@ func TestScript(t *testing.T) { } v, err = c.Receive() + if err != nil { + t.Errorf("s.Recieve() returned %v", err) + } if !reflect.DeepEqual(v, reply) { t.Errorf("s.Send(c, ..); c.Receive() = %v, want %v", v, reply) } diff --git a/redis/test_test.go b/redis/test_test.go index dfa48bf6..fcbb7bd1 100644 --- a/redis/test_test.go +++ b/redis/test_test.go @@ -107,13 +107,17 @@ func (s *Server) watch(r io.Reader, ready chan error) { if !listening { ready <- fmt.Errorf("server exited: %s", text) } - s.cmd.Wait() + if err := s.cmd.Wait(); err != nil { + if listening { + ready <- err + } + } fmt.Fprintf(serverLog, "%d STOP %s \n", s.cmd.Process.Pid, s.name) close(s.done) } func (s *Server) Stop() { - s.cmd.Process.Signal(os.Interrupt) + s.cmd.Process.Signal(os.Interrupt) // nolint: errcheck <-s.done } @@ -156,7 +160,9 @@ func DialDefaultServer(options ...DialOption) (Conn, error) { if err != nil { return nil, err } - c.Do("FLUSHDB") + if _, err = c.Do("FLUSHDB"); err != nil { + return nil, err + } return c, nil } diff --git a/redis/zpop_example_test.go b/redis/zpop_example_test.go index f7702e03..3d1af521 100644 --- a/redis/zpop_example_test.go +++ b/redis/zpop_example_test.go @@ -26,7 +26,7 @@ func zpop(c redis.Conn, key string) (result string, err error) { defer func() { // Return connection to normal state on error. if err != nil { - c.Do("DISCARD") + c.Do("DISCARD") // nolint: errcheck } }() @@ -44,8 +44,12 @@ func zpop(c redis.Conn, key string) (result string, err error) { return "", redis.ErrNil } - c.Send("MULTI") - c.Send("ZREM", key, members[0]) + if err = c.Send("MULTI"); err != nil { + return "", err + } + if err = c.Send("ZREM", key, members[0]); err != nil { + return "", err + } queued, err := c.Do("EXEC") if err != nil { return "", err @@ -83,8 +87,12 @@ func Example_zpop() { // Add test data using a pipeline. for i, member := range []string{"red", "blue", "green"} { - c.Send("ZADD", "zset", i, member) + if err = c.Send("ZADD", "zset", i, member); err != nil { + fmt.Println(err) + return + } } + if _, err := c.Do(""); err != nil { fmt.Println(err) return diff --git a/redisx/commandinfo.go b/redisx/commandinfo.go index b911cc4a..45c76633 100644 --- a/redisx/commandinfo.go +++ b/redisx/commandinfo.go @@ -18,13 +18,6 @@ import ( "strings" ) -const ( - connectionWatchState = 1 << iota - connectionMultiState - connectionSubscribeState - connectionMonitorState -) - type commandInfo struct { notMuxable bool } diff --git a/redisx/connmux_test.go b/redisx/connmux_test.go index e035a59f..a1d38ba5 100644 --- a/redisx/connmux_test.go +++ b/redisx/connmux_test.go @@ -21,6 +21,7 @@ import ( "github.com/gomodule/redigo/redis" "github.com/gomodule/redigo/redisx" + "github.com/stretchr/testify/require" ) func TestConnMux(t *testing.T) { @@ -33,8 +34,10 @@ func TestConnMux(t *testing.T) { c1 := m.Get() c2 := m.Get() - c1.Send("ECHO", "hello") - c2.Send("ECHO", "world") + err = c1.Send("ECHO", "hello") + require.NoError(t, err) + err = c2.Send("ECHO", "world") + require.NoError(t, err) c1.Flush() c2.Flush() s, err := redis.String(c1.Receive()) @@ -172,9 +175,8 @@ func BenchmarkConnMuxConcurrent(b *testing.B) { defer wg.Done() for i := 0; i < b.N; i++ { c := m.Get() - if _, err := c.Do("PING"); err != nil { - b.Fatal(err) - } + _, err := c.Do("PING") + require.NoError(b, err) c.Close() } }() @@ -211,9 +213,8 @@ func BenchmarkPoolConcurrent(b *testing.B) { defer wg.Done() for i := 0; i < b.N; i++ { c := p.Get() - if _, err := c.Do("PING"); err != nil { - b.Fatal(err) - } + _, err := c.Do("PING") + require.NoError(b, err) c.Close() } }() @@ -242,14 +243,13 @@ func BenchmarkPipelineConcurrency(b *testing.B) { for i := 0; i < b.N; i++ { id := pipeline.Next() pipeline.StartRequest(id) - c.Send("PING") + err = c.Send("PING") + require.NoError(b, err) c.Flush() pipeline.EndRequest(id) pipeline.StartResponse(id) _, err := c.Receive() - if err != nil { - b.Fatal(err) - } + require.NoError(b, err) pipeline.EndResponse(id) } }() diff --git a/redisx/db_test.go b/redisx/db_test.go index ead64744..8b72eb14 100644 --- a/redisx/db_test.go +++ b/redisx/db_test.go @@ -42,7 +42,10 @@ func (t testConn) Close() error { // stomping on real data, DialTestDB fails if database 9 contains data. The // returned connection flushes database 9 on close. func DialTest() (redis.Conn, error) { - c, err := redis.DialTimeout("tcp", ":6379", 0, 1*time.Second, 1*time.Second) + c, err := redis.Dial("tcp", ":6379", + redis.DialReadTimeout(time.Second), + redis.DialWriteTimeout(time.Second), + ) if err != nil { return nil, err } From 9e4eba012287620796007c4fd8274bf71185c647 Mon Sep 17 00:00:00 2001 From: yjh Date: Thu, 18 Feb 2021 21:22:13 +0800 Subject: [PATCH 04/36] chore(docs): update godoc links (#482) Update godoc links to point to pkg.go.dev --- README.markdown | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/README.markdown b/README.markdown index 2a637ac3..d418cbb2 100644 --- a/README.markdown +++ b/README.markdown @@ -2,26 +2,26 @@ Redigo ====== [![Build Status](https://travis-ci.org/gomodule/redigo.svg?branch=master)](https://travis-ci.org/gomodule/redigo) -[![GoDoc](https://godoc.org/github.com/gomodule/redigo/redis?status.svg)](https://godoc.org/github.com/gomodule/redigo/redis) +[![GoDoc](https://godoc.org/github.com/gomodule/redigo/redis?status.svg)](https://pkg.go.dev/github.com/gomodule/redigo/redis) Redigo is a [Go](http://golang.org/) client for the [Redis](http://redis.io/) database. Features ------- -* A [Print-like](http://godoc.org/github.com/gomodule/redigo/redis#hdr-Executing_Commands) API with support for all Redis commands. -* [Pipelining](http://godoc.org/github.com/gomodule/redigo/redis#hdr-Pipelining), including pipelined transactions. -* [Publish/Subscribe](http://godoc.org/github.com/gomodule/redigo/redis#hdr-Publish_and_Subscribe). -* [Connection pooling](http://godoc.org/github.com/gomodule/redigo/redis#Pool). -* [Script helper type](http://godoc.org/github.com/gomodule/redigo/redis#Script) with optimistic use of EVALSHA. -* [Helper functions](http://godoc.org/github.com/gomodule/redigo/redis#hdr-Reply_Helpers) for working with command replies. +* A [Print-like](https://pkg.go.dev/github.com/gomodule/redigo/redis#hdr-Executing_Commands) API with support for all Redis commands. +* [Pipelining](https://pkg.go.dev/github.com/gomodule/redigo/redis#hdr-Pipelining), including pipelined transactions. +* [Publish/Subscribe](https://pkg.go.dev/github.com/gomodule/redigo/redis#hdr-Publish_and_Subscribe). +* [Connection pooling](https://pkg.go.dev/github.com/gomodule/redigo/redis#Pool). +* [Script helper type](https://pkg.go.dev/github.com/gomodule/redigo/redis#Script) with optimistic use of EVALSHA. +* [Helper functions](https://pkg.go.dev/github.com/gomodule/redigo/redis#hdr-Reply_Helpers) for working with command replies. Documentation ------------- -- [API Reference](http://godoc.org/github.com/gomodule/redigo/redis) +- [API Reference](https://pkg.go.dev/github.com/gomodule/redigo/redis) - [FAQ](https://github.com/gomodule/redigo/wiki/FAQ) -- [Examples](https://godoc.org/github.com/gomodule/redigo/redis#pkg-examples) +- [Examples](https://pkg.go.dev/github.com/gomodule/redigo/redis#pkg-examples) Installation ------------ @@ -35,7 +35,7 @@ The Go distribution is Redigo's only dependency. Related Projects ---------------- -- [rafaeljusto/redigomock](https://godoc.org/github.com/rafaeljusto/redigomock) - A mock library for Redigo. +- [rafaeljusto/redigomock](https://pkg.go.dev/github.com/rafaeljusto/redigomock) - A mock library for Redigo. - [chasex/redis-go-cluster](https://github.com/chasex/redis-go-cluster) - A Redis cluster client implementation. - [FZambia/sentinel](https://github.com/FZambia/sentinel) - Redis Sentinel support for Redigo - [mna/redisc](https://github.com/mna/redisc) - Redis Cluster client built on top of Redigo From 72af8129e040d6f962772a8c582e5e9f22085788 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Thu, 18 Feb 2021 22:32:39 +0000 Subject: [PATCH 05/36] chore: Add clog config (#555) Add config for clog which is used for generating release notes from our conventional commits. For details on the tool see: https://github.com/clog-tool/clog-cli --- .clog.toml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .clog.toml diff --git a/.clog.toml b/.clog.toml new file mode 100644 index 00000000..50735f66 --- /dev/null +++ b/.clog.toml @@ -0,0 +1,14 @@ +[clog] +repository = "https://github.com/gomodule/redigo" +subtitle = "Release Notes" +from-latest-tag = true + +[sections] +"Refactors" = ["refactor"] +"Chores" = ["chore"] +"Continuous Integration" = ["ci"] +"Improvements" = ["imp", "improvement"] +"Features" = ["feat", "feature"] +"Legacy" = ["legacy"] +"QA" = ["qa", "test"] +"Documentation" = ["doc", "docs"] From 46992b0f02f74066bcdfd9b03e33bc03abd10dc7 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Thu, 10 Jun 2021 10:17:14 +0100 Subject: [PATCH 06/36] fix: DialURL compatibility with redis-cli (#566) Fix compatibility of DialURL with respect for single component user-info records. This enables URLs such as redis://mypass@localhost/1 as supported by redis-cli to be used. --- redis/conn.go | 13 ++++++++++++- redis/conn_test.go | 29 +++++++++++++++++++---------- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/redis/conn.go b/redis/conn.go index 1398b4d1..7d757dce 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -168,6 +168,7 @@ func DialPassword(password string) DialOption { // DialUsername specifies the username to use when connecting to // the Redis server when Redis ACLs are used. +// A DialPassword must also be passed otherwise this option will have no effect. func DialUsername(username string) DialOption { return DialOption{func(do *dialOptions) { do.username = username @@ -347,8 +348,18 @@ func DialURL(rawurl string, options ...DialOption) (Conn, error) { if u.User != nil { password, isSet := u.User.Password() + username := u.User.Username() if isSet { - options = append(options, DialUsername(u.User.Username()), DialPassword(password)) + if username != "" { + // ACL + options = append(options, DialUsername(username), DialPassword(password)) + } else { + // requirepass - user-info username:password with blank username + options = append(options, DialPassword(password)) + } + } else if username != "" { + // requirepass - redis-cli compatibility which treats as single arg in user-info as a password + options = append(options, DialPassword(username)) } } diff --git a/redis/conn_test.go b/redis/conn_test.go index 425c4f09..b33cc41a 100644 --- a/redis/conn_test.go +++ b/redis/conn_test.go @@ -640,6 +640,13 @@ var dialURLTests = []struct { w string }{ {"password", "redis://:abc123@localhost", "+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n"}, + {"password redis-cli compat", "redis://abc123@localhost", "+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n"}, + {"password db1", "redis://:abc123@localhost/1", "+OK\r\n+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n*2\r\n$6\r\nSELECT\r\n$1\r\n1\r\n"}, + {"password db1 redis-cli compat", "redis://abc123@localhost/1", "+OK\r\n+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n*2\r\n$6\r\nSELECT\r\n$1\r\n1\r\n"}, + {"password no host db0", "redis://:abc123@/0", "+OK\r\n+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n"}, + {"password no host db0 redis-cli compat", "redis://abc123@/0", "+OK\r\n+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n"}, + {"password no host db1", "redis://:abc123@/1", "+OK\r\n+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n*2\r\n$6\r\nSELECT\r\n$1\r\n1\r\n"}, + {"password no host db1 redis-cli compat", "redis://abc123@/1", "+OK\r\n+OK\r\n", "*2\r\n$4\r\nAUTH\r\n$6\r\nabc123\r\n*2\r\n$6\r\nSELECT\r\n$1\r\n1\r\n"}, {"username and password", "redis://user:password@localhost", "+OK\r\n", "*3\r\n$4\r\nAUTH\r\n$4\r\nuser\r\n$8\r\npassword\r\n"}, {"username", "redis://x:@localhost", "+OK\r\n", ""}, {"database 3", "redis://localhost/3", "+OK\r\n", "*2\r\n$6\r\nSELECT\r\n$1\r\n3\r\n"}, @@ -649,16 +656,18 @@ var dialURLTests = []struct { func TestDialURL(t *testing.T) { for _, tt := range dialURLTests { - var buf bytes.Buffer - // UseTLS should be ignored in all of these tests. - _, err := redis.DialURL(tt.url, dialTestConn(tt.r, &buf), redis.DialUseTLS(true)) - if err != nil { - t.Errorf("%s dial error: %v", tt.description, err) - continue - } - if w := buf.String(); w != tt.w { - t.Errorf("%s commands = %q, want %q", tt.description, w, tt.w) - } + t.Run(tt.description, func(t *testing.T) { + var buf bytes.Buffer + // UseTLS should be ignored in all of these tests. + _, err := redis.DialURL(tt.url, dialTestConn(tt.r, &buf), redis.DialUseTLS(true)) + if err != nil { + t.Errorf("%s dial error: %v, buf: %v", tt.description, err, buf.String()) + return + } + if w := buf.String(); w != tt.w { + t.Errorf("%s commands = %q, want %q", tt.description, w, tt.w) + } + }) } } From bf63cd5feb4dcd13eed615b517a60cc2afc29563 Mon Sep 17 00:00:00 2001 From: Cameron Elliott <868689+cameronelliott@users.noreply.github.com> Date: Wed, 29 Sep 2021 18:10:11 -0700 Subject: [PATCH 07/36] feat: implement DialURLContext(...) (#574) Add DialURLContext so that we consumers have control over cancelation and timeout Convert DialURL to call DialURLContext() and update docs to ensure consumers are aware of the new method, which should be preferred to ensure requests can't hang forever. --- redis/conn.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/redis/conn.go b/redis/conn.go index 7d757dce..99dc6fa1 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -316,10 +316,17 @@ func DialContext(ctx context.Context, network, address string, options ...DialOp var pathDBRegexp = regexp.MustCompile(`/(\d*)\z`) -// DialURL connects to a Redis server at the given URL using the Redis +// DialURL wraps DialURLContext using context.Background. +func DialURL(rawurl string, options ...DialOption) (Conn, error) { + ctx := context.Background() + + return DialURLContext(ctx, rawurl, options...) +} + +// DialURLContext connects to a Redis server at the given URL using the Redis // URI scheme. URLs should follow the draft IANA specification for the // scheme (https://www.iana.org/assignments/uri-schemes/prov/redis). -func DialURL(rawurl string, options ...DialOption) (Conn, error) { +func DialURLContext(ctx context.Context, rawurl string, options ...DialOption) (Conn, error) { u, err := url.Parse(rawurl) if err != nil { return nil, err @@ -381,7 +388,7 @@ func DialURL(rawurl string, options ...DialOption) (Conn, error) { options = append(options, DialUseTLS(u.Scheme == "rediss")) - return Dial("tcp", address, options...) + return DialContext(ctx, "tcp", address, options...) } // NewConn returns a new Redigo connection for the given net connection. From 56d644832b684fdd69ed4afae019a0709aefd9bc Mon Sep 17 00:00:00 2001 From: chenjie199234 Date: Thu, 30 Sep 2021 21:20:55 +0800 Subject: [PATCH 08/36] feat: add DoContext and ReceiveContext (#537) Add support for context during the Do cycle of a request. This is supported by DoContext and ReceiveContext to control the command life by both context and read timeout. Co-authored-by: Mikhail Mazurskiy <126021+ash2k@users.noreply.github.com> Co-authored-by: Lilith Games --- redis/conn.go | 60 ++++++++++++++++++++++++++++++++++++++++++++ redis/log.go | 13 ++++++++++ redis/pool.go | 30 ++++++++++++++++++++++ redis/redis.go | 61 +++++++++++++++++++++++++++++++++++++++++---- redis/redis_test.go | 52 ++++++++++++++++++++++++++++++++++++++ redis/script.go | 13 ++++++++++ 6 files changed, 224 insertions(+), 5 deletions(-) diff --git a/redis/conn.go b/redis/conn.go index 99dc6fa1..526a5649 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -710,6 +710,36 @@ func (c *conn) Receive() (interface{}, error) { return c.ReceiveWithTimeout(c.readTimeout) } +func (c *conn) ReceiveContext(ctx context.Context) (interface{}, error) { + var realTimeout time.Duration + if dl, ok := ctx.Deadline(); ok { + timeout := time.Until(dl) + if timeout >= c.readTimeout && c.readTimeout != 0 { + realTimeout = c.readTimeout + } else if timeout <= 0 { + return nil, c.fatal(context.DeadlineExceeded) + } else { + realTimeout = timeout + } + } else { + realTimeout = c.readTimeout + } + endch := make(chan struct{}) + var r interface{} + var e error + go func() { + defer close(endch) + + r, e = c.ReceiveWithTimeout(realTimeout) + }() + select { + case <-ctx.Done(): + return nil, c.fatal(ctx.Err()) + case <-endch: + return r, e + } +} + func (c *conn) ReceiveWithTimeout(timeout time.Duration) (reply interface{}, err error) { var deadline time.Time if timeout != 0 { @@ -744,6 +774,36 @@ func (c *conn) Do(cmd string, args ...interface{}) (interface{}, error) { return c.DoWithTimeout(c.readTimeout, cmd, args...) } +func (c *conn) DoContext(ctx context.Context, cmd string, args ...interface{}) (interface{}, error) { + var realTimeout time.Duration + if dl, ok := ctx.Deadline(); ok { + timeout := time.Until(dl) + if timeout >= c.readTimeout && c.readTimeout != 0 { + realTimeout = c.readTimeout + } else if timeout <= 0 { + return nil, c.fatal(context.DeadlineExceeded) + } else { + realTimeout = timeout + } + } else { + realTimeout = c.readTimeout + } + endch := make(chan struct{}) + var r interface{} + var e error + go func() { + defer close(endch) + + r, e = c.DoWithTimeout(realTimeout, cmd, args) + }() + select { + case <-ctx.Done(): + return nil, c.fatal(ctx.Err()) + case <-endch: + return r, e + } +} + func (c *conn) DoWithTimeout(readTimeout time.Duration, cmd string, args ...interface{}) (interface{}, error) { c.mu.Lock() pending := c.pending diff --git a/redis/log.go b/redis/log.go index ef8cd7a0..72e054f0 100644 --- a/redis/log.go +++ b/redis/log.go @@ -16,6 +16,7 @@ package redis import ( "bytes" + "context" "fmt" "log" "time" @@ -121,6 +122,12 @@ func (c *loggingConn) Do(commandName string, args ...interface{}) (interface{}, return reply, err } +func (c *loggingConn) DoContext(ctx context.Context, commandName string, args ...interface{}) (interface{}, error) { + reply, err := DoContext(c.Conn, ctx, commandName, args...) + c.print("DoContext", commandName, args, reply, err) + return reply, err +} + func (c *loggingConn) DoWithTimeout(timeout time.Duration, commandName string, args ...interface{}) (interface{}, error) { reply, err := DoWithTimeout(c.Conn, timeout, commandName, args...) c.print("DoWithTimeout", commandName, args, reply, err) @@ -139,6 +146,12 @@ func (c *loggingConn) Receive() (interface{}, error) { return reply, err } +func (c *loggingConn) ReceiveContext(ctx context.Context) (interface{}, error) { + reply, err := ReceiveContext(c.Conn, ctx) + c.print("ReceiveContext", "", nil, reply, err) + return reply, err +} + func (c *loggingConn) ReceiveWithTimeout(timeout time.Duration) (interface{}, error) { reply, err := ReceiveWithTimeout(c.Conn, timeout) c.print("ReceiveWithTimeout", "", nil, reply, err) diff --git a/redis/pool.go b/redis/pool.go index c7a2f194..d7bb71e0 100644 --- a/redis/pool.go +++ b/redis/pool.go @@ -512,6 +512,20 @@ func (ac *activeConn) Err() error { return pc.c.Err() } +func (ac *activeConn) DoContext(ctx context.Context, commandName string, args ...interface{}) (reply interface{}, err error) { + pc := ac.pc + if pc == nil { + return nil, errConnClosed + } + cwt, ok := pc.c.(ConnWithContext) + if !ok { + return nil, errContextNotSupported + } + ci := lookupCommandInfo(commandName) + ac.state = (ac.state | ci.Set) &^ ci.Clear + return cwt.DoContext(ctx, commandName, args...) +} + func (ac *activeConn) Do(commandName string, args ...interface{}) (reply interface{}, err error) { pc := ac.pc if pc == nil { @@ -562,6 +576,18 @@ func (ac *activeConn) Receive() (reply interface{}, err error) { return pc.c.Receive() } +func (ac *activeConn) ReceiveContext(ctx context.Context) (reply interface{}, err error) { + pc := ac.pc + if pc == nil { + return nil, errConnClosed + } + cwt, ok := pc.c.(ConnWithContext) + if !ok { + return nil, errContextNotSupported + } + return cwt.ReceiveContext(ctx) +} + func (ac *activeConn) ReceiveWithTimeout(timeout time.Duration) (reply interface{}, err error) { pc := ac.pc if pc == nil { @@ -577,6 +603,9 @@ func (ac *activeConn) ReceiveWithTimeout(timeout time.Duration) (reply interface type errorConn struct{ err error } func (ec errorConn) Do(string, ...interface{}) (interface{}, error) { return nil, ec.err } +func (ec errorConn) DoContext(context.Context, string, ...interface{}) (interface{}, error) { + return nil, ec.err +} func (ec errorConn) DoWithTimeout(time.Duration, string, ...interface{}) (interface{}, error) { return nil, ec.err } @@ -585,6 +614,7 @@ func (ec errorConn) Err() error { ret func (ec errorConn) Close() error { return nil } func (ec errorConn) Flush() error { return ec.err } func (ec errorConn) Receive() (interface{}, error) { return nil, ec.err } +func (ec errorConn) ReceiveContext(context.Context) (interface{}, error) { return nil, ec.err } func (ec errorConn) ReceiveWithTimeout(time.Duration) (interface{}, error) { return nil, ec.err } type idleList struct { diff --git a/redis/redis.go b/redis/redis.go index e4464874..5529dbd2 100644 --- a/redis/redis.go +++ b/redis/redis.go @@ -15,6 +15,7 @@ package redis import ( + "context" "errors" "time" ) @@ -33,6 +34,7 @@ type Conn interface { Err() error // Do sends a command to the server and returns the received reply. + // This function will use the timeout which was set when the connection is created Do(commandName string, args ...interface{}) (reply interface{}, err error) // Send writes the command to the client's output buffer. @@ -82,17 +84,52 @@ type Scanner interface { type ConnWithTimeout interface { Conn - // Do sends a command to the server and returns the received reply. - // The timeout overrides the read timeout set when dialing the - // connection. + // DoWithTimeout sends a command to the server and returns the received reply. + // The timeout overrides the readtimeout set when dialing the connection. DoWithTimeout(timeout time.Duration, commandName string, args ...interface{}) (reply interface{}, err error) - // Receive receives a single reply from the Redis server. The timeout - // overrides the read timeout set when dialing the connection. + // ReceiveWithTimeout receives a single reply from the Redis server. + // The timeout overrides the readtimeout set when dialing the connection. ReceiveWithTimeout(timeout time.Duration) (reply interface{}, err error) } +// ConnWithContext is an optional interface that allows the caller to control the command's life with context. +type ConnWithContext interface { + Conn + + // DoContext sends a command to server and returns the received reply. + // min(ctx,DialReadTimeout()) will be used as the deadline. + // The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. + // DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). + // ctx timeout return err context.DeadlineExceeded. + // ctx canceled return err context.Canceled. + DoContext(ctx context.Context, commandName string, args ...interface{}) (reply interface{}, err error) + + // ReceiveContext receives a single reply from the Redis server. + // min(ctx,DialReadTimeout()) will be used as the deadline. + // The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. + // DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). + // ctx timeout return err context.DeadlineExceeded. + // ctx canceled return err context.Canceled. + ReceiveContext(ctx context.Context) (reply interface{}, err error) +} + var errTimeoutNotSupported = errors.New("redis: connection does not support ConnWithTimeout") +var errContextNotSupported = errors.New("redis: connection does not support ConnWithContext") + +// DoContext sends a command to server and returns the received reply. +// min(ctx,DialReadTimeout()) will be used as the deadline. +// The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. +// DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). +// ctx timeout return err context.DeadlineExceeded. +// ctx canceled return err context.Canceled. +func DoContext(c Conn, ctx context.Context, cmd string, args ...interface{}) (interface{}, error) { + cwt, ok := c.(ConnWithContext) + if !ok { + return nil, errContextNotSupported + } + return cwt.DoContext(ctx, cmd, args...) +} // DoWithTimeout executes a Redis command with the specified read timeout. If // the connection does not satisfy the ConnWithTimeout interface, then an error @@ -105,6 +142,20 @@ func DoWithTimeout(c Conn, timeout time.Duration, cmd string, args ...interface{ return cwt.DoWithTimeout(timeout, cmd, args...) } +// ReceiveContext receives a single reply from the Redis server. +// min(ctx,DialReadTimeout()) will be used as the deadline. +// The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. +// DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). +// ctx timeout return err context.DeadlineExceeded. +// ctx canceled return err context.Canceled. +func ReceiveContext(c Conn, ctx context.Context) (interface{}, error) { + cwt, ok := c.(ConnWithContext) + if !ok { + return nil, errContextNotSupported + } + return cwt.ReceiveContext(ctx) +} + // ReceiveWithTimeout receives a reply with the specified read timeout. If the // connection does not satisfy the ConnWithTimeout interface, then an error is // returned. diff --git a/redis/redis_test.go b/redis/redis_test.go index 5a98f535..e0f4538d 100644 --- a/redis/redis_test.go +++ b/redis/redis_test.go @@ -15,6 +15,7 @@ package redis_test import ( + "context" "testing" "time" @@ -26,6 +27,7 @@ type timeoutTestConn int func (tc timeoutTestConn) Do(string, ...interface{}) (interface{}, error) { return time.Duration(-1), nil } + func (tc timeoutTestConn) DoWithTimeout(timeout time.Duration, cmd string, args ...interface{}) (interface{}, error) { return timeout, nil } @@ -33,6 +35,7 @@ func (tc timeoutTestConn) DoWithTimeout(timeout time.Duration, cmd string, args func (tc timeoutTestConn) Receive() (interface{}, error) { return time.Duration(-1), nil } + func (tc timeoutTestConn) ReceiveWithTimeout(timeout time.Duration) (interface{}, error) { return timeout, nil } @@ -69,3 +72,52 @@ func TestPoolConnTimeout(t *testing.T) { p := &redis.Pool{Dial: func() (redis.Conn, error) { return timeoutTestConn(0), nil }} testTimeout(t, p.Get()) } + +type contextDeadTestConn int + +func (cc contextDeadTestConn) Do(string, ...interface{}) (interface{}, error) { + return -1, nil +} +func (cc contextDeadTestConn) DoContext(ctx context.Context, cmd string, args ...interface{}) (interface{}, error) { + return 1, nil +} +func (cc contextDeadTestConn) Receive() (interface{}, error) { + return -1, nil +} +func (cc contextDeadTestConn) ReceiveContext(ctx context.Context) (interface{}, error) { + return 1, nil +} +func (cc contextDeadTestConn) Send(string, ...interface{}) error { return nil } +func (cc contextDeadTestConn) Err() error { return nil } +func (cc contextDeadTestConn) Close() error { return nil } +func (cc contextDeadTestConn) Flush() error { return nil } + +func testcontext(t *testing.T, c redis.Conn) { + r, e := c.Do("PING") + if r != -1 || e != nil { + t.Errorf("Do() = %v, %v, want %v, %v", r, e, -1, nil) + } + ctx, f := context.WithTimeout(context.Background(), time.Minute) + defer f() + r, e = redis.DoContext(c, ctx, "PING") + if r != 1 || e != nil { + t.Errorf("DoContext() = %v, %v, want %v, %v", r, e, 1, nil) + } + r, e = c.Receive() + if r != -1 || e != nil { + t.Errorf("Receive() = %v, %v, want %v, %v", r, e, -1, nil) + } + r, e = redis.ReceiveContext(c, ctx) + if r != 1 || e != nil { + t.Errorf("ReceiveContext() = %v, %v, want %v, %v", r, e, 1, nil) + } +} + +func TestConnContext(t *testing.T) { + testcontext(t, contextDeadTestConn(0)) +} + +func TestPoolConnContext(t *testing.T) { + p := redis.Pool{Dial: func() (redis.Conn, error) { return contextDeadTestConn(0), nil }} + testcontext(t, p.Get()) +} diff --git a/redis/script.go b/redis/script.go index d0cec1ed..bb5d7b00 100644 --- a/redis/script.go +++ b/redis/script.go @@ -15,6 +15,7 @@ package redis import ( + "context" "crypto/sha1" "encoding/hex" "io" @@ -60,6 +61,18 @@ func (s *Script) Hash() string { return s.hash } +func (s *Script) DoContext(ctx context.Context, c Conn, keysAndArgs ...interface{}) (interface{}, error) { + cwt, ok := c.(ConnWithContext) + if !ok { + return nil, errContextNotSupported + } + v, err := cwt.DoContext(ctx, "EVALSHA", s.args(s.hash, keysAndArgs)...) + if e, ok := err.(Error); ok && strings.HasPrefix(string(e), "NOSCRIPT ") { + v, err = cwt.DoContext(ctx, "EVAL", s.args(s.src, keysAndArgs)...) + } + return v, err +} + // Do evaluates the script. Under the covers, Do optimistically evaluates the // script using the EVALSHA command. If the command fails because the script is // not loaded, then Do evaluates the script using the EVAL command (thus From 574218b5788d47ef16ad8eb15a6704314a3e42d5 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Fri, 15 Oct 2021 17:55:44 +0100 Subject: [PATCH 09/36] fix: DoContext call to DoWithTimeout (#576) Fix DoContext call to DoWithTimeout passing args as variadic. Fixes #575 --- redis/conn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/conn.go b/redis/conn.go index 526a5649..a30100d7 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -794,7 +794,7 @@ func (c *conn) DoContext(ctx context.Context, cmd string, args ...interface{}) ( go func() { defer close(endch) - r, e = c.DoWithTimeout(realTimeout, cmd, args) + r, e = c.DoWithTimeout(realTimeout, cmd, args...) }() select { case <-ctx.Done(): From cc6be4883c1590416b62144eceaecea9cb04a1c5 Mon Sep 17 00:00:00 2001 From: Guy Korland Date: Sat, 25 Dec 2021 02:11:02 +0200 Subject: [PATCH 10/36] fix(modules): upgrade testify Upgrade testify to v1.7.0 eliminate security warning --- go.mod | 2 +- go.sum | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index a86a793c..0c2122ca 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module github.com/gomodule/redigo go 1.14 -require github.com/stretchr/testify v1.5.1 +require github.com/stretchr/testify v1.7.0 diff --git a/go.sum b/go.sum index 331fa698..acb88a48 100644 --- a/go.sum +++ b/go.sum @@ -3,9 +3,9 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= -github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 2fd670f4e04a61259027ad949b8ae9ba49702c1c Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Tue, 4 Jan 2022 11:16:55 +0000 Subject: [PATCH 11/36] fix: retract tagged versions (#590) Retract unpublished versions from go tooling. Bump go version to 1.16 required for retract statement. Fixes #585 --- go.mod | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 0c2122ca..4efebf8d 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,10 @@ module github.com/gomodule/redigo -go 1.14 +go 1.16 require github.com/stretchr/testify v1.7.0 + +retract ( + v2.0.0+incompatible // Old development version not maintained or published. + v0.0.0-do-not-use // Never used only present due to lack of retract. +) From a83ebbeea6928a0236f332458532b8e978d51f11 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Tue, 4 Jan 2022 11:27:23 +0000 Subject: [PATCH 12/36] fix: surface underlying error for slice / map helpers (#580) Surface the underlying error when processing results for slice and map helpers so that the user can see the real cause and not a type mismatch error. Also: * Formatting changes to improve error case separation. * Leverage %w in reply errors. Fixes: #579 --- redis/reply.go | 105 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 32 deletions(-) diff --git a/redis/reply.go b/redis/reply.go index dfe6aff7..20232e70 100644 --- a/redis/reply.go +++ b/redis/reply.go @@ -277,13 +277,16 @@ func sliceHelper(reply interface{}, err error, name string, makeSlice func(int), func Float64s(reply interface{}, err error) ([]float64, error) { var result []float64 err = sliceHelper(reply, err, "Float64s", func(n int) { result = make([]float64, n) }, func(i int, v interface{}) error { - p, ok := v.([]byte) - if !ok { - return fmt.Errorf("redigo: unexpected element type for Floats64, got type %T", v) + switch v := v.(type) { + case []byte: + f, err := strconv.ParseFloat(string(v), 64) + result[i] = f + return err + case Error: + return v + default: + return fmt.Errorf("redigo: unexpected element type for Float64s, got type %T", v) } - f, err := strconv.ParseFloat(string(p), 64) - result[i] = f - return err }) return result, err } @@ -302,6 +305,8 @@ func Strings(reply interface{}, err error) ([]string, error) { case []byte: result[i] = string(v) return nil + case Error: + return v default: return fmt.Errorf("redigo: unexpected element type for Strings, got type %T", v) } @@ -316,12 +321,15 @@ func Strings(reply interface{}, err error) ([]string, error) { func ByteSlices(reply interface{}, err error) ([][]byte, error) { var result [][]byte err = sliceHelper(reply, err, "ByteSlices", func(n int) { result = make([][]byte, n) }, func(i int, v interface{}) error { - p, ok := v.([]byte) - if !ok { + switch v := v.(type) { + case []byte: + result[i] = v + return nil + case Error: + return v + default: return fmt.Errorf("redigo: unexpected element type for ByteSlices, got type %T", v) } - result[i] = p - return nil }) return result, err } @@ -341,6 +349,8 @@ func Int64s(reply interface{}, err error) ([]int64, error) { n, err := strconv.ParseInt(string(v), 10, 64) result[i] = n return err + case Error: + return v default: return fmt.Errorf("redigo: unexpected element type for Int64s, got type %T", v) } @@ -367,6 +377,8 @@ func Ints(reply interface{}, err error) ([]int, error) { n, err := strconv.Atoi(string(v)) result[i] = n return err + case Error: + return v default: return fmt.Errorf("redigo: unexpected element type for Ints, got type %T", v) } @@ -382,16 +394,23 @@ func StringMap(result interface{}, err error) (map[string]string, error) { if err != nil { return nil, err } + if len(values)%2 != 0 { - return nil, errors.New("redigo: StringMap expects even number of values result") + return nil, fmt.Errorf("redigo: StringMap expects even number of values result, got %d", len(values)) } + m := make(map[string]string, len(values)/2) for i := 0; i < len(values); i += 2 { - key, okKey := values[i].([]byte) - value, okValue := values[i+1].([]byte) - if !okKey || !okValue { - return nil, errors.New("redigo: StringMap key not a bulk string value") + key, ok := values[i].([]byte) + if !ok { + return nil, fmt.Errorf("redigo: StringMap key[%d] not a bulk string value, got %T", i, values[i]) } + + value, ok := values[i+1].([]byte) + if !ok { + return nil, fmt.Errorf("redigo: StringMap value[%d] not a bulk string value, got %T", i+1, values[i+1]) + } + m[string(key)] = string(value) } return m, nil @@ -405,19 +424,23 @@ func IntMap(result interface{}, err error) (map[string]int, error) { if err != nil { return nil, err } + if len(values)%2 != 0 { - return nil, errors.New("redigo: IntMap expects even number of values result") + return nil, fmt.Errorf("redigo: IntMap expects even number of values result, got %d", len(values)) } + m := make(map[string]int, len(values)/2) for i := 0; i < len(values); i += 2 { key, ok := values[i].([]byte) if !ok { - return nil, errors.New("redigo: IntMap key not a bulk string value") + return nil, fmt.Errorf("redigo: IntMap key[%d] not a bulk string value, got %T", i, values[i]) } + value, err := Int(values[i+1], nil) if err != nil { return nil, err } + m[string(key)] = value } return m, nil @@ -431,19 +454,23 @@ func Int64Map(result interface{}, err error) (map[string]int64, error) { if err != nil { return nil, err } + if len(values)%2 != 0 { - return nil, errors.New("redigo: Int64Map expects even number of values result") + return nil, fmt.Errorf("redigo: Int64Map expects even number of values result, got %d", len(values)) } + m := make(map[string]int64, len(values)/2) for i := 0; i < len(values); i += 2 { key, ok := values[i].([]byte) if !ok { - return nil, errors.New("redigo: Int64Map key not a bulk string value") + return nil, fmt.Errorf("redigo: Int64Map key[%d] not a bulk string value, got %T", i, values[i]) } + value, err := Int64(values[i+1], nil) if err != nil { return nil, err } + m[string(key)] = value } return m, nil @@ -461,21 +488,26 @@ func Positions(result interface{}, err error) ([]*[2]float64, error) { if values[i] == nil { continue } + p, ok := values[i].([]interface{}) if !ok { return nil, fmt.Errorf("redigo: unexpected element type for interface slice, got type %T", values[i]) } + if len(p) != 2 { return nil, fmt.Errorf("redigo: unexpected number of values for a member position, got %d", len(p)) } + lat, err := Float64(p[0], nil) if err != nil { return nil, err } + long, err := Float64(p[1], nil) if err != nil { return nil, err } + positions[i] = &[2]float64{lat, long} } return positions, nil @@ -496,6 +528,8 @@ func Uint64s(reply interface{}, err error) ([]uint64, error) { n, err := strconv.ParseUint(string(v), 10, 64) result[i] = n return err + case Error: + return v default: return fmt.Errorf("redigo: unexpected element type for Uint64s, got type %T", v) } @@ -512,18 +546,20 @@ func Uint64Map(result interface{}, err error) (map[string]uint64, error) { return nil, err } if len(values)%2 != 0 { - return nil, errors.New("redigo: Uint64Map expects even number of values result") + return nil, fmt.Errorf("redigo: Uint64Map expects even number of values result, got %d", len(values)) } m := make(map[string]uint64, len(values)/2) for i := 0; i < len(values); i += 2 { key, ok := values[i].([]byte) if !ok { - return nil, errors.New("redigo: Uint64Map key not a bulk string value") + return nil, fmt.Errorf("redigo: Uint64Map key[%d] not a bulk string value, got %T", i, values[i]) } + value, err := Uint64(values[i+1], nil) if err != nil { return nil, err } + m[string(key)] = value } return m, nil @@ -537,44 +573,49 @@ func SlowLogs(result interface{}, err error) ([]SlowLog, error) { return nil, err } logs := make([]SlowLog, len(rawLogs)) - for i, rawLog := range rawLogs { - rawLog, ok := rawLog.([]interface{}) + for i, e := range rawLogs { + rawLog, ok := e.([]interface{}) if !ok { - return nil, errors.New("redigo: slowlog element is not an array") + return nil, fmt.Errorf("redigo: slowlog element is not an array, got %T", e) } var log SlowLog - if len(rawLog) < 4 { - return nil, errors.New("redigo: slowlog element has less than four elements") + return nil, fmt.Errorf("redigo: slowlog element has %d elements, expected at least 4", len(rawLog)) } + log.ID, ok = rawLog[0].(int64) if !ok { - return nil, errors.New("redigo: slowlog element[0] not an int64") + return nil, fmt.Errorf("redigo: slowlog element[0] not an int64, got %T", rawLog[0]) } + timestamp, ok := rawLog[1].(int64) if !ok { - return nil, errors.New("redigo: slowlog element[1] not an int64") + return nil, fmt.Errorf("redigo: slowlog element[1] not an int64, got %T", rawLog[0]) } + log.Time = time.Unix(timestamp, 0) duration, ok := rawLog[2].(int64) if !ok { - return nil, errors.New("redigo: slowlog element[2] not an int64") + return nil, fmt.Errorf("redigo: slowlog element[2] not an int64, got %T", rawLog[0]) } + log.ExecutionTime = time.Duration(duration) * time.Microsecond log.Args, err = Strings(rawLog[3], nil) if err != nil { - return nil, fmt.Errorf("redigo: slowlog element[3] is not array of string. actual error is : %s", err.Error()) + return nil, fmt.Errorf("redigo: slowlog element[3] is not array of strings: %w", err) } + if len(rawLog) >= 6 { log.ClientAddr, err = String(rawLog[4], nil) if err != nil { - return nil, fmt.Errorf("redigo: slowlog element[4] is not a string. actual error is : %s", err.Error()) + return nil, fmt.Errorf("redigo: slowlog element[4] is not a string: %w", err) } + log.ClientName, err = String(rawLog[5], nil) if err != nil { - return nil, fmt.Errorf("redigo: slowlog element[5] is not a string. actual error is : %s", err.Error()) + return nil, fmt.Errorf("redigo: slowlog element[5] is not a string: %w", err) } } logs[i] = log From 8eb562556f216cfdedaeda62155f411aae308599 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Mon, 14 Mar 2022 16:27:50 -0500 Subject: [PATCH 13/36] fix: correct instructions for detecting DialReadTimeout (#601) The actual string is "i/o timeout", but it's more clear to test error identity in the way documented for net.Conn. Fixes: #600 --- redis/redis.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/redis/redis.go b/redis/redis.go index 5529dbd2..faf574e0 100644 --- a/redis/redis.go +++ b/redis/redis.go @@ -100,7 +100,7 @@ type ConnWithContext interface { // DoContext sends a command to server and returns the received reply. // min(ctx,DialReadTimeout()) will be used as the deadline. // The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. - // DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). + // DialReadTimeout() timeout return err can be checked by errors.Is(err, os.ErrDeadlineExceeded). // ctx timeout return err context.DeadlineExceeded. // ctx canceled return err context.Canceled. DoContext(ctx context.Context, commandName string, args ...interface{}) (reply interface{}, err error) @@ -108,7 +108,7 @@ type ConnWithContext interface { // ReceiveContext receives a single reply from the Redis server. // min(ctx,DialReadTimeout()) will be used as the deadline. // The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. - // DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). + // DialReadTimeout() timeout return err can be checked by errors.Is(err, os.ErrDeadlineExceeded). // ctx timeout return err context.DeadlineExceeded. // ctx canceled return err context.Canceled. ReceiveContext(ctx context.Context) (reply interface{}, err error) @@ -120,7 +120,7 @@ var errContextNotSupported = errors.New("redis: connection does not support Conn // DoContext sends a command to server and returns the received reply. // min(ctx,DialReadTimeout()) will be used as the deadline. // The connection will be closed if DialReadTimeout() timeout or ctx timeout or ctx canceled when this function is running. -// DialReadTimeout() timeout return err can be checked by strings.Contains(e.Error(), "io/timeout"). +// DialReadTimeout() timeout return err can be checked by errors.Is(err, os.ErrDeadlineExceeded). // ctx timeout return err context.DeadlineExceeded. // ctx canceled return err context.Canceled. func DoContext(c Conn, ctx context.Context, cmd string, args ...interface{}) (interface{}, error) { From 3eb077495dae2b6e83c28ff853df73ad86b1d03c Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Wed, 23 Mar 2022 18:56:02 -0500 Subject: [PATCH 14/36] feat: add RequestContext to PubSubConn (#603) Add a wrapper that goes through the standard receiveInternal processing to match the API of the existing PubSubConn Receive methods. Fixes: #592 --- redis/pubsub.go | 8 ++++++++ redis/pubsub_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/redis/pubsub.go b/redis/pubsub.go index cc585757..67b885b9 100644 --- a/redis/pubsub.go +++ b/redis/pubsub.go @@ -15,6 +15,7 @@ package redis import ( + "context" "errors" "time" ) @@ -116,6 +117,13 @@ func (c PubSubConn) ReceiveWithTimeout(timeout time.Duration) interface{} { return c.receiveInternal(ReceiveWithTimeout(c.Conn, timeout)) } +// ReceiveContext is like Receive, but it allows termination of the receive +// via a Context. If the call returns due to closure of the context's Done +// channel the underlying Conn will have been closed. +func (c PubSubConn) ReceiveContext(ctx context.Context) interface{} { + return c.receiveInternal(ReceiveContext(c.Conn, ctx)) +} + func (c PubSubConn) receiveInternal(replyArg interface{}, errArg error) interface{} { reply, err := Values(replyArg, errArg) if err != nil { diff --git a/redis/pubsub_test.go b/redis/pubsub_test.go index 83b91580..63e08692 100644 --- a/redis/pubsub_test.go +++ b/redis/pubsub_test.go @@ -15,6 +15,8 @@ package redis_test import ( + "context" + "errors" "reflect" "testing" "time" @@ -74,3 +76,25 @@ func TestPushed(t *testing.T) { t.Errorf("recv /w timeout got %v, want %v", got, want) } } + +func TestPubSubReceiveContext(t *testing.T) { + sc, err := redis.DialDefaultServer() + if err != nil { + t.Fatalf("error connection to database, %v", err) + } + defer sc.Close() + + c := redis.PubSubConn{Conn: sc} + + require.NoError(t, c.Subscribe("c1")) + expectPushed(t, c, "Subscribe(c1)", redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + got := c.ReceiveContext(ctx) + if err, ok := got.(error); !ok { + t.Errorf("recv w/canceled expected Canceled got non-error type %T", got) + } else if !errors.Is(err, context.Canceled) { + t.Errorf("recv w/canceled expected Canceled got %v", err) + } +} From dbebed54533cf806f080241811e4ea10b0568f92 Mon Sep 17 00:00:00 2001 From: Sown Date: Thu, 24 Mar 2022 23:58:10 +0800 Subject: [PATCH 15/36] feat: add Float64Map (#605) Add a Float64Map helper which can be used to convert a HGETALL responses to a map[string]float64. --- redis/reply.go | 30 ++++++++++++++++++++++++++++++ redis/reply_test.go | 5 +++++ 2 files changed, 35 insertions(+) diff --git a/redis/reply.go b/redis/reply.go index 20232e70..5ece6d55 100644 --- a/redis/reply.go +++ b/redis/reply.go @@ -476,6 +476,36 @@ func Int64Map(result interface{}, err error) (map[string]int64, error) { return m, nil } +// Float64Map is a helper that converts an array of strings (alternating key, value) +// into a map[string]float64. The HGETALL commands return replies in this format. +// Requires an even number of values in result. +func Float64Map(result interface{}, err error) (map[string]float64, error) { + values, err := Values(result, err) + if err != nil { + return nil, err + } + + if len(values)%2 != 0 { + return nil, fmt.Errorf("redigo: Float64Map expects even number of values result, got %d", len(values)) + } + + m := make(map[string]float64, len(values)/2) + for i := 0; i < len(values); i += 2 { + key, ok := values[i].([]byte) + if !ok { + return nil, fmt.Errorf("redigo: Float64Map key[%d] not a bulk string value, got %T", i, values[i]) + } + + value, err := Float64(values[i+1], nil) + if err != nil { + return nil, err + } + + m[string(key)] = value + } + return m, nil +} + // Positions is a helper that converts an array of positions (lat, long) // into a [][2]float64. The GEOPOS command returns replies in this format. func Positions(result interface{}, err error) ([]*[2]float64, error) { diff --git a/redis/reply_test.go b/redis/reply_test.go index 8993bad1..8f43828c 100644 --- a/redis/reply_test.go +++ b/redis/reply_test.go @@ -123,6 +123,11 @@ var replyTests = []struct { ve(redis.Float64(nil, nil)), ve(float64(0.0), redis.ErrNil), }, + { + "float64Map([[]byte, []byte])", + ve(redis.Float64Map([]interface{}{[]byte("key1"), []byte("1.234"), []byte("key2"), []byte("5.678")}, nil)), + ve(map[string]float64{"key1": 1.234, "key2": 5.678}, nil), + }, { "uint64(1)", ve(redis.Uint64(int64(1), nil)), From dc6757622d9f31acc732374ca037753d4e8916e3 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Thu, 24 Mar 2022 23:16:57 +0000 Subject: [PATCH 16/36] chore: add go test action (#607) Add a github action which runs go test so we can ensure that tests are passing before we merge changes. --- .github/workflows/go-test.yml | 58 +++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .github/workflows/go-test.yml diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml new file mode 100644 index 00000000..1086bf3a --- /dev/null +++ b/.github/workflows/go-test.yml @@ -0,0 +1,58 @@ +name: go-test +on: + push: + tags: + - v* + branches: + - master + pull_request: +jobs: + build: + runs-on: ${{ matrix.os }} + strategy: + matrix: + go-version: + - '1.17.x' + - '1.18.x' + os: + - 'ubuntu-latest' + redis: + - '6.2' + - '6.0' + - '5.0' + - '4.0' + name: Test go ${{ matrix.go-version }} redis ${{ matrix.redis }} on ${{ matrix.os }} + steps: + - name: Setup redis + uses: shogo82148/actions-setup-redis@v1 + with: + redis-version: ${{ matrix.redis }} + + - name: Install Go + uses: actions/setup-go@v3 + with: + go-version: ${{ matrix.go-version }} + + - name: Go Cache Paths + id: go-cache-paths + run: | + echo "::set-output name=go-build::$(go env GOCACHE)" + echo "::set-output name=go-mod::$(go env GOMODCACHE)" + + - name: Checkout code + uses: actions/checkout@v3 + + - name: Go Build Cache + uses: actions/cache@v3 + with: + path: ${{ steps.go-cache-paths.outputs.go-build }} + key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }} + + - name: Go Mod Cache + uses: actions/cache@v3 + with: + path: ${{ steps.go-cache-paths.outputs.go-mod }} + key: ${{ runner.os }}-go-mod-${{ hashFiles('**/go.sum') }} + + - name: Go Test + run: go test -v ./... From 5b789c6cfe824c4f0b7d08ed1bc0960d5142adf1 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Thu, 24 Mar 2022 23:21:15 +0000 Subject: [PATCH 17/36] chore: refactor map helpers to reduce duplication (#606) Refactor the reply Map helpers to reduce the code duplication in the body of those functions by creating a mapHelper along the same lines as the existing sliceHelper. --- redis/reply.go | 189 ++++++++++++++++++++++++------------------------- 1 file changed, 91 insertions(+), 98 deletions(-) diff --git a/redis/reply.go b/redis/reply.go index 5ece6d55..11a8aa16 100644 --- a/redis/reply.go +++ b/redis/reply.go @@ -386,124 +386,122 @@ func Ints(reply interface{}, err error) ([]int, error) { return result, err } -// StringMap is a helper that converts an array of strings (alternating key, value) -// into a map[string]string. The HGETALL and CONFIG GET commands return replies in this format. -// Requires an even number of values in result. -func StringMap(result interface{}, err error) (map[string]string, error) { - values, err := Values(result, err) +// mapHelper builds a map from the data in reply. +func mapHelper(reply interface{}, err error, name string, makeMap func(int), assign func(key string, value interface{}) error) error { + values, err := Values(reply, err) if err != nil { - return nil, err + return err } if len(values)%2 != 0 { - return nil, fmt.Errorf("redigo: StringMap expects even number of values result, got %d", len(values)) + return fmt.Errorf("redigo: %s expects even number of values result, got %d", name, len(values)) } - m := make(map[string]string, len(values)/2) + makeMap(len(values) / 2) for i := 0; i < len(values); i += 2 { key, ok := values[i].([]byte) if !ok { - return nil, fmt.Errorf("redigo: StringMap key[%d] not a bulk string value, got %T", i, values[i]) + return fmt.Errorf("redigo: %s key[%d] not a bulk string value, got %T", name, i, values[i]) } - value, ok := values[i+1].([]byte) - if !ok { - return nil, fmt.Errorf("redigo: StringMap value[%d] not a bulk string value, got %T", i+1, values[i+1]) + if err := assign(string(key), values[i+1]); err != nil { + return err } - - m[string(key)] = string(value) } - return m, nil + + return nil +} + +// StringMap is a helper that converts an array of strings (alternating key, value) +// into a map[string]string. The HGETALL and CONFIG GET commands return replies in this format. +// Requires an even number of values in result. +func StringMap(reply interface{}, err error) (map[string]string, error) { + var result map[string]string + err = mapHelper(reply, err, "StringMap", + func(n int) { + result = make(map[string]string, n) + }, func(key string, v interface{}) error { + value, ok := v.([]byte) + if !ok { + return fmt.Errorf("redigo: StringMap for %q not a bulk string value, got %T", key, v) + } + + result[key] = string(value) + + return nil + }, + ) + + return result, err } // IntMap is a helper that converts an array of strings (alternating key, value) // into a map[string]int. The HGETALL commands return replies in this format. // Requires an even number of values in result. func IntMap(result interface{}, err error) (map[string]int, error) { - values, err := Values(result, err) - if err != nil { - return nil, err - } - - if len(values)%2 != 0 { - return nil, fmt.Errorf("redigo: IntMap expects even number of values result, got %d", len(values)) - } + var m map[string]int + err = mapHelper(result, err, "IntMap", + func(n int) { + m = make(map[string]int, n) + }, func(key string, v interface{}) error { + value, err := Int(v, nil) + if err != nil { + return err + } - m := make(map[string]int, len(values)/2) - for i := 0; i < len(values); i += 2 { - key, ok := values[i].([]byte) - if !ok { - return nil, fmt.Errorf("redigo: IntMap key[%d] not a bulk string value, got %T", i, values[i]) - } + m[key] = value - value, err := Int(values[i+1], nil) - if err != nil { - return nil, err - } + return nil + }, + ) - m[string(key)] = value - } - return m, nil + return m, err } // Int64Map is a helper that converts an array of strings (alternating key, value) // into a map[string]int64. The HGETALL commands return replies in this format. // Requires an even number of values in result. func Int64Map(result interface{}, err error) (map[string]int64, error) { - values, err := Values(result, err) - if err != nil { - return nil, err - } - - if len(values)%2 != 0 { - return nil, fmt.Errorf("redigo: Int64Map expects even number of values result, got %d", len(values)) - } + var m map[string]int64 + err = mapHelper(result, err, "Int64Map", + func(n int) { + m = make(map[string]int64, n) + }, func(key string, v interface{}) error { + value, err := Int64(v, nil) + if err != nil { + return err + } - m := make(map[string]int64, len(values)/2) - for i := 0; i < len(values); i += 2 { - key, ok := values[i].([]byte) - if !ok { - return nil, fmt.Errorf("redigo: Int64Map key[%d] not a bulk string value, got %T", i, values[i]) - } + m[key] = value - value, err := Int64(values[i+1], nil) - if err != nil { - return nil, err - } + return nil + }, + ) - m[string(key)] = value - } - return m, nil + return m, err } // Float64Map is a helper that converts an array of strings (alternating key, value) // into a map[string]float64. The HGETALL commands return replies in this format. // Requires an even number of values in result. func Float64Map(result interface{}, err error) (map[string]float64, error) { - values, err := Values(result, err) - if err != nil { - return nil, err - } - - if len(values)%2 != 0 { - return nil, fmt.Errorf("redigo: Float64Map expects even number of values result, got %d", len(values)) - } + var m map[string]float64 + err = mapHelper(result, err, "Float64Map", + func(n int) { + m = make(map[string]float64, n) + }, func(key string, v interface{}) error { + value, err := Float64(v, nil) + if err != nil { + return err + } - m := make(map[string]float64, len(values)/2) - for i := 0; i < len(values); i += 2 { - key, ok := values[i].([]byte) - if !ok { - return nil, fmt.Errorf("redigo: Float64Map key[%d] not a bulk string value, got %T", i, values[i]) - } + m[key] = value - value, err := Float64(values[i+1], nil) - if err != nil { - return nil, err - } + return nil + }, + ) - m[string(key)] = value - } - return m, nil + return m, err } // Positions is a helper that converts an array of positions (lat, long) @@ -571,28 +569,23 @@ func Uint64s(reply interface{}, err error) ([]uint64, error) { // into a map[string]uint64. The HGETALL commands return replies in this format. // Requires an even number of values in result. func Uint64Map(result interface{}, err error) (map[string]uint64, error) { - values, err := Values(result, err) - if err != nil { - return nil, err - } - if len(values)%2 != 0 { - return nil, fmt.Errorf("redigo: Uint64Map expects even number of values result, got %d", len(values)) - } - m := make(map[string]uint64, len(values)/2) - for i := 0; i < len(values); i += 2 { - key, ok := values[i].([]byte) - if !ok { - return nil, fmt.Errorf("redigo: Uint64Map key[%d] not a bulk string value, got %T", i, values[i]) - } + var m map[string]uint64 + err = mapHelper(result, err, "Uint64Map", + func(n int) { + m = make(map[string]uint64, n) + }, func(key string, v interface{}) error { + value, err := Uint64(v, nil) + if err != nil { + return err + } - value, err := Uint64(values[i+1], nil) - if err != nil { - return nil, err - } + m[key] = value - m[string(key)] = value - } - return m, nil + return nil + }, + ) + + return m, err } // SlowLogs is a helper that parse the SLOWLOG GET command output and From 222ca62caed6b63d9ebb57709c26d0bca6572f5d Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Mon, 6 Jun 2022 14:56:29 +0100 Subject: [PATCH 18/36] fix: update golangci-lint for golang 1.18 (#616) Update golangci-lint to a version compatible with golang 1.18. --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index e59fd3c7..eb7a22f8 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -16,7 +16,7 @@ jobs: uses: golangci/golangci-lint-action@v2 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.36 + version: v1.46.2 # Optional: working directory, useful for monorepos # working-directory: somedir From bcef0d823f2c8dee231057b80a6e0f0d44529fc9 Mon Sep 17 00:00:00 2001 From: dmitri-lerko Date: Tue, 7 Jun 2022 20:17:50 +0100 Subject: [PATCH 19/36] fix: error message for SlowLog conversion (#612) Fix incorrect offset in error message for SlowLog conversion. --- redis/reply.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redis/reply.go b/redis/reply.go index 11a8aa16..7a2b3fef 100644 --- a/redis/reply.go +++ b/redis/reply.go @@ -614,13 +614,13 @@ func SlowLogs(result interface{}, err error) ([]SlowLog, error) { timestamp, ok := rawLog[1].(int64) if !ok { - return nil, fmt.Errorf("redigo: slowlog element[1] not an int64, got %T", rawLog[0]) + return nil, fmt.Errorf("redigo: slowlog element[1] not an int64, got %T", rawLog[1]) } log.Time = time.Unix(timestamp, 0) duration, ok := rawLog[2].(int64) if !ok { - return nil, fmt.Errorf("redigo: slowlog element[2] not an int64, got %T", rawLog[0]) + return nil, fmt.Errorf("redigo: slowlog element[2] not an int64, got %T", rawLog[2]) } log.ExecutionTime = time.Duration(duration) * time.Microsecond From 95c091f7bec57b79bbc3e2d96b184c042ddf85d4 Mon Sep 17 00:00:00 2001 From: mitchsw Date: Fri, 1 Jul 2022 11:03:21 -0400 Subject: [PATCH 20/36] fix: respect ctx in cmds run in DialContext (#620) Update DialContext() to use the new DoContext() methods (added in v1.8.6) when configuring auth/clientName/db during connection creation. This prevents DialContex() from blocking for a long time if the redis server is unresponsive. --- redis/conn.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/redis/conn.go b/redis/conn.go index a30100d7..3695b843 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -291,21 +291,21 @@ func DialContext(ctx context.Context, network, address string, options ...DialOp authArgs = append(authArgs, do.username) } authArgs = append(authArgs, do.password) - if _, err := c.Do("AUTH", authArgs...); err != nil { + if _, err := c.DoContext(ctx, "AUTH", authArgs...); err != nil { netConn.Close() return nil, err } } if do.clientName != "" { - if _, err := c.Do("CLIENT", "SETNAME", do.clientName); err != nil { + if _, err := c.DoContext(ctx, "CLIENT", "SETNAME", do.clientName); err != nil { netConn.Close() return nil, err } } if do.db != 0 { - if _, err := c.Do("SELECT", do.db); err != nil { + if _, err := c.DoContext(ctx, "SELECT", do.db); err != nil { netConn.Close() return nil, err } From 2c2a5c2cd0f50646d6240f8df729cb0dc1438f63 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Fri, 1 Jul 2022 16:04:19 +0100 Subject: [PATCH 21/36] chore: remove go1.7 and below support (#623) Remove go1.7 support which hasn't been maintained for a long time now. --- redis/conn.go | 2 +- redis/go17.go | 29 ----------------------------- redis/go18.go | 9 --------- 3 files changed, 1 insertion(+), 39 deletions(-) delete mode 100644 redis/go17.go delete mode 100644 redis/go18.go diff --git a/redis/conn.go b/redis/conn.go index 3695b843..753644b1 100644 --- a/redis/conn.go +++ b/redis/conn.go @@ -246,7 +246,7 @@ func DialContext(ctx context.Context, network, address string, options ...DialOp if do.tlsConfig == nil { tlsConfig = &tls.Config{InsecureSkipVerify: do.skipVerify} } else { - tlsConfig = cloneTLSConfig(do.tlsConfig) + tlsConfig = do.tlsConfig.Clone() } if tlsConfig.ServerName == "" { host, _, err := net.SplitHostPort(address) diff --git a/redis/go17.go b/redis/go17.go deleted file mode 100644 index 5f363791..00000000 --- a/redis/go17.go +++ /dev/null @@ -1,29 +0,0 @@ -// +build go1.7,!go1.8 - -package redis - -import "crypto/tls" - -func cloneTLSConfig(cfg *tls.Config) *tls.Config { - return &tls.Config{ - Rand: cfg.Rand, - Time: cfg.Time, - Certificates: cfg.Certificates, - NameToCertificate: cfg.NameToCertificate, - GetCertificate: cfg.GetCertificate, - RootCAs: cfg.RootCAs, - NextProtos: cfg.NextProtos, - ServerName: cfg.ServerName, - ClientAuth: cfg.ClientAuth, - ClientCAs: cfg.ClientCAs, - InsecureSkipVerify: cfg.InsecureSkipVerify, - CipherSuites: cfg.CipherSuites, - PreferServerCipherSuites: cfg.PreferServerCipherSuites, - ClientSessionCache: cfg.ClientSessionCache, - MinVersion: cfg.MinVersion, - MaxVersion: cfg.MaxVersion, - CurvePreferences: cfg.CurvePreferences, - DynamicRecordSizingDisabled: cfg.DynamicRecordSizingDisabled, - Renegotiation: cfg.Renegotiation, - } -} diff --git a/redis/go18.go b/redis/go18.go deleted file mode 100644 index 558363be..00000000 --- a/redis/go18.go +++ /dev/null @@ -1,9 +0,0 @@ -// +build go1.8 - -package redis - -import "crypto/tls" - -func cloneTLSConfig(cfg *tls.Config) *tls.Config { - return cfg.Clone() -} From d3b4cc3b15ed30170612869b851328d21cb64356 Mon Sep 17 00:00:00 2001 From: Wenpeng Date: Fri, 1 Jul 2022 23:19:52 +0800 Subject: [PATCH 22/36] fix: correct do script error check (#563) Correct the error check in the Do method so it's more idiomatic. --- redis/script.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/script.go b/redis/script.go index bb5d7b00..c585518b 100644 --- a/redis/script.go +++ b/redis/script.go @@ -79,7 +79,7 @@ func (s *Script) DoContext(ctx context.Context, c Conn, keysAndArgs ...interface // causing the script to load). func (s *Script) Do(c Conn, keysAndArgs ...interface{}) (interface{}, error) { v, err := c.Do("EVALSHA", s.args(s.hash, keysAndArgs)...) - if e, ok := err.(Error); ok && strings.HasPrefix(string(e), "NOSCRIPT ") { + if err != nil && strings.HasPrefix(err.Error(), "NOSCRIPT ") { v, err = c.Do("EVAL", s.args(s.src, keysAndArgs)...) } return v, err From f1e923c7e2cbb9bc8c294c858074a7c2b8a695cb Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Wed, 6 Jul 2022 12:23:14 +0100 Subject: [PATCH 23/36] fix: Anonymous field pointers (#622) Fix panic using ArgsFlat or ScanStruct on structs with nil Anonymous field pointers. Catch the anonymous struct recursion and prevent it. In the case of ScanStruct an error will be returned, in the case of ArgsFlat it will panic with a nice error. --- README.markdown | 1 - redis/reflect.go | 48 ++++++++ redis/reflect_go117.go | 34 ++++++ redis/reflect_go118.go | 16 +++ redis/scan.go | 69 +++++++++--- redis/scan_test.go | 245 ++++++++++++++++++++++++++++++++--------- 6 files changed, 342 insertions(+), 71 deletions(-) create mode 100644 redis/reflect.go create mode 100644 redis/reflect_go117.go create mode 100644 redis/reflect_go118.go diff --git a/README.markdown b/README.markdown index d418cbb2..7b7234b3 100644 --- a/README.markdown +++ b/README.markdown @@ -1,7 +1,6 @@ Redigo ====== -[![Build Status](https://travis-ci.org/gomodule/redigo.svg?branch=master)](https://travis-ci.org/gomodule/redigo) [![GoDoc](https://godoc.org/github.com/gomodule/redigo/redis?status.svg)](https://pkg.go.dev/github.com/gomodule/redigo/redis) Redigo is a [Go](http://golang.org/) client for the [Redis](http://redis.io/) database. diff --git a/redis/reflect.go b/redis/reflect.go new file mode 100644 index 00000000..e135aed7 --- /dev/null +++ b/redis/reflect.go @@ -0,0 +1,48 @@ +package redis + +import ( + "reflect" + "runtime" +) + +// methodName returns the name of the calling method, +// assumed to be two stack frames above. +func methodName() string { + pc, _, _, _ := runtime.Caller(2) + f := runtime.FuncForPC(pc) + if f == nil { + return "unknown method" + } + return f.Name() +} + +// mustBe panics if f's kind is not expected. +func mustBe(v reflect.Value, expected reflect.Kind) { + if v.Kind() != expected { + panic(&reflect.ValueError{Method: methodName(), Kind: v.Kind()}) + } +} + +// fieldByIndexCreate returns the nested field corresponding +// to index creating elements that are nil when stepping through. +// It panics if v is not a struct. +func fieldByIndexCreate(v reflect.Value, index []int) reflect.Value { + if len(index) == 1 { + return v.Field(index[0]) + } + + mustBe(v, reflect.Struct) + for i, x := range index { + if i > 0 { + if v.Kind() == reflect.Ptr && v.Type().Elem().Kind() == reflect.Struct { + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) + } + v = v.Elem() + } + } + v = v.Field(x) + } + + return v +} diff --git a/redis/reflect_go117.go b/redis/reflect_go117.go new file mode 100644 index 00000000..e985192a --- /dev/null +++ b/redis/reflect_go117.go @@ -0,0 +1,34 @@ +//go:build !go1.18 +// +build !go1.18 + +package redis + +import ( + "errors" + "reflect" +) + +// fieldByIndexErr returns the nested field corresponding to index. +// It returns an error if evaluation requires stepping through a nil +// pointer, but panics if it must step through a field that +// is not a struct. +func fieldByIndexErr(v reflect.Value, index []int) (reflect.Value, error) { + if len(index) == 1 { + return v.Field(index[0]), nil + } + + mustBe(v, reflect.Struct) + for i, x := range index { + if i > 0 { + if v.Kind() == reflect.Ptr && v.Type().Elem().Kind() == reflect.Struct { + if v.IsNil() { + return reflect.Value{}, errors.New("reflect: indirection through nil pointer to embedded struct field " + v.Type().Elem().Name()) + } + v = v.Elem() + } + } + v = v.Field(x) + } + + return v, nil +} diff --git a/redis/reflect_go118.go b/redis/reflect_go118.go new file mode 100644 index 00000000..3356e76f --- /dev/null +++ b/redis/reflect_go118.go @@ -0,0 +1,16 @@ +//go:build go1.18 +// +build go1.18 + +package redis + +import ( + "reflect" +) + +// fieldByIndexErr returns the nested field corresponding to index. +// It returns an error if evaluation requires stepping through a nil +// pointer, but panics if it must step through a field that +// is not a struct. +func fieldByIndexErr(v reflect.Value, index []int) (reflect.Value, error) { + return v.FieldByIndexErr(index) +} diff --git a/redis/scan.go b/redis/scan.go index 379206ed..82121011 100644 --- a/redis/scan.go +++ b/redis/scan.go @@ -355,7 +355,13 @@ func (ss *structSpec) fieldSpec(name []byte) *fieldSpec { return ss.m[string(name)] } -func compileStructSpec(t reflect.Type, depth map[string]int, index []int, ss *structSpec) { +func compileStructSpec(t reflect.Type, depth map[string]int, index []int, ss *structSpec, seen map[reflect.Type]struct{}) error { + if _, ok := seen[t]; ok { + // Protect against infinite recursion. + return fmt.Errorf("recursive struct definition for %v", t) + } + + seen[t] = struct{}{} LOOP: for i := 0; i < t.NumField(); i++ { f := t.Field(i) @@ -365,20 +371,21 @@ LOOP: case f.Anonymous: switch f.Type.Kind() { case reflect.Struct: - compileStructSpec(f.Type, depth, append(index, i), ss) + if err := compileStructSpec(f.Type, depth, append(index, i), ss, seen); err != nil { + return err + } case reflect.Ptr: - // TODO(steve): Protect against infinite recursion. if f.Type.Elem().Kind() == reflect.Struct { - compileStructSpec(f.Type.Elem(), depth, append(index, i), ss) + if err := compileStructSpec(f.Type.Elem(), depth, append(index, i), ss, seen); err != nil { + return err + } } } default: fs := &fieldSpec{name: f.Name} tag := f.Tag.Get("redis") - var ( - p string - ) + var p string first := true for len(tag) > 0 { i := strings.IndexByte(tag, ',') @@ -402,10 +409,12 @@ LOOP: } } } + d, found := depth[fs.name] if !found { d = 1 << 30 } + switch { case len(index) == d: // At same depth, remove from result. @@ -428,6 +437,8 @@ LOOP: } } } + + return nil } var ( @@ -435,26 +446,27 @@ var ( structSpecCache = make(map[reflect.Type]*structSpec) ) -func structSpecForType(t reflect.Type) *structSpec { - +func structSpecForType(t reflect.Type) (*structSpec, error) { structSpecMutex.RLock() ss, found := structSpecCache[t] structSpecMutex.RUnlock() if found { - return ss + return ss, nil } structSpecMutex.Lock() defer structSpecMutex.Unlock() ss, found = structSpecCache[t] if found { - return ss + return ss, nil } ss = &structSpec{m: make(map[string]*fieldSpec)} - compileStructSpec(t, make(map[string]int), nil, ss) + if err := compileStructSpec(t, make(map[string]int), nil, ss, make(map[reflect.Type]struct{})); err != nil { + return nil, fmt.Errorf("compile struct: %s: %w", t, err) + } structSpecCache[t] = ss - return ss + return ss, nil } var errScanStructValue = errors.New("redigo.ScanStruct: value must be non-nil pointer to a struct") @@ -480,30 +492,38 @@ func ScanStruct(src []interface{}, dest interface{}) error { if d.Kind() != reflect.Ptr || d.IsNil() { return errScanStructValue } + d = d.Elem() if d.Kind() != reflect.Struct { return errScanStructValue } - ss := structSpecForType(d.Type()) if len(src)%2 != 0 { return errors.New("redigo.ScanStruct: number of values not a multiple of 2") } + ss, err := structSpecForType(d.Type()) + if err != nil { + return fmt.Errorf("redigo.ScanStruct: %w", err) + } + for i := 0; i < len(src); i += 2 { s := src[i+1] if s == nil { continue } + name, ok := src[i].([]byte) if !ok { return fmt.Errorf("redigo.ScanStruct: key %d not a bulk string value", i) } + fs := ss.fieldSpec(name) if fs == nil { continue } - if err := convertAssignValue(d.FieldByIndex(fs.index), s); err != nil { + + if err := convertAssignValue(fieldByIndexCreate(d, fs.index), s); err != nil { return fmt.Errorf("redigo.ScanStruct: cannot assign field %s: %v", fs.name, err) } } @@ -555,7 +575,11 @@ func ScanSlice(src []interface{}, dest interface{}, fieldNames ...string) error return nil } - ss := structSpecForType(t) + ss, err := structSpecForType(t) + if err != nil { + return fmt.Errorf("redigo.ScanSlice: %w", err) + } + fss := ss.l if len(fieldNames) > 0 { fss = make([]*fieldSpec, len(fieldNames)) @@ -618,6 +642,7 @@ func (args Args) Add(value ...interface{}) Args { // for more information on the use of the 'redis' field tag. // // Other types are appended to args as is. +// panics if v includes a recursive anonymous struct. func (args Args) AddFlat(v interface{}) Args { rv := reflect.ValueOf(v) switch rv.Kind() { @@ -646,9 +671,17 @@ func (args Args) AddFlat(v interface{}) Args { } func flattenStruct(args Args, v reflect.Value) Args { - ss := structSpecForType(v.Type()) + ss, err := structSpecForType(v.Type()) + if err != nil { + panic(fmt.Errorf("redigo.AddFlat: %w", err)) + } + for _, fs := range ss.l { - fv := v.FieldByIndex(fs.index) + fv, err := fieldByIndexErr(v, fs.index) + if err != nil { + // Nil item ignore. + continue + } if fs.omitEmpty { var empty = false switch fv.Kind() { diff --git a/redis/scan_test.go b/redis/scan_test.go index d43c372e..53556ebb 100644 --- a/redis/scan_test.go +++ b/redis/scan_test.go @@ -233,12 +233,15 @@ type s1 struct { Sdp *durationScan `redis:"sdp"` } -var boolTrue = true +var ( + boolTrue = true + int5 = 5 +) var scanStructTests = []struct { - title string - reply []string - value interface{} + name string + reply []string + expected interface{} }{ {"basic", []string{ @@ -273,25 +276,54 @@ var scanStructTests = []struct { []string{}, &s1{}, }, + {"struct-anonymous-nil", + []string{"edi", "2"}, + &struct { + Ed + *Edp + }{ + Ed: Ed{EdI: 2}, + }, + }, + {"struct-anonymous-multi-nil-early", + []string{"edi", "2"}, + &struct { + Ed + *Ed2 + }{ + Ed: Ed{EdI: 2}, + }, + }, + {"struct-anonymous-multi-nil-late", + []string{"edi", "2", "ed2i", "3", "edp2i", "4"}, + &struct { + Ed + *Ed2 + }{ + Ed: Ed{EdI: 2}, + Ed2: &Ed2{ + Ed2I: 3, + Edp2: &Edp2{ + Edp2I: 4, + }, + }, + }, + }, } func TestScanStruct(t *testing.T) { for _, tt := range scanStructTests { + t.Run(tt.name, func(t *testing.T) { + reply := make([]interface{}, len(tt.reply)) + for i, v := range tt.reply { + reply[i] = []byte(v) + } - var reply []interface{} - for _, v := range tt.reply { - reply = append(reply, []byte(v)) - } - - value := reflect.New(reflect.ValueOf(tt.value).Type().Elem()) - - if err := redis.ScanStruct(reply, value.Interface()); err != nil { - t.Fatalf("ScanStruct(%s) returned error %v", tt.title, err) - } - - if !reflect.DeepEqual(value.Interface(), tt.value) { - t.Fatalf("ScanStruct(%s) returned %v, want %v", tt.title, value.Interface(), tt.value) - } + value := reflect.New(reflect.ValueOf(tt.expected).Type().Elem()).Interface() + err := redis.ScanStruct(reply, value) + require.NoError(t, err) + require.Equal(t, tt.expected, value) + }) } } @@ -486,73 +518,182 @@ type Edp struct { EdpI int `redis:"edpi"` } +type Ed2 struct { + Ed2I int `redis:"ed2i"` + *Edp2 +} + +type Edp2 struct { + Edp2I int `redis:"edp2i"` + *Edp +} + +type Edpr1 struct { + Edpr1I int `redis:"edpr1i"` + *Edpr2 +} + +type Edpr2 struct { + Edpr2I int `redis:"edpr2i"` + *Edpr1 +} + var argsTests = []struct { title string - actual redis.Args + fn func() redis.Args expected redis.Args + panics bool }{ {"struct-ptr", - redis.Args{}.AddFlat(&struct { - I int `redis:"i"` - U uint `redis:"u"` - S string `redis:"s"` - P []byte `redis:"p"` - M map[string]string `redis:"m"` - Bt bool - Bf bool - PtrB *bool - PtrI *int - }{ - -1234, 5678, "hello", []byte("world"), map[string]string{"hello": "world"}, true, false, &boolTrue, nil, - }), - redis.Args{"i", int(-1234), "u", uint(5678), "s", "hello", "p", []byte("world"), "m", map[string]string{"hello": "world"}, "Bt", true, "Bf", false, "PtrB", true}, + func() redis.Args { + return redis.Args{}.AddFlat(&struct { + I int `redis:"i"` + U uint `redis:"u"` + S string `redis:"s"` + P []byte `redis:"p"` + M map[string]string `redis:"m"` + Bt bool + Bf bool + PtrB *bool + PtrI *int + PtrI2 *int + }{ + -1234, 5678, "hello", []byte("world"), map[string]string{"hello": "world"}, true, false, &boolTrue, nil, &int5, + }) + }, + redis.Args{"i", int(-1234), "u", uint(5678), "s", "hello", "p", []byte("world"), "m", map[string]string{"hello": "world"}, "Bt", true, "Bf", false, "PtrB", true, "PtrI2", 5}, + false, }, {"struct", - redis.Args{}.AddFlat(struct{ I int }{123}), + func() redis.Args { + return redis.Args{}.AddFlat(struct{ I int }{123}) + }, redis.Args{"I", 123}, + false, }, {"struct-with-RedisArg-direct", - redis.Args{}.AddFlat(struct{ T CustomTime }{CustomTime{Time: time.Unix(1573231058, 0)}}), + func() redis.Args { + return redis.Args{}.AddFlat(struct{ T CustomTime }{CustomTime{Time: time.Unix(1573231058, 0)}}) + }, redis.Args{"T", int64(1573231058)}, + false, }, {"struct-with-RedisArg-direct-ptr", - redis.Args{}.AddFlat(struct{ T *CustomTime }{&CustomTime{Time: time.Unix(1573231058, 0)}}), + func() redis.Args { + return redis.Args{}.AddFlat(struct{ T *CustomTime }{&CustomTime{Time: time.Unix(1573231058, 0)}}) + }, redis.Args{"T", int64(1573231058)}, + false, }, {"struct-with-RedisArg-ptr", - redis.Args{}.AddFlat(struct{ T *CustomTimePtr }{&CustomTimePtr{Time: time.Unix(1573231058, 0)}}), + func() redis.Args { + return redis.Args{}.AddFlat(struct{ T *CustomTimePtr }{&CustomTimePtr{Time: time.Unix(1573231058, 0)}}) + }, redis.Args{"T", int64(1573231058)}, + false, }, {"slice", - redis.Args{}.Add(1).AddFlat([]string{"a", "b", "c"}).Add(2), + func() redis.Args { + return redis.Args{}.Add(1).AddFlat([]string{"a", "b", "c"}).Add(2) + }, redis.Args{1, "a", "b", "c", 2}, + false, }, {"struct-omitempty", - redis.Args{}.AddFlat(&struct { - Sdp *durationArg `redis:"Sdp,omitempty"` - }{ - nil, - }), + func() redis.Args { + return redis.Args{}.AddFlat(&struct { + Sdp *durationArg `redis:"Sdp,omitempty"` + }{ + nil, + }) + }, redis.Args{}, + false, }, {"struct-anonymous", - redis.Args{}.AddFlat(struct { - Ed - *Edp - }{ - Ed{EdI: 2}, - &Edp{EdpI: 3}, - }), + func() redis.Args { + return redis.Args{}.AddFlat(struct { + Ed + *Edp + }{ + Ed{EdI: 2}, + &Edp{EdpI: 3}, + }) + }, redis.Args{"edi", 2, "edpi", 3}, + false, + }, + {"struct-anonymous-nil", + func() redis.Args { + return redis.Args{}.AddFlat(struct { + Ed + *Edp + }{ + Ed: Ed{EdI: 2}, + }) + }, + redis.Args{"edi", 2}, + false, + }, + {"struct-anonymous-multi-nil-early", + func() redis.Args { + return redis.Args{}.AddFlat(struct { + Ed + *Ed2 + }{ + Ed: Ed{EdI: 2}, + }) + }, + redis.Args{"edi", 2}, + false, + }, + {"struct-anonymous-multi-nil-late", + func() redis.Args { + return redis.Args{}.AddFlat(struct { + Ed + *Ed2 + }{ + Ed: Ed{EdI: 2}, + Ed2: &Ed2{ + Ed2I: 3, + Edp2: &Edp2{ + Edp2I: 4, + }, + }, + }) + }, + redis.Args{"edi", 2, "ed2i", 3, "edp2i", 4}, + false, + }, + {"struct-recursive-ptr", + func() redis.Args { + return redis.Args{}.AddFlat(struct { + Edpr1 + }{ + Edpr1: Edpr1{ + Edpr1I: 1, + Edpr2: &Edpr2{ + Edpr2I: 2, + Edpr1: &Edpr1{ + Edpr1I: 10, + }, + }, + }, + }) + }, + redis.Args{}, + true, }, } func TestArgs(t *testing.T) { for _, tt := range argsTests { t.Run(tt.title, func(t *testing.T) { - if !reflect.DeepEqual(tt.actual, tt.expected) { - t.Fatalf("is %v, want %v", tt.actual, tt.expected) + if tt.panics { + require.Panics(t, func() { tt.fn() }) + return } + require.Equal(t, tt.expected, tt.fn()) }) } } From d6854479365f0307560fa28e18e2bd0634b05229 Mon Sep 17 00:00:00 2001 From: dmitri-lerko Date: Wed, 6 Jul 2022 12:30:13 +0100 Subject: [PATCH 24/36] feat: add support for latency command parsing (#614) Add support for LATENCY LATEST, LATEST HISTORY command parsing. --- redis/redis.go | 24 ++++++++++++ redis/reply.go | 88 ++++++++++++++++++++++++++++++++++++++++++++ redis/reply_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) diff --git a/redis/redis.go b/redis/redis.go index faf574e0..e3a3968c 100644 --- a/redis/redis.go +++ b/redis/redis.go @@ -187,3 +187,27 @@ type SlowLog struct { // ClientName is the name set via the CLIENT SETNAME command (4.0 only). ClientName string } + +// Latency represents a redis LATENCY LATEST. +type Latency struct { + // Name of the latest latency spike event. + Name string + + // Time of the latest latency spike for the event. + Time time.Time + + // Latest is the latest recorded latency for the named event. + Latest time.Duration + + // Max is the maximum latency for the named event. + Max time.Duration +} + +// LatencyHistory represents a redis LATENCY HISTORY. +type LatencyHistory struct { + // Time is the unix timestamp at which the event was processed. + Time time.Time + + // ExecutationTime is the amount of time needed for the command execution. + ExecutionTime time.Duration +} diff --git a/redis/reply.go b/redis/reply.go index 7a2b3fef..aabf5989 100644 --- a/redis/reply.go +++ b/redis/reply.go @@ -645,3 +645,91 @@ func SlowLogs(result interface{}, err error) ([]SlowLog, error) { } return logs, nil } + +// Latencies is a helper that parses the LATENCY LATEST command output and +// return the slice of Latency values. +func Latencies(result interface{}, err error) ([]Latency, error) { + rawLatencies, err := Values(result, err) + if err != nil { + return nil, err + } + + latencies := make([]Latency, len(rawLatencies)) + for i, e := range rawLatencies { + rawLatency, ok := e.([]interface{}) + if !ok { + return nil, fmt.Errorf("redigo: latencies element is not slice, got %T", e) + } + + var event Latency + if len(rawLatency) != 4 { + return nil, fmt.Errorf("redigo: latencies element has %d elements, expected 4", len(rawLatency)) + } + + event.Name, err = String(rawLatency[0], nil) + if err != nil { + return nil, fmt.Errorf("redigo: latencies element[0] is not a string: %w", err) + } + + timestamp, ok := rawLatency[1].(int64) + if !ok { + return nil, fmt.Errorf("redigo: latencies element[1] not an int64, got %T", rawLatency[1]) + } + + event.Time = time.Unix(timestamp, 0) + + latestDuration, ok := rawLatency[2].(int64) + if !ok { + return nil, fmt.Errorf("redigo: latencies element[2] not an int64, got %T", rawLatency[2]) + } + + event.Latest = time.Duration(latestDuration) * time.Millisecond + + maxDuration, ok := rawLatency[3].(int64) + if !ok { + return nil, fmt.Errorf("redigo: latencies element[3] not an int64, got %T", rawLatency[3]) + } + + event.Max = time.Duration(maxDuration) * time.Millisecond + + latencies[i] = event + } + + return latencies, nil +} + +// LatencyHistories is a helper that parse the LATENCY HISTORY command output and +// returns a LatencyHistory slice. +func LatencyHistories(result interface{}, err error) ([]LatencyHistory, error) { + rawLogs, err := Values(result, err) + if err != nil { + return nil, err + } + + latencyHistories := make([]LatencyHistory, len(rawLogs)) + for i, e := range rawLogs { + rawLog, ok := e.([]interface{}) + if !ok { + return nil, fmt.Errorf("redigo: latency history element is not an slice, got %T", e) + } + + var event LatencyHistory + timestamp, ok := rawLog[0].(int64) + if !ok { + return nil, fmt.Errorf("redigo: latency history element[0] not an int64, got %T", rawLog[0]) + } + + event.Time = time.Unix(timestamp, 0) + + duration, ok := rawLog[1].(int64) + if !ok { + return nil, fmt.Errorf("redigo: latency history element[1] not an int64, got %T", rawLog[1]) + } + + event.ExecutionTime = time.Duration(duration) * time.Millisecond + + latencyHistories[i] = event + } + + return latencyHistories, nil +} diff --git a/redis/reply_test.go b/redis/reply_test.go index 8f43828c..7362a36c 100644 --- a/redis/reply_test.go +++ b/redis/reply_test.go @@ -16,6 +16,7 @@ package redis_test import ( "fmt" + "github.com/stretchr/testify/require" "math" "reflect" "strconv" @@ -225,6 +226,94 @@ func TestSlowLog(t *testing.T) { } } +func TestLatency(t *testing.T) { + c, err := dial() + require.NoError(t, err) + defer c.Close() + + resultStr, err := redis.Strings(c.Do("CONFIG", "GET", "latency-monitor-threshold")) + require.NoError(t, err) + // LATENCY commands were added in 2.8.13 so might not be supported. + if len(resultStr) == 0 { + t.Skip("Latency commands not supported") + } + latencyMonitorThresholdOldCfg, err := strconv.Atoi(resultStr[1]) + require.NoError(t, err) + // Enable latency monitoring for events that take 1ms or longer. + result, err := c.Do("CONFIG", "SET", "latency-monitor-threshold", "1") + // reset the old configuration after test. + defer func() { + res, err := c.Do("CONFIG", "SET", "latency-monitor-threshold", latencyMonitorThresholdOldCfg) + require.NoError(t, err) + require.Equal(t, "OK", res) + }() + + require.NoError(t, err) + require.Equal(t, "OK", result) + + // Sleep for 1ms to register a slow event. + _, err = c.Do("DEBUG", "SLEEP", 0.001) + require.NoError(t, err) + + result, err = c.Do("LATENCY", "LATEST") + require.NoError(t, err) + + latestLatencies, err := redis.Latencies(result, err) + require.NoError(t, err) + + require.Equal(t, 1, len(latestLatencies)) + + latencyEvent := latestLatencies[0] + expected := redis.Latency{ + Name: "command", + Latest: time.Millisecond, + Max: time.Millisecond, + Time: latencyEvent.Time, + } + require.Equal(t, latencyEvent, expected) +} + +func TestLatencyHistories(t *testing.T) { + c, err := dial() + require.NoError(t, err) + defer c.Close() + + res, err := redis.Strings(c.Do("CONFIG", "GET", "latency-monitor-threshold")) + require.NoError(t, err) + + // LATENCY commands were added in 2.8.13 so might not be supported. + if len(res) == 0 { + t.Skip("Latency commands not supported") + } + latencyMonitorThresholdOldCfg, err := strconv.Atoi(res[1]) + require.NoError(t, err) + + // Enable latency monitoring for events that take 1ms or longer + result, err := c.Do("CONFIG", "SET", "latency-monitor-threshold", "1") + // reset the old configuration after test. + defer func() { + res, err := c.Do("CONFIG", "SET", "latency-monitor-threshold", latencyMonitorThresholdOldCfg) + require.NoError(t, err) + require.Equal(t, "OK", res) + }() + require.NoError(t, err) + require.Equal(t, "OK", result) + + // Sleep for 1ms to register a slow event + _, err = c.Do("DEBUG", "SLEEP", 0.001) + require.NoError(t, err) + + result, err = c.Do("LATENCY", "HISTORY", "command") + require.NoError(t, err) + + latencyHistory, err := redis.LatencyHistories(result, err) + require.NoError(t, err) + + require.Len(t, latencyHistory, 1) + latencyEvent := latencyHistory[0] + require.Equal(t, time.Millisecond, latencyEvent.ExecutionTime) +} + // dial wraps DialDefaultServer() with a more suitable function name for examples. func dial() (redis.Conn, error) { return redis.DialDefaultServer() From 78e255f9bd2ae9c9885793a751f42f5698a5da8c Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Fri, 12 May 2023 00:11:01 +0100 Subject: [PATCH 25/36] fix: test goroutine leaks (#643) Ensure that goroutines started by tests are cleaned up on termination. Also: * make TestLatencyHistories compatible with -count=X. * Update to the supported versions of go 1.19 and 1.20. * Update golangci-lint to v1.15.2 Fixes #641 --- .github/workflows/go-test.yml | 4 ++-- .github/workflows/golangci-lint.yml | 2 +- redis/conn_test.go | 34 ++++++++++++++++++++++++++--- redis/reply_test.go | 6 ++++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 1086bf3a..5363fb31 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -12,8 +12,8 @@ jobs: strategy: matrix: go-version: - - '1.17.x' - - '1.18.x' + - '1.19.x' + - '1.20.x' os: - 'ubuntu-latest' redis: diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index eb7a22f8..177259b0 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -16,7 +16,7 @@ jobs: uses: golangci/golangci-lint-action@v2 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.46.2 + version: v1.51.2 # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/redis/conn_test.go b/redis/conn_test.go index b33cc41a..0c942ce2 100644 --- a/redis/conn_test.go +++ b/redis/conn_test.go @@ -23,6 +23,7 @@ import ( "io" "math" "net" + "sync" "os" "reflect" "strings" @@ -479,22 +480,49 @@ func TestError(t *testing.T) { } func TestReadTimeout(t *testing.T) { + done := make(chan struct{}) + errs := make(chan error, 2) + defer func() { + close(done) + for err := range errs { + require.NoError(t, err) + } + }() + + var wg sync.WaitGroup + defer func() { + wg.Wait() + close(errs) + }() + l, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("net.Listen returned %v", err) } defer l.Close() + wg.Add(1) go func() { + defer wg.Done() + for { c, err := l.Accept() if err != nil { return } + wg.Add(1) go func() { - time.Sleep(time.Second) + defer wg.Done() + + to := time.NewTimer(time.Second) + defer to.Stop() + select { + case <-to.C: + case <-done: + return + } _, err := c.Write([]byte("+OK\r\n")) - require.NoError(t, err) + errs <- err c.Close() }() } @@ -719,7 +747,7 @@ type blockedReader struct { func (b blockedReader) Read(p []byte) (n int, err error) { <-b.ch - return 0, nil + return 0, io.EOF } func dialTestBlockedConn(ch chan struct{}, w io.Writer) redis.DialOption { diff --git a/redis/reply_test.go b/redis/reply_test.go index 7362a36c..5ff3b5f0 100644 --- a/redis/reply_test.go +++ b/redis/reply_test.go @@ -16,7 +16,6 @@ package redis_test import ( "fmt" - "github.com/stretchr/testify/require" "math" "reflect" "strconv" @@ -24,6 +23,7 @@ import ( "time" "github.com/gomodule/redigo/redis" + "github.com/stretchr/testify/require" ) var ( @@ -288,6 +288,10 @@ func TestLatencyHistories(t *testing.T) { latencyMonitorThresholdOldCfg, err := strconv.Atoi(res[1]) require.NoError(t, err) + // Reset so we're compatible with -count=X + _, err = c.Do("LATENCY", "RESET", "command") + require.NoError(t, err) + // Enable latency monitoring for events that take 1ms or longer result, err := c.Do("CONFIG", "SET", "latency-monitor-threshold", "1") // reset the old configuration after test. From a60882bf9e77d7f428bdfa7e27516979d833f663 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Sun, 20 Aug 2023 12:34:35 +0100 Subject: [PATCH 26/36] chore: update testify (#653) Update testify to eliminate security issue in dependent package gopkg.in/yaml.v3. Fixes #652 --- go.mod | 2 +- go.sum | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 4efebf8d..ff1d95bf 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/gomodule/redigo go 1.16 -require github.com/stretchr/testify v1.7.0 +require github.com/stretchr/testify v1.8.4 retract ( v2.0.0+incompatible // Old development version not maintained or published. diff --git a/go.sum b/go.sum index acb88a48..479781e6 100644 --- a/go.sum +++ b/go.sum @@ -1,11 +1,17 @@ -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 91297458446615564ead7370d273480ccd2120f3 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Mon, 29 Jan 2024 19:33:58 +0000 Subject: [PATCH 27/36] core: update github actions and fix tests (#657) Update GitHub actions to use the latest versions and add go mod tidy check. This bumps go version for tests to 1.20 and 1.21 the currently supported versions. Update checks to validate against Redis 7.2 and 7.0, removing 5.0 and 4.0 which may still work but haven't been updated since 2020. Fix pubsub test instability due to unsubscribe notifications ordering not being guaranteed. Fix latency tests on Redis 7 due to DEBUG being disabled by default. --- .github/workflows/go-test.yml | 32 +++++-------------- .github/workflows/golangci-lint.yml | 31 ++++++++++--------- redis/pubsub_test.go | 48 ++++++++++++++--------------- redis/reply_test.go | 16 ++++++++-- 4 files changed, 61 insertions(+), 66 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 5363fb31..da05c811 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -12,15 +12,15 @@ jobs: strategy: matrix: go-version: - - '1.19.x' - - '1.20.x' + - '1.20' + - '1.21' os: - 'ubuntu-latest' redis: + - '7.2' + - '7.0' - '6.2' - '6.0' - - '5.0' - - '4.0' name: Test go ${{ matrix.go-version }} redis ${{ matrix.redis }} on ${{ matrix.os }} steps: - name: Setup redis @@ -29,30 +29,12 @@ jobs: redis-version: ${{ matrix.redis }} - name: Install Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} - - name: Go Cache Paths - id: go-cache-paths - run: | - echo "::set-output name=go-build::$(go env GOCACHE)" - echo "::set-output name=go-mod::$(go env GOMODCACHE)" - - name: Checkout code - uses: actions/checkout@v3 - - - name: Go Build Cache - uses: actions/cache@v3 - with: - path: ${{ steps.go-cache-paths.outputs.go-build }} - key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }} - - - name: Go Mod Cache - uses: actions/cache@v3 - with: - path: ${{ steps.go-cache-paths.outputs.go-mod }} - key: ${{ runner.os }}-go-mod-${{ hashFiles('**/go.sum') }} + uses: actions/checkout@v4 - name: Go Test - run: go test -v ./... + run: go test ./... diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 177259b0..ea1e4933 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -11,21 +11,22 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - name: golangci-lint - uses: golangci/golangci-lint-action@v2 - with: - # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.51.2 - - # Optional: working directory, useful for monorepos - # working-directory: somedir + - name: Checkout code + uses: actions/checkout@v4 - # Optional: golangci-lint command line arguments. - # args: --issues-exit-code=0 + - name: Setup golang + uses: actions/setup-go@v4 + with: + go-version: '1.21' + cache: false # Handled by golangci-lint. - # Optional: show only new issues if it's a pull request. The default value is `false`. - # only-new-issues: true + - name: Validate go mod + run: | + go mod tidy + git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]] - # Optional: if set to true then the action will use pre-installed Go. - # skip-go-installation: true + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: v1.55.2 + args: --out-format=colored-line-number diff --git a/redis/pubsub_test.go b/redis/pubsub_test.go index 63e08692..1119910c 100644 --- a/redis/pubsub_test.go +++ b/redis/pubsub_test.go @@ -17,7 +17,6 @@ package redis_test import ( "context" "errors" - "reflect" "testing" "time" @@ -25,50 +24,51 @@ import ( "github.com/stretchr/testify/require" ) -func expectPushed(t *testing.T, c redis.PubSubConn, message string, expected interface{}) { - actual := c.Receive() - if !reflect.DeepEqual(actual, expected) { - t.Errorf("%s = %v, want %v", message, actual, expected) - } -} - func TestPushed(t *testing.T) { pc, err := redis.DialDefaultServer() - if err != nil { - t.Fatalf("error connection to database, %v", err) - } + require.NoError(t, err) defer pc.Close() sc, err := redis.DialDefaultServer() - if err != nil { - t.Fatalf("error connection to database, %v", err) - } + require.NoError(t, err) defer sc.Close() c := redis.PubSubConn{Conn: sc} require.NoError(t, c.Subscribe("c1")) - expectPushed(t, c, "Subscribe(c1)", redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}) + require.Equal(t, redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}, c.Receive()) require.NoError(t, c.Subscribe("c2")) - expectPushed(t, c, "Subscribe(c2)", redis.Subscription{Kind: "subscribe", Channel: "c2", Count: 2}) + require.Equal(t, redis.Subscription{Kind: "subscribe", Channel: "c2", Count: 2}, c.Receive()) require.NoError(t, c.PSubscribe("p1")) - expectPushed(t, c, "PSubscribe(p1)", redis.Subscription{Kind: "psubscribe", Channel: "p1", Count: 3}) + require.Equal(t, redis.Subscription{Kind: "psubscribe", Channel: "p1", Count: 3}, c.Receive()) require.NoError(t, c.PSubscribe("p2")) - expectPushed(t, c, "PSubscribe(p2)", redis.Subscription{Kind: "psubscribe", Channel: "p2", Count: 4}) + require.Equal(t, redis.Subscription{Kind: "psubscribe", Channel: "p2", Count: 4}, c.Receive()) require.NoError(t, c.PUnsubscribe()) - expectPushed(t, c, "Punsubscribe(p1)", redis.Subscription{Kind: "punsubscribe", Channel: "p1", Count: 3}) - expectPushed(t, c, "Punsubscribe()", redis.Subscription{Kind: "punsubscribe", Channel: "p2", Count: 2}) + + // Response can return in any order. + v := c.Receive() + require.IsType(t, redis.Subscription{}, v) + u := v.(redis.Subscription) + expected1 := redis.Subscription{Kind: "punsubscribe", Channel: "p1", Count: 3} + expected2 := redis.Subscription{Kind: "punsubscribe", Channel: "p2", Count: 2} + if u.Channel == "p2" { + // Order reversed. + expected1.Channel = "p2" + expected2.Channel = "p1" + } + require.Equal(t, expected1, u) + require.Equal(t, expected2, c.Receive()) _, err = pc.Do("PUBLISH", "c1", "hello") require.NoError(t, err) - expectPushed(t, c, "PUBLISH c1 hello", redis.Message{Channel: "c1", Data: []byte("hello")}) + require.Equal(t, redis.Message{Channel: "c1", Data: []byte("hello")}, c.Receive()) require.NoError(t, c.Ping("hello")) - expectPushed(t, c, `Ping("hello")`, redis.Pong{Data: "hello"}) + require.Equal(t, redis.Pong{Data: "hello"}, c.Receive()) require.NoError(t, c.Conn.Send("PING")) c.Conn.Flush() - expectPushed(t, c, `Send("PING")`, redis.Pong{}) + require.Equal(t, redis.Pong{}, c.Receive()) require.NoError(t, c.Ping("timeout")) got := c.ReceiveWithTimeout(time.Minute) @@ -87,7 +87,7 @@ func TestPubSubReceiveContext(t *testing.T) { c := redis.PubSubConn{Conn: sc} require.NoError(t, c.Subscribe("c1")) - expectPushed(t, c, "Subscribe(c1)", redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}) + require.Equal(t, redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}, c.Receive()) ctx, cancel := context.WithCancel(context.Background()) cancel() diff --git a/redis/reply_test.go b/redis/reply_test.go index 5ff3b5f0..59bdba77 100644 --- a/redis/reply_test.go +++ b/redis/reply_test.go @@ -19,6 +19,7 @@ import ( "math" "reflect" "strconv" + "strings" "testing" "time" @@ -226,6 +227,17 @@ func TestSlowLog(t *testing.T) { } } +// debugCheck skips t if err indicates that DEBUG is not allowed, +// otherwise it fails if err is not nil. +func debugCheck(t *testing.T, err error) { + t.Helper() + + if err != nil && strings.Contains(err.Error(), "ERR DEBUG command not allowed") { + t.Skip("DEBUG command not allowed") + } + require.NoError(t, err) +} + func TestLatency(t *testing.T) { c, err := dial() require.NoError(t, err) @@ -253,7 +265,7 @@ func TestLatency(t *testing.T) { // Sleep for 1ms to register a slow event. _, err = c.Do("DEBUG", "SLEEP", 0.001) - require.NoError(t, err) + debugCheck(t, err) result, err = c.Do("LATENCY", "LATEST") require.NoError(t, err) @@ -305,7 +317,7 @@ func TestLatencyHistories(t *testing.T) { // Sleep for 1ms to register a slow event _, err = c.Do("DEBUG", "SLEEP", 0.001) - require.NoError(t, err) + debugCheck(t, err) result, err = c.Do("LATENCY", "HISTORY", "command") require.NoError(t, err) From 8b1c13e0376bc43a2443a0b312567718b4afbcd9 Mon Sep 17 00:00:00 2001 From: Arnaud Rebillout Date: Sat, 3 Feb 2024 22:50:47 +0700 Subject: [PATCH 28/36] chore: run test server with enable-debug-command (#654) Run the Redis test server with --enable-debug-command local on Redis >= 7.0 which disables MODULE and DEBUG commands by default for better security. Without this, some unit tests fail on later Redis versions. --- redis/test_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/redis/test_test.go b/redis/test_test.go index fcbb7bd1..11c34734 100644 --- a/redis/test_test.go +++ b/redis/test_test.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "os" "os/exec" + "regexp" "strconv" "strings" "sync" @@ -54,7 +55,51 @@ type Server struct { done chan struct{} } +type version struct { + major int + minor int + patch int +} + +func redisServerVersion() (*version, error) { + out, err := exec.Command("redis-server", "--version").Output() + if err != nil { + return nil, fmt.Errorf("server version: %w", err) + } + + ver := string(out) + re := regexp.MustCompile(`v=(\d+)\.(\d+)\.(\d+)`) + match := re.FindStringSubmatch(ver) + if len(match) != 4 { + return nil, fmt.Errorf("no server version found in %q", ver) + } + + var v version + if v.major, err = strconv.Atoi(match[1]); err != nil { + return nil, fmt.Errorf("invalid major version %q", match[1]) + } + + if v.minor, err = strconv.Atoi(match[2]); err != nil { + return nil, fmt.Errorf("invalid minor version %q", match[2]) + } + + if v.patch, err = strconv.Atoi(match[3]); err != nil { + return nil, fmt.Errorf("invalid patch version %q", match[3]) + } + + return &v, nil +} + func NewServer(name string, args ...string) (*Server, error) { + version, err := redisServerVersion() + if err != nil { + return nil, err + } + + if version.major >= 7 { + args = append(args, "--enable-debug-command", "local") + } + s := &Server{ name: name, cmd: exec.Command(*serverPath, args...), From 9f0d2e92e55d22b4c6d05bf8528ae90ed7fea855 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Sat, 3 Feb 2024 16:00:44 +0000 Subject: [PATCH 29/36] chore: remove debug check (#659) Remove debug check now we run the test server with debug enabled if needed. --- redis/reply_test.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/redis/reply_test.go b/redis/reply_test.go index 59bdba77..5ff3b5f0 100644 --- a/redis/reply_test.go +++ b/redis/reply_test.go @@ -19,7 +19,6 @@ import ( "math" "reflect" "strconv" - "strings" "testing" "time" @@ -227,17 +226,6 @@ func TestSlowLog(t *testing.T) { } } -// debugCheck skips t if err indicates that DEBUG is not allowed, -// otherwise it fails if err is not nil. -func debugCheck(t *testing.T, err error) { - t.Helper() - - if err != nil && strings.Contains(err.Error(), "ERR DEBUG command not allowed") { - t.Skip("DEBUG command not allowed") - } - require.NoError(t, err) -} - func TestLatency(t *testing.T) { c, err := dial() require.NoError(t, err) @@ -265,7 +253,7 @@ func TestLatency(t *testing.T) { // Sleep for 1ms to register a slow event. _, err = c.Do("DEBUG", "SLEEP", 0.001) - debugCheck(t, err) + require.NoError(t, err) result, err = c.Do("LATENCY", "LATEST") require.NoError(t, err) @@ -317,7 +305,7 @@ func TestLatencyHistories(t *testing.T) { // Sleep for 1ms to register a slow event _, err = c.Do("DEBUG", "SLEEP", 0.001) - debugCheck(t, err) + require.NoError(t, err) result, err = c.Do("LATENCY", "HISTORY", "command") require.NoError(t, err) From 1d393b301b27d4934d4b76ff4df60859e296cdec Mon Sep 17 00:00:00 2001 From: Vasily Tsybenko Date: Tue, 20 Feb 2024 23:47:21 +0300 Subject: [PATCH 30/36] feat: add TestOnBorrowContext (#660) Add TestOnBorrowContext to the Pool struct for checking the health of the idle connection with a given context. --- redis/pool.go | 15 +++- redis/pool_test.go | 189 ++++++++++++++++++++++++++++++++++++++++----- redis/test_test.go | 11 ++- 3 files changed, 191 insertions(+), 24 deletions(-) diff --git a/redis/pool.go b/redis/pool.go index d7bb71e0..8e22f66b 100644 --- a/redis/pool.go +++ b/redis/pool.go @@ -128,16 +128,24 @@ type Pool struct { // DialContext is an application supplied function for creating and configuring a // connection with the given context. // - // The connection returned from Dial must not be in a special state + // The connection returned from DialContext must not be in a special state // (subscribed to pubsub channel, transaction started, ...). DialContext func(ctx context.Context) (Conn, error) // TestOnBorrow is an optional application supplied function for checking // the health of an idle connection before the connection is used again by - // the application. Argument t is the time that the connection was returned + // the application. Argument lastUsed is the time when the connection was returned + // to the pool. If the function returns an error, then the connection is + // closed. + TestOnBorrow func(c Conn, lastUsed time.Time) error + + // TestOnBorrowContext is an optional application supplied function + // for checking the health of an idle connection with the given context + // before the connection is used again by the application. + // Argument lastUsed is the time when the connection was returned // to the pool. If the function returns an error, then the connection is // closed. - TestOnBorrow func(c Conn, t time.Time) error + TestOnBorrowContext func(ctx context.Context, c Conn, lastUsed time.Time) error // Maximum number of idle connections in the pool. MaxIdle int @@ -228,6 +236,7 @@ func (p *Pool) GetContext(ctx context.Context) (Conn, error) { p.idle.popFront() p.mu.Unlock() if (p.TestOnBorrow == nil || p.TestOnBorrow(pc.c, pc.t) == nil) && + (p.TestOnBorrowContext == nil || p.TestOnBorrowContext(ctx, pc.c, pc.t) == nil) && (p.MaxConnLifetime == 0 || nowFunc().Sub(pc.created) < p.MaxConnLifetime) { return &activeConn{p: p, pc: pc}, nil } diff --git a/redis/pool_test.go b/redis/pool_test.go index f420b124..2748d977 100644 --- a/redis/pool_test.go +++ b/redis/pool_test.go @@ -48,6 +48,25 @@ func (c *poolTestConn) Close() error { func (c *poolTestConn) Err() error { return c.err } func (c *poolTestConn) Do(commandName string, args ...interface{}) (interface{}, error) { + return c.do(c.Conn.Do, commandName, args...) +} + +func (c *poolTestConn) DoContext(ctx context.Context, commandName string, args ...interface{}) (interface{}, error) { + cwc, ok := c.Conn.(redis.ConnWithContext) + if !ok { + return nil, errors.New("redis: connection does not support ConnWithContext") + } + return c.do( + func(c string, a ...interface{}) (interface{}, error) { + return cwc.DoContext(ctx, c, a...) + }, + commandName, args) +} + +func (c *poolTestConn) do( + fn func(commandName string, args ...interface{}) (interface{}, error), + commandName string, args ...interface{}, +) (interface{}, error) { if commandName == "ERR" { c.err = args[0].(error) commandName = "PING" @@ -55,7 +74,7 @@ func (c *poolTestConn) Do(commandName string, args ...interface{}) (interface{}, if commandName != "" { c.d.commands = append(c.d.commands, commandName) } - return c.Conn.Do(commandName, args...) + return fn(commandName, args...) } func (c *poolTestConn) Send(commandName string, args ...interface{}) error { @@ -63,6 +82,14 @@ func (c *poolTestConn) Send(commandName string, args ...interface{}) error { return c.Conn.Send(commandName, args...) } +func (c *poolTestConn) ReceiveContext(ctx context.Context) (reply interface{}, err error) { + cwc, ok := c.Conn.(redis.ConnWithContext) + if !ok { + return nil, errors.New("redis: connection does not support ConnWithContext") + } + return cwc.ReceiveContext(ctx) +} + type poolDialer struct { mu sync.Mutex t *testing.T @@ -73,6 +100,10 @@ type poolDialer struct { } func (d *poolDialer) dial() (redis.Conn, error) { + return d.dialContext(context.Background()) +} + +func (d *poolDialer) dialContext(ctx context.Context) (redis.Conn, error) { d.mu.Lock() d.dialed += 1 dialErr := d.dialErr @@ -80,7 +111,7 @@ func (d *poolDialer) dial() (redis.Conn, error) { if dialErr != nil { return nil, d.dialErr } - c, err := redis.DialDefaultServer() + c, err := redis.DialDefaultServerContext(ctx) if err != nil { return nil, err } @@ -90,15 +121,14 @@ func (d *poolDialer) dial() (redis.Conn, error) { return &poolTestConn{d: d, Conn: c}, nil } -func (d *poolDialer) dialContext(ctx context.Context) (redis.Conn, error) { - return d.dial() -} - func (d *poolDialer) check(message string, p *redis.Pool, dialed, open, inuse int) { + d.t.Helper() d.checkAll(message, p, dialed, open, inuse, 0, 0) } func (d *poolDialer) checkAll(message string, p *redis.Pool, dialed, open, inuse int, waitCountMax int64, waitDurationMax time.Duration) { + d.t.Helper() + d.mu.Lock() defer d.mu.Unlock() @@ -368,21 +398,142 @@ func TestPoolConcurrenSendReceive(t *testing.T) { } func TestPoolBorrowCheck(t *testing.T) { - d := poolDialer{t: t} - p := &redis.Pool{ - MaxIdle: 2, - Dial: d.dial, - TestOnBorrow: func(redis.Conn, time.Time) error { return redis.Error("BLAH") }, + pingN := func(ctx context.Context, p *redis.Pool, n int) { + for i := 0; i < n; i++ { + func() { + c, err := p.GetContext(ctx) + require.NoError(t, err) + defer func() { + require.NoError(t, c.Close()) + }() + _, err = redis.DoContext(c, ctx, "PING") + require.NoError(t, err) + }() + } } - defer p.Close() - for i := 0; i < 10; i++ { - c := p.Get() - _, err := c.Do("PING") - require.NoError(t, err) - c.Close() + checkLastUsedTimes := func(lastUsedTimes []time.Time, startTime, endTime time.Time, wantLen int) { + require.Len(t, lastUsedTimes, wantLen) + for i, lastUsed := range lastUsedTimes { + if i == 0 { + require.True(t, lastUsed.After(startTime)) + } else { + require.True(t, lastUsed.After(lastUsedTimes[i-1])) + } + require.True(t, lastUsed.Before(endTime)) + } } - d.check("1", p, 10, 1, 0) + + t.Run("TestOnBorrow-error", func(t *testing.T) { + d := poolDialer{t: t} + p := &redis.Pool{ + MaxIdle: 2, + DialContext: d.dialContext, + TestOnBorrow: func(redis.Conn, time.Time) error { return redis.Error("BLAH") }, + } + defer p.Close() + pingN(context.Background(), p, 10) + d.check("1", p, 10, 1, 0) + }) + + t.Run("TestOnBorrow-nil-error", func(t *testing.T) { + d := poolDialer{t: t} + var borrowErrs []error + var lastUsedTimes []time.Time + p := &redis.Pool{ + MaxIdle: 2, + DialContext: d.dialContext, + TestOnBorrow: func(c redis.Conn, lastUsed time.Time) error { + lastUsedTimes = append(lastUsedTimes, lastUsed) + _, err := c.Do("PING") + if err != nil { + borrowErrs = append(borrowErrs, err) + } + return err + }, + } + defer p.Close() + + startTime := time.Now() + pingN(context.Background(), p, 10) + endTime := time.Now() + + require.Empty(t, borrowErrs) + checkLastUsedTimes(lastUsedTimes, startTime, endTime, 9) + d.check("1", p, 1, 1, 0) + }) + + t.Run("TestOnBorrowContext-error", func(t *testing.T) { + d := poolDialer{t: t} + p := &redis.Pool{ + MaxIdle: 2, + DialContext: d.dialContext, + TestOnBorrowContext: func(context.Context, redis.Conn, time.Time) error { return redis.Error("BLAH") }, + } + defer p.Close() + pingN(context.Background(), p, 10) + d.check("1", p, 10, 1, 0) + }) + + t.Run("TestOnBorrowContext-nil-error", func(t *testing.T) { + d := poolDialer{t: t} + var borrowErrs []error + var lastUsedTimes []time.Time + p := &redis.Pool{ + MaxIdle: 2, + DialContext: d.dialContext, + TestOnBorrowContext: func(ctx context.Context, c redis.Conn, lastUsed time.Time) error { + lastUsedTimes = append(lastUsedTimes, lastUsed) + _, err := redis.DoContext(c, ctx, "PING") + if err != nil { + borrowErrs = append(borrowErrs, err) + } + return err + }, + } + defer p.Close() + + startTime := time.Now() + pingN(context.Background(), p, 10) + endTime := time.Now() + + require.Empty(t, borrowErrs) + checkLastUsedTimes(lastUsedTimes, startTime, endTime, 9) + d.check("1", p, 1, 1, 0) + }) + + t.Run("TestOnBorrowContext-context.Canceled", func(t *testing.T) { + d := poolDialer{t: t} + var borrowErrs []error + p := &redis.Pool{ + MaxIdle: 2, + DialContext: d.dialContext, + TestOnBorrowContext: func(ctx context.Context, c redis.Conn, _ time.Time) error { + _, err := redis.DoContext(c, ctx, "PING") + if err != nil { + borrowErrs = append(borrowErrs, err) + } + return err + }, + } + defer p.Close() + + ctx, ctxCancel := context.WithCancel(context.Background()) + defer ctxCancel() + + pingN(ctx, p, 2) + d.check("1", p, 1, 1, 0) + require.Empty(t, borrowErrs) + + ctxCancel() + + _, err := p.GetContext(ctx) + require.ErrorIs(t, err, context.Canceled) + + d.check("1", p, 2, 0, 0) + require.Len(t, borrowErrs, 1) + require.ErrorIs(t, borrowErrs[0], context.Canceled) + }) } func TestPoolMaxActive(t *testing.T) { @@ -757,7 +908,7 @@ func TestLocking_TestOnBorrowFails_PoolDoesntCrash(t *testing.T) { MaxIdle: count, MaxActive: count, Dial: d.dial, - TestOnBorrow: func(c redis.Conn, t time.Time) error { + TestOnBorrow: func(redis.Conn, time.Time) error { return errors.New("No way back into the real world.") }, } diff --git a/redis/test_test.go b/redis/test_test.go index 11c34734..f7598683 100644 --- a/redis/test_test.go +++ b/redis/test_test.go @@ -16,6 +16,7 @@ package redis import ( "bufio" + "context" "errors" "flag" "fmt" @@ -197,15 +198,21 @@ func DefaultServerAddr() (string, error) { // DialDefaultServer starts the test server if not already started and dials a // connection to the server. func DialDefaultServer(options ...DialOption) (Conn, error) { + return DialDefaultServerContext(context.Background(), options...) +} + +// DialDefaultServerContext starts the test server if not already started and +// dials a connection to the server with the given context. +func DialDefaultServerContext(ctx context.Context, options ...DialOption) (Conn, error) { addr, err := DefaultServerAddr() if err != nil { return nil, err } - c, err := Dial("tcp", addr, append([]DialOption{DialReadTimeout(1 * time.Second), DialWriteTimeout(1 * time.Second)}, options...)...) + c, err := DialContext(ctx, "tcp", addr, append([]DialOption{DialReadTimeout(1 * time.Second), DialWriteTimeout(1 * time.Second)}, options...)...) if err != nil { return nil, err } - if _, err = c.Do("FLUSHDB"); err != nil { + if _, err = DoContext(c, ctx, "FLUSHDB"); err != nil { return nil, err } return c, nil From 1bfd3c16b111e999b1ba506ad0613c8322a97848 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Tue, 20 Feb 2024 21:21:32 +0000 Subject: [PATCH 31/36] ci: add goreleaser, update action and go versions (#662) Add goreleaser to automate the generation of release notes and bump the versions of github actions golang go the latest versions. --- .github/workflows/go-test.yml | 4 +-- .github/workflows/golangci-lint.yml | 6 ++-- .github/workflows/release-build.yml | 32 ++++++++++++++++++++ .goreleaser.yaml | 46 +++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/release-build.yml create mode 100644 .goreleaser.yaml diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index da05c811..884d3a14 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -12,8 +12,8 @@ jobs: strategy: matrix: go-version: - - '1.20' - '1.21' + - '1.22' os: - 'ubuntu-latest' redis: @@ -37,4 +37,4 @@ jobs: uses: actions/checkout@v4 - name: Go Test - run: go test ./... + run: go test -race ./... diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index ea1e4933..02bf1393 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -17,7 +17,7 @@ jobs: - name: Setup golang uses: actions/setup-go@v4 with: - go-version: '1.21' + go-version: '1.22' cache: false # Handled by golangci-lint. - name: Validate go mod @@ -26,7 +26,7 @@ jobs: git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]] - name: golangci-lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v4 with: - version: v1.55.2 + version: v1.56.2 args: --out-format=colored-line-number diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml new file mode 100644 index 00000000..64963238 --- /dev/null +++ b/.github/workflows/release-build.yml @@ -0,0 +1,32 @@ +name: Build Release + +on: + push: + tags: + - 'v[0-9]+.[0-9]+.[0-9]+*' + +permissions: + contents: write + +jobs: + goreleaser: + name: Release Go Binary + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.22' + + - name: Run GoReleaser + uses: goreleaser/goreleaser-action@v5 + with: + distribution: goreleaser + version: latest + args: release --rm-dist + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.goreleaser.yaml b/.goreleaser.yaml new file mode 100644 index 00000000..899eb96b --- /dev/null +++ b/.goreleaser.yaml @@ -0,0 +1,46 @@ +# When adding options check the documentation at https://goreleaser.com +builds: + - skip: true +release: + header: | + + ### {{.Tag}} Release Notes ({{.Date}}) + footer: | + [Full Changelog](https://{{ .ModulePath }}/compare/{{ .PreviousTag }}...{{ .Tag }}) +changelog: + use: github + sort: asc + filters: + exclude: + - Merge pull request + - Merge remote-tracking branch + - Merge branch + + # Group commits messages by given regex and title. + # Order value defines the order of the groups. + # Proving no regex means all commits will be grouped under the default group. + # Groups are disabled when using github-native, as it already groups things by itself. + # Matches are performed against strings of the form: "[:] ". + # + # Default is no groups. + groups: + - title: Features + regexp: '^.*?(feat|feature)(\([[:word:]]+\))??!?:.+$' + order: 0 + - title: 'Bug fixes' + regexp: '^.*?fix(\([[:word:]]+\))??!?:.+$' + order: 1 + - title: 'Chores' + regexp: '^.*?chore(\([[:word:]]+\))??!?:.+$' + order: 2 + - title: 'Quality' + regexp: '^.*?(qa|test|tests)(\([[:word:]]+\))??!?:.+$' + order: 3 + - title: 'Documentation' + regexp: '^.*?(doc|docs)(\([[:word:]]+\))??!?:.+$' + order: 4 + - title: 'Continuous Integration' + regexp: '^.*?ci(\([[:word:]]+\))??!?:.+$' + order: 5 + - title: Other + order: 999 From cfabb1f017433cdf852a4841a8c0125c48275c0b Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Tue, 20 Feb 2024 21:35:21 +0000 Subject: [PATCH 32/36] ci: remove deprecated goreleaser option (#663) Remove deprecated goreleaser option --rm-dist and use --clean instead to prevent deprecation warning. --- .github/workflows/release-build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml index 64963238..2882b299 100644 --- a/.github/workflows/release-build.yml +++ b/.github/workflows/release-build.yml @@ -27,6 +27,6 @@ jobs: with: distribution: goreleaser version: latest - args: release --rm-dist + args: release --clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From 162ed022a35991830c265f24896b07d6537b3702 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Tue, 20 Feb 2024 23:50:35 +0000 Subject: [PATCH 33/36] chore: retract v1.8.10 (#664) Retract v1.8.10 which was tagged incorrectly for a feature release and only available for a few minutes. v1.9.0 is identical. --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index ff1d95bf..ea9821d8 100644 --- a/go.mod +++ b/go.mod @@ -6,5 +6,6 @@ require github.com/stretchr/testify v1.8.4 retract ( v2.0.0+incompatible // Old development version not maintained or published. + v1.8.10 // Incorrect version tag for feature. v0.0.0-do-not-use // Never used only present due to lack of retract. ) From e05a63bfd93c1db480ee706c1686d9dd2385016b Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Sun, 25 Feb 2024 12:46:42 +0000 Subject: [PATCH 34/36] fix: bump go version to 1.17 (#666) Due to our test dependency requiring 1.17 minimum and the 1.16 being well out of support, currently only 1.21 and 1.22 are support releases, bump our required go version to 1.16. Fixes #665 --- go.mod | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index ea9821d8..573acf9d 100644 --- a/go.mod +++ b/go.mod @@ -1,9 +1,15 @@ module github.com/gomodule/redigo -go 1.16 +go 1.17 require github.com/stretchr/testify v1.8.4 +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) + retract ( v2.0.0+incompatible // Old development version not maintained or published. v1.8.10 // Incorrect version tag for feature. From 4c535aa56d60a1dddd457a8e63caa463bcb5a70b Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Sun, 25 Feb 2024 12:53:24 +0000 Subject: [PATCH 35/36] ci: fix go caching (#667) Move checkout before go-setup so caching works. --- .github/workflows/go-test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 884d3a14..e2bfff07 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -28,13 +28,13 @@ jobs: with: redis-version: ${{ matrix.redis }} + - name: Checkout code + uses: actions/checkout@v4 + - name: Install Go uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} - - name: Checkout code - uses: actions/checkout@v4 - - name: Go Test run: go test -race ./... From 247f6c0e0a0ea200f727a5280d0d55f6bce6d2e7 Mon Sep 17 00:00:00 2001 From: rustfix <155627174+rustfix@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:23:52 +0800 Subject: [PATCH 36/36] chore: fix function name in comment (#668) Signed-off-by: rustfix <771054535@qq.com> --- redis/conn_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/conn_test.go b/redis/conn_test.go index 0c942ce2..b94f0f42 100644 --- a/redis/conn_test.go +++ b/redis/conn_test.go @@ -887,7 +887,7 @@ func ExampleDialURL() { defer c.Close() } -// TextExecError tests handling of errors in a transaction. See +// TestExecError tests handling of errors in a transaction. See // http://redis.io/topics/transactions for information on how Redis handles // errors in a transaction. func TestExecError(t *testing.T) {