diff --git a/pkg/notifier/gitlab/comment.go b/pkg/notifier/gitlab/comment.go index 9068170..b476393 100644 --- a/pkg/notifier/gitlab/comment.go +++ b/pkg/notifier/gitlab/comment.go @@ -3,9 +3,15 @@ package gitlab import ( "fmt" + "github.com/sirupsen/logrus" gitlab "github.com/xanzy/go-gitlab" ) +const ( + listPerPage = 100 + maxPages = 100 +) + // CommentService handles communication with the comment related // methods of GitLab API type CommentService service @@ -59,9 +65,40 @@ func (g *CommentService) Patch(note int, body string, opt PostOptions) error { // List lists comments on GitLab merge requests func (g *CommentService) List(number int) ([]*gitlab.Note, error) { - comments, _, err := g.client.API.ListMergeRequestNotes( - number, - &gitlab.ListMergeRequestNotesOptions{}, - ) - return comments, err + allComments := make([]*gitlab.Note, 0) + + opt := &gitlab.ListMergeRequestNotesOptions{ + ListOptions: gitlab.ListOptions{ + Page: 1, + PerPage: listPerPage, + }, + } + + for sentinel := 1; ; sentinel++ { + comments, resp, err := g.client.API.ListMergeRequestNotes( + number, + opt, + ) + if err != nil { + return nil, err + } + + allComments = append(allComments, comments...) + + if resp.NextPage == 0 { + break + } + + if sentinel >= maxPages { + logE := logrus.WithFields(logrus.Fields{ + "program": "tfcmt", + }) + logE.WithField("maxPages", maxPages).Debug("gitlab.comment.list: too many pages, something went wrong") + break + } + + opt.Page = resp.NextPage + } + + return allComments, nil } diff --git a/pkg/notifier/gitlab/comment_test.go b/pkg/notifier/gitlab/comment_test.go index 626f752..79cdec4 100644 --- a/pkg/notifier/gitlab/comment_test.go +++ b/pkg/notifier/gitlab/comment_test.go @@ -124,16 +124,6 @@ func TestCommentPost(t *testing.T) { func TestCommentList(t *testing.T) { t.Parallel() - comments := []*gitlab.Note{ - { - ID: 371748792, - Body: "comment 1", - }, - { - ID: 371765743, - Body: "comment 2", - }, - } testCases := []struct { name string config Config @@ -147,12 +137,56 @@ func TestCommentList(t *testing.T) { config: newFakeConfig(), createMockGitLabAPI: func(ctrl *gomock.Controller) *gitlabmock.MockAPI { api := gitlabmock.NewMockAPI(ctrl) - api.EXPECT().ListMergeRequestNotes(1, gomock.Any()).Return(comments, &gitlab.Response{TotalPages: 1, NextPage: 1}, nil) + api.EXPECT().ListMergeRequestNotes(1, gomock.Any()).MaxTimes(2).DoAndReturn( + func(mergeRequest int, opt *gitlab.ListMergeRequestNotesOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Note, *gitlab.Response, error) { + res := []*gitlab.Note{ + // same response to any page for now + { + ID: 371748792, + Body: "comment 1", + }, + { + ID: 371765743, + Body: "comment 2", + }, + } + + // fake pagination with 2 pages + resp := &gitlab.Response{ + NextPage: 0, + } + if opt.Page == 1 { + resp.NextPage = 2 + } + + return res, resp, nil + }, + ) + return api }, - number: 1, - ok: true, - comments: comments, + number: 1, + ok: true, + comments: []*gitlab.Note{ + // page1 + { + ID: 371748792, + Body: "comment 1", + }, + { + ID: 371765743, + Body: "comment 2", + }, + // page2 + { + ID: 371748792, + Body: "comment 1", + }, + { + ID: 371765743, + Body: "comment 2", + }, + }, }, } @@ -180,3 +214,35 @@ func TestCommentList(t *testing.T) { }) } } + +func TestCommentListSentinel(t *testing.T) { + t.Parallel() + + client, err := NewClient(newFakeConfig()) + if err != nil { + t.Fatal(err) + } + + createMockGitLabAPI := func(ctrl *gomock.Controller) *gitlabmock.MockAPI { + api := gitlabmock.NewMockAPI(ctrl) + api.EXPECT().ListMergeRequestNotes(1, gomock.Any()).MaxTimes(100).Return( + []*gitlab.Note{}, + &gitlab.Response{ + NextPage: 1, // just cause infinite loop + }, + nil, + ) + + return api + } + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + client.API = createMockGitLabAPI(mockCtrl) + + _, err = client.Comment.List(1) // no assert res, only assert `.MaxTimes(100)` + if err != nil { + t.Errorf("got error %q", err) + } +}