From caafe6c2cfe58854557ea9c74c1b7a28d2cfbf3d Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Tue, 7 May 2024 21:52:18 +0200 Subject: [PATCH] refactor: rework github client and comment methods (#113) --- go.mod | 8 ++ go.sum | 9 ++ plugin/comment.go | 83 ------------------ plugin/github.go | 96 +++++++++++++++++++++ plugin/github_test.go | 192 ++++++++++++++++++++++++++++++++++++++++++ plugin/impl.go | 24 +++--- 6 files changed, 316 insertions(+), 96 deletions(-) delete mode 100644 plugin/comment.go create mode 100644 plugin/github.go create mode 100644 plugin/github_test.go diff --git a/go.mod b/go.mod index 41f1e23..91793ae 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,9 @@ go 1.22 require ( github.com/google/go-github/v61 v61.0.0 + github.com/migueleliasweb/go-github-mock v0.0.23 github.com/rs/zerolog v1.32.0 + github.com/stretchr/testify v1.9.0 github.com/thegeeklab/wp-plugin-go/v2 v2.3.1 github.com/urfave/cli/v2 v2.27.2 golang.org/x/oauth2 v0.20.0 @@ -15,8 +17,11 @@ require ( github.com/Masterminds/semver/v3 v3.2.1 // indirect github.com/Masterminds/sprig/v3 v3.2.3 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/google/go-github/v59 v59.0.0 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/google/uuid v1.1.1 // indirect + github.com/gorilla/mux v1.8.0 // indirect github.com/huandu/xstrings v1.3.3 // indirect github.com/imdario/mergo v0.3.11 // indirect github.com/joho/godotenv v1.5.1 // indirect @@ -24,6 +29,7 @@ require ( github.com/mattn/go-isatty v0.0.19 // indirect github.com/mitchellh/copystructure v1.0.0 // indirect github.com/mitchellh/reflectwalk v1.0.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/shopspring/decimal v1.2.0 // indirect github.com/spf13/cast v1.3.1 // indirect @@ -31,4 +37,6 @@ require ( golang.org/x/crypto v0.23.0 // indirect golang.org/x/net v0.25.0 // indirect golang.org/x/sys v0.20.0 // indirect + golang.org/x/time v0.3.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index ec0763a..e08ef00 100644 --- a/go.sum +++ b/go.sum @@ -15,12 +15,16 @@ github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5x github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-github/v59 v59.0.0 h1:7h6bgpF5as0YQLLkEiVqpgtJqjimMYhBkD4jT5aN3VA= +github.com/google/go-github/v59 v59.0.0/go.mod h1:rJU4R0rQHFVFDOkqGWxfLNo6vEk4dv40oDjhV/gH6wM= github.com/google/go-github/v61 v61.0.0 h1:VwQCBwhyE9JclCI+22/7mLB1PuU9eowCXKY5pNlu1go= github.com/google/go-github/v61 v61.0.0/go.mod h1:0WR+KmsWX75G2EbpyGsGmradjo3IiciuI4BmdVCobQY= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= +github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/huandu/xstrings v1.3.3 h1:/Gcsuc1x8JVbJ9/rlye4xZnVAbEkGauT8lbebqcQws4= github.com/huandu/xstrings v1.3.3/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= github.com/imdario/mergo v0.3.11 h1:3tnifQM4i+fbajXKBHXWEH+KvNHqojZ778UH75j3bGA= @@ -32,6 +36,8 @@ github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovk github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.19 h1:JITubQf0MOLdlGRuRq+jtsDlekdYPia9ZFsB8h/APPA= github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/migueleliasweb/go-github-mock v0.0.23 h1:GOi9oX/+Seu9JQ19V8bPDLqDI7M9iEOjo3g8v1k6L2c= +github.com/migueleliasweb/go-github-mock v0.0.23/go.mod h1:NsT8FGbkvIZQtDu38+295sZEX8snaUiiQgsGxi6GUxk= github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ= github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw= github.com/mitchellh/reflectwalk v1.0.0 h1:9D+8oIskB4VJBN5SFlmc27fSlIBZaov1Wpk/IfikLNY= @@ -94,11 +100,14 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= +golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= diff --git a/plugin/comment.go b/plugin/comment.go deleted file mode 100644 index 565c4a9..0000000 --- a/plugin/comment.go +++ /dev/null @@ -1,83 +0,0 @@ -package plugin - -import ( - "context" - "fmt" - "strings" - - "github.com/google/go-github/v61/github" -) - -type commentClient struct { - *github.Client - Message string - Update bool - Key string - Repo string - Owner string - IssueNum int -} - -func (cc *commentClient) issueComment(ctx context.Context) error { - var ( - err error - comment *github.IssueComment - resp *github.Response - ) - - issueComment := &github.IssueComment{ - Body: &cc.Message, - } - - if cc.Update { - // Append plugin comment ID to comment message so we can search for it later - message := fmt.Sprintf("%s\n\n", cc.Message, cc.Key) - issueComment.Body = &message - - comment, err = cc.comment(ctx) - - if err == nil && comment != nil { - _, resp, err = cc.Client.Issues.EditComment(ctx, cc.Owner, cc.Repo, *comment.ID, issueComment) - } - } - - if err == nil && resp == nil { - _, _, err = cc.Client.Issues.CreateComment(ctx, cc.Owner, cc.Repo, cc.IssueNum, issueComment) - } - - if err != nil { - return err - } - - return nil -} - -func (cc *commentClient) comment(ctx context.Context) (*github.IssueComment, error) { - var allComments []*github.IssueComment - - opts := &github.IssueListCommentsOptions{} - - for { - comments, resp, err := cc.Client.Issues.ListComments(ctx, cc.Owner, cc.Repo, cc.IssueNum, opts) - if err != nil { - return nil, err - } - - allComments = append(allComments, comments...) - - if resp.NextPage == 0 { - break - } - - opts.Page = resp.NextPage - } - - for _, comment := range allComments { - if strings.Contains(*comment.Body, fmt.Sprintf("", cc.Key)) { - return comment, nil - } - } - - //nolint:nilnil - return nil, nil -} diff --git a/plugin/github.go b/plugin/github.go new file mode 100644 index 0000000..c1f57df --- /dev/null +++ b/plugin/github.go @@ -0,0 +1,96 @@ +package plugin + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/google/go-github/v61/github" +) + +var ErrCommentNotFound = errors.New("no comment found") + +type GithubClient struct { + Client *github.Client + Issue *GithubIssue +} + +type GithubIssue struct { + *github.Client + Number int + Message string + Key string + Repo string + Owner string + Update bool +} + +// Constructor function for Parent. +func NewGithubClient(client *github.Client) *GithubClient { + return &GithubClient{ + Client: client, + Issue: &GithubIssue{Client: client}, + } +} + +// AddComment adds a new comment or updates an existing comment on a GitHub issue. +// If the Update field is true, it will append a unique identifier to the comment +// body and attempt to find and update the existing comment with that identifier. +// Otherwise, it will create a new comment on the issue. +func (i *GithubIssue) AddComment(ctx context.Context) (*github.IssueComment, error) { + issueComment := &github.IssueComment{ + Body: &i.Message, + } + + if i.Update { + // Append plugin comment ID to comment message so we can search for it later + *issueComment.Body = fmt.Sprintf("%s\n\n", i.Message, i.Key) + + comment, err := i.FindComment(ctx) + if err != nil && !errors.Is(err, ErrCommentNotFound) { + return comment, err + } + + if comment != nil { + comment, _, err = i.Client.Issues.EditComment(ctx, i.Owner, i.Repo, *comment.ID, issueComment) + + return comment, err + } + } + + comment, _, err := i.Client.Issues.CreateComment(ctx, i.Owner, i.Repo, i.Number, issueComment) + + return comment, err +} + +// FindComment returns the GitHub issue comment that contains the specified key, or nil if no such comment exists. +// It retrieves all comments on the issue and searches for one that contains the specified key in the comment body. +func (i *GithubIssue) FindComment(ctx context.Context) (*github.IssueComment, error) { + var allComments []*github.IssueComment + + opts := &github.IssueListCommentsOptions{} + + for { + comments, resp, err := i.Client.Issues.ListComments(ctx, i.Owner, i.Repo, i.Number, opts) + if err != nil { + return nil, err + } + + allComments = append(allComments, comments...) + + if resp.NextPage == 0 { + break + } + + opts.Page = resp.NextPage + } + + for _, comment := range allComments { + if strings.Contains(*comment.Body, fmt.Sprintf("", i.Key)) { + return comment, nil + } + } + + return nil, fmt.Errorf("%w key: %s", ErrCommentNotFound, i.Key) +} diff --git a/plugin/github_test.go b/plugin/github_test.go new file mode 100644 index 0000000..49bfc63 --- /dev/null +++ b/plugin/github_test.go @@ -0,0 +1,192 @@ +package plugin + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/google/go-github/v61/github" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" +) + +func TestGithubIssue_FindComment(t *testing.T) { + ctx := context.Background() + key := "test-key" + keyPattern := "" + owner := "test-owner" + repo := "test-repo" + number := 1 + + tests := []struct { + name string + comments []*github.IssueComment + want *github.IssueComment + wantErr error + }{ + { + name: "no comments", + want: nil, + wantErr: ErrCommentNotFound, + }, + { + name: "comment found", + comments: []*github.IssueComment{ + {Body: github.String(keyPattern)}, + }, + want: &github.IssueComment{Body: github.String(keyPattern)}, + }, + { + name: "comment not found", + comments: []*github.IssueComment{ + {Body: github.String("other comment")}, + }, + want: nil, + wantErr: ErrCommentNotFound, + }, + { + name: "multiple comments", + comments: []*github.IssueComment{ + {Body: github.String("other comment")}, + {Body: github.String(keyPattern)}, + {Body: github.String("another comment")}, + }, + want: &github.IssueComment{Body: github.String(keyPattern)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockedHTTPClient := mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, + tt.comments, + ), + ) + + client := github.NewClient(mockedHTTPClient) + issue := &GithubIssue{ + Client: client, + Owner: owner, + Repo: repo, + Number: number, + Key: key, + } + + got, err := issue.FindComment(ctx) + if tt.wantErr != nil { + assert.Error(t, err) + assert.Equal(t, tt.want, got) + + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestGithubIssue_AddComment(t *testing.T) { + ctx := context.Background() + key := "test-key" + keyPattern := "" + owner := "test-owner" + repo := "test-repo" + number := 1 + message := "test message" + + tests := []struct { + name string + update bool + existingKey string + comments []*github.IssueComment + want *github.IssueComment + wantErr bool + }{ + { + name: "create new comment", + update: false, + want: &github.IssueComment{Body: github.String("test message\n\n")}, + }, + { + name: "update existing comment", + update: true, + comments: []*github.IssueComment{ + {ID: github.Int64(123), Body: github.String(keyPattern)}, + }, + want: &github.IssueComment{Body: github.String("test message\n\n")}, + }, + { + name: "update non-existing comment", + update: true, + want: &github.IssueComment{Body: github.String("test message\n\n")}, + }, + { + name: "create new comment with error", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockedHTTPClient := mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, + tt.comments, + ), + mock.WithRequestMatchHandler( + mock.PostReposIssuesCommentsByOwnerByRepoByIssueNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if tt.wantErr { + mock.WriteError(w, http.StatusInternalServerError, "internal server error") + } else { + _, _ = w.Write(mock.MustMarshal( + &github.IssueComment{ + Body: github.String(fmt.Sprintf("%s\n%s\n", message, keyPattern)), + }, + )) + } + }), + ), + mock.WithRequestMatchHandler( + mock.PatchReposIssuesCommentsByOwnerByRepoByCommentId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if tt.wantErr { + mock.WriteError(w, http.StatusInternalServerError, "internal server error") + } else { + _, _ = w.Write(mock.MustMarshal( + &github.IssueComment{ + Body: github.String(fmt.Sprintf("%s\n%s\n", message, keyPattern)), + }, + )) + } + }), + ), + ) + + client := github.NewClient(mockedHTTPClient) + issue := &GithubIssue{ + Client: client, + Owner: owner, + Repo: repo, + Number: number, + Key: key, + Message: message, + Update: tt.update, + } + + got, err := issue.AddComment(ctx) + if tt.wantErr { + assert.Error(t, err) + assert.Equal(t, tt.want, got) + + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/plugin/impl.go b/plugin/impl.go index aa46e59..b0a857c 100644 --- a/plugin/impl.go +++ b/plugin/impl.go @@ -74,18 +74,16 @@ func (p *Plugin) Execute() error { ts, ) - client := github.NewClient(tc) - client.BaseURL = p.Settings.baseURL - - commentClient := commentClient{ - Client: client, - Repo: p.Metadata.Repository.Name, - Owner: p.Metadata.Repository.Owner, - Message: p.Settings.Message, - Update: p.Settings.Update, - Key: p.Settings.Key, - IssueNum: p.Metadata.Curr.PullRequest, - } + gh := github.NewClient(tc) + gh.BaseURL = p.Settings.baseURL + + client := NewGithubClient(gh) + client.Issue.Repo = p.Metadata.Repository.Name + client.Issue.Owner = p.Metadata.Repository.Owner + client.Issue.Message = p.Settings.Message + client.Issue.Update = p.Settings.Update + client.Issue.Key = p.Settings.Key + client.Issue.Number = p.Metadata.Curr.PullRequest if p.Settings.SkipMissing && !p.Settings.IsFile { log.Info(). @@ -94,7 +92,7 @@ func (p *Plugin) Execute() error { return nil } - err := commentClient.issueComment(p.Network.Context) + _, err := client.Issue.AddComment(p.Network.Context) if err != nil { return fmt.Errorf("failed to create or update comment: %w", err) }