Improved GitHub comments performance

Signed-off-by: Jonas Franz <info@jonasfranz.software>

Gopkg.lock

@@ -2,11 +2,10 @@
 
 
 [[projects]]
-  branch = "myuserinfo"
+  branch = "master"
   name = "code.gitea.io/sdk"
   packages = ["gitea"]
-  revision = "6c33946d3f5b59646286ec6d6f02e88de690a1b3"
+  revision = "79a281c4e34ae44cf96a23f0283729a074a6c2a0"
-  source = "github.com/JonasFranzDEV/go-sdk"
 
 [[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

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++
+				migratedIssues[issue.GetNumber()], err = fm.Issue(issue)
-				giteaIssue, 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
+var issueIndexRegex = regexp.MustCompile(`/(issues|pull)/([0-9]+)#`)
-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)
+func getIssueIndexFromHTMLURL(htmlURL string) (int, error) {
-	if err != nil {
+	// Alt is 4 times faster but more error prune
-		// TODO log errors
+	if res, err := getIssueIndexFromHTMLURLAlt(htmlURL); err == nil {
-		return
+		return res, nil
 	}
-	fm.Status.Comments += int64(len(comments))
+	matches := issueIndexRegex.FindStringSubmatch(htmlURL)
-	for _, gc := range comments {
+	if len(matches) < 3 {
-		if _, err := fm.IssueComment(giteaIssue, gc); err != nil {
+		return 0, fmt.Errorf("cannot parse issue id from HTML URL: %s", htmlURL)
-			fm.Status.CommentsError++
-			// TODO log errors
-			return
-		}
-		fm.Status.CommentsMigrated++
 	}
+	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
+}

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")
+	}
+}

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");

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);

web/templates/migration.tmpl

@@ -42,7 +42,7 @@
         <div class="bar"></div>
         <div class="label">Migrating comments</div>
     </div>
-    <p><b class="failed-issues">0</b> migration(s) of comments failed</p>
+    <p><b class="failed-comments">0</b> migration(s) of comments failed</p>
 </div>
 <div id="content-failed" style="display: none;">
     <div class="ui negative message">