diff --git a/modules/migrations/base/downloader.go b/modules/migrations/base/downloader.go index 9a09fdac0..f28d0b61e 100644 --- a/modules/migrations/base/downloader.go +++ b/modules/migrations/base/downloader.go @@ -11,9 +11,9 @@ type Downloader interface { GetMilestones() ([]*Milestone, error) GetReleases() ([]*Release, error) GetLabels() ([]*Label, error) - GetIssues(start, limit int) ([]*Issue, error) + GetIssues(page, perPage int) ([]*Issue, bool, error) GetComments(issueNumber int64) ([]*Comment, error) - GetPullRequests(start, limit int) ([]*PullRequest, error) + GetPullRequests(page, perPage int) ([]*PullRequest, error) } // DownloaderFactory defines an interface to match a downloader implementation and create a downloader diff --git a/modules/migrations/git.go b/modules/migrations/git.go index cbaa37282..335d44ec9 100644 --- a/modules/migrations/git.go +++ b/modules/migrations/git.go @@ -53,9 +53,9 @@ func (g *PlainGitDownloader) GetReleases() ([]*base.Release, error) { return nil, ErrNotSupported } -// GetIssues returns issues according start and limit -func (g *PlainGitDownloader) GetIssues(start, limit int) ([]*base.Issue, error) { - return nil, ErrNotSupported +// GetIssues returns issues according page and perPage +func (g *PlainGitDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, error) { + return nil, false, ErrNotSupported } // GetComments returns comments according issueNumber @@ -63,7 +63,7 @@ func (g *PlainGitDownloader) GetComments(issueNumber int64) ([]*base.Comment, er return nil, ErrNotSupported } -// GetPullRequests returns pull requests according start and limit +// GetPullRequests returns pull requests according page and perPage func (g *PlainGitDownloader) GetPullRequests(start, limit int) ([]*base.PullRequest, error) { return nil, ErrNotSupported } diff --git a/modules/migrations/github.go b/modules/migrations/github.go index 8e1cd67df..21c1beced 100644 --- a/modules/migrations/github.go +++ b/modules/migrations/github.go @@ -272,71 +272,65 @@ func convertGithubReactions(reactions *github.Reactions) *base.Reactions { } // GetIssues returns issues according start and limit -func (g *GithubDownloaderV3) GetIssues(start, limit int) ([]*base.Issue, error) { - var perPage = 100 +func (g *GithubDownloaderV3) GetIssues(page, perPage int) ([]*base.Issue, bool, error) { opt := &github.IssueListByRepoOptions{ Sort: "created", Direction: "asc", State: "all", ListOptions: github.ListOptions{ PerPage: perPage, + Page: page, }, } - var allIssues = make([]*base.Issue, 0, limit) - for { - issues, resp, err := g.client.Issues.ListByRepo(g.ctx, g.repoOwner, g.repoName, opt) - if err != nil { - return nil, fmt.Errorf("error while listing repos: %v", err) - } - for _, issue := range issues { - if issue.IsPullRequest() { - continue - } - var body string - if issue.Body != nil { - body = *issue.Body - } - var milestone string - if issue.Milestone != nil { - milestone = *issue.Milestone.Title - } - var labels = make([]*base.Label, 0, len(issue.Labels)) - for _, l := range issue.Labels { - labels = append(labels, convertGithubLabel(&l)) - } - var reactions *base.Reactions - if issue.Reactions != nil { - reactions = convertGithubReactions(issue.Reactions) - } - var email string - if issue.User.Email != nil { - email = *issue.User.Email - } - allIssues = append(allIssues, &base.Issue{ - Title: *issue.Title, - Number: int64(*issue.Number), - PosterName: *issue.User.Login, - PosterEmail: email, - Content: body, - Milestone: milestone, - State: *issue.State, - Created: *issue.CreatedAt, - Labels: labels, - Reactions: reactions, - Closed: issue.ClosedAt, - IsLocked: *issue.Locked, - }) - if len(allIssues) >= limit { - return allIssues, nil - } + var allIssues = make([]*base.Issue, 0, perPage) + + issues, _, err := g.client.Issues.ListByRepo(g.ctx, g.repoOwner, g.repoName, opt) + if err != nil { + return nil, false, fmt.Errorf("error while listing repos: %v", err) + } + for _, issue := range issues { + if issue.IsPullRequest() { + continue } - if resp.NextPage == 0 { - break + var body string + if issue.Body != nil { + body = *issue.Body } - opt.Page = resp.NextPage + var milestone string + if issue.Milestone != nil { + milestone = *issue.Milestone.Title + } + var labels = make([]*base.Label, 0, len(issue.Labels)) + for _, l := range issue.Labels { + labels = append(labels, convertGithubLabel(&l)) + } + var reactions *base.Reactions + if issue.Reactions != nil { + reactions = convertGithubReactions(issue.Reactions) + } + + var email string + if issue.User.Email != nil { + email = *issue.User.Email + } + allIssues = append(allIssues, &base.Issue{ + Title: *issue.Title, + Number: int64(*issue.Number), + PosterName: *issue.User.Login, + PosterEmail: email, + Content: body, + Milestone: milestone, + State: *issue.State, + Created: *issue.CreatedAt, + Labels: labels, + Reactions: reactions, + Closed: issue.ClosedAt, + IsLocked: *issue.Locked, + }) } - return allIssues, nil + + return allIssues, len(issues) < perPage, nil } // GetComments returns comments according issueNumber @@ -379,97 +373,91 @@ func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, er return allComments, nil } -// GetPullRequests returns pull requests according start and limit -func (g *GithubDownloaderV3) GetPullRequests(start, limit int) ([]*base.PullRequest, error) { +// GetPullRequests returns pull requests according page and perPage +func (g *GithubDownloaderV3) GetPullRequests(page, perPage int) ([]*base.PullRequest, error) { opt := &github.PullRequestListOptions{ Sort: "created", Direction: "asc", State: "all", ListOptions: github.ListOptions{ - PerPage: 100, + PerPage: perPage, + Page: page, }, } - var allPRs = make([]*base.PullRequest, 0, 100) - for { - prs, resp, err := g.client.PullRequests.List(g.ctx, g.repoOwner, g.repoName, opt) - if err != nil { - return nil, fmt.Errorf("error while listing repos: %v", err) - } - for _, pr := range prs { - var body string - if pr.Body != nil { - body = *pr.Body - } - var milestone string - if pr.Milestone != nil { - milestone = *pr.Milestone.Title - } - var labels = make([]*base.Label, 0, len(pr.Labels)) - for _, l := range pr.Labels { - labels = append(labels, convertGithubLabel(l)) - } + var allPRs = make([]*base.PullRequest, 0, perPage) - // FIXME: This API missing reactions, we may need another extra request to get reactions + prs, _, err := g.client.PullRequests.List(g.ctx, g.repoOwner, g.repoName, opt) + if err != nil { + return nil, fmt.Errorf("error while listing repos: %v", err) + } + for _, pr := range prs { + var body string + if pr.Body != nil { + body = *pr.Body + } + var milestone string + if pr.Milestone != nil { + milestone = *pr.Milestone.Title + } + var labels = make([]*base.Label, 0, len(pr.Labels)) + for _, l := range pr.Labels { + labels = append(labels, convertGithubLabel(l)) + } - var email string - if pr.User.Email != nil { - email = *pr.User.Email - } - var merged bool - // pr.Merged is not valid, so use MergedAt to test if it's merged - if pr.MergedAt != nil { - merged = true - } + // FIXME: This API missing reactions, we may need another extra request to get reactions - var headRepoName string - var cloneURL string - if pr.Head.Repo != nil { - headRepoName = *pr.Head.Repo.Name - cloneURL = *pr.Head.Repo.CloneURL - } - var mergeCommitSHA string - if pr.MergeCommitSHA != nil { - mergeCommitSHA = *pr.MergeCommitSHA - } + var email string + if pr.User.Email != nil { + email = *pr.User.Email + } + var merged bool + // pr.Merged is not valid, so use MergedAt to test if it's merged + if pr.MergedAt != nil { + merged = true + } - allPRs = append(allPRs, &base.PullRequest{ - Title: *pr.Title, - Number: int64(*pr.Number), - PosterName: *pr.User.Login, - PosterEmail: email, - Content: body, - Milestone: milestone, - State: *pr.State, - Created: *pr.CreatedAt, - Closed: pr.ClosedAt, - Labels: labels, - Merged: merged, - MergeCommitSHA: mergeCommitSHA, - MergedTime: pr.MergedAt, - IsLocked: pr.ActiveLockReason != nil, - Head: base.PullRequestBranch{ - Ref: *pr.Head.Ref, - SHA: *pr.Head.SHA, - RepoName: headRepoName, - OwnerName: *pr.Head.User.Login, - CloneURL: cloneURL, - }, - Base: base.PullRequestBranch{ - Ref: *pr.Base.Ref, - SHA: *pr.Base.SHA, - RepoName: *pr.Base.Repo.Name, - OwnerName: *pr.Base.User.Login, - }, - PatchURL: *pr.PatchURL, - }) - if len(allPRs) >= limit { - return allPRs, nil - } + var headRepoName string + var cloneURL string + if pr.Head.Repo != nil { + headRepoName = *pr.Head.Repo.Name + cloneURL = *pr.Head.Repo.CloneURL } - if resp.NextPage == 0 { - break + var mergeCommitSHA string + if pr.MergeCommitSHA != nil { + mergeCommitSHA = *pr.MergeCommitSHA } - opt.Page = resp.NextPage + + allPRs = append(allPRs, &base.PullRequest{ + Title: *pr.Title, + Number: int64(*pr.Number), + PosterName: *pr.User.Login, + PosterEmail: email, + Content: body, + Milestone: milestone, + State: *pr.State, + Created: *pr.CreatedAt, + Closed: pr.ClosedAt, + Labels: labels, + Merged: merged, + MergeCommitSHA: mergeCommitSHA, + MergedTime: pr.MergedAt, + IsLocked: pr.ActiveLockReason != nil, + Head: base.PullRequestBranch{ + Ref: *pr.Head.Ref, + SHA: *pr.Head.SHA, + RepoName: headRepoName, + OwnerName: *pr.Head.User.Login, + CloneURL: cloneURL, + }, + Base: base.PullRequestBranch{ + Ref: *pr.Base.Ref, + SHA: *pr.Base.SHA, + RepoName: *pr.Base.Repo.Name, + OwnerName: *pr.Base.User.Login, + }, + PatchURL: *pr.PatchURL, + }) } + return allPRs, nil } diff --git a/modules/migrations/github_test.go b/modules/migrations/github_test.go index e1d3efad5..c14292ecc 100644 --- a/modules/migrations/github_test.go +++ b/modules/migrations/github_test.go @@ -166,9 +166,11 @@ func TestGitHubDownloadRepo(t *testing.T) { }, releases[len(releases)-1:]) // downloader.GetIssues() - issues, err := downloader.GetIssues(0, 3) + issues, isEnd, err := downloader.GetIssues(1, 8) assert.NoError(t, err) assert.EqualValues(t, 3, len(issues)) + assert.False(t, isEnd) + var ( closed1 = time.Date(2018, 10, 23, 02, 57, 43, 0, time.UTC) ) @@ -319,7 +321,7 @@ something like in the latest 15days could be enough don't you think ? }, comments[:3]) // downloader.GetPullRequests() - prs, err := downloader.GetPullRequests(0, 3) + prs, err := downloader.GetPullRequests(1, 3) assert.NoError(t, err) assert.EqualValues(t, 3, len(prs)) diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index ac55a2e72..4b1229f94 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -128,8 +128,8 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts if opts.Issues { log.Trace("migrating issues and comments") - for { - issues, err := downloader.GetIssues(0, 100) + for i := 1; ; i++ { + issues, isEnd, err := downloader.GetIssues(i, 100) if err != nil { return err } @@ -160,7 +160,7 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts } } - if len(issues) < 100 { + if isEnd { break } } @@ -168,8 +168,8 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts if opts.PullRequests { log.Trace("migrating pull requests and comments") - for { - prs, err := downloader.GetPullRequests(0, 100) + for i := 1; ; i++ { + prs, err := downloader.GetPullRequests(i, 100) if err != nil { return err }