Commit d5176c7dfc
Verified · cmc
Layout: unified · split
internal/control/query.go +35 −2
| @@ -70,6 +70,13 @@ const queryDefaultLimit = 50 | ||
| 70 | 70 | // dashboard carries. |
| 71 | 71 | const dashboardQueryItems = 5 |
| 72 | 72 | |
| 73 | // Every pinned query is run on each dashboard read, so an account keeps | |
| 74 | // a bounded number of each. | |
| 75 | const ( | |
| 76 | maxSavedQueries = 50 | |
| 77 | maxPinnedQueries = 10 | |
| 78 | ) | |
| 79 | ||
| 73 | 80 | var queryNamePat = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]{0,63}$`) |
| 74 | 81 | |
| 75 | 82 | // SavedQueryOut is one saved query. Count is filled by query show. |
| @@ -289,6 +296,17 @@ func runQuerySave(c *Ctx, args []string) int { | ||
| 289 | 296 | if err != nil { |
| 290 | 297 | return c.fail(protocol.ExitUsage, "%v", err) |
| 291 | 298 | } |
| 299 | if _, err := c.Store.SavedQueryByName(c.User.ID, name); errors.Is(err, store.ErrNotFound) { | |
| 300 | saved, _, err := c.Store.CountSavedQueries(c.User.ID) | |
| 301 | if err != nil { | |
| 302 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 303 | } | |
| 304 | if saved >= maxSavedQueries { | |
| 305 | return c.fail(protocol.ExitUsage, "saved query limit reached (%d); remove one first", maxSavedQueries) | |
| 306 | } | |
| 307 | } else if err != nil { | |
| 308 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 309 | } | |
| 292 | 310 | err = c.Store.SaveQuery(c.User.ID, name, q.String(), fl.Has("--force")) |
| 293 | 311 | if errors.Is(err, store.ErrExists) { |
| 294 | 312 | return c.fail(protocol.ExitFailure, "you already have a query named %s; pass --force to replace it", name) |
| @@ -390,17 +408,32 @@ func runQueryPin(c *Ctx, args []string, pinned bool) int { | ||
| 390 | 408 | if len(args) != 1 { |
| 391 | 409 | return c.usage() |
| 392 | 410 | } |
| 393 | err := c.Store.PinSavedQuery(c.User.ID, args[0], pinned) | |
| 411 | sq, err := c.Store.SavedQueryByName(c.User.ID, args[0]) | |
| 394 | 412 | if errors.Is(err, store.ErrNotFound) { |
| 395 | 413 | return c.fail(protocol.ExitNotFound, "no saved query %q; query list shows yours", args[0]) |
| 396 | 414 | } |
| 397 | 415 | if err != nil { |
| 398 | 416 | return c.fail(protocol.ExitFailure, "%v", err) |
| 399 | 417 | } |
| 400 | sq, err := c.Store.SavedQueryByName(c.User.ID, args[0]) | |
| 418 | if pinned && !sq.Pinned { | |
| 419 | _, n, err := c.Store.CountSavedQueries(c.User.ID) | |
| 420 | if err != nil { | |
| 421 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 422 | } | |
| 423 | if n >= maxPinnedQueries { | |
| 424 | return c.fail(protocol.ExitUsage, "pinned query limit reached (%d); unpin one first", maxPinnedQueries) | |
| 425 | } | |
| 426 | } | |
| 427 | err = c.Store.PinSavedQuery(c.User.ID, args[0], pinned) | |
| 428 | if errors.Is(err, store.ErrNotFound) { | |
| 429 | return c.fail(protocol.ExitNotFound, "no saved query %q; query list shows yours", args[0]) | |
| 430 | } | |
| 401 | 431 | if err != nil { |
| 402 | 432 | return c.fail(protocol.ExitFailure, "%v", err) |
| 403 | 433 | } |
| 434 | if sq, err = c.Store.SavedQueryByName(c.User.ID, args[0]); err != nil { | |
| 435 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 436 | } | |
| 404 | 437 | return c.emit(savedQueryOut(sq), func(w io.Writer) { |
| 405 | 438 | verb := "unpinned" |
| 406 | 439 | if pinned { |
internal/control/query_test.go +37 −1
| @@ -3,6 +3,7 @@ package control | ||
| 3 | 3 | import ( |
| 4 | 4 | "bytes" |
| 5 | 5 | "encoding/json" |
| 6 | "fmt" | |
| 6 | 7 | "strings" |
| 7 | 8 | "testing" |
| 8 | 9 | |
| @@ -41,7 +42,7 @@ func newQueryEnv(t *testing.T) queryEnv { | ||
| 41 | 42 | // the envelope's data and stderr. |
| 42 | 43 | func (e queryEnv) run(user string, argv ...string) (int, json.RawMessage, string) { |
| 43 | 44 | var out, errOut bytes.Buffer |
| 44 | c := &Ctx{User: e.users[user], Scope: "full", Store: e.st, Cfg: config.Config{}, | |
| 45 | c := &Ctx{User: e.users[user], Scope: "full", Store: e.st, Cfg: config.Config{Limits: config.Limits{WriteRate: -1}}, | |
| 45 | 46 | Stdin: strings.NewReader(""), Stdout: &out, Stderr: &errOut} |
| 46 | 47 | code := Dispatch(c, append(argv, "--json")) |
| 47 | 48 | var env struct { |
| @@ -220,3 +221,38 @@ func TestQueryCommands(t *testing.T) { | ||
| 220 | 221 | t.Errorf("bob lost his query when alice removed hers") |
| 221 | 222 | } |
| 222 | 223 | } |
| 224 | ||
| 225 | // An account keeps at most maxSavedQueries, maxPinnedQueries of them | |
| 226 | // pinned; replacing or re-pinning one it has is not a new one. | |
| 227 | func TestQueryCaps(t *testing.T) { | |
| 228 | e := newQueryEnv(t) | |
| 229 | for i := 0; i < maxSavedQueries; i++ { | |
| 230 | if code, _, msg := e.run("alice", "query", "save", fmt.Sprintf("q%d", i), "is:open"); code != 0 { | |
| 231 | t.Fatalf("save %d: exit %d %s", i, code, msg) | |
| 232 | } | |
| 233 | } | |
| 234 | if code, _, msg := e.run("alice", "query", "save", "one-more", "is:open"); code != protocol.ExitUsage || !strings.Contains(msg, "limit") { | |
| 235 | t.Fatalf("save past the cap: exit %d %s", code, msg) | |
| 236 | } | |
| 237 | if code, _, msg := e.run("alice", "query", "save", "q0", "is:closed", "--force"); code != 0 { | |
| 238 | t.Fatalf("replace at the cap: exit %d %s", code, msg) | |
| 239 | } | |
| 240 | if code, _, _ := e.run("bob", "query", "save", "mine", "is:open"); code != 0 { | |
| 241 | t.Fatal("the cap is per account") | |
| 242 | } | |
| 243 | for i := 0; i < maxPinnedQueries; i++ { | |
| 244 | if code, _, msg := e.run("alice", "query", "pin", fmt.Sprintf("q%d", i)); code != 0 { | |
| 245 | t.Fatalf("pin %d: exit %d %s", i, code, msg) | |
| 246 | } | |
| 247 | } | |
| 248 | if code, _, msg := e.run("alice", "query", "pin", "q0"); code != 0 { | |
| 249 | t.Fatalf("re-pin at the cap: exit %d %s", code, msg) | |
| 250 | } | |
| 251 | if code, _, msg := e.run("alice", "query", "pin", fmt.Sprintf("q%d", maxPinnedQueries)); code != protocol.ExitUsage || !strings.Contains(msg, "limit") { | |
| 252 | t.Fatalf("pin past the cap: exit %d %s", code, msg) | |
| 253 | } | |
| 254 | e.run("alice", "query", "unpin", "q0") | |
| 255 | if code, _, msg := e.run("alice", "query", "pin", fmt.Sprintf("q%d", maxPinnedQueries)); code != 0 { | |
| 256 | t.Fatalf("pin after an unpin: exit %d %s", code, msg) | |
| 257 | } | |
| 258 | } | |
internal/control/querylang.go +6 −1
| @@ -200,12 +200,17 @@ func (q *ItemQuery) term(tok string, quoted bool) error { | ||
| 200 | 200 | if q.Kind == "mr" { |
| 201 | 201 | return bad("merge requests have no assignees") |
| 202 | 202 | } |
| 203 | return user(&q.Assignee, "assignee") | |
| 203 | if err := user(&q.Assignee, "assignee"); err != nil { | |
| 204 | return err | |
| 205 | } | |
| 204 | 206 | case "author": |
| 205 | 207 | return user(&q.Author, "author") |
| 206 | 208 | default: |
| 207 | 209 | return bad("unknown qualifier; the qualifiers are repo:, owner:, is:, label:, no:, milestone:, assignee: and author:") |
| 208 | 210 | } |
| 211 | if issues, mrs := q.Selects(); !issues && !mrs { | |
| 212 | return bad("is:merged and assignee: never both match: only merge requests merge, and they have no assignees") | |
| 213 | } | |
| 209 | 214 | return nil |
| 210 | 215 | } |
| 211 | 216 | |
internal/control/querylang_test.go +2
| @@ -59,6 +59,8 @@ func TestParseItemQueryRefuses(t *testing.T) { | ||
| 59 | 59 | {"is:mr assignee:cmc", "assignee:cmc"}, |
| 60 | 60 | {"assignee:cmc is:mr", "is:mr"}, |
| 61 | 61 | {"assignee:cmc assignee:bob", "assignee:bob"}, |
| 62 | {"is:merged assignee:cmc", "assignee:cmc"}, | |
| 63 | {"assignee:@me is:merged", "is:merged"}, | |
| 62 | 64 | {"author:Not_A_User", "author:Not_A_User"}, |
| 63 | 65 | {"repo:krz", "repo:krz"}, |
| 64 | 66 | {"repo:*/gitbay", "repo:*/gitbay"}, |
internal/store/migrations/0071_saved_queries.down.sql +2
| @@ -1 +1,3 @@ | ||
| 1 | DROP INDEX merge_requests_repo_created; | |
| 2 | DROP INDEX issues_repo_created; | |
| 1 | 3 | DROP TABLE saved_queries; |
internal/store/migrations/0071_saved_queries.up.sql +4
| @@ -11,3 +11,7 @@ CREATE TABLE saved_queries ( | ||
| 11 | 11 | updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), |
| 12 | 12 | UNIQUE (user_id, name) |
| 13 | 13 | ); |
| 14 | ||
| 15 | -- A query reads each table by the repositories it may see, newest first. | |
| 16 | CREATE INDEX issues_repo_created ON issues(repo_id, created_at); | |
| 17 | CREATE INDEX merge_requests_repo_created ON merge_requests(repo_id, created_at); | |
internal/store/savedqueries.go +35 −16
| @@ -68,6 +68,14 @@ func (s *Store) SavedQueries(userID int64, pinnedOnly bool) ([]SavedQuery, error | ||
| 68 | 68 | return out, rows.Err() |
| 69 | 69 | } |
| 70 | 70 | |
| 71 | // CountSavedQueries is how many queries the user has saved, and how many | |
| 72 | // of them are pinned. | |
| 73 | func (s *Store) CountSavedQueries(userID int64) (saved, pinned int, err error) { | |
| 74 | err = s.DB.QueryRow("SELECT COUNT(*), COALESCE(SUM(pinned), 0) FROM saved_queries WHERE user_id = ?", userID). | |
| 75 | Scan(&saved, &pinned) | |
| 76 | return | |
| 77 | } | |
| 78 | ||
| 71 | 79 | func (s *Store) RemoveSavedQuery(userID int64, name string) error { |
| 72 | 80 | res, err := s.DB.Exec("DELETE FROM saved_queries WHERE user_id = ? AND name = ?", userID, name) |
| 73 | 81 | if err != nil { |
| @@ -113,8 +121,8 @@ type ItemFilter struct { | ||
| 113 | 121 | } |
| 114 | 122 | |
| 115 | 123 | // ItemCursor is the sort key of the last row of a page: rows are newest |
| 116 | // first by creation, issues before merge requests at the same instant, | |
| 117 | // then by id. | |
| 124 | // first by creation, merge requests before issues at the same instant, | |
| 125 | // then by id, highest first. | |
| 118 | 126 | type ItemCursor struct { |
| 119 | 127 | CreatedAt string |
| 120 | 128 | Kind int // 0 issue, 1 merge request |
| @@ -154,18 +162,12 @@ func (n *numbered) add(v any) string { | ||
| 154 | 162 | return fmt.Sprintf("?%d", len(n.args)) |
| 155 | 163 | } |
| 156 | 164 | |
| 157 | // itemBranch is one table's half of the query: every row of it on a | |
| 158 | // repository the user may read (visibleCond, public or reached) that f | |
| 159 | // admits. userID is ?1 in a. | |
| 160 | func itemBranch(kind string, f ItemFilter, a *numbered, after *ItemCursor) string { | |
| 161 | table, kord, labels, draft := "issues", "0", "issue_labels il", "0" | |
| 162 | onItem := "il.issue_id = x.id" | |
| 163 | if kind == "mr" { | |
| 164 | table, kord, labels, draft = "merge_requests", "1", "mr_labels il", "x.draft" | |
| 165 | onItem = "il.mr_id = x.id" | |
| 166 | } | |
| 167 | var where []string | |
| 168 | where = append(where, visibleCond) | |
| 165 | // itemRepos is the subquery naming the repositories a query reads: those | |
| 166 | // the user may read (visibleCond, public or reached) within f's scopes. | |
| 167 | // Deciding readability per repository rather than per row lets each | |
| 168 | // table be read through its repo_id index. userID is ?1 in a. | |
| 169 | func itemRepos(f ItemFilter, a *numbered) string { | |
| 170 | where := []string{visibleCond} | |
| 169 | 171 | if len(f.Scopes) > 0 { |
| 170 | 172 | var scopes []string |
| 171 | 173 | for _, sc := range f.Scopes { |
| @@ -177,6 +179,22 @@ func itemBranch(kind string, f ItemFilter, a *numbered, after *ItemCursor) strin | ||
| 177 | 179 | } |
| 178 | 180 | where = append(where, "("+strings.Join(scopes, " OR ")+")") |
| 179 | 181 | } |
| 182 | return `SELECT r.id FROM repos r | |
| 183 | LEFT JOIN users u ON r.owner_kind = 'user' AND u.id = r.owner_id | |
| 184 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id | |
| 185 | WHERE ` + strings.Join(where, " AND ") | |
| 186 | } | |
| 187 | ||
| 188 | // itemBranch is one table's half of the query: every row of it in one of | |
| 189 | // the repositories repos names that f admits. | |
| 190 | func itemBranch(kind string, f ItemFilter, a *numbered, repos string, after *ItemCursor) string { | |
| 191 | table, kord, labels, draft := "issues", "0", "issue_labels il", "0" | |
| 192 | onItem := "il.issue_id = x.id" | |
| 193 | if kind == "mr" { | |
| 194 | table, kord, labels, draft = "merge_requests", "1", "mr_labels il", "x.draft" | |
| 195 | onItem = "il.mr_id = x.id" | |
| 196 | } | |
| 197 | where := []string{"x.repo_id IN (" + repos + ")"} | |
| 180 | 198 | switch { |
| 181 | 199 | case f.State == "": |
| 182 | 200 | case f.State == "open" && kind == "mr": |
| @@ -229,12 +247,13 @@ func itemBranch(kind string, f ItemFilter, a *numbered, after *ItemCursor) strin | ||
| 229 | 247 | func itemUnion(userID int64, f ItemFilter, after *ItemCursor) (string, []any) { |
| 230 | 248 | a := &numbered{} |
| 231 | 249 | a.add(userID) |
| 250 | repos := itemRepos(f, a) | |
| 232 | 251 | var parts []string |
| 233 | 252 | if f.Issues { |
| 234 | parts = append(parts, itemBranch("issue", f, a, after)) | |
| 253 | parts = append(parts, itemBranch("issue", f, a, repos, after)) | |
| 235 | 254 | } |
| 236 | 255 | if f.MRs { |
| 237 | parts = append(parts, itemBranch("mr", f, a, after)) | |
| 256 | parts = append(parts, itemBranch("mr", f, a, repos, after)) | |
| 238 | 257 | } |
| 239 | 258 | return strings.Join(parts, "\nUNION ALL\n"), a.args |
| 240 | 259 | } |