diff --git a/bin/.golangci-lint-1.51.1.pkg b/bin/.golangci-lint-1.54.1.pkg similarity index 100% rename from bin/.golangci-lint-1.51.1.pkg rename to bin/.golangci-lint-1.54.1.pkg diff --git a/bin/golangci-lint b/bin/golangci-lint index 2237293..726c591 120000 --- a/bin/golangci-lint +++ b/bin/golangci-lint @@ -1 +1 @@ -.golangci-lint-1.51.1.pkg \ No newline at end of file +.golangci-lint-1.54.1.pkg \ No newline at end of file diff --git a/frontend/components/Review/PageChrome.vue b/frontend/components/Review/PageChrome.vue index acebb99..1817b18 100644 --- a/frontend/components/Review/PageChrome.vue +++ b/frontend/components/Review/PageChrome.vue @@ -44,6 +44,7 @@ const { pending, data, error } = await useFetch("/api/review", { lazy: true, params: Object.assign({}, route.params, { refresh: route.query.refresh, + kind: "review", }), server: false, }); diff --git a/server/cmd/server/api.go b/server/cmd/server/api.go index 75868f5..b1a8ed1 100644 --- a/server/cmd/server/api.go +++ b/server/cmd/server/api.go @@ -1,8 +1,11 @@ package main import ( + "fmt" + "github.com/pkg/errors" + "github.com/stanistan/present-me/internal/api" "github.com/stanistan/present-me/internal/github" "github.com/stanistan/present-me/internal/http" ) @@ -47,8 +50,17 @@ var apiRoutes = http.Routes( // GET /review hydrates the the full review from github. http.GET("/review", func(r *http.Request) (*http.JSONResponse, error) { - source := github.ReviewAPISource{ - ReviewParamsMap: github.NewReviewParamsMap(r.URL.Query()), + params := github.NewReviewParamsMap(r.URL.Query()) + var source api.Source + switch params.Kind { + case "review": + source = &github.ReviewAPISource{ReviewParamsMap: params} + case "prme": + source = &github.CommentsAPISource{ReviewParamsMap: params} + case "": + source = &api.ErrSource{Err: fmt.Errorf("missing kind")} + default: + source = &api.ErrSource{Err: fmt.Errorf("invalid kind: %s", params.Kind)} } review, err := source.GetReview(r.Context()) diff --git a/server/cmd/server/main.go b/server/cmd/server/main.go index b0a7c6f..4068a48 100644 --- a/server/cmd/server/main.go +++ b/server/cmd/server/main.go @@ -2,7 +2,6 @@ package main import ( "context" - "fmt" "net/http" "time" @@ -15,27 +14,24 @@ import ( "github.com/stanistan/present-me/internal/github" ) -var ( - version = "development" // dynamically linked -) +const name = "present-me" + +var version = "development" // dynamically linked func main() { var config pm.Config - _ = kong.Parse( - &config, - kong.Name("present-me"), - kong.UsageOnError(), - kong.Description(fmt.Sprintf("build version: %s", version)), - ) + _ = kong.Parse(&config, + kong.Name(name), + kong.Vars{"version": version}, + kong.UsageOnError()) - // 0. Standard Deps - // - logger, - // - ctx withLogger - // - disk cache - // - github client - log := config.Logger() - ctx := log.WithContext(context.Background()) + // set up our logger + ctx, log := config.Logger(context.Background()) + + // get our cache setup for the server diskCache := config.Cache(ctx) + + // configure our github client gh, err := config.GithubClient(ctx) if err != nil { log.Fatal().Err(err).Msg("could not configure GH client") @@ -47,6 +43,7 @@ func main() { api := r.PathPrefix("/api").Subrouter() api.Use( hlog.NewHandler(log), + hlog.URLHandler("url"), github.Middleware(gh), cache.Middleware(diskCache, func(r *http.Request) *cache.Options { return &cache.Options{ diff --git a/server/config.go b/server/config.go index 65db7bf..940b5fc 100644 --- a/server/config.go +++ b/server/config.go @@ -11,6 +11,11 @@ import ( "github.com/stanistan/present-me/internal/log" ) +// Config is the main configuration wrapper struct for +// the present-me application. +// +// It is basically the DI provider for all of the runtime +// dependencies and how to configure them. type Config struct { ServeConfig Log log.Config `embed:"" prefix:"log-"` @@ -23,8 +28,9 @@ func (c *Config) GithubClient(ctx context.Context) (*github.Client, error) { return g, errors.WithStack(err) } -func (c *Config) Logger() zerolog.Logger { - return log.NewLogger(c.Log) +func (c *Config) Logger(ctx context.Context) (context.Context, zerolog.Logger) { + l := log.NewLogger(c.Log) + return l.WithContext(ctx), l } func (c *Config) Cache(ctx context.Context) *cache.Cache { diff --git a/server/go.mod b/server/go.mod index 379277e..f37a2da 100644 --- a/server/go.mod +++ b/server/go.mod @@ -3,7 +3,7 @@ module github.com/stanistan/present-me go 1.20 require ( - github.com/alecthomas/kong v0.7.2-0.20230225013903-a9be85c4d3d6 + github.com/alecthomas/kong v0.8.0 github.com/bradleyfalzon/ghinstallation/v2 v2.1.0 github.com/cespare/reflex v0.3.1 github.com/google/go-github/v52 v52.0.0 diff --git a/server/go.sum b/server/go.sum index e47cac0..3513a2e 100644 --- a/server/go.sum +++ b/server/go.sum @@ -1,8 +1,8 @@ github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 h1:wPbRQzjjwFc0ih8puEVAOFGELsn1zoIIYdxvML7mDxA= github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g= github.com/alecthomas/assert/v2 v2.1.0 h1:tbredtNcQnoSd3QBhQWI7QZ3XHOVkw1Moklp2ojoH/0= -github.com/alecthomas/kong v0.7.2-0.20230225013903-a9be85c4d3d6 h1:yZxc3zrUYRv+1w6sqPX+0s7lfBasfojCN/SeZ2wueXk= -github.com/alecthomas/kong v0.7.2-0.20230225013903-a9be85c4d3d6/go.mod h1:n1iCIO2xS46oE8ZfYCNDqdR0b0wZNrXAIAqro/2132U= +github.com/alecthomas/kong v0.8.0 h1:ryDCzutfIqJPnNn0omnrgHLbAggDQM2VWHikE1xqK7s= +github.com/alecthomas/kong v0.8.0/go.mod h1:n1iCIO2xS46oE8ZfYCNDqdR0b0wZNrXAIAqro/2132U= github.com/alecthomas/repr v0.1.0 h1:ENn2e1+J3k09gyj2shc0dHr/yjaWSHRlrJ4DPMevDqE= github.com/bradleyfalzon/ghinstallation/v2 v2.1.0 h1:5+NghM1Zred9Z078QEZtm28G/kfDfZN/92gkDlLwGVA= github.com/bradleyfalzon/ghinstallation/v2 v2.1.0/go.mod h1:Xg3xPRN5Mcq6GDqeUVhFbjEWMb4JHCyWEeeBGEYQoTU= diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 46f2d8b..5550a74 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -41,6 +41,7 @@ func (r *Review) GetReview(_ context.Context) (Review, error) { return *r, nil } +// Comment (basically a standard card), is a named and annotated code block. type Comment struct { Number int `json:"number"` Title MaybeLinked `json:"title"` @@ -48,6 +49,7 @@ type Comment struct { CodeBlock CodeBlock `json:"code"` } +// CodeBlock is the code block and its diff. type CodeBlock struct { Content string `json:"content"` Language string `json:"lang"` diff --git a/server/internal/api/default.go b/server/internal/api/default.go new file mode 100644 index 0000000..8e77b8a --- /dev/null +++ b/server/internal/api/default.go @@ -0,0 +1,15 @@ +package api + +import ( + "context" +) + +type ErrSource struct { + Err error +} + +var _ Source = &ErrSource{} + +func (s *ErrSource) GetReview(_ context.Context) (Review, error) { + return Review{}, s.Err +} diff --git a/server/internal/errors/errors.go b/server/internal/errors/errors.go index 3a55717..4a6c5a5 100644 --- a/server/internal/errors/errors.go +++ b/server/internal/errors/errors.go @@ -61,3 +61,7 @@ func WrapErr(err error) *Error { HttpCode: 500, } } + +func New(msg string) error { + return errors.New(msg) +} diff --git a/server/internal/github/api_source_prme.go b/server/internal/github/api_source_prme.go new file mode 100644 index 0000000..7f79ca5 --- /dev/null +++ b/server/internal/github/api_source_prme.go @@ -0,0 +1,68 @@ +package github + +import ( + "context" + "fmt" + "strings" + + "github.com/stanistan/present-me/internal/api" + "github.com/stanistan/present-me/internal/errors" +) + +// CommentsAPISource can construct a api.Review from a tag +// on a pull requests' comments. +type CommentsAPISource struct { + ReviewParamsMap ReviewParamsMap +} + +var _ api.Source = &CommentsAPISource{} + +func (s *CommentsAPISource) GetReview(ctx context.Context) (api.Review, error) { + params, err := ReviewParamsFromMap(s.ReviewParamsMap) + if err != nil { + return api.Review{}, errors.WithStack(err) + } + + model, err := FetchReviewModel(ctx, params, CommentMatchesTag(s.ReviewParamsMap.Tag)) + if err != nil { + return api.Review{}, errors.WithStack(err) + } + + var body []string + if model.PR.Body != nil && *model.PR.Body != "" { + body = append(body, *model.PR.Body) + } + + return api.Review{ + Title: api.MaybeLinked{ + Text: fmt.Sprintf("%s (#%d)", *model.PR.Title, *model.PR.Number), + HRef: *model.PR.Links.HTML.HRef, + }, + Links: []api.LabelledLink{ + { + Label: "Author", + MaybeLinked: api.MaybeLinked{ + Text: *model.PR.User.Login, + HRef: *model.PR.User.HTMLURL, + }, + }, + { + Label: "Pull Request", + MaybeLinked: api.MaybeLinked{ + Text: fmt.Sprintf( + "%s/%s/pull/%d", + params.Owner, + params.Repo, + params.Pull, + ), + HRef: *model.PR.HTMLURL, + }, + }, + }, + MetaData: map[string]any{ + "params": s.ReviewParamsMap, + }, + Body: strings.Join(body, "\n\n---\n\n"), + Comments: transformComments(model.Comments), + }, nil +} diff --git a/server/internal/github/review_api_source.go b/server/internal/github/api_source_review.go similarity index 63% rename from server/internal/github/review_api_source.go rename to server/internal/github/api_source_review.go index 9961f1d..b38d3d8 100644 --- a/server/internal/github/review_api_source.go +++ b/server/internal/github/api_source_review.go @@ -3,7 +3,6 @@ package github import ( "context" "fmt" - "path" "strings" "github.com/stanistan/present-me/internal/api" @@ -22,12 +21,11 @@ func (s *ReviewAPISource) GetReview(ctx context.Context) (api.Review, error) { return api.Review{}, errors.WithStack(err) } - gh, ok := Ctx(ctx) - if !ok || gh == nil { - return api.Review{}, fmt.Errorf("need gh context") + if params.ReviewID == 0 { + return api.Review{}, errors.New("review must have review id") } - model, err := params.Model(ctx, gh) + model, err := FetchReviewModel(ctx, params, CommentMatchesReview(params.ReviewID)) if err != nil { return api.Review{}, errors.WithStack(err) } @@ -81,51 +79,3 @@ func (s *ReviewAPISource) GetReview(ctx context.Context) (api.Review, error) { Comments: transformComments(model.Comments), }, nil } - -func transformComments(cs []*PullRequestComment) []api.Comment { - out := make([]api.Comment, len(cs)) - for idx, c := range cs { - c := c - out[idx] = api.Comment{ - Number: idx + 1, - Title: api.MaybeLinked{ - Text: *c.Path, - HRef: *c.HTMLURL, - }, - Description: commentBody(*c.Body), - CodeBlock: api.CodeBlock{ - IsDiff: true, - Content: *c.DiffHunk, - Language: detectLanguage(*c.Path), - }, - } - } - return out -} - -func commentBody(s string) string { - return trimStartsWithNumber(s) -} - -func detectLanguage(p string) string { - var ( - base = path.Base(p) - ext = path.Ext(p) - ) - - switch ext { - case "": - if base == "Dockerfile" { - return "docker" - } - return "bash" - case ".rs": - return "rust" - case ".vue": - return "html" - case ".Dockerfile": - return "docker" - } - - return ext[1:] -} diff --git a/server/internal/github/comment.go b/server/internal/github/comment.go new file mode 100644 index 0000000..261ecdc --- /dev/null +++ b/server/internal/github/comment.go @@ -0,0 +1,29 @@ +package github + +import "github.com/stanistan/present-me/internal/api" + +func transformComments(cs []*PullRequestComment) []api.Comment { + out := make([]api.Comment, len(cs)) + for idx, c := range cs { + c := c + out[idx] = api.Comment{ + Number: idx + 1, + Title: api.MaybeLinked{ + Text: *c.Path, + HRef: *c.HTMLURL, + }, + Description: commentBody(*c.Body), + CodeBlock: api.CodeBlock{ + IsDiff: true, + Content: *c.DiffHunk, + Language: detectLanguage(*c.Path), + }, + } + } + return out +} + +func commentBody(s string) string { + out := startsWithNumberRegexp.ReplaceAllString(s, "") + return prmeTagRegexp.ReplaceAllString(out, "") +} diff --git a/server/internal/github/diff.go b/server/internal/github/diff.go index 1e6888b..40e5cfd 100644 --- a/server/internal/github/diff.go +++ b/server/internal/github/diff.go @@ -101,7 +101,6 @@ func (p *diffScanner) filter(lines []string, auto bool) []string { } func diffRange(c *PullRequestComment) (int, int, bool, error) { - // - endLine is the line that the comment is on or after, // - startLine is the beginning line that we'll include in our diff, // and it looks like github defaults to 4 lines included if there is diff --git a/server/internal/github/github.go b/server/internal/github/github.go index e4e855a..a555bf0 100644 --- a/server/internal/github/github.go +++ b/server/internal/github/github.go @@ -60,14 +60,43 @@ func New(ctx context.Context, opts ClientOptions) (*Client, error) { return &Client{c: github.NewClient(c)}, nil } +type Fetch[T any] func() (T, error) + +func listApply[T any]( + ctx context.Context, + k cache.DataKey, + fetch Fetch[[]T], + predicate Pred[T], +) ([]T, error) { + items, err := cache.Apply(ctx, k, fetch) + if err != nil { + return nil, errors.WithStack(err) + } + + if predicate == nil { + return items, nil + } + + idx := 0 + for _, item := range items { + if predicate(item) { + items[idx] = item + idx++ + } + } + + return items[:idx], nil +} + func (g *Client) ListFiles(ctx context.Context, r *ReviewParams) ([]*CommitFile, error) { - return cache.Apply( + return listApply( ctx, cache.DataKeyFor("files", r.Owner, r.Repo, r.Pull), func() ([]*CommitFile, error) { d, _, err := g.c.PullRequests.ListFiles(ctx, r.Owner, r.Repo, r.Pull, nil) return d, errors.WrapGithubErr(err, "call to ListFiles failed") }, + nil, ) } @@ -83,13 +112,14 @@ func (g *Client) GetPullRequest(ctx context.Context, r *ReviewParams) (*PullRequ } func (g *Client) ListReviews(ctx context.Context, r *ReviewParams) ([]*PullRequestReview, error) { - return cache.Apply( + return listApply( ctx, cache.DataKeyFor("reviews", r.Owner, r.Repo, r.Pull), func() ([]*PullRequestReview, error) { reviews, _, err := g.c.PullRequests.ListReviews(ctx, r.Owner, r.Repo, r.Pull, nil) return reviews, errors.WrapGithubErr(err, "call to ListReviews failed") }, + nil, ) } @@ -105,41 +135,48 @@ func (g *Client) GetReview(ctx context.Context, r *ReviewParams) (*PullRequestRe } func (g *Client) ListReviewComments(ctx context.Context, r *ReviewParams) ([]*PullRequestComment, error) { - return cache.Apply( + return listApply( ctx, cache.DataKeyFor("review-comments", r.Owner, r.Repo, r.Pull, r.ReviewID), func() ([]*PullRequestComment, error) { cs, _, err := g.c.PullRequests.ListReviewComments(ctx, r.Owner, r.Repo, r.Pull, r.ReviewID, nil) return cs, errors.WrapGithubErr(err, "call to ListReviewComments failed") }, + nil, ) } -func (g *Client) ListComments(ctx context.Context, r *ReviewParams) ([]*PullRequestComment, error) { - cs, err := cache.Apply( +func (g *Client) ListComments( + ctx context.Context, r *ReviewParams, pred CommentPredicate, +) ([]*PullRequestComment, error) { + return listApply( ctx, cache.DataKeyFor("pull-comments", r.Owner, r.Repo, r.Pull), func() ([]*PullRequestComment, error) { cs, _, err := g.c.PullRequests.ListComments(ctx, r.Owner, r.Repo, r.Pull, nil) return cs, errors.WrapGithubErr(err, "call to ListComments failed") }, + pred, ) - if err != nil { - return nil, err +} + +func FetchReviewModel(ctx context.Context, r *ReviewParams, pred CommentPredicate) (*ReviewModel, error) { + client, ok := Ctx(ctx) + if !ok || client == nil { + return nil, errors.New("context missing github client") } - var ret []*PullRequestComment - for _, c := range cs { - if c.PullRequestReviewID == nil || *c.PullRequestReviewID != r.ReviewID { - continue - } - ret = append(ret, c) + model, err := client.FetchReviewModel(ctx, r, pred) + if err != nil { + return nil, errors.WithStack(err) } - return ret, nil + return model, nil } -func (g *Client) FetchReviewModel(ctx context.Context, r *ReviewParams) (*ReviewModel, error) { +func (g *Client) FetchReviewModel( + ctx context.Context, r *ReviewParams, pred CommentPredicate, +) (*ReviewModel, error) { model := &ReviewModel{Params: r} group, ctx := errgroup.WithContext(ctx) @@ -151,16 +188,18 @@ func (g *Client) FetchReviewModel(ctx context.Context, r *ReviewParams) (*Review return err }) - group.Go(func() error { - review, err := g.GetReview(ctx, r) - if err == nil { - model.Review = review - } - return err - }) + if r.ReviewID != 0 { + group.Go(func() error { + review, err := g.GetReview(ctx, r) + if err == nil { + model.Review = review + } + return err + }) + } group.Go(func() error { - comments, err := g.ListComments(ctx, r) + comments, err := g.ListComments(ctx, r, pred) if err == nil { for idx := range comments { comment := comments[idx] diff --git a/server/internal/github/language.go b/server/internal/github/language.go new file mode 100644 index 0000000..08ae4e1 --- /dev/null +++ b/server/internal/github/language.go @@ -0,0 +1,26 @@ +package github + +import "path" + +func detectLanguage(p string) string { + var ( + base = path.Base(p) + ext = path.Ext(p) + ) + + switch ext { + case "": + if base == "Dockerfile" { + return "docker" + } + return "bash" + case ".rs": + return "rust" + case ".vue": + return "html" + case ".Dockerfile": + return "docker" + } + + return ext[1:] +} diff --git a/server/internal/github/predicate.go b/server/internal/github/predicate.go new file mode 100644 index 0000000..d6890b4 --- /dev/null +++ b/server/internal/github/predicate.go @@ -0,0 +1,44 @@ +package github + +import ( + "regexp" + "strconv" +) + +type Pred[T any] func(T) bool + +type CommentPredicate = Pred[*PullRequestComment] + +func CommentMatchesReview(reviewID int64) CommentPredicate { + return func(c *PullRequestComment) bool { + return c.PullRequestReviewID != nil && *c.PullRequestReviewID == reviewID + } +} + +var prmeTagRegexp = regexp.MustCompile(`\(prme([^-\s]+)?(-(\d+))?\)\s*$`) + +type reviewTag struct { + Review string + Order int +} + +func parseReviewTag(s string) (reviewTag, bool) { + m := prmeTagRegexp.FindStringSubmatch(s) + if m == nil { + return reviewTag{}, false + } + + n, _ := strconv.Atoi(m[3]) + return reviewTag{Review: m[1], Order: n}, true +} + +func CommentMatchesTag(tag string) CommentPredicate { + return func(c *PullRequestComment) bool { + if c.Body == nil { + return false + } + + rtag, ok := parseReviewTag(*c.Body) + return ok && rtag.Review == tag + } +} diff --git a/server/internal/github/predicate_test.go b/server/internal/github/predicate_test.go new file mode 100644 index 0000000..9d7bea9 --- /dev/null +++ b/server/internal/github/predicate_test.go @@ -0,0 +1,53 @@ +package github + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCommentTagRegex(t *testing.T) { + type testData struct { + name string + input string + expectedOk bool + expected reviewTag + } + + for _, data := range []testData{ + { + "standard", "foo (prme1)", true, reviewTag{"1", 0}, + }, + { + "no-prefix", "(prme2)", true, reviewTag{"2", 0}, + }, + { + "trailing space", "(prme2) ", true, reviewTag{"2", 0}, + }, + { + "with order", "end of text (prmesomething-1)\n", true, reviewTag{"something", 1}, + }, + { + "with order 3", "(prmesomething-3)", true, reviewTag{"something", 3}, + }, + { + "doesnt match in the middle", "(prme1) banana", false, reviewTag{}, + }, + { + "can have naked prme", "foo (prme)", true, reviewTag{"", 0}, + }, + { + "won't parse one without parentheses", "foo prme", false, reviewTag{}, + }, + { + "no tag but order", "anything (prme-5)", true, reviewTag{"", 5}, + }, + } { + d := data + t.Run(d.name, func(t *testing.T) { + tag, ok := parseReviewTag(d.input) + require.Equal(t, d.expectedOk, ok) + require.Equal(t, d.expected, tag) + }) + } +} diff --git a/server/internal/github/review_model.go b/server/internal/github/review_model.go index 008bceb..b983492 100644 --- a/server/internal/github/review_model.go +++ b/server/internal/github/review_model.go @@ -35,10 +35,6 @@ func orderOf(c string) (int, bool) { return n, true } -func trimStartsWithNumber(c string) string { - return startsWithNumberRegexp.ReplaceAllString(c, "") -} - func generateDiff(comment *PullRequestComment) (string, error) { // 1. we extract the metadata, we know which side we are going to be starting on. meta, err := diff.ParseHunkMeta(*comment.DiffHunk) diff --git a/server/internal/github/review_params.go b/server/internal/github/review_params.go index cc892bc..36ae94e 100644 --- a/server/internal/github/review_params.go +++ b/server/internal/github/review_params.go @@ -59,6 +59,8 @@ type ReviewParamsMap struct { Repo string `json:"repo"` Pull string `json:"pull"` Review string `json:"review"` + Tag string `json:"tag"` + Kind string `json:"kind"` } func NewReviewParamsMap(values url.Values) ReviewParamsMap { @@ -67,6 +69,8 @@ func NewReviewParamsMap(values url.Values) ReviewParamsMap { Repo: values.Get("repo"), Pull: values.Get("pull"), Review: values.Get("review"), + Tag: values.Get("tag"), + Kind: values.Get("kind"), } } @@ -107,10 +111,6 @@ func ReviewParamsFromMap(m ReviewParamsMap) (*ReviewParams, error) { }, nil } -func (r *ReviewParams) Model(ctx context.Context, g *Client) (*ReviewModel, error) { - return g.FetchReviewModel(ctx, r) -} - func (r *ReviewParams) EnsureReviewID(ctx context.Context, g *Client) (bool, error) { if r.ReviewID != 0 { return false, nil