Commit 7cc95d1fc1
Verified · cmc ci/build: success ci/test: success ci/vuln: success
Layout: unified · split
e2e/issuesearch_test.go added +114
| @@ -0,0 +1,114 @@ | ||
| 1 | package e2e | |
| 2 | ||
| 3 | import ( | |
| 4 | "encoding/json" | |
| 5 | "strings" | |
| 6 | "testing" | |
| 7 | ) | |
| 8 | ||
| 9 | // issueNumbers runs issue list with the given flags and returns the numbers. | |
| 10 | func issueNumbers(t *testing.T, inst *instance, key string, args ...string) []int64 { | |
| 11 | t.Helper() | |
| 12 | out, errOut, code := inst.ssh(t, key, "", append([]string{"issue", "list"}, append(args, "--json")...)...) | |
| 13 | if code != 0 { | |
| 14 | t.Fatalf("issue list: %s", errOut) | |
| 15 | } | |
| 16 | var env struct { | |
| 17 | Data []struct { | |
| 18 | Number int64 `json:"number"` | |
| 19 | } `json:"data"` | |
| 20 | } | |
| 21 | if err := json.Unmarshal([]byte(out), &env); err != nil { | |
| 22 | t.Fatalf("not JSON: %v\n%s", err, out) | |
| 23 | } | |
| 24 | var ns []int64 | |
| 25 | for _, d := range env.Data { | |
| 26 | ns = append(ns, d.Number) | |
| 27 | } | |
| 28 | return ns | |
| 29 | } | |
| 30 | ||
| 31 | func TestIssueSearch(t *testing.T) { | |
| 32 | inst := startInstance(t) | |
| 33 | aliceKey := inst.newKey(t, "alice") | |
| 34 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | |
| 35 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 { | |
| 36 | t.Fatalf("repo create: %s", errOut) | |
| 37 | } | |
| 38 | ||
| 39 | mk := func(title, body string) { | |
| 40 | t.Helper() | |
| 41 | if _, errOut, code := inst.ssh(t, aliceKey, "", "issue", "create", "alice/app", | |
| 42 | "--title", "'"+title+"'", "--body", "'"+body+"'"); code != 0 { | |
| 43 | t.Fatalf("issue create: %s", errOut) | |
| 44 | } | |
| 45 | } | |
| 46 | mk("memory leak in the parser", "resident size climbs forever") | |
| 47 | mk("flaky test", "the runner times out sometimes") | |
| 48 | mk("docs typo", "spelled parser wrong") | |
| 49 | ||
| 50 | // Title match. | |
| 51 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "leak"); len(got) != 1 || got[0] != 1 { | |
| 52 | t.Fatalf("title search = %v, want [1]", got) | |
| 53 | } | |
| 54 | // Body match — the half a title-only search could never reach. | |
| 55 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "climbs"); len(got) != 1 || got[0] != 1 { | |
| 56 | t.Fatalf("body search = %v, want [1]", got) | |
| 57 | } | |
| 58 | // One word in two issues, one in a title and one in a body. | |
| 59 | got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "parser") | |
| 60 | if len(got) != 2 { | |
| 61 | t.Fatalf("search across title and body = %v, want two issues", got) | |
| 62 | } | |
| 63 | // Terms are ANDed, not ORed. | |
| 64 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "'parser runner'"); len(got) != 0 { | |
| 65 | t.Fatalf("terms ORed rather than ANDed: %v", got) | |
| 66 | } | |
| 67 | // Search composes with the other filters. | |
| 68 | if _, _, code := inst.ssh(t, aliceKey, "", "issue", "close", "alice/app", "1"); code != 0 { | |
| 69 | t.Fatal("close failed") | |
| 70 | } | |
| 71 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "parser"); len(got) != 1 || got[0] != 3 { | |
| 72 | t.Fatalf("search ignored --state open: %v", got) | |
| 73 | } | |
| 74 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "parser", "--state", "all"); len(got) != 2 { | |
| 75 | t.Fatalf("--state all with search = %v", got) | |
| 76 | } | |
| 77 | ||
| 78 | // An edit moves the issue in the index rather than leaving the old | |
| 79 | // words matching. | |
| 80 | if _, errOut, code := inst.ssh(t, aliceKey, "", "issue", "edit", "alice/app", "2", | |
| 81 | "--title", "'renamed entirely'", "--body", "'nothing of the original remains'"); code != 0 { | |
| 82 | t.Fatalf("issue edit: %s", errOut) | |
| 83 | } | |
| 84 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "flaky", "--state", "all"); len(got) != 0 { | |
| 85 | t.Fatalf("edited-away words still match: %v", got) | |
| 86 | } | |
| 87 | if got := issueNumbers(t, inst, aliceKey, "alice/app", "--search", "renamed", "--state", "all"); len(got) != 1 { | |
| 88 | t.Fatalf("new words not indexed: %v", got) | |
| 89 | } | |
| 90 | ||
| 91 | // FTS5 operators typed by a person are terms, not syntax errors. | |
| 92 | // Single characters are refused for length before they get this far, | |
| 93 | // which the "x" case below covers. | |
| 94 | for _, q := range []string{"c++", "AND", "NOT", "foo:", "a AND", "b*", "()"} { | |
| 95 | if _, errOut, code := inst.ssh(t, aliceKey, "", "issue", "list", "alice/app", | |
| 96 | "--search", "'"+q+"'", "--json"); code != 0 { | |
| 97 | t.Fatalf("search %q failed: %s", q, errOut) | |
| 98 | } | |
| 99 | } | |
| 100 | // Too short is a usage error, the same as every other query. | |
| 101 | if _, errOut, code := inst.ssh(t, aliceKey, "", "issue", "list", "alice/app", "--search", "x"); code != 2 || | |
| 102 | !strings.Contains(errOut, "2 to 200 characters") { | |
| 103 | t.Fatalf("short search: exit %d, %s", code, errOut) | |
| 104 | } | |
| 105 | ||
| 106 | // The instance-wide search reaches bodies too now. | |
| 107 | out, errOut, code := inst.ssh(t, aliceKey, "", "search", "climbs", "--json") | |
| 108 | if code != 0 { | |
| 109 | t.Fatalf("search: %s", errOut) | |
| 110 | } | |
| 111 | if !strings.Contains(out, "alice/app") || !strings.Contains(out, "memory leak") { | |
| 112 | t.Fatalf("global body search: %s", out) | |
| 113 | } | |
| 114 | } | |
internal/control/issue.go +9 −3
| @@ -21,7 +21,7 @@ func init() { | ||
| 21 | 21 | ReadsStdin: true, Run: runIssueCreate}) |
| 22 | 22 | register(Command{Path: []string{"issue", "list"}, |
| 23 | 23 | Summary: "list issues", |
| 24 | Usage: "issue list <owner/name> [--state open|closed|all] [--label <l>] [--assignee <user>] [--author <user>] [--milestone <title>|none] [--limit <n>] [--cursor <c>]", ReadOnly: true, Run: runIssueList}) | |
| 24 | Usage: "issue list <owner/name> [--state open|closed|all] [--label <l>] [--assignee <user>] [--author <user>] [--milestone <title>|none] [--search <text>] [--limit <n>] [--cursor <c>]", ReadOnly: true, Run: runIssueList}) | |
| 25 | 25 | register(Command{Path: []string{"issue", "show"}, |
| 26 | 26 | Summary: "show an issue with comments", |
| 27 | 27 | Usage: "issue show <owner/name> <n>", ReadOnly: true, Run: runIssueShow}) |
| @@ -165,9 +165,9 @@ func runIssueList(c *Ctx, args []string) int { | ||
| 165 | 165 | if code >= 0 { |
| 166 | 166 | return code |
| 167 | 167 | } |
| 168 | const usage = "usage: issue list <owner/name> [--state open|closed|all] [--label <l>] [--assignee <user>] [--author <user>] [--milestone <title>|none] [--limit <n>] [--cursor <c>]" | |
| 168 | const usage = "usage: issue list <owner/name> [--state open|closed|all] [--label <l>] [--assignee <user>] [--author <user>] [--milestone <title>|none] [--search <text>] [--limit <n>] [--cursor <c>]" | |
| 169 | 169 | f := store.IssueFilter{State: "open"} |
| 170 | fl, err := parseFlags(args, flagSpec{Values: []string{"--state", "--label", "--assignee", "--author", "--milestone"}, MaxPos: 1, Usage: usage}) | |
| 170 | fl, err := parseFlags(args, flagSpec{Values: []string{"--state", "--label", "--assignee", "--author", "--milestone", "--search"}, MaxPos: 1, Usage: usage}) | |
| 171 | 171 | if err != nil { |
| 172 | 172 | return c.fail(protocol.ExitUsage, "%v", err) |
| 173 | 173 | } |
| @@ -176,6 +176,12 @@ func runIssueList(c *Ctx, args []string) int { | ||
| 176 | 176 | f.State = fl.Value("--state") |
| 177 | 177 | } |
| 178 | 178 | f.Label, f.Assignee, f.Author, f.Milestone = fl.Value("--label"), fl.Value("--assignee"), fl.Value("--author"), fl.Value("--milestone") |
| 179 | f.Search = fl.Value("--search") | |
| 180 | if fl.Has("--search") { | |
| 181 | if err := validQuery(f.Search); err != nil { | |
| 182 | return c.failErr(err) | |
| 183 | } | |
| 184 | } | |
| 179 | 185 | if path == "" || (f.State != "open" && f.State != "closed" && f.State != "all") { |
| 180 | 186 | return c.fail(protocol.ExitUsage, usage) |
| 181 | 187 | } |
internal/control/mr.go +9 −3
| @@ -41,7 +41,7 @@ func init() { | ||
| 41 | 41 | ReadsStdin: true, Run: runMRCreate}) |
| 42 | 42 | register(Command{Path: []string{"mr", "list"}, |
| 43 | 43 | Summary: "list merge requests", |
| 44 | Usage: "mr list <owner/name> [--state open|merged|closed|source_gone|all] [--author <user>] [--milestone <title>|none] [--limit <n>] [--cursor <c>]", ReadOnly: true, Run: runMRList}) | |
| 44 | Usage: "mr list <owner/name> [--state open|merged|closed|source_gone|all] [--author <user>] [--milestone <title>|none] [--search <text>] [--limit <n>] [--cursor <c>]", ReadOnly: true, Run: runMRList}) | |
| 45 | 45 | register(Command{Path: []string{"mr", "show"}, |
| 46 | 46 | Summary: "show a merge request", |
| 47 | 47 | Usage: "mr show <owner/name> <n>", ReadOnly: true, Run: runMRShow}) |
| @@ -388,9 +388,9 @@ func runMRList(c *Ctx, args []string) int { | ||
| 388 | 388 | if code >= 0 { |
| 389 | 389 | return code |
| 390 | 390 | } |
| 391 | const usage = "usage: mr list <owner/name> [--state open|merged|closed|source_gone|all] [--author <user>] [--milestone <title>|none] [--limit <n>] [--cursor <c>]" | |
| 391 | const usage = "usage: mr list <owner/name> [--state open|merged|closed|source_gone|all] [--author <user>] [--milestone <title>|none] [--search <text>] [--limit <n>] [--cursor <c>]" | |
| 392 | 392 | f := store.MRFilter{State: "open"} |
| 393 | fl, err := parseFlags(args, flagSpec{Values: []string{"--state", "--author", "--milestone"}, MaxPos: 1, Usage: usage}) | |
| 393 | fl, err := parseFlags(args, flagSpec{Values: []string{"--state", "--author", "--milestone", "--search"}, MaxPos: 1, Usage: usage}) | |
| 394 | 394 | if err != nil { |
| 395 | 395 | return c.fail(protocol.ExitUsage, "%v", err) |
| 396 | 396 | } |
| @@ -399,6 +399,12 @@ func runMRList(c *Ctx, args []string) int { | ||
| 399 | 399 | f.State = fl.Value("--state") |
| 400 | 400 | } |
| 401 | 401 | f.Author, f.Milestone = fl.Value("--author"), fl.Value("--milestone") |
| 402 | f.Search = fl.Value("--search") | |
| 403 | if fl.Has("--search") { | |
| 404 | if err := validQuery(f.Search); err != nil { | |
| 405 | return c.failErr(err) | |
| 406 | } | |
| 407 | } | |
| 402 | 408 | valid := map[string]bool{"open": true, "merged": true, "closed": true, "source_gone": true, "all": true} |
| 403 | 409 | if path == "" || !valid[f.State] { |
| 404 | 410 | return c.fail(protocol.ExitUsage, usage) |
internal/httpd/web.go +10 −4
| @@ -1584,7 +1584,8 @@ func (s *Server) issues(w http.ResponseWriter, r *http.Request) { | ||
| 1584 | 1584 | // label chips and author links point here. |
| 1585 | 1585 | qv := r.URL.Query() |
| 1586 | 1586 | f := store.IssueFilter{State: state, Label: qv.Get("label"), Assignee: qv.Get("assignee"), |
| 1587 | Author: qv.Get("author"), Milestone: qv.Get("milestone"), Limit: listPage + 1} | |
| 1587 | Author: qv.Get("author"), Milestone: qv.Get("milestone"), | |
| 1588 | Search: strings.TrimSpace(qv.Get("q")), Limit: listPage + 1} | |
| 1588 | 1589 | f.Before, _ = strconv.ParseInt(qv.Get("before"), 10, 64) |
| 1589 | 1590 | issues, err := s.st.QueryIssues(p.Repo.ID, f) |
| 1590 | 1591 | if err != nil { |
| @@ -1605,11 +1606,13 @@ func (s *Server) issues(w http.ResponseWriter, r *http.Request) { | ||
| 1605 | 1606 | repoPage |
| 1606 | 1607 | State string |
| 1607 | 1608 | Label string |
| 1609 | Query string | |
| 1608 | 1610 | Filters []listFilter |
| 1609 | 1611 | Issues []store.Issue |
| 1610 | 1612 | LabelColors map[string]template.CSS |
| 1611 | 1613 | Older string |
| 1612 | }{p, state, f.Label, activeFilters(state, [][2]string{{"label", f.Label}, {"assignee", f.Assignee}, {"author", f.Author}, {"milestone", f.Milestone}}), | |
| 1614 | }{p, state, f.Label, f.Search, | |
| 1615 | activeFilters(state, [][2]string{{"label", f.Label}, {"assignee", f.Assignee}, {"author", f.Author}, {"milestone", f.Milestone}}), | |
| 1613 | 1616 | issues, s.labelColors(p.Repo.ID), older}) |
| 1614 | 1617 | } |
| 1615 | 1618 | |
| @@ -1696,7 +1699,8 @@ func (s *Server) mrs(w http.ResponseWriter, r *http.Request) { | ||
| 1696 | 1699 | state = "open" |
| 1697 | 1700 | } |
| 1698 | 1701 | qv := r.URL.Query() |
| 1699 | mf := store.MRFilter{State: state, Author: qv.Get("author"), Milestone: qv.Get("milestone"), Limit: listPage + 1} | |
| 1702 | mf := store.MRFilter{State: state, Author: qv.Get("author"), Milestone: qv.Get("milestone"), | |
| 1703 | Search: strings.TrimSpace(qv.Get("q")), Limit: listPage + 1} | |
| 1700 | 1704 | mf.Before, _ = strconv.ParseInt(qv.Get("before"), 10, 64) |
| 1701 | 1705 | mrs, err := s.st.QueryMRs(p.Repo.ID, mf) |
| 1702 | 1706 | if err != nil { |
| @@ -1711,10 +1715,12 @@ func (s *Server) mrs(w http.ResponseWriter, r *http.Request) { | ||
| 1711 | 1715 | s.render(w, "mrs.html", struct { |
| 1712 | 1716 | repoPage |
| 1713 | 1717 | State string |
| 1718 | Query string | |
| 1714 | 1719 | Filters []listFilter |
| 1715 | 1720 | MRs []store.MR |
| 1716 | 1721 | Older string |
| 1717 | }{p, state, activeFilters(state, [][2]string{{"author", mf.Author}, {"milestone", mf.Milestone}}), mrs, older}) | |
| 1722 | }{p, state, mf.Search, | |
| 1723 | activeFilters(state, [][2]string{{"author", mf.Author}, {"milestone", mf.Milestone}}), mrs, older}) | |
| 1718 | 1724 | } |
| 1719 | 1725 | |
| 1720 | 1726 | func (s *Server) mr(w http.ResponseWriter, r *http.Request) { |
internal/store/fts.go added +27
| @@ -0,0 +1,27 @@ | ||
| 1 | package store | |
| 2 | ||
| 3 | import "strings" | |
| 4 | ||
| 5 | // FTSQuery turns what someone typed into an FTS5 MATCH expression. | |
| 6 | // | |
| 7 | // FTS5's query language is not free text: bare `-`, `*`, `:`, `(`, `NOT` | |
| 8 | // and an odd number of quotes are all syntax, and a syntax error surfaces | |
| 9 | // as a failed query rather than no results. Nobody searching for "c++" or | |
| 10 | // "foo: bar" means any of that. Every whitespace-separated run becomes a | |
| 11 | // quoted phrase, which FTS5 treats as a literal and joins with an | |
| 12 | // implicit AND, so the result is "rows containing all of these words". | |
| 13 | func FTSQuery(q string) string { | |
| 14 | var terms []string | |
| 15 | for _, f := range strings.Fields(q) { | |
| 16 | // A double quote inside a phrase is escaped by doubling it. | |
| 17 | f = strings.ReplaceAll(f, `"`, `""`) | |
| 18 | terms = append(terms, `"`+f+`"`) | |
| 19 | } | |
| 20 | if len(terms) == 0 { | |
| 21 | // Matches nothing, rather than being a syntax error. A caller | |
| 22 | // should not run a search for an empty query, but if one does the | |
| 23 | // answer is no rows, not a failure. | |
| 24 | return `""` | |
| 25 | } | |
| 26 | return strings.Join(terms, " ") | |
| 27 | } | |
internal/store/fts_test.go added +181
| @@ -0,0 +1,181 @@ | ||
| 1 | package store | |
| 2 | ||
| 3 | import ( | |
| 4 | "strings" | |
| 5 | "testing" | |
| 6 | ) | |
| 7 | ||
| 8 | func TestFTSQuerySanitises(t *testing.T) { | |
| 9 | cases := map[string]string{ | |
| 10 | "memory leak": `"memory" "leak"`, | |
| 11 | "c++": `"c++"`, | |
| 12 | // Bare FTS5 operators are terms here, not syntax. | |
| 13 | "foo AND": `"foo" "AND"`, | |
| 14 | "a NOT b": `"a" "NOT" "b"`, | |
| 15 | // A double quote inside a phrase is escaped by doubling it. | |
| 16 | `say "hi"`: `"say" """hi"""`, | |
| 17 | " spaced ": `"spaced"`, | |
| 18 | "": `""`, | |
| 19 | "trailing -": `"trailing" "-"`, | |
| 20 | "col:on": `"col:on"`, | |
| 21 | } | |
| 22 | for in, want := range cases { | |
| 23 | if got := FTSQuery(in); got != want { | |
| 24 | t.Errorf("FTSQuery(%q) = %q, want %q", in, got, want) | |
| 25 | } | |
| 26 | } | |
| 27 | } | |
| 28 | ||
| 29 | // Every one of these is a syntax error as a bare FTS5 expression. The | |
| 30 | // search must return no rows, not fail. | |
| 31 | func TestFTSQueryNeverErrors(t *testing.T) { | |
| 32 | s, repoID, _ := ftsFixture(t) | |
| 33 | for _, q := range []string{ | |
| 34 | "c++", `"`, `""`, "AND", "NOT", "*", "-", "(", "a AND", "foo:", "^", "a OR", | |
| 35 | } { | |
| 36 | if _, err := s.QueryIssues(repoID, IssueFilter{State: "all", Search: q}); err != nil { | |
| 37 | t.Errorf("search %q failed: %v", q, err) | |
| 38 | } | |
| 39 | } | |
| 40 | } | |
| 41 | ||
| 42 | func ftsFixture(t *testing.T) (*Store, int64, int64) { | |
| 43 | t.Helper() | |
| 44 | s := open(t) | |
| 45 | if err := s.MigrateUp(); err != nil { | |
| 46 | t.Fatal(err) | |
| 47 | } | |
| 48 | uid, err := s.CreateUser("cmc", true) | |
| 49 | if err != nil { | |
| 50 | t.Fatal(err) | |
| 51 | } | |
| 52 | repoID, err := s.CreateRepo("user", uid, "lib", "public") | |
| 53 | if err != nil { | |
| 54 | t.Fatal(err) | |
| 55 | } | |
| 56 | return s, repoID, uid | |
| 57 | } | |
| 58 | ||
| 59 | func searchNumbers(t *testing.T, s *Store, repoID int64, q string) []int64 { | |
| 60 | t.Helper() | |
| 61 | got, err := s.QueryIssues(repoID, IssueFilter{State: "all", Search: q}) | |
| 62 | if err != nil { | |
| 63 | t.Fatalf("search %q: %v", q, err) | |
| 64 | } | |
| 65 | var ns []int64 | |
| 66 | for _, i := range got { | |
| 67 | ns = append(ns, i.Number) | |
| 68 | } | |
| 69 | return ns | |
| 70 | } | |
| 71 | ||
| 72 | func TestIssueSearchMatchesTitleAndBody(t *testing.T) { | |
| 73 | s, repoID, uid := ftsFixture(t) | |
| 74 | if _, err := s.CreateIssue(repoID, uid, "memory leak in the parser", "it climbs forever", "md"); err != nil { | |
| 75 | t.Fatal(err) | |
| 76 | } | |
| 77 | if _, err := s.CreateIssue(repoID, uid, "unrelated", "nothing to see", "md"); err != nil { | |
| 78 | t.Fatal(err) | |
| 79 | } | |
| 80 | ||
| 81 | if got := searchNumbers(t, s, repoID, "parser"); len(got) != 1 || got[0] != 1 { | |
| 82 | t.Fatalf("title match = %v", got) | |
| 83 | } | |
| 84 | // The body is the half a LIKE over titles could never reach, which is | |
| 85 | // the whole point of #114. | |
| 86 | if got := searchNumbers(t, s, repoID, "climbs"); len(got) != 1 || got[0] != 1 { | |
| 87 | t.Fatalf("body match = %v", got) | |
| 88 | } | |
| 89 | // Terms are ANDed. | |
| 90 | if got := searchNumbers(t, s, repoID, "memory nothing"); len(got) != 0 { | |
| 91 | t.Fatalf("terms are not ANDed: %v", got) | |
| 92 | } | |
| 93 | if got := searchNumbers(t, s, repoID, "MEMORY"); len(got) != 1 { | |
| 94 | t.Fatalf("search is case sensitive: %v", got) | |
| 95 | } | |
| 96 | } | |
| 97 | ||
| 98 | // An external-content FTS table is not maintained automatically: an edit | |
| 99 | // or a delete leaves the old terms indexed unless the trigger removes | |
| 100 | // them first. Stale terms are invisible until someone searches for a word | |
| 101 | // that was deleted and gets a row that no longer says it. | |
| 102 | func TestIssueSearchFollowsEdits(t *testing.T) { | |
| 103 | s, repoID, uid := ftsFixture(t) | |
| 104 | n, err := s.CreateIssue(repoID, uid, "original title", "original body", "md") | |
| 105 | if err != nil { | |
| 106 | t.Fatal(err) | |
| 107 | } | |
| 108 | issue, err := s.IssueByNumber(repoID, n) | |
| 109 | if err != nil { | |
| 110 | t.Fatal(err) | |
| 111 | } | |
| 112 | ||
| 113 | if got := searchNumbers(t, s, repoID, "original"); len(got) != 1 { | |
| 114 | t.Fatalf("fresh issue not indexed: %v", got) | |
| 115 | } | |
| 116 | title, body := "replaced title", "replaced body" | |
| 117 | if err := s.UpdateIssueText(issue.ID, &title, &body, nil); err != nil { | |
| 118 | t.Fatal(err) | |
| 119 | } | |
| 120 | if got := searchNumbers(t, s, repoID, "original"); len(got) != 0 { | |
| 121 | t.Fatalf("edited-away terms still match: %v", got) | |
| 122 | } | |
| 123 | if got := searchNumbers(t, s, repoID, "replaced"); len(got) != 1 { | |
| 124 | t.Fatalf("new terms not indexed: %v", got) | |
| 125 | } | |
| 126 | ||
| 127 | // Deleting the repository cascades to its issues; their terms must go | |
| 128 | // with them rather than pointing at rows that no longer exist. | |
| 129 | if err := s.DeleteRepo(repoID); err != nil { | |
| 130 | t.Fatal(err) | |
| 131 | } | |
| 132 | var n2 int | |
| 133 | if err := s.DB.QueryRow("SELECT count(*) FROM issue_fts WHERE issue_fts MATCH 'replaced'").Scan(&n2); err != nil { | |
| 134 | t.Fatal(err) | |
| 135 | } | |
| 136 | if n2 != 0 { | |
| 137 | t.Fatalf("%d index rows survived the delete", n2) | |
| 138 | } | |
| 139 | } | |
| 140 | ||
| 141 | // The migration backfills what was already in the database, since the | |
| 142 | // triggers only see writes from their own creation onward. | |
| 143 | func TestFTSBackfillsExistingRows(t *testing.T) { | |
| 144 | s := open(t) | |
| 145 | // Stop one short of the FTS migration, write rows the triggers cannot | |
| 146 | // have seen, then apply it. | |
| 147 | if err := s.MigrateTo(35); err != nil { | |
| 148 | t.Fatal(err) | |
| 149 | } | |
| 150 | uid, err := s.CreateUser("cmc", true) | |
| 151 | if err != nil { | |
| 152 | t.Fatal(err) | |
| 153 | } | |
| 154 | repoID, err := s.CreateRepo("user", uid, "lib", "public") | |
| 155 | if err != nil { | |
| 156 | t.Fatal(err) | |
| 157 | } | |
| 158 | if _, err := s.CreateIssue(repoID, uid, "older than the index", "prose from before", "md"); err != nil { | |
| 159 | t.Fatal(err) | |
| 160 | } | |
| 161 | if err := s.MigrateUp(); err != nil { | |
| 162 | t.Fatal(err) | |
| 163 | } | |
| 164 | if got := searchNumbers(t, s, repoID, "prose"); len(got) != 1 { | |
| 165 | t.Fatalf("pre-existing issue not backfilled: %v", got) | |
| 166 | } | |
| 167 | } | |
| 168 | ||
| 169 | func TestGlobalSearchReachesBodies(t *testing.T) { | |
| 170 | s, repoID, uid := ftsFixture(t) | |
| 171 | if _, err := s.CreateIssue(repoID, uid, "a title", "haystack needle haystack", "md"); err != nil { | |
| 172 | t.Fatal(err) | |
| 173 | } | |
| 174 | got, err := s.SearchIssues(uid, "needle", 20) | |
| 175 | if err != nil { | |
| 176 | t.Fatal(err) | |
| 177 | } | |
| 178 | if len(got) != 1 || !strings.Contains(got[0].RepoPath, "lib") { | |
| 179 | t.Fatalf("global body search = %+v", got) | |
| 180 | } | |
| 181 | } | |
internal/store/issues.go +5
| @@ -108,6 +108,7 @@ type IssueFilter struct { | ||
| 108 | 108 | Assignee string |
| 109 | 109 | Author string |
| 110 | 110 | Milestone string |
| 111 | Search string // full-text over title and body | |
| 111 | 112 | Limit int |
| 112 | 113 | Before int64 |
| 113 | 114 | } |
| @@ -150,6 +151,10 @@ func (s *Store) QueryIssues(repoID int64, f IssueFilter) ([]Issue, error) { | ||
| 150 | 151 | q += " AND m.title = ?" |
| 151 | 152 | args = append(args, f.Milestone) |
| 152 | 153 | } |
| 154 | if f.Search != "" { | |
| 155 | q += " AND i.id IN (SELECT rowid FROM issue_fts WHERE issue_fts MATCH ?)" | |
| 156 | args = append(args, FTSQuery(f.Search)) | |
| 157 | } | |
| 153 | 158 | if f.Before > 0 { |
| 154 | 159 | q += " AND i.number < ?" |
| 155 | 160 | args = append(args, f.Before) |
internal/store/migrations/0036_issue_search.down.sql added +8
| @@ -0,0 +1,8 @@ | ||
| 1 | DROP TRIGGER mrs_fts_update; | |
| 2 | DROP TRIGGER mrs_fts_delete; | |
| 3 | DROP TRIGGER mrs_fts_insert; | |
| 4 | DROP TRIGGER issues_fts_update; | |
| 5 | DROP TRIGGER issues_fts_delete; | |
| 6 | DROP TRIGGER issues_fts_insert; | |
| 7 | DROP TABLE mr_fts; | |
| 8 | DROP TABLE issue_fts; | |
internal/store/migrations/0036_issue_search.up.sql added +51
| @@ -0,0 +1,51 @@ | ||
| 1 | -- Full-text search over issue and merge request prose. Finding an old | |
| 2 | -- issue meant listing and scrolling, and the instance-wide `search` | |
| 3 | -- matched titles only, because a LIKE with a leading wildcard cannot use | |
| 4 | -- an index (#114). | |
| 5 | -- | |
| 6 | -- External-content tables: the FTS index stores only the terms and points | |
| 7 | -- at the row it came from, so the prose is not duplicated. content_rowid | |
| 8 | -- ties a row to issues.id / merge_requests.id. | |
| 9 | CREATE VIRTUAL TABLE issue_fts USING fts5( | |
| 10 | title, body, | |
| 11 | content = 'issues', | |
| 12 | content_rowid = 'id', | |
| 13 | tokenize = 'unicode61' | |
| 14 | ); | |
| 15 | CREATE VIRTUAL TABLE mr_fts USING fts5( | |
| 16 | title, body, | |
| 17 | content = 'merge_requests', | |
| 18 | content_rowid = 'id', | |
| 19 | tokenize = 'unicode61' | |
| 20 | ); | |
| 21 | ||
| 22 | -- An external-content table is not maintained for you: every write to the | |
| 23 | -- base table has to be mirrored, and a delete or update must first insert | |
| 24 | -- the old values under the 'delete' command or the index keeps terms for | |
| 25 | -- prose that no longer exists. | |
| 26 | CREATE TRIGGER issues_fts_insert AFTER INSERT ON issues BEGIN | |
| 27 | INSERT INTO issue_fts (rowid, title, body) VALUES (new.id, new.title, new.body); | |
| 28 | END; | |
| 29 | CREATE TRIGGER issues_fts_delete AFTER DELETE ON issues BEGIN | |
| 30 | INSERT INTO issue_fts (issue_fts, rowid, title, body) VALUES ('delete', old.id, old.title, old.body); | |
| 31 | END; | |
| 32 | CREATE TRIGGER issues_fts_update AFTER UPDATE OF title, body ON issues BEGIN | |
| 33 | INSERT INTO issue_fts (issue_fts, rowid, title, body) VALUES ('delete', old.id, old.title, old.body); | |
| 34 | INSERT INTO issue_fts (rowid, title, body) VALUES (new.id, new.title, new.body); | |
| 35 | END; | |
| 36 | ||
| 37 | CREATE TRIGGER mrs_fts_insert AFTER INSERT ON merge_requests BEGIN | |
| 38 | INSERT INTO mr_fts (rowid, title, body) VALUES (new.id, new.title, new.body); | |
| 39 | END; | |
| 40 | CREATE TRIGGER mrs_fts_delete AFTER DELETE ON merge_requests BEGIN | |
| 41 | INSERT INTO mr_fts (mr_fts, rowid, title, body) VALUES ('delete', old.id, old.title, old.body); | |
| 42 | END; | |
| 43 | CREATE TRIGGER mrs_fts_update AFTER UPDATE OF title, body ON merge_requests BEGIN | |
| 44 | INSERT INTO mr_fts (mr_fts, rowid, title, body) VALUES ('delete', old.id, old.title, old.body); | |
| 45 | INSERT INTO mr_fts (rowid, title, body) VALUES (new.id, new.title, new.body); | |
| 46 | END; | |
| 47 | ||
| 48 | -- Everything already in the database, since the triggers only see writes | |
| 49 | -- from here on. | |
| 50 | INSERT INTO issue_fts (rowid, title, body) SELECT id, title, body FROM issues; | |
| 51 | INSERT INTO mr_fts (rowid, title, body) SELECT id, title, body FROM merge_requests; | |
internal/store/mrs.go +5
| @@ -102,6 +102,7 @@ type MRFilter struct { | ||
| 102 | 102 | State string |
| 103 | 103 | Author string |
| 104 | 104 | Milestone string |
| 105 | Search string // full-text over title and body | |
| 105 | 106 | Limit int |
| 106 | 107 | Before int64 |
| 107 | 108 | } |
| @@ -131,6 +132,10 @@ func (s *Store) QueryMRs(repoID int64, f MRFilter) ([]MR, error) { | ||
| 131 | 132 | q += " AND ms.title = ?" |
| 132 | 133 | args = append(args, f.Milestone) |
| 133 | 134 | } |
| 135 | if f.Search != "" { | |
| 136 | q += " AND m.id IN (SELECT rowid FROM mr_fts WHERE mr_fts MATCH ?)" | |
| 137 | args = append(args, FTSQuery(f.Search)) | |
| 138 | } | |
| 134 | 139 | if f.Before > 0 { |
| 135 | 140 | q += " AND m.number < ?" |
| 136 | 141 | args = append(args, f.Before) |
internal/store/search.go +8 −21
| @@ -27,19 +27,18 @@ func (s *Store) VisibleRepos(userID int64) ([]Repo, error) { | ||
| 27 | 27 | return out, rows.Err() |
| 28 | 28 | } |
| 29 | 29 | |
| 30 | // SearchIssues returns issues whose title contains q, newest activity | |
| 31 | // first. Titles only: body search is FTS work that belongs with the | |
| 32 | // per-repository issue search. | |
| 30 | // SearchIssues returns issues matching q in title or body, newest | |
| 31 | // activity first. | |
| 33 | 32 | func (s *Store) SearchIssues(userID int64, q string, limit int) ([]DashboardItem, error) { |
| 34 | return s.searchQuery("issues", userID, q, limit) | |
| 33 | return s.searchQuery("issues", "issue_fts", userID, q, limit) | |
| 35 | 34 | } |
| 36 | 35 | |
| 37 | 36 | // SearchMRs is SearchIssues for merge requests. |
| 38 | 37 | func (s *Store) SearchMRs(userID int64, q string, limit int) ([]DashboardItem, error) { |
| 39 | return s.searchQuery("merge_requests", userID, q, limit) | |
| 38 | return s.searchQuery("merge_requests", "mr_fts", userID, q, limit) | |
| 40 | 39 | } |
| 41 | 40 | |
| 42 | func (s *Store) searchQuery(table string, userID int64, q string, limit int) ([]DashboardItem, error) { | |
| 41 | func (s *Store) searchQuery(table, index string, userID int64, q string, limit int) ([]DashboardItem, error) { | |
| 43 | 42 | rows, err := s.DB.Query(` |
| 44 | 43 | SELECT COALESCE(u.username, o.name) || '/' || r.name, |
| 45 | 44 | x.number, x.title, au.username, x.state, x.updated_at |
| @@ -48,8 +47,9 @@ func (s *Store) searchQuery(table string, userID int64, q string, limit int) ([] | ||
| 48 | 47 | LEFT JOIN users u ON r.owner_kind = 'user' AND u.id = r.owner_id |
| 49 | 48 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id |
| 50 | 49 | JOIN users au ON au.id = x.author_id |
| 51 | WHERE x.title LIKE '%' || ?2 || '%' ESCAPE '\' AND `+visibleCond+` | |
| 52 | ORDER BY x.updated_at DESC LIMIT ?3`, userID, escapeLike(q), limit) | |
| 50 | WHERE x.id IN (SELECT rowid FROM `+index+` WHERE `+index+` MATCH ?2) | |
| 51 | AND `+visibleCond+` | |
| 52 | ORDER BY x.updated_at DESC LIMIT ?3`, userID, FTSQuery(q), limit) | |
| 53 | 53 | if err != nil { |
| 54 | 54 | return nil, err |
| 55 | 55 | } |
| @@ -64,16 +64,3 @@ func (s *Store) searchQuery(table string, userID int64, q string, limit int) ([] | ||
| 64 | 64 | } |
| 65 | 65 | return out, rows.Err() |
| 66 | 66 | } |
| 67 | ||
| 68 | // escapeLike neutralises the LIKE wildcards, so a query containing % or _ | |
| 69 | // matches those characters rather than everything. | |
| 70 | func escapeLike(q string) string { | |
| 71 | var b []byte | |
| 72 | for i := 0; i < len(q); i++ { | |
| 73 | if c := q[i]; c == '%' || c == '_' || c == '\\' { | |
| 74 | b = append(b, '\\') | |
| 75 | } | |
| 76 | b = append(b, q[i]) | |
| 77 | } | |
| 78 | return string(b) | |
| 79 | } | |
internal/web/templates/globalsearch.html +2 −2
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | </nav> |
| 11 | 11 | </div> |
| 12 | 12 | <form method="get" action="/search" class="searchform"> |
| 13 | <input type="search" name="q" aria-label="Search" value="{{.Query}}" placeholder="repository names, topics, issue and merge request titles" autofocus> | |
| 13 | <input type="search" name="q" aria-label="Search" value="{{.Query}}" placeholder="repository names and topics, issue and merge request text" autofocus> | |
| 14 | 14 | {{if .Kind}}<input type="hidden" name="kind" value="{{.Kind}}">{{end}} |
| 15 | 15 | <button type="submit">search</button> |
| 16 | 16 | </form> |
| @@ -29,6 +29,6 @@ | ||
| 29 | 29 | </ul> |
| 30 | 30 | {{else}}<p class="empty-note">no matches for “{{.Query}}”</p>{{end}} |
| 31 | 31 | {{else}} |
| 32 | <p class="empty-note">Repository names, descriptions and topics, and issue and merge request titles. File contents are searched per repository, from a repository's Code tab.</p> | |
| 32 | <p class="empty-note">Repository names, descriptions and topics, and the title and body of every issue and merge request you can read. File contents are searched per repository, from a repository's Code tab.</p> | |
| 33 | 33 | {{end}} |
| 34 | 34 | {{end}} |
internal/web/templates/issues.html +6 −1
| @@ -8,6 +8,10 @@ | ||
| 8 | 8 | <a {{if eq .State "all"}}class="active" aria-current="page" {{end}}href="?state=all">all</a> |
| 9 | 9 | </nav> |
| 10 | 10 | {{range .Filters}}<p class="meta">{{.Key}}: {{if eq .Key "label"}}<span class="chip label" style="{{index $.LabelColors .Value}}">{{.Value}}</span>{{else}}<b>{{.Value}}</b>{{end}} <a href="{{.Clear}}">clear</a></p>{{end}} |
| 11 | <form method="get" class="searchform compact"> | |
| 12 | <input type="search" name="q" aria-label="Search issues" value="{{.Query}}" placeholder="search title and body"> | |
| 13 | <input type="hidden" name="state" value="{{.State}}"> | |
| 14 | </form> | |
| 11 | 15 | <span class="spacer"></span> |
| 12 | 16 | <p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/milestones">milestones</a>{{if .Viewer}} · <a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/issues/new">new issue</a>{{end}}</p> |
| 13 | 17 | </div> |
| @@ -20,7 +24,8 @@ | ||
| 20 | 24 | </div> |
| 21 | 25 | <span class="chip {{if eq .State "open"}}chip-open{{else}}chip-done{{end}}">{{.State}}</span> |
| 22 | 26 | </li> |
| 23 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}issues — open one with <code>gitbay issue create {{.Repo.OwnerName}}/{{.Repo.Name}} --title "..."</code></li>{{end}} | |
| 27 | {{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}issues matching “{{.Query}}”</li> | |
| 28 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}issues — open one with <code>gitbay issue create {{.Repo.OwnerName}}/{{.Repo.Name}} --title "..."</code></li>{{end}}{{end}} | |
| 24 | 29 | </ul> |
| 25 | 30 | {{if .Older}}<p class="pager"><a href="{{.Older}}">older →</a></p>{{end}} |
| 26 | 31 | {{end}} |
internal/web/templates/mrs.html +6 −1
| @@ -8,6 +8,10 @@ | ||
| 8 | 8 | <a {{if eq .State "closed"}}class="active" aria-current="page" {{end}}href="?state=closed">closed</a> |
| 9 | 9 | <a {{if eq .State "all"}}class="active" aria-current="page" {{end}}href="?state=all">all</a> |
| 10 | 10 | </nav> |
| 11 | <form method="get" class="searchform compact"> | |
| 12 | <input type="search" name="q" aria-label="Search merge requests" value="{{.Query}}" placeholder="search title and body"> | |
| 13 | <input type="hidden" name="state" value="{{.State}}"> | |
| 14 | </form> | |
| 11 | 15 | {{range .Filters}}<p class="meta">{{.Key}}: <b>{{.Value}}</b> <a href="{{.Clear}}">clear</a></p>{{end}} |
| 12 | 16 | </div> |
| 13 | 17 | {{if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p>{{end}} |
| @@ -19,7 +23,8 @@ | ||
| 19 | 23 | </div> |
| 20 | 24 | <span class="chip chip-{{.State}}">{{.State}}</span> |
| 21 | 25 | </li> |
| 22 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code></li>{{end}} | |
| 26 | {{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”</li> | |
| 27 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code></li>{{end}}{{end}} | |
| 23 | 28 | </ul> |
| 24 | 29 | {{if .Older}}<p class="pager"><a href="{{.Older}}">older →</a></p>{{end}} |
| 25 | 30 | {{end}} |