From 07479dd6455184e1c9271d9d079c3e7c3005f969 Mon Sep 17 00:00:00 2001 From: Alconety Date: Fri, 19 Jan 2024 04:15:47 +0100 Subject: [PATCH] Retrieve all user repo perms with a single API call (#3211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pull request addresses the issue https://github.com/woodpecker-ci/woodpecker/issues/3210. Ideally, the Bitbucket API should include repository permissions when utilizing the 'get all repositories' endpoint. However, as it currently does not provide this information, a viable solution is to fetch all permissions for every repository and then employ a dictionary to associate each repository with its respective permissions. Without implementing this fix, logging in becomes problematic for users with access to a substantial number of repositories (300+), as the process takes over 2 minutes to complete. --------- Co-authored-by: Alberto Alcón --- server/forge/bitbucket/bitbucket.go | 17 +++++--- server/forge/bitbucket/fixtures/handler.go | 49 +++++++++++++--------- server/forge/bitbucket/internal/client.go | 22 +++++++++- server/forge/bitbucket/internal/types.go | 1 + 4 files changed, 63 insertions(+), 26 deletions(-) diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index 5965330c1..9247de60c 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -203,6 +203,16 @@ func (c *config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error return nil, err } + userPermisions, err := client.ListPermissionsAll() + if err != nil { + return nil, err + } + + userPermissionsByRepo := make(map[string]*internal.RepoPerm) + for _, permission := range userPermisions { + userPermissionsByRepo[permission.Repo.FullName] = permission + } + var all []*model.Repo for _, workspace := range workspaces { repos, err := client.ListReposAll(workspace.Slug) @@ -210,12 +220,9 @@ func (c *config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error return nil, err } for _, repo := range repos { - perm, err := client.GetPermission(repo.FullName) - if err != nil { - return nil, err + if perm, ok := userPermissionsByRepo[repo.FullName]; ok { + all = append(all, convertRepo(repo, perm)) } - - all = append(all, convertRepo(repo, perm)) } } return all, nil diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go index 4a7f8a985..5e65e48e4 100644 --- a/server/forge/bitbucket/fixtures/handler.go +++ b/server/forge/bitbucket/fixtures/handler.go @@ -185,21 +185,11 @@ func getUserRepos(c *gin.Context) { } } -func permission(p string) string { - return fmt.Sprintf(permissionPayload, p) -} - func getPermissions(c *gin.Context) { - query := c.Request.URL.Query()["q"][0] - switch query { - case `repository.full_name="test_name/permission_read"`: - c.String(200, permission("read")) - case `repository.full_name="test_name/permission_write"`: - c.String(200, permission("write")) - case `repository.full_name="test_name/permission_admin"`: - c.String(200, permission("admin")) - default: - c.String(200, permission("read")) + if c.Query("page") == "" || c.Query("page") == "1" { + c.String(200, permissionsPayLoad) + } else { + c.String(200, "{\"values\":[]}") } } @@ -367,14 +357,35 @@ const workspacesPayload = ` } ` -const permissionPayload = ` +const permissionsPayLoad = ` { - "pagelen": 1, + "pagelen": 100, + "page": 1, "values": [ { - "permission": "%s" + "repository": { + "full_name": "test_name/repo_name" + }, + "permission": "read" + }, + { + "repository": { + "full_name": "test_name/permission_read" + }, + "permission": "read" + }, + { + "repository": { + "full_name": "test_name/permission_write" + }, + "permission": "write" + }, + { + "repository": { + "full_name": "test_name/permission_admin" + }, + "permission": "admin" } - ], - "page": 1 + ] } ` diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go index 38028f0ab..03b4b6e4a 100644 --- a/server/forge/bitbucket/internal/client.go +++ b/server/forge/bitbucket/internal/client.go @@ -38,7 +38,8 @@ const ( const ( pathUser = "%s/2.0/user/" pathEmails = "%s/2.0/user/emails" - pathPermissions = "%s/2.0/user/permissions/repositories?q=repository.full_name=%q" + pathPermission = "%s/2.0/user/permissions/repositories?q=repository.full_name=%q" + pathPermissions = "%s/2.0/user/permissions/repositories?%s" pathWorkspaces = "%s/2.0/workspaces/?%s" pathWorkspace = "%s/2.0/workspaces/%s" pathRepo = "%s/2.0/repositories/%s/%s" @@ -161,7 +162,7 @@ func (c *Client) CreateStatus(owner, name, revision string, status *PipelineStat func (c *Client) GetPermission(fullName string) (*RepoPerm, error) { out := new(RepoPermResp) - uri := fmt.Sprintf(pathPermissions, c.base, fullName) + uri := fmt.Sprintf(pathPermission, c.base, fullName) _, err := c.do(uri, get, nil, out) if err != nil { return nil, err @@ -173,6 +174,23 @@ func (c *Client) GetPermission(fullName string) (*RepoPerm, error) { return out.Values[0], nil } +func (c *Client) ListPermissions(opts *ListOpts) (*RepoPermResp, error) { + out := new(RepoPermResp) + uri := fmt.Sprintf(pathPermissions, c.base, opts.Encode()) + _, err := c.do(uri, get, nil, out) + return out, err +} + +func (c *Client) ListPermissionsAll() ([]*RepoPerm, error) { + return shared_utils.Paginate(func(page int) ([]*RepoPerm, error) { + resp, err := c.ListPermissions(&ListOpts{Page: page, PageLen: 100}) + if err != nil { + return nil, err + } + return resp.Values, nil + }) +} + func (c *Client) ListBranches(owner, name string, opts *ListOpts) ([]*Branch, error) { out := new(BranchResp) uri := fmt.Sprintf(pathBranches, c.base, owner, name, opts.Encode()) diff --git a/server/forge/bitbucket/internal/types.go b/server/forge/bitbucket/internal/types.go index 8bb4b10dd..670959b4d 100644 --- a/server/forge/bitbucket/internal/types.go +++ b/server/forge/bitbucket/internal/types.go @@ -254,6 +254,7 @@ type RepoPermResp struct { type RepoPerm struct { Permission string `json:"permission"` + Repo Repo `json:"repository"` } type BranchResp struct {