From 8498deaa194f7ad3d69787d89e26feb280986d5c Mon Sep 17 00:00:00 2001 From: Amine Date: Tue, 1 Oct 2019 10:53:54 +0200 Subject: bridge/gitlab: fix integration tests --- bridge/gitlab/import_test.go | 2 +- bridge/gitlab/iterator.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/bridge/gitlab/import_test.go b/bridge/gitlab/import_test.go index 20fc67c7..a6fd8524 100644 --- a/bridge/gitlab/import_test.go +++ b/bridge/gitlab/import_test.go @@ -19,7 +19,7 @@ import ( ) func TestImport(t *testing.T) { - author := identity.NewIdentity("Amine hilaly", "hilalyamine@gmail.com") + author := identity.NewIdentity("Amine Hilaly", "hilalyamine@gmail.com") tests := []struct { name string url string diff --git a/bridge/gitlab/iterator.go b/bridge/gitlab/iterator.go index 198af79b..d5425cee 100644 --- a/bridge/gitlab/iterator.go +++ b/bridge/gitlab/iterator.go @@ -2,6 +2,7 @@ package gitlab import ( "context" + "sort" "time" "github.com/xanzy/go-gitlab" @@ -25,6 +26,20 @@ type labelEventIterator struct { cache []*gitlab.LabelEvent } +func (l *labelEventIterator) Len() int { + return len(l.cache) +} + +func (l *labelEventIterator) Swap(i, j int) { + element := l.cache[i] + l.cache[i] = l.cache[j] + l.cache[j] = element +} + +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 @@ -240,6 +255,8 @@ func (i *iterator) getNextLabelEvents() bool { i.labelEvent.cache = labelEvents i.labelEvent.page++ i.labelEvent.index = 0 + + sort.Sort(i.labelEvent) return true } -- cgit From 312bc58c540902901ca9ad7238b4cf85eefb9d24 Mon Sep 17 00:00:00 2001 From: Amine Date: Wed, 2 Oct 2019 15:39:24 +0200 Subject: bridge/gitlab: iterator now query all label events when NextLabelEvent() i called, and sort them by ID --- bridge/gitlab/iterator.go | 55 +++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/bridge/gitlab/iterator.go b/bridge/gitlab/iterator.go index d5425cee..aef68faa 100644 --- a/bridge/gitlab/iterator.go +++ b/bridge/gitlab/iterator.go @@ -224,40 +224,39 @@ func (i *iterator) NoteValue() *gitlab.Note { return i.note.cache[i.note.index] } -func (i *iterator) getNextLabelEvents() bool { +func (i *iterator) getLabelEvents() bool { ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout) defer cancel() - labelEvents, _, err := i.gc.ResourceLabelEvents.ListIssueLabelEvents( - i.project, - i.IssueValue().IID, - &gitlab.ListLabelEventsOptions{ - ListOptions: gitlab.ListOptions{ - Page: i.labelEvent.page, - PerPage: i.capacity, + hasNextPage := true + for hasNextPage { + labelEvents, _, err := i.gc.ResourceLabelEvents.ListIssueLabelEvents( + i.project, + i.IssueValue().IID, + &gitlab.ListLabelEventsOptions{ + ListOptions: gitlab.ListOptions{ + Page: i.labelEvent.page, + PerPage: i.capacity, + }, }, - }, - gitlab.WithContext(ctx), - ) - - if err != nil { - i.err = err - return false - } - - if len(labelEvents) == 0 { - i.labelEvent.page = 1 - i.labelEvent.index = -1 - i.labelEvent.cache = nil - return false + gitlab.WithContext(ctx), + ) + if err != nil { + i.err = err + return false + } + + i.labelEvent.page++ + hasNextPage = len(labelEvents) != 0 + i.labelEvent.cache = append(i.labelEvent.cache, labelEvents...) } - i.labelEvent.cache = labelEvents - i.labelEvent.page++ + i.labelEvent.page = 1 i.labelEvent.index = 0 - sort.Sort(i.labelEvent) - return true + + // if the label events list is empty return false + return len(i.labelEvent.cache) != 0 } // because Gitlab @@ -271,7 +270,7 @@ func (i *iterator) NextLabelEvent() bool { } if len(i.labelEvent.cache) == 0 { - return i.getNextLabelEvents() + return i.getLabelEvents() } // move cursor index @@ -280,7 +279,7 @@ func (i *iterator) NextLabelEvent() bool { return true } - return i.getNextLabelEvents() + return false } func (i *iterator) LabelEventValue() *gitlab.LabelEvent { -- cgit From ed774e4e505caa480284dcbff38a933a5041e998 Mon Sep 17 00:00:00 2001 From: Amine Date: Thu, 3 Oct 2019 12:29:20 +0200 Subject: bridge/gitlab: iterator use simple swap bridge/gitlab: add documentation explaining why we are doing this --- bridge/gitlab/iterator.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/bridge/gitlab/iterator.go b/bridge/gitlab/iterator.go index aef68faa..902dc9f1 100644 --- a/bridge/gitlab/iterator.go +++ b/bridge/gitlab/iterator.go @@ -20,8 +20,10 @@ type noteIterator struct { 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 { - page int index int cache []*gitlab.LabelEvent } @@ -31,9 +33,7 @@ func (l *labelEventIterator) Len() int { } func (l *labelEventIterator) Swap(i, j int) { - element := l.cache[i] - l.cache[i] = l.cache[j] - l.cache[j] = element + l.cache[i], l.cache[j] = l.cache[j], l.cache[i] } func (l *labelEventIterator) Less(i, j int) bool { @@ -88,7 +88,6 @@ func NewIterator(ctx context.Context, capacity int, projectID, token string, sin }, labelEvent: &labelEventIterator{ index: -1, - page: 1, }, } } @@ -228,6 +227,9 @@ 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( @@ -235,7 +237,7 @@ func (i *iterator) getLabelEvents() bool { i.IssueValue().IID, &gitlab.ListLabelEventsOptions{ ListOptions: gitlab.ListOptions{ - Page: i.labelEvent.page, + Page: page, PerPage: i.capacity, }, }, @@ -246,12 +248,11 @@ func (i *iterator) getLabelEvents() bool { return false } - i.labelEvent.page++ + page++ hasNextPage = len(labelEvents) != 0 i.labelEvent.cache = append(i.labelEvent.cache, labelEvents...) } - i.labelEvent.page = 1 i.labelEvent.index = 0 sort.Sort(i.labelEvent) -- cgit