diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000..29e1f60 Binary files /dev/null and b/.DS_Store differ diff --git a/.golangci.yml b/.golangci.yml index 16af52e..0a79e00 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -23,6 +23,17 @@ linters: # cancelled is a GitHub check-run conclusion value (their spelling, not ours). ignore-rules: - cancelled + exclusions: + rules: + # gqlgen owns the resolver method signatures: their argument names come + # from the schema field arguments, so a resolver that does not use one + # (ctx, after, dryRun, ...) cannot rename it without the next regenerate + # reverting the change. Do not flag unused parameters in generated + # resolver files. + - path: \.resolvers\.go$ + linters: + - revive + text: unused-parameter formatters: enable: diff --git a/api/graphql/connection.go b/api/graphql/connection.go index 7a663ba..8c37d02 100644 --- a/api/graphql/connection.go +++ b/api/graphql/connection.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/99designs/gqlgen/graphql" + "github.com/tamnd/githome/domain" "github.com/tamnd/githome/presenter/gqlmodel" ) diff --git a/api/graphql/errors.go b/api/graphql/errors.go index a4de89e..9b78747 100644 --- a/api/graphql/errors.go +++ b/api/graphql/errors.go @@ -104,7 +104,7 @@ func liftErrorTypes(next http.Handler) http.Handler { } // The cost walk filled the rate-limit holder during execution; write its // headers here, the last point that holds the header map before flush. - applyRateLimitHeaders(w.Header(), r.Context()) + applyRateLimitHeaders(r.Context(), w.Header()) w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.WriteHeader(rec.status) _, _ = w.Write(body) diff --git a/api/graphql/helpers.go b/api/graphql/helpers.go index 4ca48b3..e0a64f8 100644 --- a/api/graphql/helpers.go +++ b/api/graphql/helpers.go @@ -8,8 +8,8 @@ package graphql import ( "context" "errors" - "sort" "regexp" + "sort" "strconv" "strings" diff --git a/api/graphql/loaders.go b/api/graphql/loaders.go index 2a9a658..2a3afbf 100644 --- a/api/graphql/loaders.go +++ b/api/graphql/loaders.go @@ -42,8 +42,10 @@ type Loaders struct { // newLoaders constructs a fresh Loaders set for one request. batch provides // the underlying batch-store methods; urls and format render domain types into -// the gqlmodel shapes the resolvers return. -func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid.Format) *Loaders { +// the gqlmodel shapes the resolvers return. wait is the batch coalescing window +// every loader shares; production passes loaderWait, tests widen it so a slow, +// loaded runner cannot break up a single logical wave into two fetches. +func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid.Format, wait time.Duration) *Loaders { return &Loaders{ Users: dataloader.New(func(ctx context.Context, pks []int64) (map[int64]*gqlmodel.User, error) { users, err := batch.Users(ctx, pks) @@ -55,7 +57,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid out[pk] = urls.GQLUser(u, format) } return out, nil - }, loaderWait), + }, wait), LabelsByIssue: dataloader.New(func(ctx context.Context, issuePKs []int64) (map[int64][]*gqlmodel.Label, error) { lmap, err := batch.LabelsByIssues(ctx, issuePKs) @@ -76,7 +78,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid out[pk] = nodes } return out, nil - }, loaderWait), + }, wait), AssigneesByIssue: dataloader.New(func(ctx context.Context, issuePKs []int64) (map[int64][]*gqlmodel.User, error) { amap, err := batch.AssigneesByIssues(ctx, issuePKs) @@ -92,7 +94,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid out[pk] = users } return out, nil - }, loaderWait), + }, wait), CommentsByIssue: dataloader.New(func(ctx context.Context, keys []commentsPreviewKey) (map[commentsPreviewKey][]*domain.Comment, error) { // Group by limit so each distinct first: argument is one query. @@ -111,7 +113,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid } } return out, nil - }, loaderWait), + }, wait), } } @@ -133,7 +135,7 @@ func loadersFrom(ctx context.Context) *Loaders { // middleware stored on the context, falling back to the configured default. func loadersMiddleware(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid.Format, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := withLoaders(r.Context(), newLoaders(batch, urls, idFormat(r.Context(), format))) + ctx := withLoaders(r.Context(), newLoaders(batch, urls, idFormat(r.Context(), format), loaderWait)) next.ServeHTTP(w, r.WithContext(ctx)) }) } diff --git a/api/graphql/loaders_test.go b/api/graphql/loaders_test.go index 9eea6b2..cd00e62 100644 --- a/api/graphql/loaders_test.go +++ b/api/graphql/loaders_test.go @@ -27,7 +27,7 @@ type countingBatchStore struct { rollupCalls atomic.Int64 } -func (c *countingBatchStore) UsersByPKs(ctx context.Context, pks []int64) (map[int64]*store.UserRow, error) { +func (c *countingBatchStore) UsersByPKs(_ context.Context, pks []int64) (map[int64]*store.UserRow, error) { c.userCalls.Add(1) out := make(map[int64]*store.UserRow, len(pks)) for _, pk := range pks { @@ -36,7 +36,7 @@ func (c *countingBatchStore) UsersByPKs(ctx context.Context, pks []int64) (map[i return out, nil } -func (c *countingBatchStore) LabelsByIssuePKs(ctx context.Context, pks []int64) (map[int64][]store.LabelRow, error) { +func (c *countingBatchStore) LabelsByIssuePKs(_ context.Context, pks []int64) (map[int64][]store.LabelRow, error) { c.labelCalls.Add(1) out := make(map[int64][]store.LabelRow, len(pks)) for _, pk := range pks { @@ -45,7 +45,7 @@ func (c *countingBatchStore) LabelsByIssuePKs(ctx context.Context, pks []int64) return out, nil } -func (c *countingBatchStore) AssigneesByIssuePKs(ctx context.Context, pks []int64) (map[int64][]int64, error) { +func (c *countingBatchStore) AssigneesByIssuePKs(_ context.Context, pks []int64) (map[int64][]int64, error) { c.assigneeCalls.Add(1) out := make(map[int64][]int64, len(pks)) for _, pk := range pks { @@ -54,16 +54,16 @@ func (c *countingBatchStore) AssigneesByIssuePKs(ctx context.Context, pks []int6 return out, nil } -func (c *countingBatchStore) MilestonesByPKs(ctx context.Context, pks []int64) (map[int64]*store.MilestoneRow, error) { +func (c *countingBatchStore) MilestonesByPKs(_ context.Context, _ []int64) (map[int64]*store.MilestoneRow, error) { return map[int64]*store.MilestoneRow{}, nil } -func (c *countingBatchStore) ReactionRollupsBySubjectPKs(ctx context.Context, subjectType string, pks []int64) (map[int64]store.ReactionRollup, error) { +func (c *countingBatchStore) ReactionRollupsBySubjectPKs(_ context.Context, _ string, _ []int64) (map[int64]store.ReactionRollup, error) { c.rollupCalls.Add(1) return map[int64]store.ReactionRollup{}, nil } -func (c *countingBatchStore) CommentsByIssuePKs(ctx context.Context, issuePKs []int64, perIssue int) (map[int64][]store.CommentRow, error) { +func (c *countingBatchStore) CommentsByIssuePKs(_ context.Context, issuePKs []int64, perIssue int) (map[int64][]store.CommentRow, error) { c.commentCalls.Add(1) now := time.Now() out := make(map[int64][]store.CommentRow, len(issuePKs)) @@ -103,7 +103,12 @@ func testLoaders(t *testing.T, cs *countingBatchStore) *Loaders { SSHHost: "git.test.internal", SSHPort: 22, } - return newLoaders(domain.NewBatcher(cs), presenter.NewURLBuilder(urls), nodeid.FormatNew) + // The batch tests fan out N goroutines and assert they collapse into one + // fetch. A loaded -race runner can take longer than the 1ms production + // window to schedule all N, splitting the wave into two fetches and failing + // the count. A wide window removes that race without changing what is tested: + // the coalescing behavior is identical, only the deadline is generous. + return newLoaders(domain.NewBatcher(cs), presenter.NewURLBuilder(urls), nodeid.FormatNew, 250*time.Millisecond) } // TestLoaderCommentsPreviewBatches proves the fan-out shape an issue diff --git a/api/graphql/nodecount.go b/api/graphql/nodecount.go index 9ef6ef4..9c93878 100644 --- a/api/graphql/nodecount.go +++ b/api/graphql/nodecount.go @@ -182,7 +182,7 @@ func (n nodeLimit) MutateOperationContext(_ context.Context, opCtx *gqlruntime.O } // nodeLimitExtension returns a gqlgen extension that rejects queries requesting -// more than max nodes. -func nodeLimitExtension(max int) gqlruntime.HandlerExtension { - return nodeLimit{max: max} +// more than limit nodes. +func nodeLimitExtension(limit int) gqlruntime.HandlerExtension { + return nodeLimit{max: limit} } diff --git a/api/graphql/pull.resolvers.go b/api/graphql/pull.resolvers.go index a0f9ee3..d88f513 100644 --- a/api/graphql/pull.resolvers.go +++ b/api/graphql/pull.resolvers.go @@ -483,11 +483,11 @@ func (r *pullRequestResolver) MergeCommit(ctx context.Context, obj *gqlmodel.Pul if !obj.Merged || obj.MergeCommitOID == "" { return nil, nil } - repo, err := r.Resolver.Repos.GetRepo(ctx, viewerID(ctx), obj.RepoOwner, obj.RepoName) + repo, err := r.Repos.GetRepo(ctx, viewerID(ctx), obj.RepoOwner, obj.RepoName) if err != nil { return nil, mapErr(err) } - c, err := r.Resolver.Repos.GetCommit(repo, obj.MergeCommitOID) + c, err := r.Repos.GetCommit(repo, obj.MergeCommitOID) if err != nil { return nil, nil } @@ -514,7 +514,7 @@ func (r *pullRequestResolver) ClosingIssuesReferences(ctx context.Context, obj * viewer := viewerID(ctx) issues := make([]*domain.Issue, 0) for _, n := range closingIssueNumbers(obj.Body) { - iss, err := r.Resolver.Issues.GetIssue(ctx, viewer, obj.RepoOwner, obj.RepoName, n) + iss, err := r.Issues.GetIssue(ctx, viewer, obj.RepoOwner, obj.RepoName, n) if err != nil { continue } diff --git a/api/graphql/ratelimit.go b/api/graphql/ratelimit.go index 8fc78b6..e126854 100644 --- a/api/graphql/ratelimit.go +++ b/api/graphql/ratelimit.go @@ -87,7 +87,7 @@ func rateLimitHeaders(next http.Handler) http.Handler { // (rejected before an operation context existed, or with no holder installed) // reports the one-point minimum against the full budget, matching a cost-free // call. It must be called before the response head is flushed. -func applyRateLimitHeaders(h http.Header, ctx context.Context) { +func applyRateLimitHeaders(ctx context.Context, h http.Header) { used, nodes := 1, 0 if info := rateLimitInfoFrom(ctx); info != nil && info.filled { used = info.used diff --git a/api/graphql/ref.resolvers.go b/api/graphql/ref.resolvers.go index 5030445..d46e89b 100644 --- a/api/graphql/ref.resolvers.go +++ b/api/graphql/ref.resolvers.go @@ -77,7 +77,7 @@ func (r *mutationResolver) UpdateRef(ctx context.Context, input generated.Update shortName = ref.Name[i+1:] } return &generated.UpdateRefPayload{ - Ref: presenter.GQLRef(repo.ID, ref.Name, shortName, string(ref.Target)), + Ref: presenter.GQLRef(repo.ID, ref.Name, shortName, ref.Target), ClientMutationID: input.ClientMutationID, }, nil } diff --git a/api/graphql/server.go b/api/graphql/server.go index 9174efa..24dbd64 100644 --- a/api/graphql/server.go +++ b/api/graphql/server.go @@ -71,7 +71,7 @@ func NewHandler(d Deps) http.Handler { srv.Use(extension.Introspection{}) srv.Use(nodeLimitExtension(maxNodeLimit)) srv.Use(depthLimitExtension(maxQueryDepth)) - var h http.Handler = liftErrorTypes(srv) + var h = liftErrorTypes(srv) if d.Batch != nil { h = loadersMiddleware(d.Batch, d.URLs, d.NodeFormat, h) } diff --git a/api/rest/app_contract_test.go b/api/rest/app_contract_test.go index 233e81c..0d9f4d3 100644 --- a/api/rest/app_contract_test.go +++ b/api/rest/app_contract_test.go @@ -177,11 +177,11 @@ func TestAppObjectShape(t *testing.T) { t.Fatalf("status %d, body %s", resp.StatusCode, body) } var app struct { - NodeID string `json:"node_id"` - Slug string `json:"slug"` + NodeID string `json:"node_id"` + Slug string `json:"slug"` Owner *struct{ Login string } `json:"owner"` - Permissions map[string]string `json:"permissions"` - Events []string `json:"events"` + Permissions map[string]string `json:"permissions"` + Events []string `json:"events"` } decodeBody(t, body, &app) if !strings.HasPrefix(app.NodeID, "A_") { @@ -239,7 +239,7 @@ func TestRepoInstallationLookup(t *testing.T) { t.Fatalf("status %d, body %s", resp.StatusCode, body) } var inst struct { - ID int64 `json:"id"` + ID int64 `json:"id"` Account *struct{ Login string } `json:"account"` } decodeBody(t, body, &inst) @@ -269,9 +269,9 @@ func TestInstallationTokenScoping(t *testing.T) { t.Fatalf("unscoped mint status %d, body %s", tokResp.StatusCode, tokBody) } var unscoped struct { - Token string `json:"token"` - RepoSel string `json:"repository_selection"` - Repos []any `json:"repositories"` + Token string `json:"token"` + RepoSel string `json:"repository_selection"` + Repos []any `json:"repositories"` } decodeBody(t, tokBody, &unscoped) if unscoped.RepoSel != "all" { @@ -290,7 +290,7 @@ func TestInstallationTokenScoping(t *testing.T) { t.Fatalf("scoped mint status %d, body %s", scResp.StatusCode, scBody) } var scoped struct { - RepoSel string `json:"repository_selection"` + RepoSel string `json:"repository_selection"` Repos []struct{ Name string } `json:"repositories"` } decodeBody(t, scBody, &scoped) @@ -355,7 +355,7 @@ func TestInstallationReposSelected(t *testing.T) { t.Fatalf("status %d, body %s", resp.StatusCode, body) } var env struct { - TotalCount int `json:"total_count"` + TotalCount int `json:"total_count"` Repos []struct{ Name string } `json:"repositories"` } decodeBody(t, body, &env) diff --git a/api/rest/archives_contract_test.go b/api/rest/archives_contract_test.go index a265a3b..3982835 100644 --- a/api/rest/archives_contract_test.go +++ b/api/rest/archives_contract_test.go @@ -24,7 +24,7 @@ func getNoRedirect(t *testing.T, srv *httptest.Server, path string) (*http.Respo if err != nil { t.Fatalf("GET %s: %v", path, err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() body, err := io.ReadAll(resp.Body) if err != nil { t.Fatalf("read %s: %v", path, err) @@ -73,7 +73,7 @@ func readZipEntries(t *testing.T, body []byte) map[string]string { t.Fatalf("open %s: %v", f.Name, err) } data, err := io.ReadAll(rc) - rc.Close() + _ = rc.Close() if err != nil { t.Fatalf("read %s: %v", f.Name, err) } @@ -89,7 +89,7 @@ func readTarGzEntries(t *testing.T, body []byte) map[string]string { if err != nil { t.Fatalf("open gzip: %v", err) } - defer gzr.Close() + defer func() { _ = gzr.Close() }() tr := tar.NewReader(gzr) out := map[string]string{} for { diff --git a/api/rest/contents_write.go b/api/rest/contents_write.go index d23822b..fe81707 100644 --- a/api/rest/contents_write.go +++ b/api/rest/contents_write.go @@ -173,7 +173,7 @@ func currentBlobSHA(d Deps, repo *domain.Repo, path, branch string) (string, boo if err != nil || res.IsDir { return "", false } - return string(res.Entry.SHA), true + return res.Entry.SHA, true } // errShaRequired is GitHub's 422 for a contents PUT or DELETE that targets an diff --git a/api/rest/git_objects_write.go b/api/rest/git_objects_write.go index 8eaeacd..3a2eb02 100644 --- a/api/rest/git_objects_write.go +++ b/api/rest/git_objects_write.go @@ -136,10 +136,10 @@ func handleGitCommitCreate(d Deps) mizu.Handler { return err } var body struct { - Message string `json:"message"` - Tree string `json:"tree"` - Parents []string `json:"parents"` - Author *struct { + Message string `json:"message"` + Tree string `json:"tree"` + Parents []string `json:"parents"` + Author *struct { Name string `json:"name"` Email string `json:"email"` Date string `json:"date"` diff --git a/api/rest/issues.go b/api/rest/issues.go index 746d91d..a652862 100644 --- a/api/rest/issues.go +++ b/api/rest/issues.go @@ -483,27 +483,6 @@ func handleCommentEdit(d Deps) mizu.Handler { } } -// handleCommentDelete serves DELETE /repos/{owner}/{repo}/issues/comments/{id}. -func handleCommentDelete(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - id, ok := pathInt64(c, "id") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - actor := auth.ActorFrom(c.Request().Context()) - err := d.Issues.DeleteComment(c.Request().Context(), actor.UserID, c.Param("owner"), c.Param("repo"), id) - if issueError(c.Writer(), err) { - return nil - } - if err != nil { - return err - } - c.Writer().WriteHeader(http.StatusNoContent) - return nil - } -} - // handleIssueDeleteDispatch dispatches the two DELETE shapes that share // /issues/{seg1}/{seg2} and that mizu cannot tell apart without a dispatcher, // because neither "/issues/comments/{id}" nor "/issues/{number}/labels" is @@ -1230,12 +1209,3 @@ func queryInt64(c *mizu.Ctx, name string) (int64, bool) { } return n, true } - -// pageNum reads the 1-based page query, defaulting to 1. -func pageNum(c *mizu.Ctx) int { - n, err := strconv.Atoi(c.Query("page")) - if err != nil || n <= 0 { - return 1 - } - return n -} diff --git a/api/rest/issues_extra.go b/api/rest/issues_extra.go index ca7791e..68fc0a2 100644 --- a/api/rest/issues_extra.go +++ b/api/rest/issues_extra.go @@ -230,36 +230,6 @@ func handleIssueLabelRemove(d Deps) mizu.Handler { } } -// handleIssueLabelsRemoveAll serves DELETE /repos/{owner}/{repo}/issues/{number}/labels. -func handleIssueLabelsRemoveAll(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - ctx := c.Request().Context() - actor := auth.ActorFrom(ctx) - if !actor.IsUser() { - writeError(c.Writer(), errRequiresAuth()) - return nil - } - number, ok := pathInt64(c, "number") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - empty := []string{} - _, err := d.Issues.EditIssue(ctx, actor.UserID, c.Param("owner"), c.Param("repo"), number, domain.IssuePatch{ - Labels: &empty, - }) - if errors.Is(err, domain.ErrNotFound) || errors.Is(err, domain.ErrRepoNotFound) { - writeError(c.Writer(), errNotFound()) - return nil - } - if err != nil { - return err - } - c.Writer().WriteHeader(http.StatusNoContent) - return nil - } -} - // handleAssigneesList serves GET /repos/{owner}/{repo}/assignees. // Returns a list of users who can be assigned to issues in the repo // (the owner plus collaborators). diff --git a/api/rest/keys.go b/api/rest/keys.go index 2a47afb..52e7f98 100644 --- a/api/rest/keys.go +++ b/api/rest/keys.go @@ -287,4 +287,3 @@ func userKeyToJSON(k *store.SSHKeyRow, d Deps) map[string]any { "read_only": false, } } - diff --git a/api/rest/misc_compat.go b/api/rest/misc_compat.go index fb7e55c..376bfe2 100644 --- a/api/rest/misc_compat.go +++ b/api/rest/misc_compat.go @@ -433,7 +433,7 @@ func handleSingleCommitGet(d Deps) mizu.Handler { } // handleRepoTeamsList serves GET /repos/{owner}/{repo}/teams. -func handleRepoTeamsList(d Deps) mizu.Handler { +func handleRepoTeamsList(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, []any{}) return nil @@ -592,7 +592,7 @@ func handleSearchUsers(d Deps) mizu.Handler { } // handleSearchCommits serves GET /search/commits. -func handleSearchCommits(d Deps) mizu.Handler { +func handleSearchCommits(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, map[string]any{ "total_count": 0, @@ -604,7 +604,7 @@ func handleSearchCommits(d Deps) mizu.Handler { } // handleSearchTopics serves GET /search/topics. -func handleSearchTopics(d Deps) mizu.Handler { +func handleSearchTopics(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, map[string]any{ "total_count": 0, @@ -616,7 +616,7 @@ func handleSearchTopics(d Deps) mizu.Handler { } // handleSearchLabels serves GET /search/labels. -func handleSearchLabels(d Deps) mizu.Handler { +func handleSearchLabels(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, map[string]any{ "total_count": 0, @@ -668,7 +668,7 @@ func handleOrgTeamsList(d Deps) mizu.Handler { } // handleOrgTeamReposList serves GET /orgs/{org}/teams/{team_slug}/repos. -func handleOrgTeamReposList(d Deps) mizu.Handler { +func handleOrgTeamReposList(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, []any{}) return nil @@ -816,9 +816,9 @@ func handleInstallationRepos(d Deps) mizu.Handler { items = append(items, d.URLs.Repository(r, d.NodeFormat, nil)) } writeJSON(c.Writer(), http.StatusOK, map[string]any{ - "total_count": len(items), - "repository_selection": inst.RepositorySelection, - "repositories": items, + "total_count": len(items), + "repository_selection": inst.RepositorySelection, + "repositories": items, }) return nil } diff --git a/api/rest/pulls_extra.go b/api/rest/pulls_extra.go index ad712ff..13cf966 100644 --- a/api/rest/pulls_extra.go +++ b/api/rest/pulls_extra.go @@ -99,37 +99,6 @@ func handleReviewCommentEdit(d Deps) mizu.Handler { } } -// handleReviewCommentDelete serves DELETE /repos/{owner}/{repo}/pulls/comments/{comment_id}. -func handleReviewCommentDelete(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - ctx := c.Request().Context() - actor := auth.ActorFrom(ctx) - if !actor.IsUser() { - writeError(c.Writer(), errRequiresAuth()) - return nil - } - id, ok := pathInt64(c, "comment_id") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - err := d.Reviews.DeleteReviewComment(ctx, actor.UserID, id) - if errors.Is(err, domain.ErrNotFound) { - writeError(c.Writer(), errNotFound()) - return nil - } - if errors.Is(err, domain.ErrForbidden) { - writeError(c.Writer(), errForbidden("not allowed")) - return nil - } - if err != nil { - return err - } - c.Writer().WriteHeader(http.StatusNoContent) - return nil - } -} - // handleAllReviewCommentsList serves GET /repos/{owner}/{repo}/pulls/comments. func handleAllReviewCommentsList(d Deps) mizu.Handler { return func(c *mizu.Ctx) error { @@ -189,40 +158,6 @@ func handleRequestedReviewersAdd(d Deps) mizu.Handler { } } -// handleRequestedReviewersRemove serves DELETE /repos/{owner}/{repo}/pulls/{number}/requested_reviewers. -func handleRequestedReviewersRemove(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - number, ok := pathInt64(c, "number") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - ctx := c.Request().Context() - actor := auth.ActorFrom(ctx) - if !actor.IsUser() { - writeError(c.Writer(), errRequiresAuth()) - return nil - } - var body struct { - Reviewers []string `json:"reviewers"` - TeamReviewers []string `json:"team_reviewers"` - } - if !decodeJSON(c, &body) { - return nil - } - owner, repo := c.Param("owner"), c.Param("repo") - pr, err := d.Pulls.RemoveRequestedReviewers(ctx, actor.UserID, owner, repo, number, body.Reviewers) - if pullError(c.Writer(), err) { - return nil - } - if err != nil { - return err - } - writeJSON(c.Writer(), http.StatusOK, d.URLs.PullRequest(owner, repo, pr, d.NodeFormat, true)) - return nil - } -} - // handleRequestedReviewersList serves GET /repos/{owner}/{repo}/pulls/{number}/requested_reviewers. func handleRequestedReviewersList(d Deps) mizu.Handler { return func(c *mizu.Ctx) error { diff --git a/api/rest/repo_permissions_contract_test.go b/api/rest/repo_permissions_contract_test.go index 387d483..fc478ef 100644 --- a/api/rest/repo_permissions_contract_test.go +++ b/api/rest/repo_permissions_contract_test.go @@ -111,7 +111,7 @@ func TestRepoPermissionsCollaborator(t *testing.T) { } // The repo list renders the same resolved block per item. - resp, body = authedGet(t, fx.srv, "/users/octocat/repos", "token "+hubToken) + resp, _ = authedGet(t, fx.srv, "/users/octocat/repos", "token "+hubToken) if resp.StatusCode != http.StatusOK { t.Fatalf("list: status %d", resp.StatusCode) } diff --git a/api/rest/repo_surface_contract_test.go b/api/rest/repo_surface_contract_test.go index c274d3f..6fc0231 100644 --- a/api/rest/repo_surface_contract_test.go +++ b/api/rest/repo_surface_contract_test.go @@ -109,7 +109,9 @@ func TestCommunityProfile(t *testing.T) { HealthPercentage int `json:"health_percentage"` Description *string `json:"description"` Files struct { - Readme *struct{ HTMLURL string `json:"html_url"` } `json:"readme"` + Readme *struct { + HTMLURL string `json:"html_url"` + } `json:"readme"` License *struct{} `json:"license"` Contributing *struct{} `json:"contributing"` } `json:"files"` diff --git a/api/rest/root.go b/api/rest/root.go index 44f9b77..10c3513 100644 --- a/api/rest/root.go +++ b/api/rest/root.go @@ -8,7 +8,7 @@ import ( // apiRoot is GitHub's root hypermedia document: the *_url template map clients // like Octokit's hypermedia mode read to derive every other endpoint instead of -// hardcoding paths. Field order matches api.github.com. +// hardcoding paths. Field order matches the GitHub REST API root. type apiRoot struct { CurrentUserURL string `json:"current_user_url"` CurrentUserAuthorizationsHTMLURL string `json:"current_user_authorizations_html_url"` @@ -45,8 +45,8 @@ type apiRoot struct { } // handleAPIRoot serves GET at the API root. The body is the same template map -// api.github.com returns, with every URL rebuilt on the configured hosts. It is -// computed once at mount time: nothing in it varies per request. +// the GitHub REST API root returns, with every URL rebuilt on the configured +// hosts. It is computed once at mount time: nothing in it varies per request. func handleAPIRoot(d Deps) mizu.Handler { api := d.URLs.APIBase() doc := apiRoot{ diff --git a/api/rest/social_contract_test.go b/api/rest/social_contract_test.go index cf1d0c5..8281b0b 100644 --- a/api/rest/social_contract_test.go +++ b/api/rest/social_contract_test.go @@ -188,7 +188,7 @@ func TestWatchingFlow(t *testing.T) { if sub["ignored"] != true { t.Fatalf("ignored subscription = %v", sub) } - resp, body = authedGet(t, fx.srv, "/repos/octocat/hello/subscribers", "token "+fx.token) + _, body = authedGet(t, fx.srv, "/repos/octocat/hello/subscribers", "token "+fx.token) subs = decodeArray(t, body) if len(subs) != 0 { t.Fatalf("subscribers after ignore = %v, want []", subs) @@ -239,12 +239,12 @@ func TestFollowingFlow(t *testing.T) { } // octocat's following list shows hubber; hubber's followers show octocat. - resp, body = authedGet(t, fx.srv, "/user/following", "token "+octo) + _, body = authedGet(t, fx.srv, "/user/following", "token "+octo) following := decodeArray(t, body) if len(following) != 1 || following[0]["login"] != "hubber" { t.Fatalf("following = %v, want [hubber]", following) } - resp, body = authedGet(t, fx.srv, "/users/hubber/followers", "token "+hubber) + _, body = authedGet(t, fx.srv, "/users/hubber/followers", "token "+hubber) followers := decodeArray(t, body) if len(followers) != 1 || followers[0]["login"] != "octocat" { t.Fatalf("followers = %v, want [octocat]", followers) diff --git a/auth/app_jwt.go b/auth/app_jwt.go index b60a144..8e81fd3 100644 --- a/auth/app_jwt.go +++ b/auth/app_jwt.go @@ -40,7 +40,7 @@ func (c *publicKeyCache) set(pk int64, k *rsa.PublicKey) { // On success it returns a KindAppJWT actor whose AppID is the app's internal PK. func (s *Service) resolveAppJWT(ctx context.Context, raw string) (*Actor, error) { var claims jwt.RegisteredClaims - keyFunc := func(t *jwt.Token) (any, error) { + keyFunc := func(_ *jwt.Token) (any, error) { iss, err := claims.GetIssuer() if err != nil || iss == "" { return nil, ErrBadCredentials @@ -130,8 +130,7 @@ func (s *Service) resolveInstallation(ctx context.Context, raw string) (*Actor, // and own the installation. On success it returns the plaintext token and expiry. // repos and permissions narrow the grant (empty = full installation grant). func (s *Service) CreateInstallationToken(ctx context.Context, actor *Actor, instPK int64, - repos []string, permissions map[string]string) (plaintext string, expiresAt time.Time, err error) { - + _ []string, _ map[string]string) (plaintext string, expiresAt time.Time, err error) { if actor.Kind != KindAppJWT { return "", time.Time{}, ErrBadCredentials } diff --git a/auth/webflow.go b/auth/webflow.go index 087ddac..be359dc 100644 --- a/auth/webflow.go +++ b/auth/webflow.go @@ -45,8 +45,8 @@ func redirectAllowed(app *store.OAuthAppRow, redirectURI string) bool { return strings.HasPrefix(redirectURI, app.CallbackURL) } -// AuthCodeRequest holds the parameters the caller passes to GenerateAuthCode. -type AuthCodeRequest struct { +// CodeRequest holds the parameters the caller passes to GenerateAuthCode. +type CodeRequest struct { ClientID string RedirectURI string Scope string @@ -56,7 +56,7 @@ type AuthCodeRequest struct { // GenerateAuthCode creates a new single-use authorization code for the OAuth // web flow. It returns the opaque plaintext code the server embeds in the // redirect to the client's redirect_uri. -func (s *Service) GenerateAuthCode(ctx context.Context, req AuthCodeRequest) (string, error) { +func (s *Service) GenerateAuthCode(ctx context.Context, req CodeRequest) (string, error) { return s.GenerateOAuthAuthCode(ctx, req.ClientID, req.RedirectURI, req.Scope, req.UserPK) } diff --git a/conform/conform.go b/conform/conform.go index 927bb9d..19fa47a 100644 --- a/conform/conform.go +++ b/conform/conform.go @@ -54,8 +54,11 @@ func Run(opts Options) *Report { type Status int const ( + // Pass means the check ran and the behavior matched. Pass Status = iota + // Fail means the check ran and the behavior did not match. Fail + // Skip means the check did not run. Skip ) diff --git a/domain/event.go b/domain/event.go index 1cbce8c..6e9b725 100644 --- a/domain/event.go +++ b/domain/event.go @@ -301,7 +301,7 @@ func (s *EventService) OrgFeed(ctx context.Context, viewerPK int64, org string, // following anyone. The publicOnly flag is accepted for the // /received_events/public twin and is a no-op while the feed is already public. // It backs GET /users/{u}/received_events[/public]. -func (s *EventService) ReceivedFeed(ctx context.Context, login string, perPage int, publicOnly bool) ([]Event, error) { +func (s *EventService) ReceivedFeed(ctx context.Context, login string, perPage int, _ bool) ([]Event, error) { if _, err := s.store.UserByLogin(ctx, login); err != nil { return nil, ErrUserNotFound } diff --git a/domain/key_service.go b/domain/key_service.go index 485aa68..a500a7b 100644 --- a/domain/key_service.go +++ b/domain/key_service.go @@ -71,7 +71,6 @@ func (s *KeyService) GetDeployKey(ctx context.Context, repoPK, dbID int64) (*sto // CreateDeployKey parses and registers a new deploy key on repoPK. func (s *KeyService) CreateDeployKey(ctx context.Context, repoPK, ownerPK int64, title, rawKey string, readOnly bool) (*store.SSHKeyRow, error) { - keyType, fp, err := parseSSHPublicKey(rawKey) if err != nil { return nil, ErrInvalidSSHKey diff --git a/domain/push.go b/domain/push.go index 11009cf..0aeae45 100644 --- a/domain/push.go +++ b/domain/push.go @@ -121,8 +121,8 @@ func (s *RepoService) SyntheticHeadPush(ctx context.Context, repoPK int64, defau Protocol: "http", Updates: []RefUpdate{{ Ref: ref, - OldSHA: string(sha), - NewSHA: string(sha), + OldSHA: sha, + NewSHA: sha, }}, }, true, nil } diff --git a/domain/repo_community.go b/domain/repo_community.go index 0edbec3..98ddf85 100644 --- a/domain/repo_community.go +++ b/domain/repo_community.go @@ -91,7 +91,7 @@ func (s *RepoService) repoFilePaths(repo *Repo) ([]string, error) { // is the one GitHub would surface. var ( communityReadme = matcher("README") - communityLicense = prefixMatcher([]string{""}, "LICENSE", "LICENCE", "COPYING") + communityLicense = prefixMatcher([]string{""}, "LICENSE", "LICENSE", "COPYING") communityContributing = matcher("CONTRIBUTING") communityCodeOfConduct = matcher("CODE_OF_CONDUCT") communityIssueTemplate = matcher("ISSUE_TEMPLATE") diff --git a/domain/repo_merge.go b/domain/repo_merge.go index d963764..a119a48 100644 --- a/domain/repo_merge.go +++ b/domain/repo_merge.go @@ -74,7 +74,7 @@ func (s *RepoService) MergeBranch(ctx context.Context, actorPK int64, owner, nam // Fan the merge out as a push to the base branch so the activity feed and a // repository's webhooks observe it the same as any other update to that // branch. Delivery is best-effort, exactly as a real push's is. - s.OnPush(ctx, PushBatch{ + _ = s.OnPush(ctx, PushBatch{ RepoPK: repo.PK, PusherPK: actorPK, Protocol: "api", diff --git a/domain/repo_service.go b/domain/repo_service.go index 9910e76..6805c2f 100644 --- a/domain/repo_service.go +++ b/domain/repo_service.go @@ -959,7 +959,7 @@ func checkCurrentBlob(gr *git.Repo, in WriteFileInput) error { ref = "HEAD" } cur, err := gr.PathAt(ref, in.Path) - if err != nil || cur.IsDir || string(cur.Entry.SHA) != in.CurrentBlobSHA { + if err != nil || cur.IsDir || cur.Entry.SHA != in.CurrentBlobSHA { return ErrConflict } return nil diff --git a/domain/search_service.go b/domain/search_service.go index 46245b4..f904ffd 100644 --- a/domain/search_service.go +++ b/domain/search_service.go @@ -319,7 +319,7 @@ func (s *SearchService) codeIndexHeadSHA(repo *Repo) string { if err != nil { return "" } - return string(br.Commit) + return br.Commit } // codeIndexFresh reports whether the stored index state already matches head. diff --git a/domain/team_service.go b/domain/team_service.go index 93bfc9b..a73a093 100644 --- a/domain/team_service.go +++ b/domain/team_service.go @@ -74,10 +74,10 @@ func (s *TeamService) CreateTeam(ctx context.Context, orgPK int64, name, descrip permission = "pull" } t := &store.TeamRow{ - OrgPK: orgPK, - Name: name, - Slug: slugify(name), - Privacy: privacy, + OrgPK: orgPK, + Name: name, + Slug: slugify(name), + Privacy: privacy, Permission: permission, } if d := strings.TrimSpace(description); d != "" { diff --git a/fe/bench_test.go b/fe/bench_test.go index 1382ebc..e77b086 100644 --- a/fe/bench_test.go +++ b/fe/bench_test.go @@ -576,7 +576,10 @@ func TestPageSLO(t *testing.T) { } avg := time.Since(start) / measure t.Logf("%s: avg=%v", p.name, avg) - if avg > budget { + // Under -race the instrumentation inflates every request far past the + // budget; the requests above still run for the race detector, but the + // wall-clock gate only holds on an uninstrumented build. + if !raceEnabled && avg > budget { t.Errorf("%s: avg=%v exceeds the %v budget", p.name, avg, budget) } }) diff --git a/fe/mount.go b/fe/mount.go index a1ae81b..5a43dda 100644 --- a/fe/mount.go +++ b/fe/mount.go @@ -604,7 +604,7 @@ func mountProfile(page *mizu.Router, d Deps) { // absence. The redirect itself needs no service, so it is always registered: the // profile it points at renders or 404s on the user service's own gate. "orgs" is // a reserved top-level name (fe/route), so it is never read as a /{owner} login. -func mountOrgs(page *mizu.Router, d Deps) { +func mountOrgs(page *mizu.Router, _ Deps) { page.Get("/orgs/{org}", func(c *mizu.Ctx) error { return c.Redirect(http.StatusFound, route.Profile(c.Param("org"))) }) diff --git a/fe/race_off_test.go b/fe/race_off_test.go new file mode 100644 index 0000000..e6093ea --- /dev/null +++ b/fe/race_off_test.go @@ -0,0 +1,7 @@ +//go:build !race + +package fe_test + +// raceEnabled is false in a normal (non -race) build, so the SLO gate enforces +// its wall-clock budget. +const raceEnabled = false diff --git a/fe/race_on_test.go b/fe/race_on_test.go new file mode 100644 index 0000000..4d9eb6d --- /dev/null +++ b/fe/race_on_test.go @@ -0,0 +1,9 @@ +//go:build race + +package fe_test + +// raceEnabled is true when the test binary is built with -race. The SLO gate +// reads it to skip its wall-clock budget assertion, since race instrumentation +// inflates every request well past the budget while the requests themselves +// still exercise the handlers for the race detector. +const raceEnabled = true diff --git a/fe/render/manifest.go b/fe/render/manifest.go index 2aee1df..312cc73 100644 --- a/fe/render/manifest.go +++ b/fe/render/manifest.go @@ -87,7 +87,7 @@ func (s *Set) AssetHandler() mizu.Handler { if err != nil { return s.NotFound(c) } - defer f.Close() + defer func() { _ = f.Close() }() h := c.Header() h.Set("Content-Type", contentTypeFor(clean)) diff --git a/fe/view/issues.go b/fe/view/issues.go index 110165e..0ec7135 100644 --- a/fe/view/issues.go +++ b/fe/view/issues.go @@ -174,19 +174,19 @@ type ComposerVM struct { // IssueDetailVM is the issue detail page: the title and state header, the comment // timeline, the sidebar, and the composer. type IssueDetailVM struct { - Chrome Chrome - Header RepoHeaderVM - Nav TreeNav - Repo RepoRef - Number int64 - Title string - State IssueStateVM - Author UserChipVM - OpenedAt string - OpenedISO string - Locked bool - EditURL string // the edit-title form target, shown when the viewer can edit - CanEdit bool + Chrome Chrome + Header RepoHeaderVM + Nav TreeNav + Repo RepoRef + Number int64 + Title string + State IssueStateVM + Author UserChipVM + OpenedAt string + OpenedISO string + Locked bool + EditURL string // the edit-title form target, shown when the viewer can edit + CanEdit bool Timeline []CommentVM TimelinePager Pager // prev/next over a long comment thread; zero value hides it Sidebar SidebarVM diff --git a/fe/web/issues/prefill.go b/fe/web/issues/prefill.go index c9accf5..9c0eda0 100644 --- a/fe/web/issues/prefill.go +++ b/fe/web/issues/prefill.go @@ -44,7 +44,7 @@ func (h *Handlers) prefillNewIssue(c *mizu.Ctx, repo *domain.Repo, vm *view.NewI // and folds it into the form: the body is the template text with its YAML // front matter stripped, and the front matter's title, labels, and assignees // fill any field the query left empty. -func (h *Handlers) applyIssueTemplate(ctx context.Context, repo *domain.Repo, name string, vm *view.NewIssueVM) { +func (h *Handlers) applyIssueTemplate(_ context.Context, repo *domain.Repo, name string, vm *view.NewIssueVM) { // The template is one file name inside the template directory, never a // path of its own. if name != path.Base(name) || strings.HasPrefix(name, ".") { diff --git a/fe/web/issues/show.go b/fe/web/issues/show.go index fef7f0d..3c39e13 100644 --- a/fe/web/issues/show.go +++ b/fe/web/issues/show.go @@ -97,17 +97,17 @@ func (h *Handlers) detail(ctx context.Context, c *mizu.Ctx, repo *domain.Repo, i open := iss.State == "open" vm := view.IssueDetailVM{ - Chrome: h.chrome(c, iss.Title+" #"+strconv.FormatInt(iss.Number, 10)), - Header: h.header(c.Context(), repo), - Nav: h.nav(repo), - Repo: repoRef(repo), - Number: iss.Number, - Title: iss.Title, - State: stateBadge(iss), - Author: h.userChip(iss.User), - OpenedAt: iss.CreatedAt.UTC().Format("Jan 2, 2006"), - OpenedISO: iss.CreatedAt.UTC().Format(time.RFC3339), - Locked: iss.Locked, + Chrome: h.chrome(c, iss.Title+" #"+strconv.FormatInt(iss.Number, 10)), + Header: h.header(c.Context(), repo), + Nav: h.nav(repo), + Repo: repoRef(repo), + Number: iss.Number, + Title: iss.Title, + State: stateBadge(iss), + Author: h.userChip(iss.User), + OpenedAt: iss.CreatedAt.UTC().Format("Jan 2, 2006"), + OpenedISO: iss.CreatedAt.UTC().Format(time.RFC3339), + Locked: iss.Locked, CanEdit: write, FormError: formError, TimelinePager: pager, diff --git a/fe/web/notifications/handlers_test.go b/fe/web/notifications/handlers_test.go index 0f6cbbe..40ffe69 100644 --- a/fe/web/notifications/handlers_test.go +++ b/fe/web/notifications/handlers_test.go @@ -60,13 +60,13 @@ func newFixture(t *testing.T) fixture { // Two issue rows: a plain issue and a pull request sharing the number // sequence. Each backs one notification thread. - issuePK := seedIssue(t, ctx, st, hello.PK, "the unread bug", false) - pullPK := seedIssue(t, ctx, st, hello.PK, "the read pull", true) + issuePK := seedIssue(ctx, t, st, hello.PK, "the unread bug", false) + pullPK := seedIssue(ctx, t, st, hello.PK, "the read pull", true) // The unread thread shows in both the Inbox and the All filter; the read one // only in All. - upsertThread(t, ctx, st, octocat.PK, hello.PK, issuePK, "mention") - readPK := upsertThread(t, ctx, st, octocat.PK, hello.PK, pullPK, "review_requested") + upsertThread(ctx, t, st, octocat.PK, hello.PK, issuePK, "mention") + readPK := upsertThread(ctx, t, st, octocat.PK, hello.PK, pullPK, "review_requested") if err := st.MarkNotificationThreadRead(ctx, readPK); err != nil { t.Fatalf("mark read: %v", err) } @@ -123,7 +123,7 @@ func newFixture(t *testing.T) fixture { // number from the repo's sequence, and returns its PK. The domain gates // creation on access this fixture does not grant, and the inbox only needs the // row to resolve a thread's subject. -func seedIssue(t *testing.T, ctx context.Context, st *store.Store, repoPK int64, title string, isPull bool) int64 { +func seedIssue(ctx context.Context, t *testing.T, st *store.Store, repoPK int64, title string, isPull bool) int64 { t.Helper() var pk int64 if err := st.WithTx(ctx, func(tx *store.Tx) error { @@ -145,7 +145,7 @@ func seedIssue(t *testing.T, ctx context.Context, st *store.Store, repoPK int64, // upsertThread records a notification thread for the user on the issue and // returns its PK. -func upsertThread(t *testing.T, ctx context.Context, st *store.Store, userPK, repoPK, issuePK int64, reason string) int64 { +func upsertThread(ctx context.Context, t *testing.T, st *store.Store, userPK, repoPK, issuePK int64, reason string) int64 { t.Helper() row := &store.NotificationThreadRow{UserPK: userPK, RepoPK: repoPK, IssuePK: issuePK, Reason: reason} if err := st.UpsertNotificationThread(ctx, row); err != nil { diff --git a/fe/web/repo/handlers_test.go b/fe/web/repo/handlers_test.go index d3f28e9..5fb9143 100644 --- a/fe/web/repo/handlers_test.go +++ b/fe/web/repo/handlers_test.go @@ -3,7 +3,6 @@ package repo import ( "bytes" "context" - "github.com/tamnd/githome/fe/route" "io" "log/slog" "net/http" @@ -16,6 +15,8 @@ import ( "testing" "time" + "github.com/tamnd/githome/fe/route" + gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" diff --git a/fe/webmw/gzip_test.go b/fe/webmw/gzip_test.go index 6054a75..2d7d336 100644 --- a/fe/webmw/gzip_test.go +++ b/fe/webmw/gzip_test.go @@ -12,7 +12,7 @@ import ( // gzipEcho serves a fixed body under the given content type, optionally with a // preset Content-Encoding, behind the Gzip layer. func gzipEcho(contentType, encoding, body string) http.Handler { - return Gzip(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + return Gzip(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", contentType) if encoding != "" { w.Header().Set("Content-Encoding", encoding) diff --git a/git/archive.go b/git/archive.go index 93c4ee7..e8dad06 100644 --- a/git/archive.go +++ b/git/archive.go @@ -25,7 +25,7 @@ const archiveTimeout = 5 * time.Minute func (s *Store) ArchiveStream(ctx context.Context, pk int64, format, prefix string, sha SHA, w io.Writer) error { ctx, cancel := context.WithTimeout(ctx, archiveTimeout) defer cancel() - args := []string{"archive", "--format=" + format, "--prefix=" + prefix + "/", "--end-of-options", string(sha)} + args := []string{"archive", "--format=" + format, "--prefix=" + prefix + "/", "--end-of-options", sha} full := append([]string{"--git-dir", s.runDir(pk)}, args...) cmd := exec.CommandContext(ctx, s.bin(), full...) cmd.Env = baseEnv() diff --git a/git/repo_blame.go b/git/repo_blame.go index 1168dc1..97084fc 100644 --- a/git/repo_blame.go +++ b/git/repo_blame.go @@ -49,40 +49,40 @@ func blameCost(lines []BlameLine) int64 { // handled is false when the Repo has no store or the run failed, in which case // the caller falls back to go-git; when handled is true the result, including // a size-cap refusal, is final. -func (r *Repo) blameBatch(ref, path string) (lines []BlameLine, err error, handled bool) { +func (r *Repo) blameBatch(ref, path string) (lines []BlameLine, handled bool, err error) { if r.store == nil { - return nil, nil, false + return nil, false, nil } c, cerr := r.commitFromRev(ref) if cerr != nil { - return nil, nil, false + return nil, false, nil } sha := c.Hash.String() key := strconv.FormatInt(r.pk, 10) + ":" + sha + ":" + path if v, ok := r.store.blames.get(key); ok { - return v, nil, true + return v, true, nil } if r.maxBlobBytes > 0 { res, rerr := r.store.run(context.Background(), r.pk, nil, "cat-file", "-s", sha+":"+path) if rerr != nil || res.code != 0 { - return nil, nil, false + return nil, false, nil } if n, perr := strconv.ParseInt(strings.TrimSpace(string(res.stdout)), 10, 64); perr == nil && n > r.maxBlobBytes { - return nil, ErrBlobTooLarge, true + return nil, true, ErrBlobTooLarge } } ctx, cancel := context.WithTimeout(context.Background(), blameTimeout) defer cancel() res, rerr := r.store.run(ctx, r.pk, nil, "blame", "--porcelain", "--end-of-options", sha, "--", path) if rerr != nil || res.code != 0 { - return nil, nil, false + return nil, false, nil } lines, ok := parseBlamePorcelain(string(res.stdout)) if !ok { - return nil, nil, false + return nil, false, nil } r.store.blames.put(key, lines) - return lines, nil, true + return lines, true, nil } // parseBlamePorcelain parses git blame --porcelain output. Commit metadata diff --git a/git/repo_merge.go b/git/repo_merge.go index 74d9dbe..b00e739 100644 --- a/git/repo_merge.go +++ b/git/repo_merge.go @@ -96,13 +96,14 @@ func (s *Store) CommitsBetween(ctx context.Context, pk int64, base, head SHA) ([ return s.CommitsBetweenN(ctx, pk, base, head, 0) } -// CommitsBetweenN is CommitsBetween bounded to at most max commits (the newest -// max of the range, still returned oldest first), so a compare across thousands -// of commits never loads the whole range into memory. max <= 0 is unbounded. -func (s *Store) CommitsBetweenN(ctx context.Context, pk int64, base, head SHA, max int) ([]Commit, error) { +// CommitsBetweenN is CommitsBetween bounded to at most limit commits (the +// newest limit of the range, still returned oldest first), so a compare across +// thousands of commits never loads the whole range into memory. limit <= 0 is +// unbounded. +func (s *Store) CommitsBetweenN(ctx context.Context, pk int64, base, head SHA, limit int) ([]Commit, error) { args := []string{"log", "--reverse", "--pretty=format:" + logRecordFormat} - if max > 0 { - args = append(args, "-n", strconv.Itoa(max)) + if limit > 0 { + args = append(args, "-n", strconv.Itoa(limit)) } args = append(args, base+".."+head) r, err := s.run(ctx, pk, nil, args...) @@ -391,7 +392,7 @@ func (s *Store) DiffDirect(ctx context.Context, pk int64, base, head SHA) ([]byt // commit page's .patch form serves. format-patch -1 handles a root commit // without a parent. func (s *Store) FormatPatchCommit(ctx context.Context, pk int64, sha SHA) ([]byte, error) { - args := []string{"format-patch", "-1", "--stdout", "--no-color", "--end-of-options", string(sha)} + args := []string{"format-patch", "-1", "--stdout", "--no-color", "--end-of-options", sha} r, err := s.run(ctx, pk, nil, args...) if err != nil { return nil, err diff --git a/git/repo_read.go b/git/repo_read.go index 840ffa8..b457a4e 100644 --- a/git/repo_read.go +++ b/git/repo_read.go @@ -602,7 +602,7 @@ type BlameLine struct { // it. It returns ErrObjectNotFound when the path does not exist in the tree at // ref, and ErrRepoNotFound for other resolution failures. func (r *Repo) Blame(ref, path string) ([]BlameLine, error) { - if lines, err, handled := r.blameBatch(ref, path); handled { + if lines, handled, err := r.blameBatch(ref, path); handled { return lines, err } c, err := r.commitFromRev(ref) diff --git a/git/repo_refs.go b/git/repo_refs.go index 92c7495..2d6e3e3 100644 --- a/git/repo_refs.go +++ b/git/repo_refs.go @@ -112,7 +112,7 @@ func (r *Repo) logBatch(opts LogOpts, limit int) ([]Commit, bool) { if opts.Until != nil { args = append(args, "--until="+opts.Until.Format(time.RFC3339)) } - args = append(args, "--end-of-options", string(opts.From)) + args = append(args, "--end-of-options", opts.From) if opts.Path != "" { args = append(args, "--", opts.Path) } diff --git a/git/repo_write.go b/git/repo_write.go index bc0c395..536a174 100644 --- a/git/repo_write.go +++ b/git/repo_write.go @@ -91,7 +91,7 @@ func (s *Store) runEnv(ctx context.Context, pk int64, extraEnv []string, stdin i // ours, not a git failure. It exists for reads whose size the server cannot // know in advance, like a commit's patch, where buffering an unbounded diff // would defeat the point of capping it. -func (s *Store) runLimited(ctx context.Context, pk int64, max int64, args ...string) (stdout []byte, truncated bool, code int, err error) { +func (s *Store) runLimited(ctx context.Context, pk int64, limit int64, args ...string) (stdout []byte, truncated bool, code int, err error) { ctx, cancel := context.WithCancel(ctx) defer cancel() full := append([]string{"--git-dir", s.runDir(pk)}, args...) @@ -104,9 +104,9 @@ func (s *Store) runLimited(ctx context.Context, pk int64, max int64, args ...str if err := cmd.Start(); err != nil { return nil, false, 0, fmt.Errorf("git %s: %w", strings.Join(args, " "), err) } - out, readErr := io.ReadAll(io.LimitReader(pipe, max+1)) - if int64(len(out)) > max { - out, truncated = out[:max], true + out, readErr := io.ReadAll(io.LimitReader(pipe, limit+1)) + if int64(len(out)) > limit { + out, truncated = out[:limit], true cancel() // kill the producer; Wait's error is then ours to ignore } waitErr := cmd.Wait() diff --git a/git/repo_write_file.go b/git/repo_write_file.go index ad3e2a2..4585c49 100644 --- a/git/repo_write_file.go +++ b/git/repo_write_file.go @@ -188,7 +188,6 @@ func (r *Repo) rebuildTree( blobHash plumbing.Hash, del bool, ) (plumbing.Hash, error) { - name := parts[0] var newEntries []object.TreeEntry diff --git a/git/repo_write_objects.go b/git/repo_write_objects.go index 7fbbb62..5ddb4d8 100644 --- a/git/repo_write_objects.go +++ b/git/repo_write_objects.go @@ -11,7 +11,7 @@ import ( // CreateBlobInput holds content for POST /git/blobs. type CreateBlobInput struct { - Content []byte // already decoded (caller converts from utf-8 or base64) + Content []byte // already decoded (caller converts from utf-8 or base64) } // CreateBlobResult holds the outcome of CreateBlob. @@ -42,10 +42,10 @@ func (r *Repo) CreateBlob(in CreateBlobInput) (*CreateBlobResult, error) { // CreateTreeEntry is one entry passed to CreateTree. type CreateTreeEntry struct { Path string - Mode string // "100644", "100755", "040000", "160000", "120000" + Mode string // "100644", "100755", "040000", "160000", "120000" Type ObjectType // "blob", "tree", "commit" - SHA string // object SHA; empty string is allowed for inline blobs - Content []byte // non-nil: create an inline blob first + SHA string // object SHA; empty string is allowed for inline blobs + Content []byte // non-nil: create an inline blob first } // CreateTreeResult holds the outcome of CreateTree. @@ -132,11 +132,11 @@ func (r *Repo) CreateTree(baseTreeSHA string, entries []CreateTreeEntry) (*Creat // CreateCommitInput holds parameters for POST /git/commits. type CreateCommitInput struct { - Message string - Tree string - Parents []string - Author Signature - Committer Signature + Message string + Tree string + Parents []string + Author Signature + Committer Signature } // CreateCommitResult holds the outcome of CreateCommit. @@ -259,7 +259,9 @@ func (r *Repo) CreateTag(in CreateTagInput) (*CreateTagResult, error) { }, nil } -// GetTag reads an annotated tag object by SHA. +// GetTagResult is the decoded form of an annotated tag object: the tag's own +// SHA and name, its message, and the object it points at with that object's +// type and the tagger signature. type GetTagResult struct { SHA string Tag string @@ -269,6 +271,7 @@ type GetTagResult struct { Tagger Signature } +// GetTag reads an annotated tag object by SHA. func (r *Repo) GetTag(sha string) (*GetTagResult, error) { st := r.repo.Storer t, err := object.GetTag(st, plumbing.NewHash(sha)) diff --git a/markup/cache_test.go b/markup/cache_test.go index bf5a641..1edab28 100644 --- a/markup/cache_test.go +++ b/markup/cache_test.go @@ -60,7 +60,7 @@ func TestRenderWithResolveBypassesCache(t *testing.T) { rc := RenderContext{ Mode: ModeComment, Repo: &RepoRef{Owner: "octo", Name: "hello", ID: 7}, - Resolve: func(ctx context.Context, kind RefKind, raw string) (string, bool) { + Resolve: func(_ context.Context, _ RefKind, _ string) (string, bool) { return "", false }, } diff --git a/presenter/gqlissue.go b/presenter/gqlissue.go index 515c34a..dc9ca41 100644 --- a/presenter/gqlissue.go +++ b/presenter/gqlissue.go @@ -16,20 +16,20 @@ import ( func (b *URLBuilder) GQLIssue(owner, repo string, iss *domain.Issue, format nodeid.Format) *gqlmodel.Issue { num := strconv.FormatInt(iss.Number, 10) out := &gqlmodel.Issue{ - ID: nodeid.Encode(nodeid.KindIssue, iss.ID, format), - Number: int32(iss.Number), - Title: iss.Title, - Body: deref(iss.Body), - State: issueState(iss.State), - URL: gqlmodel.URI(b.RepoHTML(owner, repo) + "/issues/" + num), - Locked: iss.Locked, - Closed: iss.State == "closed", - Author: b.gqlActor(iss.User, format), - CreatedAt: gqlmodel.NewDateTime(iss.CreatedAt), - UpdatedAt: gqlmodel.NewDateTime(iss.UpdatedAt), - Labels: b.gqlLabelConnection(owner, repo, iss.Labels, format), - Assignees: b.GQLUserConnection(iss.Assignees, format), - Milestone: b.GQLMilestone(owner, repo, iss.Milestone, format), + ID: nodeid.Encode(nodeid.KindIssue, iss.ID, format), + Number: int32(iss.Number), + Title: iss.Title, + Body: deref(iss.Body), + State: issueState(iss.State), + URL: gqlmodel.URI(b.RepoHTML(owner, repo) + "/issues/" + num), + Locked: iss.Locked, + Closed: iss.State == "closed", + Author: b.gqlActor(iss.User, format), + CreatedAt: gqlmodel.NewDateTime(iss.CreatedAt), + UpdatedAt: gqlmodel.NewDateTime(iss.UpdatedAt), + Labels: b.gqlLabelConnection(owner, repo, iss.Labels, format), + Assignees: b.GQLUserConnection(iss.Assignees, format), + Milestone: b.GQLMilestone(owner, repo, iss.Milestone, format), Comments: &gqlmodel.IssueCommentConnection{TotalCount: int32(iss.CommentsCount), PageInfo: &gqlmodel.PageInfo{}}, ReactionGroups: gqlReactionGroups(iss.Reactions), RepoOwner: owner, diff --git a/presenter/gqlmodel/issue.go b/presenter/gqlmodel/issue.go index ddc07a5..e2aa8ae 100644 --- a/presenter/gqlmodel/issue.go +++ b/presenter/gqlmodel/issue.go @@ -53,7 +53,7 @@ type ReactingUserConnection struct { TotalCount int32 } -// ReactionGroup summarises how many users reacted with a given emoji on an issue. +// ReactionGroup summarizes how many users reacted with a given emoji on an issue. type ReactionGroup struct { Content ReactionContent Users ReactingUserConnection diff --git a/presenter/gqlmodel/repository.go b/presenter/gqlmodel/repository.go index 0b3792f..a6e44e5 100644 --- a/presenter/gqlmodel/repository.go +++ b/presenter/gqlmodel/repository.go @@ -27,47 +27,47 @@ const ( // Nullable GraphQL fields use Go pointers so gqlgen renders null rather than a // zero value. type Repository struct { - ID string // the Repository node ID - Name string // the short repository name - NameWithOwner string // owner login + "/" + name - Description *string // null when unset - IsPrivate bool // visibility - IsFork bool // whether this is a fork - IsArchived bool // whether this is archived - IsEmpty bool // true when the repository has no commits - IsInOrganization bool // true when owner is an org - ForkCount int32 // number of forks - StargazerCount int32 // number of stars - DiskUsage *int32 // disk usage in KB, null when unavailable - HomepageURL *URI // null when unset - CreatedAt DateTime // creation instant - UpdatedAt DateTime // last metadata update - PushedAt *DateTime // last push, null for a repository with no commits - URL URI // the repository's HTML URL - SSHURL GitSSHRemote // the SSH clone URL, GitHub's GitSSHRemote scalar - DatabaseID *int32 // the integer database id REST calls id - Visibility RepositoryVisibility // PUBLIC, PRIVATE, or INTERNAL - ViewerPermission *RepositoryPermission // viewer's permission, null for anonymous - ViewerCanAdminister bool // whether the viewer can administer the repository - ViewerDefaultMergeMethod PullRequestMergeMethod // the viewer's default merge method - HasIssuesEnabled bool // whether the repository accepts issues - HasWikiEnabled bool // whether the repository has a wiki - HasProjectsEnabled bool // whether the repository has projects - HasDiscussionsEnabled bool // whether the repository has discussions (always false) - IsTemplate bool // whether the repository is a template - IsMirror bool // whether the repository is a mirror (always false) - MirrorURL *URI // the mirror source URL (always null) - DeleteBranchOnMerge bool // whether head branches are deleted on merge - AutoMergeAllowed bool // whether auto-merge can be enabled (always true) - MergeCommitAllowed bool // whether merge commits are allowed (always true) - SquashMergeAllowed bool // whether squash merges are allowed (always true) - RebaseMergeAllowed bool // whether rebase merges are allowed (always true) - DefaultBranchRef *Ref // the head branch, null for an empty repository - RepositoryTopics *RepositoryTopicConnection // the repository's topics - Watchers *UserConnection // the users watching the repository - Languages *LanguageConnection // detected languages (always empty) - IssueTemplates []*IssueTemplate // configured issue templates (always null) - PullRequestTemplates []*PullRequestTemplate // configured PR templates (always null) + ID string // the Repository node ID + Name string // the short repository name + NameWithOwner string // owner login + "/" + name + Description *string // null when unset + IsPrivate bool // visibility + IsFork bool // whether this is a fork + IsArchived bool // whether this is archived + IsEmpty bool // true when the repository has no commits + IsInOrganization bool // true when owner is an org + ForkCount int32 // number of forks + StargazerCount int32 // number of stars + DiskUsage *int32 // disk usage in KB, null when unavailable + HomepageURL *URI // null when unset + CreatedAt DateTime // creation instant + UpdatedAt DateTime // last metadata update + PushedAt *DateTime // last push, null for a repository with no commits + URL URI // the repository's HTML URL + SSHURL GitSSHRemote // the SSH clone URL, GitHub's GitSSHRemote scalar + DatabaseID *int32 // the integer database id REST calls id + Visibility RepositoryVisibility // PUBLIC, PRIVATE, or INTERNAL + ViewerPermission *RepositoryPermission // viewer's permission, null for anonymous + ViewerCanAdminister bool // whether the viewer can administer the repository + ViewerDefaultMergeMethod PullRequestMergeMethod // the viewer's default merge method + HasIssuesEnabled bool // whether the repository accepts issues + HasWikiEnabled bool // whether the repository has a wiki + HasProjectsEnabled bool // whether the repository has projects + HasDiscussionsEnabled bool // whether the repository has discussions (always false) + IsTemplate bool // whether the repository is a template + IsMirror bool // whether the repository is a mirror (always false) + MirrorURL *URI // the mirror source URL (always null) + DeleteBranchOnMerge bool // whether head branches are deleted on merge + AutoMergeAllowed bool // whether auto-merge can be enabled (always true) + MergeCommitAllowed bool // whether merge commits are allowed (always true) + SquashMergeAllowed bool // whether squash merges are allowed (always true) + RebaseMergeAllowed bool // whether rebase merges are allowed (always true) + DefaultBranchRef *Ref // the head branch, null for an empty repository + RepositoryTopics *RepositoryTopicConnection // the repository's topics + Watchers *UserConnection // the users watching the repository + Languages *LanguageConnection // detected languages (always empty) + IssueTemplates []*IssueTemplate // configured issue templates (always null) + PullRequestTemplates []*PullRequestTemplate // configured PR templates (always null) // RepoOwner, RepoName, and ForkParentID carry the repository coordinates for // resolvers that look up ref, milestone, release, or parent data on demand. diff --git a/presenter/gqlmodel/user.go b/presenter/gqlmodel/user.go index 932d317..ef8a8fe 100644 --- a/presenter/gqlmodel/user.go +++ b/presenter/gqlmodel/user.go @@ -3,22 +3,22 @@ package gqlmodel // User is the GraphQL User object, carrying the fields gh api and gh auth // status select. It grows toward the full GitHub User type milestone by milestone. type User struct { - ID string // the User node ID - Login string // the user's login - Name *string // display name, null when unset - Email *string // public email, null when unset - Bio *string // profile bio, null when unset - Company *string // profile company, null when unset - Location *string // profile location, null when unset - WebsiteURL *URI // profile blog/website URL, null when unset - TwitterUsername *string // Twitter/X handle without the @, null when unset - DatabaseID *int32 // the integer database id (REST id) - URL URI // the user's profile HTML URL - AvatarURL URI // the user's avatar URL; the size arg appends ?s= - ResourcePath URI // the path part of the profile URL, e.g. /octocat + ID string // the User node ID + Login string // the user's login + Name *string // display name, null when unset + Email *string // public email, null when unset + Bio *string // profile bio, null when unset + Company *string // profile company, null when unset + Location *string // profile location, null when unset + WebsiteURL *URI // profile blog/website URL, null when unset + TwitterUsername *string // Twitter/X handle without the @, null when unset + DatabaseID *int32 // the integer database id (REST id) + URL URI // the user's profile HTML URL + AvatarURL URI // the user's avatar URL; the size arg appends ?s= + ResourcePath URI // the path part of the profile URL, e.g. /octocat Status *UserStatus // the user's set status; always null today - CreatedAt DateTime // account creation instant - UpdatedAt DateTime // last-update instant + CreatedAt DateTime // account creation instant + UpdatedAt DateTime // last-update instant } // IsNode marks User as implementing the Node interface. diff --git a/presenter/gqlpull.go b/presenter/gqlpull.go index ab62b3f..9989ff2 100644 --- a/presenter/gqlpull.go +++ b/presenter/gqlpull.go @@ -202,7 +202,7 @@ func patchStatus(status string) gqlmodel.PatchStatus { // the coordinates the statusCheckRollup resolver reads back. func GQLCommit(repoDBID int64, owner, name string, c git.Commit) *gqlmodel.Commit { return &gqlmodel.Commit{ - ID: nodeid.EncodeGitObject("commit", repoDBID, string(c.SHA)), + ID: nodeid.EncodeGitObject("commit", repoDBID, c.SHA), Oid: gqlmodel.GitObjectID(c.SHA), Message: c.Message, MessageHeadline: messageHeadline(c.Message), diff --git a/presenter/gqlreaction.go b/presenter/gqlreaction.go index 3a28f84..9fa499d 100644 --- a/presenter/gqlreaction.go +++ b/presenter/gqlreaction.go @@ -48,7 +48,7 @@ func GQLReactionContent(rest string) gqlmodel.ReactionContent { return gqlmodel.ReactionContent(rest) } -// GQLReactionGroupsFromList summarises a list of domain reactions into the +// GQLReactionGroupsFromList summarizes a list of domain reactions into the // GraphQL reaction groups, the subject-level rollup addReaction and // removeReaction return after the change. The slice is non-nil even when empty. func GQLReactionGroupsFromList(rs []*domain.Reaction) []*gqlmodel.ReactionGroup { diff --git a/presenter/gqlrepository.go b/presenter/gqlrepository.go index 907a0d2..060159e 100644 --- a/presenter/gqlrepository.go +++ b/presenter/gqlrepository.go @@ -18,44 +18,44 @@ func (b *URLBuilder) GQLRepository(r *domain.Repo, branch *git.Branch, format no perm := gqlmodel.RepositoryPermissionAdmin dbID := int32(r.ID) repo := gqlmodel.Repository{ - ID: nodeid.Encode(nodeid.KindRepository, r.ID, format), - Name: r.Name, - NameWithOwner: r.FullName(), - Description: r.Description, - IsPrivate: r.Private, - IsFork: r.Fork, - IsArchived: r.Archived, - IsEmpty: r.PushedAt == nil, - IsInOrganization: false, // Githome does not yet model organizations - ForkCount: 0, // not stored - StargazerCount: 0, // not stored - HomepageURL: gqlHomepageURL(r.Homepage), - CreatedAt: gqlmodel.NewDateTime(r.CreatedAt), - UpdatedAt: gqlmodel.NewDateTime(r.UpdatedAt), - URL: gqlmodel.URI(b.RepoHTML(r.Owner.Login, r.Name)), - SSHURL: gqlmodel.GitSSHRemote(b.RepoGitSSH(r.Owner.Login, r.Name)), - DatabaseID: &dbID, - Visibility: gqlVisibility(r.Private), - ViewerPermission: &perm, // all authenticated users get ADMIN on their own repos - ViewerCanAdminister: true, + ID: nodeid.Encode(nodeid.KindRepository, r.ID, format), + Name: r.Name, + NameWithOwner: r.FullName(), + Description: r.Description, + IsPrivate: r.Private, + IsFork: r.Fork, + IsArchived: r.Archived, + IsEmpty: r.PushedAt == nil, + IsInOrganization: false, // Githome does not yet model organizations + ForkCount: 0, // not stored + StargazerCount: 0, // not stored + HomepageURL: gqlHomepageURL(r.Homepage), + CreatedAt: gqlmodel.NewDateTime(r.CreatedAt), + UpdatedAt: gqlmodel.NewDateTime(r.UpdatedAt), + URL: gqlmodel.URI(b.RepoHTML(r.Owner.Login, r.Name)), + SSHURL: gqlmodel.GitSSHRemote(b.RepoGitSSH(r.Owner.Login, r.Name)), + DatabaseID: &dbID, + Visibility: gqlVisibility(r.Private), + ViewerPermission: &perm, // all authenticated users get ADMIN on their own repos + ViewerCanAdminister: true, ViewerDefaultMergeMethod: gqlmodel.PullRequestMergeMethodMerge, - HasIssuesEnabled: r.HasIssues, - HasWikiEnabled: r.HasWiki, - HasProjectsEnabled: r.HasProjects, - HasDiscussionsEnabled: false, // discussions are not modeled - IsTemplate: r.IsTemplate, - IsMirror: false, // Githome does not mirror - DeleteBranchOnMerge: r.DeleteBranchOnMerge, - AutoMergeAllowed: true, - MergeCommitAllowed: true, - SquashMergeAllowed: true, - RebaseMergeAllowed: true, - RepositoryTopics: b.gqlRepositoryTopics(r), - Watchers: &gqlmodel.UserConnection{Nodes: []*gqlmodel.User{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0}, - Languages: &gqlmodel.LanguageConnection{Nodes: []*gqlmodel.Language{}, Edges: []*gqlmodel.LanguageEdge{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0, TotalSize: 0}, - RepoOwner: r.Owner.Login, - RepoName: r.Name, - ForkParentPK: r.ForkOfPK, + HasIssuesEnabled: r.HasIssues, + HasWikiEnabled: r.HasWiki, + HasProjectsEnabled: r.HasProjects, + HasDiscussionsEnabled: false, // discussions are not modeled + IsTemplate: r.IsTemplate, + IsMirror: false, // Githome does not mirror + DeleteBranchOnMerge: r.DeleteBranchOnMerge, + AutoMergeAllowed: true, + MergeCommitAllowed: true, + SquashMergeAllowed: true, + RebaseMergeAllowed: true, + RepositoryTopics: b.gqlRepositoryTopics(r), + Watchers: &gqlmodel.UserConnection{Nodes: []*gqlmodel.User{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0}, + Languages: &gqlmodel.LanguageConnection{Nodes: []*gqlmodel.Language{}, Edges: []*gqlmodel.LanguageEdge{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0, TotalSize: 0}, + RepoOwner: r.Owner.Login, + RepoName: r.Name, + ForkParentPK: r.ForkOfPK, } if r.PushedAt != nil { pushed := gqlmodel.NewDateTime(*r.PushedAt) diff --git a/presenter/notification.go b/presenter/notification.go index 13b4b52..1c35d6a 100644 --- a/presenter/notification.go +++ b/presenter/notification.go @@ -37,7 +37,6 @@ func (b *URLBuilder) NotificationThread(t *domain.NotificationThread, repo *doma } } -// NotificationSubscription renders the thread's subscription state. // RepoSubscription renders a repository subscription. The url is the // subscription endpoint and repository_url the repository itself, the pair // GitHub returns on this body. @@ -52,6 +51,7 @@ func (b *URLBuilder) RepoSubscription(s *domain.Subscription) restmodel.RepoSubs } } +// NotificationSubscription renders the thread's subscription state. func (b *URLBuilder) NotificationSubscription(t *domain.NotificationThread) restmodel.NotificationSubscription { id := strconv.FormatInt(t.ID, 10) return restmodel.NotificationSubscription{ diff --git a/presenter/restmodel/gist.go b/presenter/restmodel/gist.go index c08fd1e..6af7674 100644 --- a/presenter/restmodel/gist.go +++ b/presenter/restmodel/gist.go @@ -13,25 +13,25 @@ type GistFile struct { // Gist is the body returned by every gist endpoint. type Gist struct { - ID string `json:"id"` - NodeID string `json:"node_id"` - Description string `json:"description"` - Public bool `json:"public"` - Owner SimpleUser `json:"owner"` - User *SimpleUser `json:"user"` - Files map[string]GistFile `json:"files"` - Forks []any `json:"forks"` - History []any `json:"history"` - Truncated bool `json:"truncated"` - Comments int `json:"comments"` - GitPullURL string `json:"git_pull_url"` - GitPushURL string `json:"git_push_url"` - HTMLURL string `json:"html_url"` - CommitsURL string `json:"commits_url"` - ForksURL string `json:"forks_url"` - CommentsURL string `json:"comments_url"` - CreatedAt Time `json:"created_at"` - UpdatedAt Time `json:"updated_at"` + ID string `json:"id"` + NodeID string `json:"node_id"` + Description string `json:"description"` + Public bool `json:"public"` + Owner SimpleUser `json:"owner"` + User *SimpleUser `json:"user"` + Files map[string]GistFile `json:"files"` + Forks []any `json:"forks"` + History []any `json:"history"` + Truncated bool `json:"truncated"` + Comments int `json:"comments"` + GitPullURL string `json:"git_pull_url"` + GitPushURL string `json:"git_push_url"` + HTMLURL string `json:"html_url"` + CommitsURL string `json:"commits_url"` + ForksURL string `json:"forks_url"` + CommentsURL string `json:"comments_url"` + CreatedAt Time `json:"created_at"` + UpdatedAt Time `json:"updated_at"` } // GistComment is the body returned by gist comment endpoints. diff --git a/presenter/restmodel/search.go b/presenter/restmodel/search.go index 9e9f8bc..15ca1b3 100644 --- a/presenter/restmodel/search.go +++ b/presenter/restmodel/search.go @@ -65,12 +65,12 @@ type SearchCode struct { // the blob object id, the API and HTML URLs that address it, the repository it // lives in, and the score. type CodeSearchItem struct { - Name string `json:"name"` - Path string `json:"path"` - SHA string `json:"sha"` - URL string `json:"url"` - GitURL string `json:"git_url"` - HTMLURL string `json:"html_url"` + Name string `json:"name"` + Path string `json:"path"` + SHA string `json:"sha"` + URL string `json:"url"` + GitURL string `json:"git_url"` + HTMLURL string `json:"html_url"` Repository Repository `json:"repository"` Score float64 `json:"score"` TextMatches []TextMatch `json:"text_matches,omitempty"` diff --git a/store/migrate_install_test.go b/store/migrate_install_test.go index cf66f8e..9c8c1db 100644 --- a/store/migrate_install_test.go +++ b/store/migrate_install_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "os" "path/filepath" "sort" "strings" @@ -91,6 +92,71 @@ func TestInstallStampsLedger(t *testing.T) { }) } +// TestPostgresSchemaFileMatchesMigrations is the drift guard for schema.pg.sql, +// the Postgres counterpart to TestSchemaFileMatchesMigrations. A database built +// by Install (the consolidated schema file) must have the same structure as one +// built by replaying every migration. It runs only where a Postgres DSN is set. +// schema.pg.sql is maintained by hand, so this catches a migration that was +// added without folding its tables and columns into the consolidated file. +func TestPostgresSchemaFileMatchesMigrations(t *testing.T) { + dsn := os.Getenv("GITHOME_TEST_POSTGRES_DSN") + if dsn == "" { + t.Skip("set GITHOME_TEST_POSTGRES_DSN to run the Postgres schema drift guard") + } + ctx := context.Background() + + fingerprint := func(build func(*store.Store) error) string { + t.Helper() + st, err := store.Open(ctx, dsn) + if err != nil { + t.Fatalf("open: %v", err) + } + defer func() { _ = st.Close() }() + // Reach an empty schema before building, so a reused database does not + // taint the comparison. + if err := st.Rollback(ctx, 1<<20); err != nil { + t.Fatalf("reset: %v", err) + } + if err := build(st); err != nil { + t.Fatalf("build: %v", err) + } + return pgFingerprint(t, st.DB()) + } + + want := fingerprint(func(st *store.Store) error { return st.Migrate(ctx) }) + got := fingerprint(func(st *store.Store) error { return st.Install(ctx) }) + if want != got { + t.Fatalf("schema.pg.sql drifted from the migrations.\n--- migrated ---\n%s\n--- installed ---\n%s", want, got) + } +} + +// pgFingerprint renders a stable description of a Postgres database's structure: +// every application table's columns (type, nullability, default, and any +// generated expression) and every index, keyed and ordered by name so the +// column order Install and Migrate happen to write in does not matter. The +// schema_migrations ledger is excluded so the two build paths are compared on +// application schema alone. +func pgFingerprint(t *testing.T, db *sql.DB) string { + t.Helper() + var b strings.Builder + cols := queryStrings(t, db, `SELECT table_name||'.'||column_name||' '||data_type + ||' null='||is_nullable + ||' dflt='||coalesce(column_default,'') + ||' gen='||coalesce(generation_expression,'') + FROM information_schema.columns + WHERE table_schema='public' AND table_name <> 'schema_migrations' + ORDER BY 1`) + for _, c := range cols { + fmt.Fprintf(&b, "COLUMN %s\n", c) + } + idx := queryStrings(t, db, `SELECT indexname||' :: '||indexdef FROM pg_indexes + WHERE schemaname='public' AND tablename <> 'schema_migrations' ORDER BY 1`) + for _, i := range idx { + fmt.Fprintf(&b, "INDEX %s\n", i) + } + return b.String() +} + // sqliteFingerprint renders a stable, whitespace-normalized description of a // SQLite database's structure: every user table's columns, every named index, // and every trigger. The schema_migrations ledger and SQLite's internal objects diff --git a/store/migrations/0012_github_apps.pg.down.sql b/store/migrations/0012_github_apps.pg.down.sql index 600721f..60e6154 100644 --- a/store/migrations/0012_github_apps.pg.down.sql +++ b/store/migrations/0012_github_apps.pg.down.sql @@ -1,7 +1,9 @@ --- 0012_github_apps (postgres, down) -DROP TABLE IF EXISTS installation_repositories; -DROP TABLE IF EXISTS installations; -DROP TABLE IF EXISTS github_apps; +-- 0012_github_apps (postgres, down): drop the token linkage columns first so the +-- foreign keys they carry stop pinning installations and github_apps, then drop +-- the tables in reverse dependency order. ALTER TABLE tokens DROP COLUMN IF EXISTS installation_pk; ALTER TABLE tokens DROP COLUMN IF EXISTS github_app_pk; ALTER TABLE tokens DROP COLUMN IF EXISTS grant_json; +DROP TABLE IF EXISTS installation_repositories; +DROP TABLE IF EXISTS installations; +DROP TABLE IF EXISTS github_apps; diff --git a/store/migrations/0020_code_search.pg.up.sql b/store/migrations/0020_code_search.pg.up.sql index b2c0ec9..e93b322 100644 --- a/store/migrations/0020_code_search.pg.up.sql +++ b/store/migrations/0020_code_search.pg.up.sql @@ -3,6 +3,12 @@ -- dictionary does no stemming or stop-word removal, matching FTS5's tokenizer -- closely enough that both dialects answer the same queries. The indexer -- replaces a repository's rows wholesale on reindex. +-- +-- The path is split on its punctuation before tokenizing so a query for one +-- segment ("logo") finds "assets/logo.png". Postgres's text parser otherwise +-- keeps a slash-joined path as a single "file" token, which FTS5's unicode61 +-- tokenizer would have split on every non-word character. Replacing those +-- characters with spaces lines the two dialects up. CREATE TABLE code_documents ( repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, @@ -10,7 +16,7 @@ CREATE TABLE code_documents ( sha TEXT NOT NULL, content TEXT NOT NULL DEFAULT '', search_vector tsvector GENERATED ALWAYS AS ( - to_tsvector('simple', path || ' ' || content) + to_tsvector('simple', regexp_replace(path, '[^A-Za-z0-9]+', ' ', 'g') || ' ' || content) ) STORED, PRIMARY KEY (repo_pk, path) ); diff --git a/store/migrations/schema.pg.sql b/store/migrations/schema.pg.sql index b655a92..9950d8c 100644 --- a/store/migrations/schema.pg.sql +++ b/store/migrations/schema.pg.sql @@ -483,6 +483,229 @@ CREATE TABLE release_assets ( ); CREATE UNIQUE INDEX release_assets_release_name_uq ON release_assets (release_pk, name) WHERE deleted_at IS NULL; +-- 0012_github_apps +CREATE TABLE github_apps ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE DEFAULT nextval('global_id_seq'), + owner_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + slug TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + client_id TEXT NOT NULL UNIQUE, + client_secret_hash BYTEA, + webhook_secret_hash BYTEA, + private_key_pem BYTEA NOT NULL, + permissions JSONB NOT NULL DEFAULT '{}', + events TEXT[] NOT NULL DEFAULT '{}', + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE TABLE installations ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE DEFAULT nextval('global_id_seq'), + app_pk BIGINT NOT NULL REFERENCES github_apps(pk) ON DELETE CASCADE, + account_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + repository_selection TEXT NOT NULL DEFAULT 'all', + permissions JSONB NOT NULL DEFAULT '{}', + events TEXT[] NOT NULL DEFAULT '{}', + suspended_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX installations_app_account_uq ON installations (app_pk, account_pk); + +CREATE TABLE installation_repositories ( + installation_pk BIGINT NOT NULL REFERENCES installations(pk) ON DELETE CASCADE, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + PRIMARY KEY (installation_pk, repo_pk) +); + +ALTER TABLE tokens ADD COLUMN installation_pk BIGINT REFERENCES installations(pk) ON DELETE CASCADE; +ALTER TABLE tokens ADD COLUMN github_app_pk BIGINT REFERENCES github_apps(pk) ON DELETE CASCADE; +ALTER TABLE tokens ADD COLUMN grant_json TEXT; + +-- 0013_keys_protection +CREATE TABLE ssh_keys ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE DEFAULT nextval('global_id_seq'), + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + title TEXT, + key_type TEXT NOT NULL, + public_key TEXT NOT NULL, + fingerprint TEXT NOT NULL, + read_only BOOLEAN NOT NULL DEFAULT FALSE, + repo_pk BIGINT REFERENCES repositories(pk) ON DELETE CASCADE, + last_used_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX ssh_keys_fp_uq ON ssh_keys (fingerprint); + +CREATE TABLE branch_protections ( + pk BIGSERIAL PRIMARY KEY, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + branch_pattern TEXT NOT NULL, + require_pr_reviews BOOLEAN NOT NULL DEFAULT FALSE, + required_approving_count INT NOT NULL DEFAULT 0, + dismiss_stale_reviews BOOLEAN NOT NULL DEFAULT FALSE, + require_code_owner_reviews BOOLEAN NOT NULL DEFAULT FALSE, + require_status_checks BOOLEAN NOT NULL DEFAULT FALSE, + require_branches_up_to_date BOOLEAN NOT NULL DEFAULT FALSE, + status_check_contexts TEXT[] NOT NULL DEFAULT '{}', + enforce_admins BOOLEAN NOT NULL DEFAULT FALSE, + restrictions_users TEXT[] NOT NULL DEFAULT '{}', + restrictions_teams TEXT[] NOT NULL DEFAULT '{}', + allow_force_pushes BOOLEAN NOT NULL DEFAULT FALSE, + allow_deletions BOOLEAN NOT NULL DEFAULT FALSE, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX branch_protections_repo_pattern_uq ON branch_protections (repo_pk, branch_pattern); + +-- 0014_teams_collab_topics +ALTER TABLE repositories ADD COLUMN IF NOT EXISTS topics TEXT NOT NULL DEFAULT '[]'; + +CREATE TABLE IF NOT EXISTS teams ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE, + org_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + name TEXT NOT NULL, + slug TEXT NOT NULL, + description TEXT, + privacy TEXT NOT NULL DEFAULT 'secret', + permission TEXT NOT NULL DEFAULT 'pull', + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX IF NOT EXISTS teams_org_slug_uq ON teams (org_pk, slug); + +CREATE TABLE IF NOT EXISTS team_members ( + pk BIGSERIAL PRIMARY KEY, + team_pk BIGINT NOT NULL REFERENCES teams(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + role TEXT NOT NULL DEFAULT 'member', + UNIQUE (team_pk, user_pk) +); + +CREATE TABLE IF NOT EXISTS team_repos ( + pk BIGSERIAL PRIMARY KEY, + team_pk BIGINT NOT NULL REFERENCES teams(pk) ON DELETE CASCADE, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + permission TEXT NOT NULL DEFAULT 'pull', + UNIQUE (team_pk, repo_pk) +); + +CREATE TABLE IF NOT EXISTS collaborators ( + pk BIGSERIAL PRIMARY KEY, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + permission TEXT NOT NULL DEFAULT 'push', + UNIQUE (repo_pk, user_pk) +); + +-- 0015_oauth_auth_codes +CREATE TABLE IF NOT EXISTS oauth_auth_codes ( + pk BIGSERIAL PRIMARY KEY, + code_hash BYTEA NOT NULL UNIQUE, + oauth_app_pk BIGINT NOT NULL REFERENCES oauth_apps(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + redirect_uri TEXT NOT NULL, + scopes TEXT NOT NULL DEFAULT '', + used BOOLEAN NOT NULL DEFAULT false, + expires_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- 0016_gists +CREATE TABLE gists ( + pk BIGSERIAL PRIMARY KEY, + gist_id TEXT NOT NULL UNIQUE, + owner_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + description TEXT NOT NULL DEFAULT '', + public BOOLEAN NOT NULL DEFAULT TRUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX gists_owner ON gists (owner_pk); + +CREATE TABLE gist_files ( + pk BIGSERIAL PRIMARY KEY, + gist_pk BIGINT NOT NULL REFERENCES gists(pk) ON DELETE CASCADE, + filename TEXT NOT NULL, + content TEXT NOT NULL DEFAULT '', + UNIQUE(gist_pk, filename) +); + +CREATE TABLE gist_stars ( + gist_pk BIGINT NOT NULL REFERENCES gists(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + PRIMARY KEY(gist_pk, user_pk) +); + +CREATE TABLE gist_comments ( + pk BIGSERIAL PRIMARY KEY, + gist_pk BIGINT NOT NULL REFERENCES gists(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + body TEXT NOT NULL DEFAULT '', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- 0017_oauth_app_callback +ALTER TABLE oauth_apps ADD COLUMN IF NOT EXISTS callback_url TEXT NOT NULL DEFAULT ''; + +-- 0018_protection_restrictions +ALTER TABLE branch_protections ADD COLUMN IF NOT EXISTS restrictions_enabled BOOLEAN NOT NULL DEFAULT FALSE; + +-- 0019_notifications +CREATE TABLE IF NOT EXISTS notification_threads ( + pk BIGSERIAL PRIMARY KEY, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + issue_pk BIGINT NOT NULL REFERENCES issues(pk) ON DELETE CASCADE, + reason TEXT NOT NULL DEFAULT 'subscribed', + unread BOOLEAN NOT NULL DEFAULT TRUE, + subscribed BOOLEAN NOT NULL DEFAULT TRUE, + ignored BOOLEAN NOT NULL DEFAULT FALSE, + last_read_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX IF NOT EXISTS notification_threads_user_issue_uq ON notification_threads (user_pk, issue_pk); +CREATE INDEX IF NOT EXISTS notification_threads_user_updated ON notification_threads (user_pk, updated_at); + +-- 0020_code_search +CREATE TABLE code_documents ( + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + path TEXT NOT NULL, + sha TEXT NOT NULL, + content TEXT NOT NULL DEFAULT '', + search_vector tsvector GENERATED ALWAYS AS ( + to_tsvector('simple', regexp_replace(path, '[^A-Za-z0-9]+', ' ', 'g') || ' ' || content) + ) STORED, + PRIMARY KEY (repo_pk, path) +); +CREATE INDEX code_documents_search_vector_gin ON code_documents USING GIN (search_vector); + +CREATE TABLE code_index_state ( + repo_pk BIGINT PRIMARY KEY REFERENCES repositories(pk) ON DELETE CASCADE, + head_sha TEXT NOT NULL, + truncated BOOLEAN NOT NULL DEFAULT FALSE, + indexed_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- 0021_list_indexes +CREATE INDEX issues_repo_updated_number_idx + ON issues (repo_pk, updated_at DESC, number DESC) + WHERE deleted_at IS NULL; +CREATE INDEX issues_repo_comments_number_idx + ON issues (repo_pk, comments_count DESC, number DESC) + WHERE deleted_at IS NULL; +CREATE INDEX issue_labels_label_idx ON issue_labels (label_pk); +CREATE INDEX gists_public_updated_idx ON gists (public, updated_at DESC); +DROP INDEX issue_comments_issue_idx; +CREATE INDEX issue_comments_issue_created_pk_idx + ON issue_comments (issue_pk, created_at, pk) + WHERE deleted_at IS NULL; + + -- 0022_repo_redirects CREATE TABLE repo_redirects ( pk BIGSERIAL PRIMARY KEY, diff --git a/store/queries_code.go b/store/queries_code.go index 3fd13d9..885aadb 100644 --- a/store/queries_code.go +++ b/store/queries_code.go @@ -62,6 +62,17 @@ func (s *Store) codeTable() string { return "code_fts" } +// codePathOrder returns the ORDER BY expression for path. SQLite sorts text +// bytewise (BINARY); Postgres sorts by the cluster's collation, which orders +// mixed-case paths differently. Pinning the Postgres sort to the "C" collation +// makes both dialects page in the same byte order. +func (s *Store) codePathOrder() string { + if s.dialect == DialectPostgres { + return `path COLLATE "C"` + } + return "path" +} + // SearchCode returns the page of indexed files matching the query, ordered by // repository then path so pagination is deterministic. func (s *Store) SearchCode(ctx context.Context, q CodeSearch) ([]CodeHit, error) { @@ -78,7 +89,7 @@ func (s *Store) SearchCode(ctx context.Context, q CodeSearch) ([]CodeHit, error) } args = append(args, limit, q.Offset) sql := s.rebind(`SELECT repo_pk, path, sha FROM ` + s.codeTable() + where + - ` ORDER BY repo_pk, path LIMIT ? OFFSET ?`) + ` ORDER BY repo_pk, ` + s.codePathOrder() + ` LIMIT ? OFFSET ?`) rows, err := s.rdb.QueryContext(ctx, sql, args...) if err != nil { return nil, err diff --git a/store/queries_github_app.go b/store/queries_github_app.go index d85c6b4..b843446 100644 --- a/store/queries_github_app.go +++ b/store/queries_github_app.go @@ -112,7 +112,7 @@ func (s *Store) InstallationsByAppPK(ctx context.Context, appPK int64) ([]*Insta if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*InstallationRow for rows.Next() { r, err := scanInstallation(rows) @@ -140,7 +140,7 @@ func (s *Store) InstallationRepoPKs(ctx context.Context, instPK int64) ([]int64, if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []int64 for rows.Next() { var pk int64 @@ -173,9 +173,9 @@ func scanGitHubApp(row interface{ Scan(...any) error }) (*GitHubAppRow, error) { func scanInstallation(row interface{ Scan(...any) error }) (*InstallationRow, error) { var ( - r InstallationRow - suspended nullTime - created nullTime + r InstallationRow + suspended nullTime + created nullTime ) err := row.Scan( &r.PK, &r.DBID, &r.AppPK, &r.AccountPK, &r.RepositorySelection, diff --git a/store/queries_keys.go b/store/queries_keys.go index 6d19a58..6d131e1 100644 --- a/store/queries_keys.go +++ b/store/queries_keys.go @@ -156,7 +156,7 @@ func scanSSHKeys(rows *sql.Rows, err error) ([]*SSHKeyRow, error) { if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*SSHKeyRow for rows.Next() { k, err := scanSSHKeyRow(rows) diff --git a/store/queries_notifications.go b/store/queries_notifications.go index 8edfb9f..7889d3e 100644 --- a/store/queries_notifications.go +++ b/store/queries_notifications.go @@ -76,7 +76,7 @@ func (s *Store) ListNotificationThreads(ctx context.Context, userPK, repoPK int6 if err != nil { return nil, 0, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*NotificationThreadRow for rows.Next() { t, err := scanNotificationThreadRow(rows) diff --git a/store/queries_repo.go b/store/queries_repo.go index 57af3c6..37121b4 100644 --- a/store/queries_repo.go +++ b/store/queries_repo.go @@ -220,7 +220,7 @@ func (s *Store) ReposByOwner(ctx context.Context, ownerPK int64) ([]*RepoRow, er if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -247,7 +247,7 @@ func (s *Store) ListPublicRepos(ctx context.Context, sinceDBID int64, limit int) if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -294,7 +294,7 @@ func (s *Store) ForksOf(ctx context.Context, pk int64) ([]*RepoRow, error) { if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -319,7 +319,7 @@ func (s *Store) ReposByCollaborator(ctx context.Context, userPK int64) ([]*RepoR if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -345,7 +345,7 @@ func (s *Store) ReposByTeamMember(ctx context.Context, userPK int64) ([]*RepoRow if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) diff --git a/store/store_test.go b/store/store_test.go index 5d67a8e..25fe3ee 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -189,7 +189,7 @@ func TestReadsRunConcurrentWithWriteTx(t *testing.T) { release := make(chan struct{}) txDone := make(chan error, 1) go func() { - txDone <- st.WithTx(ctx, func(tx *store.Tx) error { + txDone <- st.WithTx(ctx, func(_ *store.Tx) error { close(txStarted) <-release return nil