Merge pull request #363 from MichaelMure/gitlab-iterator0.7.1

gitlab: refactor the iterator, fix bug

6 files changed, 425 insertions, 290 deletions

diff --git a/bridge/gitlab/import.go b/bridge/gitlab/import.go
index 0a47a783..8aeab1ce 100644
--- a/bridge/gitlab/import.go
+++ b/bridge/gitlab/import.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/MichaelMure/git-bug/bridge/core"
 	"github.com/MichaelMure/git-bug/bridge/core/auth"
+	"github.com/MichaelMure/git-bug/bridge/gitlab/iterator"
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/cache"
 	"github.com/MichaelMure/git-bug/entity"
@@ -24,7 +25,7 @@ type gitlabImporter struct {
 	client *gitlab.Client
 
 	// iterator
-	iterator *iterator
+	iterator *iterator.Iterator
 
 	// send only channel
 	out chan<- core.ImportResult
@@ -58,7 +59,7 @@ func (gi *gitlabImporter) Init(_ context.Context, repo *cache.RepoCache, conf co
 // ImportAll iterate over all the configured repository issues (notes) and ensure the creation
 // of the missing issues / comments / label events / title changes ...
 func (gi *gitlabImporter) ImportAll(ctx context.Context, repo *cache.RepoCache, since time.Time) (<-chan core.ImportResult, error) {
-	gi.iterator = NewIterator(ctx, gi.client, 10, gi.conf[confKeyProjectID], since)
+	gi.iterator = iterator.NewIterator(ctx, gi.client, 10, gi.conf[confKeyProjectID], since)
 	out := make(chan core.ImportResult)
 	gi.out = out
 
diff --git a/bridge/gitlab/iterator.go b/bridge/gitlab/iterator.go
deleted file mode 100644
index 07f9cce9..00000000
--- a/bridge/gitlab/iterator.go
+++ /dev/null
@@ -1,288 +0,0 @@
-package gitlab
-
-import (
-	"context"
-	"sort"
-	"time"
-
-	"github.com/xanzy/go-gitlab"
-)
-
-type issueIterator struct {
-	page  int
-	index int
-	cache []*gitlab.Issue
-}
-
-type noteIterator struct {
-	page  int
-	index int
-	cache []*gitlab.Note
-}
-
-// Since Gitlab does not return the label events items in the correct order
-// we need to sort the list our selfs and stop relying on the pagination model
-// #BecauseGitlab
-type labelEventIterator struct {
-	index int
-	cache []*gitlab.LabelEvent
-}
-
-func (l *labelEventIterator) Len() int {
-	return len(l.cache)
-}
-
-func (l *labelEventIterator) Swap(i, j int) {
-	l.cache[i], l.cache[j] = l.cache[j], l.cache[i]
-}
-
-func (l *labelEventIterator) Less(i, j int) bool {
-	return l.cache[i].ID < l.cache[j].ID
-}
-
-type iterator struct {
-	// gitlab api v4 client
-	gc *gitlab.Client
-
-	// 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
-
-	// shared context
-	ctx context.Context
-
-	// sticky error
-	err error
-
-	// issues iterator
-	issue *issueIterator
-
-	// notes iterator
-	note *noteIterator
-
-	// labelEvent iterator
-	labelEvent *labelEventIterator
-}
-
-// NewIterator create a new iterator
-func NewIterator(ctx context.Context, client *gitlab.Client, capacity int, projectID string, since time.Time) *iterator {
-	return &iterator{
-		gc:       client,
-		project:  projectID,
-		since:    since,
-		capacity: capacity,
-		ctx:      ctx,
-		issue: &issueIterator{
-			index: -1,
-			page:  1,
-		},
-		note: &noteIterator{
-			index: -1,
-			page:  1,
-		},
-		labelEvent: &labelEventIterator{
-			index: -1,
-		},
-	}
-}
-
-// Error return last encountered error
-func (i *iterator) Error() error {
-	return i.err
-}
-
-func (i *iterator) getNextIssues() bool {
-	ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout)
-	defer cancel()
-
-	issues, _, err := i.gc.Issues.ListProjectIssues(
-		i.project,
-		&gitlab.ListProjectIssuesOptions{
-			ListOptions: gitlab.ListOptions{
-				Page:    i.issue.page,
-				PerPage: i.capacity,
-			},
-			Scope:        gitlab.String("all"),
-			UpdatedAfter: &i.since,
-			Sort:         gitlab.String("asc"),
-		},
-		gitlab.WithContext(ctx),
-	)
-
-	if err != nil {
-		i.err = err
-		return false
-	}
-
-	// if repository doesn't have any issues
-	if len(issues) == 0 {
-		return false
-	}
-
-	i.issue.cache = issues
-	i.issue.index = 0
-	i.issue.page++
-	i.note.index = -1
-	i.note.cache = nil
-
-	return true
-}
-
-func (i *iterator) NextIssue() bool {
-	if i.err != nil {
-		return false
-	}
-
-	if i.ctx.Err() != nil {
-		return false
-	}
-
-	// first query
-	if i.issue.cache == nil {
-		return i.getNextIssues()
-	}
-
-	// move cursor index
-	if i.issue.index < len(i.issue.cache)-1 {
-		i.issue.index++
-		return true
-	}
-
-	return i.getNextIssues()
-}
-
-func (i *iterator) IssueValue() *gitlab.Issue {
-	return i.issue.cache[i.issue.index]
-}
-
-func (i *iterator) getNextNotes() bool {
-	ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout)
-	defer cancel()
-
-	notes, _, err := i.gc.Notes.ListIssueNotes(
-		i.project,
-		i.IssueValue().IID,
-		&gitlab.ListIssueNotesOptions{
-			ListOptions: gitlab.ListOptions{
-				Page:    i.note.page,
-				PerPage: i.capacity,
-			},
-			Sort:    gitlab.String("asc"),
-			OrderBy: gitlab.String("created_at"),
-		},
-		gitlab.WithContext(ctx),
-	)
-
-	if err != nil {
-		i.err = err
-		return false
-	}
-
-	if len(notes) == 0 {
-		i.note.index = -1
-		i.note.page = 1
-		i.note.cache = nil
-		return false
-	}
-
-	i.note.cache = notes
-	i.note.page++
-	i.note.index = 0
-	return true
-}
-
-func (i *iterator) NextNote() bool {
-	if i.err != nil {
-		return false
-	}
-
-	if i.ctx.Err() != nil {
-		return false
-	}
-
-	if len(i.note.cache) == 0 {
-		return i.getNextNotes()
-	}
-
-	// move cursor index
-	if i.note.index < len(i.note.cache)-1 {
-		i.note.index++
-		return true
-	}
-
-	return i.getNextNotes()
-}
-
-func (i *iterator) NoteValue() *gitlab.Note {
-	return i.note.cache[i.note.index]
-}
-
-func (i *iterator) getLabelEvents() bool {
-	ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout)
-	defer cancel()
-
-	// since order is not garanteed we should query all label events
-	// and sort them by ID
-	page := 1
-	hasNextPage := true
-	for hasNextPage {
-		labelEvents, _, err := i.gc.ResourceLabelEvents.ListIssueLabelEvents(
-			i.project,
-			i.IssueValue().IID,
-			&gitlab.ListLabelEventsOptions{
-				ListOptions: gitlab.ListOptions{
-					Page:    page,
-					PerPage: i.capacity,
-				},
-			},
-			gitlab.WithContext(ctx),
-		)
-		if err != nil {
-			i.err = err
-			return false
-		}
-
-		page++
-		hasNextPage = len(labelEvents) != 0
-		i.labelEvent.cache = append(i.labelEvent.cache, labelEvents...)
-	}
-
-	i.labelEvent.index = 0
-	sort.Sort(i.labelEvent)
-
-	// if the label events list is empty return false
-	return len(i.labelEvent.cache) != 0
-}
-
-// because Gitlab
-func (i *iterator) NextLabelEvent() bool {
-	if i.err != nil {
-		return false
-	}
-
-	if i.ctx.Err() != nil {
-		return false
-	}
-
-	if len(i.labelEvent.cache) == 0 {
-		return i.getLabelEvents()
-	}
-
-	// move cursor index
-	if i.labelEvent.index < len(i.labelEvent.cache)-1 {
-		i.labelEvent.index++
-		return true
-	}
-
-	return false
-}
-
-func (i *iterator) LabelEventValue() *gitlab.LabelEvent {
-	return i.labelEvent.cache[i.labelEvent.index]
-}
diff --git a/bridge/gitlab/iterator/issue.go b/bridge/gitlab/iterator/issue.go
new file mode 100644
index 00000000..9361b496
--- /dev/null
+++ b/bridge/gitlab/iterator/issue.go
@@ -0,0 +1,89 @@
+package iterator
+
+import (
+	"context"
+
+	"github.com/xanzy/go-gitlab"
+)
+
+type issueIterator struct {
+	page     int
+	lastPage bool
+	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) {
+	if ii.lastPage {
+		return false, nil
+	}
+
+	ctx, cancel := context.WithTimeout(ctx, conf.timeout)
+	defer cancel()
+
+	issues, resp, 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 resp.TotalPages == ii.page {
+		ii.lastPage = true
+	}
+
+	// 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.lastPage = false
+	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..812e6646
--- /dev/null
+++ b/bridge/gitlab/iterator/labelEvent.go
@@ -0,0 +1,105 @@
+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, resp, 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...)
+
+		if resp.TotalPages == page {
+			break
+		}
+
+		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..a1e0544c
--- /dev/null
+++ b/bridge/gitlab/iterator/note.go
@@ -0,0 +1,90 @@
+package iterator
+
+import (
+	"context"
+
+	"github.com/xanzy/go-gitlab"
+)
+
+type noteIterator struct {
+	issue    int
+	page     int
+	lastPage bool
+	index    int
+	cache    []*gitlab.Note
+}
+
+func newNoteIterator() *noteIterator {
+	in := &noteIterator{}
+	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) {
+	if in.lastPage {
+		return false, nil
+	}
+
+	ctx, cancel := context.WithTimeout(ctx, conf.timeout)
+	defer cancel()
+
+	notes, resp, 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 resp.TotalPages == in.page {
+		in.lastPage = true
+	}
+
+	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.lastPage = false
+	in.cache = nil
+}