From f4ca533fe10f7fa893e1953f8c8d9ed3e783486c Mon Sep 17 00:00:00 2001 From: Michael MurĂ© Date: Sat, 28 Mar 2020 21:09:36 +0100 Subject: gitlab: refactor the iterator, fix bugs Notably, properly reset sub iterators when changing to the next issue --- bridge/gitlab/iterator/issue.go | 79 ++++++++++++++++++++ bridge/gitlab/iterator/iterator.go | 138 +++++++++++++++++++++++++++++++++++ bridge/gitlab/iterator/labelEvent.go | 99 +++++++++++++++++++++++++ bridge/gitlab/iterator/note.go | 80 ++++++++++++++++++++ 4 files changed, 396 insertions(+) create mode 100644 bridge/gitlab/iterator/issue.go create mode 100644 bridge/gitlab/iterator/iterator.go create mode 100644 bridge/gitlab/iterator/labelEvent.go create mode 100644 bridge/gitlab/iterator/note.go (limited to 'bridge/gitlab/iterator') diff --git a/bridge/gitlab/iterator/issue.go b/bridge/gitlab/iterator/issue.go new file mode 100644 index 00000000..a6bcebf1 --- /dev/null +++ b/bridge/gitlab/iterator/issue.go @@ -0,0 +1,79 @@ +package iterator + +import ( + "context" + + "github.com/xanzy/go-gitlab" +) + +type issueIterator struct { + page int + index int + cache []*gitlab.Issue +} + +func newIssueIterator() *issueIterator { + ii := &issueIterator{} + ii.Reset() + return ii +} + +func (ii *issueIterator) Next(ctx context.Context, conf config) (bool, error) { + // first query + if ii.cache == nil { + return ii.getNext(ctx, conf) + } + + // move cursor index + if ii.index < len(ii.cache)-1 { + ii.index++ + return true, nil + } + + return ii.getNext(ctx, conf) +} + +func (ii *issueIterator) Value() *gitlab.Issue { + return ii.cache[ii.index] +} + +func (ii *issueIterator) getNext(ctx context.Context, conf config) (bool, error) { + ctx, cancel := context.WithTimeout(ctx, conf.timeout) + defer cancel() + + issues, _, err := conf.gc.Issues.ListProjectIssues( + conf.project, + &gitlab.ListProjectIssuesOptions{ + ListOptions: gitlab.ListOptions{ + Page: ii.page, + PerPage: conf.capacity, + }, + Scope: gitlab.String("all"), + UpdatedAfter: &conf.since, + Sort: gitlab.String("asc"), + }, + gitlab.WithContext(ctx), + ) + + if err != nil { + ii.Reset() + return false, err + } + + // if repository doesn't have any issues + if len(issues) == 0 { + return false, nil + } + + ii.cache = issues + ii.index = 0 + ii.page++ + + return true, nil +} + +func (ii *issueIterator) Reset() { + ii.index = -1 + ii.page = -1 + ii.cache = nil +} diff --git a/bridge/gitlab/iterator/iterator.go b/bridge/gitlab/iterator/iterator.go new file mode 100644 index 00000000..ee2090b0 --- /dev/null +++ b/bridge/gitlab/iterator/iterator.go @@ -0,0 +1,138 @@ +package iterator + +import ( + "context" + "time" + + "github.com/xanzy/go-gitlab" +) + +type Iterator struct { + // shared context + ctx context.Context + + // to pass to sub-iterators + conf config + + // sticky error + err error + + // issues iterator + issue *issueIterator + + // notes iterator + note *noteIterator + + // labelEvent iterator + labelEvent *labelEventIterator +} + +type config struct { + // gitlab api v4 client + gc *gitlab.Client + + timeout time.Duration + + // if since is given the iterator will query only the issues + // updated after this date + since time.Time + + // project id + project string + + // number of issues and notes to query at once + capacity int +} + +// NewIterator create a new iterator +func NewIterator(ctx context.Context, client *gitlab.Client, capacity int, projectID string, since time.Time) *Iterator { + return &Iterator{ + ctx: ctx, + conf: config{ + gc: client, + timeout: 60 * time.Second, + since: since, + project: projectID, + capacity: capacity, + }, + issue: newIssueIterator(), + note: newNoteIterator(), + labelEvent: newLabelEventIterator(), + } +} + +// Error return last encountered error +func (i *Iterator) Error() error { + return i.err +} + +func (i *Iterator) NextIssue() bool { + if i.err != nil { + return false + } + + if i.ctx.Err() != nil { + return false + } + + more, err := i.issue.Next(i.ctx, i.conf) + if err != nil { + i.err = err + return false + } + + // Also reset the other sub iterators as they would + // no longer be valid + i.note.Reset(i.issue.Value().IID) + i.labelEvent.Reset(i.issue.Value().IID) + + return more +} + +func (i *Iterator) IssueValue() *gitlab.Issue { + return i.issue.Value() +} + +func (i *Iterator) NextNote() bool { + if i.err != nil { + return false + } + + if i.ctx.Err() != nil { + return false + } + + more, err := i.note.Next(i.ctx, i.conf) + if err != nil { + i.err = err + return false + } + + return more +} + +func (i *Iterator) NoteValue() *gitlab.Note { + return i.note.Value() +} + +func (i *Iterator) NextLabelEvent() bool { + if i.err != nil { + return false + } + + if i.ctx.Err() != nil { + return false + } + + more, err := i.labelEvent.Next(i.ctx, i.conf) + if err != nil { + i.err = err + return false + } + + return more +} + +func (i *Iterator) LabelEventValue() *gitlab.LabelEvent { + return i.labelEvent.Value() +} diff --git a/bridge/gitlab/iterator/labelEvent.go b/bridge/gitlab/iterator/labelEvent.go new file mode 100644 index 00000000..7ee2604b --- /dev/null +++ b/bridge/gitlab/iterator/labelEvent.go @@ -0,0 +1,99 @@ +package iterator + +import ( + "context" + "sort" + + "github.com/xanzy/go-gitlab" +) + +// Since Gitlab does not return the label events items in the correct order +// we need to sort the list ourselves and stop relying on the pagination model +// #BecauseGitlab +type labelEventIterator struct { + issue int + index int + cache []*gitlab.LabelEvent +} + +func newLabelEventIterator() *labelEventIterator { + lei := &labelEventIterator{} + lei.Reset(-1) + return lei +} + +func (lei *labelEventIterator) Next(ctx context.Context, conf config) (bool, error) { + // first query + if lei.cache == nil { + return lei.getNext(ctx, conf) + } + + // move cursor index + if lei.index < len(lei.cache)-1 { + lei.index++ + return true, nil + } + + return false, nil +} + +func (lei *labelEventIterator) Value() *gitlab.LabelEvent { + return lei.cache[lei.index] +} + +func (lei *labelEventIterator) getNext(ctx context.Context, conf config) (bool, error) { + ctx, cancel := context.WithTimeout(ctx, conf.timeout) + defer cancel() + + // since order is not guaranteed we should query all label events + // and sort them by ID + page := 1 + for { + labelEvents, _, err := conf.gc.ResourceLabelEvents.ListIssueLabelEvents( + conf.project, + lei.issue, + &gitlab.ListLabelEventsOptions{ + ListOptions: gitlab.ListOptions{ + Page: page, + PerPage: conf.capacity, + }, + }, + gitlab.WithContext(ctx), + ) + if err != nil { + lei.Reset(-1) + return false, err + } + + if len(labelEvents) == 0 { + break + } + lei.cache = append(lei.cache, labelEvents...) + page++ + } + + sort.Sort(lei) + lei.index = 0 + + return len(lei.cache) > 0, nil +} + +func (lei *labelEventIterator) Reset(issue int) { + lei.issue = issue + lei.index = -1 + lei.cache = nil +} + +// ORDERING + +func (lei *labelEventIterator) Len() int { + return len(lei.cache) +} + +func (lei *labelEventIterator) Swap(i, j int) { + lei.cache[i], lei.cache[j] = lei.cache[j], lei.cache[i] +} + +func (lei *labelEventIterator) Less(i, j int) bool { + return lei.cache[i].ID < lei.cache[j].ID +} diff --git a/bridge/gitlab/iterator/note.go b/bridge/gitlab/iterator/note.go new file mode 100644 index 00000000..486ca94e --- /dev/null +++ b/bridge/gitlab/iterator/note.go @@ -0,0 +1,80 @@ +package iterator + +import ( + "context" + + "github.com/xanzy/go-gitlab" +) + +type noteIterator struct { + issue int + page int + index int + cache []*gitlab.Note +} + +func newNoteIterator() *noteIterator { + in := ¬eIterator{} + in.Reset(-1) + return in +} + +func (in *noteIterator) Next(ctx context.Context, conf config) (bool, error) { + // first query + if in.cache == nil { + return in.getNext(ctx, conf) + } + + // move cursor index + if in.index < len(in.cache)-1 { + in.index++ + return true, nil + } + + return in.getNext(ctx, conf) +} + +func (in *noteIterator) Value() *gitlab.Note { + return in.cache[in.index] +} + +func (in *noteIterator) getNext(ctx context.Context, conf config) (bool, error) { + ctx, cancel := context.WithTimeout(ctx, conf.timeout) + defer cancel() + + notes, _, err := conf.gc.Notes.ListIssueNotes( + conf.project, + in.issue, + &gitlab.ListIssueNotesOptions{ + ListOptions: gitlab.ListOptions{ + Page: in.page, + PerPage: conf.capacity, + }, + Sort: gitlab.String("asc"), + OrderBy: gitlab.String("created_at"), + }, + gitlab.WithContext(ctx), + ) + + if err != nil { + in.Reset(-1) + return false, err + } + + if len(notes) == 0 { + return false, nil + } + + in.cache = notes + in.index = 0 + in.page++ + + return true, nil +} + +func (in *noteIterator) Reset(issue int) { + in.issue = issue + in.index = -1 + in.page = -1 + in.cache = nil +} -- cgit From 903549cadf40ede3771053781eb6e9fd31aaa64e Mon Sep 17 00:00:00 2001 From: Michael MurĂ© Date: Sat, 4 Apr 2020 11:59:53 +0200 Subject: gitlab: fix iterator (paginate with first index 1) and avoid the trailing API call --- bridge/gitlab/iterator/issue.go | 20 +++++++++++++++----- bridge/gitlab/iterator/labelEvent.go | 8 +++++++- bridge/gitlab/iterator/note.go | 22 ++++++++++++++++------ 3 files changed, 38 insertions(+), 12 deletions(-) (limited to 'bridge/gitlab/iterator') diff --git a/bridge/gitlab/iterator/issue.go b/bridge/gitlab/iterator/issue.go index a6bcebf1..9361b496 100644 --- a/bridge/gitlab/iterator/issue.go +++ b/bridge/gitlab/iterator/issue.go @@ -7,9 +7,10 @@ import ( ) type issueIterator struct { - page int - index int - cache []*gitlab.Issue + page int + lastPage bool + index int + cache []*gitlab.Issue } func newIssueIterator() *issueIterator { @@ -38,10 +39,14 @@ func (ii *issueIterator) Value() *gitlab.Issue { } func (ii *issueIterator) getNext(ctx context.Context, conf config) (bool, error) { + if ii.lastPage { + return false, nil + } + ctx, cancel := context.WithTimeout(ctx, conf.timeout) defer cancel() - issues, _, err := conf.gc.Issues.ListProjectIssues( + issues, resp, err := conf.gc.Issues.ListProjectIssues( conf.project, &gitlab.ListProjectIssuesOptions{ ListOptions: gitlab.ListOptions{ @@ -60,6 +65,10 @@ func (ii *issueIterator) getNext(ctx context.Context, conf config) (bool, error) return false, err } + if resp.TotalPages == ii.page { + ii.lastPage = true + } + // if repository doesn't have any issues if len(issues) == 0 { return false, nil @@ -74,6 +83,7 @@ func (ii *issueIterator) getNext(ctx context.Context, conf config) (bool, error) func (ii *issueIterator) Reset() { ii.index = -1 - ii.page = -1 + ii.page = 1 + ii.lastPage = false ii.cache = nil } diff --git a/bridge/gitlab/iterator/labelEvent.go b/bridge/gitlab/iterator/labelEvent.go index 7ee2604b..812e6646 100644 --- a/bridge/gitlab/iterator/labelEvent.go +++ b/bridge/gitlab/iterator/labelEvent.go @@ -49,7 +49,7 @@ func (lei *labelEventIterator) getNext(ctx context.Context, conf config) (bool, // and sort them by ID page := 1 for { - labelEvents, _, err := conf.gc.ResourceLabelEvents.ListIssueLabelEvents( + labelEvents, resp, err := conf.gc.ResourceLabelEvents.ListIssueLabelEvents( conf.project, lei.issue, &gitlab.ListLabelEventsOptions{ @@ -68,7 +68,13 @@ func (lei *labelEventIterator) getNext(ctx context.Context, conf config) (bool, if len(labelEvents) == 0 { break } + lei.cache = append(lei.cache, labelEvents...) + + if resp.TotalPages == page { + break + } + page++ } diff --git a/bridge/gitlab/iterator/note.go b/bridge/gitlab/iterator/note.go index 486ca94e..a1e0544c 100644 --- a/bridge/gitlab/iterator/note.go +++ b/bridge/gitlab/iterator/note.go @@ -7,10 +7,11 @@ import ( ) type noteIterator struct { - issue int - page int - index int - cache []*gitlab.Note + issue int + page int + lastPage bool + index int + cache []*gitlab.Note } func newNoteIterator() *noteIterator { @@ -39,10 +40,14 @@ func (in *noteIterator) Value() *gitlab.Note { } func (in *noteIterator) getNext(ctx context.Context, conf config) (bool, error) { + if in.lastPage { + return false, nil + } + ctx, cancel := context.WithTimeout(ctx, conf.timeout) defer cancel() - notes, _, err := conf.gc.Notes.ListIssueNotes( + notes, resp, err := conf.gc.Notes.ListIssueNotes( conf.project, in.issue, &gitlab.ListIssueNotesOptions{ @@ -61,6 +66,10 @@ func (in *noteIterator) getNext(ctx context.Context, conf config) (bool, error) return false, err } + if resp.TotalPages == in.page { + in.lastPage = true + } + if len(notes) == 0 { return false, nil } @@ -75,6 +84,7 @@ func (in *noteIterator) getNext(ctx context.Context, conf config) (bool, error) func (in *noteIterator) Reset(issue int) { in.issue = issue in.index = -1 - in.page = -1 + in.page = 1 + in.lastPage = false in.cache = nil } -- cgit