Skip to content

Commit

Permalink
Merge pull request #313 from redis/client-setinfo-default
Browse files Browse the repository at this point in the history
feat: always using Redis 7.2 CLIENT SETINFO command to set library name
  • Loading branch information
rueian committed Jul 28, 2023
2 parents 9e1f9f0 + c5261ee commit 3cdbe5a
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
2 changes: 2 additions & 0 deletions mux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ func TestNewMux(t *testing.T) {
})
mock.Expect("CLIENT", "TRACKING", "ON", "OPTIN").
ReplyString("OK")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("QUIT").ReplyString("OK")
mock.Close()
}()
Expand Down
16 changes: 11 additions & 5 deletions pipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import (
"github.com/redis/rueidis/internal/util"
)

const LIB_NAME = "rueidis"
const LIB_VER = "1.0.14"

var noHello = regexp.MustCompile("unknown command .?(HELLO|hello).?")

type wire interface {
Expand Down Expand Up @@ -158,7 +161,7 @@ func _newPipe(connFn func() (net.Conn, error), option *ClientOption, r2ps bool)
helloCmd = append(helloCmd, "SETNAME", option.ClientName)
}

init := make([][]string, 0, 3)
init := make([][]string, 0, 4)
if option.ClientTrackingOptions == nil {
init = append(init, helloCmd, []string{"CLIENT", "TRACKING", "ON", "OPTIN"})
} else {
Expand All @@ -178,6 +181,8 @@ func _newPipe(connFn func() (net.Conn, error), option *ClientOption, r2ps bool)
}
if option.ClientSetInfo != nil {
init = append(init, append([]string{"CLIENT", "SETINFO"}, option.ClientSetInfo...))
} else {
init = append(init, []string{"CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER})
}

timeout := option.Dialer.Timeout
Expand All @@ -192,7 +197,7 @@ func _newPipe(connFn func() (net.Conn, error), option *ClientOption, r2ps bool)
if !r2 && !r2ps {
resp := p.DoMulti(ctx, cmds.NewMultiCompleted(init)...)
defer resultsp.Put(resp)
for i, r := range resp.s {
for i, r := range resp.s[:len(resp.s)-1] { // skip error checking on the last CLIENT SETINFO
if i == 0 {
p.info, err = r.AsMap()
} else {
Expand Down Expand Up @@ -250,19 +255,20 @@ func _newPipe(connFn func() (net.Conn, error), option *ClientOption, r2ps bool)
}
if option.ClientSetInfo != nil {
init = append(init, append([]string{"CLIENT", "SETINFO"}, option.ClientSetInfo...))
} else {
init = append(init, []string{"CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER})
}

p.version = 5
if len(init) != 0 {
resp := p.DoMulti(ctx, cmds.NewMultiCompleted(init)...)
defer resultsp.Put(resp)
for _, r := range resp.s {
for _, r := range resp.s[:len(resp.s)-1] { // skip error checking on the last CLIENT SETINFO
if err = r.Error(); err != nil {
p.Close()
return nil, err
}
}
}
p.version = 5
}
if p.onInvalidations != nil || option.AlwaysPipelining {
p.background()
Expand Down
22 changes: 22 additions & 0 deletions pipe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ func setup(t *testing.T, option ClientOption) (*pipe, *redisMock, func(), func()
mock.Expect("CLIENT", "TRACKING", "ON", "OPTIN").
ReplyString("OK")
}
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
}()
p, err := newPipe(func() (net.Conn, error) { return n1, nil }, &option)
if err != nil {
Expand Down Expand Up @@ -271,6 +273,8 @@ func TestNewPipe(t *testing.T) {
ReplyString("OK")
mock.Expect("SELECT", "1").
ReplyString("OK")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
}()
p, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{
SelectDB: 1,
Expand Down Expand Up @@ -301,6 +305,8 @@ func TestNewPipe(t *testing.T) {
})
mock.Expect("CLIENT", "TRACKING", "ON", "OPTIN", "NOLOOP").
ReplyString("OK")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
}()
p, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{
ClientTrackingOptions: []string{"OPTIN", "NOLOOP"},
Expand Down Expand Up @@ -334,6 +340,8 @@ func TestNewRESP2Pipe(t *testing.T) {
ReplyError("ERR unknown command `HELLO`")
mock.Expect("CLIENT", "TRACKING", "ON", "OPTIN").
ReplyError("ERR unknown subcommand or wrong number of arguments for 'TRACKING'. Try CLIENT HELP")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("QUIT").ReplyString("OK")
}()
if _, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{}); !errors.Is(err, ErrNoCache) {
Expand All @@ -351,6 +359,8 @@ func TestNewRESP2Pipe(t *testing.T) {
ReplyError("ERR unknown command `HELLO`")
mock.Expect("CLIENT", "TRACKING", "ON", "OPTIN").
ReplyString("OK")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("QUIT").ReplyString("OK")
}()
if _, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{}); !errors.Is(err, ErrNoCache) {
Expand All @@ -371,6 +381,8 @@ func TestNewRESP2Pipe(t *testing.T) {
{typ: '+', string: "proto"},
{typ: ':', integer: 2},
}})
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
}()
p, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{
DisableCache: true,
Expand All @@ -395,12 +407,16 @@ func TestNewRESP2Pipe(t *testing.T) {
ReplyError("ERR unknown command `HELLO`")
mock.Expect("SELECT", "1").
ReplyError("ERR ACL")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("AUTH", "pa").
ReplyString("OK")
mock.Expect("CLIENT", "SETNAME", "cn").
ReplyString("OK")
mock.Expect("SELECT", "1").
ReplyString("OK")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
}()
p, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{
SelectDB: 1,
Expand Down Expand Up @@ -428,12 +444,16 @@ func TestNewRESP2Pipe(t *testing.T) {
ReplyError("ERR unknown command `HELLO`")
mock.Expect("SELECT", "1").
ReplyError("ERR ACL")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("AUTH", "ua", "pa").
ReplyString("OK")
mock.Expect("CLIENT", "SETNAME", "cn").
ReplyString("OK")
mock.Expect("SELECT", "1").
ReplyString("OK")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
}()
p, err := newPipe(func() (net.Conn, error) { return n1, nil }, &ClientOption{
SelectDB: 1,
Expand Down Expand Up @@ -462,6 +482,8 @@ func TestNewRESP2Pipe(t *testing.T) {
ReplyError("ERR unknown command `HELLO`")
mock.Expect("SELECT", "1").
ReplyError("ERR ACL")
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
n1.Close()
n2.Close()
}()
Expand Down
12 changes: 11 additions & 1 deletion rueidis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ func TestNewClusterClient(t *testing.T) {
return
}
slots, _ := slotsResp.ToMessage()
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("CLUSTER", "SLOTS").Reply(slots)
mock.Close()
close(done)
Expand Down Expand Up @@ -108,6 +110,8 @@ func TestNewClusterClientError(t *testing.T) {
if err != nil {
return
}
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("CLUSTER", "SLOTS").Reply(RedisMessage{typ: '-', string: "other error"})
mock.Expect("QUIT").ReplyString("OK")
mock.Close()
Expand Down Expand Up @@ -136,6 +140,8 @@ func TestFallBackSingleClient(t *testing.T) {
if err != nil {
return
}
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("CLUSTER", "SLOTS").Reply(RedisMessage{typ: '-', string: "ERR This instance has cluster support disabled"})
mock.Expect("QUIT").ReplyString("OK")
mock.Close()
Expand Down Expand Up @@ -169,13 +175,15 @@ func TestForceSingleClient(t *testing.T) {
if err != nil {
return
}
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("QUIT").ReplyString("OK")
mock.Close()
close(done)
}()
_, port, _ := net.SplitHostPort(ln.Addr().String())
client, err := NewClient(ClientOption{
InitAddress: []string{"127.0.0.1:" + port},
InitAddress: []string{"127.0.0.1:" + port},
ForceSingleClient: true,
})
if err != nil {
Expand Down Expand Up @@ -247,6 +255,8 @@ func TestTLSClient(t *testing.T) {
if err != nil {
return
}
mock.Expect("CLIENT", "SETINFO", "LIB-NAME", LIB_NAME, "LIB-VER", LIB_VER).
ReplyError("UNKNOWN COMMAND")
mock.Expect("CLUSTER", "SLOTS").Reply(RedisMessage{typ: '-', string: "ERR This instance has cluster support disabled"})
mock.Expect("QUIT").ReplyString("OK")
mock.Close()
Expand Down

0 comments on commit 3cdbe5a

Please sign in to comment.