From 1e8c0ab382e39cf63457e89183401854f0fc4ff9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 31 May 2022 21:25:32 +0100 Subject: [PATCH 1/5] Prevent NPE whilst migrating if there is a team request review A pr.Reviewer may be nil when migrating from Gitea if this is a team request review. We do not migrate teams therefore we cannot map these requests, but we can migrate user requests. Signed-off-by: Andrew Thornton --- modules/migration/review.go | 1 + services/migrations/gitea_downloader.go | 32 +++++++++++++++---------- services/migrations/gitea_uploader.go | 2 ++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/modules/migration/review.go b/modules/migration/review.go index e4db33d98fd38..b5a054c642fe1 100644 --- a/modules/migration/review.go +++ b/modules/migration/review.go @@ -18,6 +18,7 @@ const ( ReviewStateApproved = "APPROVED" ReviewStateChangesRequested = "CHANGES_REQUESTED" ReviewStateCommented = "COMMENTED" + ReviewStateRequestReview = "REQUEST_REVIEW" ) // Review is a standard review information diff --git a/services/migrations/gitea_downloader.go b/services/migrations/gitea_downloader.go index 3c02e112ca73e..70683b621a9cc 100644 --- a/services/migrations/gitea_downloader.go +++ b/services/migrations/gitea_downloader.go @@ -664,18 +664,26 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review }) } - allReviews = append(allReviews, &base.Review{ - ID: pr.ID, - IssueIndex: reviewable.GetLocalIndex(), - ReviewerID: pr.Reviewer.ID, - ReviewerName: pr.Reviewer.UserName, - Official: pr.Official, - CommitID: pr.CommitID, - Content: pr.Body, - CreatedAt: pr.Submitted, - State: string(pr.State), - Comments: reviewComments, - }) + review := &base.Review{ + ID: pr.ID, + IssueIndex: reviewable.GetLocalIndex(), + Official: pr.Official, + CommitID: pr.CommitID, + Content: pr.Body, + CreatedAt: pr.Submitted, + State: string(pr.State), + Comments: reviewComments, + } + + if pr.Reviewer != nil { + review.ReviewerID = pr.Reviewer.ID + review.ReviewerName = pr.Reviewer.UserName + } else { + // we have to skip this review as it will be mapped on to something incorrect + continue + } + + allReviews = append(allReviews, review) } if len(prl) < g.maxPerPage { diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index fec1fc8c7dac3..408704adef20d 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -696,6 +696,8 @@ func convertReviewState(state string) models.ReviewType { return models.ReviewTypeReject case base.ReviewStateCommented: return models.ReviewTypeComment + case base.ReviewStateRequestReview: + return models.ReviewTypeRequest default: return models.ReviewTypePending } From fb2def08aae9721b105aa45c85ebeeef491d6e37 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 1 Jun 2022 04:03:23 +0100 Subject: [PATCH 2/5] Update services/migrations/gitea_downloader.go Co-authored-by: delvh --- services/migrations/gitea_downloader.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/services/migrations/gitea_downloader.go b/services/migrations/gitea_downloader.go index 70683b621a9cc..159e65a1ed49f 100644 --- a/services/migrations/gitea_downloader.go +++ b/services/migrations/gitea_downloader.go @@ -675,14 +675,13 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review Comments: reviewComments, } - if pr.Reviewer != nil { - review.ReviewerID = pr.Reviewer.ID - review.ReviewerName = pr.Reviewer.UserName - } else { - // we have to skip this review as it will be mapped on to something incorrect + if pr.Reviewer == nil { + // Presumably this is a team review which we cannot migrate at present but we have to skip this review as otherwise the review will be mapped on to an incorrect user. + // TODO: handle team reviews continue } - + review.ReviewerID = pr.Reviewer.ID + review.ReviewerName = pr.Reviewer.UserName allReviews = append(allReviews, review) } From b290a3e774879a557df69cf673f237ef8216f2c8 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 1 Jun 2022 20:17:51 +0100 Subject: [PATCH 3/5] update github api and include listreviewers Signed-off-by: Andrew Thornton --- go.mod | 2 +- go.sum | 6 +++--- services/migrations/error.go | 2 +- services/migrations/github.go | 25 ++++++++++++++++++++++++- 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 23e922ecef2df..16e3c84da12e9 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/gogs/cron v0.0.0-20171120032916-9f6c956d3e14 github.com/gogs/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85 github.com/golang-jwt/jwt/v4 v4.4.1 - github.com/google/go-github/v39 v39.2.0 + github.com/google/go-github/v45 v45.0.0 github.com/google/pprof v0.0.0-20220509035851-59ca7ad80af3 github.com/google/uuid v1.3.0 github.com/gorilla/feeds v1.1.1 diff --git a/go.sum b/go.sum index 3af17c070b0fa..d1ce36fe1fb15 100644 --- a/go.sum +++ b/go.sum @@ -730,11 +730,11 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= github.com/google/go-github/v28 v28.1.1/go.mod h1:bsqJWQX05omyWVmc00nEUql9mhQyv38lDZ8kPZcQVoM= -github.com/google/go-github/v39 v39.2.0 h1:rNNM311XtPOz5rDdsJXAp2o8F67X9FnROXTvto3aSnQ= -github.com/google/go-github/v39 v39.2.0/go.mod h1:C1s8C5aCC9L+JXIYpJM5GYytdX52vC1bLvHEF1IhBrE= +github.com/google/go-github/v45 v45.0.0 h1:LU0WBjYidxIVyx7PZeWb+FP4JZJ3Wh3FQgdumnGqiLs= +github.com/google/go-github/v45 v45.0.0/go.mod h1:FObaZJEDSTa/WGCzZ2Z3eoCDXWJKMenWWTrd8jrta28= github.com/google/go-licenses v0.0.0-20210329231322-ce1d9163b77d/go.mod h1:+TYOmkVoJOpwnS0wfdsJCV9CoD5nJYsHoFk/0CrTK4M= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= diff --git a/services/migrations/error.go b/services/migrations/error.go index 3b3f975012f1e..d26fa8112cbf5 100644 --- a/services/migrations/error.go +++ b/services/migrations/error.go @@ -8,7 +8,7 @@ package migrations import ( "errors" - "github.com/google/go-github/v39/github" + "github.com/google/go-github/v45/github" ) // ErrRepoNotCreated returns the error that repository not created diff --git a/services/migrations/github.go b/services/migrations/github.go index faf0cf0794179..1b08e262e4baa 100644 --- a/services/migrations/github.go +++ b/services/migrations/github.go @@ -21,7 +21,7 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" - "github.com/google/go-github/v39/github" + "github.com/google/go-github/v45/github" "golang.org/x/oauth2" ) @@ -817,5 +817,28 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev } opt.Page = resp.NextPage } + for { + g.waitAndPickClient() + reviewers, resp, err := g.getClient().PullRequests.ListReviewers(g.ctx, g.repoOwner, g.repoName, int(reviewable.GetForeignIndex()), opt) + if err != nil { + return nil, fmt.Errorf("error while listing repos: %v", err) + } + g.setRate(&resp.Rate) + for _, user := range reviewers.Users { + r := &base.Review{ + ID: user.GetID(), + ReviewerID: user.GetID(), + ReviewerName: user.GetLogin(), + State: base.ReviewStateRequestReview, + IssueIndex: reviewable.GetLocalIndex(), + } + allReviews = append(allReviews, r) + } + // TODO: Handle Team requests + if resp.NextPage == 0 { + break + } + opt.Page = resp.NextPage + } return allReviews, nil } From 28fbdba4c351af4334bc6612af9d31e292a34c34 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 2 Jun 2022 22:36:19 +0100 Subject: [PATCH 4/5] as per review Signed-off-by: Andrew Thornton --- services/migrations/gitea_downloader.go | 10 +++++----- services/migrations/github.go | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/services/migrations/gitea_downloader.go b/services/migrations/gitea_downloader.go index 159e65a1ed49f..39ef19637a4b1 100644 --- a/services/migrations/gitea_downloader.go +++ b/services/migrations/gitea_downloader.go @@ -639,6 +639,11 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review } for _, pr := range prl { + if pr.Reviewer == nil { + // Presumably this is a team review which we cannot migrate at present but we have to skip this review as otherwise the review will be mapped on to an incorrect user. + // TODO: handle team reviews + continue + } rcl, _, err := g.client.ListPullReviewComments(g.repoOwner, g.repoName, reviewable.GetForeignIndex(), pr.ID) if err != nil { @@ -675,11 +680,6 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review Comments: reviewComments, } - if pr.Reviewer == nil { - // Presumably this is a team review which we cannot migrate at present but we have to skip this review as otherwise the review will be mapped on to an incorrect user. - // TODO: handle team reviews - continue - } review.ReviewerID = pr.Reviewer.ID review.ReviewerName = pr.Reviewer.UserName allReviews = append(allReviews, review) diff --git a/services/migrations/github.go b/services/migrations/github.go index 1b08e262e4baa..cbe5469570441 100644 --- a/services/migrations/github.go +++ b/services/migrations/github.go @@ -826,7 +826,6 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev g.setRate(&resp.Rate) for _, user := range reviewers.Users { r := &base.Review{ - ID: user.GetID(), ReviewerID: user.GetID(), ReviewerName: user.GetLogin(), State: base.ReviewStateRequestReview, From 5b095fb500b6f716f2c7699d27a2606e0eaca947 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 7 Jun 2022 21:49:39 +0100 Subject: [PATCH 5/5] as per review Signed-off-by: Andrew Thornton --- services/migrations/gitea_downloader.go | 20 ++++++++++---------- services/migrations/github.go | 2 ++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/services/migrations/gitea_downloader.go b/services/migrations/gitea_downloader.go index 39ef19637a4b1..4ad55894ee28f 100644 --- a/services/migrations/gitea_downloader.go +++ b/services/migrations/gitea_downloader.go @@ -670,18 +670,18 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review } review := &base.Review{ - ID: pr.ID, - IssueIndex: reviewable.GetLocalIndex(), - Official: pr.Official, - CommitID: pr.CommitID, - Content: pr.Body, - CreatedAt: pr.Submitted, - State: string(pr.State), - Comments: reviewComments, + ID: pr.ID, + IssueIndex: reviewable.GetLocalIndex(), + ReviewerID: pr.Reviewer.ID, + ReviewerName: pr.Reviewer.UserName, + Official: pr.Official, + CommitID: pr.CommitID, + Content: pr.Body, + CreatedAt: pr.Submitted, + State: string(pr.State), + Comments: reviewComments, } - review.ReviewerID = pr.Reviewer.ID - review.ReviewerName = pr.Reviewer.UserName allReviews = append(allReviews, review) } diff --git a/services/migrations/github.go b/services/migrations/github.go index cbe5469570441..5f5b430fa9e07 100644 --- a/services/migrations/github.go +++ b/services/migrations/github.go @@ -778,6 +778,7 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev opt := &github.ListOptions{ PerPage: g.maxPerPage, } + // Get approve/request change reviews for { g.waitAndPickClient() reviews, resp, err := g.getClient().PullRequests.ListReviews(g.ctx, g.repoOwner, g.repoName, int(reviewable.GetForeignIndex()), opt) @@ -817,6 +818,7 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev } opt.Page = resp.NextPage } + // Get requested reviews for { g.waitAndPickClient() reviewers, resp, err := g.getClient().PullRequests.ListReviewers(g.ctx, g.repoOwner, g.repoName, int(reviewable.GetForeignIndex()), opt)