From df782bdd99616cc0fc9c4a9b85135a77d9a77ca8 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Mon, 9 Jul 2018 19:18:30 +0200 Subject: [PATCH] Improved GitHub comments performance Signed-off-by: Jonas Franz --- Gopkg.lock | 54 +++++-------- migrations/github.go | 144 ++++++++++++++++++++++++++++------ migrations/github_test.go | 17 ++++ web/public/js/repos-status.js | 27 ++++++- web/public/js/select-repos.js | 2 +- web/templates/migration.tmpl | 2 +- 6 files changed, 184 insertions(+), 62 deletions(-) create mode 100644 migrations/github_test.go diff --git a/Gopkg.lock b/Gopkg.lock index dfed5f1..60fcdcc 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -2,11 +2,10 @@ [[projects]] - branch = "myuserinfo" + branch = "master" name = "code.gitea.io/sdk" packages = ["gitea"] - revision = "6c33946d3f5b59646286ec6d6f02e88de690a1b3" - source = "github.com/JonasFranzDEV/go-sdk" + revision = "79a281c4e34ae44cf96a23f0283729a074a6c2a0" [[projects]] name = "github.com/BurntSushi/toml" @@ -26,6 +25,12 @@ packages = ["."] revision = "88fd7c52a2c704c5d530718c1be454292a806e2b" +[[projects]] + name = "github.com/davecgh/go-spew" + packages = ["spew"] + revision = "346938d642f2ec3594ed81d874461961cd0faa76" + version = "v1.1.0" + [[projects]] branch = "master" name = "github.com/go-macaron/binding" @@ -50,12 +55,6 @@ revision = "bd47f2894846e32edcf9aa37290fef76c327883f" version = "v1.11.1" -[[projects]] - name = "github.com/davecgh/go-spew" - packages = ["spew"] - revision = "346938d642f2ec3594ed81d874461961cd0faa76" - version = "v1.1.0" - [[projects]] name = "github.com/golang/protobuf" packages = ["proto"] @@ -74,29 +73,6 @@ packages = ["query"] revision = "53e6ce116135b80d037921a7fdd5138cf32d7a8a" -[[projects]] - name = "github.com/pmezard/go-difflib" - packages = ["difflib"] - revision = "792786c7400a136282c1664665ae0a8db921c6c2" - version = "v1.0.0" - -[[projects]] - name = "github.com/stretchr/objx" - packages = ["."] - revision = "477a77ecc69700c7cdeb1fa9e129548e1c1c393c" - version = "v0.1.1" - -[[projects]] - name = "github.com/stretchr/testify" - packages = [ - ".", - "assert", - "http", - "mock" - ] - revision = "f35b8ab0b5a2cef36673838d662e249dd9c94686" - version = "v1.2.2" - [[projects]] branch = "master" name = "github.com/jinzhu/configor" @@ -109,6 +85,18 @@ revision = "645ef00459ed84a119197bfb8d8205042c6df63d" version = "v0.8.0" +[[projects]] + name = "github.com/pmezard/go-difflib" + packages = ["difflib"] + revision = "792786c7400a136282c1664665ae0a8db921c6c2" + version = "v1.0.0" + +[[projects]] + name = "github.com/stretchr/testify" + packages = ["assert"] + revision = "f35b8ab0b5a2cef36673838d662e249dd9c94686" + version = "v1.2.2" + [[projects]] name = "github.com/urfave/cli" packages = ["."] @@ -175,6 +163,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "884bf618611f0aaad19defa64a14fa70fd2b522afe8cf99dedbfada2a9d8b88d" + inputs-digest = "d73409e3342f71d7a737dd452af78159ec22f8ae128c1b009d7189b3e87f27fb" solver-name = "gps-cdcl" solver-version = 1 diff --git a/migrations/github.go b/migrations/github.go index 8d9e4c3..72431fd 100644 --- a/migrations/github.go +++ b/migrations/github.go @@ -3,6 +3,9 @@ package migrations import ( "context" "fmt" + "regexp" + "strconv" + "strings" "sync" "code.gitea.io/sdk/gitea" @@ -38,8 +41,11 @@ func (fm *FetchMigratory) MigrateFromGitHub() error { fm.Status.FatalError = err return fmt.Errorf("Repository migration: %v", err) } - var wg sync.WaitGroup if fm.Options.Issues || fm.Options.PullRequests { + var commentsChan chan *[]*github.IssueComment + if fm.Options.Comments { + commentsChan = fm.fetchCommentsAsync() + } issues, err := fm.FetchIssues() if err != nil { fm.Status.Stage = Failed @@ -47,26 +53,71 @@ func (fm *FetchMigratory) MigrateFromGitHub() error { return err } fm.Status.Stage = Migrating + fm.Status.Issues = int64(len(issues)) + migratedIssues := make(map[int]*gitea.Issue) for _, issue := range issues { if (!issue.IsPullRequest() || fm.Options.PullRequests) && (issue.IsPullRequest() || fm.Options.Issues) { - fm.Status.Issues++ - giteaIssue, err := fm.Issue(issue) + migratedIssues[issue.GetNumber()], err = fm.Issue(issue) if err != nil { fm.Status.IssuesError++ // TODO log errors continue } - wg.Add(1) - go func() { - fm.FetchAndMigrateComments(issue, giteaIssue) - wg.Done() - }() fm.Status.IssuesMigrated++ + } else { + fm.Status.Issues-- + } + } + if fm.Options.Comments { + var comments []*github.IssueComment + if cmts := <-commentsChan; cmts == nil { + fm.Status.Stage = Failed + return err + } else { + comments = *cmts + } + + if err != nil { + fm.Status.Stage = Failed + fm.Status.FatalError = err + return err + } + fm.Status.Comments = int64(len(comments)) + commentsByIssue := make(map[*gitea.Issue][]*github.IssueComment, len(migratedIssues)) + for _, comment := range comments { + issueIndex, err := getIssueIndexFromHTMLURL(comment.GetHTMLURL()) + if err != nil { + fm.Status.CommentsError++ + continue + } + if issue, ok := migratedIssues[issueIndex]; ok && issue != nil { + if list, ok := commentsByIssue[issue]; !ok && list != nil { + commentsByIssue[issue] = []*github.IssueComment{comment} + } else { + commentsByIssue[issue] = append(list, comment) + } + } else { + fm.Status.CommentsError++ + continue + } + } + wg := sync.WaitGroup{} + for issue, comms := range commentsByIssue { + wg.Add(1) + go func(i *gitea.Issue, cs []*github.IssueComment) { + for _, comm := range cs { + if _, err := fm.IssueComment(i, comm); err != nil { + fm.Status.CommentsError++ + continue + } + fm.Status.CommentsMigrated++ + } + }(issue, comms) } + wg.Wait() } } - wg.Wait() if fm.Status.FatalError != nil { fm.Status.Stage = Failed return nil @@ -75,22 +126,30 @@ func (fm *FetchMigratory) MigrateFromGitHub() error { return nil } -// FetchAndMigrateComments loads all comments from GitHub and migrates them to Gitea -func (fm *FetchMigratory) FetchAndMigrateComments(issue *github.Issue, giteaIssue *gitea.Issue) { - comments, _, err := fm.GHClient.Issues.ListComments(fm.ctx(), fm.RepoOwner, fm.RepoName, issue.GetNumber(), nil) - if err != nil { - // TODO log errors - return +var issueIndexRegex = regexp.MustCompile(`/(issues|pull)/([0-9]+)#`) + +func getIssueIndexFromHTMLURL(htmlURL string) (int, error) { + // Alt is 4 times faster but more error prune + if res, err := getIssueIndexFromHTMLURLAlt(htmlURL); err == nil { + return res, nil } - fm.Status.Comments += int64(len(comments)) - for _, gc := range comments { - if _, err := fm.IssueComment(giteaIssue, gc); err != nil { - fm.Status.CommentsError++ - // TODO log errors - return - } - fm.Status.CommentsMigrated++ + matches := issueIndexRegex.FindStringSubmatch(htmlURL) + if len(matches) < 3 { + return 0, fmt.Errorf("cannot parse issue id from HTML URL: %s", htmlURL) } + return strconv.Atoi(matches[2]) +} +func getIssueIndexFromHTMLURLAlt(htmlURL string) (int, error) { + res := strings.Split(htmlURL, "/issues/") + if len(res) != 2 { + res = strings.Split(htmlURL, "/pull/") + } + if len(res) != 2 { + return 0, fmt.Errorf("invalid HTMLURL: %s", htmlURL) + } + number := res[1] + number = strings.Split(number, "#")[0] + return strconv.Atoi(number) } // FetchIssues fetches all issues from GitHub @@ -117,3 +176,42 @@ func (fm *FetchMigratory) FetchIssues() ([]*github.Issue, error) { } return allIssues, nil } + +// FetchComments fetches all comments from GitHub +func (fm *FetchMigratory) FetchComments() ([]*github.IssueComment, error) { + var allComments = make([]*github.IssueComment, 0) + opt := &github.IssueListCommentsOptions{ + Sort: "created", + Direction: "asc", + ListOptions: github.ListOptions{ + PerPage: 100, + }, + } + for { + comments, resp, err := fm.GHClient.Issues.ListComments(fm.ctx(), fm.RepoOwner, fm.RepoName, 0, opt) + if err != nil { + return nil, fmt.Errorf("error while listing repos: %v", err) + } + allComments = append(allComments, comments...) + if resp.NextPage == 0 { + break + } + opt.Page = resp.NextPage + } + return allComments, nil +} + +func (fm *FetchMigratory) fetchCommentsAsync() chan *[]*github.IssueComment { + ret := make(chan *[]*github.IssueComment, 1) + go func(f *FetchMigratory) { + comments, err := f.FetchComments() + if err != nil { + f.Status.FatalError = err + ret <- nil + return + } + f.Status.Comments = int64(len(comments)) + ret <- &comments + }(fm) + return ret +} diff --git a/migrations/github_test.go b/migrations/github_test.go new file mode 100644 index 0000000..c0a736f --- /dev/null +++ b/migrations/github_test.go @@ -0,0 +1,17 @@ +package migrations + +import ( + "testing" +) + +func BenchmarkGetIssueIndexFromHTMLURLAlt(b *testing.B) { + for i := 0; i <= b.N; i++ { + getIssueIndexFromHTMLURLAlt("https://github.com/octocat/Hello-World/issues/1347#issuecomment-1") + } +} + +func BenchmarkGetIssueIndexFromHTMLURL(b *testing.B) { + for i := 0; i <= b.N; i++ { + getIssueIndexFromHTMLURL("https://github.com/octocat/Hello-World/issues/1347#issuecomment-1") + } +} diff --git a/web/public/js/repos-status.js b/web/public/js/repos-status.js index 0fddbbf..2098822 100644 --- a/web/public/js/repos-status.js +++ b/web/public/js/repos-status.js @@ -1,8 +1,12 @@ +var done = false; + function update() { $.getJSON("/status", function(data) { handleData(data); }).always(function() { - window.setTimeout(update, 1000); + if(!done){ + window.setTimeout(update, 1000); + } }); } $(function() { @@ -19,6 +23,7 @@ $(function() { function handleData(data) { if(Object.keys(data.finished).length + Object.keys(data.failed).length === $(".repo-progress").progress('get total')) { $(".repo-progress").progress('complete'); + done = true; } else { $(".repo-progress").progress('set progress', Object.keys(data.finished).length + Object.keys(data.failed).length); } @@ -38,13 +43,27 @@ function handleData(data) { }); forEach(data.running, function (repo, report) { var content = handleNonPending(repo, report); - content.find(".comment-progress").progress('set progress', report.migrated_comments + report.failed_commments); + if (content.find(".comment-progress").progress('get total') !== report.total_comments) { + content.find(".comment-progress").progress('set total', report.total_comments) + } + if (content.find(".issue-progress").progress('get total') !== report.total_issues) { + content.find(".issue-progress").progress('set total', report.total_issues) + } + content.find(".comment-progress").progress('set progress', report.migrated_comments + report.failed_comments); content.find(".issue-progress").progress('set progress', report.migrated_issues + report.failed_issues); }); forEach(data.finished, function (repo, report) { var content = handleNonPending(repo, report); - content.find(".comment-progress").progress('complete'); + if (content.find(".comment-progress").progress('get total') !== report.total_comments) { + content.find(".comment-progress").progress('set total', report.total_comments) + } + if (content.find(".issue-progress").progress('get total') !== report.total_issues) { + content.find(".issue-progress").progress('set total', report.total_issues) + } + content.find(".comment-progress").progress('set progress', report.migrated_comments + report.failed_comments); + content.find(".issue-progress").progress('set progress', report.migrated_issues + report.failed_issues); content.find(".issue-progress").progress('complete'); + content.find(".comment-progress").progress('complete'); }); } function forEach(object, callback) { @@ -72,7 +91,7 @@ function handleNonPending(repo, report) { active : 'Migrated {value} of {total} comments', success : '{total} comments migrated!' }, - total: report.total_comments, + total: report.total_comments+1, value: report.migrated_comments + report.failed_comments, }); content.addClass("non-pending"); diff --git a/web/public/js/select-repos.js b/web/public/js/select-repos.js index dd7d1e8..6939f97 100644 --- a/web/public/js/select-repos.js +++ b/web/public/js/select-repos.js @@ -23,7 +23,7 @@ function parseReposInTextArea() { } function addRepoToList(repo) { - var item = $("#repo-item").children('.item'); + var item = $("#repo-item").children('.item').clone(); item.html(item.html().replace(/FULL_REPO_NAME/g, repo)); console.log(repo, item.html()); $("#repo-list").append(item); diff --git a/web/templates/migration.tmpl b/web/templates/migration.tmpl index 8bc2a71..0f3baef 100644 --- a/web/templates/migration.tmpl +++ b/web/templates/migration.tmpl @@ -42,7 +42,7 @@
Migrating comments
-

0 migration(s) of comments failed

+

0 migration(s) of comments failed