Skip to content

Commit

Permalink
feat: Search and Filter for ListAppeals and ListGrants (#62)
Browse files Browse the repository at this point in the history 
* feat: Add filter q, account_type for litAppeals and listGrants and added getTotalCount

* chore: added testing for total count

* chore: fix testing

* chore: fix test total

* chore: fix test mock appeal and grant

* chore: fix test mock user appeal and grant

* chore: fix test coverage

* chore: fix test coverage 2

* chore: fix test coverage 3

* chore: resolve comments

* test: resolve comments

* test: fix testing

---------

Co-authored-by: Lifosmin Simon <[email protected]>
  • Loading branch information
@lifosmin
lifosmin and Lifosmin Simon authored Aug 31, 2023
1 parent 9fb85ea commit 1740dd4
Show file tree
Hide file tree
Showing 39 changed files with 1,520 additions and 359 deletions.
Binary file added __debug_bin513583757
View file
Binary file not shown.
47 changes: 39 additions & 8 deletions api/handler/v1beta1/appeal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
guardianv1beta1 "github.com/goto/guardian/api/proto/gotocompany/guardian/v1beta1"
"github.com/goto/guardian/core/appeal"
"github.com/goto/guardian/domain"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -35,27 +36,36 @@ func (s *GRPCServer) ListUserAppeals(ctx context.Context, req *guardianv1beta1.L
if req.GetResourceTypes() != nil {
filters.ResourceTypes = req.GetResourceTypes()
}
if req.GetAccountTypes() != nil {
filters.AccountTypes = req.GetAccountTypes()
}
if req.GetResourceUrns() != nil {
filters.ResourceURNs = req.GetResourceUrns()
}
if req.GetOrderBy() != nil {
filters.OrderBy = req.GetOrderBy()
}
if req.GetQ() != "" {
filters.Q = req.GetQ()
}
filters.Offset = int(req.GetOffset())
filters.Size = int(req.GetSize())

appeals, err := s.listAppeals(ctx, filters)
appeals, total, err := s.listAppeals(ctx, filters)
if err != nil {
return nil, err
}

return &guardianv1beta1.ListUserAppealsResponse{
Appeals: appeals,
Total: int32(total),
}, nil
}

func (s *GRPCServer) ListAppeals(ctx context.Context, req *guardianv1beta1.ListAppealsRequest) (*guardianv1beta1.ListAppealsResponse, error) {
filters := &domain.ListAppealsFilter{
Q: req.GetQ(),
AccountTypes: req.GetAccountTypes(),
AccountID: req.GetAccountId(),
Statuses: req.GetStatuses(),
Role: req.GetRole(),
Expand All @@ -67,13 +77,14 @@ func (s *GRPCServer) ListAppeals(ctx context.Context, req *guardianv1beta1.ListA
Offset: int(req.GetOffset()),
OrderBy: req.GetOrderBy(),
}
appeals, err := s.listAppeals(ctx, filters)
appeals, total, err := s.listAppeals(ctx, filters)
if err != nil {
return nil, err
}

return &guardianv1beta1.ListAppealsResponse{
Appeals: appeals,
Total: int32(total),
}, nil
}

Expand Down Expand Up @@ -166,20 +177,40 @@ func (s *GRPCServer) CancelAppeal(ctx context.Context, req *guardianv1beta1.Canc
}, nil
}

func (s *GRPCServer) listAppeals(ctx context.Context, filters *domain.ListAppealsFilter) ([]*guardianv1beta1.Appeal, error) {
appeals, err := s.appealService.Find(ctx, filters)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get appeal list: %s", err)
func (s *GRPCServer) listAppeals(ctx context.Context, filters *domain.ListAppealsFilter) ([]*guardianv1beta1.Appeal, int64, error) {
eg, ctx := errgroup.WithContext(ctx)
var appeals []*domain.Appeal
var total int64

eg.Go(func() error {
appealRecords, err := s.appealService.Find(ctx, filters)
if err != nil {
return status.Errorf(codes.Internal, "failed to get appeal list: %s", err)
}
appeals = appealRecords
return nil
})
eg.Go(func() error {
totalRecord, err := s.appealService.GetAppealsTotalCount(ctx, filters)
if err != nil {
return status.Errorf(codes.Internal, "failed to get appeal total count: %s", err)
}
total = totalRecord
return nil
})

if err := eg.Wait(); err != nil {
return nil, 0, err
}

appealProtos := []*guardianv1beta1.Appeal{}
for _, a := range appeals {
appealProto, err := s.adapter.ToAppealProto(a)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to parse appeal: %s", err)
return nil, 0, status.Errorf(codes.Internal, "failed to parse appeal: %s", err)
}
appealProtos = append(appealProtos, appealProto)
}

return appealProtos, nil
return appealProtos, total, nil
}
32 changes: 26 additions & 6 deletions api/handler/v1beta1/appeal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package v1beta1_test
import (
"context"
"errors"
"fmt"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -33,6 +34,8 @@ func (s *GrpcHandlersSuite) TestListUserAppeals() {
ResourceTypes: []string{"test-resource-type"},
ResourceURNs: []string{"test-resource-urn"},
OrderBy: []string{"test-order"},
AccountTypes: []string{"test-account-type"},
Q: "test",
}
expectedAppeals := []*domain.Appeal{
{
Expand Down Expand Up @@ -117,9 +120,12 @@ func (s *GrpcHandlersSuite) TestListUserAppeals() {
UpdatedAt: timestamppb.New(timeNow),
},
},
Total: 1,
}
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.valueCtx"), expectedFilters).
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), expectedFilters).
Return(expectedAppeals, nil).Once()
s.appealService.EXPECT().GetAppealsTotalCount(mock.AnythingOfType("*context.cancelCtx"), expectedFilters).
Return(int64(1), nil).Once()

req := &guardianv1beta1.ListUserAppealsRequest{
Statuses: []string{"active", "pending"},
Expand All @@ -129,6 +135,8 @@ func (s *GrpcHandlersSuite) TestListUserAppeals() {
ResourceTypes: []string{"test-resource-type"},
ResourceUrns: []string{"test-resource-urn"},
OrderBy: []string{"test-order"},
AccountTypes: []string{"test-account-type"},
Q: "test",
}
ctx := context.WithValue(context.Background(), authEmailTestContextKey{}, expectedUser)
res, err := s.grpcServer.ListUserAppeals(ctx, req)
Expand Down Expand Up @@ -156,13 +164,16 @@ func (s *GrpcHandlersSuite) TestListUserAppeals() {
s.setup()

expectedError := errors.New("random error")
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.valueCtx"), mock.Anything).
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(nil, expectedError).Once()
s.appealService.EXPECT().GetAppealsTotalCount(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(int64(0), nil).Once()

req := &guardianv1beta1.ListUserAppealsRequest{}
ctx := context.WithValue(context.Background(), authEmailTestContextKey{}, "test-user")
res, err := s.grpcServer.ListUserAppeals(ctx, req)

fmt.Println(status.Code(err))
s.Equal(codes.Internal, status.Code(err))
s.Nil(res)
s.appealService.AssertExpectations(s.T())
Expand All @@ -178,8 +189,10 @@ func (s *GrpcHandlersSuite) TestListUserAppeals() {
},
},
}
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.valueCtx"), mock.Anything).
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(invalidAppeals, nil).Once()
s.appealService.EXPECT().GetAppealsTotalCount(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(int64(1), nil).Once()

req := &guardianv1beta1.ListUserAppealsRequest{}
ctx := context.WithValue(context.Background(), authEmailTestContextKey{}, "test-user")
Expand Down Expand Up @@ -288,9 +301,12 @@ func (s *GrpcHandlersSuite) TestListAppeals() {
UpdatedAt: timestamppb.New(timeNow),
},
},
Total: 1,
}
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx"), expectedFilters).
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), expectedFilters).
Return(expectedAppeals, nil).Once()
s.appealService.EXPECT().GetAppealsTotalCount(mock.AnythingOfType("*context.cancelCtx"), expectedFilters).
Return(int64(1), nil).Once()

req := &guardianv1beta1.ListAppealsRequest{
AccountId: expectedUser,
Expand All @@ -313,8 +329,10 @@ func (s *GrpcHandlersSuite) TestListAppeals() {
s.setup()

expectedError := errors.New("random error")
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx"), mock.Anything).
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(nil, expectedError).Once()
s.appealService.EXPECT().GetAppealsTotalCount(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(int64(0), nil).Once()

req := &guardianv1beta1.ListAppealsRequest{}
res, err := s.grpcServer.ListAppeals(context.Background(), req)
Expand All @@ -334,8 +352,10 @@ func (s *GrpcHandlersSuite) TestListAppeals() {
},
},
}
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx"), mock.Anything).
s.appealService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(invalidAppeals, nil).Once()
s.appealService.EXPECT().GetAppealsTotalCount(mock.AnythingOfType("*context.cancelCtx"), mock.Anything).
Return(int64(1), nil).Once()

req := &guardianv1beta1.ListAppealsRequest{}
res, err := s.grpcServer.ListAppeals(context.Background(), req)
Expand Down
40 changes: 32 additions & 8 deletions api/handler/v1beta1/grant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import (
"github.com/goto/guardian/core/grant"
"github.com/goto/guardian/core/provider"
"github.com/goto/guardian/domain"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func (s *GRPCServer) ListGrants(ctx context.Context, req *guardianv1beta1.ListGrantsRequest) (*guardianv1beta1.ListGrantsResponse, error) {
filter := domain.ListGrantsFilter{
Q: req.GetQ(),
Statuses: req.GetStatuses(),
AccountIDs: req.GetAccountIds(),
AccountTypes: req.GetAccountTypes(),
Expand All @@ -29,13 +31,14 @@ func (s *GRPCServer) ListGrants(ctx context.Context, req *guardianv1beta1.ListGr
Size: int(req.GetSize()),
Offset: int(req.GetOffset()),
}
grants, err := s.listGrants(ctx, filter)
grants, total, err := s.listGrants(ctx, filter)
if err != nil {
return nil, err
}

return &guardianv1beta1.ListGrantsResponse{
Grants: grants,
Total: int32(total),
}, nil
}

Expand All @@ -60,13 +63,14 @@ func (s *GRPCServer) ListUserGrants(ctx context.Context, req *guardianv1beta1.Li
Offset: int(req.GetOffset()),
Owner: user,
}
grants, err := s.listGrants(ctx, filter)
grants, total, err := s.listGrants(ctx, filter)
if err != nil {
return nil, err
}

return &guardianv1beta1.ListUserGrantsResponse{
Grants: grants,
Total: int32(total),
}, nil
}

Expand Down Expand Up @@ -171,22 +175,42 @@ func (s *GRPCServer) RevokeGrants(ctx context.Context, req *guardianv1beta1.Revo
}, nil
}

func (s *GRPCServer) listGrants(ctx context.Context, filter domain.ListGrantsFilter) ([]*guardianv1beta1.Grant, error) {
grants, err := s.grantService.List(ctx, filter)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to list grants: %v", err)
func (s *GRPCServer) listGrants(ctx context.Context, filter domain.ListGrantsFilter) ([]*guardianv1beta1.Grant, int64, error) {
eg, ctx := errgroup.WithContext(ctx)
var grants []domain.Grant
var total int64

eg.Go(func() error {
grantRecords, err := s.grantService.List(ctx, filter)
if err != nil {
return status.Errorf(codes.Internal, "failed to get grant list: %s", err)
}
grants = grantRecords
return nil
})
eg.Go(func() error {
totalRecord, err := s.grantService.GetGrantsTotalCount(ctx, filter)
if err != nil {
return status.Errorf(codes.Internal, "failed to get grant total count: %s", err)
}
total = totalRecord
return nil
})

if err := eg.Wait(); err != nil {
return nil, 0, err
}

var grantProtos []*guardianv1beta1.Grant
for i, a := range grants {
grantProto, err := s.adapter.ToGrantProto(&grants[i])
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to parse grant %q: %v", a.ID, err)
return nil, 0, status.Errorf(codes.Internal, "failed to parse grant %q: %v", a.ID, err)
}
grantProtos = append(grantProtos, grantProto)
}

return grantProtos, nil
return grantProtos, total, nil
}

func (s *GRPCServer) ImportGrantsFromProvider(ctx context.Context, req *guardianv1beta1.ImportGrantsFromProviderRequest) (*guardianv1beta1.ImportGrantsFromProviderResponse, error) {
Expand Down
17 changes: 13 additions & 4 deletions api/handler/v1beta1/grant_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (s *GrpcHandlersSuite) TestListGrants() {
},
},
},
Total: 1,
}
expectedFilter := domain.ListGrantsFilter{
Statuses: []string{"test-status"},
Expand All @@ -74,8 +75,11 @@ func (s *GrpcHandlersSuite) TestListGrants() {
ResourceIDs: []string{"test-resource-id"},
}
s.grantService.EXPECT().
List(mock.AnythingOfType("*context.emptyCtx"), expectedFilter).
List(mock.AnythingOfType("*context.cancelCtx"), expectedFilter).
Return(dummyGrants, nil).Once()
s.grantService.EXPECT().
GetGrantsTotalCount(mock.AnythingOfType("*context.cancelCtx"), expectedFilter).
Return(int64(1), nil).Once()

req := &guardianv1beta1.ListGrantsRequest{
Statuses: expectedFilter.Statuses,
Expand All @@ -95,8 +99,11 @@ func (s *GrpcHandlersSuite) TestListGrants() {

expectedError := errors.New("unexpected error")
s.grantService.EXPECT().
List(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).
List(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).
Return(nil, expectedError).Once()
s.grantService.EXPECT().
GetGrantsTotalCount(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).
Return(int64(0), nil).Once()

req := &guardianv1beta1.ListGrantsRequest{}
res, err := s.grpcServer.ListGrants(context.Background(), req)
Expand All @@ -119,9 +126,11 @@ func (s *GrpcHandlersSuite) TestListGrants() {
},
}
s.grantService.EXPECT().
List(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).
List(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).
Return(expectedGrants, nil).Once()

s.grantService.EXPECT().
GetGrantsTotalCount(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).
Return(int64(1), nil).Once()
req := &guardianv1beta1.ListGrantsRequest{}
res, err := s.grpcServer.ListGrants(context.Background(), req)

Expand Down
2 changes: 2 additions & 0 deletions api/handler/v1beta1/grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ type policyService interface {

//go:generate mockery --name=appealService --exported --with-expecter
type appealService interface {
GetAppealsTotalCount(context.Context, *domain.ListAppealsFilter) (int64, error)
GetByID(context.Context, string) (*domain.Appeal, error)
Find(context.Context, *domain.ListAppealsFilter) ([]*domain.Appeal, error)
Create(context.Context, []*domain.Appeal, ...appeal.CreateAppealOption) error
Expand All @@ -101,6 +102,7 @@ type approvalService interface {

//go:generate mockery --name=grantService --exported --with-expecter
type grantService interface {
GetGrantsTotalCount(context.Context, domain.ListGrantsFilter) (int64, error)
List(context.Context, domain.ListGrantsFilter) ([]domain.Grant, error)
GetByID(context.Context, string) (*domain.Grant, error)
Update(context.Context, *domain.Grant) error
Expand Down
Loading

0 comments on commit 1740dd4

Please sign in to comment.