From fbaeddfcd8431baceae889d6d46af3145440961fdd5354ae9e85d5f611af40b7 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Wed, 15 Jan 2025 00:46:03 +0100 Subject: [PATCH] add support for maintainership directories --- bots-common/gitea_utils.go | 11 +++- workflow-pr/maintainership.go | 60 ++++++++++++++--- workflow-pr/maintainership_test.go | 101 ++++++++++++++++++++++------- 3 files changed, 138 insertions(+), 34 deletions(-) diff --git a/bots-common/gitea_utils.go b/bots-common/gitea_utils.go index 143b129..26e82a3 100644 --- a/bots-common/gitea_utils.go +++ b/bots-common/gitea_utils.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "os" + "path" "path/filepath" "slices" "time" @@ -38,7 +39,10 @@ import ( //go:generate mockgen -source=gitea_utils.go -destination=mock/gitea_utils.go -typed // maintainer list file in ProjectGit -const MaintainershipFile = "_maitnainership.json" +const ( + MaintainershipFile = "_maitnainership.json" + MaintainershipDir = "maintaineirship" +) const ( // from Gitea @@ -58,6 +62,7 @@ const ( type GiteaMaintainershipInterface interface { FetchMaintainershipFile(org, prjGit, branch string) ([]byte, error) + FetchMaintainershipDirFile(org, prjGit, branch, pkg string) ([]byte, error) } type GiteaPRFetcher interface { @@ -115,6 +120,10 @@ func (gitea *GiteaTransport) FetchMaintainershipFile(org, repo, branch string) ( return gitea.GetRepositoryFileContent(org, repo, branch, MaintainershipFile) } +func (gitea *GiteaTransport) FetchMaintainershipDirFile(org, repo, branch, pkg string) ([]byte, error) { + return gitea.GetRepositoryFileContent(org, repo, branch, path.Join(MaintainershipDir, pkg)) +} + func (gitea *GiteaTransport) GetPullRequest(org, project string, num int64) (*models.PullRequest, error) { pr, err := gitea.client.Repository.RepoGetPullRequest( repository.NewRepoGetPullRequestParams(). diff --git a/workflow-pr/maintainership.go b/workflow-pr/maintainership.go index d5758bc..e170bbd 100644 --- a/workflow-pr/maintainership.go +++ b/workflow-pr/maintainership.go @@ -14,25 +14,44 @@ type MaintainershipData interface { } const ProjectKey = "" +const ProjectFileKey = "_project" -type MaintainershipMap map[string][]string +type MaintainershipMap struct { + data map[string][]string + is_dir bool + fetchPackage func(string) ([]byte, error) +} func parseMaintainershipData(data []byte) (*MaintainershipMap, error) { - maintainers := make(MaintainershipMap) - if err := json.Unmarshal(data, &maintainers); err != nil { + maintainers := &MaintainershipMap{ + data: make(map[string][]string), + } + if err := json.Unmarshal(data, &maintainers.data); err != nil { return nil, err } - return &maintainers, nil + return maintainers, nil } func FetchProjectMaintainershipData(gitea common.GiteaMaintainershipInterface, org, prjGit, branch string) (*MaintainershipMap, error) { - data, err := gitea.FetchMaintainershipFile(org, prjGit, branch) + data, err := gitea.FetchMaintainershipDirFile(org, prjGit, branch, ProjectFileKey) + dir := true if err != nil || data == nil { - return nil, err + dir = false + data, err = gitea.FetchMaintainershipFile(org, prjGit, branch) + if err != nil || data == nil { + return nil, err + } } - return parseMaintainershipData(data) + m, err := parseMaintainershipData(data) + if m != nil { + m.is_dir = dir + m.fetchPackage = func(pkg string) ([]byte, error) { + return gitea.FetchMaintainershipDirFile(org, prjGit, branch, pkg) + } + } + return m, err } func (data *MaintainershipMap) ListProjectMaintainers() []string { @@ -40,7 +59,7 @@ func (data *MaintainershipMap) ListProjectMaintainers() []string { return nil } - m, found := (*data)[ProjectKey] + m, found := data.data[ProjectKey] if !found { return nil } @@ -48,12 +67,34 @@ func (data *MaintainershipMap) ListProjectMaintainers() []string { return m } +func parsePkgDirData(pkg string, data []byte) []string { + m := make(map[string][]string) + if err := json.Unmarshal(data, &m); err != nil { + return nil + } + + pkgMaintainers, found := m[pkg] + if !found { + return nil + } + return pkgMaintainers +} + func (data *MaintainershipMap) ListPackageMaintainers(pkg string) []string { if data == nil { return nil } - pkgMaintainers := (*data)[pkg] + pkgMaintainers, found := data.data[pkg] + if !found && data.is_dir { + pkgData, err := data.fetchPackage(pkg) + if err == nil { + pkgMaintainers = parsePkgDirData(pkg, pkgData) + if len(pkgMaintainers) > 0 { + data.data[pkg] = pkgMaintainers + } + } + } prjMaintainers := data.ListProjectMaintainers() prjMaintainer: @@ -68,4 +109,3 @@ prjMaintainer: return pkgMaintainers } - diff --git a/workflow-pr/maintainership_test.go b/workflow-pr/maintainership_test.go index d31f2d9..075a50f 100644 --- a/workflow-pr/maintainership_test.go +++ b/workflow-pr/maintainership_test.go @@ -18,44 +18,57 @@ func TestMaintainership(t *testing.T) { } packageTests := []struct { - name string + name string + maintainers []string + otherError bool + packageName string + maintainersFile []byte maintainersFileErr error - maintainers []string - otherError bool - packageName string + + maintainersDir map[string][]byte }{ /* PACKAGE MAINTAINERS */ - // package tests have packageName, projects do not + // package tests have packageName, projects do not { - name: "No maintainer in empty package", + name: "No maintainer in empty package", packageName: "foo", }, { name: "Error in MaintainerListForPackage when remote has an error", maintainersFileErr: errors.New("some error here"), - packageName: "foo", + packageName: "foo", }, { name: "Multiple package maintainers", maintainersFile: []byte(`{"pkg": ["user1", "user2"], "": ["user1", "user3"]}`), - maintainers: []string{"user1", "user2", "user3"}, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": ["user1", "user3"]}`), + "pkg": []byte(`{"pkg": ["user1", "user2"]}`), + }, + maintainers: []string{"user1", "user2", "user3"}, packageName: "pkg", }, { name: "No package maintainers and only project maintainer", maintainersFile: []byte(`{"pkg2": ["user1", "user2"], "": ["user1", "user3"]}`), - maintainers: []string{"user1", "user3"}, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": ["user1", "user3"]}`), + }, + maintainers: []string{"user1", "user3"}, packageName: "pkg", }, { name: "Invalid list of package maintainers", maintainersFile: []byte(`{"pkg": 3,"": ["user", 4]}`), - otherError: true, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": ["user1", 4]}`), + "pkg": []byte(`"pkg": 3`), + }, + otherError: true, packageName: "pkg", }, - /* PROJECT MAINTAINERS */ { name: "No maintainer for empty project", @@ -63,6 +76,9 @@ func TestMaintainership(t *testing.T) { { name: "No maintainer for empty project maintainer file", maintainersFile: []byte("{}"), + maintainersDir: map[string][]byte{ + "_project": []byte(`{}`), + }, }, { name: "Error in MaintainerListForProject when remote has an error", @@ -71,33 +87,40 @@ func TestMaintainership(t *testing.T) { { name: "Multiple project maintainers", maintainersFile: []byte(`{"": ["user1", "user2"]}`), - maintainers: []string{"user1", "user2"}, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": ["user1", "user2"]}`), + }, + maintainers: []string{"user1", "user2"}, }, { name: "Single project maintainer", maintainersFile: []byte(`{"": ["user"]}`), - maintainers: []string{"user"}, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": ["user"]}`), + }, + maintainers: []string{"user"}, }, { name: "Invalid list of project maintainers", maintainersFile: []byte(`{"": ["user", 4]}`), - otherError: true, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": ["user", 4]}`), + }, + otherError: true, }, { name: "Invalid list of project maintainers", maintainersFile: []byte(`{"": 4}`), - otherError: true, + maintainersDir: map[string][]byte{ + "_project": []byte(`{"": 4}`), + }, + otherError: true, }, } + notFoundError := errors.New("not found") for _, test := range packageTests { - t.Run(test.name, func(t *testing.T) { - ctl := gomock.NewController(t) - mi := mock_common.NewMockGiteaMaintainershipInterface(ctl) - - mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar"). - Return(test.maintainersFile, test.maintainersFileErr) - + runTests := func(t *testing.T, mi common.GiteaMaintainershipInterface) { maintainers, err := FetchProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch) if err != nil && !test.otherError { if test.maintainersFileErr == nil { @@ -119,14 +142,46 @@ func TestMaintainership(t *testing.T) { } if len(m) != len(test.maintainers) { - t.Error("Invalid number of maintainers for package", err) + t.Error("Invalid number of maintainers for package", test.packageName, len(m), "vs", len(test.maintainers)) } for i := range m { if !slices.Contains(test.maintainers, m[i]) { t.Fatal("Can't find expected users. Found:", m) } } + } + + t.Run(test.name+"_File", func(t *testing.T) { + ctl := gomock.NewController(t) + mi := mock_common.NewMockGiteaMaintainershipInterface(ctl) + + // tests with maintainership file + mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar"). + Return(test.maintainersFile, test.maintainersFileErr) + mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", ProjectFileKey). + Return(nil, notFoundError) + + runTests(t, mi) + }) + + t.Run(test.name+"_Dir", func(t *testing.T) { + ctl := gomock.NewController(t) + mi := mock_common.NewMockGiteaMaintainershipInterface(ctl) + + // run same tests with directory maintainership data + for filename, data := range test.maintainersDir { + mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", filename).Return(data, test.maintainersFileErr).AnyTimes() + } + if _, found := test.maintainersDir[ProjectFileKey]; !found { + mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", ProjectFileKey).Return(nil, test.maintainersFileErr).AnyTimes() + mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return(nil, test.maintainersFileErr).AnyTimes() + } + mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", gomock.Any()).Return(nil, notFoundError).AnyTimes() + + runTests(t, mi) }) } } +func TestMaintainershipDir(t *testing.T) { +}