From 766c174fe95b9b4e3db7f7fb74c79a942ca3aabf Mon Sep 17 00:00:00 2001 From: Todd Knight Date: Mon, 22 Jun 2020 10:46:45 -0700 Subject: [PATCH] Implement List Groups (#133) * Implement List Groups * Updating ListGroup's comments and tweaking variable name. --- .../handlers/groups/group_service.go | 37 +++++++++- .../handlers/groups/group_service_test.go | 72 +++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/internal/servers/controller/handlers/groups/group_service.go b/internal/servers/controller/handlers/groups/group_service.go index 9d900bdadb..f03bf94043 100644 --- a/internal/servers/controller/handlers/groups/group_service.go +++ b/internal/servers/controller/handlers/groups/group_service.go @@ -43,9 +43,16 @@ func NewService(repo func() (*iam.Repository, error)) (Service, error) { var _ pbs.GroupServiceServer = Service{} -// CreateGroup is not yet implemented but will implement the interface pbs.GroupServiceServer. -func (s Service) ListGroups(context.Context, *pbs.ListGroupsRequest) (*pbs.ListGroupsResponse, error) { - return nil, status.Error(codes.Unimplemented, "List not enabled for this resource.") +// ListGroups implements the interface pbs.GroupServiceServer. +func (s Service) ListGroups(ctx context.Context, req *pbs.ListGroupsRequest) (*pbs.ListGroupsResponse, error) { + if err := validateListRequest(req); err != nil { + return nil, err + } + gl, err := s.listFromRepo(ctx, req.GetOrgId()) + if err != nil { + return nil, err + } + return &pbs.ListGroupsResponse{Items: gl}, nil } // GetGroups implements the interface pbs.GroupServiceServer. @@ -189,6 +196,22 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { return rows > 0, nil } +func (s Service) listFromRepo(ctx context.Context, orgId string) ([]*pb.Group, error) { + repo, err := s.repoFn() + if err != nil { + return nil, err + } + gl, err := repo.ListGroups(ctx, orgId) + if err != nil { + return nil, err + } + var outGl []*pb.Group + for _, g := range gl { + outGl = append(outGl, toProto(g)) + } + return outGl, nil +} + // toDbUpdateMask converts the wire format's FieldMask into a list of strings containing FieldMask paths used func toDbUpdateMask(paths []string) ([]string, error) { var dbPaths []string @@ -299,6 +322,14 @@ func validateDeleteRequest(req *pbs.DeleteGroupRequest) error { return nil } +func validateListRequest(req *pbs.ListGroupsRequest) error { + badFields := validateAncestors(req) + if len(badFields) > 0 { + return handlers.InvalidArgumentErrorf("Improperly formatted identifier.", badFields) + } + return nil +} + func validId(id, prefix string) bool { if !strings.HasPrefix(id, prefix) { return false diff --git a/internal/servers/controller/handlers/groups/group_service_test.go b/internal/servers/controller/handlers/groups/group_service_test.go index accc5852c8..4393b03861 100644 --- a/internal/servers/controller/handlers/groups/group_service_test.go +++ b/internal/servers/controller/handlers/groups/group_service_test.go @@ -107,6 +107,78 @@ func TestGet(t *testing.T) { } } +func TestList(t *testing.T) { + assert, require := assert.New(t), require.New(t) + cleanup, conn, _ := db.TestSetup(t, "postgres") + t.Cleanup(func() { + if err := conn.Close(); err != nil { + t.Errorf("Error when closing gorm DB: %v", err) + } + if err := cleanup(); err != nil { + t.Errorf("Error when cleaning up TestSetup: %v", err) + } + }) + rw := db.New(conn) + wrap := db.TestWrapper(t) + repoFn := func() (*iam.Repository, error) { + return iam.NewRepository(rw, rw, wrap) + } + oNoGroups, _ := iam.TestScopes(t, conn) + oWithGroups, _ := iam.TestScopes(t, conn) + var wantGroups []*pb.Group + for i := 0; i < 10; i++ { + g := iam.TestGroup(t, conn, oWithGroups.GetPublicId()) + wantGroups = append(wantGroups, &pb.Group{ + Id: g.GetPublicId(), + CreatedTime: g.GetCreateTime().GetTimestamp(), + UpdatedTime: g.GetUpdateTime().GetTimestamp(), + }) + } + + cases := []struct { + name string + req *pbs.ListGroupsRequest + res *pbs.ListGroupsResponse + errCode codes.Code + }{ + { + name: "List Many Group", + req: &pbs.ListGroupsRequest{OrgId: oWithGroups.GetPublicId()}, + res: &pbs.ListGroupsResponse{Items: wantGroups}, + errCode: codes.OK, + }, + { + name: "List No Groups", + req: &pbs.ListGroupsRequest{OrgId: oNoGroups.GetPublicId()}, + res: &pbs.ListGroupsResponse{}, + errCode: codes.OK, + }, + { + name: "Invalid Org Id", + req: &pbs.ListGroupsRequest{OrgId: iam.OrganizationScope.Prefix() + "_this is invalid"}, + res: nil, + errCode: codes.InvalidArgument, + }, + // TODO: When an org doesn't exist, we should return a 404 instead of an empty list. + { + name: "Unfound Org", + req: &pbs.ListGroupsRequest{OrgId: iam.OrganizationScope.Prefix() + "_DoesntExis"}, + res: &pbs.ListGroupsResponse{}, + errCode: codes.OK, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s, err := groups.NewService(repoFn) + require.NoError(err, "Couldn't create new group service.") + + got, gErr := s.ListGroups(context.Background(), tc.req) + assert.Equal(tc.errCode, status.Code(gErr), "ListGroups(%+v) got error %v, wanted %v", tc.req, gErr, tc.errCode) + assert.True(proto.Equal(got, tc.res), "ListGroups(%q) got response %q, wanted %q", tc.req, got, tc.res) + }) + } +} + func TestDelete(t *testing.T) { require := require.New(t) g, repo := createDefaultGroupAndRepo(t)