diff --git a/internal/server/client.go b/internal/server/client.go index 6af0a108..ab23550f 100644 --- a/internal/server/client.go +++ b/internal/server/client.go @@ -52,7 +52,8 @@ func (arr byID) Swap(a, b int) { arr[a], arr[b] = arr[b], arr[a] } -func (s *Server) cmdClient(msg *Message, client *Client) (resp.Value, error) { +// CLIENT (LIST | KILL | GETNAME | SETNAME) +func (s *Server) cmdCLIENT(msg *Message, client *Client) (resp.Value, error) { start := time.Now() if len(msg.Args) == 1 { @@ -89,8 +90,7 @@ func (s *Server) cmdClient(msg *Message, client *Client) (resp.Value, error) { ) client.mu.Unlock() } - switch msg.OutputType { - case JSON: + if msg.OutputType == JSON { // Create a map of all key/value info fields var cmap []map[string]interface{} clients := strings.Split(string(buf), "\n") @@ -110,32 +110,23 @@ func (s *Server) cmdClient(msg *Message, client *Client) (resp.Value, error) { } } - // Marshal the map and use the output in the JSON response - data, err := json.Marshal(cmap) - if err != nil { - return NOMessage, err - } - return resp.StringValue(`{"ok":true,"list":` + string(data) + `,"elapsed":"` + time.Since(start).String() + "\"}"), nil - case RESP: - return resp.BytesValue(buf), nil + data, _ := json.Marshal(cmap) + return resp.StringValue(`{"ok":true,"list":` + string(data) + + `,"elapsed":"` + time.Since(start).String() + "\"}"), nil } - return NOMessage, nil + return resp.BytesValue(buf), nil case "getname": if len(msg.Args) != 2 { return NOMessage, errInvalidNumberOfArguments } - name := "" - switch msg.OutputType { - case JSON: - client.mu.Lock() - name := client.name - client.mu.Unlock() - return resp.StringValue(`{"ok":true,"name":` + - jsonString(name) + + client.mu.Lock() + name := client.name + client.mu.Unlock() + if msg.OutputType == JSON { + return resp.StringValue(`{"ok":true,"name":` + jsonString(name) + `,"elapsed":"` + time.Since(start).String() + "\"}"), nil - case RESP: - return resp.StringValue(name), nil } + return resp.StringValue(name), nil case "setname": if len(msg.Args) != 3 { return NOMessage, errInvalidNumberOfArguments diff --git a/internal/server/server.go b/internal/server/server.go index d7c6a399..4365dab4 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -977,7 +977,7 @@ func (s *Server) handleInputCommand(client *Client, msg *Message) error { return err } } - if !isRespValueEmptyString(res) { + if false || !isRespValueEmptyString(res) { var resStr string resStr, err := serializeOutput(res) if err != nil { @@ -1140,7 +1140,7 @@ func (s *Server) command(msg *Message, client *Client) ( return s.command(msg, client) } case "client": - res, err = s.cmdClient(msg, client) + res, err = s.cmdCLIENT(msg, client) case "eval", "evalro", "evalna": res, err = s.cmdEvalUnified(false, msg) case "evalsha", "evalrosha", "evalnasha": @@ -1162,6 +1162,7 @@ func (s *Server) command(msg *Message, client *Client) ( case "monitor": res, err = s.cmdMonitor(msg) } + s.sendMonitor(err, msg, client, false) return } diff --git a/tests/client_test.go b/tests/client_test.go index 016b88aa..941ce6e7 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -3,6 +3,7 @@ package tests import ( "errors" "fmt" + "strings" "testing" "github.com/gomodule/redigo/redis" @@ -10,18 +11,19 @@ import ( ) func subTestClient(t *testing.T, mc *mockServer) { - runStep(t, mc, "valid json", client_valid_json_test) - runStep(t, mc, "valid client count", info_valid_client_count_test) + runStep(t, mc, "OUTPUT", client_OUTPUT_test) + runStep(t, mc, "CLIENT", client_CLIENT_test) } -func client_valid_json_test(mc *mockServer) error { - if err := mc.DoBatch([][]interface{}{ +func client_OUTPUT_test(mc *mockServer) error { + if err := mc.DoBatch( // tests removal of "elapsed" member. - {"OUTPUT", "json"}, {`{"ok":true}`}, - {"OUTPUT", "resp"}, {`OK`}, - }); err != nil { + Do("OUTPUT", "json").Str(`{"ok":true}`), + Do("OUTPUT", "resp").OK(), + ); err != nil { return err } + // run direct commands if _, err := mc.Do("OUTPUT", "json"); err != nil { return err @@ -45,9 +47,14 @@ func client_valid_json_test(mc *mockServer) error { return nil } -func info_valid_client_count_test(mc *mockServer) error { +func client_CLIENT_test(mc *mockServer) error { numConns := 20 var conns []redis.Conn + defer func() { + for i := range conns { + conns[i].Close() + } + }() for i := 0; i <= numConns; i++ { conn, err := redis.Dial("tcp", fmt.Sprintf(":%d", mc.port)) if err != nil { @@ -55,9 +62,6 @@ func info_valid_client_count_test(mc *mockServer) error { } conns = append(conns, conn) } - for i := range conns { - defer conns[i].Close() - } if _, err := mc.Do("OUTPUT", "JSON"); err != nil { return err } @@ -73,5 +77,26 @@ func info_valid_client_count_test(mc *mockServer) error { if len(gjson.Get(sres, "list").Array()) < numConns { return errors.New("Invalid number of connections") } - return nil + + return mc.DoBatch( + Do("CLIENT", "list").JSON().Func(func(s string) error { + if int(gjson.Get(s, "list.#").Int()) < numConns { + return errors.New("Invalid number of connections") + } + return nil + }), + Do("CLIENT", "list").Func(func(s string) error { + if len(strings.Split(strings.TrimSpace(s), "\n")) < numConns { + return errors.New("Invalid number of connections") + } + return nil + }), + Do("CLIENT").Err(`wrong number of arguments for 'client' command`), + Do("CLIENT", "hello").Err(`Syntax error, try CLIENT (LIST | KILL | GETNAME | SETNAME)`), + Do("CLIENT", "list", "arg3").Err(`wrong number of arguments for 'client' command`), + Do("CLIENT", "getname", "arg3").Err(`wrong number of arguments for 'client' command`), + Do("CLIENT", "getname").JSON().Str(`{"ok":true,"name":""}`), + Do("CLIENT", "getname").Str(``), + ) + } diff --git a/tests/tests_test.go b/tests/tests_test.go index be9d3b3a..3fb3b424 100644 --- a/tests/tests_test.go +++ b/tests/tests_test.go @@ -56,10 +56,10 @@ func TestAll(t *testing.T) { runSubTest(t, "json", mc, subTestJSON) runSubTest(t, "search", mc, subTestSearch) runSubTest(t, "testcmd", mc, subTestTestCmd) - runSubTest(t, "fence", mc, subTestFence) - runSubTest(t, "scripts", mc, subTestScripts) - runSubTest(t, "info", mc, subTestInfo) runSubTest(t, "client", mc, subTestClient) + runSubTest(t, "scripts", mc, subTestScripts) + runSubTest(t, "fence", mc, subTestFence) + runSubTest(t, "info", mc, subTestInfo) runSubTest(t, "timeouts", mc, subTestTimeout) runSubTest(t, "metrics", mc, subTestMetrics) }