From 194b2c49650cee0ac76e54d66779829aa76b9b7d Mon Sep 17 00:00:00 2001 From: Paulo Pires Date: Thu, 12 Nov 2015 17:04:02 +0000 Subject: [PATCH 1/5] CLI history skips blank lines. Fixes #4719 --- cmd/influx/cli/cli.go | 97 +++++++++++++++++++++++--------------- cmd/influx/cli/cli_test.go | 74 +++++++++++++++++++++++++++-- 2 files changed, 129 insertions(+), 42 deletions(-) diff --git a/cmd/influx/cli/cli.go b/cmd/influx/cli/cli.go index 4ce49cf5d0f..67acd9c34bd 100644 --- a/cmd/influx/cli/cli.go +++ b/cmd/influx/cli/cli.go @@ -1,11 +1,11 @@ package cli import ( + "bytes" "encoding/csv" "encoding/json" "fmt" "io" - "io/ioutil" "net" "net/url" "os" @@ -26,6 +26,7 @@ const ( noTokenMsg = "Visit https://enterprise.influxdata.com to register for updates, InfluxDB server management, and monitoring.\n" ) +// CommandLine holds CLI configuration and state type CommandLine struct { Client *client.Client Line *liner.State @@ -48,12 +49,19 @@ type CommandLine struct { PPS int // Controls how many points per second the import will allow via throttling Path string Compressed bool + Quit chan struct{} + historyFile *os.File } +// New returns an instance of CommandLine func New(version string) *CommandLine { - return &CommandLine{ClientVersion: version} + return &CommandLine{ + ClientVersion: version, + Quit: make(chan struct{}, 1), + } } +// Run executes the CLI func (c *CommandLine) Run() { var promptForPassword bool // determine if they set the password flag but provided no value @@ -139,38 +147,47 @@ func (c *CommandLine) Run() { c.Version() - var historyFile string + var historyFilePath string usr, err := user.Current() - // Only load history if we can get the user + // Only load/write history if we can get the user if err == nil { - historyFile = filepath.Join(usr.HomeDir, ".influx_history") - - if f, err := os.Open(historyFile); err == nil { - c.Line.ReadHistory(f) - f.Close() + historyFilePath = filepath.Join(usr.HomeDir, ".influx_history") + if c.historyFile, err = os.OpenFile(historyFilePath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0640); err == nil { + defer c.historyFile.Close() + c.Line.ReadHistory(c.historyFile) } } +Loop: + // read from prompt until exit is run for { - l, e := c.Line.Prompt("> ") - if e != nil { - break - } - if c.ParseCommand(l) { - // write out the history - if len(historyFile) > 0 { + select { + case <-c.Quit: + // write to history file + _, err := c.Line.WriteHistory(c.historyFile) + if err != nil { + fmt.Printf("There was an error writing history file: %s\n", err) + } + // exit CLI + break Loop + default: + l, e := c.Line.Prompt("> ") + if e != nil { + break + } + // write valid commands only to in-memory history + if c.ParseCommand(l) { c.Line.AppendHistory(l) - if f, err := os.Create(historyFile); err == nil { - c.Line.WriteHistory(f) - f.Close() + _, err := c.Line.WriteHistory(c.historyFile) + if err != nil { + fmt.Printf("There was an error writing history file: %s\n", err) } } - } else { - break // exit main loop } } } +// ParseCommand parses an instruction and calls related method, if any func (c *CommandLine) ParseCommand(cmd string) bool { lcmd := strings.TrimSpace(strings.ToLower(cmd)) @@ -184,11 +201,9 @@ func (c *CommandLine) ParseCommand(cmd string) bool { if len(tokens) > 0 { switch tokens[0] { - case "": - break case "exit": // signal the program to exit - return false + close(c.Quit) case "gopher": c.gopher() case "connect": @@ -221,8 +236,10 @@ func (c *CommandLine) ParseCommand(cmd string) bool { default: c.ExecuteQuery(cmd) } + + return true } - return true + return false } // Connect connects client to a server @@ -255,17 +272,17 @@ func (c *CommandLine) Connect(cmd string) error { return fmt.Errorf("Could not create client %s", err) } c.Client = cl - if _, v, e := c.Client.Ping(); e != nil { + + var v string + if _, v, e = c.Client.Ping(); e != nil { return fmt.Errorf("Failed to connect to %s\n", c.Client.Addr()) - } else { - c.ServerVersion = v } - - _, c.ServerVersion, _ = c.Client.Ping() + c.ServerVersion = v return nil } +// SetAuth sets client authentication credentials func (c *CommandLine) SetAuth(cmd string) { // If they pass in the entire command, we should parse it // auth @@ -309,6 +326,7 @@ func (c *CommandLine) use(cmd string) { fmt.Printf("Using database %s\n", d) } +// SetPrecision sets client precision func (c *CommandLine) SetPrecision(cmd string) { // Remove the "precision" keyword if it exists cmd = strings.TrimSpace(strings.Replace(cmd, "precision", "", -1)) @@ -327,6 +345,7 @@ func (c *CommandLine) SetPrecision(cmd string) { } } +// SetFormat sets output format func (c *CommandLine) SetFormat(cmd string) { // Remove the "format" keyword if it exists cmd = strings.TrimSpace(strings.Replace(cmd, "format", "", -1)) @@ -341,6 +360,7 @@ func (c *CommandLine) SetFormat(cmd string) { } } +// SetWriteConsistency sets cluster consistency level func (c *CommandLine) SetWriteConsistency(cmd string) { // Remove the "consistency" keyword if it exists cmd = strings.TrimSpace(strings.Replace(cmd, "consistency", "", -1)) @@ -425,6 +445,7 @@ func (c *CommandLine) parseInto(stmt string) string { return stmt } +// Insert runs an INSERT statement func (c *CommandLine) Insert(stmt string) error { i, point := parseNextIdentifier(stmt) if !strings.EqualFold(i, "insert") { @@ -455,6 +476,7 @@ func (c *CommandLine) Insert(stmt string) error { return nil } +// ExecuteQuery runs any query statement func (c *CommandLine) ExecuteQuery(query string) error { response, err := c.Client.Query(client.Query{Command: query, Database: c.Database}) if err != nil { @@ -473,6 +495,7 @@ func (c *CommandLine) ExecuteQuery(query string) error { return nil } +// DatabaseToken retrieves database token func (c *CommandLine) DatabaseToken() (string, error) { response, err := c.Client.Query(client.Query{Command: "SHOW DIAGNOSTICS for 'registration'"}) if err != nil { @@ -491,6 +514,7 @@ func (c *CommandLine) DatabaseToken() (string, error) { return "", nil } +// FormatResponse formats output to previsouly chosen format func (c *CommandLine) FormatResponse(response *client.Response, w io.Writer) { switch c.Format { case "json": @@ -644,6 +668,7 @@ func interfaceToString(v interface{}) string { } } +// Settings prints current settings func (c *CommandLine) Settings() { w := new(tabwriter.Writer) w.Init(os.Stdout, 0, 8, 1, '\t', 0) @@ -685,14 +710,9 @@ func (c *CommandLine) help() { } func (c *CommandLine) history() { - usr, err := user.Current() - // Only load history if we can get the user - if err == nil { - historyFile := filepath.Join(usr.HomeDir, ".influx_history") - if history, err := ioutil.ReadFile(historyFile); err == nil { - fmt.Print(string(history)) - } - } + var buf bytes.Buffer + c.Line.WriteHistory(&buf) + fmt.Print(buf.String()) } func (c *CommandLine) gopher() { @@ -752,6 +772,7 @@ func (c *CommandLine) gopher() { `) } +// Version prints CLI version func (c *CommandLine) Version() { fmt.Println("InfluxDB shell " + c.ClientVersion) } diff --git a/cmd/influx/cli/cli_test.go b/cmd/influx/cli/cli_test.go index 00755d123e1..d25f7756d35 100644 --- a/cmd/influx/cli/cli_test.go +++ b/cmd/influx/cli/cli_test.go @@ -1,6 +1,8 @@ package cli_test import ( + "bufio" + "bytes" "encoding/json" "net/http" "net/http/httptest" @@ -9,6 +11,7 @@ import ( "github.com/influxdb/influxdb/client" "github.com/influxdb/influxdb/cmd/influx/cli" + "github.com/peterh/liner" ) func TestParseCommand_CommandsExist(t *testing.T) { @@ -22,7 +25,6 @@ func TestParseCommand_CommandsExist(t *testing.T) { {cmd: "help"}, {cmd: "pretty"}, {cmd: "use"}, - {cmd: ""}, // test that a blank command just returns } for _, test := range tests { if !c.ParseCommand(test.cmd) { @@ -31,6 +33,21 @@ func TestParseCommand_CommandsExist(t *testing.T) { } } +func TestParseCommand_BlankCommand(t *testing.T) { + t.Parallel() + c := cli.CommandLine{} + tests := []struct { + cmd string + }{ + {cmd: ""}, // test that a blank command doesn't work + } + for _, test := range tests { + if c.ParseCommand(test.cmd) { + t.Fatalf(`Command failed for %q.`, test.cmd) + } + } +} + func TestParseCommand_TogglePretty(t *testing.T) { t.Parallel() c := cli.CommandLine{} @@ -49,7 +66,6 @@ func TestParseCommand_TogglePretty(t *testing.T) { func TestParseCommand_Exit(t *testing.T) { t.Parallel() - c := cli.CommandLine{} tests := []struct { cmd string }{ @@ -60,7 +76,10 @@ func TestParseCommand_Exit(t *testing.T) { } for _, test := range tests { - if c.ParseCommand(test.cmd) { + c := cli.CommandLine{Quit: make(chan struct{}, 1)} + c.ParseCommand(test.cmd) + // channel should be closed + if _, ok := <-c.Quit; ok { t.Fatalf(`Command "exit" failed for %q.`, test.cmd) } } @@ -220,7 +239,12 @@ func TestParseCommand_InsertInto(t *testing.T) { func TestParseCommand_History(t *testing.T) { t.Parallel() - c := cli.CommandLine{} + c := cli.CommandLine{Line: liner.NewLiner()} + defer c.Line.Close() + + // append one entry to history + c.Line.AppendHistory("abc") + tests := []struct { cmd string }{ @@ -235,4 +259,46 @@ func TestParseCommand_History(t *testing.T) { t.Fatalf(`Command "history" failed for %q.`, test.cmd) } } + + // buf size should be at least 1 + var buf bytes.Buffer + c.Line.WriteHistory(&buf) + if buf.Len() < 1 { + t.Fatal("History is borked") + } +} + +func TestParseCommand_HistoryWithBlankCommand(t *testing.T) { + t.Parallel() + c := cli.CommandLine{Line: liner.NewLiner()} + defer c.Line.Close() + + // append one entry to history + c.Line.AppendHistory("x") + + tests := []struct { + cmd string + }{ + {cmd: "history"}, + {cmd: " history"}, + {cmd: "history "}, + {cmd: "History "}, + {cmd: ""}, // shouldn't be persisted in history + {cmd: " "}, // shouldn't be persisted in history + } + + // don't validate because blank commands are never executed + for _, test := range tests { + c.ParseCommand(test.cmd) + } + + // buf shall not contain empty commands + var buf bytes.Buffer + c.Line.WriteHistory(&buf) + scanner := bufio.NewScanner(&buf) + for scanner.Scan() { + if scanner.Text() == "" || scanner.Text() == " " { + t.Fatal("Empty commands should not be persisted in history.") + } + } } From c812aaa36b06fb0a32c63845fa8a7ce0159c6ae8 Mon Sep 17 00:00:00 2001 From: Paulo Pires Date: Thu, 12 Nov 2015 20:52:41 +0000 Subject: [PATCH 2/5] Refactored command parsing tokenization. --- cmd/influx/cli/cli.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/cmd/influx/cli/cli.go b/cmd/influx/cli/cli.go index 67acd9c34bd..18250a587d3 100644 --- a/cmd/influx/cli/cli.go +++ b/cmd/influx/cli/cli.go @@ -190,14 +190,7 @@ Loop: // ParseCommand parses an instruction and calls related method, if any func (c *CommandLine) ParseCommand(cmd string) bool { lcmd := strings.TrimSpace(strings.ToLower(cmd)) - - split := strings.Split(lcmd, " ") - var tokens []string - for _, token := range split { - if token != "" { - tokens = append(tokens, token) - } - } + tokens := strings.Fields(lcmd) if len(tokens) > 0 { switch tokens[0] { From 81658de1be0df0d5bfd77510460762a79930ebf5 Mon Sep 17 00:00:00 2001 From: Paulo Pires Date: Thu, 12 Nov 2015 22:42:11 +0000 Subject: [PATCH 3/5] Exit gracefully on forced CLI termination. --- cmd/influx/cli/cli.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/cmd/influx/cli/cli.go b/cmd/influx/cli/cli.go index 18250a587d3..8aa098fb77f 100644 --- a/cmd/influx/cli/cli.go +++ b/cmd/influx/cli/cli.go @@ -9,11 +9,13 @@ import ( "net" "net/url" "os" + "os/signal" "os/user" "path/filepath" "sort" "strconv" "strings" + "syscall" "text/tabwriter" "github.com/influxdb/influxdb/client" @@ -50,6 +52,7 @@ type CommandLine struct { Path string Compressed bool Quit chan struct{} + osSignals chan os.Signal historyFile *os.File } @@ -58,11 +61,15 @@ func New(version string) *CommandLine { return &CommandLine{ ClientVersion: version, Quit: make(chan struct{}, 1), + osSignals: make(chan os.Signal, 1), } } // Run executes the CLI func (c *CommandLine) Run() { + // register OS signals for graceful termination + signal.Notify(c.osSignals, os.Kill, os.Interrupt, syscall.SIGTERM) + var promptForPassword bool // determine if they set the password flag but provided no value for _, v := range os.Args { @@ -158,18 +165,13 @@ func (c *CommandLine) Run() { } } -Loop: // read from prompt until exit is run for { select { + case <-c.osSignals: + close(c.Quit) case <-c.Quit: - // write to history file - _, err := c.Line.WriteHistory(c.historyFile) - if err != nil { - fmt.Printf("There was an error writing history file: %s\n", err) - } - // exit CLI - break Loop + c.exit() default: l, e := c.Line.Prompt("> ") if e != nil { @@ -769,3 +771,16 @@ func (c *CommandLine) gopher() { func (c *CommandLine) Version() { fmt.Println("InfluxDB shell " + c.ClientVersion) } + +func (c *CommandLine) exit() { + // write to history file + _, err := c.Line.WriteHistory(c.historyFile) + if err != nil { + fmt.Printf("There was an error writing history file: %s\n", err) + } + // release line resources + c.Line.Close() + c.Line = nil + // exit CLI + os.Exit(0) +} From aa6ec0cc6f61487cb677e3bdaf1a324fcff1ff79 Mon Sep 17 00:00:00 2001 From: Paulo Pires Date: Fri, 13 Nov 2015 11:38:35 +0000 Subject: [PATCH 4/5] Added more tests to increase coverage. Refs #2313 --- cmd/influx/cli/cli_test.go | 160 ++++++++++++++++++++++++++++++++++--- 1 file changed, 149 insertions(+), 11 deletions(-) diff --git a/cmd/influx/cli/cli_test.go b/cmd/influx/cli/cli_test.go index d25f7756d35..7d476239bbd 100644 --- a/cmd/influx/cli/cli_test.go +++ b/cmd/influx/cli/cli_test.go @@ -3,10 +3,11 @@ package cli_test import ( "bufio" "bytes" - "encoding/json" + "net" "net/http" "net/http/httptest" "net/url" + "strconv" "testing" "github.com/influxdb/influxdb/client" @@ -14,6 +15,142 @@ import ( "github.com/peterh/liner" ) +const ( + CLIENT_VERSION = "y.y" + SERVER_VERSION = "x.x" +) + +func TestNewCLI(t *testing.T) { + t.Parallel() + c := cli.New(CLIENT_VERSION) + + if c == nil { + t.Fatal("CommandLine shouldn't be nil.") + } + + if c.ClientVersion != CLIENT_VERSION { + t.Fatalf("CommandLine version is %s but should be %s", c.ClientVersion, CLIENT_VERSION) + } +} + +func TestRunCLI(t *testing.T) { + t.Parallel() + ts := emptyTestServer() + defer ts.Close() + + u, _ := url.Parse(ts.URL) + h, p, _ := net.SplitHostPort(u.Host) + c := cli.New(CLIENT_VERSION) + c.Host = h + c.Port, _ = strconv.Atoi(p) + c.Run() +} + +func TestConnect(t *testing.T) { + t.Parallel() + ts := emptyTestServer() + defer ts.Close() + + u, _ := url.Parse(ts.URL) + cmd := "connect " + u.Host + c := cli.CommandLine{} + + // assert connection is established + if err := c.Connect(cmd); err != nil { + t.Fatalf("There was an error while connecting to %s: %s", u.Path, err) + } + + // assert server version is populated + if c.ServerVersion != SERVER_VERSION { + t.Fatalf("Server version is %s but should be %s.", c.ServerVersion, SERVER_VERSION) + } +} + +func TestSetAuth(t *testing.T) { + t.Parallel() + c := cli.New(CLIENT_VERSION) + config := client.NewConfig() + client, _ := client.NewClient(config) + c.Client = client + u := "userx" + p := "pwdy" + c.SetAuth("auth " + u + " " + p) + + // validate CLI configuration + if c.Username != u { + t.Fatalf("Username is %s but should be %s", c.Username, u) + } + if c.Password != p { + t.Fatalf("Password is %s but should be %s", c.Password, p) + } +} + +func TestSetPrecision(t *testing.T) { + t.Parallel() + c := cli.New(CLIENT_VERSION) + config := client.NewConfig() + client, _ := client.NewClient(config) + c.Client = client + + // validate set non-default precision + p := "ns" + c.SetPrecision("precision " + p) + if c.Precision != p { + t.Fatalf("Precision is %s but should be %s", c.Precision, p) + } + + // validate set default precision which equals empty string + p = "rfc3339" + c.SetPrecision("precision " + p) + if c.Precision != "" { + t.Fatalf("Precision is %s but should be empty", c.Precision) + } +} + +func TestSetFormat(t *testing.T) { + t.Parallel() + c := cli.New(CLIENT_VERSION) + config := client.NewConfig() + client, _ := client.NewClient(config) + c.Client = client + + // validate set non-default format + f := "json" + c.SetFormat("format " + f) + if c.Format != f { + t.Fatalf("Format is %s but should be %s", c.Format, f) + } +} + +func TestSetWriteConsistency(t *testing.T) { + t.Parallel() + c := cli.New(CLIENT_VERSION) + config := client.NewConfig() + client, _ := client.NewClient(config) + c.Client = client + + // set valid write consistency + consistency := "all" + c.SetWriteConsistency("consistency " + consistency) + if c.WriteConsistency != consistency { + t.Fatalf("WriteConsistency is %s but should be %s", c.WriteConsistency, consistency) + } + + // set different valid write consistency and validate change + consistency = "quorum" + c.SetWriteConsistency("consistency " + consistency) + if c.WriteConsistency != consistency { + t.Fatalf("WriteConsistency is %s but should be %s", c.WriteConsistency, consistency) + } + + // set invalid write consistency and verify there was no change + invalidConsistency := "invalid_consistency" + c.SetWriteConsistency("consistency " + invalidConsistency) + if c.WriteConsistency == invalidConsistency { + t.Fatalf("WriteConsistency is %s but should be %s", c.WriteConsistency, consistency) + } +} + func TestParseCommand_CommandsExist(t *testing.T) { t.Parallel() c := cli.CommandLine{} @@ -137,11 +274,7 @@ func TestParseCommand_Consistency(t *testing.T) { func TestParseCommand_Insert(t *testing.T) { t.Parallel() - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var data client.Response - w.WriteHeader(http.StatusNoContent) - _ = json.NewEncoder(w).Encode(data) - })) + ts := emptyTestServer() defer ts.Close() u, _ := url.Parse(ts.URL) @@ -174,11 +307,7 @@ func TestParseCommand_Insert(t *testing.T) { func TestParseCommand_InsertInto(t *testing.T) { t.Parallel() - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var data client.Response - w.WriteHeader(http.StatusNoContent) - _ = json.NewEncoder(w).Encode(data) - })) + ts := emptyTestServer() defer ts.Close() u, _ := url.Parse(ts.URL) @@ -302,3 +431,12 @@ func TestParseCommand_HistoryWithBlankCommand(t *testing.T) { } } } + +// helper methods + +func emptyTestServer() *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Influxdb-Version", SERVER_VERSION) + return + })) +} From a8fa170f29f128eccd39dc68145c9a355d7975b0 Mon Sep 17 00:00:00 2001 From: Paulo Pires Date: Sat, 14 Nov 2015 11:12:31 +0000 Subject: [PATCH 5/5] Removed bogus comment. --- cmd/influx/cli/cli.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/influx/cli/cli.go b/cmd/influx/cli/cli.go index 8aa098fb77f..9f6b84e1251 100644 --- a/cmd/influx/cli/cli.go +++ b/cmd/influx/cli/cli.go @@ -177,7 +177,6 @@ func (c *CommandLine) Run() { if e != nil { break } - // write valid commands only to in-memory history if c.ParseCommand(l) { c.Line.AppendHistory(l) _, err := c.Line.WriteHistory(c.historyFile)