From c628d3307d71eba9fe64d00609ab37d81a2cd9ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ga=C4=87e=C5=A1a?= Date: Wed, 9 Oct 2024 10:43:32 +0000 Subject: [PATCH] feat: [CODE-2483]: skip permission check for space pullreq API and fix the inner loop (#2791) * skip permission check for space pullreq API and fix the inner loop --- app/api/controller/space/pr_list.go | 8 +++++--- app/services/pullreq/service_list.go | 11 ++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/api/controller/space/pr_list.go b/app/api/controller/space/pr_list.go index d101aab91..92bb69c17 100644 --- a/app/api/controller/space/pr_list.go +++ b/app/api/controller/space/pr_list.go @@ -20,7 +20,6 @@ import ( "github.com/harness/gitness/app/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/enum" ) // ListPullReqs returns a list of pull requests from the provided space. @@ -31,11 +30,14 @@ func (c *Controller) ListPullReqs( includeSubspaces bool, filter *types.PullReqFilter, ) ([]types.PullReqRepo, error) { - space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceView) + space, err := c.spaceStore.FindByRef(ctx, spaceRef) if err != nil { - return nil, fmt.Errorf("failed to acquire access to space: %w", err) + return nil, fmt.Errorf("space not found: %w", err) } + // We deliberately don't check for space permission because the pull request service + // will check for repo-view permission for every returned pull request. + pullReqs, err := c.prListService.ListForSpace(ctx, session, space, includeSubspaces, filter) if err != nil { return nil, fmt.Errorf("failed to fetch pull requests from space: %w", err) diff --git a/app/services/pullreq/service_list.go b/app/services/pullreq/service_list.go index 3e97e6b70..9daccf795 100644 --- a/app/services/pullreq/service_list.go +++ b/app/services/pullreq/service_list.go @@ -185,8 +185,9 @@ func (c *ListService) streamPullReqs( pullReqs := make([]*types.PullReq, 0, opts.Size) ch, chErr := c.pullreqStore.Stream(ctx, opts) for pr := range ch { - if pr == nil { - return pullReqs, repoUnchecked, nil + if len(pullReqs) >= pullReqLimit || len(repoUnchecked) >= newRepoLimit { + cancelFn() // the loop must be exited by canceling the context + continue } if _, ok := repoWhitelist[pr.TargetRepoID]; !ok { @@ -194,13 +195,9 @@ func (c *ListService) streamPullReqs( } pullReqs = append(pullReqs, pr) - - if len(pullReqs) >= pullReqLimit || len(repoUnchecked) >= newRepoLimit { - break - } } - if err := <-chErr; err != nil { + if err := <-chErr; err != nil && !errors.Is(err, context.Canceled) { return nil, nil, fmt.Errorf("failed to stream pull requests: %w", err) }