From 5325fc75aec96d354052264585a5daab1413b8be Mon Sep 17 00:00:00 2001 From: David Dworken Date: Mon, 11 Apr 2022 22:36:52 -0700 Subject: [PATCH] Add negative conditions to search queries + tests + better error messages by including filename:line in error messages --- client/client_test.go | 47 +++++++++++++++++++++++- client/data/data.go | 84 ++++++++++++++++++++++++++++--------------- client/lib/lib.go | 7 ++-- shared/testutils.go | 7 ++-- 4 files changed, 112 insertions(+), 33 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 253ae2f..c53d363 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "regexp" + "runtime" "strings" "testing" @@ -18,7 +19,8 @@ import ( func RunInteractiveBashCommands(t *testing.T, script string) string { out, err := RunInteractiveBashCommandsWithoutStrictMode(t, "set -emo pipefail\n"+script) if err != nil { - t.Fatalf("error when running command: %v", err) + _, filename, line, _ := runtime.Caller(1) + t.Fatalf("error when running command at %s:%d: %v", filename, line, err) } return out } @@ -262,6 +264,7 @@ func TestAdvancedQuery(t *testing.T) { notacommand cd /tmp/ echo querybydir + hishtory disable `) // Query based on cwd @@ -367,6 +370,48 @@ func TestAdvancedQuery(t *testing.T) { if strings.Count(out, "\n") != 2 { t.Fatalf("hishtory query has the wrong number of lines=%d, out=%#v", strings.Count(out, "\n"), out) } + + // Test filtering out a search item + out = RunInteractiveBashCommands(t, `hishtory query`) + if !strings.Contains(out, "cmd_with_diff_hostname_and_username") { + t.Fatalf("hishtory query doesn't contain expected result, out=%#v", out) + } + out = RunInteractiveBashCommands(t, `hishtory query -cmd_with_diff_hostname_and_username`) + if strings.Contains(out, "cmd_with_diff_hostname_and_username") { + t.Fatalf("hishtory query contains unexpected result, out=%#v", out) + } + out = RunInteractiveBashCommands(t, `hishtory query -echo`) + if strings.Contains(out, "echo") { + t.Fatalf("hishtory query contains unexpected result, out=%#v", out) + } + if strings.Count(out, "\n") != 7 { + t.Fatalf("hishtory query has the wrong number of lines=%d, out=%#v", strings.Count(out, "\n"), out) + } + + // Test filtering out with an atom + out = RunInteractiveBashCommands(t, `hishtory query -hostname:otherhostname`) + if strings.Contains(out, "cmd_with_diff_hostname_and_username") { + t.Fatalf("hishtory query contains unexpected result, out=%#v", out) + } + out = RunInteractiveBashCommands(t, `hishtory query -user:otheruser`) + if strings.Contains(out, "cmd_with_diff_hostname_and_username") { + t.Fatalf("hishtory query contains unexpected result, out=%#v", out) + } + out = RunInteractiveBashCommands(t, `hishtory query -exit_code:0`) + if strings.Count(out, "\n") != 3 { + t.Fatalf("hishtory query has the wrong number of lines=%d, out=%#v", strings.Count(out, "\n"), out) + } + + // Test filtering out a search item that also looks like it could be a search for a flag + entry = data.MakeFakeHistoryEntry("foo -echo") + manuallySubmitHistoryEntry(t, userSecret, entry) + out = RunInteractiveBashCommands(t, `hishtory query -echo`) + if strings.Contains(out, "echo") { + t.Fatalf("hishtory query contains unexpected result, out=%#v", out) + } + if strings.Count(out, "\n") != 7 { + t.Fatalf("hishtory query has the wrong number of lines=%d, out=%#v", strings.Count(out, "\n"), out) + } } func TestUpdate(t *testing.T) { diff --git a/client/data/data.go b/client/data/data.go index 3b6e765..95f1b03 100644 --- a/client/data/data.go +++ b/client/data/data.go @@ -157,40 +157,32 @@ func Search(db *gorm.DB, query string, limit int) ([]*HistoryEntry, error) { } tx := db.Where("true") for _, token := range tokens { - if strings.Contains(token, ":") { - splitToken := strings.SplitN(token, ":", 2) - field := splitToken[0] - val := splitToken[1] - switch field { - case "user": - tx = tx.Where("local_username = ?", val) - case "hostname": - tx = tx.Where("instr(hostname, ?) > 0", val) - case "cwd": - // TODO: Can I make this support querying via ~/ too? - tx = tx.Where("instr(current_working_directory, ?) > 0", val) - case "exit_code": - tx = tx.Where("exit_code = ?", val) - case "before": - t, err := parseTimeGenerously(val) + if strings.HasPrefix(token, "-") { + if strings.Contains(token, ":") { + query, val, err := parseAtomizedToken(token[1:]) if err != nil { - return nil, fmt.Errorf("failed to parse before:%s as a timestamp: %v", val, err) + return nil, err } - tx = tx.Where("CAST(strftime(\"%s\",start_time) AS INTEGER) < ?", t.Unix()) - case "after": - t, err := parseTimeGenerously(val) + tx = tx.Where("NOT "+query, val) + } else { + query, v1, v2, v3, err := parseNonAtomizedToken(token[1:]) if err != nil { - return nil, fmt.Errorf("failed to parse after:%s as a timestamp: %v", val, err) + return nil, err } - tx = tx.Where("CAST(strftime(\"%s\",start_time) AS INTEGER) > ?", t.Unix()) - default: - panic("TODO: probably return an error?") + tx = tx.Where("NOT "+query, v1, v2, v3) } - } else if strings.HasPrefix(token, "-") { - panic("TODO(ddworken): Implement -foo as filtering out foo") + } else if strings.Contains(token, ":") { + query, val, err := parseAtomizedToken(token) + if err != nil { + return nil, err + } + tx = tx.Where(query, val) } else { - wildcardedToken := "%" + token + "%" - tx = tx.Where("(command LIKE ? OR hostname LIKE ? OR current_working_directory LIKE ?)", wildcardedToken, wildcardedToken, wildcardedToken) + query, v1, v2, v3, err := parseNonAtomizedToken(token) + if err != nil { + return nil, err + } + tx = tx.Where(query, v1, v2, v3) } } tx = tx.Order("end_time DESC") @@ -205,6 +197,42 @@ func Search(db *gorm.DB, query string, limit int) ([]*HistoryEntry, error) { return historyEntries, nil } +func parseNonAtomizedToken(token string) (string, interface{}, interface{}, interface{}, error) { + wildcardedToken := "%" + token + "%" + return "(command LIKE ? OR hostname LIKE ? OR current_working_directory LIKE ?)", wildcardedToken, wildcardedToken, wildcardedToken, nil +} + +func parseAtomizedToken(token string) (string, interface{}, error) { + splitToken := strings.SplitN(token, ":", 2) + field := splitToken[0] + val := splitToken[1] + switch field { + case "user": + return "(local_username = ?)", val, nil + case "hostname": + return "(instr(hostname, ?) > 0)", val, nil + case "cwd": + // TODO: Can I make this support querying via ~/ too? + return "(instr(current_working_directory, ?) > 0)", val, nil + case "exit_code": + return "(exit_code = ?)", val, nil + case "before": + t, err := parseTimeGenerously(val) + if err != nil { + return "", nil, fmt.Errorf("failed to parse before:%s as a timestamp: %v", val, err) + } + return "(CAST(strftime(\"%s\",start_time) AS INTEGER) < ?)", t.Unix(), nil + case "after": + t, err := parseTimeGenerously(val) + if err != nil { + return "", nil, fmt.Errorf("failed to parse after:%s as a timestamp: %v", val, err) + } + return "(CAST(strftime(\"%s\",start_time) AS INTEGER) > ?)", t.Unix(), nil + default: + return "", nil, fmt.Errorf("search query contains unknown search atom %s", field) + } +} + func tokenize(query string) ([]string, error) { if query == "" { return []string{}, nil diff --git a/client/lib/lib.go b/client/lib/lib.go index bc880a7..615caf0 100644 --- a/client/lib/lib.go +++ b/client/lib/lib.go @@ -2,7 +2,6 @@ package lib import ( "bytes" - _ "embed" // for embedding config.sh "encoding/json" "fmt" "io" @@ -14,11 +13,14 @@ import ( "os/user" "path" "path/filepath" + "runtime" "strconv" "strings" "syscall" "time" + _ "embed" // for embedding config.sh + "gorm.io/driver/sqlite" "gorm.io/gorm" @@ -276,7 +278,8 @@ func Disable() error { func CheckFatalError(err error) { if err != nil { - log.Fatalf("hishtory fatal error: %v", err) + _, filename, line, _ := runtime.Caller(1) + log.Fatalf("hishtory fatal error at %s:%d: %v", filename, line, err) } } diff --git a/shared/testutils.go b/shared/testutils.go index 4ad3bf7..00e7154 100644 --- a/shared/testutils.go +++ b/shared/testutils.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path" + "runtime" "strings" "testing" "time" @@ -98,12 +99,14 @@ func RunTestServer(t *testing.T) func() { func Check(t *testing.T, err error) { if err != nil { - t.Fatalf("Unexpected error: %v", err) + _, filename, line, _ := runtime.Caller(1) + t.Fatalf("Unexpected error at %s:%d: %v", filename, line, err) } } func CheckWithInfo(t *testing.T, err error, additionalInfo string) { if err != nil { - t.Fatalf("Unexpected error: %v! Additional info: %v", err, additionalInfo) + _, filename, line, _ := runtime.Caller(1) + t.Fatalf("Unexpected error: %v at %s:%d! Additional info: %v", err, filename, line, additionalInfo) } }