From 75042e9293009ca3e2820cc20439cf9c18fd2d02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ga=C4=87e=C5=A1a?= Date: Thu, 29 Aug 2024 11:49:33 +0000 Subject: [PATCH] add pullreq list API for spaces (#2606) * moved the service to the existing pullreq package * add pullreq list API for spaces --- app/api/controller/pullreq/controller.go | 3 + app/api/controller/pullreq/pr_find.go | 23 +-- app/api/controller/pullreq/pr_list.go | 2 +- app/api/controller/pullreq/wire.go | 4 +- app/api/controller/space/controller.go | 6 +- app/api/controller/space/pr_list.go | 45 +++++ app/api/controller/space/wire.go | 6 +- app/api/handler/space/pr_list.go | 55 ++++++ app/api/openapi/common.go | 45 +++++ app/api/openapi/pullreq.go | 18 +- app/api/openapi/space.go | 19 ++ app/api/request/common.go | 22 +++ app/api/request/pullreq.go | 40 ++-- app/api/request/space.go | 12 ++ app/router/api.go | 1 + app/services/pullreq/service_list.go | 233 +++++++++++++++++++++++ app/services/pullreq/wire.go | 26 +++ app/store/database.go | 5 +- app/store/database/pullreq.go | 108 +++++++++-- cmd/gitness/wire.go | 5 +- cmd/gitness/wire_gen.go | 11 +- types/list_filters.go | 5 + types/pullreq.go | 39 ++-- types/space.go | 2 +- 24 files changed, 658 insertions(+), 77 deletions(-) create mode 100644 app/api/controller/space/pr_list.go create mode 100644 app/api/handler/space/pr_list.go create mode 100644 app/services/pullreq/service_list.go diff --git a/app/api/controller/pullreq/controller.go b/app/api/controller/pullreq/controller.go index 29b1d56cd..c787c9c45 100644 --- a/app/api/controller/pullreq/controller.go +++ b/app/api/controller/pullreq/controller.go @@ -66,6 +66,7 @@ type Controller struct { eventReporter *pullreqevents.Reporter codeCommentMigrator *codecomments.Migrator pullreqService *pullreq.Service + pullreqListService *pullreq.ListService protectionManager *protection.Manager sseStreamer sse.Streamer codeOwners *codeowners.Service @@ -97,6 +98,7 @@ func NewController( eventReporter *pullreqevents.Reporter, codeCommentMigrator *codecomments.Migrator, pullreqService *pullreq.Service, + pullreqListService *pullreq.ListService, protectionManager *protection.Manager, sseStreamer sse.Streamer, codeowners *codeowners.Service, @@ -127,6 +129,7 @@ func NewController( codeCommentMigrator: codeCommentMigrator, eventReporter: eventReporter, pullreqService: pullreqService, + pullreqListService: pullreqListService, protectionManager: protectionManager, sseStreamer: sseStreamer, codeOwners: codeowners, diff --git a/app/api/controller/pullreq/pr_find.go b/app/api/controller/pullreq/pr_find.go index 7e97133ba..be885ccc1 100644 --- a/app/api/controller/pullreq/pr_find.go +++ b/app/api/controller/pullreq/pr_find.go @@ -20,7 +20,6 @@ import ( "github.com/harness/gitness/app/api/usererror" "github.com/harness/gitness/app/auth" - "github.com/harness/gitness/git" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" @@ -53,29 +52,9 @@ func (c *Controller) Find( return nil, fmt.Errorf("failed to backfill labels assigned to pull request: %w", err) } - if err := c.backfillStats(ctx, repo, pr); err != nil { + if err := c.pullreqListService.BackfillStats(ctx, pr); err != nil { log.Ctx(ctx).Warn().Err(err).Msg("failed to backfill PR stats") } return pr, nil } - -func (c *Controller) backfillStats(ctx context.Context, repo *types.Repository, pr *types.PullReq) error { - s := pr.Stats.DiffStats - if s.Commits != nil && s.FilesChanged != nil && s.Additions != nil && s.Deletions != nil { - return nil - } - - output, err := c.git.DiffStats(ctx, &git.DiffParams{ - ReadParams: git.CreateReadParams(repo), - BaseRef: pr.MergeBaseSHA, - HeadRef: pr.SourceSHA, - }) - if err != nil { - return fmt.Errorf("failed get diff stats: %w", err) - } - - pr.Stats.DiffStats = types.NewDiffStats(output.Commits, output.FilesChanged, output.Additions, output.Deletions) - - return nil -} diff --git a/app/api/controller/pullreq/pr_list.go b/app/api/controller/pullreq/pr_list.go index f558ff9e0..5995378fd 100644 --- a/app/api/controller/pullreq/pr_list.go +++ b/app/api/controller/pullreq/pr_list.go @@ -82,7 +82,7 @@ func (c *Controller) List( } for _, pr := range list { - if err := c.backfillStats(ctx, repo, pr); err != nil { + if err := c.pullreqListService.BackfillStats(ctx, pr); err != nil { log.Ctx(ctx).Warn().Err(err).Msg("failed to backfill PR stats") } } diff --git a/app/api/controller/pullreq/wire.go b/app/api/controller/pullreq/wire.go index 72b4ee2e1..0c371278a 100644 --- a/app/api/controller/pullreq/wire.go +++ b/app/api/controller/pullreq/wire.go @@ -53,7 +53,8 @@ func ProvideController(tx dbtx.Transactor, urlProvider url.Provider, authorizer membershipStore store.MembershipStore, checkStore store.CheckStore, rpcClient git.Interface, eventReporter *pullreqevents.Reporter, codeCommentMigrator *codecomments.Migrator, - pullreqService *pullreq.Service, ruleManager *protection.Manager, sseStreamer sse.Streamer, + pullreqService *pullreq.Service, pullreqListService *pullreq.ListService, + ruleManager *protection.Manager, sseStreamer sse.Streamer, codeOwners *codeowners.Service, locker *locker.Locker, importer *migrate.PullReq, labelSvc *label.Service, instrumentation instrument.Service, @@ -79,6 +80,7 @@ func ProvideController(tx dbtx.Transactor, urlProvider url.Provider, authorizer eventReporter, codeCommentMigrator, pullreqService, + pullreqListService, ruleManager, sseStreamer, codeOwners, diff --git a/app/api/controller/space/controller.go b/app/api/controller/space/controller.go index 9582d9d87..ac2dfad23 100644 --- a/app/api/controller/space/controller.go +++ b/app/api/controller/space/controller.go @@ -27,6 +27,7 @@ import ( "github.com/harness/gitness/app/services/instrument" "github.com/harness/gitness/app/services/label" "github.com/harness/gitness/app/services/publicaccess" + "github.com/harness/gitness/app/services/pullreq" "github.com/harness/gitness/app/sse" "github.com/harness/gitness/app/store" "github.com/harness/gitness/app/url" @@ -79,6 +80,7 @@ type Controller struct { principalStore store.PrincipalStore repoCtrl *repo.Controller membershipStore store.MembershipStore + prListService *pullreq.ListService importer *importer.Repository exporter *exporter.Repository resourceLimiter limiter.ResourceLimiter @@ -94,7 +96,8 @@ func NewController(config *types.Config, tx dbtx.Transactor, urlProvider url.Pro spacePathStore store.SpacePathStore, pipelineStore store.PipelineStore, secretStore store.SecretStore, connectorStore store.ConnectorStore, templateStore store.TemplateStore, spaceStore store.SpaceStore, repoStore store.RepoStore, principalStore store.PrincipalStore, repoCtrl *repo.Controller, - membershipStore store.MembershipStore, importer *importer.Repository, exporter *exporter.Repository, + membershipStore store.MembershipStore, prListService *pullreq.ListService, + importer *importer.Repository, exporter *exporter.Repository, limiter limiter.ResourceLimiter, publicAccess publicaccess.Service, auditService audit.Service, gitspaceSvc *gitspace.Service, labelSvc *label.Service, instrumentation instrument.Service, @@ -116,6 +119,7 @@ func NewController(config *types.Config, tx dbtx.Transactor, urlProvider url.Pro principalStore: principalStore, repoCtrl: repoCtrl, membershipStore: membershipStore, + prListService: prListService, importer: importer, exporter: exporter, resourceLimiter: limiter, diff --git a/app/api/controller/space/pr_list.go b/app/api/controller/space/pr_list.go new file mode 100644 index 000000000..d101aab91 --- /dev/null +++ b/app/api/controller/space/pr_list.go @@ -0,0 +1,45 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package space + +import ( + "context" + "fmt" + + "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. +func (c *Controller) ListPullReqs( + ctx context.Context, + session *auth.Session, + spaceRef string, + includeSubspaces bool, + filter *types.PullReqFilter, +) ([]types.PullReqRepo, error) { + space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceView) + if err != nil { + return nil, fmt.Errorf("failed to acquire access to space: %w", err) + } + + 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) + } + + return pullReqs, nil +} diff --git a/app/api/controller/space/wire.go b/app/api/controller/space/wire.go index 812b69638..5cd144c5b 100644 --- a/app/api/controller/space/wire.go +++ b/app/api/controller/space/wire.go @@ -24,6 +24,7 @@ import ( "github.com/harness/gitness/app/services/instrument" "github.com/harness/gitness/app/services/label" "github.com/harness/gitness/app/services/publicaccess" + "github.com/harness/gitness/app/services/pullreq" "github.com/harness/gitness/app/sse" "github.com/harness/gitness/app/store" "github.com/harness/gitness/app/url" @@ -45,7 +46,8 @@ func ProvideController(config *types.Config, tx dbtx.Transactor, urlProvider url pipelineStore store.PipelineStore, secretStore store.SecretStore, connectorStore store.ConnectorStore, templateStore store.TemplateStore, spaceStore store.SpaceStore, repoStore store.RepoStore, principalStore store.PrincipalStore, - repoCtrl *repo.Controller, membershipStore store.MembershipStore, importer *importer.Repository, + repoCtrl *repo.Controller, membershipStore store.MembershipStore, prListService *pullreq.ListService, + importer *importer.Repository, exporter *exporter.Repository, limiter limiter.ResourceLimiter, publicAccess publicaccess.Service, auditService audit.Service, gitspaceService *gitspace.Service, labelSvc *label.Service, @@ -55,7 +57,7 @@ func ProvideController(config *types.Config, tx dbtx.Transactor, urlProvider url spacePathStore, pipelineStore, secretStore, connectorStore, templateStore, spaceStore, repoStore, principalStore, - repoCtrl, membershipStore, importer, + repoCtrl, membershipStore, prListService, importer, exporter, limiter, publicAccess, auditService, gitspaceService, labelSvc, diff --git a/app/api/handler/space/pr_list.go b/app/api/handler/space/pr_list.go new file mode 100644 index 000000000..e8662f40d --- /dev/null +++ b/app/api/handler/space/pr_list.go @@ -0,0 +1,55 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package space + +import ( + "net/http" + + "github.com/harness/gitness/app/api/controller/space" + "github.com/harness/gitness/app/api/render" + "github.com/harness/gitness/app/api/request" +) + +func HandleListPullReqs(spaceCtrl *space.Controller) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + session, _ := request.AuthSessionFrom(ctx) + spaceRef, err := request.GetSpaceRefFromPath(r) + if err != nil { + render.TranslatedUserError(ctx, w, err) + return + } + + includeSubspaces, err := request.GetIncludeSubspacesFromQuery(r) + if err != nil { + render.TranslatedUserError(ctx, w, err) + return + } + + pullReqFilter, err := request.ParsePullReqFilter(r) + if err != nil { + render.TranslatedUserError(ctx, w, err) + return + } + + pullreqs, err := spaceCtrl.ListPullReqs(ctx, session, spaceRef, includeSubspaces, pullReqFilter) + if err != nil { + render.TranslatedUserError(ctx, w, err) + return + } + + render.JSON(w, http.StatusOK, pullreqs) + } +} diff --git a/app/api/openapi/common.go b/app/api/openapi/common.go index ac93b1c63..ed231ddd3 100644 --- a/app/api/openapi/common.go +++ b/app/api/openapi/common.go @@ -126,3 +126,48 @@ var queryParameterCreatedGt = openapi3.ParameterOrRef{ }, }, } + +var queryParameterEditedLt = openapi3.ParameterOrRef{ + Parameter: &openapi3.Parameter{ + Name: request.QueryParamEditedLt, + In: openapi3.ParameterInQuery, + Description: ptr.String("The result should contain only entries edited before this timestamp (unix millis)."), + Required: ptr.Bool(false), + Schema: &openapi3.SchemaOrRef{ + Schema: &openapi3.Schema{ + Type: ptrSchemaType(openapi3.SchemaTypeInteger), + Minimum: ptr.Float64(0), + }, + }, + }, +} + +var queryParameterEditedGt = openapi3.ParameterOrRef{ + Parameter: &openapi3.Parameter{ + Name: request.QueryParamEditedGt, + In: openapi3.ParameterInQuery, + Description: ptr.String("The result should contain only entries edited after this timestamp (unix millis)."), + Required: ptr.Bool(false), + Schema: &openapi3.SchemaOrRef{ + Schema: &openapi3.Schema{ + Type: ptrSchemaType(openapi3.SchemaTypeInteger), + Minimum: ptr.Float64(0), + }, + }, + }, +} + +var queryParameterIncludeSubspaces = openapi3.ParameterOrRef{ + Parameter: &openapi3.Parameter{ + Name: request.QueryParamIncludeSubspaces, + In: openapi3.ParameterInQuery, + Description: ptr.String("The result should contain entries from the desired space and of its subspaces."), + Required: ptr.Bool(false), + Schema: &openapi3.SchemaOrRef{ + Schema: &openapi3.Schema{ + Type: ptrSchemaType(openapi3.SchemaTypeBoolean), + Default: ptrptr(false), + }, + }, + }, +} diff --git a/app/api/openapi/pullreq.go b/app/api/openapi/pullreq.go index acb276c22..07a205f9c 100644 --- a/app/api/openapi/pullreq.go +++ b/app/api/openapi/pullreq.go @@ -378,6 +378,21 @@ var QueryParameterValueID = openapi3.ParameterOrRef{ }, } +var queryParameterIncludeDescription = openapi3.ParameterOrRef{ + Parameter: &openapi3.Parameter{ + Name: request.QueryParamIncludeDescription, + In: openapi3.ParameterInQuery, + Description: ptr.String("By providing this parameter the description would be included in the response."), + Required: ptr.Bool(false), + Schema: &openapi3.SchemaOrRef{ + Schema: &openapi3.Schema{ + Type: ptrSchemaType(openapi3.SchemaTypeBoolean), + Default: ptrptr(false), + }, + }, + }, +} + //nolint:funlen func pullReqOperations(reflector *openapi3.Reflector) { createPullReq := openapi3.Operation{} @@ -399,7 +414,8 @@ func pullReqOperations(reflector *openapi3.Reflector) { queryParameterSourceBranchPullRequest, queryParameterTargetBranchPullRequest, queryParameterQueryPullRequest, queryParameterCreatedByPullRequest, queryParameterOrder, queryParameterSortPullRequest, - queryParameterCreatedLt, queryParameterCreatedGt, + queryParameterCreatedLt, queryParameterCreatedGt, queryParameterEditedLt, queryParameterEditedGt, + queryParameterIncludeDescription, QueryParameterPage, QueryParameterLimit, QueryParameterLabelID, QueryParameterValueID) _ = reflector.SetRequest(&listPullReq, new(listPullReqRequest), http.MethodGet) diff --git a/app/api/openapi/space.go b/app/api/openapi/space.go index abb15f5ad..827690ea4 100644 --- a/app/api/openapi/space.go +++ b/app/api/openapi/space.go @@ -600,4 +600,23 @@ func spaceOperations(reflector *openapi3.Reflector) { _ = reflector.SetJSONResponse(&opUpdateLabelValue, new(usererror.Error), http.StatusNotFound) _ = reflector.Spec.AddOperation(http.MethodPatch, "/spaces/{space_ref}/labels/{key}/values/{value}", opUpdateLabelValue) + + listPullReq := openapi3.Operation{} + listPullReq.WithTags("space") + listPullReq.WithMapOfAnything(map[string]interface{}{"operationId": "listSpacePullReq"}) + listPullReq.WithParameters( + queryParameterStatePullRequest, queryParameterSourceRepoRefPullRequest, + queryParameterSourceBranchPullRequest, queryParameterTargetBranchPullRequest, + queryParameterQueryPullRequest, queryParameterCreatedByPullRequest, + queryParameterCreatedLt, queryParameterCreatedGt, queryParameterEditedLt, + queryParameterIncludeDescription, queryParameterIncludeSubspaces, + QueryParameterLimit, + QueryParameterLabelID, QueryParameterValueID) + _ = reflector.SetRequest(&listPullReq, new(listPullReqRequest), http.MethodGet) + _ = reflector.SetJSONResponse(&listPullReq, new([]types.PullReq), http.StatusOK) + _ = reflector.SetJSONResponse(&listPullReq, new(usererror.Error), http.StatusBadRequest) + _ = reflector.SetJSONResponse(&listPullReq, new(usererror.Error), http.StatusInternalServerError) + _ = reflector.SetJSONResponse(&listPullReq, new(usererror.Error), http.StatusUnauthorized) + _ = reflector.SetJSONResponse(&listPullReq, new(usererror.Error), http.StatusForbidden) + _ = reflector.Spec.AddOperation(http.MethodGet, "/spaces/{repo_ref}/pullreq", listPullReq) } diff --git a/app/api/request/common.go b/app/api/request/common.go index fb7888c08..cef57da4b 100644 --- a/app/api/request/common.go +++ b/app/api/request/common.go @@ -46,6 +46,8 @@ const ( QueryParamCreatedLt = "created_lt" QueryParamCreatedGt = "created_gt" + QueryParamEditedLt = "edited_lt" + QueryParamEditedGt = "edited_gt" QueryParamPage = "page" QueryParamLimit = "limit" @@ -150,6 +152,26 @@ func ParseCreated(r *http.Request) (types.CreatedFilter, error) { return filter, nil } +// ParseEdited extracts the edited filter from the url query param. +func ParseEdited(r *http.Request) (types.EditedFilter, error) { + filter := types.EditedFilter{} + + editedLt, err := QueryParamAsPositiveInt64OrDefault(r, QueryParamEditedLt, 0) + if err != nil { + return filter, fmt.Errorf("encountered error parsing edited lt: %w", err) + } + + editedGt, err := QueryParamAsPositiveInt64OrDefault(r, QueryParamEditedGt, 0) + if err != nil { + return filter, fmt.Errorf("encountered error parsing edited gt: %w", err) + } + + filter.EditedGt = editedGt + filter.EditedLt = editedLt + + return filter, nil +} + // GetContentEncodingFromHeadersOrDefault returns the content encoding from the request headers. func GetContentEncodingFromHeadersOrDefault(r *http.Request, dflt string) string { return GetHeaderOrDefault(r, HeaderContentEncoding, dflt) diff --git a/app/api/request/pullreq.go b/app/api/request/pullreq.go index fe3b48f83..a92aa5ff9 100644 --- a/app/api/request/pullreq.go +++ b/app/api/request/pullreq.go @@ -27,6 +27,8 @@ const ( PathParamPullReqCommentID = "pullreq_comment_id" PathParamReviewerID = "pullreq_reviewer_id" PathParamUserGroupID = "user_group_id" + + QueryParamIncludeDescription = "include_description" ) func GetPullReqNumberFromPath(r *http.Request) (int64, error) { @@ -89,20 +91,32 @@ func ParsePullReqFilter(r *http.Request) (*types.PullReqFilter, error) { return nil, fmt.Errorf("encountered error parsing pr created filter: %w", err) } + editedAtFilter, err := ParseEdited(r) + if err != nil { + return nil, fmt.Errorf("encountered error parsing pr edited filter: %w", err) + } + + includeDescription, err := QueryParamAsBoolOrDefault(r, QueryParamIncludeDescription, false) + if err != nil { + return nil, fmt.Errorf("encountered error parsing include description filter: %w", err) + } + return &types.PullReqFilter{ - Page: ParsePage(r), - Size: ParseLimit(r), - Query: ParseQuery(r), - CreatedBy: createdBy, - SourceRepoRef: r.URL.Query().Get("source_repo_ref"), - SourceBranch: r.URL.Query().Get("source_branch"), - TargetBranch: r.URL.Query().Get("target_branch"), - States: parsePullReqStates(r), - Sort: ParseSortPullReq(r), - Order: ParseOrder(r), - LabelID: labelID, - ValueID: valueID, - CreatedFilter: createdAtFilter, + Page: ParsePage(r), + Size: ParseLimit(r), + Query: ParseQuery(r), + CreatedBy: createdBy, + SourceRepoRef: r.URL.Query().Get("source_repo_ref"), + SourceBranch: r.URL.Query().Get("source_branch"), + TargetBranch: r.URL.Query().Get("target_branch"), + States: parsePullReqStates(r), + Sort: ParseSortPullReq(r), + Order: ParseOrder(r), + LabelID: labelID, + ValueID: valueID, + IncludeDescription: includeDescription, + CreatedFilter: createdAtFilter, + EditedFilter: editedAtFilter, }, nil } diff --git a/app/api/request/space.go b/app/api/request/space.go index 9ec55761a..7d6c2b562 100644 --- a/app/api/request/space.go +++ b/app/api/request/space.go @@ -15,6 +15,7 @@ package request import ( + "fmt" "net/http" "github.com/harness/gitness/types" @@ -23,6 +24,8 @@ import ( const ( PathParamSpaceRef = "space_ref" + + QueryParamIncludeSubspaces = "include_subspaces" ) func GetSpaceRefFromPath(r *http.Request) (string, error) { @@ -75,3 +78,12 @@ func ParseSpaceFilter(r *http.Request) (*types.SpaceFilter, error) { DeletedBeforeOrAt: deletedBeforeOrAt, }, nil } + +func GetIncludeSubspacesFromQuery(r *http.Request) (bool, error) { + v, err := QueryParamAsBoolOrDefault(r, QueryParamIncludeSubspaces, false) + if err != nil { + return false, fmt.Errorf("failed to parse include subspaces parameter: %w", err) + } + + return v, nil +} diff --git a/app/router/api.go b/app/router/api.go index 9eb7d1974..08dbad61b 100644 --- a/app/router/api.go +++ b/app/router/api.go @@ -278,6 +278,7 @@ func setupSpaces( r.Post("/export", handlerspace.HandleExport(spaceCtrl)) r.Get("/export-progress", handlerspace.HandleExportProgress(spaceCtrl)) r.Post("/public-access", handlerspace.HandleUpdatePublicAccess(spaceCtrl)) + r.Get("/pullreq", handlerspace.HandleListPullReqs(spaceCtrl)) r.Route("/members", func(r chi.Router) { r.Get("/", handlerspace.HandleMembershipList(spaceCtrl)) diff --git a/app/services/pullreq/service_list.go b/app/services/pullreq/service_list.go new file mode 100644 index 000000000..2e8d51cd9 --- /dev/null +++ b/app/services/pullreq/service_list.go @@ -0,0 +1,233 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pullreq + +import ( + "context" + "fmt" + + apiauth "github.com/harness/gitness/app/api/auth" + "github.com/harness/gitness/app/auth" + "github.com/harness/gitness/app/auth/authz" + "github.com/harness/gitness/app/services/label" + "github.com/harness/gitness/app/store" + "github.com/harness/gitness/errors" + "github.com/harness/gitness/git" + gitness_store "github.com/harness/gitness/store" + "github.com/harness/gitness/store/database/dbtx" + "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" + + "github.com/rs/zerolog/log" +) + +type ListService struct { + tx dbtx.Transactor + git git.Interface + authorizer authz.Authorizer + spaceStore store.SpaceStore + repoStore store.RepoStore + repoGitInfoCache store.RepoGitInfoCache + pullreqStore store.PullReqStore + labelSvc *label.Service +} + +func NewListService( + tx dbtx.Transactor, + git git.Interface, + authorizer authz.Authorizer, + spaceStore store.SpaceStore, + repoStore store.RepoStore, + repoGitInfoCache store.RepoGitInfoCache, + pullreqStore store.PullReqStore, + labelSvc *label.Service, +) *ListService { + return &ListService{ + tx: tx, + git: git, + authorizer: authorizer, + spaceStore: spaceStore, + repoStore: repoStore, + repoGitInfoCache: repoGitInfoCache, + pullreqStore: pullreqStore, + labelSvc: labelSvc, + } +} + +// ListForSpace returns a list of pull requests and their respective repositories for a specific space. +// +//nolint:gocognit +func (c *ListService) ListForSpace( + ctx context.Context, + session *auth.Session, + space *types.Space, + includeSubspaces bool, + filter *types.PullReqFilter, +) ([]types.PullReqRepo, error) { + // list of unsupported filter options + filter.Sort = enum.PullReqSortEdited // the only supported option, hardcoded in the SQL query + filter.Order = enum.OrderDesc // the only supported option, hardcoded in the SQL query + filter.Page = 0 // unsupported, pagination should be done with the EditedLt parameter + filter.EditedGt = 0 // unsupported + + if includeSubspaces { + subspaces, err := c.spaceStore.GetDescendantsData(ctx, space.ID) + if err != nil { + return nil, fmt.Errorf("failed to get space descendant data: %w", err) + } + + filter.SpaceIDs = make([]int64, 0, len(subspaces)) + for i := range subspaces { + filter.SpaceIDs = append(filter.SpaceIDs, subspaces[i].ID) + } + } else { + filter.SpaceIDs = []int64{space.ID} + } + + repoWhitelist := make(map[int64]struct{}) + + list := make([]*types.PullReq, 0, 16) + repoMap := make(map[int64]*types.Repository) + + for loadMore := true; loadMore; { + const prLimit = 100 + const repoLimit = 10 + + pullReqs, repoUnchecked, err := c.streamPullReqs(ctx, filter, prLimit, repoLimit, repoWhitelist) + if err != nil { + return nil, fmt.Errorf("failed to load pull requests: %w", err) + } + + loadMore = len(pullReqs) == prLimit || len(repoUnchecked) == repoLimit + if loadMore && len(pullReqs) > 0 { + filter.EditedLt = pullReqs[len(pullReqs)-1].Edited + } + + for repoID := range repoUnchecked { + repo, err := c.repoStore.Find(ctx, repoID) + if errors.Is(err, gitness_store.ErrResourceNotFound) { + filter.RepoIDBlacklist = append(filter.RepoIDBlacklist, repoID) + continue + } else if err != nil { + return nil, fmt.Errorf("failed to find repo: %w", err) + } + + err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoView) + switch { + case err == nil: + repoWhitelist[repoID] = struct{}{} + repoMap[repoID] = repo + case errors.Is(err, apiauth.ErrNotAuthorized): + filter.RepoIDBlacklist = append(filter.RepoIDBlacklist, repoID) + default: + return nil, fmt.Errorf("failed to check access check: %w", err) + } + } + + for _, pullReq := range pullReqs { + if _, ok := repoWhitelist[pullReq.TargetRepoID]; ok { + list = append(list, pullReq) + } + } + + if len(list) >= filter.Size { + list = list[:filter.Size] + loadMore = false + } + } + + if err := c.labelSvc.BackfillMany(ctx, list); err != nil { + return nil, fmt.Errorf("failed to backfill labels assigned to pull requests: %w", err) + } + + for _, pr := range list { + if err := c.BackfillStats(ctx, pr); err != nil { + log.Ctx(ctx).Warn().Err(err).Msg("failed to backfill PR stats") + } + } + + response := make([]types.PullReqRepo, len(list)) + for i := range list { + response[i] = types.PullReqRepo{ + PullRequest: list[i], + Repository: repoMap[list[i].TargetRepoID], + } + } + + return response, nil +} + +// streamPullReqs loads pull requests until it gets either pullReqLimit pull requests +// or newRepoLimit distinct repositories. +func (c *ListService) streamPullReqs( + ctx context.Context, + opts *types.PullReqFilter, + pullReqLimit, newRepoLimit int, + repoWhitelist map[int64]struct{}, +) ([]*types.PullReq, map[int64]struct{}, error) { + ctx, cancelFn := context.WithCancel(ctx) + defer cancelFn() + + repoUnchecked := map[int64]struct{}{} + + 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 _, ok := repoWhitelist[pr.TargetRepoID]; !ok { + repoUnchecked[pr.TargetRepoID] = struct{}{} + } + + pullReqs = append(pullReqs, pr) + + if len(pullReqs) >= pullReqLimit || len(repoUnchecked) >= newRepoLimit { + break + } + } + + if err := <-chErr; err != nil { + return nil, nil, fmt.Errorf("failed to stream pull requests: %w", err) + } + + return pullReqs, repoUnchecked, nil +} + +func (c *ListService) BackfillStats(ctx context.Context, pr *types.PullReq) error { + s := pr.Stats.DiffStats + if s.Commits != nil && s.FilesChanged != nil && s.Additions != nil && s.Deletions != nil { + return nil + } + + repoGitInfo, err := c.repoGitInfoCache.Get(ctx, pr.TargetRepoID) + if err != nil { + return fmt.Errorf("failed get repo git info to fetch diff stats: %w", err) + } + + output, err := c.git.DiffStats(ctx, &git.DiffParams{ + ReadParams: git.CreateReadParams(repoGitInfo), + BaseRef: pr.MergeBaseSHA, + HeadRef: pr.SourceSHA, + }) + if err != nil { + return fmt.Errorf("failed get diff stats: %w", err) + } + + pr.Stats.DiffStats = types.NewDiffStats(output.Commits, output.FilesChanged, output.Additions, output.Deletions) + + return nil +} diff --git a/app/services/pullreq/wire.go b/app/services/pullreq/wire.go index b59404341..c2d9e6284 100644 --- a/app/services/pullreq/wire.go +++ b/app/services/pullreq/wire.go @@ -17,15 +17,18 @@ package pullreq import ( "context" + "github.com/harness/gitness/app/auth/authz" gitevents "github.com/harness/gitness/app/events/git" pullreqevents "github.com/harness/gitness/app/events/pullreq" "github.com/harness/gitness/app/services/codecomments" + "github.com/harness/gitness/app/services/label" "github.com/harness/gitness/app/sse" "github.com/harness/gitness/app/store" "github.com/harness/gitness/app/url" "github.com/harness/gitness/events" "github.com/harness/gitness/git" "github.com/harness/gitness/pubsub" + "github.com/harness/gitness/store/database/dbtx" "github.com/harness/gitness/types" "github.com/google/wire" @@ -33,6 +36,7 @@ import ( var WireSet = wire.NewSet( ProvideService, + ProvideListService, ) func ProvideService(ctx context.Context, @@ -72,3 +76,25 @@ func ProvideService(ctx context.Context, sseStreamer, ) } + +func ProvideListService( + tx dbtx.Transactor, + git git.Interface, + authorizer authz.Authorizer, + spaceStore store.SpaceStore, + repoStore store.RepoStore, + repoGitInfoCache store.RepoGitInfoCache, + pullreqStore store.PullReqStore, + labelSvc *label.Service, +) *ListService { + return NewListService( + tx, + git, + authorizer, + spaceStore, + repoStore, + repoGitInfoCache, + pullreqStore, + labelSvc, + ) +} diff --git a/app/store/database.go b/app/store/database.go index 0e52a552f..ac808a5a9 100644 --- a/app/store/database.go +++ b/app/store/database.go @@ -372,8 +372,11 @@ type ( // Count of pull requests in a space. Count(ctx context.Context, opts *types.PullReqFilter) (int64, error) - // List returns a list of pull requests in a space. + // List returns a list of pull requests in a repository. List(ctx context.Context, opts *types.PullReqFilter) ([]*types.PullReq, error) + + // Stream returns streams pull requests from repositories. + Stream(ctx context.Context, opts *types.PullReqFilter) (<-chan *types.PullReq, <-chan error) } PullReqActivityStore interface { diff --git a/app/store/database/pullreq.go b/app/store/database/pullreq.go index 149d4f6fc..67832e4c1 100644 --- a/app/store/database/pullreq.go +++ b/app/store/database/pullreq.go @@ -98,7 +98,7 @@ type pullReq struct { } const ( - pullReqColumns = ` + pullReqColumnsNoDescription = ` pullreq_id ,pullreq_version ,pullreq_number @@ -112,7 +112,6 @@ const ( ,pullreq_comment_count ,pullreq_unresolved_count ,pullreq_title - ,pullreq_description ,pullreq_source_repo_id ,pullreq_source_branch ,pullreq_source_sha @@ -132,6 +131,9 @@ const ( ,pullreq_additions ,pullreq_deletions` + pullReqColumns = pullReqColumnsNoDescription + ` + ,pullreq_description` + pullReqSelectBase = ` SELECT` + pullReqColumns + ` FROM pullreqs` @@ -457,16 +459,7 @@ func (s *PullReqStore) Count(ctx context.Context, opts *types.PullReqFilter) (in // List returns a list of pull requests for a repo. func (s *PullReqStore) List(ctx context.Context, opts *types.PullReqFilter) ([]*types.PullReq, error) { - var stmt squirrel.SelectBuilder - - if len(opts.LabelID) > 0 || len(opts.ValueID) > 0 { - stmt = database.Builder.Select("DISTINCT " + pullReqColumns) - } else { - stmt = database.Builder.Select(pullReqColumns) - } - stmt = stmt.From("pullreqs") - - s.applyFilter(&stmt, opts) + stmt := s.listQuery(opts) stmt = stmt.Limit(database.Limit(opts.Size)) stmt = stmt.Offset(database.Offset(opts.Page, opts.Size)) @@ -498,6 +491,75 @@ func (s *PullReqStore) List(ctx context.Context, opts *types.PullReqFilter) ([]* return result, nil } +// Stream returns a list of pull requests for a repo. +func (s *PullReqStore) Stream(ctx context.Context, opts *types.PullReqFilter) (<-chan *types.PullReq, <-chan error) { + stmt := s.listQuery(opts) + + stmt = stmt.OrderBy("pullreq_edited desc") + + chPRs := make(chan *types.PullReq) + chErr := make(chan error, 1) + + go func() { + defer close(chPRs) + defer close(chErr) + + sql, args, err := stmt.ToSql() + if err != nil { + chErr <- errors.Wrap(err, "Failed to convert query to sql") + return + } + + db := dbtx.GetAccessor(ctx, s.db) + + rows, err := db.QueryxContext(ctx, sql, args...) + if err != nil { + chErr <- database.ProcessSQLErrorf(ctx, err, "Failed to execute stream query") + return + } + + defer func() { _ = rows.Close() }() + + for rows.Next() { + var prData pullReq + err = rows.StructScan(&prData) + if err != nil { + chErr <- fmt.Errorf("failed to scan pull request: %w", err) + return + } + + chPRs <- s.mapPullReq(ctx, &prData) + } + + if err := rows.Err(); err != nil { + chErr <- fmt.Errorf("failed to scan pull request: %w", err) + } + }() + + return chPRs, chErr +} + +func (s *PullReqStore) listQuery(opts *types.PullReqFilter) squirrel.SelectBuilder { + var stmt squirrel.SelectBuilder + + columns := pullReqColumnsNoDescription + if opts.IncludeDescription { + columns = pullReqColumns + } + + if len(opts.LabelID) > 0 || len(opts.ValueID) > 0 { + stmt = database.Builder.Select("DISTINCT " + columns) + } else { + stmt = database.Builder.Select(columns) + } + + stmt = stmt.From("pullreqs") + + s.applyFilter(&stmt, opts) + + return stmt +} + func (*PullReqStore) applyFilter(stmt *squirrel.SelectBuilder, opts *types.PullReqFilter) { if len(opts.States) == 1 { *stmt = stmt.Where("pullreq_state = ?", opts.States[0]) @@ -537,6 +599,28 @@ func (*PullReqStore) applyFilter(stmt *squirrel.SelectBuilder, opts *types.PullR *stmt = stmt.Where("pullreq_created > ?", opts.CreatedGt) } + if opts.EditedLt > 0 { + *stmt = stmt.Where("pullreq_edited < ?", opts.EditedLt) + } + + if opts.EditedGt > 0 { + *stmt = stmt.Where("pullreq_edited > ?", opts.EditedGt) + } + + if len(opts.SpaceIDs) == 1 { + *stmt = stmt.InnerJoin("repositories ON repo_id = pullreq_target_repo_id") + *stmt = stmt.Where("repo_parent_id = ?", opts.SpaceIDs[0]) + } else if len(opts.SpaceIDs) > 1 { + *stmt = stmt.InnerJoin("repositories ON repo_id = pullreq_target_repo_id") + *stmt = stmt.Where(squirrel.Eq{"repo_parent_id": opts.SpaceIDs}) + } + + if len(opts.RepoIDBlacklist) == 1 { + *stmt = stmt.Where("pullreq_target_repo_id <> ?", opts.RepoIDBlacklist[0]) + } else if len(opts.RepoIDBlacklist) > 1 { + *stmt = stmt.Where(squirrel.NotEq{"pullreq_target_repo_id": opts.RepoIDBlacklist}) + } + // labels if len(opts.LabelID) == 0 && len(opts.ValueID) == 0 { diff --git a/cmd/gitness/wire.go b/cmd/gitness/wire.go index d842dbdd5..8fddd3e10 100644 --- a/cmd/gitness/wire.go +++ b/cmd/gitness/wire.go @@ -19,9 +19,6 @@ package main import ( "context" - "github.com/harness/gitness/app/api/controller/system" - "github.com/harness/gitness/app/api/controller/usergroup" - "github.com/harness/gitness/app/api/controller/aiagent" "github.com/harness/gitness/app/api/controller/capabilities" checkcontroller "github.com/harness/gitness/app/api/controller/check" @@ -44,10 +41,12 @@ import ( "github.com/harness/gitness/app/api/controller/service" "github.com/harness/gitness/app/api/controller/serviceaccount" "github.com/harness/gitness/app/api/controller/space" + "github.com/harness/gitness/app/api/controller/system" "github.com/harness/gitness/app/api/controller/template" controllertrigger "github.com/harness/gitness/app/api/controller/trigger" "github.com/harness/gitness/app/api/controller/upload" "github.com/harness/gitness/app/api/controller/user" + "github.com/harness/gitness/app/api/controller/usergroup" controllerwebhook "github.com/harness/gitness/app/api/controller/webhook" "github.com/harness/gitness/app/api/openapi" "github.com/harness/gitness/app/auth/authn" diff --git a/cmd/gitness/wire_gen.go b/cmd/gitness/wire_gen.go index ccaa53409..acc3923ae 100644 --- a/cmd/gitness/wire_gen.go +++ b/cmd/gitness/wire_gen.go @@ -264,6 +264,10 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro spaceIdentifier := check.ProvideSpaceIdentifierCheck() secretStore := database.ProvideSecretStore(db) connectorStore := database.ProvideConnectorStore(db) + repoGitInfoView := database.ProvideRepoGitInfoView(db) + repoGitInfoCache := cache.ProvideRepoGitInfoCache(repoGitInfoView) + pullReqStore := database.ProvidePullReqStore(db, principalInfoCache) + listService := pullreq.ProvideListService(transactor, gitInterface, authorizer, spaceStore, repoStore, repoGitInfoCache, pullReqStore, labelService) exporterRepository, err := exporter.ProvideSpaceExporter(provider, gitInterface, repoStore, jobScheduler, executor, encrypter, streamer) if err != nil { return nil, err @@ -286,7 +290,7 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro factory := infraprovider.ProvideFactory(dockerProvider) infraproviderService := infraprovider2.ProvideInfraProvider(transactor, infraProviderResourceStore, infraProviderConfigStore, infraProviderTemplateStore, factory, spaceStore) gitspaceService := gitspace.ProvideGitspace(transactor, gitspaceConfigStore, gitspaceInstanceStore, spaceStore, infraproviderService) - spaceController := space.ProvideController(config, transactor, provider, streamer, spaceIdentifier, authorizer, spacePathStore, pipelineStore, secretStore, connectorStore, templateStore, spaceStore, repoStore, principalStore, repoController, membershipStore, repository, exporterRepository, resourceLimiter, publicaccessService, auditService, gitspaceService, labelService, instrumentService) + spaceController := space.ProvideController(config, transactor, provider, streamer, spaceIdentifier, authorizer, spacePathStore, pipelineStore, secretStore, connectorStore, templateStore, spaceStore, repoStore, principalStore, repoController, membershipStore, listService, repository, exporterRepository, resourceLimiter, publicaccessService, auditService, gitspaceService, labelService, instrumentService) reporter2, err := events4.ProvideReporter(eventsSystem) if err != nil { return nil, err @@ -297,7 +301,6 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro connectorController := connector.ProvideController(connectorStore, authorizer, spaceStore) templateController := template.ProvideController(templateStore, authorizer, spaceStore) pluginController := plugin.ProvideController(pluginStore) - pullReqStore := database.ProvidePullReqStore(db, principalInfoCache) pullReqActivityStore := database.ProvidePullReqActivityStore(db, principalInfoCache) codeCommentView := database.ProvideCodeCommentView(db) pullReqReviewStore := database.ProvidePullReqReviewStore(db) @@ -318,15 +321,13 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro if err != nil { return nil, err } - repoGitInfoView := database.ProvideRepoGitInfoView(db) - repoGitInfoCache := cache.ProvideRepoGitInfoCache(repoGitInfoView) pullreqService, err := pullreq.ProvideService(ctx, config, readerFactory, eventsReaderFactory, reporter3, gitInterface, repoGitInfoCache, repoStore, pullReqStore, pullReqActivityStore, principalInfoCache, codeCommentView, migrator, pullReqFileViewStore, pubSub, provider, streamer) if err != nil { return nil, err } pullReq := migrate.ProvidePullReqImporter(provider, gitInterface, principalStore, repoStore, pullReqStore, pullReqActivityStore, transactor) searchService := usergroup.ProvideSearchService() - pullreqController := pullreq2.ProvideController(transactor, provider, authorizer, pullReqStore, pullReqActivityStore, codeCommentView, pullReqReviewStore, pullReqReviewerStore, repoStore, principalStore, userGroupStore, userGroupReviewersStore, principalInfoCache, pullReqFileViewStore, membershipStore, checkStore, gitInterface, reporter3, migrator, pullreqService, protectionManager, streamer, codeownersService, lockerLocker, pullReq, labelService, instrumentService, searchService) + pullreqController := pullreq2.ProvideController(transactor, provider, authorizer, pullReqStore, pullReqActivityStore, codeCommentView, pullReqReviewStore, pullReqReviewerStore, repoStore, principalStore, userGroupStore, userGroupReviewersStore, principalInfoCache, pullReqFileViewStore, membershipStore, checkStore, gitInterface, reporter3, migrator, pullreqService, listService, protectionManager, streamer, codeownersService, lockerLocker, pullReq, labelService, instrumentService, searchService) webhookConfig := server.ProvideWebhookConfig(config) webhookStore := database.ProvideWebhookStore(db) webhookExecutionStore := database.ProvideWebhookExecutionStore(db) diff --git a/types/list_filters.go b/types/list_filters.go index 7e61a1735..2b10c936b 100644 --- a/types/list_filters.go +++ b/types/list_filters.go @@ -24,3 +24,8 @@ type CreatedFilter struct { CreatedGt int64 `json:"created_gt"` CreatedLt int64 `json:"created_lt"` } + +type EditedFilter struct { + EditedGt int64 `json:"edited_gt"` + EditedLt int64 `json:"edited_lt"` +} diff --git a/types/pullreq.go b/types/pullreq.go index 6ce4895e3..61ed4438e 100644 --- a/types/pullreq.go +++ b/types/pullreq.go @@ -94,21 +94,27 @@ type PullReqStats struct { // PullReqFilter stores pull request query parameters. type PullReqFilter struct { - Page int `json:"page"` - Size int `json:"size"` - Query string `json:"query"` - CreatedBy []int64 `json:"created_by"` - SourceRepoID int64 `json:"-"` // caller should use source_repo_ref - SourceRepoRef string `json:"source_repo_ref"` - SourceBranch string `json:"source_branch"` - TargetRepoID int64 `json:"-"` - TargetBranch string `json:"target_branch"` - States []enum.PullReqState `json:"state"` - Sort enum.PullReqSort `json:"sort"` - Order enum.Order `json:"order"` - LabelID []int64 `json:"label_id"` - ValueID []int64 `json:"value_id"` + Page int `json:"page"` + Size int `json:"size"` + Query string `json:"query"` + CreatedBy []int64 `json:"created_by"` + SourceRepoID int64 `json:"-"` // caller should use source_repo_ref + SourceRepoRef string `json:"source_repo_ref"` + SourceBranch string `json:"source_branch"` + TargetRepoID int64 `json:"-"` + TargetBranch string `json:"target_branch"` + States []enum.PullReqState `json:"state"` + Sort enum.PullReqSort `json:"sort"` + Order enum.Order `json:"order"` + LabelID []int64 `json:"label_id"` + ValueID []int64 `json:"value_id"` + IncludeDescription bool `json:"include_description"` CreatedFilter + EditedFilter + + // internal use only + SpaceIDs []int64 + RepoIDBlacklist []int64 } // PullReqReview holds pull request review. @@ -203,3 +209,8 @@ type MergeViolations struct { ConflictFiles []string `json:"conflict_files,omitempty"` RuleViolations []RuleViolations `json:"rule_violations,omitempty"` } + +type PullReqRepo struct { + PullRequest *PullReq `json:"pull_request"` + Repository *Repository `json:"repository"` +} diff --git a/types/space.go b/types/space.go index f2f3d7dbd..6460be892 100644 --- a/types/space.go +++ b/types/space.go @@ -49,7 +49,7 @@ type SpaceParentData struct { ParentID int64 `json:"parent_id"` } -// Stores spaces query parameters. +// SpaceFilter stores spaces query parameters. type SpaceFilter struct { Page int `json:"page"` Size int `json:"size"`