From 7948cb3149ab64484a8d4c6644f53f9f39accbef Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 9 Jun 2022 03:50:05 +0100 Subject: [PATCH] Prevent NPE whilst migrating if there is a team request review (#19855) 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 --- go.mod | 2 +- go.sum | 6 +++--- modules/migration/review.go | 1 + services/migrations/error.go | 2 +- services/migrations/gitea_downloader.go | 11 +++++++++-- services/migrations/gitea_uploader.go | 2 ++ services/migrations/github.go | 26 ++++++++++++++++++++++++- 7 files changed, 42 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 33f9893d10..4dea7f9423 100644 --- a/go.mod +++ b/go.mod @@ -43,7 +43,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 b20cc1ca8e..d0a1cb84ed 100644 --- a/go.sum +++ b/go.sum @@ -735,11 +735,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/modules/migration/review.go b/modules/migration/review.go index e4db33d98f..b5a054c642 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/error.go b/services/migrations/error.go index 3b3f975012..d26fa8112c 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/gitea_downloader.go b/services/migrations/gitea_downloader.go index 3c02e112ca..4ad55894ee 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 { @@ -664,7 +669,7 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review }) } - allReviews = append(allReviews, &base.Review{ + review := &base.Review{ ID: pr.ID, IssueIndex: reviewable.GetLocalIndex(), ReviewerID: pr.Reviewer.ID, @@ -675,7 +680,9 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review CreatedAt: pr.Submitted, State: string(pr.State), Comments: reviewComments, - }) + } + + allReviews = append(allReviews, review) } if len(prl) < g.maxPerPage { diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index fec1fc8c7d..408704adef 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 } diff --git a/services/migrations/github.go b/services/migrations/github.go index faf0cf0794..5f5b430fa9 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" ) @@ -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,5 +818,28 @@ 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) + 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{ + 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 }