From b2dacaf258465d1992e538b1f671c040488da0e0 Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Thu, 14 Sep 2023 10:41:40 +0200 Subject: [PATCH] *: add cursor fetching to GetOrgMembers api. --- cmd/agola/cmd/orgmemberlist.go | 37 ++++-- go.mod | 2 +- go.sum | 2 + internal/services/configstore/action/org.go | 51 ++++++-- internal/services/configstore/api/api.go | 48 ++++++- internal/services/configstore/api/org.go | 46 +++++-- .../services/configstore/configstore_test.go | 115 +++++++++++++++++ internal/services/configstore/db/db.go | 30 +++-- internal/services/gateway/action/action.go | 7 + internal/services/gateway/action/auth.go | 2 +- internal/services/gateway/action/cursor.go | 38 ++++++ internal/services/gateway/action/org.go | 46 ++++++- internal/services/gateway/api/api.go | 65 ++++++++++ internal/services/gateway/api/org.go | 32 +++-- internal/services/gateway/gateway.go | 4 +- services/configstore/client/client.go | 21 ++- services/gateway/client/client.go | 7 +- tests/setup_test.go | 121 +++++++++++++++++- 18 files changed, 604 insertions(+), 70 deletions(-) create mode 100644 internal/services/gateway/action/cursor.go diff --git a/cmd/agola/cmd/orgmemberlist.go b/cmd/agola/cmd/orgmemberlist.go index 41bf2cb0c..097ee37e7 100644 --- a/cmd/agola/cmd/orgmemberlist.go +++ b/cmd/agola/cmd/orgmemberlist.go @@ -23,6 +23,7 @@ import ( "github.com/sorintlab/errors" "github.com/spf13/cobra" + gwapitypes "agola.io/agola/services/gateway/api/types" gwclient "agola.io/agola/services/gateway/client" ) @@ -54,19 +55,39 @@ func init() { cmdOrgMember.AddCommand(cmdOrgMemberList) } +func printOrgMembers(orgMembers []*gwapitypes.OrgMemberResponse) error { + for _, orgMember := range orgMembers { + out, err := json.MarshalIndent(orgMember, "", "\t") + if err != nil { + return errors.WithStack(err) + } + os.Stdout.Write(out) + } + + return nil +} + func orgMemberList(cmd *cobra.Command, args []string) error { gwClient := gwclient.NewClient(gatewayURL, token) - orgMembers, _, err := gwClient.GetOrgMembers(context.TODO(), orgMemberListOpts.orgname) - if err != nil { - return errors.Wrapf(err, "failed to get organization member") - } + var cursor string + orgMembers := []*gwapitypes.OrgMemberResponse{} + for { + orgMembersResp, resp, err := gwClient.GetOrgMembers(context.TODO(), orgMemberListOpts.orgname, &gwclient.ListOptions{Cursor: cursor}) + if err != nil { + return errors.Wrapf(err, "failed to get organization member") + } - out, err := json.MarshalIndent(orgMembers, "", "\t") - if err != nil { - return errors.WithStack(err) + orgMembers = append(orgMembers, orgMembersResp.Members...) + if err := printOrgMembers(orgMembersResp.Members); err != nil { + return errors.WithStack(err) + } + + cursor = resp.Cursor + if cursor == "" { + break + } } - os.Stdout.Write(out) return nil } diff --git a/go.mod b/go.mod index 6b79c366d..f3591e93c 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/gorilla/mux v1.8.0 github.com/gorilla/securecookie v1.1.1 github.com/hashicorp/go-sockaddr v1.0.2 - github.com/huandu/go-sqlbuilder v1.21.0 + github.com/huandu/go-sqlbuilder v1.22.0 github.com/huandu/xstrings v1.4.0 github.com/iancoleman/strcase v0.2.0 github.com/lib/pq v1.10.7 diff --git a/go.sum b/go.sum index 734720aa1..87cae9bb7 100644 --- a/go.sum +++ b/go.sum @@ -204,6 +204,8 @@ github.com/huandu/go-assert v1.1.5 h1:fjemmA7sSfYHJD7CUqs9qTwwfdNAx7/j2/ZlHXzNB3 github.com/huandu/go-assert v1.1.5/go.mod h1:yOLvuqZwmcHIC5rIzrBhT7D3Q9c3GFnd0JrPVhn/06U= github.com/huandu/go-sqlbuilder v1.21.0 h1:+NLH8PQg5/WGMXJLIpAXTdoH1pv9Q3BU6w4P7OabBmc= github.com/huandu/go-sqlbuilder v1.21.0/go.mod h1:nUVmMitjOmn/zacMLXT0d3Yd3RHoO2K+vy906JzqxMI= +github.com/huandu/go-sqlbuilder v1.22.0 h1:69SpvXvhAoeb7y5uERUCB0/Ck09DwQ6ccYovejm1zHA= +github.com/huandu/go-sqlbuilder v1.22.0/go.mod h1:nUVmMitjOmn/zacMLXT0d3Yd3RHoO2K+vy906JzqxMI= github.com/huandu/xstrings v1.3.2/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= github.com/huandu/xstrings v1.3.3/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= github.com/huandu/xstrings v1.4.0 h1:D17IlohoQq4UcpqD7fDk80P7l+lwAmlFaBHgOipl2FU= diff --git a/internal/services/configstore/action/org.go b/internal/services/configstore/action/org.go index b7d0310f3..07abd6b91 100644 --- a/internal/services/configstore/action/org.go +++ b/internal/services/configstore/action/org.go @@ -26,43 +26,70 @@ import ( "agola.io/agola/services/configstore/types" ) -type OrgMemberResponse struct { +type OrgMember struct { User *types.User Role types.MemberRole } -func orgMemberResponse(orgUser *db.OrgUser) *OrgMemberResponse { - return &OrgMemberResponse{ +func orgMemberResponse(orgUser *db.OrgUser) *OrgMember { + return &OrgMember{ User: orgUser.User, Role: orgUser.Role, } } -func (h *ActionHandler) GetOrgMembers(ctx context.Context, orgRef string) ([]*OrgMemberResponse, error) { - var orgUsers []*db.OrgUser +type GetOrgMembersRequest struct { + OrgRef string + StartUserName string + + Limit int + SortDirection types.SortDirection +} + +type GetOrgMembersResponse struct { + OrgMembers []*OrgMember + + HasMore bool +} + +func (h *ActionHandler) GetOrgMembers(ctx context.Context, req *GetOrgMembersRequest) (*GetOrgMembersResponse, error) { + limit := req.Limit + if limit > 0 { + limit += 1 + } + + var dbOrgMembers []*db.OrgUser err := h.d.Do(ctx, func(tx *sql.Tx) error { var err error - org, err := h.d.GetOrg(tx, orgRef) + org, err := h.d.GetOrg(tx, req.OrgRef) if err != nil { return errors.WithStack(err) } if org == nil { - return util.NewAPIError(util.ErrNotExist, errors.Errorf("org %q doesn't exist", orgRef)) + return util.NewAPIError(util.ErrNotExist, errors.Errorf("org %q doesn't exist", req.OrgRef)) } - orgUsers, err = h.d.GetOrgUsers(tx, org.ID) + dbOrgMembers, err = h.d.GetOrgMembers(tx, org.ID, req.StartUserName, limit, req.SortDirection) return errors.WithStack(err) }) if err != nil { return nil, errors.WithStack(err) } - res := make([]*OrgMemberResponse, len(orgUsers)) - for i, orgUser := range orgUsers { - res[i] = orgMemberResponse(orgUser) + orgMembers := make([]*OrgMember, len(dbOrgMembers)) + for i, orgUser := range dbOrgMembers { + orgMembers[i] = orgMemberResponse(orgUser) + } + + hasMore := len(orgMembers) > req.Limit + if hasMore { + orgMembers = orgMembers[0:req.Limit] } - return res, nil + return &GetOrgMembersResponse{ + OrgMembers: orgMembers, + HasMore: hasMore, + }, nil } type CreateOrgRequest struct { diff --git a/internal/services/configstore/api/api.go b/internal/services/configstore/api/api.go index da04ca0ee..6ed16be3c 100644 --- a/internal/services/configstore/api/api.go +++ b/internal/services/configstore/api/api.go @@ -17,6 +17,7 @@ package api import ( "net/http" "net/url" + "strconv" "github.com/gorilla/mux" "github.com/sorintlab/errors" @@ -25,9 +26,9 @@ import ( "agola.io/agola/services/configstore/types" ) -type ErrorResponse struct { - Message string `json:"message"` -} +const ( + agolaHasMoreHeader = "X-Agola-HasMore" +) func GetObjectKindRef(r *http.Request) (types.ObjectKind, string, error) { vars := mux.Vars(r) @@ -49,3 +50,44 @@ func GetObjectKindRef(r *http.Request) (types.ObjectKind, string, error) { return "", "", util.NewAPIError(util.ErrBadRequest, errors.Errorf("cannot get project or projectgroup ref")) } + +type requestOptions struct { + Limit int + SortDirection types.SortDirection +} + +func parseRequestOptions(r *http.Request) (*requestOptions, error) { + query := r.URL.Query() + + limit := 0 + limitS := query.Get("limit") + if limitS != "" { + var err error + limit, err = strconv.Atoi(limitS) + if err != nil { + return nil, util.NewAPIError(util.ErrBadRequest, errors.Wrapf(err, "cannot parse limit")) + } + } + if limit < 0 { + return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("limit must be greater or equal than 0")) + } + + sortDirection := types.SortDirection(query.Get("sortdirection")) + if sortDirection != "" { + switch sortDirection { + case types.SortDirectionAsc: + case types.SortDirectionDesc: + default: + return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("wrong sort direction %q", sortDirection)) + } + } + + return &requestOptions{ + Limit: limit, + SortDirection: sortDirection, + }, nil +} + +func addHasMoreHeader(w http.ResponseWriter, hasMore bool) { + w.Header().Add(agolaHasMoreHeader, strconv.FormatBool(hasMore)) +} diff --git a/internal/services/configstore/api/org.go b/internal/services/configstore/api/org.go index dd8239f32..6c38fceab 100644 --- a/internal/services/configstore/api/org.go +++ b/internal/services/configstore/api/org.go @@ -284,7 +284,7 @@ func (h *RemoveOrgMemberHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques } } -func orgMemberResponse(orgUser *action.OrgMemberResponse) *csapitypes.OrgMemberResponse { +func orgMemberResponse(orgUser *action.OrgMember) *csapitypes.OrgMemberResponse { return &csapitypes.OrgMemberResponse{ User: orgUser.User, Role: orgUser.Role, @@ -301,24 +301,50 @@ func NewOrgMembersHandler(log zerolog.Logger, ah *action.ActionHandler) *OrgMemb } func (h *OrgMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + res, err := h.do(w, r) + if util.HTTPError(w, err) { + h.log.Err(err).Send() + return + } + + if err := util.HTTPResponse(w, http.StatusOK, res); err != nil { + h.log.Err(err).Send() + } +} + +func (h *OrgMembersHandler) do(w http.ResponseWriter, r *http.Request) ([]*csapitypes.OrgMemberResponse, error) { ctx := r.Context() + query := r.URL.Query() vars := mux.Vars(r) orgRef := vars["orgref"] - orgUsers, err := h.ah.GetOrgMembers(ctx, orgRef) - if util.HTTPError(w, err) { - h.log.Err(err).Send() - return + ropts, err := parseRequestOptions(r) + if err != nil { + return nil, errors.WithStack(err) } - res := make([]*csapitypes.OrgMemberResponse, len(orgUsers)) - for i, orgUser := range orgUsers { - res[i] = orgMemberResponse(orgUser) + startUserName := query.Get("startusername") + + areq := &action.GetOrgMembersRequest{ + OrgRef: orgRef, + StartUserName: startUserName, + + Limit: ropts.Limit, + SortDirection: ropts.SortDirection, + } + ares, err := h.ah.GetOrgMembers(ctx, areq) + if err != nil { + return nil, errors.WithStack(err) } - if err := util.HTTPResponse(w, http.StatusOK, res); err != nil { - h.log.Err(err).Send() + res := make([]*csapitypes.OrgMemberResponse, len(ares.OrgMembers)) + for i, orgMember := range ares.OrgMembers { + res[i] = orgMemberResponse(orgMember) } + + addHasMoreHeader(w, ares.HasMore) + + return res, nil } type OrgInvitationsHandler struct { diff --git a/internal/services/configstore/configstore_test.go b/internal/services/configstore/configstore_test.go index eb5b6a868..1f2f43710 100644 --- a/internal/services/configstore/configstore_test.go +++ b/internal/services/configstore/configstore_test.go @@ -1067,6 +1067,121 @@ func TestOrgMembers(t *testing.T) { }) } +func TestGetOrgMembers(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + ctx := context.Background() + log := testutil.NewLogger(t) + + cs := setupConfigstore(ctx, t, log, dir) + + t.Logf("starting cs") + go func() { _ = cs.Run(ctx) }() + + users := []*types.User{} + for i := 1; i < 10; i++ { + user, err := cs.ah.CreateUser(ctx, &action.CreateUserRequest{UserName: fmt.Sprintf("user%d", i)}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + users = append(users, user) + } + + org, err := cs.ah.CreateOrg(ctx, &action.CreateOrgRequest{Name: "org01", Visibility: types.VisibilityPublic, CreatorUserID: users[0].ID}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + for _, user := range users { + if _, err := cs.ah.AddOrgMember(ctx, org.ID, user.ID, types.MemberRoleMember); err != nil { + t.Fatalf("unexpected err: %v", err) + } + } + + t.Run("test get all org members", func(t *testing.T) { + res, err := cs.ah.GetOrgMembers(ctx, &action.GetOrgMembersRequest{OrgRef: org.ID, SortDirection: types.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + expectedOrgMembers := 9 + if len(res.OrgMembers) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.OrgMembers)) + } + if res.HasMore { + t.Fatalf("expected hasMore false, got %t", res.HasMore) + } + }) + + t.Run("test get org members with limit less than org members", func(t *testing.T) { + res, err := cs.ah.GetOrgMembers(ctx, &action.GetOrgMembersRequest{OrgRef: org.ID, Limit: 5, SortDirection: types.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + expectedOrgMembers := 5 + if len(res.OrgMembers) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.OrgMembers)) + } + if !res.HasMore { + t.Fatalf("expected hasMore true, got %t", res.HasMore) + } + }) + + t.Run("test get org members with limit less than org members continuation", func(t *testing.T) { + orgMembers := []*action.OrgMember{} + + res, err := cs.ah.GetOrgMembers(ctx, &action.GetOrgMembersRequest{OrgRef: org.ID, Limit: 5, SortDirection: types.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + expectedOrgMembers := 5 + if len(res.OrgMembers) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.OrgMembers)) + } + if !res.HasMore { + t.Fatalf("expected hasMore true, got %t", res.HasMore) + } + + orgMembers = append(orgMembers, res.OrgMembers...) + lastUser := res.OrgMembers[len(res.OrgMembers)-1] + + // fetch next results + for { + res, err = cs.ah.GetOrgMembers(ctx, &action.GetOrgMembersRequest{OrgRef: org.ID, StartUserName: lastUser.User.Name, Limit: 5, SortDirection: types.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + expectedOrgMembers := 5 + if res.HasMore && len(res.OrgMembers) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.OrgMembers)) + } + + orgMembers = append(orgMembers, res.OrgMembers...) + + if !res.HasMore { + break + } + + lastUser = res.OrgMembers[len(res.OrgMembers)-1] + } + + expectedOrgMembers = 9 + if len(orgMembers) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(orgMembers)) + } + + orgMemberUsers := []*types.User{} + for _, orgMember := range orgMembers { + orgMemberUsers = append(orgMemberUsers, orgMember.User) + + } + if diff := cmpDiffObject(users, orgMemberUsers); diff != "" { + t.Fatalf("mismatch (-want +got):\n%s", diff) + } + }) +} + func TestRemoteSource(t *testing.T) { t.Parallel() diff --git a/internal/services/configstore/db/db.go b/internal/services/configstore/db/db.go index bd84a5b03..433935bef 100644 --- a/internal/services/configstore/db/db.go +++ b/internal/services/configstore/db/db.go @@ -476,14 +476,6 @@ func (d *DB) GetOrgs(tx *sql.Tx, startOrgName string, limit int, asc bool) ([]*t return orgs, errors.WithStack(err) } -func (d *DB) GetOrgMembers(tx *sql.Tx, orgID string) ([]*types.OrganizationMember, error) { - q := organizationMemberSelect() - q.Where(q.E("organization_id", orgID)) - orgMembers, _, err := d.fetchOrganizationMembers(tx, q) - - return orgMembers, errors.WithStack(err) -} - func (d *DB) GetOrgMemberByOrgUserID(tx *sql.Tx, orgID, userID string) (*types.OrganizationMember, error) { q := organizationMemberSelect() q.Where(q.E("orgmember.organization_id", orgID), q.E("orgmember.user_id", userID)) @@ -534,8 +526,7 @@ type OrgUser struct { Role types.MemberRole } -// TODO(sgotti) implement cursor fetching -func (d *DB) GetOrgUsers(tx *sql.Tx, orgID string) ([]*OrgUser, error) { +func (d *DB) GetOrgMembers(tx *sql.Tx, orgID string, startUserName string, limit int, sortDirection types.SortDirection) ([]*OrgUser, error) { cols := organizationMemberSelectColumns() cols = append(cols, userSelectColumns()...) @@ -543,6 +534,25 @@ func (d *DB) GetOrgUsers(tx *sql.Tx, orgID string) ([]*OrgUser, error) { q = q.Join("user_t", "user_t.id = orgmember.user_id") q = q.Where(q.E("orgmember.organization_id", orgID)) q = q.OrderBy("user_t.name") + switch sortDirection { + case types.SortDirectionAsc: + q.Asc() + case types.SortDirectionDesc: + q.Desc() + } + + if startUserName != "" { + switch sortDirection { + case types.SortDirectionAsc: + q = q.Where(q.G("user_t.name", startUserName)) + case types.SortDirectionDesc: + q = q.Where(q.L("user_t.name", startUserName)) + } + } + + if limit > 0 { + q = q.Limit(limit) + } rows, err := d.query(tx, q) if err != nil { diff --git a/internal/services/gateway/action/action.go b/internal/services/gateway/action/action.go index 639517fbd..2736ea746 100644 --- a/internal/services/gateway/action/action.go +++ b/internal/services/gateway/action/action.go @@ -22,6 +22,13 @@ import ( rsclient "agola.io/agola/services/runservice/client" ) +type SortDirection string + +const ( + SortDirectionAsc SortDirection = "asc" + SortDirectionDesc SortDirection = "desc" +) + type ActionHandler struct { log zerolog.Logger sd *scommon.TokenSigningData diff --git a/internal/services/gateway/action/auth.go b/internal/services/gateway/action/auth.go index 8d5f99103..7230fab32 100644 --- a/internal/services/gateway/action/auth.go +++ b/internal/services/gateway/action/auth.go @@ -228,7 +228,7 @@ func (h *ActionHandler) IsOrgMember(ctx context.Context, userRef, orgRef string) return false, errors.Wrapf(err, "failed to get user %s:", userRef) } - orgMembers, err := h.GetOrgMembers(ctx, orgRef) + orgMembers, err := h.GetOrgMembers(ctx, &GetOrgMembersRequest{OrgRef: orgRef}) if err != nil { return false, errors.Wrapf(err, "failed to get org %s members:", orgRef) } diff --git a/internal/services/gateway/action/cursor.go b/internal/services/gateway/action/cursor.go new file mode 100644 index 000000000..8be4b3c84 --- /dev/null +++ b/internal/services/gateway/action/cursor.go @@ -0,0 +1,38 @@ +package action + +import ( + "encoding/base64" + "encoding/json" + + "github.com/sorintlab/errors" + + util "agola.io/agola/internal/util" +) + +func MarshalCursor(c any) (string, error) { + cj, err := json.Marshal(c) + if err != nil { + return "", errors.WithStack(err) + } + + return base64.StdEncoding.EncodeToString(cj), nil +} + +func UnmarshalCursor(cs string, c any) error { + cj, err := base64.StdEncoding.DecodeString(cs) + if err != nil { + return util.NewAPIError(util.ErrBadRequest, err) + } + + if err := json.Unmarshal(cj, c); err != nil { + return util.NewAPIError(util.ErrBadRequest, err) + } + + return nil +} + +type StartCursor struct { + Start string + + SortDirection SortDirection +} diff --git a/internal/services/gateway/action/org.go b/internal/services/gateway/action/org.go index 5f92efb84..c1f4c1a7c 100644 --- a/internal/services/gateway/action/org.go +++ b/internal/services/gateway/action/org.go @@ -22,6 +22,7 @@ import ( "agola.io/agola/internal/services/gateway/common" "agola.io/agola/internal/util" csapitypes "agola.io/agola/services/configstore/api/types" + "agola.io/agola/services/configstore/client" cstypes "agola.io/agola/services/configstore/types" ) @@ -47,9 +48,13 @@ func (h *ActionHandler) GetOrgs(ctx context.Context, req *GetOrgsRequest) ([]*cs return orgs, nil } -type OrgMembersResponse struct { - Organization *cstypes.Organization - Members []*OrgMemberResponse +type GetOrgMembersRequest struct { + OrgRef string + + Cursor string + + Limit int + SortDirection SortDirection } type OrgMemberResponse struct { @@ -57,27 +62,54 @@ type OrgMemberResponse struct { Role cstypes.MemberRole } -func (h *ActionHandler) GetOrgMembers(ctx context.Context, orgRef string) (*OrgMembersResponse, error) { - org, _, err := h.configstoreClient.GetOrg(ctx, orgRef) +type GetOrgMembersResponse struct { + Organization *cstypes.Organization + Members []*OrgMemberResponse + Cursor string +} + +func (h *ActionHandler) GetOrgMembers(ctx context.Context, req *GetOrgMembersRequest) (*GetOrgMembersResponse, error) { + inCursor := &StartCursor{} + sortDirection := req.SortDirection + if req.Cursor != "" { + if err := UnmarshalCursor(req.Cursor, inCursor); err != nil { + return nil, errors.WithStack(err) + } + sortDirection = inCursor.SortDirection + } + + org, _, err := h.configstoreClient.GetOrg(ctx, req.OrgRef) if err != nil { return nil, util.NewAPIError(util.KindFromRemoteError(err), err) } - orgMembers, _, err := h.configstoreClient.GetOrgMembers(ctx, orgRef) + orgMembers, resp, err := h.configstoreClient.GetOrgMembers(ctx, req.OrgRef, &client.GetOrgMembersOptions{ListOptions: &client.ListOptions{Limit: req.Limit, SortDirection: cstypes.SortDirection(sortDirection)}, StartUserName: inCursor.Start}) if err != nil { return nil, util.NewAPIError(util.KindFromRemoteError(err), err) } - res := &OrgMembersResponse{ + var outCursor string + if resp.HasMore && len(orgMembers) > 0 { + lastUserName := orgMembers[len(orgMembers)-1].User.Name + outCursor, err = MarshalCursor(&StartCursor{Start: lastUserName}) + if err != nil { + return nil, errors.WithStack(err) + } + } + + res := &GetOrgMembersResponse{ Organization: org, Members: make([]*OrgMemberResponse, len(orgMembers)), + Cursor: outCursor, } + for i, orgMember := range orgMembers { res.Members[i] = &OrgMemberResponse{ User: orgMember.User, Role: orgMember.Role, } } + return res, nil } diff --git a/internal/services/gateway/api/api.go b/internal/services/gateway/api/api.go index f4c7fbab6..2bf3d17a0 100644 --- a/internal/services/gateway/api/api.go +++ b/internal/services/gateway/api/api.go @@ -17,12 +17,23 @@ package api import ( "net/http" "net/url" + "strconv" "github.com/gorilla/mux" "github.com/sorintlab/errors" util "agola.io/agola/internal/util" cstypes "agola.io/agola/services/configstore/types" + gwapitypes "agola.io/agola/services/gateway/api/types" +) + +const ( + DefaultLimit = 30 + MaxLimit = 30 +) + +const ( + agolaCursorHeader = "X-Agola-Cursor" ) func GetConfigTypeRef(r *http.Request) (cstypes.ObjectKind, string, error) { @@ -45,3 +56,57 @@ func GetConfigTypeRef(r *http.Request) (cstypes.ObjectKind, string, error) { return "", "", util.NewAPIError(util.ErrBadRequest, errors.Errorf("cannot get project or projectgroup ref")) } + +type requestOptions struct { + Cursor string + + Limit int + SortDirection gwapitypes.SortDirection +} + +func parseRequestOptions(r *http.Request) (*requestOptions, error) { + query := r.URL.Query() + + cursor := query.Get("cursor") + + limit := DefaultLimit + limitS := query.Get("limit") + if limitS != "" { + var err error + limit, err = strconv.Atoi(limitS) + if err != nil { + return nil, util.NewAPIError(util.ErrBadRequest, errors.Wrapf(err, "cannot parse limit")) + } + } + if limit < 0 { + return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("limit must be greater or equal than 0")) + } + if limit > MaxLimit { + limit = MaxLimit + } + + sortDirection := gwapitypes.SortDirection(query.Get("sortdirection")) + if sortDirection != "" { + switch sortDirection { + case gwapitypes.SortDirectionAsc: + case gwapitypes.SortDirectionDesc: + default: + return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("wrong sort direction %q", sortDirection)) + } + } + + if cursor != "" && sortDirection != "" { + return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("only one of cursor or sortdirection should be provided")) + } + + return &requestOptions{ + Cursor: cursor, + + Limit: limit, + SortDirection: sortDirection, + }, nil +} + +func addCursorHeader(w http.ResponseWriter, cursor string) { + w.Header().Add(agolaCursorHeader, cursor) +} diff --git a/internal/services/gateway/api/org.go b/internal/services/gateway/api/org.go index f7bc5ab8b..969d03699 100644 --- a/internal/services/gateway/api/org.go +++ b/internal/services/gateway/api/org.go @@ -245,15 +245,30 @@ func NewOrgMembersHandler(log zerolog.Logger, ah *action.ActionHandler) *OrgMemb } func (h *OrgMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() + res, err := h.do(w, r) + if util.HTTPError(w, err) { + h.log.Err(err).Send() + return + } + if err := util.HTTPResponse(w, http.StatusOK, res); err != nil { + h.log.Err(err).Send() + } +} + +func (h *OrgMembersHandler) do(w http.ResponseWriter, r *http.Request) (*gwapitypes.OrgMembersResponse, error) { + ctx := r.Context() vars := mux.Vars(r) orgRef := vars["orgref"] - ares, err := h.ah.GetOrgMembers(ctx, orgRef) - if util.HTTPError(w, err) { - h.log.Err(err).Send() - return + ropts, err := parseRequestOptions(r) + if err != nil { + return nil, errors.WithStack(err) + } + + ares, err := h.ah.GetOrgMembers(ctx, &action.GetOrgMembersRequest{OrgRef: orgRef, Cursor: ropts.Cursor, Limit: ropts.Limit, SortDirection: action.SortDirection(ropts.SortDirection)}) + if err != nil { + return nil, errors.WithStack(err) } res := &gwapitypes.OrgMembersResponse{ @@ -263,9 +278,10 @@ func (h *OrgMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { for i, m := range ares.Members { res.Members[i] = createOrgMemberResponse(m.User, m.Role) } - if err := util.HTTPResponse(w, http.StatusOK, res); err != nil { - h.log.Err(err).Send() - } + + addCursorHeader(w, ares.Cursor) + + return res, nil } func createAddOrgMemberResponse(org *cstypes.Organization, user *cstypes.User, role cstypes.MemberRole) *gwapitypes.AddOrgMemberResponse { diff --git a/internal/services/gateway/gateway.go b/internal/services/gateway/gateway.go index 9964943c5..42ba94cfb 100644 --- a/internal/services/gateway/gateway.go +++ b/internal/services/gateway/gateway.go @@ -152,9 +152,9 @@ func (g *Gateway) Run(ctx context.Context) error { if len(g.c.Web.AllowedOrigins) > 0 { corsAllowedMethodsOptions := ghandlers.AllowedMethods([]string{"GET", "HEAD", "POST", "PUT", "DELETE"}) - corsAllowedHeadersOptions := ghandlers.AllowedHeaders([]string{"Accept", "Accept-Encoding", "Content-Length", "Content-Type", "Content-Range", "X-Csrf-Token", "Authorization"}) + corsAllowedHeadersOptions := ghandlers.AllowedHeaders([]string{"Accept", "Accept-Encoding", "Content-Length", "Content-Type", "Content-Range", "X-Csrf-Token", "X-Agola-Cursor", "Authorization"}) corsAllowedOriginsOptions := ghandlers.AllowedOrigins(g.c.Web.AllowedOrigins) - corsExposeHeadersOptions := ghandlers.ExposedHeaders([]string{"X-Csrf-Token"}) + corsExposeHeadersOptions := ghandlers.ExposedHeaders([]string{"X-Csrf-Token", "X-Agola-Cursor"}) corsHandler = ghandlers.CORS(corsAllowedMethodsOptions, corsAllowedHeadersOptions, corsAllowedOriginsOptions, corsExposeHeadersOptions, ghandlers.AllowCredentials()) } diff --git a/services/configstore/client/client.go b/services/configstore/client/client.go index 61c7d025d..f2428782c 100644 --- a/services/configstore/client/client.go +++ b/services/configstore/client/client.go @@ -628,9 +628,26 @@ func (c *Client) GetOrg(ctx context.Context, orgRef string) (*cstypes.Organizati return org, resp, errors.WithStack(err) } -func (c *Client) GetOrgMembers(ctx context.Context, orgRef string) ([]*csapitypes.OrgMemberResponse, *Response, error) { +type GetOrgMembersOptions struct { + *ListOptions + + StartUserName string +} + +func (o *GetOrgMembersOptions) Add(q url.Values) { + o.ListOptions.Add(q) + + if o.StartUserName != "" { + q.Add("startusername", o.StartUserName) + } +} + +func (c *Client) GetOrgMembers(ctx context.Context, orgRef string, opts *GetOrgMembersOptions) ([]*csapitypes.OrgMemberResponse, *Response, error) { + q := url.Values{} + opts.Add(q) + orgMembers := []*csapitypes.OrgMemberResponse{} - resp, err := c.GetParsedResponse(ctx, "GET", fmt.Sprintf("/orgs/%s/members", orgRef), nil, common.JSONContent, nil, &orgMembers) + resp, err := c.GetParsedResponse(ctx, "GET", fmt.Sprintf("/orgs/%s/members", orgRef), q, common.JSONContent, nil, &orgMembers) return orgMembers, resp, errors.WithStack(err) } diff --git a/services/gateway/client/client.go b/services/gateway/client/client.go index adc94333b..c62fd33b9 100644 --- a/services/gateway/client/client.go +++ b/services/gateway/client/client.go @@ -722,9 +722,12 @@ func (c *Client) RemoveOrgMember(ctx context.Context, orgRef, userRef string) (* return c.getResponse(ctx, "DELETE", fmt.Sprintf("/orgs/%s/members/%s", orgRef, userRef), nil, jsonContent, nil) } -func (c *Client) GetOrgMembers(ctx context.Context, orgRef string) (*gwapitypes.OrgMembersResponse, *Response, error) { +func (c *Client) GetOrgMembers(ctx context.Context, orgRef string, opts *ListOptions) (*gwapitypes.OrgMembersResponse, *Response, error) { + q := url.Values{} + opts.Add(q) + res := &gwapitypes.OrgMembersResponse{} - resp, err := c.getParsedResponse(ctx, "GET", fmt.Sprintf("/orgs/%s/members", orgRef), nil, jsonContent, nil, &res) + resp, err := c.getParsedResponse(ctx, "GET", fmt.Sprintf("/orgs/%s/members", orgRef), q, jsonContent, nil, &res) return res, resp, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index f46ea6b0a..85cf0aca2 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -2970,7 +2970,7 @@ func TestAddUpdateOrgUserMembers(t *testing.T) { Role: gwapitypes.MemberRoleMember, } - orgMembers, _, err := gwClient.GetOrgMembers(ctx, agolaOrg01) + orgMembers, _, err := gwClient.GetOrgMembers(ctx, agolaOrg01, nil) if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -2992,7 +2992,7 @@ func TestAddUpdateOrgUserMembers(t *testing.T) { expectedOrgMember.Role = gwapitypes.MemberRoleOwner - orgMembers, _, err = gwClient.GetOrgMembers(ctx, agolaOrg01) + orgMembers, _, err = gwClient.GetOrgMembers(ctx, agolaOrg01, nil) if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -3258,7 +3258,7 @@ func TestOrgInvitation(t *testing.T) { t.Fatalf("expected err %v, got err: %v", expectedErr, err) } - org01Members, _, err := tc.gwClientUser01.GetOrgMembers(ctx, agolaOrg01) + org01Members, _, err := tc.gwClientUser01.GetOrgMembers(ctx, agolaOrg01, nil) if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -3399,7 +3399,7 @@ func TestOrgInvitation(t *testing.T) { t.Fatalf("expected err %v, got err: %v", expectedErr, err) } - orgMembers, _, err := gwClientUser01.GetOrgMembers(ctx, agolaOrg01) + orgMembers, _, err := gwClientUser01.GetOrgMembers(ctx, agolaOrg01, nil) if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -3697,6 +3697,119 @@ func TestGetUsers(t *testing.T) { } } +func TestGetOrgMembers(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + sc := setup(ctx, t, dir, withGitea(true)) + defer sc.stop() + + createLinkedAccount(ctx, t, sc.gitea, sc.config) + gwClient := gwclient.NewClient(sc.config.Gateway.APIExposedURL, sc.config.Gateway.AdminToken) + + users := []*gwapitypes.UserResponse{} + for i := 1; i < 10; i++ { + user, _, err := gwClient.CreateUser(ctx, &gwapitypes.CreateUserRequest{UserName: fmt.Sprintf("orguser%d", i)}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + users = append(users, user) + } + + org, _, err := gwClient.CreateOrg(ctx, &gwapitypes.CreateOrgRequest{Name: agolaOrg01, Visibility: gwapitypes.VisibilityPublic}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + for _, user := range users { + if _, _, err := gwClient.AddOrgMember(ctx, agolaOrg01, user.ID, gwapitypes.MemberRoleMember); err != nil { + t.Fatalf("unexpected err: %v", err) + } + } + + t.Run("test get org members with default limit greater than org members", func(t *testing.T) { + res, resp, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + expectedOrgMembers := 9 + if len(res.Members) != expectedOrgMembers { + t.Fatalf("expected %d members, got %d members", expectedOrgMembers, len(res.Members)) + } + if resp.Cursor != "" { + t.Fatalf("expected no cursor, got cursor %q", resp.Cursor) + } + }) + + t.Run("test get org members with limit less than org members", func(t *testing.T) { + res, resp, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + expectedOrgMembers := 5 + if len(res.Members) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.Members)) + } + if resp.Cursor == "" { + t.Fatalf("expected cursor, got no cursor") + } + }) + + t.Run("test get org members with limit less than org members continuation", func(t *testing.T) { + orgMembers := []*gwapitypes.OrgMemberResponse{} + + res, resp, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + expectedOrgMembers := 5 + if len(res.Members) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.Members)) + } + if resp.Cursor == "" { + t.Fatalf("expected cursor, got no cursor") + } + + orgMembers = append(orgMembers, res.Members...) + + // fetch next results + for { + res, resp, err = gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + expectedOrgMembers := 5 + if resp.Cursor != "" && len(res.Members) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(res.Members)) + } + + orgMembers = append(orgMembers, res.Members...) + + if resp.Cursor == "" { + break + } + } + + expectedOrgMembers = 9 + if len(orgMembers) != expectedOrgMembers { + t.Fatalf("expected %d org members, got %d org members", expectedOrgMembers, len(orgMembers)) + } + + orgMemberUsers := []*gwapitypes.UserResponse{} + for _, orgMember := range orgMembers { + orgMemberUsers = append(orgMemberUsers, orgMember.User) + } + if diff := cmp.Diff(users, orgMemberUsers); diff != "" { + t.Fatalf("mismatch (-want +got):\n%s", diff) + } + }) +} + func TestMaintenance(t *testing.T) { t.Parallel()