diff --git a/app/api/controller/pullreq/merge.go b/app/api/controller/pullreq/merge.go index f103fe44f..fbecfd7b0 100644 --- a/app/api/controller/pullreq/merge.go +++ b/app/api/controller/pullreq/merge.go @@ -29,10 +29,12 @@ import ( "github.com/harness/gitness/app/services/codeowners" "github.com/harness/gitness/app/services/instrument" "github.com/harness/gitness/app/services/protection" + "github.com/harness/gitness/app/services/pullreq" "github.com/harness/gitness/audit" "github.com/harness/gitness/contextutil" "github.com/harness/gitness/errors" "github.com/harness/gitness/git" + gitapi "github.com/harness/gitness/git/api" gitenum "github.com/harness/gitness/git/enum" "github.com/harness/gitness/git/sha" gitness_store "github.com/harness/gitness/store" @@ -304,6 +306,16 @@ func (c *Controller) Merge( }, nil, nil } + targetBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{ + ReadParams: git.ReadParams{RepoUID: targetRepo.GitUID}, + BranchName: pr.TargetBranch, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to get pull request target branch: %w", err) + } + + targetSHA := targetBranch.Branch.SHA + // we want to complete the merge independent of request cancel - start with new, time restricted context. // TODO: This is a small change to reduce likelihood of dirty state. // We still require a proper solution to handle an application crash or very slow execution times @@ -357,11 +369,28 @@ func (c *Controller) Merge( mergeOutput, err = c.git.Merge(ctx, &git.MergeParams{ WriteParams: writeParams, - BaseBranch: pr.TargetBranch, + BaseSHA: targetSHA, HeadSHA: sourceSHA, Refs: nil, // update no refs -> no commit will be created Method: gitenum.MergeMethod(in.Method), }) + if errors.IsInvalidArgument(err) || gitapi.IsUnrelatedHistoriesError(err) { + inClose := pullreq.NonUniqueMergeBaseInput{ + PullReqStore: c.pullreqStore, + ActivityStore: c.activityStore, + PullReqEvReporter: c.eventReporter, + SSEStreamer: c.sseStreamer, + } + + errClose := pullreq.CloseBecauseNonUniqueMergeBase(ctx, inClose, targetSHA, sourceSHA, pr) + if errClose != nil { + return nil, nil, + fmt.Errorf("failed to close pull request after non-unique merge base: %w", errClose) + } + + return nil, nil, err + } + if err != nil { return nil, nil, fmt.Errorf("failed merge check with method=%s: %w", in.Method, err) } @@ -530,7 +559,7 @@ func (c *Controller) Merge( // Update the target branch to the result of the merge. refUpdates = append(refUpdates, git.RefUpdate{ Name: refTargetBranch, - Old: sha.SHA{}, // don't care about the current commit SHA of the target branch. + Old: targetSHA, New: sha.SHA{}, // update to the result of the merge. }) @@ -551,7 +580,7 @@ func (c *Controller) Merge( now := time.Now() mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ WriteParams: targetWriteParams, - BaseBranch: pr.TargetBranch, + BaseSHA: targetSHA, HeadSHA: sourceSHA, Message: git.CommitMessage(in.Title, in.Message), Committer: committer, @@ -561,6 +590,22 @@ func (c *Controller) Merge( Refs: refUpdates, Method: gitenum.MergeMethod(in.Method), }) + if errors.IsInvalidArgument(err) || gitapi.IsUnrelatedHistoriesError(err) { + inClose := pullreq.NonUniqueMergeBaseInput{ + PullReqStore: c.pullreqStore, + ActivityStore: c.activityStore, + PullReqEvReporter: c.eventReporter, + SSEStreamer: c.sseStreamer, + } + + errClose := pullreq.CloseBecauseNonUniqueMergeBase(ctx, inClose, targetSHA, sourceSHA, pr) + if errClose != nil { + return nil, nil, + fmt.Errorf("failed to close pull request after non-unique merge base: %w", errClose) + } + + return nil, nil, err + } if err != nil { return nil, nil, fmt.Errorf("merge execution failed: %w", err) } diff --git a/app/services/pullreq/close.go b/app/services/pullreq/close.go new file mode 100644 index 000000000..6b65210c4 --- /dev/null +++ b/app/services/pullreq/close.go @@ -0,0 +1,118 @@ +// 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" + + "github.com/harness/gitness/app/bootstrap" + pullreqevents "github.com/harness/gitness/app/events/pullreq" + "github.com/harness/gitness/app/sse" + "github.com/harness/gitness/app/store" + "github.com/harness/gitness/errors" + "github.com/harness/gitness/git/sha" + "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" + + "github.com/gotidy/ptr" + "github.com/rs/zerolog/log" +) + +type NonUniqueMergeBaseInput struct { + PullReqStore store.PullReqStore + ActivityStore store.PullReqActivityStore + PullReqEvReporter *pullreqevents.Reporter + SSEStreamer sse.Streamer +} + +func CloseBecauseNonUniqueMergeBase( + ctx context.Context, + in NonUniqueMergeBaseInput, + targetSHA sha.SHA, + sourceSHA sha.SHA, + pr *types.PullReq, +) error { + systemPrincipal := bootstrap.NewSystemServiceSession().Principal + systemPrincipalID := systemPrincipal.ID + + var activitySeqMergeBase, activitySeqPRClosed int64 + pr, err := in.PullReqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error { + // to avoid racing conditions with merge + if pr.State != enum.PullReqStateOpen { + return errPRNotOpen + } + + pr.ActivitySeq += 2 + activitySeqMergeBase = pr.ActivitySeq - 1 + activitySeqPRClosed = pr.ActivitySeq + + pr.SourceSHA = sourceSHA.String() + pr.MergeTargetSHA = ptr.String(targetSHA.String()) + + pr.State = enum.PullReqStateClosed + pr.MergeSHA = nil + pr.MarkAsMergeUnchecked() + + return nil + }) + if errors.Is(err, errPRNotOpen) { + return nil + } + if err != nil { + return fmt.Errorf("failed to close pull request after non-unique merge base: %w", err) + } + + pr.ActivitySeq = activitySeqMergeBase + payloadNonUniqueMergeBase := &types.PullRequestActivityPayloadNonUniqueMergeBase{ + TargetSHA: targetSHA, + SourceSHA: sourceSHA, + } + _, err = in.ActivityStore.CreateWithPayload(ctx, pr, systemPrincipalID, payloadNonUniqueMergeBase, nil) + if err != nil { + // non-critical error + log.Ctx(ctx).Err(err).Msg("failed to write pull request activity for non-unique merge-base") + } + + pr.ActivitySeq = activitySeqPRClosed + payloadStateChange := &types.PullRequestActivityPayloadStateChange{ + Old: enum.PullReqStateOpen, + New: enum.PullReqStateClosed, + OldDraft: pr.IsDraft, + NewDraft: pr.IsDraft, + } + if _, err := in.ActivityStore.CreateWithPayload(ctx, pr, systemPrincipalID, payloadStateChange, nil); err != nil { + // non-critical error + log.Ctx(ctx).Err(err).Msg( + "failed to write pull request activity for pull request closure after non-unique merge-base", + ) + } + + in.PullReqEvReporter.Closed(ctx, &pullreqevents.ClosedPayload{ + Base: pullreqevents.Base{ + PullReqID: pr.ID, + SourceRepoID: pr.SourceRepoID, + TargetRepoID: pr.TargetRepoID, + PrincipalID: systemPrincipalID, + Number: pr.Number, + }, + SourceSHA: pr.SourceSHA, + SourceBranch: pr.SourceBranch, + }) + + in.SSEStreamer.Publish(ctx, pr.TargetRepoID, enum.SSETypePullReqUpdated, pr) + + return nil +} diff --git a/app/services/pullreq/handlers_branch.go b/app/services/pullreq/handlers_branch.go index 5d32c4378..7b48d56e3 100644 --- a/app/services/pullreq/handlers_branch.go +++ b/app/services/pullreq/handlers_branch.go @@ -16,15 +16,16 @@ package pullreq import ( "context" - "errors" "fmt" "strconv" "strings" gitevents "github.com/harness/gitness/app/events/git" pullreqevents "github.com/harness/gitness/app/events/pullreq" + "github.com/harness/gitness/errors" "github.com/harness/gitness/events" "github.com/harness/gitness/git" + gitapi "github.com/harness/gitness/git/api" gitenum "github.com/harness/gitness/git/enum" "github.com/harness/gitness/git/sha" gitness_store "github.com/harness/gitness/store" @@ -169,6 +170,20 @@ func (s *Service) updatePullReqOnBranchUpdate(ctx context.Context, Ref1: event.Payload.NewSHA, Ref2: targetSHA.String(), }) + if errors.IsInvalidArgument(err) || gitapi.IsUnrelatedHistoriesError(err) { + in := NonUniqueMergeBaseInput{ + PullReqStore: s.pullreqStore, + ActivityStore: s.activityStore, + PullReqEvReporter: s.pullreqEvReporter, + SSEStreamer: s.sseStreamer, + } + err = CloseBecauseNonUniqueMergeBase(ctx, in, targetSHA, newSHA, pr) + if err != nil { + return fmt.Errorf("failed to close pull request after non-unique merge base: %w", err) + } + + return nil + } if err != nil { return fmt.Errorf("failed to get merge base after branch update to=%s for PR=%d: %w", event.Payload.NewSHA, pr.Number, err) diff --git a/types/enum/pullreq.go b/types/enum/pullreq.go index 9877976f4..95e8a50d9 100644 --- a/types/enum/pullreq.go +++ b/types/enum/pullreq.go @@ -93,6 +93,7 @@ const ( PullReqActivityTypeTargetBranchChange PullReqActivityType = "target-branch-change" PullReqActivityTypeMerge PullReqActivityType = "merge" PullReqActivityTypeLabelModify PullReqActivityType = "label-modify" + PullReqActivityTypeNonUniqueMergeBase PullReqActivityType = "non-unique-merge-base" ) var pullReqActivityTypes = sortEnum([]PullReqActivityType{ diff --git a/types/pullreq_activity_payload.go b/types/pullreq_activity_payload.go index 4d10a1591..0fd7f237f 100644 --- a/types/pullreq_activity_payload.go +++ b/types/pullreq_activity_payload.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" + "github.com/harness/gitness/git/sha" "github.com/harness/gitness/types/enum" ) @@ -232,3 +233,12 @@ type PullRequestActivityLabels struct { func (a *PullRequestActivityLabels) ActivityType() enum.PullReqActivityType { return enum.PullReqActivityTypeLabelModify } + +type PullRequestActivityPayloadNonUniqueMergeBase struct { + TargetSHA sha.SHA `json:"target_sha"` + SourceSHA sha.SHA `json:"source_sha"` +} + +func (a *PullRequestActivityPayloadNonUniqueMergeBase) ActivityType() enum.PullReqActivityType { + return enum.PullReqActivityTypeNonUniqueMergeBase +}