From f4462190c9e6d3da66b8850bbd7b142c620a7cf3878b1bd5e8694b665190d5d5 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Fri, 29 Nov 2024 17:33:01 +0100 Subject: [PATCH] workflow-pr: maintainership API change --- bots-common/gitea_utils.go | 69 ++++-------------- workflow-pr/maintainership.go | 61 +++++++++------- workflow-pr/maintainership_test.go | 110 +++++++++++++++++++++++------ 3 files changed, 134 insertions(+), 106 deletions(-) diff --git a/bots-common/gitea_utils.go b/bots-common/gitea_utils.go index d2de081..3ec0316 100644 --- a/bots-common/gitea_utils.go +++ b/bots-common/gitea_utils.go @@ -23,7 +23,6 @@ import ( "io" "os" "path/filepath" - "slices" "strings" "time" @@ -40,6 +39,9 @@ import ( const PrPattern = "PR: %s/%s#%d" +// maintainer list file in ProjectGit +const MaintainershipFile = "_maitnainership.json" + const ( // from Gitea // ReviewStateApproved pr is approved @@ -65,10 +67,9 @@ type Gitea interface { CreateRepositoryIfNotExist(git *GitHandler, org Organization, repoName string) (*models.Repository, error) CreatePullRequestIfNotExist(repo *models.Repository, srcId, targetId, title, body string) (*models.PullRequest, error) RequestReviews(pr *models.PullRequest, reviewer string) ([]*models.PullReview, error) - IsReviewed(pr *models.PullRequest) (bool, error) AddReviewComment(pr *models.PullRequest, state models.ReviewStateType, comment string) (*models.PullReview, error) GetAssociatedPrjGitPR(pr *PullRequestWebhookEvent) (*models.PullRequest, error) - GetRepositoryFileContent(repo *models.Repository, hash, path string) ([]byte, error) + GetRepositoryFileContent(org, repo, hash, path string) ([]byte, error) GetPullRequestFileContent(pr *models.PullRequest, path string) ([]byte, error) GetRecentPullRequests(org, repo string) ([]*models.PullRequest, error) GetRecentCommits(org, repo, branch string, commitNo int64) ([]*models.Commit, error) @@ -90,6 +91,10 @@ func AllocateGiteaTransport(host string) Gitea { return &r } +func (gitea *GiteaTransport) FetchMaintainershipFile(org, repo, branch string) ([]byte, error) { + return gitea.GetRepositoryFileContent(org, repo, branch, MaintainershipFile) +} + func (gitea *GiteaTransport) GetPullRequestAndReviews(org, project string, num int64) (*models.PullRequest, []*models.PullReview, error) { pr, err := gitea.client.Repository.RepoGetPullRequest( repository.NewRepoGetPullRequestParams(). @@ -314,56 +319,6 @@ func (gitea *GiteaTransport) RequestReviews(pr *models.PullRequest, reviewer str return review.GetPayload(), nil } -func (gitea *GiteaTransport) IsReviewed(pr *models.PullRequest) (bool, error) { - // TODO: get review from project git - reviewers := pr.RequestedReviewers - var page int64 - reviews := make([]*models.PullReview, 0, 10) - for { - page++ - res, err := gitea.client.Repository.RepoListPullReviews( - repository.NewRepoListPullReviewsParams(). - WithOwner(pr.Base.Repo.Owner.UserName). - WithRepo(pr.Base.Repo.Name). - WithPage(&page), - gitea.transport.DefaultAuthentication) - - if err != nil { - return false, err - } - - if res.IsSuccess() { - reviews = append(reviews, res.Payload...) - if len(res.Payload) < 10 { - break - } - } - } - - slices.Reverse(reviews) - - for _, review := range reviews { - if review.Stale || review.Dismissed { - continue - } - - next_review: - for i, reviewer := range reviewers { - if review.User.UserName == reviewer.UserName { - switch review.State { - case ReviewStateApproved: - reviewers = slices.Delete(reviewers, i, i) - break next_review - case ReviewStateRequestChanges: - return false, nil - } - } - } - } - - return len(reviewers) == 0, nil -} - func (gitea *GiteaTransport) AddReviewComment(pr *models.PullRequest, state models.ReviewStateType, comment string) (*models.PullReview, error) { c, err := gitea.client.Repository.RepoCreatePullReview( repository.NewRepoCreatePullReviewParams(). @@ -452,7 +407,7 @@ func (gitea *GiteaTransport) GetAssociatedPrjGitPR(pr *PullRequestWebhookEvent) return nil, nil } -func (gitea *GiteaTransport) GetRepositoryFileContent(repo *models.Repository, hash, path string) ([]byte, error) { +func (gitea *GiteaTransport) GetRepositoryFileContent(org, repo, hash, path string) ([]byte, error) { var retData []byte dataOut := writeFunc(func(data []byte) (int, error) { @@ -464,8 +419,8 @@ func (gitea *GiteaTransport) GetRepositoryFileContent(repo *models.Repository, h }) _, err := gitea.client.Repository.RepoGetRawFile( repository.NewRepoGetRawFileParams(). - WithOwner(repo.Owner.UserName). - WithRepo(repo.Name). + WithOwner(org). + WithRepo(repo). WithFilepath(path). WithRef(&hash), gitea.transport.DefaultAuthentication, @@ -481,7 +436,7 @@ func (gitea *GiteaTransport) GetRepositoryFileContent(repo *models.Repository, h } func (gitea *GiteaTransport) GetPullRequestFileContent(pr *models.PullRequest, path string) ([]byte, error) { - return gitea.GetRepositoryFileContent(pr.Head.Repo, pr.Head.Sha, path) + return gitea.GetRepositoryFileContent(pr.Head.Repo.Owner.UserName, pr.Head.Repo.Name, pr.Head.Sha, path) } func (gitea *GiteaTransport) GetRecentPullRequests(org, repo string) ([]*models.PullRequest, error) { diff --git a/workflow-pr/maintainership.go b/workflow-pr/maintainership.go index 389600c..06727cf 100644 --- a/workflow-pr/maintainership.go +++ b/workflow-pr/maintainership.go @@ -4,6 +4,7 @@ import ( "encoding/json" "src.opensuse.org/autogits/common" + "src.opensuse.org/autogits/common/gitea-generated/models" ) //go:generate mockgen -source=maintainership.go -destination=mock/maintainership.go -typed @@ -13,45 +14,40 @@ const ProjectKey = "" type MaintainershipMap map[string][]string type GiteaMaintainershipInterface interface { - FetchMaintainershipFile(org, prj, branch string) ([]byte, error) + FetchMaintainershipFile(org, prjGit, branch string) ([]byte, error) + GetPullRequestAndReviews(org, pkg string, num int64) (*models.PullRequest, []*models.PullReview, error) } -func parseMaintainerhipData(gitea GiteaMaintainershipInterface, org, prjGit, branch string) (MaintainershipMap, error) { +func parseMaintainershipData(data []byte) (MaintainershipMap, error) { + maintainers := make(MaintainershipMap) + if err := json.Unmarshal(data, &maintainers); err != nil { + return nil, err + } + + return maintainers, nil +} + +func ProjectMaintainershipData(gitea GiteaMaintainershipInterface, org, prjGit, branch string) (MaintainershipMap, error) { data, err := gitea.FetchMaintainershipFile(org, prjGit, branch) if err != nil || data == nil { return nil, err } - maintainer := make(MaintainershipMap) - if err := json.Unmarshal(data, &maintainer); err != nil { - return nil, err - } - - return maintainer, nil + return parseMaintainershipData(data) } -func MaintainerListForProject(gitea GiteaMaintainershipInterface, config common.AutogitConfig) ([]string, error) { - maintainer, err := parseMaintainerhipData(gitea, config.Organization, config.GitProjectName, config.Branch) - if err != nil { - return nil, err - } - - m, found := maintainer[ProjectKey] +func MaintainerListForProject(maintainers MaintainershipMap) []string { + m, found := maintainers[ProjectKey] if !found { - return nil, nil + return nil } - return m, nil + return m } -func MaintainerListForPackage(gitea GiteaMaintainershipInterface, config common.AutogitConfig, pkg string) ([]string, error) { - maintainer, err := parseMaintainerhipData(gitea, config.Organization, config.GitProjectName, config.Branch) - if err != nil { - return nil, err - } - - pkgMaintainers := maintainer[pkg] - prjMaintainers := maintainer[ProjectKey] +func MaintainerListForPackage(maintainers MaintainershipMap, pkg string) []string { + pkgMaintainers := maintainers[pkg] + prjMaintainers := maintainers[ProjectKey] prjMaintainer: for _, prjm := range prjMaintainers { @@ -63,5 +59,18 @@ prjMaintainer: pkgMaintainers = append(pkgMaintainers, prjm) } - return pkgMaintainers, nil + return pkgMaintainers +} + +func CheckIfMaintainersApproved(gitea GiteaMaintainershipInterface, config common.AutogitConfig, prjGitPRNumber int64) (bool, error) { + gitea.GetPullRequestAndReviews(config.Organization, config.GitProjectName, prjGitPRNumber) + gitea.FetchMaintainershipFile(config.Organization, config.GitProjectName, config.Branch) + + /* + pr, reviews, _ := gitea.GetPullRequestAndReviews(config.Organization, config.GitProjectName, prjGitPRNumber) + data, _ := gitea.FetchMaintainershipFile(config.Organization, config.GitProjectName, config.Branch) + + maintainers, _ := parseMaintainershipData(data) + */ + return false, nil } diff --git a/workflow-pr/maintainership_test.go b/workflow-pr/maintainership_test.go index 568249f..45864c9 100644 --- a/workflow-pr/maintainership_test.go +++ b/workflow-pr/maintainership_test.go @@ -6,22 +6,22 @@ import ( "go.uber.org/mock/gomock" "src.opensuse.org/autogits/common" + "src.opensuse.org/autogits/common/gitea-generated/models" mock_main "src.opensuse.org/workflow-pr/mock" ) func TestMaintainership(t *testing.T) { allocateMaintainershipInterface := func(t *testing.T) *mock_main.MockGiteaMaintainershipInterface { - t.Parallel() ctl := gomock.NewController(t) mi := mock_main.NewMockGiteaMaintainershipInterface(ctl) return mi } - config := common.AutogitConfig { - Branch: "bar", - Organization: "foo", + config := common.AutogitConfig{ + Branch: "bar", + Organization: "foo", GitProjectName: common.DefaultGitPrj, } @@ -30,8 +30,13 @@ func TestMaintainership(t *testing.T) { mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return(nil, nil) - m, err := MaintainerListForPackage(mi, config, "bar") - if len(m) != 0 || err != nil { + maintainers, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err != nil { + t.Error("Invalid number of maintainers for package", err) + } + + m := MaintainerListForPackage(maintainers, "bar") + if len(m) != 0 { t.Error("Invalid number of maintainers for package", err) } }) @@ -41,14 +46,16 @@ func TestMaintainership(t *testing.T) { mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return(nil, nil) - m, err := MaintainerListForProject(mi, config) + maintainers, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + m := MaintainerListForProject(maintainers) if len(m) != 0 || err != nil { t.Error("Invalid number of maintainers for project", err) } mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return([]byte("{}"), nil) - m, err = MaintainerListForProject(mi, config) + maintainers, err = ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + m = MaintainerListForProject(maintainers) if len(m) != 0 || err != nil { t.Error("Invalid number of maintainers for project", err) } @@ -60,7 +67,7 @@ func TestMaintainership(t *testing.T) { err := errors.New("some error here") mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return(nil, err) - _, errRet := MaintainerListForPackage(mi, config, "bar") + _, errRet := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) switch errRet { case nil: t.Error("Should have returned an error") @@ -77,7 +84,7 @@ func TestMaintainership(t *testing.T) { err := errors.New("some error here") mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return(nil, err) - _, errRet := MaintainerListForProject(mi, config) + _, errRet := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) switch errRet { case nil: t.Error("Should have returned an error") @@ -97,8 +104,12 @@ func TestMaintainership(t *testing.T) { } `), nil) - m, err := MaintainerListForProject(mi, config) - if len(m) != 2 || err != nil { + maintainers, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err != nil { + t.Error("Invalid number of maintainers for project", err) + } + m := MaintainerListForProject(maintainers) + if len(m) != 2 { t.Error("Invalid number of maintainers for project", err) } @@ -116,8 +127,12 @@ func TestMaintainership(t *testing.T) { } `), nil) - m, err := MaintainerListForProject(mi, config) - if len(m) != 1 || err != nil { + maintainers, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err != nil { + t.Error("Invalid number of maintainers for project", err) + } + m := MaintainerListForProject(maintainers) + if len(m) != 1 { t.Error("Invalid number of maintainers for project", err) } @@ -135,8 +150,8 @@ func TestMaintainership(t *testing.T) { } `), nil) - m, err := MaintainerListForProject(mi, config) - if len(m) != 0 || err == nil { + _, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err == nil { t.Error("Invalid number of maintainers for project", err) } }) @@ -150,7 +165,7 @@ func TestMaintainership(t *testing.T) { } `), nil) - _, err := MaintainerListForProject(mi, config) + _, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) if err == nil { t.Error("Invalid number of maintainers for project", err) } @@ -166,8 +181,12 @@ func TestMaintainership(t *testing.T) { } `), nil) - m, err := MaintainerListForPackage(mi, config, "pkg") - if len(m) != 3 || err != nil { + maintainers, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err != nil { + t.Error("Invalid number of maintainers for package") + } + m := MaintainerListForPackage(maintainers, "pkg") + if len(m) != 3 { t.Error("Invalid number of maintainers for package", m) } @@ -186,8 +205,13 @@ func TestMaintainership(t *testing.T) { } `), nil) - m, err := MaintainerListForPackage(mi, config, "bar") - if len(m) != 2 || err != nil { + maintainers, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err != nil { + t.Error("Invalid number of maintainers for package", maintainers) + } + + m := MaintainerListForPackage(maintainers, "bar") + if len(m) != 2 { t.Error("Invalid number of maintainers for package", m) } @@ -206,11 +230,51 @@ func TestMaintainership(t *testing.T) { } `), nil) - m, err := MaintainerListForPackage(mi, config, "pkg") - if len(m) != 0 || err == nil { + _, err := ProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) + if err == nil { t.Error("Invalid number of maintainers for project", err) } }) + t.Run("Maintainers not appoved", func(t *testing.T) { + mi := allocateMaintainershipInterface(t) + + mi.EXPECT().GetPullRequestAndReviews("foo", common.DefaultGitPrj, int64(10)).Return( + &models.PullRequest{ + Index: 10, + RequestedReviewers: []*models.User{}, + }, + []*models.PullReview{}, + nil, + ) + mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return([]byte(`{}`), nil) + + approved, err := CheckIfMaintainersApproved(mi, config, 10) + if approved || err != nil { + t.Error("Unexpected approved or err:", err) + } + }) + + t.Run("Maintainers approved", func(t *testing.T) { + mi := allocateMaintainershipInterface(t) + + mi.EXPECT().GetPullRequestAndReviews("foo", common.DefaultGitPrj, int64(10)).Return( + &models.PullRequest{ + Index: 10, + RequestedReviewers: []*models.User{}, + }, + []*models.PullReview{}, + nil, + ) + mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return([]byte(` + "": ["test"] + `), nil) + + approved, err := CheckIfMaintainersApproved(mi, config, 10) + if !approved || err != nil { + t.Error("Unexpected disapproval or err:", err) + } + }) + t.Parallel() }