From 069a35a4d8e3d5087671acaa1b5951dbe727bb81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ga=C4=87e=C5=A1a?= Date: Tue, 15 Oct 2024 16:49:02 +0000 Subject: [PATCH] feat: [CODE-2484]: add API to squash commits on a branch (#2793) * add openapi entry for squash * minor fixes * squash api --- .../pullreq/comment_apply_suggestions.go | 5 +- app/api/controller/pullreq/mapper.go | 27 -- app/api/controller/pullreq/merge.go | 9 +- app/api/controller/repo/rebase.go | 31 +- app/api/controller/repo/squash.go | 272 ++++++++++++++++++ app/api/controller/util.go | 12 + app/api/handler/repo/squash.go | 57 ++++ app/api/openapi/repo.go | 18 ++ app/router/api.go | 1 + git/merge.go | 16 +- types/rebase.go | 9 + 11 files changed, 405 insertions(+), 52 deletions(-) delete mode 100644 app/api/controller/pullreq/mapper.go create mode 100644 app/api/controller/repo/squash.go create mode 100644 app/api/handler/repo/squash.go diff --git a/app/api/controller/pullreq/comment_apply_suggestions.go b/app/api/controller/pullreq/comment_apply_suggestions.go index 54fb7419e..9c54deb92 100644 --- a/app/api/controller/pullreq/comment_apply_suggestions.go +++ b/app/api/controller/pullreq/comment_apply_suggestions.go @@ -23,7 +23,6 @@ import ( "github.com/harness/gitness/app/api/controller" "github.com/harness/gitness/app/api/usererror" "github.com/harness/gitness/app/auth" - "github.com/harness/gitness/app/bootstrap" "github.com/harness/gitness/app/services/instrument" "github.com/harness/gitness/app/services/protection" "github.com/harness/gitness/contextutil" @@ -304,9 +303,9 @@ func (c *Controller) CommentApplySuggestions( Title: in.Title, Message: in.Message, Branch: pr.SourceBranch, - Committer: identityFromPrincipalInfo(*bootstrap.NewSystemServiceSession().Principal.ToPrincipalInfo()), + Committer: controller.SystemServicePrincipalInfo(), CommitterDate: &now, - Author: identityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()), + Author: controller.IdentityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()), AuthorDate: &now, Actions: actions, }) diff --git a/app/api/controller/pullreq/mapper.go b/app/api/controller/pullreq/mapper.go deleted file mode 100644 index 7e2c5c284..000000000 --- a/app/api/controller/pullreq/mapper.go +++ /dev/null @@ -1,27 +0,0 @@ -// 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 ( - "github.com/harness/gitness/git" - "github.com/harness/gitness/types" -) - -func identityFromPrincipalInfo(p types.PrincipalInfo) *git.Identity { - return &git.Identity{ - Name: p.DisplayName, - Email: p.Email, - } -} diff --git a/app/api/controller/pullreq/merge.go b/app/api/controller/pullreq/merge.go index c0f7dad1e..bd6dba2ae 100644 --- a/app/api/controller/pullreq/merge.go +++ b/app/api/controller/pullreq/merge.go @@ -24,7 +24,6 @@ import ( "github.com/harness/gitness/app/api/controller" "github.com/harness/gitness/app/api/usererror" "github.com/harness/gitness/app/auth" - "github.com/harness/gitness/app/bootstrap" pullreqevents "github.com/harness/gitness/app/events/pullreq" "github.com/harness/gitness/app/paths" "github.com/harness/gitness/app/services/codeowners" @@ -371,9 +370,9 @@ func (c *Controller) Merge( switch in.Method { case enum.MergeMethodMerge: - author = identityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()) + author = controller.IdentityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()) case enum.MergeMethodSquash: - author = identityFromPrincipalInfo(pr.Author) + author = controller.IdentityFromPrincipalInfo(pr.Author) case enum.MergeMethodRebase, enum.MergeMethodFastForward: author = nil // Not important for these merge methods: the author info in the commits will be preserved. } @@ -382,9 +381,9 @@ func (c *Controller) Merge( switch in.Method { case enum.MergeMethodMerge, enum.MergeMethodSquash: - committer = identityFromPrincipalInfo(*bootstrap.NewSystemServiceSession().Principal.ToPrincipalInfo()) + committer = controller.SystemServicePrincipalInfo() case enum.MergeMethodRebase: - committer = identityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()) + committer = controller.IdentityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()) case enum.MergeMethodFastForward: committer = nil // Not important for fast-forward merge } diff --git a/app/api/controller/repo/rebase.go b/app/api/controller/repo/rebase.go index 3dfd29085..a9c31771e 100644 --- a/app/api/controller/repo/rebase.go +++ b/app/api/controller/repo/rebase.go @@ -31,6 +31,8 @@ import ( type RebaseInput struct { BaseBranch string `json:"base_branch"` + BaseCommitSHA sha.SHA `json:"base_commit_sha"` + HeadBranch string `json:"head_branch"` HeadCommitSHA sha.SHA `json:"head_commit_sha"` @@ -40,8 +42,8 @@ type RebaseInput struct { } func (in *RebaseInput) validate() error { - if in.BaseBranch == "" { - return usererror.BadRequest("Base branch name must be provided") + if in.BaseBranch == "" && in.BaseCommitSHA.IsEmpty() { + return usererror.BadRequest("Either base branch or base commit SHA name must be provided") } if in.HeadBranch == "" { @@ -115,22 +117,27 @@ func (c *Controller) Rebase( return nil, nil, fmt.Errorf("failed to get head branch: %w", err) } - baseBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{ - ReadParams: readParams, - BranchName: in.BaseBranch, - }) - if err != nil { - return nil, nil, fmt.Errorf("failed to get base branch: %w", err) - } - if !headBranch.Branch.SHA.Equal(in.HeadCommitSHA) { return nil, nil, usererror.BadRequestf("The commit %s isn't the latest commit on the branch %s", in.HeadCommitSHA, headBranch.Branch.Name) } + baseCommitSHA := in.BaseCommitSHA + if baseCommitSHA.IsEmpty() { + baseBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{ + ReadParams: readParams, + BranchName: in.BaseBranch, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to get base branch: %w", err) + } + + baseCommitSHA = baseBranch.Branch.SHA + } + isAncestor, err := c.git.IsAncestor(ctx, git.IsAncestorParams{ ReadParams: readParams, - AncestorCommitSHA: baseBranch.Branch.SHA, + AncestorCommitSHA: baseCommitSHA, DescendantCommitSHA: headBranch.Branch.SHA, }) if err != nil { @@ -159,7 +166,7 @@ func (c *Controller) Rebase( mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ WriteParams: writeParams, - BaseBranch: in.BaseBranch, + BaseSHA: baseCommitSHA, HeadRepoUID: repo.GitUID, HeadBranch: in.HeadBranch, RefType: refType, diff --git a/app/api/controller/repo/squash.go b/app/api/controller/repo/squash.go new file mode 100644 index 000000000..45ca3014f --- /dev/null +++ b/app/api/controller/repo/squash.go @@ -0,0 +1,272 @@ +// 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 repo + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/harness/gitness/app/api/controller" + "github.com/harness/gitness/app/api/usererror" + "github.com/harness/gitness/app/auth" + "github.com/harness/gitness/app/services/protection" + "github.com/harness/gitness/git" + gitenum "github.com/harness/gitness/git/enum" + "github.com/harness/gitness/git/sha" + "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" +) + +type SquashInput struct { + BaseBranch string `json:"base_branch"` + BaseCommitSHA sha.SHA `json:"base_commit_sha"` + + HeadBranch string `json:"head_branch"` + HeadCommitSHA sha.SHA `json:"head_commit_sha"` + + Title string `json:"title"` + Message string `json:"message"` + + DryRun bool `json:"dry_run"` + DryRunRules bool `json:"dry_run_rules"` + BypassRules bool `json:"bypass_rules"` +} + +func (in *SquashInput) validate() error { + if in.BaseBranch == "" && in.BaseCommitSHA.IsEmpty() { + return usererror.BadRequest("Either base branch or base commit SHA name must be provided") + } + + if in.HeadBranch == "" { + return usererror.BadRequest("Head branch name must be provided") + } + + if in.HeadCommitSHA.IsEmpty() { + return usererror.BadRequest("Head branch commit SHA must be provided") + } + + // cleanup title / message (NOTE: git doesn't support white space only) + in.Title = strings.TrimSpace(in.Title) + in.Message = strings.TrimSpace(in.Message) + + return nil +} + +// Squash squashes all commits since merge-base to the latest commit of the base branch. +// This operation alters history of the base branch and therefore is considered a force push. +// +//nolint:gocognit +func (c *Controller) Squash( + ctx context.Context, + session *auth.Session, + repoRef string, + in *SquashInput, +) (*types.SquashResponse, *types.MergeViolations, error) { + if err := in.validate(); err != nil { + return nil, nil, err + } + + repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush) + if err != nil { + return nil, nil, fmt.Errorf("failed to acquire access to repo: %w", err) + } + + protectionRules, isRepoOwner, err := c.fetchRules(ctx, session, repo) + if err != nil { + return nil, nil, fmt.Errorf("failed to fetch rules: %w", err) + } + + violations, err := protectionRules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{ + ResolveUserGroupID: c.userGroupService.ListUserIDsByGroupIDs, + Actor: &session.Principal, + AllowBypass: in.BypassRules, + IsRepoOwner: isRepoOwner, + Repo: repo, + RefAction: protection.RefActionUpdateForce, + RefType: protection.RefTypeBranch, + RefNames: []string{in.HeadBranch}, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to verify protection rules: %w", err) + } + + if in.DryRunRules { + // DryRunRules is true: Just return rule violations and don't squash commits. + return &types.SquashResponse{ + RuleViolations: violations, + DryRunRules: true, + }, nil, nil + } + + if protection.IsCritical(violations) { + return nil, &types.MergeViolations{ + RuleViolations: violations, + Message: protection.GenerateErrorMessageForBlockingViolations(violations), + }, nil + } + + readParams := git.CreateReadParams(repo) + + headBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{ + ReadParams: readParams, + BranchName: in.HeadBranch, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to get head branch: %w", err) + } + + if !headBranch.Branch.SHA.Equal(in.HeadCommitSHA) { + return nil, nil, usererror.BadRequestf("The commit %s isn't the latest commit on the branch %s", + in.HeadCommitSHA, headBranch.Branch.Name) + } + + baseCommitSHA := in.BaseCommitSHA + if baseCommitSHA.IsEmpty() { + baseBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{ + ReadParams: readParams, + BranchName: in.BaseBranch, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to get base branch: %w", err) + } + + baseCommitSHA = baseBranch.Branch.SHA + } + + writeParams, err := controller.CreateRPCInternalWriteParams(ctx, c.urlProvider, session, repo) + if err != nil { + return nil, nil, fmt.Errorf("failed to create RPC write params: %w", err) + } + + refType := gitenum.RefTypeBranch + refName := in.HeadBranch + if in.DryRun { + refType = gitenum.RefTypeUndefined + refName = "" + } + + mergeBase, err := c.git.MergeBase(ctx, git.MergeBaseParams{ + ReadParams: readParams, + Ref1: baseCommitSHA.String(), + Ref2: headBranch.Branch.SHA.String(), + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to find merge base: %w", err) + } + + const commitBulletLimit = 50 + + commits, err := c.git.ListCommits(ctx, &git.ListCommitsParams{ + ReadParams: readParams, + GitREF: in.HeadCommitSHA.String(), + After: mergeBase.MergeBaseSHA.String(), + Limit: commitBulletLimit, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to read list of commits: %w", err) + } + + commitCount := len(commits.Commits) + + if commitCount == 0 { + return nil, nil, usererror.BadRequest("Failed to find commits between head and base") + } + + commit0Title, commit0Message := splitTitleAndMessage(commits.Commits[0].Message) + + if in.Title == "" { + in.Title = commit0Title + if commitCount > 1 { + in.Title = fmt.Sprintf("Squashed %d commits", commits.TotalCommits) + } + } + + if in.Message == "" { + in.Message = commit0Message + if commitCount > 1 { + sb := strings.Builder{} + for i := range min(commitBulletLimit, len(commits.Commits)) { + sb.WriteString("* ") + sb.WriteString(commits.Commits[i].Title) + sb.WriteByte('\n') + } + if otherCommitCount := commits.TotalCommits - len(commits.Commits); otherCommitCount > 0 { + sb.WriteString(fmt.Sprintf("* and %d more commits\n", otherCommitCount)) + } + + in.Message = sb.String() + } + } + + author := controller.IdentityFromPrincipalInfo(*session.Principal.ToPrincipalInfo()) + committer := controller.SystemServicePrincipalInfo() + now := time.Now() + + mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ + WriteParams: writeParams, + BaseSHA: mergeBase.MergeBaseSHA, + HeadRepoUID: repo.GitUID, + HeadBranch: in.HeadBranch, + Title: in.Title, + Message: in.Message, + RefType: refType, + RefName: refName, + Committer: committer, + CommitterDate: &now, + Author: author, + AuthorDate: &now, + HeadExpectedSHA: in.HeadCommitSHA, + Method: gitenum.MergeMethodSquash, + }) + if err != nil { + return nil, nil, fmt.Errorf("squash execution failed: %w", err) + } + + if in.DryRun { + // DryRun is true: Just return rule violations and list of conflicted files. + // No reference is updated, so don't return the resulting commit SHA. + return &types.SquashResponse{ + RuleViolations: violations, + DryRun: true, + ConflictFiles: mergeOutput.ConflictFiles, + }, nil, nil + } + + if mergeOutput.MergeSHA.IsEmpty() || len(mergeOutput.ConflictFiles) > 0 { + return nil, &types.MergeViolations{ + ConflictFiles: mergeOutput.ConflictFiles, + RuleViolations: violations, + Message: fmt.Sprintf("Squash blocked by conflicting files: %v", mergeOutput.ConflictFiles), + }, nil + } + + return &types.SquashResponse{ + NewHeadBranchSHA: mergeOutput.MergeSHA, + RuleViolations: violations, + }, nil, nil +} + +func splitTitleAndMessage(message string) (string, string) { + message = strings.TrimSpace(message) + + idx := strings.Index(message, "\n") + if idx < 0 { + return message, "" + } + + return message[:idx], strings.TrimLeft(message[idx+1:], "\n") +} diff --git a/app/api/controller/util.go b/app/api/controller/util.go index df34f892e..d6bc328da 100644 --- a/app/api/controller/util.go +++ b/app/api/controller/util.go @@ -19,6 +19,7 @@ import ( "fmt" "github.com/harness/gitness/app/auth" + "github.com/harness/gitness/app/bootstrap" "github.com/harness/gitness/app/githook" "github.com/harness/gitness/app/url" "github.com/harness/gitness/git" @@ -193,3 +194,14 @@ func MapSignature(s *git.Signature) (*types.Signature, error) { When: s.When, }, nil } + +func IdentityFromPrincipalInfo(p types.PrincipalInfo) *git.Identity { + return &git.Identity{ + Name: p.DisplayName, + Email: p.Email, + } +} + +func SystemServicePrincipalInfo() *git.Identity { + return IdentityFromPrincipalInfo(*bootstrap.NewSystemServiceSession().Principal.ToPrincipalInfo()) +} diff --git a/app/api/handler/repo/squash.go b/app/api/handler/repo/squash.go new file mode 100644 index 000000000..713f935bf --- /dev/null +++ b/app/api/handler/repo/squash.go @@ -0,0 +1,57 @@ +// 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 repo + +import ( + "encoding/json" + "net/http" + + "github.com/harness/gitness/app/api/controller/repo" + "github.com/harness/gitness/app/api/render" + "github.com/harness/gitness/app/api/request" +) + +func HandleSquash(repoCtrl *repo.Controller) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + session, _ := request.AuthSessionFrom(ctx) + + repoRef, err := request.GetRepoRefFromPath(r) + if err != nil { + render.TranslatedUserError(ctx, w, err) + return + } + + in := new(repo.SquashInput) + err = json.NewDecoder(r.Body).Decode(in) + if err != nil { + render.BadRequestf(ctx, w, "Invalid Request Body: %s.", err) + return + } + + result, violation, err := repoCtrl.Squash(ctx, session, repoRef, in) + if err != nil { + render.TranslatedUserError(ctx, w, err) + return + } + if violation != nil { + render.Unprocessable(w, violation) + return + } + + render.JSON(w, http.StatusOK, result) + } +} diff --git a/app/api/openapi/repo.go b/app/api/openapi/repo.go index f93e52ec0..b4534ac69 100644 --- a/app/api/openapi/repo.go +++ b/app/api/openapi/repo.go @@ -1411,4 +1411,22 @@ func repoOperations(reflector *openapi3.Reflector) { _ = reflector.SetJSONResponse(&opRebaseBranch, new(types.MergeViolations), http.StatusUnprocessableEntity) _ = reflector.Spec.AddOperation(http.MethodPost, "/repos/{repo_ref}/rebase", opRebaseBranch) + + opSquashBranch := openapi3.Operation{} + opSquashBranch.WithTags("repository") + opSquashBranch.WithMapOfAnything( + map[string]interface{}{"operationId": "squashBranch"}) + _ = reflector.SetRequest(&opSquashBranch, &struct { + repoRequest + repo.SquashInput + }{}, http.MethodPost) + _ = reflector.SetJSONResponse(&opSquashBranch, new(types.SquashResponse), http.StatusOK) + _ = reflector.SetJSONResponse(&opSquashBranch, new(usererror.Error), http.StatusBadRequest) + _ = reflector.SetJSONResponse(&opSquashBranch, new(usererror.Error), http.StatusInternalServerError) + _ = reflector.SetJSONResponse(&opSquashBranch, new(usererror.Error), http.StatusUnauthorized) + _ = reflector.SetJSONResponse(&opSquashBranch, new(usererror.Error), http.StatusForbidden) + _ = reflector.SetJSONResponse(&opSquashBranch, new(usererror.Error), http.StatusNotFound) + _ = reflector.SetJSONResponse(&opSquashBranch, new(types.MergeViolations), http.StatusUnprocessableEntity) + _ = reflector.Spec.AddOperation(http.MethodPost, + "/repos/{repo_ref}/squash", opSquashBranch) } diff --git a/app/router/api.go b/app/router/api.go index 475546da6..e36db163a 100644 --- a/app/router/api.go +++ b/app/router/api.go @@ -419,6 +419,7 @@ func setupRepos(r chi.Router, }) r.Post("/rebase", handlerrepo.HandleRebase(repoCtrl)) + r.Post("/squash", handlerrepo.HandleSquash(repoCtrl)) r.Get("/codeowners/validate", handlerrepo.HandleCodeOwnersValidate(repoCtrl)) diff --git a/git/merge.go b/git/merge.go index 2e295af2a..cdfa25208 100644 --- a/git/merge.go +++ b/git/merge.go @@ -31,7 +31,10 @@ import ( // MergeParams is input structure object for merging operation. type MergeParams struct { WriteParams + + BaseSHA sha.SHA BaseBranch string + // HeadRepoUID specifies the UID of the repo that contains the head branch (required for forking). // WARNING: This field is currently not supported yet! HeadRepoUID string @@ -70,8 +73,8 @@ func (p *MergeParams) Validate() error { return err } - if p.BaseBranch == "" { - return errors.InvalidArgument("base branch is mandatory") + if p.BaseBranch == "" && p.BaseSHA.IsEmpty() { + return errors.InvalidArgument("either base branch or commit SHA is mandatory") } if p.HeadBranch == "" { @@ -172,9 +175,12 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, // find the commit SHAs - baseCommitSHA, err := s.git.GetFullCommitID(ctx, repoPath, params.BaseBranch) - if err != nil { - return MergeOutput{}, fmt.Errorf("failed to get base branch commit SHA: %w", err) + baseCommitSHA := params.BaseSHA + if baseCommitSHA.IsEmpty() { + baseCommitSHA, err = s.git.GetFullCommitID(ctx, repoPath, params.BaseBranch) + if err != nil { + return MergeOutput{}, fmt.Errorf("failed to get base branch commit SHA: %w", err) + } } headCommitSHA, err := s.git.GetFullCommitID(ctx, repoPath, params.HeadBranch) diff --git a/types/rebase.go b/types/rebase.go index df10afbda..2ef2e7d9f 100644 --- a/types/rebase.go +++ b/types/rebase.go @@ -25,3 +25,12 @@ type RebaseResponse struct { DryRun bool `json:"dry_run,omitempty"` ConflictFiles []string `json:"conflict_files,omitempty"` } + +type SquashResponse struct { + NewHeadBranchSHA sha.SHA `json:"new_head_branch_sha"` + RuleViolations []RuleViolations `json:"rule_violations,omitempty"` + + DryRunRules bool `json:"dry_run_rules,omitempty"` + DryRun bool `json:"dry_run,omitempty"` + ConflictFiles []string `json:"conflict_files,omitempty"` +}