diff --git a/services/pull/merge.go b/services/pull/merge.go index 68a3f433e5..0acbfe22c3 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -211,7 +211,15 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U } defer releaser() defer func() { - go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, false, "", "") + go AddTestPullRequestTask(TestPullRequestOptions{ + RepoID: pr.BaseRepo.ID, + Doer: doer, + Branch: pr.BaseBranch, + IsSync: false, + IsForcePush: false, + OldCommitID: "", + NewCommitID: "", + }) }() _, err = doMergeAndPush(ctx, pr, doer, mergeStyle, expectedHeadCommitID, message, repo_module.PushTriggerPRMergeToBase) diff --git a/services/pull/pull.go b/services/pull/pull.go index 7ca5ee545d..7b8667238d 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -372,19 +372,29 @@ func checkForInvalidation(ctx context.Context, requests issues_model.PullRequest return nil } +type TestPullRequestOptions struct { + RepoID int64 + Doer *user_model.User + Branch string + IsSync bool + IsForcePush bool + OldCommitID string + NewCommitID string +} + // AddTestPullRequestTask adds new test tasks by given head/base repository and head/base branch, // and generate new patch for testing as needed. -func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, isSync, isForPush bool, oldCommitID, newCommitID string) { - log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", repoID, branch) +func AddTestPullRequestTask(opts TestPullRequestOptions) { + log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", opts.RepoID, opts.Branch) graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) { // There is no sensible way to shut this down ":-(" // If you don't let it run all the way then you will lose data // TODO: graceful: AddTestPullRequestTask needs to become a queue! // GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR. - prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repoID, branch) + prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, opts.RepoID, opts.Branch) if err != nil { - log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err) + log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", opts.RepoID, opts.Branch, err) return } @@ -400,25 +410,25 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, } AddToTaskQueue(ctx, pr) - comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID) + comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID) if err == nil && comment != nil { - notify_service.PullRequestPushCommits(ctx, doer, pr, comment) + notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment) } } - if isSync { + if opts.IsSync { requests := issues_model.PullRequestList(prs) if err = requests.LoadAttributes(ctx); err != nil { log.Error("PullRequestList.LoadAttributes: %v", err) } - if invalidationErr := checkForInvalidation(ctx, requests, repoID, doer, branch); invalidationErr != nil { + if invalidationErr := checkForInvalidation(ctx, requests, opts.RepoID, opts.Doer, opts.Branch); invalidationErr != nil { log.Error("checkForInvalidation: %v", invalidationErr) } if err == nil { for _, pr := range prs { objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName) - if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() { - changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID) + if opts.NewCommitID != "" && opts.NewCommitID != objectFormat.EmptyObjectID().String() { + changed, err := checkIfPRContentChanged(ctx, pr, opts.OldCommitID, opts.NewCommitID) if err != nil { log.Error("checkIfPRContentChanged: %v", err) } @@ -434,12 +444,12 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, log.Error("GetFirstMatchProtectedBranchRule: %v", err) } if pb != nil && pb.DismissStaleApprovals { - if err := DismissApprovalReviews(ctx, doer, pr); err != nil { + if err := DismissApprovalReviews(ctx, opts.Doer, pr); err != nil { log.Error("DismissApprovalReviews: %v", err) } } } - if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil { + if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, opts.NewCommitID); err != nil { log.Error("MarkReviewsAsNotStale: %v", err) } divergence, err := GetDiverging(ctx, pr) @@ -455,28 +465,28 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, if !pr.IsWorkInProgress(ctx) { var reviewNotifiers []*issue_service.ReviewRequestNotifier - if isForPush { + if opts.IsForcePush { reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, pr) } else { - reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, oldCommitID, newCommitID) + reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, opts.OldCommitID, opts.NewCommitID) } if err != nil { log.Error("PullRequestCodeOwnersReview: %v", err) } if len(reviewNotifiers) > 0 { - issue_service.ReviewRequestNotify(ctx, pr.Issue, doer, reviewNotifiers) + issue_service.ReviewRequestNotify(ctx, pr.Issue, opts.Doer, reviewNotifiers) } } - notify_service.PullRequestSynchronized(ctx, doer, pr) + notify_service.PullRequestSynchronized(ctx, opts.Doer, pr) } } } - log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch) - prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch) + log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", opts.RepoID, opts.Branch) + prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, opts.RepoID, opts.Branch) if err != nil { - log.Error("Find pull requests [base_repo_id: %d, base_branch: %s]: %v", repoID, branch, err) + log.Error("Find pull requests [base_repo_id: %d, base_branch: %s]: %v", opts.RepoID, opts.Branch, err) return } for _, pr := range prs { diff --git a/services/pull/update.go b/services/pull/update.go index 5708d2dfbf..3e00dd4e65 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -42,7 +42,15 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. if rebase { defer func() { - go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, false, "", "") + go AddTestPullRequestTask(TestPullRequestOptions{ + RepoID: pr.BaseRepo.ID, + Doer: doer, + Branch: pr.BaseBranch, + IsSync: false, + IsForcePush: false, + OldCommitID: "", + NewCommitID: "", + }) }() return updateHeadByRebaseOnToBase(ctx, pr, doer) @@ -83,7 +91,15 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. _, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase) defer func() { - go AddTestPullRequestTask(doer, reversePR.HeadRepo.ID, reversePR.HeadBranch, false, false, "", "") + go AddTestPullRequestTask(TestPullRequestOptions{ + RepoID: reversePR.HeadRepo.ID, + Doer: doer, + Branch: reversePR.HeadBranch, + IsSync: false, + IsForcePush: false, + OldCommitID: "", + NewCommitID: "", + }) }() return err diff --git a/services/repository/push.go b/services/repository/push.go index 7be2f06bc9..00d4fb11af 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -208,7 +208,15 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } // only update branch can trigger pull request task because the pull request hasn't been created yet when creaing a branch - go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true, isForcePush, opts.OldCommitID, opts.NewCommitID) + go pull_service.AddTestPullRequestTask(pull_service.TestPullRequestOptions{ + RepoID: repo.ID, + Doer: pusher, + Branch: branch, + IsSync: true, + IsForcePush: isForcePush, + OldCommitID: opts.OldCommitID, + NewCommitID: opts.NewCommitID, + }) if isForcePush { log.Trace("Push %s is a force push", opts.NewCommitID)