From c8ad4dbfd9511f4cfa748fa85c01fbca2edb348a Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sun, 21 Apr 2019 21:26:42 +0200 Subject: Add github iterator use `goto` in .Next* functions Update iterator.go --- bridge/github/import_query.go | 6 +- bridge/github/iterator.go | 406 +++++++++++++++++++++++++++++++++++++++++ bridge/github/iterator_test.go | 44 +++++ 3 files changed, 453 insertions(+), 3 deletions(-) create mode 100644 bridge/github/iterator.go create mode 100644 bridge/github/iterator_test.go (limited to 'bridge') diff --git a/bridge/github/import_query.go b/bridge/github/import_query.go index 59799f6a..4d5886f6 100644 --- a/bridge/github/import_query.go +++ b/bridge/github/import_query.go @@ -128,7 +128,7 @@ type issueTimelineQuery struct { Issues struct { Nodes []issueTimeline PageInfo pageInfo - } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC})"` + } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC}, filterBy: {since: $issueSince})"` } `graphql:"repository(owner: $owner, name: $name)"` } @@ -137,7 +137,7 @@ type issueEditQuery struct { Issues struct { Nodes []issueEdit PageInfo pageInfo - } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC})"` + } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC}, filterBy: {since: $issueSince})"` } `graphql:"repository(owner: $owner, name: $name)"` } @@ -156,7 +156,7 @@ type commentEditQuery struct { } } `graphql:"timeline(first: $timelineFirst, after: $timelineAfter)"` } - } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC})"` + } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC}, filterBy: {since: $issueSince})"` } `graphql:"repository(owner: $owner, name: $name)"` } diff --git a/bridge/github/iterator.go b/bridge/github/iterator.go new file mode 100644 index 00000000..cb7c9760 --- /dev/null +++ b/bridge/github/iterator.go @@ -0,0 +1,406 @@ +package github + +import ( + "context" + "time" + + "github.com/MichaelMure/git-bug/bridge/core" + "github.com/shurcooL/githubv4" +) + +/** +type iterator interface { + Count() int + Error() error + + NextIssue() bool + NextIssueEdit() bool + NextTimeline() bool + NextCommentEdit() bool + + IssueValue() issueTimeline + IssueEditValue() userContentEdit + TimelineValue() timelineItem + CommentEditValue() userContentEdit +} +*/ + +type indexer struct{ index int } + +type issueEditIterator struct { + index int + query issueEditQuery + variables map[string]interface{} +} + +type commentEditIterator struct { + index int + query commentEditQuery + variables map[string]interface{} +} + +type timelineIterator struct { + index int + query issueTimelineQuery + variables map[string]interface{} + + issueEdit indexer + commentEdit indexer +} + +type iterator struct { + // github graphql client + gc *githubv4.Client + + // if since is given the iterator will query only the updated + // and created issues after this date + since time.Time + + // number of timelines/userEditcontent/issueEdit to query + // at a time more capacity = more used memory = less queries + // to make + capacity int + + // sticky error + err error + + // count to keep track of the number of imported issues + count int + + // timeline iterator + timeline timelineIterator + + // issue edit iterator + issueEdit issueEditIterator + + // comment edit iterator + commentEdit commentEditIterator +} + +func newIterator(conf core.Configuration, since time.Time) *iterator { + return &iterator{ + since: since, + gc: buildClient(conf), + capacity: 8, + count: -1, + + timeline: timelineIterator{ + index: -1, + issueEdit: indexer{-1}, + commentEdit: indexer{-1}, + variables: map[string]interface{}{ + "owner": githubv4.String(conf["user"]), + "name": githubv4.String(conf["project"]), + }, + }, + commentEdit: commentEditIterator{ + index: -1, + variables: map[string]interface{}{ + "owner": githubv4.String(conf["user"]), + "name": githubv4.String(conf["project"]), + }, + }, + issueEdit: issueEditIterator{ + index: -1, + variables: map[string]interface{}{ + "owner": githubv4.String(conf["user"]), + "name": githubv4.String(conf["project"]), + }, + }, + } +} + +// init issue timeline variables +func (i *iterator) initTimelineQueryVariables() { + i.timeline.variables["issueFirst"] = githubv4.Int(1) + i.timeline.variables["issueAfter"] = (*githubv4.String)(nil) + i.timeline.variables["issueSince"] = githubv4.DateTime{Time: i.since} + i.timeline.variables["timelineFirst"] = githubv4.Int(i.capacity) + i.timeline.variables["timelineAfter"] = (*githubv4.String)(nil) + i.timeline.variables["issueEditLast"] = githubv4.Int(i.capacity) + i.timeline.variables["issueEditBefore"] = (*githubv4.String)(nil) + i.timeline.variables["commentEditLast"] = githubv4.Int(i.capacity) + i.timeline.variables["commentEditBefore"] = (*githubv4.String)(nil) +} + +// init issue edit variables +func (i *iterator) initIssueEditQueryVariables() { + i.issueEdit.variables["issueFirst"] = githubv4.Int(1) + i.issueEdit.variables["issueAfter"] = i.timeline.variables["issueAfter"] + i.issueEdit.variables["issueSince"] = githubv4.DateTime{Time: i.since} + i.issueEdit.variables["issueEditLast"] = githubv4.Int(i.capacity) + i.issueEdit.variables["issueEditBefore"] = (*githubv4.String)(nil) +} + +// init issue comment variables +func (i *iterator) initCommentEditQueryVariables() { + i.commentEdit.variables["issueFirst"] = githubv4.Int(1) + i.commentEdit.variables["issueAfter"] = i.timeline.variables["issueAfter"] + i.commentEdit.variables["issueSince"] = githubv4.DateTime{Time: i.since} + i.commentEdit.variables["timelineFirst"] = githubv4.Int(1) + i.commentEdit.variables["timelineAfter"] = (*githubv4.String)(nil) + i.commentEdit.variables["commentEditLast"] = githubv4.Int(i.capacity) + i.commentEdit.variables["commentEditBefore"] = (*githubv4.String)(nil) +} + +// reverse UserContentEdits arrays in both of the issue and +// comment timelines +func (i *iterator) reverseTimelineEditNodes() { + reverseEdits(i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) + for index, ce := range i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges { + if ce.Node.Typename == "IssueComment" && len(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges) != 0 { + reverseEdits(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[index].Node.IssueComment.UserContentEdits.Nodes) + } + } +} + +// Error . +func (i *iterator) Error() error { + return i.err +} + +// Count . +func (i *iterator) Count() int { + return i.count +} + +func (i *iterator) NextIssue() bool { + // we make the first move + if i.count == -1 { + + // init variables and goto queryIssue block + i.initTimelineQueryVariables() + goto queryIssue + } + + if i.err != nil { + return false + } + + if !i.timeline.query.Repository.Issues.PageInfo.HasNextPage { + return false + } + + // if we have more pages updates variables and query them + i.timeline.variables["timelineAfter"] = (*githubv4.String)(nil) + i.timeline.variables["issueAfter"] = i.timeline.query.Repository.Issues.PageInfo.EndCursor + i.timeline.index = -1 + + // query issue block +queryIssue: + if err := i.gc.Query(context.TODO(), &i.timeline.query, i.timeline.variables); err != nil { + i.err = err + return false + } + + if len(i.timeline.query.Repository.Issues.Nodes) == 0 { + return false + } + + i.reverseTimelineEditNodes() + i.count++ + return true +} + +func (i *iterator) IssueValue() issueTimeline { + return i.timeline.query.Repository.Issues.Nodes[0] +} + +func (i *iterator) NextTimeline() bool { + if i.err != nil { + return false + } + + if len(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges) == 0 { + return false + } + + if i.timeline.index < min(i.capacity, len(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges))-1 { + i.timeline.index++ + return true + } + + if !i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.HasNextPage { + return false + } + + // more timelines, query them + i.timeline.variables["timelineAfter"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor + if err := i.gc.Query(context.TODO(), &i.timeline.query, i.timeline.variables); err != nil { + i.err = err + return false + } + + i.reverseTimelineEditNodes() + i.timeline.index = 0 + return true +} + +func (i *iterator) TimelineValue() timelineItem { + return i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node +} + +func (i *iterator) timelineCursor() string { + return "" +} + +func (i *iterator) NextIssueEdit() bool { + if i.err != nil { + return false + } + + // this mean we looped over all available issue edits in the timeline. + // now we have to use i.issueEditQuery + if i.timeline.issueEdit.index == -2 { + if i.issueEdit.index < min(i.capacity, len(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes))-1 { + i.issueEdit.index++ + return true + } + + if !i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.PageInfo.HasPreviousPage { + i.timeline.issueEdit.index = -1 + i.issueEdit.index = -1 + return false + } + + // if there is more edits, query them + i.issueEdit.variables["issueEditBefore"] = i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.PageInfo.StartCursor + goto queryIssueEdit + } + + // if there is no edits + if len(i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) == 0 { + return false + } + + // loop over them timeline comment edits + if i.timeline.issueEdit.index < min(i.capacity, len(i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes))-1 { + i.timeline.issueEdit.index++ + return true + } + + if !i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.PageInfo.HasPreviousPage { + i.timeline.issueEdit.index = -1 + return false + } + + // if there is more edits, query them + i.initIssueEditQueryVariables() + i.issueEdit.variables["issueEditBefore"] = i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.PageInfo.StartCursor + +queryIssueEdit: + if err := i.gc.Query(context.TODO(), &i.issueEdit.query, i.issueEdit.variables); err != nil { + i.err = err + //i.timeline.issueEdit.index = -1 + return false + } + + // reverse issue edits because github + reverseEdits(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) + + // this is not supposed to happen + if len(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) == 0 { + i.timeline.issueEdit.index = -1 + return false + } + + i.issueEdit.index = 0 + i.timeline.issueEdit.index = -2 + return true +} + +func (i *iterator) IssueEditValue() userContentEdit { + // if we are using issue edit query + if i.timeline.issueEdit.index == -2 { + return i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes[i.issueEdit.index] + } + + // else get it from timeline issue edit query + return i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes[i.timeline.issueEdit.index] +} + +func (i *iterator) NextCommentEdit() bool { + if i.err != nil { + return false + } + + // same as NextIssueEdit + if i.timeline.commentEdit.index == -2 { + + if i.commentEdit.index < min(i.capacity, len(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes))-1 { + i.commentEdit.index++ + return true + } + + if !i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.PageInfo.HasPreviousPage { + i.timeline.commentEdit.index = -1 + i.commentEdit.index = -1 + return false + } + + // if there is more comment edits, query them + i.commentEdit.variables["commentEditBefore"] = i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.PageInfo.StartCursor + goto queryCommentEdit + } + + // if there is no comment edits + if len(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node.IssueComment.UserContentEdits.Nodes) == 0 { + return false + } + + // loop over them timeline comment edits + if i.timeline.commentEdit.index < min(i.capacity, len(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node.IssueComment.UserContentEdits.Nodes))-1 { + i.timeline.commentEdit.index++ + return true + } + + if !i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node.IssueComment.UserContentEdits.PageInfo.HasPreviousPage { + i.timeline.commentEdit.index = -1 + return false + } + + // if there is more comment edits, query them + + i.initCommentEditQueryVariables() + if i.timeline.index == 0 { + i.commentEdit.variables["timelineAfter"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor + } else { + i.commentEdit.variables["timelineAfter"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index-1].Cursor + } + + i.commentEdit.variables["commentEditBefore"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node.IssueComment.UserContentEdits.PageInfo.StartCursor + +queryCommentEdit: + if err := i.gc.Query(context.TODO(), &i.commentEdit.query, i.commentEdit.variables); err != nil { + i.err = err + return false + } + + // this is not supposed to happen + if len(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes) == 0 { + i.timeline.commentEdit.index = -1 + return false + } + + reverseEdits(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes) + + i.commentEdit.index = 0 + i.timeline.commentEdit.index = -2 + return true +} + +func (i *iterator) CommentEditValue() userContentEdit { + if i.timeline.commentEdit.index == -2 { + return i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes[i.commentEdit.index] + } + + return i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node.IssueComment.UserContentEdits.Nodes[i.timeline.commentEdit.index] +} + +func min(a, b int) int { + if a > b { + return b + } + + return a +} diff --git a/bridge/github/iterator_test.go b/bridge/github/iterator_test.go new file mode 100644 index 00000000..c5820973 --- /dev/null +++ b/bridge/github/iterator_test.go @@ -0,0 +1,44 @@ +package github + +import ( + "fmt" + "os" + "testing" + "time" +) + +func Test_Iterator(t *testing.T) { + token := os.Getenv("GITHUB_TOKEN") + user := os.Getenv("GITHUB_USER") + project := os.Getenv("GITHUB_PROJECT") + + i := newIterator(map[string]string{ + keyToken: token, + "user": user, + "project": project, + }, time.Now().Add(-14*24*time.Hour)) + + for i.NextIssue() { + v := i.IssueValue() + fmt.Printf("issue = id:%v title:%v\n", v.Id, v.Title) + + for i.NextIssueEdit() { + v := i.IssueEditValue() + fmt.Printf("issue edit = %v\n", string(*v.Diff)) + } + + for i.NextTimeline() { + v := i.TimelineValue() + fmt.Printf("timeline = type:%v\n", v.Typename) + + if v.Typename == "IssueComment" { + for i.NextCommentEdit() { + _ = i.CommentEditValue() + + //fmt.Printf("comment edit: %v\n", *v.Diff) + fmt.Printf("comment edit\n") + } + } + } + } +} -- cgit From 3bcaa35b5d25ca9e12389ab4bf78600ae5df8af8 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sat, 27 Apr 2019 00:15:02 +0100 Subject: Integrate iterator with importer --- bridge/github/config.go | 10 +- bridge/github/import.go | 594 ++++++++++++++--------------------------- bridge/github/iterator.go | 29 +- bridge/github/iterator_test.go | 10 +- 4 files changed, 222 insertions(+), 421 deletions(-) (limited to 'bridge') diff --git a/bridge/github/config.go b/bridge/github/config.go index b881c585..2a3119a6 100644 --- a/bridge/github/config.go +++ b/bridge/github/config.go @@ -20,10 +20,12 @@ import ( "golang.org/x/crypto/ssh/terminal" ) -const githubV3Url = "https://api.github.com" -const keyUser = "user" -const keyProject = "project" -const keyToken = "token" +const ( + githubV3Url = "https://api.github.com" + keyUser = "user" + keyProject = "project" + keyToken = "token" +) func (*Github) Configure(repo repository.RepoCommon) (core.Configuration, error) { conf := make(core.Configuration) diff --git a/bridge/github/import.go b/bridge/github/import.go index d641b192..74ccb776 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/MichaelMure/git-bug/bridge/core" "github.com/MichaelMure/git-bug/bug" @@ -13,308 +14,250 @@ import ( "github.com/shurcooL/githubv4" ) -const keyGithubId = "github-id" -const keyGithubUrl = "github-url" -const keyGithubLogin = "github-login" +const ( + keyGithubId = "github-id" + keyGithubUrl = "github-url" + keyGithubLogin = "github-login" +) // githubImporter implement the Importer interface type githubImporter struct { - client *githubv4.Client - conf core.Configuration + iterator *iterator + conf core.Configuration } func (gi *githubImporter) Init(conf core.Configuration) error { - gi.conf = conf - gi.client = buildClient(conf) - - return nil -} + var since time.Time -func (gi *githubImporter) ImportAll(repo *cache.RepoCache) error { - q := &issueTimelineQuery{} - variables := map[string]interface{}{ - "owner": githubv4.String(gi.conf[keyUser]), - "name": githubv4.String(gi.conf[keyProject]), - "issueFirst": githubv4.Int(1), - "issueAfter": (*githubv4.String)(nil), - "timelineFirst": githubv4.Int(10), - "timelineAfter": (*githubv4.String)(nil), - - // Fun fact, github provide the comment edition in reverse chronological - // order, because haha. Look at me, I'm dying of laughter. - "issueEditLast": githubv4.Int(10), - "issueEditBefore": (*githubv4.String)(nil), - "commentEditLast": githubv4.Int(10), - "commentEditBefore": (*githubv4.String)(nil), - } - - var b *cache.BugCache - - for { - err := gi.client.Query(context.TODO(), &q, variables) + // parse since value from configuration + if value, ok := conf["since"]; ok && value != "" { + s, err := time.Parse(time.RFC3339, value) if err != nil { return err } - if len(q.Repository.Issues.Nodes) == 0 { - return nil - } - - issue := q.Repository.Issues.Nodes[0] - - if b == nil { - b, err = gi.ensureIssue(repo, issue, variables) - if err != nil { - return err - } - } - - for _, itemEdge := range q.Repository.Issues.Nodes[0].Timeline.Edges { - err = gi.ensureTimelineItem(repo, b, itemEdge.Cursor, itemEdge.Node, variables) - if err != nil { - return err - } - } - - if !issue.Timeline.PageInfo.HasNextPage { - err = b.CommitAsNeeded() - if err != nil { - return err - } - - b = nil - - if !q.Repository.Issues.PageInfo.HasNextPage { - break - } - - variables["issueAfter"] = githubv4.NewString(q.Repository.Issues.PageInfo.EndCursor) - variables["timelineAfter"] = (*githubv4.String)(nil) - continue - } - - variables["timelineAfter"] = githubv4.NewString(issue.Timeline.PageInfo.EndCursor) + since = s } + gi.iterator = newIterator(conf, since) return nil } -func (gi *githubImporter) Import(repo *cache.RepoCache, id string) error { - fmt.Println("IMPORT") - - return nil -} - -func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline, rootVariables map[string]interface{}) (*cache.BugCache, error) { - fmt.Printf("import issue: %s\n", issue.Title) - - author, err := gi.ensurePerson(repo, issue.Author) - if err != nil { - return nil, err - } - - b, err := repo.ResolveBugCreateMetadata(keyGithubId, parseId(issue.Id)) - if err != nil && err != bug.ErrBugNotExist { - return nil, err - } - - // if there is no edit, the UserContentEdits given by github is empty. That - // means that the original message is given by the issue message. - // - // if there is edits, the UserContentEdits given by github contains both the - // original message and the following edits. The issue message give the last - // version so we don't care about that. - // - // the tricky part: for an issue older than the UserContentEdits API, github - // doesn't have the previous message version anymore and give an edition - // with .Diff == nil. We have to filter them. - - if len(issue.UserContentEdits.Nodes) == 0 { - if err == bug.ErrBugNotExist { - b, err = repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - // Todo: this might not be the initial title, we need to query the - // timeline to be sure - issue.Title, - cleanupText(string(issue.Body)), - nil, - map[string]string{ - keyGithubId: parseId(issue.Id), - keyGithubUrl: issue.Url.String(), - }, - ) - if err != nil { - return nil, err - } - } - - return b, nil - } - - // reverse the order, because github - reverseEdits(issue.UserContentEdits.Nodes) - - for i, edit := range issue.UserContentEdits.Nodes { - if b != nil && i == 0 { - // The first edit in the github result is the creation itself, we already have that - continue - } - - if b == nil { - if edit.Diff == nil { - // not enough data given by github for old edit, ignore them - continue - } - - // we create the bug as soon as we have a legit first edition - b, err = repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - // Todo: this might not be the initial title, we need to query the - // timeline to be sure - issue.Title, - cleanupText(string(*edit.Diff)), - nil, - map[string]string{ - keyGithubId: parseId(issue.Id), - keyGithubUrl: issue.Url.String(), - }, - ) - if err != nil { - return nil, err - } - continue - } +func (gi *githubImporter) ImportAll(repo *cache.RepoCache) error { + // Loop over all available issues + for gi.iterator.NextIssue() { + issue := gi.iterator.IssueValue() + fmt.Printf("importing issue: %v\n", issue.Title) - target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(issue.Id)) - if err != nil { - return nil, err - } + // In each iteration create a new bug + var b *cache.BugCache - err = gi.ensureCommentEdit(repo, b, target, edit) + // ensure issue author + author, err := gi.ensurePerson(repo, issue.Author) if err != nil { - return nil, err - } - } - - if !issue.UserContentEdits.PageInfo.HasNextPage { - // if we still didn't get a legit edit, create the bug from the issue data - if b == nil { - return repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - // Todo: this might not be the initial title, we need to query the - // timeline to be sure - issue.Title, - cleanupText(string(issue.Body)), - nil, - map[string]string{ - keyGithubId: parseId(issue.Id), - keyGithubUrl: issue.Url.String(), - }, - ) + return err } - return b, nil - } - // We have more edit, querying them - - q := &issueEditQuery{} - variables := map[string]interface{}{ - "owner": rootVariables["owner"], - "name": rootVariables["name"], - "issueFirst": rootVariables["issueFirst"], - "issueAfter": rootVariables["issueAfter"], - "issueEditLast": githubv4.Int(10), - "issueEditBefore": issue.UserContentEdits.PageInfo.StartCursor, - } - - for { - err := gi.client.Query(context.TODO(), &q, variables) - if err != nil { - return nil, err + // resolve bug + b, err = repo.ResolveBugCreateMetadata(keyGithubId, parseId(issue.Id)) + if err != nil && err != bug.ErrBugNotExist { + return err } - edits := q.Repository.Issues.Nodes[0].UserContentEdits - - if len(edits.Nodes) == 0 { - return b, nil + // get issue edits + issueEdits := []userContentEdit{} + for gi.iterator.NextIssueEdit() { + // append only edits with non empty diff + if issueEdit := gi.iterator.IssueEditValue(); issueEdit.Diff != nil { + issueEdits = append(issueEdits, issueEdit) + } } - for _, edit := range edits.Nodes { - if b == nil { - if edit.Diff == nil { - // not enough data given by github for old edit, ignore them - continue - } - - // we create the bug as soon as we have a legit first edition + // if issueEdits is empty + if len(issueEdits) == 0 { + if err == bug.ErrBugNotExist { + // create bug b, err = repo.NewBugRaw( author, issue.CreatedAt.Unix(), - // Todo: this might not be the initial title, we need to query the - // timeline to be sure issue.Title, - cleanupText(string(*edit.Diff)), + cleanupText(string(issue.Body)), nil, map[string]string{ keyGithubId: parseId(issue.Id), keyGithubUrl: issue.Url.String(), - }, - ) + }) if err != nil { - return nil, err + return err } - continue } + } else { + // create bug from given issueEdits + for _, edit := range issueEdits { + // if the bug doesn't exist + if b == nil { + // we create the bug as soon as we have a legit first edition + b, err = repo.NewBugRaw( + author, + issue.CreatedAt.Unix(), + issue.Title, + cleanupText(string(*edit.Diff)), + nil, + map[string]string{ + keyGithubId: parseId(issue.Id), + keyGithubUrl: issue.Url.String(), + }, + ) + + if err != nil { + return err + } - target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(issue.Id)) - if err != nil { - return nil, err + continue + } + + // other edits will be added as CommentEdit operations + + target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(issue.Id)) + if err != nil { + return err + } + + err = gi.ensureCommentEdit(repo, b, target, edit) + if err != nil { + return err + } } + } + + // check timeline items + for gi.iterator.NextTimeline() { + item := gi.iterator.TimelineValue() + + // if item is not a comment (label, unlabel, rename, close, open ...) + if item.Typename != "IssueComment" { + if err := gi.ensureTimelineItem(repo, b, item); err != nil { + return err + } + } else { // if item is comment + + // ensure person + author, err := gi.ensurePerson(repo, item.IssueComment.Author) + if err != nil { + return err + } + + var target git.Hash + target, err = b.ResolveOperationWithMetadata(keyGithubId, parseId(item.IssueComment.Id)) + if err != nil && err != cache.ErrNoMatchingOp { + // real error + return err + } + + // collect all edits + commentEdits := []userContentEdit{} + for gi.iterator.NextCommentEdit() { + if commentEdit := gi.iterator.CommentEditValue(); commentEdit.Diff != nil { + commentEdits = append(commentEdits, commentEdit) + } + } + + // if no edits are given we create the comment + if len(commentEdits) == 0 { + + // if comment doesn't exist + if err == cache.ErrNoMatchingOp { + + // add comment operation + op, err := b.AddCommentRaw( + author, + item.IssueComment.CreatedAt.Unix(), + cleanupText(string(item.IssueComment.Body)), + nil, + map[string]string{ + keyGithubId: parseId(item.IssueComment.Id), + }, + ) + if err != nil { + return err + } + + // set hash + target, err = op.Hash() + if err != nil { + return err + } + } + } else { + // if we have some edits + for _, edit := range item.IssueComment.UserContentEdits.Nodes { + + // create comment when target is an empty string + if target == "" { + op, err := b.AddCommentRaw( + author, + item.IssueComment.CreatedAt.Unix(), + cleanupText(string(*edit.Diff)), + nil, + map[string]string{ + keyGithubId: parseId(item.IssueComment.Id), + keyGithubUrl: item.IssueComment.Url.String(), + }, + ) + if err != nil { + return err + } + + // set hash + target, err = op.Hash() + if err != nil { + return err + } + } + + err := gi.ensureCommentEdit(repo, b, target, edit) + if err != nil { + return err + } + + } + } - err = gi.ensureCommentEdit(repo, b, target, edit) - if err != nil { - return nil, err } + } - if !edits.PageInfo.HasNextPage { - break + if err := gi.iterator.Error(); err != nil { + fmt.Printf("error importing issue %v\n", issue.Id) + return err } - variables["issueEditBefore"] = edits.PageInfo.StartCursor + // commit bug state + err = b.CommitAsNeeded() + if err != nil { + return err + } } - // TODO: check + import files - - // if we still didn't get a legit edit, create the bug from the issue data - if b == nil { - return repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - // Todo: this might not be the initial title, we need to query the - // timeline to be sure - issue.Title, - cleanupText(string(issue.Body)), - nil, - map[string]string{ - keyGithubId: parseId(issue.Id), - keyGithubUrl: issue.Url.String(), - }, - ) + if err := gi.iterator.Error(); err != nil { + fmt.Printf("import error: %v\n", err) } - return b, nil + fmt.Printf("Successfully imported %v issues from Github\n", gi.iterator.Count()) + return nil } -func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.BugCache, cursor githubv4.String, item timelineItem, rootVariables map[string]interface{}) error { - fmt.Printf("import %s\n", item.Typename) +func (gi *githubImporter) Import(repo *cache.RepoCache, id string) error { + fmt.Println("IMPORT") + return nil +} + +func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.BugCache, item timelineItem) error { + fmt.Printf("import item: %s\n", item.Typename) switch item.Typename { case "IssueComment": - return gi.ensureComment(repo, b, cursor, item.IssueComment, rootVariables) + //return gi.ensureComment(repo, b, cursor, item.IssueComment, rootVariables) case "LabeledEvent": id := parseId(item.LabeledEvent.Id) @@ -411,162 +354,13 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug return err default: - fmt.Println("ignore event ", item.Typename) + fmt.Printf("ignore event: %v\n", item.Typename) } return nil } -func (gi *githubImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, cursor githubv4.String, comment issueComment, rootVariables map[string]interface{}) error { - author, err := gi.ensurePerson(repo, comment.Author) - if err != nil { - return err - } - - var target git.Hash - target, err = b.ResolveOperationWithMetadata(keyGithubId, parseId(comment.Id)) - if err != nil && err != cache.ErrNoMatchingOp { - // real error - return err - } - - // if there is no edit, the UserContentEdits given by github is empty. That - // means that the original message is given by the comment message. - // - // if there is edits, the UserContentEdits given by github contains both the - // original message and the following edits. The comment message give the last - // version so we don't care about that. - // - // the tricky part: for a comment older than the UserContentEdits API, github - // doesn't have the previous message version anymore and give an edition - // with .Diff == nil. We have to filter them. - - if len(comment.UserContentEdits.Nodes) == 0 { - if err == cache.ErrNoMatchingOp { - op, err := b.AddCommentRaw( - author, - comment.CreatedAt.Unix(), - cleanupText(string(comment.Body)), - nil, - map[string]string{ - keyGithubId: parseId(comment.Id), - }, - ) - if err != nil { - return err - } - - target, err = op.Hash() - if err != nil { - return err - } - } - - return nil - } - - // reverse the order, because github - reverseEdits(comment.UserContentEdits.Nodes) - - for i, edit := range comment.UserContentEdits.Nodes { - if target != "" && i == 0 { - // The first edit in the github result is the comment creation itself, we already have that - continue - } - - if target == "" { - if edit.Diff == nil { - // not enough data given by github for old edit, ignore them - continue - } - - op, err := b.AddCommentRaw( - author, - comment.CreatedAt.Unix(), - cleanupText(string(*edit.Diff)), - nil, - map[string]string{ - keyGithubId: parseId(comment.Id), - keyGithubUrl: comment.Url.String(), - }, - ) - if err != nil { - return err - } - - target, err = op.Hash() - if err != nil { - return err - } - } - - err := gi.ensureCommentEdit(repo, b, target, edit) - if err != nil { - return err - } - } - - if !comment.UserContentEdits.PageInfo.HasNextPage { - return nil - } - - // We have more edit, querying them - - q := &commentEditQuery{} - variables := map[string]interface{}{ - "owner": rootVariables["owner"], - "name": rootVariables["name"], - "issueFirst": rootVariables["issueFirst"], - "issueAfter": rootVariables["issueAfter"], - "timelineFirst": githubv4.Int(1), - "timelineAfter": cursor, - "commentEditLast": githubv4.Int(10), - "commentEditBefore": comment.UserContentEdits.PageInfo.StartCursor, - } - - for { - err := gi.client.Query(context.TODO(), &q, variables) - if err != nil { - return err - } - - edits := q.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits - - if len(edits.Nodes) == 0 { - return nil - } - - for i, edit := range edits.Nodes { - if i == 0 { - // The first edit in the github result is the creation itself, we already have that - continue - } - - err := gi.ensureCommentEdit(repo, b, target, edit) - if err != nil { - return err - } - } - - if !edits.PageInfo.HasNextPage { - break - } - - variables["commentEditBefore"] = edits.PageInfo.StartCursor - } - - // TODO: check + import files - - return nil -} - func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugCache, target git.Hash, edit userContentEdit) error { - if edit.Diff == nil { - // this happen if the event is older than early 2018, Github doesn't have the data before that. - // Best we can do is to ignore the event. - return nil - } - _, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(edit.Id)) if err == nil { // already imported @@ -670,7 +464,7 @@ func (gi *githubImporter) getGhost(repo *cache.RepoCache) (*cache.IdentityCache, "login": githubv4.String("ghost"), } - err = gi.client.Query(context.TODO(), &q, variables) + err = gi.iterator.gc.Query(context.TODO(), &q, variables) if err != nil { return nil, err } diff --git a/bridge/github/iterator.go b/bridge/github/iterator.go index cb7c9760..9e1ff30e 100644 --- a/bridge/github/iterator.go +++ b/bridge/github/iterator.go @@ -46,6 +46,8 @@ type timelineIterator struct { issueEdit indexer commentEdit indexer + + lastEndCursor githubv4.String // storing timeline end cursor for future use } type iterator struct { @@ -81,9 +83,8 @@ func newIterator(conf core.Configuration, since time.Time) *iterator { return &iterator{ since: since, gc: buildClient(conf), - capacity: 8, - count: -1, - + capacity: 10, + count: 0, timeline: timelineIterator{ index: -1, issueEdit: indexer{-1}, @@ -154,19 +155,20 @@ func (i *iterator) reverseTimelineEditNodes() { } } -// Error . +// Error return last encountered error func (i *iterator) Error() error { return i.err } -// Count . +// Count return number of issues we iterated over func (i *iterator) Count() int { return i.count } +// Next issue func (i *iterator) NextIssue() bool { // we make the first move - if i.count == -1 { + if i.count == 0 { // init variables and goto queryIssue block i.initTimelineQueryVariables() @@ -181,11 +183,14 @@ func (i *iterator) NextIssue() bool { return false } - // if we have more pages updates variables and query them + // if we have more issues, query them i.timeline.variables["timelineAfter"] = (*githubv4.String)(nil) i.timeline.variables["issueAfter"] = i.timeline.query.Repository.Issues.PageInfo.EndCursor i.timeline.index = -1 + // store cursor for future use + i.timeline.lastEndCursor = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor + // query issue block queryIssue: if err := i.gc.Query(context.TODO(), &i.timeline.query, i.timeline.variables); err != nil { @@ -224,6 +229,8 @@ func (i *iterator) NextTimeline() bool { return false } + i.timeline.lastEndCursor = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor + // more timelines, query them i.timeline.variables["timelineAfter"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor if err := i.gc.Query(context.TODO(), &i.timeline.query, i.timeline.variables); err != nil { @@ -240,10 +247,6 @@ func (i *iterator) TimelineValue() timelineItem { return i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node } -func (i *iterator) timelineCursor() string { - return "" -} - func (i *iterator) NextIssueEdit() bool { if i.err != nil { return false @@ -359,11 +362,9 @@ func (i *iterator) NextCommentEdit() bool { return false } - // if there is more comment edits, query them - i.initCommentEditQueryVariables() if i.timeline.index == 0 { - i.commentEdit.variables["timelineAfter"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor + i.commentEdit.variables["timelineAfter"] = i.timeline.lastEndCursor } else { i.commentEdit.variables["timelineAfter"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index-1].Cursor } diff --git a/bridge/github/iterator_test.go b/bridge/github/iterator_test.go index c5820973..c5fad349 100644 --- a/bridge/github/iterator_test.go +++ b/bridge/github/iterator_test.go @@ -16,11 +16,12 @@ func Test_Iterator(t *testing.T) { keyToken: token, "user": user, "project": project, - }, time.Now().Add(-14*24*time.Hour)) + }, time.Time{}) + //time.Now().Add(-14*24*time.Hour)) for i.NextIssue() { v := i.IssueValue() - fmt.Printf("issue = id:%v title:%v\n", v.Id, v.Title) + fmt.Printf(" issue = id:%v title:%v\n", v.Id, v.Title) for i.NextIssueEdit() { v := i.IssueEditValue() @@ -33,12 +34,15 @@ func Test_Iterator(t *testing.T) { if v.Typename == "IssueComment" { for i.NextCommentEdit() { + _ = i.CommentEditValue() - //fmt.Printf("comment edit: %v\n", *v.Diff) fmt.Printf("comment edit\n") } } } } + + fmt.Println(i.Error()) + fmt.Println(i.Count()) } -- cgit From 0d976f66e87b7c053b10d50fe0849f6c8e5412e6 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Fri, 3 May 2019 00:02:50 +0200 Subject: Add importer tests Changes to Importer and exporter interface Improve importer Fix extra edits bug --- bridge/core/bridge.go | 47 +----- bridge/core/interfaces.go | 8 +- bridge/github/import.go | 376 +++++++++++++++++++++-------------------- bridge/github/import_test.go | 223 ++++++++++++++++++++++++ bridge/github/iterator.go | 25 +-- bridge/github/iterator_test.go | 48 ------ bridge/launchpad/import.go | 7 +- 7 files changed, 430 insertions(+), 304 deletions(-) create mode 100644 bridge/github/import_test.go delete mode 100644 bridge/github/iterator_test.go (limited to 'bridge') diff --git a/bridge/core/bridge.go b/bridge/core/bridge.go index b849bec6..aa02ceb5 100644 --- a/bridge/core/bridge.go +++ b/bridge/core/bridge.go @@ -6,6 +6,7 @@ import ( "reflect" "regexp" "strings" + "time" "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/repository" @@ -265,7 +266,7 @@ func (b *Bridge) ensureInit() error { return nil } -func (b *Bridge) ImportAll() error { +func (b *Bridge) ImportAll(since time.Time) error { importer := b.getImporter() if importer == nil { return ErrImportNotSupported @@ -281,48 +282,10 @@ func (b *Bridge) ImportAll() error { return err } - return importer.ImportAll(b.repo) + return importer.ImportAll(b.repo, since) } -func (b *Bridge) Import(id string) error { - importer := b.getImporter() - if importer == nil { - return ErrImportNotSupported - } - - err := b.ensureConfig() - if err != nil { - return err - } - - err = b.ensureInit() - if err != nil { - return err - } - - return importer.Import(b.repo, id) -} - -func (b *Bridge) ExportAll() error { - exporter := b.getExporter() - if exporter == nil { - return ErrExportNotSupported - } - - err := b.ensureConfig() - if err != nil { - return err - } - - err = b.ensureInit() - if err != nil { - return err - } - - return exporter.ExportAll(b.repo) -} - -func (b *Bridge) Export(id string) error { +func (b *Bridge) ExportAll(since time.Time) error { exporter := b.getExporter() if exporter == nil { return ErrExportNotSupported @@ -338,5 +301,5 @@ func (b *Bridge) Export(id string) error { return err } - return exporter.Export(b.repo, id) + return exporter.ExportAll(b.repo, since) } diff --git a/bridge/core/interfaces.go b/bridge/core/interfaces.go index 4836dab3..be5afa62 100644 --- a/bridge/core/interfaces.go +++ b/bridge/core/interfaces.go @@ -1,6 +1,8 @@ package core import ( + "time" + "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/repository" ) @@ -27,12 +29,10 @@ type BridgeImpl interface { type Importer interface { Init(conf Configuration) error - ImportAll(repo *cache.RepoCache) error - Import(repo *cache.RepoCache, id string) error + ImportAll(repo *cache.RepoCache, since time.Time) error } type Exporter interface { Init(conf Configuration) error - ExportAll(repo *cache.RepoCache) error - Export(repo *cache.RepoCache, id string) error + ExportAll(repo *cache.RepoCache, since time.Time) error } diff --git a/bridge/github/import.go b/bridge/github/import.go index 74ccb776..4960117a 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -27,237 +27,157 @@ type githubImporter struct { } func (gi *githubImporter) Init(conf core.Configuration) error { - var since time.Time - - // parse since value from configuration - if value, ok := conf["since"]; ok && value != "" { - s, err := time.Parse(time.RFC3339, value) - if err != nil { - return err - } - - since = s - } - - gi.iterator = newIterator(conf, since) + gi.conf = conf + gi.iterator = newIterator(conf) return nil } -func (gi *githubImporter) ImportAll(repo *cache.RepoCache) error { - // Loop over all available issues +// ImportAll . +func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) error { + gi.iterator.since = since + + // Loop over all matching issues for gi.iterator.NextIssue() { issue := gi.iterator.IssueValue() - fmt.Printf("importing issue: %v\n", issue.Title) - - // In each iteration create a new bug - var b *cache.BugCache - - // ensure issue author - author, err := gi.ensurePerson(repo, issue.Author) - if err != nil { - return err - } - - // resolve bug - b, err = repo.ResolveBugCreateMetadata(keyGithubId, parseId(issue.Id)) - if err != nil && err != bug.ErrBugNotExist { - return err - } + fmt.Printf("importing issue: %v\n", gi.iterator.count) // get issue edits issueEdits := []userContentEdit{} for gi.iterator.NextIssueEdit() { - // append only edits with non empty diff - if issueEdit := gi.iterator.IssueEditValue(); issueEdit.Diff != nil { + if issueEdit := gi.iterator.IssueEditValue(); issueEdit.Diff != nil && string(*issueEdit.Diff) != "" { issueEdits = append(issueEdits, issueEdit) } } - // if issueEdits is empty - if len(issueEdits) == 0 { - if err == bug.ErrBugNotExist { - // create bug - b, err = repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - issue.Title, - cleanupText(string(issue.Body)), - nil, - map[string]string{ - keyGithubId: parseId(issue.Id), - keyGithubUrl: issue.Url.String(), - }) - if err != nil { - return err - } - } - } else { - // create bug from given issueEdits - for _, edit := range issueEdits { - // if the bug doesn't exist - if b == nil { - // we create the bug as soon as we have a legit first edition - b, err = repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - issue.Title, - cleanupText(string(*edit.Diff)), - nil, - map[string]string{ - keyGithubId: parseId(issue.Id), - keyGithubUrl: issue.Url.String(), - }, - ) - - if err != nil { - return err - } - - continue - } - - // other edits will be added as CommentEdit operations - - target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(issue.Id)) - if err != nil { - return err - } - - err = gi.ensureCommentEdit(repo, b, target, edit) - if err != nil { - return err - } - } + // create issue + b, err := gi.ensureIssue(repo, issue, issueEdits) + if err != nil { + return fmt.Errorf("issue creation: %v", err) } - // check timeline items + // loop over timeline items for gi.iterator.NextTimeline() { item := gi.iterator.TimelineValue() - // if item is not a comment (label, unlabel, rename, close, open ...) - if item.Typename != "IssueComment" { - if err := gi.ensureTimelineItem(repo, b, item); err != nil { - return err - } - } else { // if item is comment - - // ensure person - author, err := gi.ensurePerson(repo, item.IssueComment.Author) - if err != nil { - return err - } - - var target git.Hash - target, err = b.ResolveOperationWithMetadata(keyGithubId, parseId(item.IssueComment.Id)) - if err != nil && err != cache.ErrNoMatchingOp { - // real error - return err - } - + // if item is comment + if item.Typename == "IssueComment" { // collect all edits commentEdits := []userContentEdit{} for gi.iterator.NextCommentEdit() { - if commentEdit := gi.iterator.CommentEditValue(); commentEdit.Diff != nil { + if commentEdit := gi.iterator.CommentEditValue(); commentEdit.Diff != nil && string(*commentEdit.Diff) != "" { commentEdits = append(commentEdits, commentEdit) } } - // if no edits are given we create the comment - if len(commentEdits) == 0 { - - // if comment doesn't exist - if err == cache.ErrNoMatchingOp { - - // add comment operation - op, err := b.AddCommentRaw( - author, - item.IssueComment.CreatedAt.Unix(), - cleanupText(string(item.IssueComment.Body)), - nil, - map[string]string{ - keyGithubId: parseId(item.IssueComment.Id), - }, - ) - if err != nil { - return err - } - - // set hash - target, err = op.Hash() - if err != nil { - return err - } - } - } else { - // if we have some edits - for _, edit := range item.IssueComment.UserContentEdits.Nodes { - - // create comment when target is an empty string - if target == "" { - op, err := b.AddCommentRaw( - author, - item.IssueComment.CreatedAt.Unix(), - cleanupText(string(*edit.Diff)), - nil, - map[string]string{ - keyGithubId: parseId(item.IssueComment.Id), - keyGithubUrl: item.IssueComment.Url.String(), - }, - ) - if err != nil { - return err - } - - // set hash - target, err = op.Hash() - if err != nil { - return err - } - } - - err := gi.ensureCommentEdit(repo, b, target, edit) - if err != nil { - return err - } - - } + err := gi.ensureTimelineComment(repo, b, item.IssueComment, commentEdits) + if err != nil { + return fmt.Errorf("timeline event creation: %v", err) } + } else { + if err := gi.ensureTimelineItem(repo, b, item); err != nil { + return fmt.Errorf("timeline comment creation: %v", err) + } } - - } - - if err := gi.iterator.Error(); err != nil { - fmt.Printf("error importing issue %v\n", issue.Id) - return err } // commit bug state - err = b.CommitAsNeeded() - if err != nil { - return err + if err := b.CommitAsNeeded(); err != nil { + return fmt.Errorf("bug commit: %v", err) } } if err := gi.iterator.Error(); err != nil { fmt.Printf("import error: %v\n", err) + return err } fmt.Printf("Successfully imported %v issues from Github\n", gi.iterator.Count()) return nil } -func (gi *githubImporter) Import(repo *cache.RepoCache, id string) error { - fmt.Println("IMPORT") - return nil +func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline, issueEdits []userContentEdit) (*cache.BugCache, error) { + // ensure issue author + author, err := gi.ensurePerson(repo, issue.Author) + if err != nil { + return nil, err + } + + // resolve bug + b, err := repo.ResolveBugCreateMetadata(keyGithubUrl, issue.Url.String()) + if err != nil && err != bug.ErrBugNotExist { + return nil, err + } + + // if issueEdits is empty + if len(issueEdits) == 0 { + if err == bug.ErrBugNotExist { + // create bug + b, err = repo.NewBugRaw( + author, + issue.CreatedAt.Unix(), + issue.Title, + cleanupText(string(issue.Body)), + nil, + map[string]string{ + keyGithubId: parseId(issue.Id), + keyGithubUrl: issue.Url.String(), + }) + if err != nil { + return nil, err + } + } + + } else { + // create bug from given issueEdits + for i, edit := range issueEdits { + if i == 0 && b != nil { + continue + } + + // if the bug doesn't exist + if b == nil { + // we create the bug as soon as we have a legit first edition + b, err = repo.NewBugRaw( + author, + issue.CreatedAt.Unix(), + issue.Title, + cleanupText(string(*edit.Diff)), + nil, + map[string]string{ + keyGithubId: parseId(issue.Id), + keyGithubUrl: issue.Url.String(), + }, + ) + + if err != nil { + return nil, err + } + + continue + } + + // other edits will be added as CommentEdit operations + target, err := b.ResolveOperationWithMetadata(keyGithubUrl, issue.Url.String()) + if err != nil { + return nil, err + } + + err = gi.ensureCommentEdit(repo, b, target, edit) + if err != nil { + return nil, err + } + } + } + + return b, nil } func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.BugCache, item timelineItem) error { - fmt.Printf("import item: %s\n", item.Typename) + fmt.Printf("import event item: %s\n", item.Typename) switch item.Typename { case "IssueComment": - //return gi.ensureComment(repo, b, cursor, item.IssueComment, rootVariables) case "LabeledEvent": id := parseId(item.LabeledEvent.Id) @@ -290,6 +210,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err != nil { return err } + _, _, err = b.ChangeLabelsRaw( author, item.UnlabeledEvent.CreatedAt.Unix(), @@ -360,6 +281,92 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug return nil } +func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.BugCache, item issueComment, edits []userContentEdit) error { + // ensure person + author, err := gi.ensurePerson(repo, item.Author) + if err != nil { + return err + } + + var target git.Hash + target, err = b.ResolveOperationWithMetadata(keyGithubId, parseId(item.Id)) + if err != nil && err != cache.ErrNoMatchingOp { + // real error + return err + } + // if no edits are given we create the comment + if len(edits) == 0 { + + // if comment doesn't exist + if err == cache.ErrNoMatchingOp { + + // add comment operation + op, err := b.AddCommentRaw( + author, + item.CreatedAt.Unix(), + cleanupText(string(item.Body)), + nil, + map[string]string{ + keyGithubId: parseId(item.Id), + keyGithubUrl: parseId(item.Url.String()), + }, + ) + if err != nil { + return err + } + + // set hash + target, err = op.Hash() + if err != nil { + return err + } + } + } else { + for i, edit := range item.UserContentEdits.Nodes { + if i == 0 && target != "" { + continue + } + + // ensure editor identity + editor, err := gi.ensurePerson(repo, edit.Editor) + if err != nil { + return err + } + + // create comment when target is empty + if target == "" { + op, err := b.AddCommentRaw( + editor, + edit.CreatedAt.Unix(), + cleanupText(string(*edit.Diff)), + nil, + map[string]string{ + keyGithubId: parseId(item.Id), + keyGithubUrl: item.Url.String(), + }, + ) + if err != nil { + return err + } + + // set hash + target, err = op.Hash() + if err != nil { + return err + } + + continue + } + + err = gi.ensureCommentEdit(repo, b, target, edit) + if err != nil { + return err + } + } + } + return nil +} + func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugCache, target git.Hash, edit userContentEdit) error { _, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(edit.Id)) if err == nil { @@ -381,8 +388,10 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC switch { case edit.DeletedAt != nil: // comment deletion, not supported yet + fmt.Println("comment deletion ....") case edit.DeletedAt == nil: + // comment edition _, err := b.EditCommentRaw( editor, @@ -393,6 +402,7 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC keyGithubId: parseId(edit.Id), }, ) + if err != nil { return err } diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go new file mode 100644 index 00000000..d64f0b4b --- /dev/null +++ b/bridge/github/import_test.go @@ -0,0 +1,223 @@ +package github + +import ( + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/MichaelMure/git-bug/bridge/core" + "github.com/MichaelMure/git-bug/bug" + "github.com/MichaelMure/git-bug/cache" + "github.com/MichaelMure/git-bug/identity" + "github.com/MichaelMure/git-bug/repository" + "github.com/MichaelMure/git-bug/util/interrupt" +) + +func Test_Importer(t *testing.T) { + author := identity.NewIdentity("Michael Muré", "batolettre@gmail.com") + tests := []struct { + name string + exist bool + url string + bug *bug.Snapshot + }{ + { + name: "simple issue", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/1", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "simple issue", "initial comment", nil), + bug.NewAddCommentOp(author, 0, "first comment", nil), + bug.NewAddCommentOp(author, 0, "second comment", nil)}, + }, + }, + { + name: "empty issue", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/2", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "empty issue", "", nil), + }, + }, + }, + { + name: "complex issue", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/3", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "complex issue", "initial comment", nil), + bug.NewLabelChangeOperation(author, 0, []bug.Label{"bug"}, []bug.Label{}), + bug.NewLabelChangeOperation(author, 0, []bug.Label{"duplicate"}, []bug.Label{}), + bug.NewLabelChangeOperation(author, 0, []bug.Label{}, []bug.Label{"duplicate"}), + bug.NewAddCommentOp(author, 0, "### header\n\n**bold**\n\n_italic_\n\n> with quote\n\n`inline code`\n\n```\nmultiline code\n```\n\n- bulleted\n- list\n\n1. numbered\n1. list\n\n- [ ] task\n- [x] list\n\n@MichaelMure mention\n\n#2 reference issue\n#3 auto-reference issue\n\n![image](https://user-images.githubusercontent.com/294669/56870222-811faf80-6a0c-11e9-8f2c-f0beb686303f.png)", nil), + bug.NewSetTitleOp(author, 0, "complex issue edited", "complex issue"), + bug.NewSetTitleOp(author, 0, "complex issue", "complex issue edited"), + bug.NewSetStatusOp(author, 0, bug.ClosedStatus), + bug.NewSetStatusOp(author, 0, bug.OpenStatus), + }, + }, + }, + { + name: "editions", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/4", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "editions", "initial comment edited", nil), + bug.NewEditCommentOp(author, 0, "", "erased then edited again", nil), + bug.NewAddCommentOp(author, 0, "first comment", nil), + bug.NewEditCommentOp(author, 0, "", "first comment edited", nil), + }, + }, + }, + { + name: "comment deletion", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/5", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "comment deletion", "", nil), + }, + }, + }, + { + name: "edition deletion", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/6", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "edition deletion", "initial comment", nil), + bug.NewEditCommentOp(author, 0, "", "initial comment edited again", nil), + bug.NewAddCommentOp(author, 0, "first comment", nil), + bug.NewEditCommentOp(author, 0, "", "first comment edited again", nil), + }, + }, + }, + { + name: "hidden comment", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/7", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "hidden comment", "initial comment", nil), + bug.NewAddCommentOp(author, 0, "first comment", nil), + }, + }, + }, + { + name: "transfered issue", + exist: true, + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/8", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "transfered issue", "", nil), + }, + }, + }, + } + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + + repo, err := repository.NewGitRepo(cwd, bug.Witnesser) + if err != nil { + t.Fatal(err) + } + + backend, err := cache.NewRepoCache(repo) + if err != nil { + t.Fatal(err) + } + + defer backend.Close() + interrupt.RegisterCleaner(backend.Close) + + importer := &githubImporter{} + err = importer.Init(core.Configuration{ + "user": "MichaelMure", + "project": "git-but-test-github-bridge", + "token": os.Getenv("GITHUB_TOKEN"), + }) + if err != nil { + t.Fatal(err) + } + + err = importer.ImportAll(backend, time.Time{}) + if err != nil { + t.Fatal(err) + } + + ids := backend.AllBugsIds() + assert.Equal(t, len(ids), 8) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := backend.ResolveBugCreateMetadata(keyGithubUrl, tt.url) + if err != nil { + t.Fatal(err) + } + + ops := b.Snapshot().Operations + if tt.exist { + assert.Equal(t, len(tt.bug.Operations), len(b.Snapshot().Operations)) + + for i, op := range tt.bug.Operations { + switch op.(type) { + case *bug.CreateOperation: + if op2, ok := ops[i].(*bug.CreateOperation); ok { + assert.Equal(t, op2.Title, op.(*bug.CreateOperation).Title) + assert.Equal(t, op2.Message, op.(*bug.CreateOperation).Message) + continue + } + t.Errorf("bad operation type index = %d expected = CreationOperation", i) + case *bug.SetStatusOperation: + if op2, ok := ops[i].(*bug.SetStatusOperation); ok { + assert.Equal(t, op2.Status, op.(*bug.SetStatusOperation).Status) + continue + } + t.Errorf("bad operation type index = %d expected = SetStatusOperation", i) + case *bug.SetTitleOperation: + if op2, ok := ops[i].(*bug.SetTitleOperation); ok { + assert.Equal(t, op.(*bug.SetTitleOperation).Was, op2.Was) + assert.Equal(t, op.(*bug.SetTitleOperation).Title, op2.Title) + continue + } + t.Errorf("bad operation type index = %d expected = SetTitleOperation", i) + case *bug.LabelChangeOperation: + if op2, ok := ops[i].(*bug.LabelChangeOperation); ok { + assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Added, op2.Added) + assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Removed, op2.Removed) + continue + } + t.Errorf("bad operation type index = %d expected = ChangeLabelOperation", i) + case *bug.AddCommentOperation: + if op2, ok := ops[i].(*bug.AddCommentOperation); ok { + assert.Equal(t, op.(*bug.AddCommentOperation).Message, op2.Message) + continue + } + t.Errorf("bad operation type index = %d expected = AddCommentOperation", i) + case *bug.EditCommentOperation: + if op2, ok := ops[i].(*bug.EditCommentOperation); ok { + assert.Equal(t, op.(*bug.EditCommentOperation).Message, op2.Message) + continue + } + t.Errorf("bad operation type index = %d expected = EditCommentOperation", i) + default: + + } + } + + } else { + assert.Equal(t, b, nil) + } + }) + } + +} diff --git a/bridge/github/iterator.go b/bridge/github/iterator.go index 9e1ff30e..281f8a6b 100644 --- a/bridge/github/iterator.go +++ b/bridge/github/iterator.go @@ -8,23 +8,6 @@ import ( "github.com/shurcooL/githubv4" ) -/** -type iterator interface { - Count() int - Error() error - - NextIssue() bool - NextIssueEdit() bool - NextTimeline() bool - NextCommentEdit() bool - - IssueValue() issueTimeline - IssueEditValue() userContentEdit - TimelineValue() timelineItem - CommentEditValue() userContentEdit -} -*/ - type indexer struct{ index int } type issueEditIterator struct { @@ -47,7 +30,8 @@ type timelineIterator struct { issueEdit indexer commentEdit indexer - lastEndCursor githubv4.String // storing timeline end cursor for future use + // lastEndCursor cache the timeline end cursor for one iteration + lastEndCursor githubv4.String } type iterator struct { @@ -59,7 +43,7 @@ type iterator struct { since time.Time // number of timelines/userEditcontent/issueEdit to query - // at a time more capacity = more used memory = less queries + // at a time, more capacity = more used memory = less queries // to make capacity int @@ -79,9 +63,8 @@ type iterator struct { commentEdit commentEditIterator } -func newIterator(conf core.Configuration, since time.Time) *iterator { +func newIterator(conf core.Configuration) *iterator { return &iterator{ - since: since, gc: buildClient(conf), capacity: 10, count: 0, diff --git a/bridge/github/iterator_test.go b/bridge/github/iterator_test.go deleted file mode 100644 index c5fad349..00000000 --- a/bridge/github/iterator_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package github - -import ( - "fmt" - "os" - "testing" - "time" -) - -func Test_Iterator(t *testing.T) { - token := os.Getenv("GITHUB_TOKEN") - user := os.Getenv("GITHUB_USER") - project := os.Getenv("GITHUB_PROJECT") - - i := newIterator(map[string]string{ - keyToken: token, - "user": user, - "project": project, - }, time.Time{}) - //time.Now().Add(-14*24*time.Hour)) - - for i.NextIssue() { - v := i.IssueValue() - fmt.Printf(" issue = id:%v title:%v\n", v.Id, v.Title) - - for i.NextIssueEdit() { - v := i.IssueEditValue() - fmt.Printf("issue edit = %v\n", string(*v.Diff)) - } - - for i.NextTimeline() { - v := i.TimelineValue() - fmt.Printf("timeline = type:%v\n", v.Typename) - - if v.Typename == "IssueComment" { - for i.NextCommentEdit() { - - _ = i.CommentEditValue() - - fmt.Printf("comment edit\n") - } - } - } - } - - fmt.Println(i.Error()) - fmt.Println(i.Count()) -} diff --git a/bridge/launchpad/import.go b/bridge/launchpad/import.go index 30ec5c3f..177ff3fc 100644 --- a/bridge/launchpad/import.go +++ b/bridge/launchpad/import.go @@ -44,7 +44,7 @@ func (li *launchpadImporter) ensurePerson(repo *cache.RepoCache, owner LPPerson) ) } -func (li *launchpadImporter) ImportAll(repo *cache.RepoCache) error { +func (li *launchpadImporter) ImportAll(repo *cache.RepoCache, since time.Time) error { lpAPI := new(launchpadAPI) err := lpAPI.Init() @@ -139,8 +139,3 @@ func (li *launchpadImporter) ImportAll(repo *cache.RepoCache) error { } return nil } - -func (li *launchpadImporter) Import(repo *cache.RepoCache, id string) error { - fmt.Println("IMPORT") - return nil -} -- cgit From f7ea3421caa2c8957a82454255c4fdd699b70a9c Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sat, 4 May 2019 13:19:56 +0200 Subject: Add ForceLabelChange functionalities --- bridge/github/import.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'bridge') diff --git a/bridge/github/import.go b/bridge/github/import.go index 4960117a..5f99b5ea 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -40,7 +40,7 @@ func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro for gi.iterator.NextIssue() { issue := gi.iterator.IssueValue() - fmt.Printf("importing issue: %v\n", gi.iterator.count) + fmt.Printf("importing issue: %v %v\n", gi.iterator.count, issue.Title) // get issue edits issueEdits := []userContentEdit{} for gi.iterator.NextIssueEdit() { @@ -71,12 +71,12 @@ func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro err := gi.ensureTimelineComment(repo, b, item.IssueComment, commentEdits) if err != nil { - return fmt.Errorf("timeline event creation: %v", err) + return fmt.Errorf("timeline comment creation: %v", err) } } else { if err := gi.ensureTimelineItem(repo, b, item); err != nil { - return fmt.Errorf("timeline comment creation: %v", err) + return fmt.Errorf("timeline event creation: %v", err) } } } @@ -189,7 +189,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err != nil { return err } - _, _, err = b.ChangeLabelsRaw( + _, err = b.ForceChangeLabelsRaw( author, item.LabeledEvent.CreatedAt.Unix(), []string{ @@ -198,6 +198,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug nil, map[string]string{keyGithubId: id}, ) + return err case "UnlabeledEvent": @@ -211,7 +212,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug return err } - _, _, err = b.ChangeLabelsRaw( + _, err = b.ForceChangeLabelsRaw( author, item.UnlabeledEvent.CreatedAt.Unix(), nil, -- cgit From 390ca867244004af56f05c19d26a4e9aeb20ae6c Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sat, 4 May 2019 17:38:03 +0200 Subject: Improve naming and functions signatures expose `NewIterator` in `github` package remove `exist` in tests cases skip tests when env var GITHUB_TOKEN is not given --- bridge/github/github.go | 4 +- bridge/github/import.go | 34 +++++----- bridge/github/import_test.go | 148 +++++++++++++++++++---------------------- bridge/github/iterator.go | 154 +++++++++++++++++++++++-------------------- 4 files changed, 169 insertions(+), 171 deletions(-) (limited to 'bridge') diff --git a/bridge/github/github.go b/bridge/github/github.go index b3f8d763..5fee7487 100644 --- a/bridge/github/github.go +++ b/bridge/github/github.go @@ -27,9 +27,9 @@ func (*Github) NewExporter() core.Exporter { return nil } -func buildClient(conf core.Configuration) *githubv4.Client { +func buildClient(token string) *githubv4.Client { src := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: conf[keyToken]}, + &oauth2.Token{AccessToken: token}, ) httpClient := oauth2.NewClient(context.TODO(), src) diff --git a/bridge/github/import.go b/bridge/github/import.go index 5f99b5ea..ad9d5046 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -22,29 +22,27 @@ const ( // githubImporter implement the Importer interface type githubImporter struct { - iterator *iterator - conf core.Configuration + conf core.Configuration } func (gi *githubImporter) Init(conf core.Configuration) error { gi.conf = conf - gi.iterator = newIterator(conf) return nil } // ImportAll . func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) error { - gi.iterator.since = since + iterator := NewIterator(gi.conf[keyUser], gi.conf[keyProject], gi.conf[keyToken], since) // Loop over all matching issues - for gi.iterator.NextIssue() { - issue := gi.iterator.IssueValue() + for iterator.NextIssue() { + issue := iterator.IssueValue() - fmt.Printf("importing issue: %v %v\n", gi.iterator.count, issue.Title) + fmt.Printf("importing issue: %v %v\n", iterator.importedIssues, issue.Title) // get issue edits issueEdits := []userContentEdit{} - for gi.iterator.NextIssueEdit() { - if issueEdit := gi.iterator.IssueEditValue(); issueEdit.Diff != nil && string(*issueEdit.Diff) != "" { + for iterator.NextIssueEdit() { + if issueEdit := iterator.IssueEditValue(); issueEdit.Diff != nil && string(*issueEdit.Diff) != "" { issueEdits = append(issueEdits, issueEdit) } } @@ -56,15 +54,15 @@ func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro } // loop over timeline items - for gi.iterator.NextTimeline() { - item := gi.iterator.TimelineValue() + for iterator.NextTimeline() { + item := iterator.TimelineValue() // if item is comment if item.Typename == "IssueComment" { // collect all edits commentEdits := []userContentEdit{} - for gi.iterator.NextCommentEdit() { - if commentEdit := gi.iterator.CommentEditValue(); commentEdit.Diff != nil && string(*commentEdit.Diff) != "" { + for iterator.NextCommentEdit() { + if commentEdit := iterator.CommentEditValue(); commentEdit.Diff != nil && string(*commentEdit.Diff) != "" { commentEdits = append(commentEdits, commentEdit) } } @@ -87,12 +85,12 @@ func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro } } - if err := gi.iterator.Error(); err != nil { + if err := iterator.Error(); err != nil { fmt.Printf("import error: %v\n", err) return err } - fmt.Printf("Successfully imported %v issues from Github\n", gi.iterator.Count()) + fmt.Printf("Successfully imported %v issues from Github\n", iterator.ImportedIssues()) return nil } @@ -389,7 +387,7 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC switch { case edit.DeletedAt != nil: // comment deletion, not supported yet - fmt.Println("comment deletion ....") + fmt.Println("comment deletion is not supported yet") case edit.DeletedAt == nil: @@ -475,7 +473,9 @@ func (gi *githubImporter) getGhost(repo *cache.RepoCache) (*cache.IdentityCache, "login": githubv4.String("ghost"), } - err = gi.iterator.gc.Query(context.TODO(), &q, variables) + gc := buildClient(gi.conf[keyToken]) + + err = gc.Query(context.TODO(), &q, variables) if err != nil { return nil, err } diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go index d64f0b4b..79af8450 100644 --- a/bridge/github/import_test.go +++ b/bridge/github/import_test.go @@ -18,15 +18,13 @@ import ( func Test_Importer(t *testing.T) { author := identity.NewIdentity("Michael Muré", "batolettre@gmail.com") tests := []struct { - name string - exist bool - url string - bug *bug.Snapshot + name string + url string + bug *bug.Snapshot }{ { - name: "simple issue", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/1", + name: "simple issue", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/1", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "simple issue", "initial comment", nil), @@ -35,9 +33,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "empty issue", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/2", + name: "empty issue", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/2", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "empty issue", "", nil), @@ -45,9 +42,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "complex issue", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/3", + name: "complex issue", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/3", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "complex issue", "initial comment", nil), @@ -63,9 +59,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "editions", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/4", + name: "editions", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/4", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "editions", "initial comment edited", nil), @@ -76,9 +71,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "comment deletion", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/5", + name: "comment deletion", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/5", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "comment deletion", "", nil), @@ -86,9 +80,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "edition deletion", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/6", + name: "edition deletion", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/6", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "edition deletion", "initial comment", nil), @@ -99,9 +92,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "hidden comment", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/7", + name: "hidden comment", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/7", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "hidden comment", "initial comment", nil), @@ -110,9 +102,8 @@ func Test_Importer(t *testing.T) { }, }, { - name: "transfered issue", - exist: true, - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/8", + name: "transfered issue", + url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/8", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "transfered issue", "", nil), @@ -139,11 +130,16 @@ func Test_Importer(t *testing.T) { defer backend.Close() interrupt.RegisterCleaner(backend.Close) + token := os.Getenv("GITHUB_TOKEN") + if token == "" { + t.Skip("Env var GITHUB_TOKEN missing") + } + importer := &githubImporter{} err = importer.Init(core.Configuration{ "user": "MichaelMure", "project": "git-but-test-github-bridge", - "token": os.Getenv("GITHUB_TOKEN"), + "token": token, }) if err != nil { t.Fatal(err) @@ -165,59 +161,53 @@ func Test_Importer(t *testing.T) { } ops := b.Snapshot().Operations - if tt.exist { - assert.Equal(t, len(tt.bug.Operations), len(b.Snapshot().Operations)) - - for i, op := range tt.bug.Operations { - switch op.(type) { - case *bug.CreateOperation: - if op2, ok := ops[i].(*bug.CreateOperation); ok { - assert.Equal(t, op2.Title, op.(*bug.CreateOperation).Title) - assert.Equal(t, op2.Message, op.(*bug.CreateOperation).Message) - continue - } - t.Errorf("bad operation type index = %d expected = CreationOperation", i) - case *bug.SetStatusOperation: - if op2, ok := ops[i].(*bug.SetStatusOperation); ok { - assert.Equal(t, op2.Status, op.(*bug.SetStatusOperation).Status) - continue - } - t.Errorf("bad operation type index = %d expected = SetStatusOperation", i) - case *bug.SetTitleOperation: - if op2, ok := ops[i].(*bug.SetTitleOperation); ok { - assert.Equal(t, op.(*bug.SetTitleOperation).Was, op2.Was) - assert.Equal(t, op.(*bug.SetTitleOperation).Title, op2.Title) - continue - } - t.Errorf("bad operation type index = %d expected = SetTitleOperation", i) - case *bug.LabelChangeOperation: - if op2, ok := ops[i].(*bug.LabelChangeOperation); ok { - assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Added, op2.Added) - assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Removed, op2.Removed) - continue - } - t.Errorf("bad operation type index = %d expected = ChangeLabelOperation", i) - case *bug.AddCommentOperation: - if op2, ok := ops[i].(*bug.AddCommentOperation); ok { - assert.Equal(t, op.(*bug.AddCommentOperation).Message, op2.Message) - continue - } - t.Errorf("bad operation type index = %d expected = AddCommentOperation", i) - case *bug.EditCommentOperation: - if op2, ok := ops[i].(*bug.EditCommentOperation); ok { - assert.Equal(t, op.(*bug.EditCommentOperation).Message, op2.Message) - continue - } - t.Errorf("bad operation type index = %d expected = EditCommentOperation", i) - default: - + assert.Equal(t, len(tt.bug.Operations), len(b.Snapshot().Operations)) + + for i, op := range tt.bug.Operations { + switch op.(type) { + case *bug.CreateOperation: + if op2, ok := ops[i].(*bug.CreateOperation); ok { + assert.Equal(t, op2.Title, op.(*bug.CreateOperation).Title) + assert.Equal(t, op2.Message, op.(*bug.CreateOperation).Message) + continue + } + t.Errorf("bad operation type index = %d expected = CreationOperation", i) + case *bug.SetStatusOperation: + if op2, ok := ops[i].(*bug.SetStatusOperation); ok { + assert.Equal(t, op2.Status, op.(*bug.SetStatusOperation).Status) + continue + } + t.Errorf("bad operation type index = %d expected = SetStatusOperation", i) + case *bug.SetTitleOperation: + if op2, ok := ops[i].(*bug.SetTitleOperation); ok { + assert.Equal(t, op.(*bug.SetTitleOperation).Was, op2.Was) + assert.Equal(t, op.(*bug.SetTitleOperation).Title, op2.Title) + continue } + t.Errorf("bad operation type index = %d expected = SetTitleOperation", i) + case *bug.LabelChangeOperation: + if op2, ok := ops[i].(*bug.LabelChangeOperation); ok { + assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Added, op2.Added) + assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Removed, op2.Removed) + continue + } + t.Errorf("bad operation type index = %d expected = ChangeLabelOperation", i) + case *bug.AddCommentOperation: + if op2, ok := ops[i].(*bug.AddCommentOperation); ok { + assert.Equal(t, op.(*bug.AddCommentOperation).Message, op2.Message) + continue + } + t.Errorf("bad operation type index = %d expected = AddCommentOperation", i) + case *bug.EditCommentOperation: + if op2, ok := ops[i].(*bug.EditCommentOperation); ok { + assert.Equal(t, op.(*bug.EditCommentOperation).Message, op2.Message) + continue + } + t.Errorf("bad operation type index = %d expected = EditCommentOperation", i) + default: + panic("Unknown operation type") } - - } else { - assert.Equal(t, b, nil) } }) } - } diff --git a/bridge/github/iterator.go b/bridge/github/iterator.go index 281f8a6b..239b49bd 100644 --- a/bridge/github/iterator.go +++ b/bridge/github/iterator.go @@ -4,7 +4,6 @@ import ( "context" "time" - "github.com/MichaelMure/git-bug/bridge/core" "github.com/shurcooL/githubv4" ) @@ -50,8 +49,8 @@ type iterator struct { // sticky error err error - // count to keep track of the number of imported issues - count int + // number of imported issues + importedIssues int // timeline iterator timeline timelineIterator @@ -63,32 +62,32 @@ type iterator struct { commentEdit commentEditIterator } -func newIterator(conf core.Configuration) *iterator { +func NewIterator(user, project, token string, since time.Time) *iterator { return &iterator{ - gc: buildClient(conf), + gc: buildClient(token), + since: since, capacity: 10, - count: 0, timeline: timelineIterator{ index: -1, issueEdit: indexer{-1}, commentEdit: indexer{-1}, variables: map[string]interface{}{ - "owner": githubv4.String(conf["user"]), - "name": githubv4.String(conf["project"]), + "owner": githubv4.String(user), + "name": githubv4.String(project), }, }, commentEdit: commentEditIterator{ index: -1, variables: map[string]interface{}{ - "owner": githubv4.String(conf["user"]), - "name": githubv4.String(conf["project"]), + "owner": githubv4.String(user), + "name": githubv4.String(project), }, }, issueEdit: issueEditIterator{ index: -1, variables: map[string]interface{}{ - "owner": githubv4.String(conf["user"]), - "name": githubv4.String(conf["project"]), + "owner": githubv4.String(user), + "name": githubv4.String(project), }, }, } @@ -130,10 +129,11 @@ func (i *iterator) initCommentEditQueryVariables() { // reverse UserContentEdits arrays in both of the issue and // comment timelines func (i *iterator) reverseTimelineEditNodes() { - reverseEdits(i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) - for index, ce := range i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges { - if ce.Node.Typename == "IssueComment" && len(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges) != 0 { - reverseEdits(i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[index].Node.IssueComment.UserContentEdits.Nodes) + node := i.timeline.query.Repository.Issues.Nodes[0] + reverseEdits(node.UserContentEdits.Nodes) + for index, ce := range node.Timeline.Edges { + if ce.Node.Typename == "IssueComment" && len(node.Timeline.Edges) != 0 { + reverseEdits(node.Timeline.Edges[index].Node.IssueComment.UserContentEdits.Nodes) } } } @@ -143,19 +143,34 @@ func (i *iterator) Error() error { return i.err } -// Count return number of issues we iterated over -func (i *iterator) Count() int { - return i.count +// ImportedIssues return the number of issues we iterated over +func (i *iterator) ImportedIssues() int { + return i.importedIssues +} + +func (i *iterator) queryIssue() bool { + if err := i.gc.Query(context.TODO(), &i.timeline.query, i.timeline.variables); err != nil { + i.err = err + return false + } + + if len(i.timeline.query.Repository.Issues.Nodes) == 0 { + return false + } + + i.reverseTimelineEditNodes() + i.importedIssues++ + return true } // Next issue func (i *iterator) NextIssue() bool { // we make the first move - if i.count == 0 { + if i.importedIssues == 0 { // init variables and goto queryIssue block i.initTimelineQueryVariables() - goto queryIssue + return i.queryIssue() } if i.err != nil { @@ -175,19 +190,7 @@ func (i *iterator) NextIssue() bool { i.timeline.lastEndCursor = i.timeline.query.Repository.Issues.Nodes[0].Timeline.PageInfo.EndCursor // query issue block -queryIssue: - if err := i.gc.Query(context.TODO(), &i.timeline.query, i.timeline.variables); err != nil { - i.err = err - return false - } - - if len(i.timeline.query.Repository.Issues.Nodes) == 0 { - return false - } - - i.reverseTimelineEditNodes() - i.count++ - return true + return i.queryIssue() } func (i *iterator) IssueValue() issueTimeline { @@ -230,6 +233,27 @@ func (i *iterator) TimelineValue() timelineItem { return i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node } +func (i *iterator) queryIssueEdit() bool { + if err := i.gc.Query(context.TODO(), &i.issueEdit.query, i.issueEdit.variables); err != nil { + i.err = err + //i.timeline.issueEdit.index = -1 + return false + } + + // reverse issue edits because github + reverseEdits(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) + + // this is not supposed to happen + if len(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) == 0 { + i.timeline.issueEdit.index = -1 + return false + } + + i.issueEdit.index = 0 + i.timeline.issueEdit.index = -2 + return true +} + func (i *iterator) NextIssueEdit() bool { if i.err != nil { return false @@ -251,7 +275,7 @@ func (i *iterator) NextIssueEdit() bool { // if there is more edits, query them i.issueEdit.variables["issueEditBefore"] = i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.PageInfo.StartCursor - goto queryIssueEdit + return i.queryIssueEdit() } // if there is no edits @@ -273,26 +297,7 @@ func (i *iterator) NextIssueEdit() bool { // if there is more edits, query them i.initIssueEditQueryVariables() i.issueEdit.variables["issueEditBefore"] = i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.PageInfo.StartCursor - -queryIssueEdit: - if err := i.gc.Query(context.TODO(), &i.issueEdit.query, i.issueEdit.variables); err != nil { - i.err = err - //i.timeline.issueEdit.index = -1 - return false - } - - // reverse issue edits because github - reverseEdits(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) - - // this is not supposed to happen - if len(i.issueEdit.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) == 0 { - i.timeline.issueEdit.index = -1 - return false - } - - i.issueEdit.index = 0 - i.timeline.issueEdit.index = -2 - return true + return i.queryIssueEdit() } func (i *iterator) IssueEditValue() userContentEdit { @@ -305,6 +310,25 @@ func (i *iterator) IssueEditValue() userContentEdit { return i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes[i.timeline.issueEdit.index] } +func (i *iterator) queryCommentEdit() bool { + if err := i.gc.Query(context.TODO(), &i.commentEdit.query, i.commentEdit.variables); err != nil { + i.err = err + return false + } + + // this is not supposed to happen + if len(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes) == 0 { + i.timeline.commentEdit.index = -1 + return false + } + + reverseEdits(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes) + + i.commentEdit.index = 0 + i.timeline.commentEdit.index = -2 + return true +} + func (i *iterator) NextCommentEdit() bool { if i.err != nil { return false @@ -326,7 +350,7 @@ func (i *iterator) NextCommentEdit() bool { // if there is more comment edits, query them i.commentEdit.variables["commentEditBefore"] = i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.PageInfo.StartCursor - goto queryCommentEdit + return i.queryCommentEdit() } // if there is no comment edits @@ -354,23 +378,7 @@ func (i *iterator) NextCommentEdit() bool { i.commentEdit.variables["commentEditBefore"] = i.timeline.query.Repository.Issues.Nodes[0].Timeline.Edges[i.timeline.index].Node.IssueComment.UserContentEdits.PageInfo.StartCursor -queryCommentEdit: - if err := i.gc.Query(context.TODO(), &i.commentEdit.query, i.commentEdit.variables); err != nil { - i.err = err - return false - } - - // this is not supposed to happen - if len(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes) == 0 { - i.timeline.commentEdit.index = -1 - return false - } - - reverseEdits(i.commentEdit.query.Repository.Issues.Nodes[0].Timeline.Nodes[0].IssueComment.UserContentEdits.Nodes) - - i.commentEdit.index = 0 - i.timeline.commentEdit.index = -2 - return true + return i.queryCommentEdit() } func (i *iterator) CommentEditValue() userContentEdit { -- cgit From eec17050f1fbc8565dcd117329b24c201ac476b1 Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sat, 4 May 2019 21:45:18 +0200 Subject: github: simplify and improve the import test --- bridge/github/import_test.go | 83 +++++++++++++------------------------------- 1 file changed, 25 insertions(+), 58 deletions(-) (limited to 'bridge') diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go index 79af8450..cd67c99b 100644 --- a/bridge/github/import_test.go +++ b/bridge/github/import_test.go @@ -1,18 +1,20 @@ package github import ( + "fmt" "os" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/MichaelMure/git-bug/bridge/core" "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/identity" - "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/interrupt" + "github.com/MichaelMure/git-bug/util/test" ) func Test_Importer(t *testing.T) { @@ -112,20 +114,10 @@ func Test_Importer(t *testing.T) { }, } - cwd, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - - repo, err := repository.NewGitRepo(cwd, bug.Witnesser) - if err != nil { - t.Fatal(err) - } + repo := test.CreateRepo(false) backend, err := cache.NewRepoCache(repo) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer backend.Close() interrupt.RegisterCleaner(backend.Close) @@ -141,69 +133,44 @@ func Test_Importer(t *testing.T) { "project": "git-but-test-github-bridge", "token": token, }) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) + + start := time.Now() err = importer.ImportAll(backend, time.Time{}) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) + + fmt.Printf("test repository imported in %f seconds\n", time.Since(start).Seconds()) - ids := backend.AllBugsIds() - assert.Equal(t, len(ids), 8) + require.Len(t, backend.AllBugsIds(), 8) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b, err := backend.ResolveBugCreateMetadata(keyGithubUrl, tt.url) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ops := b.Snapshot().Operations - assert.Equal(t, len(tt.bug.Operations), len(b.Snapshot().Operations)) + assert.Len(t, tt.bug.Operations, len(b.Snapshot().Operations)) for i, op := range tt.bug.Operations { + assert.IsType(t, ops[i], op) + switch op.(type) { case *bug.CreateOperation: - if op2, ok := ops[i].(*bug.CreateOperation); ok { - assert.Equal(t, op2.Title, op.(*bug.CreateOperation).Title) - assert.Equal(t, op2.Message, op.(*bug.CreateOperation).Message) - continue - } - t.Errorf("bad operation type index = %d expected = CreationOperation", i) + assert.Equal(t, ops[i].(*bug.CreateOperation).Title, op.(*bug.CreateOperation).Title) + assert.Equal(t, ops[i].(*bug.CreateOperation).Message, op.(*bug.CreateOperation).Message) case *bug.SetStatusOperation: - if op2, ok := ops[i].(*bug.SetStatusOperation); ok { - assert.Equal(t, op2.Status, op.(*bug.SetStatusOperation).Status) - continue - } - t.Errorf("bad operation type index = %d expected = SetStatusOperation", i) + assert.Equal(t, ops[i].(*bug.SetStatusOperation).Status, op.(*bug.SetStatusOperation).Status) case *bug.SetTitleOperation: - if op2, ok := ops[i].(*bug.SetTitleOperation); ok { - assert.Equal(t, op.(*bug.SetTitleOperation).Was, op2.Was) - assert.Equal(t, op.(*bug.SetTitleOperation).Title, op2.Title) - continue - } - t.Errorf("bad operation type index = %d expected = SetTitleOperation", i) + assert.Equal(t, ops[i].(*bug.SetTitleOperation).Was, op.(*bug.SetTitleOperation).Was) + assert.Equal(t, ops[i].(*bug.SetTitleOperation).Title, op.(*bug.SetTitleOperation).Title) case *bug.LabelChangeOperation: - if op2, ok := ops[i].(*bug.LabelChangeOperation); ok { - assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Added, op2.Added) - assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Removed, op2.Removed) - continue - } - t.Errorf("bad operation type index = %d expected = ChangeLabelOperation", i) + assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Added, op.(*bug.LabelChangeOperation).Added) + assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Removed, op.(*bug.LabelChangeOperation).Removed) case *bug.AddCommentOperation: - if op2, ok := ops[i].(*bug.AddCommentOperation); ok { - assert.Equal(t, op.(*bug.AddCommentOperation).Message, op2.Message) - continue - } - t.Errorf("bad operation type index = %d expected = AddCommentOperation", i) + assert.Equal(t, ops[i].(*bug.AddCommentOperation).Message, op.(*bug.AddCommentOperation).Message) case *bug.EditCommentOperation: - if op2, ok := ops[i].(*bug.EditCommentOperation); ok { - assert.Equal(t, op.(*bug.EditCommentOperation).Message, op2.Message) - continue - } - t.Errorf("bad operation type index = %d expected = EditCommentOperation", i) + assert.Equal(t, ops[i].(*bug.EditCommentOperation).Message, op.(*bug.EditCommentOperation).Message) default: panic("Unknown operation type") } -- cgit From 7d0296337287ed3d5a97f15c64d51d24340f2567 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sat, 4 May 2019 22:26:54 +0200 Subject: Add old importer comments in the iterator Test operation authors Fix typo in test repo url --- bridge/github/import.go | 4 ++++ bridge/github/import_test.go | 27 +++++++++++++++++---------- bridge/github/iterator.go | 13 ++++++++++++- 3 files changed, 33 insertions(+), 11 deletions(-) (limited to 'bridge') diff --git a/bridge/github/import.go b/bridge/github/import.go index ad9d5046..e72a2a45 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -42,6 +42,8 @@ func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro // get issue edits issueEdits := []userContentEdit{} for iterator.NextIssueEdit() { + // issueEdit.Diff == nil happen if the event is older than early 2018, Github doesn't have the data before that. + // Best we can do is to ignore the event. if issueEdit := iterator.IssueEditValue(); issueEdit.Diff != nil && string(*issueEdit.Diff) != "" { issueEdits = append(issueEdits, issueEdit) } @@ -130,6 +132,7 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline // create bug from given issueEdits for i, edit := range issueEdits { if i == 0 && b != nil { + // The first edit in the github result is the issue creation itself, we already have that continue } @@ -323,6 +326,7 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache. } else { for i, edit := range item.UserContentEdits.Nodes { if i == 0 && target != "" { + // The first edit in the github result is the comment creation itself, we already have that continue } diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go index cd67c99b..967f50ee 100644 --- a/bridge/github/import_test.go +++ b/bridge/github/import_test.go @@ -26,7 +26,7 @@ func Test_Importer(t *testing.T) { }{ { name: "simple issue", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/1", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/1", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "simple issue", "initial comment", nil), @@ -36,7 +36,7 @@ func Test_Importer(t *testing.T) { }, { name: "empty issue", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/2", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/2", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "empty issue", "", nil), @@ -45,7 +45,7 @@ func Test_Importer(t *testing.T) { }, { name: "complex issue", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/3", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/3", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "complex issue", "initial comment", nil), @@ -62,7 +62,7 @@ func Test_Importer(t *testing.T) { }, { name: "editions", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/4", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/4", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "editions", "initial comment edited", nil), @@ -74,7 +74,7 @@ func Test_Importer(t *testing.T) { }, { name: "comment deletion", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/5", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/5", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "comment deletion", "", nil), @@ -83,7 +83,7 @@ func Test_Importer(t *testing.T) { }, { name: "edition deletion", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/6", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/6", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "edition deletion", "initial comment", nil), @@ -95,7 +95,7 @@ func Test_Importer(t *testing.T) { }, { name: "hidden comment", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/7", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/7", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "hidden comment", "initial comment", nil), @@ -105,7 +105,7 @@ func Test_Importer(t *testing.T) { }, { name: "transfered issue", - url: "https://github.com/MichaelMure/git-but-test-github-bridge/issues/8", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/8", bug: &bug.Snapshot{ Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "transfered issue", "", nil), @@ -130,7 +130,7 @@ func Test_Importer(t *testing.T) { importer := &githubImporter{} err = importer.Init(core.Configuration{ "user": "MichaelMure", - "project": "git-but-test-github-bridge", + "project": "git-bug-test-github-bridge", "token": token, }) require.NoError(t, err) @@ -153,24 +153,31 @@ func Test_Importer(t *testing.T) { assert.Len(t, tt.bug.Operations, len(b.Snapshot().Operations)) for i, op := range tt.bug.Operations { - assert.IsType(t, ops[i], op) + require.IsType(t, ops[i], op) switch op.(type) { case *bug.CreateOperation: assert.Equal(t, ops[i].(*bug.CreateOperation).Title, op.(*bug.CreateOperation).Title) assert.Equal(t, ops[i].(*bug.CreateOperation).Message, op.(*bug.CreateOperation).Message) + assert.Equal(t, ops[i].(*bug.CreateOperation).Author.Name(), op.(*bug.CreateOperation).Author.Name()) case *bug.SetStatusOperation: assert.Equal(t, ops[i].(*bug.SetStatusOperation).Status, op.(*bug.SetStatusOperation).Status) + assert.Equal(t, ops[i].(*bug.SetStatusOperation).Author.Name(), op.(*bug.SetStatusOperation).Author.Name()) case *bug.SetTitleOperation: assert.Equal(t, ops[i].(*bug.SetTitleOperation).Was, op.(*bug.SetTitleOperation).Was) assert.Equal(t, ops[i].(*bug.SetTitleOperation).Title, op.(*bug.SetTitleOperation).Title) + assert.Equal(t, ops[i].(*bug.SetTitleOperation).Author.Name(), op.(*bug.SetTitleOperation).Author.Name()) case *bug.LabelChangeOperation: assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Added, op.(*bug.LabelChangeOperation).Added) assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Removed, op.(*bug.LabelChangeOperation).Removed) + assert.Equal(t, ops[i].(*bug.LabelChangeOperation).Author.Name(), op.(*bug.LabelChangeOperation).Author.Name()) case *bug.AddCommentOperation: assert.Equal(t, ops[i].(*bug.AddCommentOperation).Message, op.(*bug.AddCommentOperation).Message) + assert.Equal(t, ops[i].(*bug.AddCommentOperation).Author.Name(), op.(*bug.AddCommentOperation).Author.Name()) case *bug.EditCommentOperation: assert.Equal(t, ops[i].(*bug.EditCommentOperation).Message, op.(*bug.EditCommentOperation).Message) + assert.Equal(t, ops[i].(*bug.EditCommentOperation).Author.Name(), op.(*bug.EditCommentOperation).Author.Name()) + default: panic("Unknown operation type") } diff --git a/bridge/github/iterator.go b/bridge/github/iterator.go index 239b49bd..48e98f17 100644 --- a/bridge/github/iterator.go +++ b/bridge/github/iterator.go @@ -100,6 +100,8 @@ func (i *iterator) initTimelineQueryVariables() { i.timeline.variables["issueSince"] = githubv4.DateTime{Time: i.since} i.timeline.variables["timelineFirst"] = githubv4.Int(i.capacity) i.timeline.variables["timelineAfter"] = (*githubv4.String)(nil) + // Fun fact, github provide the comment edition in reverse chronological + // order, because haha. Look at me, I'm dying of laughter. i.timeline.variables["issueEditLast"] = githubv4.Int(i.capacity) i.timeline.variables["issueEditBefore"] = (*githubv4.String)(nil) i.timeline.variables["commentEditLast"] = githubv4.Int(i.capacity) @@ -278,7 +280,16 @@ func (i *iterator) NextIssueEdit() bool { return i.queryIssueEdit() } - // if there is no edits + // if there is no edit, the UserContentEdits given by github is empty. That + // means that the original message is given by the issue message. + // + // if there is edits, the UserContentEdits given by github contains both the + // original message and the following edits. The issue message give the last + // version so we don't care about that. + // + // the tricky part: for an issue older than the UserContentEdits API, github + // doesn't have the previous message version anymore and give an edition + // with .Diff == nil. We have to filter them. if len(i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) == 0 { return false } -- cgit From 537eddb97843a3f520fdedcd35f77b08880a4829 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sun, 5 May 2019 14:08:48 +0200 Subject: Fix import bug --- bridge/github/import.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bridge') diff --git a/bridge/github/import.go b/bridge/github/import.go index e72a2a45..9b9b790e 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -324,7 +324,7 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache. } } } else { - for i, edit := range item.UserContentEdits.Nodes { + for i, edit := range edits { if i == 0 && target != "" { // The first edit in the github result is the comment creation itself, we already have that continue -- cgit From 2e17f371758ad25a3674d65ef0e8e32a4660e6d4 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sun, 5 May 2019 17:48:49 +0200 Subject: Add unicode control characters test case Move `cleanupText` to utils/text/transform.go `text.Cleanup`: removing unicode control characters except for those allowed by `text.Safe` Add golang.org/x/text dependencies fix text.Cleanup Fix import panic --- bridge/github/import.go | 46 +++++++++++++++++++++++++++++--------------- bridge/github/import_test.go | 44 ++++++++++++++++++++++++++---------------- 2 files changed, 58 insertions(+), 32 deletions(-) (limited to 'bridge') diff --git a/bridge/github/import.go b/bridge/github/import.go index 9b9b790e..0c5468d8 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -3,7 +3,6 @@ package github import ( "context" "fmt" - "strings" "time" "github.com/MichaelMure/git-bug/bridge/core" @@ -11,6 +10,7 @@ import ( "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" + "github.com/MichaelMure/git-bug/util/text" "github.com/shurcooL/githubv4" ) @@ -112,12 +112,17 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline // if issueEdits is empty if len(issueEdits) == 0 { if err == bug.ErrBugNotExist { + cleanText, err := text.Cleanup(string(issue.Body)) + if err != nil { + return nil, err + } + // create bug b, err = repo.NewBugRaw( author, issue.CreatedAt.Unix(), issue.Title, - cleanupText(string(issue.Body)), + cleanText, nil, map[string]string{ keyGithubId: parseId(issue.Id), @@ -136,6 +141,11 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline continue } + cleanText, err := text.Cleanup(string(*edit.Diff)) + if err != nil { + return nil, err + } + // if the bug doesn't exist if b == nil { // we create the bug as soon as we have a legit first edition @@ -143,7 +153,7 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline author, issue.CreatedAt.Unix(), issue.Title, - cleanupText(string(*edit.Diff)), + cleanText, nil, map[string]string{ keyGithubId: parseId(issue.Id), @@ -301,12 +311,16 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache. // if comment doesn't exist if err == cache.ErrNoMatchingOp { + cleanText, err := text.Cleanup(string(item.Body)) + if err != nil { + return err + } // add comment operation op, err := b.AddCommentRaw( author, item.CreatedAt.Unix(), - cleanupText(string(item.Body)), + cleanText, nil, map[string]string{ keyGithubId: parseId(item.Id), @@ -338,10 +352,15 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache. // create comment when target is empty if target == "" { + cleanText, err := text.Cleanup(string(*edit.Diff)) + if err != nil { + return err + } + op, err := b.AddCommentRaw( editor, edit.CreatedAt.Unix(), - cleanupText(string(*edit.Diff)), + cleanText, nil, map[string]string{ keyGithubId: parseId(item.Id), @@ -395,12 +414,17 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC case edit.DeletedAt == nil: + cleanText, err := text.Cleanup(string(*edit.Diff)) + if err != nil { + return err + } + // comment edition - _, err := b.EditCommentRaw( + _, err = b.EditCommentRaw( editor, edit.CreatedAt.Unix(), target, - cleanupText(string(*edit.Diff)), + cleanText, map[string]string{ keyGithubId: parseId(edit.Id), }, @@ -505,14 +529,6 @@ func parseId(id githubv4.ID) string { return fmt.Sprintf("%v", id) } -func cleanupText(text string) string { - // windows new line, Github, really ? - text = strings.Replace(text, "\r\n", "\n", -1) - - // trim extra new line not displayed in the github UI but still present in the data - return strings.TrimSpace(text) -} - func reverseEdits(edits []userContentEdit) []userContentEdit { for i, j := 0, len(edits)-1; i < j; i, j = i+1, j-1 { edits[i], edits[j] = edits[j], edits[i] diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go index 967f50ee..48283b7a 100644 --- a/bridge/github/import_test.go +++ b/bridge/github/import_test.go @@ -31,7 +31,8 @@ func Test_Importer(t *testing.T) { Operations: []bug.Operation{ bug.NewCreateOp(author, 0, "simple issue", "initial comment", nil), bug.NewAddCommentOp(author, 0, "first comment", nil), - bug.NewAddCommentOp(author, 0, "second comment", nil)}, + bug.NewAddCommentOp(author, 0, "second comment", nil), + }, }, }, { @@ -112,6 +113,15 @@ func Test_Importer(t *testing.T) { }, }, }, + { + name: "unicode control characters", + url: "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/10", + bug: &bug.Snapshot{ + Operations: []bug.Operation{ + bug.NewCreateOp(author, 0, "unicode control characters", "u0000: \nu0001: \nu0002: \nu0003: \nu0004: \nu0005: \nu0006: \nu0007: \nu0008: \nu0009: \t\nu0010: \nu0011: \nu0012: \nu0013: \nu0014: \nu0015: \nu0016: \nu0017: \nu0018: \nu0019:", nil), + }, + }, + }, } repo := test.CreateRepo(false) @@ -142,7 +152,7 @@ func Test_Importer(t *testing.T) { fmt.Printf("test repository imported in %f seconds\n", time.Since(start).Seconds()) - require.Len(t, backend.AllBugsIds(), 8) + require.Len(t, backend.AllBugsIds(), 9) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -157,26 +167,26 @@ func Test_Importer(t *testing.T) { switch op.(type) { case *bug.CreateOperation: - assert.Equal(t, ops[i].(*bug.CreateOperation).Title, op.(*bug.CreateOperation).Title) - assert.Equal(t, ops[i].(*bug.CreateOperation).Message, op.(*bug.CreateOperation).Message) - assert.Equal(t, ops[i].(*bug.CreateOperation).Author.Name(), op.(*bug.CreateOperation).Author.Name()) + assert.Equal(t, op.(*bug.CreateOperation).Title, ops[i].(*bug.CreateOperation).Title) + assert.Equal(t, op.(*bug.CreateOperation).Message, ops[i].(*bug.CreateOperation).Message) + assert.Equal(t, op.(*bug.CreateOperation).Author.Name(), ops[i].(*bug.CreateOperation).Author.Name()) case *bug.SetStatusOperation: - assert.Equal(t, ops[i].(*bug.SetStatusOperation).Status, op.(*bug.SetStatusOperation).Status) - assert.Equal(t, ops[i].(*bug.SetStatusOperation).Author.Name(), op.(*bug.SetStatusOperation).Author.Name()) + assert.Equal(t, op.(*bug.SetStatusOperation).Status, ops[i].(*bug.SetStatusOperation).Status) + assert.Equal(t, op.(*bug.SetStatusOperation).Author.Name(), ops[i].(*bug.SetStatusOperation).Author.Name()) case *bug.SetTitleOperation: - assert.Equal(t, ops[i].(*bug.SetTitleOperation).Was, op.(*bug.SetTitleOperation).Was) - assert.Equal(t, ops[i].(*bug.SetTitleOperation).Title, op.(*bug.SetTitleOperation).Title) - assert.Equal(t, ops[i].(*bug.SetTitleOperation).Author.Name(), op.(*bug.SetTitleOperation).Author.Name()) + assert.Equal(t, op.(*bug.SetTitleOperation).Was, ops[i].(*bug.SetTitleOperation).Was) + assert.Equal(t, op.(*bug.SetTitleOperation).Title, ops[i].(*bug.SetTitleOperation).Title) + assert.Equal(t, op.(*bug.SetTitleOperation).Author.Name(), ops[i].(*bug.SetTitleOperation).Author.Name()) case *bug.LabelChangeOperation: - assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Added, op.(*bug.LabelChangeOperation).Added) - assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Removed, op.(*bug.LabelChangeOperation).Removed) - assert.Equal(t, ops[i].(*bug.LabelChangeOperation).Author.Name(), op.(*bug.LabelChangeOperation).Author.Name()) + assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Added, ops[i].(*bug.LabelChangeOperation).Added) + assert.ElementsMatch(t, op.(*bug.LabelChangeOperation).Removed, ops[i].(*bug.LabelChangeOperation).Removed) + assert.Equal(t, op.(*bug.LabelChangeOperation).Author.Name(), ops[i].(*bug.LabelChangeOperation).Author.Name()) case *bug.AddCommentOperation: - assert.Equal(t, ops[i].(*bug.AddCommentOperation).Message, op.(*bug.AddCommentOperation).Message) - assert.Equal(t, ops[i].(*bug.AddCommentOperation).Author.Name(), op.(*bug.AddCommentOperation).Author.Name()) + assert.Equal(t, op.(*bug.AddCommentOperation).Message, ops[i].(*bug.AddCommentOperation).Message) + assert.Equal(t, op.(*bug.AddCommentOperation).Author.Name(), ops[i].(*bug.AddCommentOperation).Author.Name()) case *bug.EditCommentOperation: - assert.Equal(t, ops[i].(*bug.EditCommentOperation).Message, op.(*bug.EditCommentOperation).Message) - assert.Equal(t, ops[i].(*bug.EditCommentOperation).Author.Name(), op.(*bug.EditCommentOperation).Author.Name()) + assert.Equal(t, op.(*bug.EditCommentOperation).Message, ops[i].(*bug.EditCommentOperation).Message) + assert.Equal(t, op.(*bug.EditCommentOperation).Author.Name(), ops[i].(*bug.EditCommentOperation).Author.Name()) default: panic("Unknown operation type") -- cgit