third_party
/
gitea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use for a repo action one database transaction (#19576)

Browse Source 
... more context

(part of #9307)
tokarchuk/v1.17
6543 3 years ago committed by GitHub
parent 730420b6b3
commit 92f139d091
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
29 changed files with 270 additions and 260 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 6
      integrations/pull_merge_test.go
  2. 20
      models/branches.go
  3. 11
      models/db/context.go
  4. 37
      models/issue.go
  5. 9
      models/issue_comment.go
  6. 10
      models/issue_dependency.go
  7. 7
      models/issue_dependency_test.go
  8. 4
      models/issue_test.go
  9. 6
      models/issue_xref.go
  10. 4
      models/issue_xref_test.go
  11. 29
      models/pull.go
  12. 4
      models/pull_list.go
  13. 21
      models/pull_test.go
  14. 6
      models/repo.go
  15. 15
      models/repo/repo.go
  16. 5
      models/repo_permission.go
  17. 2
      modules/convert/convert.go
  18. 2
      modules/git/commit_test.go
  19. 26
      routers/api/v1/repo/pull.go
  20. 2
      routers/private/hook_pre_receive.go
  21. 15
      routers/web/repo/issue.go
  22. 31
      routers/web/repo/pull.go
  23. 2
      services/asymkey/sign.go
  24. 7
      services/forms/repo_form.go
  25. 14
      services/issue/status.go
  26. 17
      services/pull/check.go
  27. 64
      services/pull/merge.go
  28. 4
      services/pull/update.go
  29. 6
      templates/repo/issue/view_content/pull.tmpl

6
integrations/pull_merge_test.go
Unescape View File

@ -243,11 +243,11 @@ func TestCantMergeConflict(t *testing.T) {
gitRepo, err := git.OpenRepository(git.DefaultContext, repo_model.RepoPath(user1.Name, repo1.Name)) gitRepo, err := git.OpenRepository(git.DefaultContext, repo_model.RepoPath(user1.Name, repo1.Name))
assert.NoError(t, err) assert.NoError(t, err)
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT") err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT")
assert.Error(t, err, "Merge should return an error due to conflict") assert.Error(t, err, "Merge should return an error due to conflict")
assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error") assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error")
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT") err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT")
assert.Error(t, err, "Merge should return an error due to conflict") assert.Error(t, err, "Merge should return an error due to conflict")
assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error") assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
gitRepo.Close() gitRepo.Close()
@ -342,7 +342,7 @@ func TestCantMergeUnrelated(t *testing.T) {
BaseBranch: "base", BaseBranch: "base",
}).(*models.PullRequest) }).(*models.PullRequest)
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED") err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED")
assert.Error(t, err, "Merge should return an error due to unrelated") assert.Error(t, err, "Merge should return an error due to unrelated")
assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error") assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
gitRepo.Close() gitRepo.Close()

20
models/branches.go
Unescape View File

@ -104,7 +104,7 @@ func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
} }
// IsUserMergeWhitelisted checks if some user is whitelisted to merge to this branch // IsUserMergeWhitelisted checks if some user is whitelisted to merge to this branch
func IsUserMergeWhitelisted(protectBranch *ProtectedBranch, userID int64, permissionInRepo Permission) bool { func IsUserMergeWhitelisted(ctx context.Context, protectBranch *ProtectedBranch, userID int64, permissionInRepo Permission) bool {
if !protectBranch.EnableMergeWhitelist { if !protectBranch.EnableMergeWhitelist {
// Then we need to fall back on whether the user has write permission // Then we need to fall back on whether the user has write permission
return permissionInRepo.CanWrite(unit.TypeCode) return permissionInRepo.CanWrite(unit.TypeCode)
@ -118,7 +118,7 @@ func IsUserMergeWhitelisted(protectBranch *ProtectedBranch, userID int64, permis
return false return false
} }
in, err := organization.IsUserInTeams(db.DefaultContext, userID, protectBranch.MergeWhitelistTeamIDs) in, err := organization.IsUserInTeams(ctx, userID, protectBranch.MergeWhitelistTeamIDs)
if err != nil { if err != nil {
log.Error("IsUserInTeams: %v", err) log.Error("IsUserInTeams: %v", err)
return false return false
@ -159,16 +159,16 @@ func isUserOfficialReviewer(ctx context.Context, protectBranch *ProtectedBranch,
} }
// HasEnoughApprovals returns true if pr has enough granted approvals. // HasEnoughApprovals returns true if pr has enough granted approvals.
func (protectBranch *ProtectedBranch) HasEnoughApprovals(pr *PullRequest) bool { func (protectBranch *ProtectedBranch) HasEnoughApprovals(ctx context.Context, pr *PullRequest) bool {
if protectBranch.RequiredApprovals == 0 { if protectBranch.RequiredApprovals == 0 {
return true return true
} }
return protectBranch.GetGrantedApprovalsCount(pr) >= protectBranch.RequiredApprovals return protectBranch.GetGrantedApprovalsCount(ctx, pr) >= protectBranch.RequiredApprovals
} }
// GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist. // GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist.
func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 { func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(ctx context.Context, pr *PullRequest) int64 {
sess := db.GetEngine(db.DefaultContext).Where("issue_id = ?", pr.IssueID). sess := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
And("type = ?", ReviewTypeApprove). And("type = ?", ReviewTypeApprove).
And("official = ?", true). And("official = ?", true).
And("dismissed = ?", false) And("dismissed = ?", false)
@ -185,11 +185,11 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest)
} }
// MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews // MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews
func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullRequest) bool { func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(ctx context.Context, pr *PullRequest) bool {
if !protectBranch.BlockOnRejectedReviews { if !protectBranch.BlockOnRejectedReviews {
return false return false
} }
rejectExist, err := db.GetEngine(db.DefaultContext).Where("issue_id = ?", pr.IssueID). rejectExist, err := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
And("type = ?", ReviewTypeReject). And("type = ?", ReviewTypeReject).
And("official = ?", true). And("official = ?", true).
And("dismissed = ?", false). And("dismissed = ?", false).
@ -204,11 +204,11 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque
// MergeBlockedByOfficialReviewRequests block merge because of some review request to official reviewer // MergeBlockedByOfficialReviewRequests block merge because of some review request to official reviewer
// of from official review // of from official review
func (protectBranch *ProtectedBranch) MergeBlockedByOfficialReviewRequests(pr *PullRequest) bool { func (protectBranch *ProtectedBranch) MergeBlockedByOfficialReviewRequests(ctx context.Context, pr *PullRequest) bool {
if !protectBranch.BlockOnOfficialReviewRequests { if !protectBranch.BlockOnOfficialReviewRequests {
return false return false
} }
has, err := db.GetEngine(db.DefaultContext).Where("issue_id = ?", pr.IssueID). has, err := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
And("type = ?", ReviewTypeRequest). And("type = ?", ReviewTypeRequest).
And("official = ?", true). And("official = ?", true).
Exist(new(Review)) Exist(new(Review))

11
models/db/context.go
Unescape View File

@ -103,7 +103,14 @@ func WithContext(f func(ctx *Context) error) error {
} }
// WithTx represents executing database operations on a transaction // WithTx represents executing database operations on a transaction
func WithTx(f func(ctx context.Context) error) error { // you can optionally change the context to a parrent one
func WithTx(f func(ctx context.Context) error, stdCtx ...context.Context) error {
parentCtx := DefaultContext
if len(stdCtx) != 0 && stdCtx[0] != nil {
// TODO: make sure parent context has no open session
parentCtx = stdCtx[0]
}
sess := x.NewSession() sess := x.NewSession()
defer sess.Close() defer sess.Close()
if err := sess.Begin(); err != nil { if err := sess.Begin(); err != nil {
@ -111,7 +118,7 @@ func WithTx(f func(ctx context.Context) error) error {
} }
if err := f(&Context{ if err := f(&Context{
Context: DefaultContext, Context: parentCtx,
e: sess, e: sess,
}); err != nil { }); err != nil {
return err return err

37
models/issue.go
Unescape View File

@ -591,7 +591,7 @@ func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (e
} }
// ReadBy sets issue to be read by given user. // ReadBy sets issue to be read by given user.
func (issue *Issue) ReadBy(userID int64) error { func (issue *Issue) ReadBy(ctx context.Context, userID int64) error {
if err := UpdateIssueUserByRead(userID, issue.ID); err != nil { if err := UpdateIssueUserByRead(userID, issue.ID); err != nil {
return err return err
} }
@ -635,7 +635,7 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
// Check for open dependencies // Check for open dependencies
if issue.IsClosed && issue.Repo.IsDependenciesEnabledCtx(ctx) { if issue.IsClosed && issue.Repo.IsDependenciesEnabledCtx(ctx) {
// only check if dependencies are enabled and we're about to close an issue, otherwise reopening an issue would fail when there are unsatisfied dependencies // only check if dependencies are enabled and we're about to close an issue, otherwise reopening an issue would fail when there are unsatisfied dependencies
noDeps, err := issueNoDependenciesLeft(e, issue) noDeps, err := IssueNoDependenciesLeft(ctx, issue)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -693,13 +693,7 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
} }
// ChangeIssueStatus changes issue status to open or closed. // ChangeIssueStatus changes issue status to open or closed.
func ChangeIssueStatus(issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) { func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
ctx, committer, err := db.TxContext()
if err != nil {
return nil, err
}
defer committer.Close()
if err := issue.LoadRepo(ctx); err != nil { if err := issue.LoadRepo(ctx); err != nil {
return nil, err return nil, err
} }
@ -707,16 +701,7 @@ func ChangeIssueStatus(issue *Issue, doer *user_model.User, isClosed bool) (*Com
return nil, err return nil, err
} }
comment, err := changeIssueStatus(ctx, issue, doer, isClosed, false) return changeIssueStatus(ctx, issue, doer, isClosed, false)
if err != nil {
return nil, err
}
if err = committer.Commit(); err != nil {
return nil, fmt.Errorf("Commit: %v", err)
}
return comment, nil
} }
// ChangeIssueTitle changes the title of this issue, as the given user. // ChangeIssueTitle changes the title of this issue, as the given user.
@ -787,16 +772,11 @@ func ChangeIssueRef(issue *Issue, doer *user_model.User, oldRef string) (err err
} }
// AddDeletePRBranchComment adds delete branch comment for pull request issue // AddDeletePRBranchComment adds delete branch comment for pull request issue
func AddDeletePRBranchComment(doer *user_model.User, repo *repo_model.Repository, issueID int64, branchName string) error { func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issueID int64, branchName string) error {
issue, err := getIssueByID(db.GetEngine(db.DefaultContext), issueID) issue, err := getIssueByID(db.GetEngine(ctx), issueID)
if err != nil {
return err
}
ctx, committer, err := db.TxContext()
if err != nil { if err != nil {
return err return err
} }
defer committer.Close()
opts := &CreateCommentOptions{ opts := &CreateCommentOptions{
Type: CommentTypeDeleteBranch, Type: CommentTypeDeleteBranch,
Doer: doer, Doer: doer,
@ -804,11 +784,8 @@ func AddDeletePRBranchComment(doer *user_model.User, repo *repo_model.Repository
Issue: issue, Issue: issue,
OldRef: branchName, OldRef: branchName,
} }
if _, err = CreateCommentCtx(ctx, opts); err != nil { _, err = CreateCommentCtx(ctx, opts)
return err return err
}
return committer.Commit()
} }
// UpdateIssueAttachments update attachments by UUIDs for the issue // UpdateIssueAttachments update attachments by UUIDs for the issue

9
models/issue_comment.go
Unescape View File

@ -284,14 +284,15 @@ type PushActionContent struct {
// LoadIssue loads issue from database // LoadIssue loads issue from database
func (c *Comment) LoadIssue() (err error) { func (c *Comment) LoadIssue() (err error) {
return c.loadIssue(db.GetEngine(db.DefaultContext)) return c.LoadIssueCtx(db.DefaultContext)
} }
func (c *Comment) loadIssue(e db.Engine) (err error) { // LoadIssueCtx loads issue from database
func (c *Comment) LoadIssueCtx(ctx context.Context) (err error) {
if c.Issue != nil { if c.Issue != nil {
return nil return nil
} }
c.Issue, err = getIssueByID(e, c.IssueID) c.Issue, err = getIssueByID(db.GetEngine(ctx), c.IssueID)
return return
} }
@ -1126,7 +1127,7 @@ func UpdateComment(c *Comment, doer *user_model.User) error {
if _, err := sess.ID(c.ID).AllCols().Update(c); err != nil { if _, err := sess.ID(c.ID).AllCols().Update(c); err != nil {
return err return err
} }
if err := c.loadIssue(sess); err != nil { if err := c.LoadIssueCtx(ctx); err != nil {
return err return err
} }
if err := c.addCrossReferences(ctx, doer, true); err != nil { if err := c.addCrossReferences(ctx, doer, true); err != nil {

10
models/issue_dependency.go
Unescape View File

@ -5,6 +5,8 @@
package models package models
import ( import (
"context"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
@ -117,12 +119,8 @@ func issueDepExists(e db.Engine, issueID, depID int64) (bool, error) {
} }
// IssueNoDependenciesLeft checks if issue can be closed // IssueNoDependenciesLeft checks if issue can be closed
func IssueNoDependenciesLeft(issue *Issue) (bool, error) { func IssueNoDependenciesLeft(ctx context.Context, issue *Issue) (bool, error) {
return issueNoDependenciesLeft(db.GetEngine(db.DefaultContext), issue) exists, err := db.GetEngine(ctx).
}
func issueNoDependenciesLeft(e db.Engine, issue *Issue) (bool, error) {
exists, err := e.
Table("issue_dependency"). Table("issue_dependency").
Select("issue.*"). Select("issue.*").
Join("INNER", "issue", "issue.id = issue_dependency.dependency_id"). Join("INNER", "issue", "issue.id = issue_dependency.dependency_id").

7
models/issue_dependency_test.go
Unescape View File

@ -7,6 +7,7 @@ package models
import ( import (
"testing" "testing"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
@ -43,15 +44,15 @@ func TestCreateIssueDependency(t *testing.T) {
_ = unittest.AssertExistsAndLoadBean(t, &Comment{Type: CommentTypeAddDependency, PosterID: user1.ID, IssueID: issue1.ID}) _ = unittest.AssertExistsAndLoadBean(t, &Comment{Type: CommentTypeAddDependency, PosterID: user1.ID, IssueID: issue1.ID})
// Check if dependencies left is correct // Check if dependencies left is correct
left, err := IssueNoDependenciesLeft(issue1) left, err := IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err) assert.NoError(t, err)
assert.False(t, left) assert.False(t, left)
// Close #2 and check again // Close #2 and check again
_, err = ChangeIssueStatus(issue2, user1, true) _, err = ChangeIssueStatus(db.DefaultContext, issue2, user1, true)
assert.NoError(t, err) assert.NoError(t, err)
left, err = IssueNoDependenciesLeft(issue1) left, err = IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, left) assert.True(t, left)

4
models/issue_test.go
Unescape View File

@ -443,12 +443,12 @@ func TestIssue_DeleteIssue(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
err = CreateIssueDependency(user, issue1, issue2) err = CreateIssueDependency(user, issue1, issue2)
assert.NoError(t, err) assert.NoError(t, err)
left, err := IssueNoDependenciesLeft(issue1) left, err := IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err) assert.NoError(t, err)
assert.False(t, left) assert.False(t, left)
err = DeleteIssue(&Issue{ID: 2}) err = DeleteIssue(&Issue{ID: 2})
assert.NoError(t, err) assert.NoError(t, err)
left, err = IssueNoDependenciesLeft(issue1) left, err = IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, left) assert.True(t, left)
} }

6
models/issue_xref.go
Unescape View File

@ -249,7 +249,7 @@ func (comment *Comment) addCrossReferences(stdCtx context.Context, doer *user_mo
if comment.Type != CommentTypeCode && comment.Type != CommentTypeComment { if comment.Type != CommentTypeCode && comment.Type != CommentTypeComment {
return nil return nil
} }
if err := comment.loadIssue(db.GetEngine(stdCtx)); err != nil { if err := comment.LoadIssueCtx(stdCtx); err != nil {
return err return err
} }
ctx := &crossReferencesContext{ ctx := &crossReferencesContext{
@ -340,9 +340,9 @@ func (comment *Comment) RefIssueIdent() string {
// \/ \/ |__| \/ \/ // \/ \/ |__| \/ \/
// ResolveCrossReferences will return the list of references to close/reopen by this PR // ResolveCrossReferences will return the list of references to close/reopen by this PR
func (pr *PullRequest) ResolveCrossReferences() ([]*Comment, error) { func (pr *PullRequest) ResolveCrossReferences(ctx context.Context) ([]*Comment, error) {
unfiltered := make([]*Comment, 0, 5) unfiltered := make([]*Comment, 0, 5)
if err := db.GetEngine(db.DefaultContext). if err := db.GetEngine(ctx).
Where("ref_repo_id = ? AND ref_issue_id = ?", pr.Issue.RepoID, pr.Issue.ID). Where("ref_repo_id = ? AND ref_issue_id = ?", pr.Issue.RepoID, pr.Issue.ID).
In("ref_action", []references.XRefAction{references.XRefActionCloses, references.XRefActionReopens}). In("ref_action", []references.XRefAction{references.XRefActionCloses, references.XRefActionReopens}).
OrderBy("id"). OrderBy("id").

4
models/issue_xref_test.go
Unescape View File

@ -98,7 +98,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
i1 := testCreateIssue(t, 1, 2, "title1", "content1", false) i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
i2 := testCreateIssue(t, 1, 2, "title2", "content2", false) i2 := testCreateIssue(t, 1, 2, "title2", "content2", false)
i3 := testCreateIssue(t, 1, 2, "title3", "content3", false) i3 := testCreateIssue(t, 1, 2, "title3", "content3", false)
_, err := ChangeIssueStatus(i3, d, true) _, err := ChangeIssueStatus(db.DefaultContext, i3, d, true)
assert.NoError(t, err) assert.NoError(t, err)
pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index)) pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))
@ -118,7 +118,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
c4 := testCreateComment(t, 1, 2, pr.Issue.ID, fmt.Sprintf("closes #%d", i3.Index)) c4 := testCreateComment(t, 1, 2, pr.Issue.ID, fmt.Sprintf("closes #%d", i3.Index))
r4 := unittest.AssertExistsAndLoadBean(t, &Comment{IssueID: i3.ID, RefIssueID: pr.Issue.ID, RefCommentID: c4.ID}).(*Comment) r4 := unittest.AssertExistsAndLoadBean(t, &Comment{IssueID: i3.ID, RefIssueID: pr.Issue.ID, RefCommentID: c4.ID}).(*Comment)
refs, err := pr.ResolveCrossReferences() refs, err := pr.ResolveCrossReferences(db.DefaultContext)
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, refs, 3) assert.Len(t, refs, 3)
assert.Equal(t, rp.ID, refs[0].ID, "bad ref rp: %+v", refs[0]) assert.Equal(t, rp.ID, refs[0].ID, "bad ref rp: %+v", refs[0])

29
models/pull.go
Unescape View File

@ -227,23 +227,23 @@ func (pr *PullRequest) LoadProtectedBranchCtx(ctx context.Context) (err error) {
} }
// GetDefaultMergeMessage returns default message used when merging pull request // GetDefaultMergeMessage returns default message used when merging pull request
func (pr *PullRequest) GetDefaultMergeMessage() (string, error) { func (pr *PullRequest) GetDefaultMergeMessage(ctx context.Context) (string, error) {
if pr.HeadRepo == nil { if pr.HeadRepo == nil {
var err error var err error
pr.HeadRepo, err = repo_model.GetRepositoryByID(pr.HeadRepoID) pr.HeadRepo, err = repo_model.GetRepositoryByIDCtx(ctx, pr.HeadRepoID)
if err != nil { if err != nil {
return "", fmt.Errorf("GetRepositoryById[%d]: %v", pr.HeadRepoID, err) return "", fmt.Errorf("GetRepositoryById[%d]: %v", pr.HeadRepoID, err)
} }
} }
if err := pr.LoadIssue(); err != nil { if err := pr.LoadIssueCtx(ctx); err != nil {
return "", fmt.Errorf("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err) return "", fmt.Errorf("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err)
} }
if err := pr.LoadBaseRepo(); err != nil { if err := pr.LoadBaseRepoCtx(ctx); err != nil {
return "", fmt.Errorf("LoadBaseRepo: %v", err) return "", fmt.Errorf("LoadBaseRepo: %v", err)
} }
issueReference := "#" issueReference := "#"
if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { if pr.BaseRepo.UnitEnabledCtx(ctx, unit.TypeExternalTracker) {
issueReference = "!" issueReference = "!"
} }
@ -337,14 +337,14 @@ func (pr *PullRequest) getReviewedByLines(writer io.Writer) error {
} }
// GetDefaultSquashMessage returns default message used when squash and merging pull request // GetDefaultSquashMessage returns default message used when squash and merging pull request
func (pr *PullRequest) GetDefaultSquashMessage() (string, error) { func (pr *PullRequest) GetDefaultSquashMessage(ctx context.Context) (string, error) {
if err := pr.LoadIssue(); err != nil { if err := pr.LoadIssueCtx(ctx); err != nil {
return "", fmt.Errorf("LoadIssue: %v", err) return "", fmt.Errorf("LoadIssue: %v", err)
} }
if err := pr.LoadBaseRepo(); err != nil { if err := pr.LoadBaseRepoCtx(ctx); err != nil {
return "", fmt.Errorf("LoadBaseRepo: %v", err) return "", fmt.Errorf("LoadBaseRepo: %v", err)
} }
if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { if pr.BaseRepo.UnitEnabledCtx(ctx, unit.TypeExternalTracker) {
return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index), nil return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index), nil
} }
return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index), nil return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index), nil
@ -371,7 +371,7 @@ func (pr *PullRequest) IsEmpty() bool {
} }
// SetMerged sets a pull request to merged and closes the corresponding issue // SetMerged sets a pull request to merged and closes the corresponding issue
func (pr *PullRequest) SetMerged() (bool, error) { func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) {
if pr.HasMerged { if pr.HasMerged {
return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index) return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index)
} }
@ -380,12 +380,6 @@ func (pr *PullRequest) SetMerged() (bool, error) {
} }
pr.HasMerged = true pr.HasMerged = true
ctx, committer, err := db.TxContext()
if err != nil {
return false, err
}
defer committer.Close()
sess := db.GetEngine(ctx) sess := db.GetEngine(ctx)
if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil { if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
@ -432,9 +426,6 @@ func (pr *PullRequest) SetMerged() (bool, error) {
return false, fmt.Errorf("Failed to update pr[%d]: %v", pr.ID, err) return false, fmt.Errorf("Failed to update pr[%d]: %v", pr.ID, err)
} }
if err := committer.Commit(); err != nil {
return false, fmt.Errorf("Commit: %v", err)
}
return true, nil return true, nil
} }

4
models/pull_list.go
Unescape View File

@ -62,8 +62,8 @@ func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string) ([]*PullRequ
// HasUnmergedPullRequestsByHeadInfo checks if there are open and not merged pull request // HasUnmergedPullRequestsByHeadInfo checks if there are open and not merged pull request
// by given head information (repo and branch) // by given head information (repo and branch)
func HasUnmergedPullRequestsByHeadInfo(repoID int64, branch string) (bool, error) { func HasUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch string) (bool, error) {
return db.GetEngine(db.DefaultContext). return db.GetEngine(ctx).
Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?", Where("head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?",
repoID, branch, false, false, PullRequestFlowGithub). repoID, branch, false, false, PullRequestFlowGithub).
Join("INNER", "issue", "issue.id = pull_request.issue_id"). Join("INNER", "issue", "issue.id = pull_request.issue_id").

21
models/pull_test.go
Unescape View File

@ -110,11 +110,11 @@ func TestGetUnmergedPullRequest(t *testing.T) {
func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) { func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
exist, err := HasUnmergedPullRequestsByHeadInfo(1, "branch2") exist, err := HasUnmergedPullRequestsByHeadInfo(db.DefaultContext, 1, "branch2")
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, true, exist) assert.Equal(t, true, exist)
exist, err = HasUnmergedPullRequestsByHeadInfo(1, "not_exist_branch") exist, err = HasUnmergedPullRequestsByHeadInfo(db.DefaultContext, 1, "not_exist_branch")
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, false, exist) assert.Equal(t, false, exist)
} }
@ -261,13 +261,13 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest) pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest)
msg, err := pr.GetDefaultMergeMessage() msg, err := pr.GetDefaultMergeMessage(db.DefaultContext)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", msg) assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", msg)
pr.BaseRepoID = 1 pr.BaseRepoID = 1
pr.HeadRepoID = 2 pr.HeadRepoID = 2
msg, err = pr.GetDefaultMergeMessage() msg, err = pr.GetDefaultMergeMessage(db.DefaultContext)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", msg) assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", msg)
} }
@ -287,13 +287,22 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2, BaseRepo: baseRepo}).(*PullRequest) pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2, BaseRepo: baseRepo}).(*PullRequest)
msg, err := pr.GetDefaultMergeMessage() msg, err := pr.GetDefaultMergeMessage(db.DefaultContext)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", msg) assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", msg)
pr.BaseRepoID = 1 pr.BaseRepoID = 1
pr.HeadRepoID = 2 pr.HeadRepoID = 2
msg, err = pr.GetDefaultMergeMessage() msg, err = pr.GetDefaultMergeMessage(db.DefaultContext)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", msg) assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", msg)
} }
func TestPullRequest_GetDefaultSquashMessage(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest)
msg, err := pr.GetDefaultSquashMessage(db.DefaultContext)
assert.NoError(t, err)
assert.Equal(t, "issue3 (#3)", msg)
}

6
models/repo.go
Unescape View File

@ -455,8 +455,8 @@ func CreateRepository(ctx context.Context, doer, u *user_model.User, repo *repo_
} }
} }
if isAdmin, err := isUserRepoAdmin(ctx, repo, doer); err != nil { if isAdmin, err := IsUserRepoAdminCtx(ctx, repo, doer); err != nil {
return fmt.Errorf("isUserRepoAdmin: %v", err) return fmt.Errorf("IsUserRepoAdminCtx: %v", err)
} else if !isAdmin { } else if !isAdmin {
// Make creator repo admin if it wasn't assigned automatically // Make creator repo admin if it wasn't assigned automatically
if err = addCollaborator(ctx, repo, doer); err != nil { if err = addCollaborator(ctx, repo, doer); err != nil {
@ -1215,7 +1215,7 @@ func DeleteDeployKey(ctx context.Context, doer *user_model.User, id int64) error
if err != nil { if err != nil {
return fmt.Errorf("GetRepositoryByID: %v", err) return fmt.Errorf("GetRepositoryByID: %v", err)
} }
has, err := isUserRepoAdmin(ctx, repo, doer) has, err := IsUserRepoAdminCtx(ctx, repo, doer)
if err != nil { if err != nil {
return fmt.Errorf("GetUserRepoPermission: %v", err) return fmt.Errorf("GetUserRepoPermission: %v", err)
} else if !has { } else if !has {

15
models/repo/repo.go
Unescape View File

@ -302,8 +302,19 @@ func (repo *Repository) LoadUnits(ctx context.Context) (err error) {
} }
// UnitEnabled if this repository has the given unit enabled // UnitEnabled if this repository has the given unit enabled
func (repo *Repository) UnitEnabled(tp unit.Type) bool { func (repo *Repository) UnitEnabled(tp unit.Type) (result bool) {
if err := repo.LoadUnits(db.DefaultContext); err != nil { if err := db.WithContext(func(ctx *db.Context) error {
result = repo.UnitEnabledCtx(ctx, tp)
return nil
}); err != nil {
log.Error("repo.UnitEnabled: %v", err)
}
return
}
// UnitEnabled if this repository has the given unit enabled
func (repo *Repository) UnitEnabledCtx(ctx context.Context, tp unit.Type) bool {
if err := repo.LoadUnits(ctx); err != nil {
log.Warn("Error loading repository (ID: %d) units: %s", repo.ID, err.Error()) log.Warn("Error loading repository (ID: %d) units: %s", repo.ID, err.Error())
} }
for _, unit := range repo.Units { for _, unit := range repo.Units {

5
models/repo_permission.go
Unescape View File

@ -331,10 +331,11 @@ func IsUserRealRepoAdmin(repo *repo_model.Repository, user *user_model.User) (bo
// IsUserRepoAdmin return true if user has admin right of a repo // IsUserRepoAdmin return true if user has admin right of a repo
func IsUserRepoAdmin(repo *repo_model.Repository, user *user_model.User) (bool, error) { func IsUserRepoAdmin(repo *repo_model.Repository, user *user_model.User) (bool, error) {
return isUserRepoAdmin(db.DefaultContext, repo, user) return IsUserRepoAdminCtx(db.DefaultContext, repo, user)
} }
func isUserRepoAdmin(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) { // IsUserRepoAdminCtx return true if user has admin right of a repo
func IsUserRepoAdminCtx(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) {
if user == nil || repo == nil { if user == nil || repo == nil {
return false, nil return false, nil
} }

2
modules/convert/convert.go
Unescape View File

@ -87,7 +87,7 @@ func ToBranch(repo *repo_model.Repository, b *git.Branch, c *git.Commit, bp *mod
return nil, err return nil, err
} }
branch.UserCanPush = bp.CanUserPush(user.ID) branch.UserCanPush = bp.CanUserPush(user.ID)
branch.UserCanMerge = models.IsUserMergeWhitelisted(bp, user.ID, permission) branch.UserCanMerge = models.IsUserMergeWhitelisted(db.DefaultContext, bp, user.ID, permission)
} }
return branch, nil return branch, nil

2
modules/git/commit_test.go
Unescape View File

@ -67,6 +67,7 @@ empty commit`
gitRepo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare")) gitRepo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare"))
assert.NoError(t, err) assert.NoError(t, err)
assert.NotNil(t, gitRepo) assert.NotNil(t, gitRepo)
defer gitRepo.Close()
commitFromReader, err := CommitFromReader(gitRepo, sha, strings.NewReader(commitString)) commitFromReader, err := CommitFromReader(gitRepo, sha, strings.NewReader(commitString))
assert.NoError(t, err) assert.NoError(t, err)
@ -111,6 +112,7 @@ func TestHasPreviousCommit(t *testing.T) {
repo, err := openRepositoryWithDefaultContext(bareRepo1Path) repo, err := openRepositoryWithDefaultContext(bareRepo1Path)
assert.NoError(t, err) assert.NoError(t, err)
defer repo.Close()
commit, err := repo.GetCommit("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0") commit, err := repo.GetCommit("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0")
assert.NoError(t, err) assert.NoError(t, err)

26
routers/api/v1/repo/pull.go
Unescape View File

@ -726,7 +726,8 @@ func MergePullRequest(ctx *context.APIContext) {
// "$ref": "#/responses/error" // "$ref": "#/responses/error"
form := web.GetForm(ctx).(*forms.MergePullRequestForm) form := web.GetForm(ctx).(*forms.MergePullRequestForm)
pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
pr, err := models.GetPullRequestByIndexCtx(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil { if err != nil {
if models.IsErrPullRequestNotExist(err) { if models.IsErrPullRequestNotExist(err) {
ctx.NotFound("GetPullRequestByIndex", err) ctx.NotFound("GetPullRequestByIndex", err)
@ -741,7 +742,7 @@ func MergePullRequest(ctx *context.APIContext) {
return return
} }
if err := pr.LoadIssue(); err != nil { if err := pr.LoadIssueCtx(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadIssue", err) ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
return return
} }
@ -749,7 +750,7 @@ func MergePullRequest(ctx *context.APIContext) {
if ctx.IsSigned { if ctx.IsSigned {
// Update issue-user. // Update issue-user.
if err = pr.Issue.ReadBy(ctx.Doer.ID); err != nil { if err = pr.Issue.ReadBy(ctx, ctx.Doer.ID); err != nil {
ctx.Error(http.StatusInternalServerError, "ReadBy", err) ctx.Error(http.StatusInternalServerError, "ReadBy", err)
return return
} }
@ -758,6 +759,7 @@ func MergePullRequest(ctx *context.APIContext) {
manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
force := form.ForceMerge != nil && *form.ForceMerge force := form.ForceMerge != nil && *form.ForceMerge
// start with merging by checking
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil { if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, force); err != nil {
if errors.Is(err, pull_service.ErrIsClosed) { if errors.Is(err, pull_service.ErrIsClosed) {
ctx.NotFound() ctx.NotFound()
@ -798,15 +800,14 @@ func MergePullRequest(ctx *context.APIContext) {
} }
// set defaults to propagate needed fields // set defaults to propagate needed fields
if err := form.SetDefaults(pr); err != nil { if err := form.SetDefaults(ctx, pr); err != nil {
ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err)) ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err))
return return
} }
if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil { if err := pull_service.Merge(pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil {
if models.IsErrInvalidMergeStyle(err) { if models.IsErrInvalidMergeStyle(err) {
ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do))) ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do)))
return
} else if models.IsErrMergeConflicts(err) { } else if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts) conflictError := err.(models.ErrMergeConflicts)
ctx.JSON(http.StatusConflict, conflictError) ctx.JSON(http.StatusConflict, conflictError)
@ -818,28 +819,25 @@ func MergePullRequest(ctx *context.APIContext) {
ctx.JSON(http.StatusConflict, conflictError) ctx.JSON(http.StatusConflict, conflictError)
} else if git.IsErrPushOutOfDate(err) { } else if git.IsErrPushOutOfDate(err) {
ctx.Error(http.StatusConflict, "Merge", "merge push out of date") ctx.Error(http.StatusConflict, "Merge", "merge push out of date")
return
} else if models.IsErrSHADoesNotMatch(err) { } else if models.IsErrSHADoesNotMatch(err) {
ctx.Error(http.StatusConflict, "Merge", "head out of date") ctx.Error(http.StatusConflict, "Merge", "head out of date")
return
} else if git.IsErrPushRejected(err) { } else if git.IsErrPushRejected(err) {
errPushRej := err.(*git.ErrPushRejected) errPushRej := err.(*git.ErrPushRejected)
if len(errPushRej.Message) == 0 { if len(errPushRej.Message) == 0 {
ctx.Error(http.StatusConflict, "Merge", "PushRejected without remote error message") ctx.Error(http.StatusConflict, "Merge", "PushRejected without remote error message")
return } else {
}
ctx.Error(http.StatusConflict, "Merge", "PushRejected with remote message: "+errPushRej.Message) ctx.Error(http.StatusConflict, "Merge", "PushRejected with remote message: "+errPushRej.Message)
return
} }
} else {
ctx.Error(http.StatusInternalServerError, "Merge", err) ctx.Error(http.StatusInternalServerError, "Merge", err)
}
return return
} }
log.Trace("Pull request merged: %d", pr.ID) log.Trace("Pull request merged: %d", pr.ID)
if form.DeleteBranchAfterMerge { if form.DeleteBranchAfterMerge {
// Don't cleanup when there are other PR's that use this branch as head branch. // Don't cleanup when there are other PR's that use this branch as head branch.
exist, err := models.HasUnmergedPullRequestsByHeadInfo(pr.HeadRepoID, pr.HeadBranch) exist, err := models.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil { if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return return
@ -873,7 +871,7 @@ func MergePullRequest(ctx *context.APIContext) {
} }
return return
} }
if err := models.AddDeletePRBranchComment(ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil { if err := models.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
// Do not fail here as branch has already been deleted // Do not fail here as branch has already been deleted
log.Error("DeleteBranch: %v", err) log.Error("DeleteBranch: %v", err)
} }

2
routers/private/hook_pre_receive.go
Unescape View File

@ -311,7 +311,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
// Now check if the user is allowed to merge PRs for this repository // Now check if the user is allowed to merge PRs for this repository
// Note: we can use ctx.perm and ctx.user directly as they will have been loaded above // Note: we can use ctx.perm and ctx.user directly as they will have been loaded above
allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, ctx.userPerm, ctx.user) allowedMerge, err := pull_service.IsUserAllowedToMerge(ctx, pr, ctx.userPerm, ctx.user)
if err != nil { if err != nil {
log.Error("Error calculating if allowed to merge: %v", err) log.Error("Error calculating if allowed to merge: %v", err)
ctx.JSON(http.StatusInternalServerError, private.Response{ ctx.JSON(http.StatusInternalServerError, private.Response{

15
routers/web/repo/issue.go
Unescape View File

@ -1295,7 +1295,7 @@ func ViewIssue(ctx *context.Context) {
if ctx.IsSigned { if ctx.IsSigned {
// Update issue-user. // Update issue-user.
if err = issue.ReadBy(ctx.Doer.ID); err != nil { if err = issue.ReadBy(ctx, ctx.Doer.ID); err != nil {
ctx.ServerError("ReadBy", err) ctx.ServerError("ReadBy", err)
return return
} }
@ -1557,7 +1557,7 @@ func ViewIssue(ctx *context.Context) {
ctx.ServerError("GetUserRepoPermission", err) ctx.ServerError("GetUserRepoPermission", err)
return return
} }
ctx.Data["AllowMerge"], err = pull_service.IsUserAllowedToMerge(pull, perm, ctx.Doer) ctx.Data["AllowMerge"], err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer)
if err != nil { if err != nil {
ctx.ServerError("IsUserAllowedToMerge", err) ctx.ServerError("IsUserAllowedToMerge", err)
return return
@ -1606,12 +1606,11 @@ func ViewIssue(ctx *context.Context) {
if ctx.Doer != nil { if ctx.Doer != nil {
showMergeInstructions = pull.ProtectedBranch.CanUserPush(ctx.Doer.ID) showMergeInstructions = pull.ProtectedBranch.CanUserPush(ctx.Doer.ID)
} }
cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull) ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(ctx, pull)
ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull) ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(ctx, pull)
ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) ctx.Data["IsBlockedByOfficialReviewRequests"] = pull.ProtectedBranch.MergeBlockedByOfficialReviewRequests(ctx, pull)
ctx.Data["IsBlockedByOfficialReviewRequests"] = pull.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pull)
ctx.Data["IsBlockedByOutdatedBranch"] = pull.ProtectedBranch.MergeBlockedByOutdatedBranch(pull) ctx.Data["IsBlockedByOutdatedBranch"] = pull.ProtectedBranch.MergeBlockedByOutdatedBranch(pull)
ctx.Data["GrantedApprovals"] = cnt ctx.Data["GrantedApprovals"] = pull.ProtectedBranch.GetGrantedApprovalsCount(ctx, pull)
ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits
ctx.Data["ChangedProtectedFiles"] = pull.ChangedProtectedFiles ctx.Data["ChangedProtectedFiles"] = pull.ChangedProtectedFiles
ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0 ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0
@ -1641,7 +1640,7 @@ func ViewIssue(ctx *context.Context) {
(!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"]) (!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"])
if isPullBranchDeletable && pull.HasMerged { if isPullBranchDeletable && pull.HasMerged {
exist, err := models.HasUnmergedPullRequestsByHeadInfo(pull.HeadRepoID, pull.HeadBranch) exist, err := models.HasUnmergedPullRequestsByHeadInfo(ctx, pull.HeadRepoID, pull.HeadBranch)
if err != nil { if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return return

31
routers/web/repo/pull.go
Unescape View File

@ -290,7 +290,7 @@ func checkPullInfo(ctx *context.Context) *models.Issue {
if ctx.IsSigned { if ctx.IsSigned {
// Update issue-user. // Update issue-user.
if err = issue.ReadBy(ctx.Doer.ID); err != nil { if err = issue.ReadBy(ctx, ctx.Doer.ID); err != nil {
ctx.ServerError("ReadBy", err) ctx.ServerError("ReadBy", err)
return nil return nil
} }
@ -866,6 +866,7 @@ func MergePullRequest(ctx *context.Context) {
manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged
forceMerge := form.ForceMerge != nil && *form.ForceMerge forceMerge := form.ForceMerge != nil && *form.ForceMerge
// start with merging by checking
if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil {
if errors.Is(err, pull_service.ErrIsClosed) { if errors.Is(err, pull_service.ErrIsClosed) {
if issue.IsPull { if issue.IsPull {
@ -899,7 +900,6 @@ func MergePullRequest(ctx *context.Context) {
} else { } else {
ctx.ServerError("WebCheck", err) ctx.ServerError("WebCheck", err)
} }
return return
} }
@ -909,14 +909,12 @@ func MergePullRequest(ctx *context.Context) {
if models.IsErrInvalidMergeStyle(err) { if models.IsErrInvalidMergeStyle(err) {
ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if strings.Contains(err.Error(), "Wrong commit ID") { } else if strings.Contains(err.Error(), "Wrong commit ID") {
ctx.Flash.Error(ctx.Tr("repo.pulls.wrong_commit_id")) ctx.Flash.Error(ctx.Tr("repo.pulls.wrong_commit_id"))
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return } else {
}
ctx.ServerError("MergedManually", err) ctx.ServerError("MergedManually", err)
}
return return
} }
@ -925,16 +923,15 @@ func MergePullRequest(ctx *context.Context) {
} }
// set defaults to propagate needed fields // set defaults to propagate needed fields
if err := form.SetDefaults(pr); err != nil { if err := form.SetDefaults(ctx, pr); err != nil {
ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err)) ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err))
return return
} }
if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil { if err := pull_service.Merge(pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil {
if models.IsErrInvalidMergeStyle(err) { if models.IsErrInvalidMergeStyle(err) {
ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if models.IsErrMergeConflicts(err) { } else if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts) conflictError := err.(models.ErrMergeConflicts)
flashError, err := ctx.RenderToString(tplAlertDetails, map[string]interface{}{ flashError, err := ctx.RenderToString(tplAlertDetails, map[string]interface{}{
@ -948,7 +945,6 @@ func MergePullRequest(ctx *context.Context) {
} }
ctx.Flash.Error(flashError) ctx.Flash.Error(flashError)
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if models.IsErrRebaseConflicts(err) { } else if models.IsErrRebaseConflicts(err) {
conflictError := err.(models.ErrRebaseConflicts) conflictError := err.(models.ErrRebaseConflicts)
flashError, err := ctx.RenderToString(tplAlertDetails, map[string]interface{}{ flashError, err := ctx.RenderToString(tplAlertDetails, map[string]interface{}{
@ -962,22 +958,18 @@ func MergePullRequest(ctx *context.Context) {
} }
ctx.Flash.Error(flashError) ctx.Flash.Error(flashError)
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if models.IsErrMergeUnrelatedHistories(err) { } else if models.IsErrMergeUnrelatedHistories(err) {
log.Debug("MergeUnrelatedHistories error: %v", err) log.Debug("MergeUnrelatedHistories error: %v", err)
ctx.Flash.Error(ctx.Tr("repo.pulls.unrelated_histories")) ctx.Flash.Error(ctx.Tr("repo.pulls.unrelated_histories"))
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if git.IsErrPushOutOfDate(err) { } else if git.IsErrPushOutOfDate(err) {
log.Debug("MergePushOutOfDate error: %v", err) log.Debug("MergePushOutOfDate error: %v", err)
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date")) ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date"))
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if models.IsErrSHADoesNotMatch(err) { } else if models.IsErrSHADoesNotMatch(err) {
log.Debug("MergeHeadOutOfDate error: %v", err) log.Debug("MergeHeadOutOfDate error: %v", err)
ctx.Flash.Error(ctx.Tr("repo.pulls.head_out_of_date")) ctx.Flash.Error(ctx.Tr("repo.pulls.head_out_of_date"))
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return
} else if git.IsErrPushRejected(err) { } else if git.IsErrPushRejected(err) {
log.Debug("MergePushRejected error: %v", err) log.Debug("MergePushRejected error: %v", err)
pushrejErr := err.(*git.ErrPushRejected) pushrejErr := err.(*git.ErrPushRejected)
@ -997,11 +989,12 @@ func MergePullRequest(ctx *context.Context) {
ctx.Flash.Error(flashError) ctx.Flash.Error(flashError)
} }
ctx.Redirect(issue.Link()) ctx.Redirect(issue.Link())
return } else {
}
ctx.ServerError("Merge", err) ctx.ServerError("Merge", err)
}
return return
} }
log.Trace("Pull request merged: %d", pr.ID)
if err := stopTimerIfAvailable(ctx.Doer, issue); err != nil { if err := stopTimerIfAvailable(ctx.Doer, issue); err != nil {
ctx.ServerError("CreateOrStopIssueStopwatch", err) ctx.ServerError("CreateOrStopIssueStopwatch", err)
@ -1012,7 +1005,7 @@ func MergePullRequest(ctx *context.Context) {
if form.DeleteBranchAfterMerge { if form.DeleteBranchAfterMerge {
// Don't cleanup when other pr use this branch as head branch // Don't cleanup when other pr use this branch as head branch
exist, err := models.HasUnmergedPullRequestsByHeadInfo(pr.HeadRepoID, pr.HeadBranch) exist, err := models.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil { if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return return
@ -1193,7 +1186,7 @@ func CleanUpPullRequest(ctx *context.Context) {
} }
// Don't cleanup when there are other PR's that use this branch as head branch. // Don't cleanup when there are other PR's that use this branch as head branch.
exist, err := models.HasUnmergedPullRequestsByHeadInfo(pr.HeadRepoID, pr.HeadBranch) exist, err := models.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil { if err != nil {
ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
return return
@ -1304,7 +1297,7 @@ func deleteBranch(ctx *context.Context, pr *models.PullRequest, gitRepo *git.Rep
return return
} }
if err := models.AddDeletePRBranchComment(ctx.Doer, pr.BaseRepo, pr.IssueID, pr.HeadBranch); err != nil { if err := models.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.IssueID, pr.HeadBranch); err != nil {
// Do not fail here as branch has already been deleted // Do not fail here as branch has already been deleted
log.Error("DeleteBranch: %v", err) log.Error("DeleteBranch: %v", err)
} }

2
services/asymkey/sign.go
Unescape View File

@ -317,7 +317,7 @@ Loop:
if protectedBranch == nil { if protectedBranch == nil {
return false, "", nil, &ErrWontSign{approved} return false, "", nil, &ErrWontSign{approved}
} }
if protectedBranch.GetGrantedApprovalsCount(pr) < 1 { if protectedBranch.GetGrantedApprovalsCount(ctx, pr) < 1 {
return false, "", nil, &ErrWontSign{approved} return false, "", nil, &ErrWontSign{approved}
} }
case baseSigned: case baseSigned:

7
services/forms/repo_form.go
Unescape View File

@ -6,6 +6,7 @@
package forms package forms
import ( import (
stdContext "context"
"net/http" "net/http"
"net/url" "net/url"
"strings" "strings"
@ -601,7 +602,7 @@ func (f *MergePullRequestForm) Validate(req *http.Request, errs binding.Errors)
} }
// SetDefaults if not provided for mergestyle and commit message // SetDefaults if not provided for mergestyle and commit message
func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) (err error) { func (f *MergePullRequestForm) SetDefaults(ctx stdContext.Context, pr *models.PullRequest) (err error) {
if f.Do == "" { if f.Do == "" {
f.Do = "merge" f.Do = "merge"
} }
@ -610,9 +611,9 @@ func (f *MergePullRequestForm) SetDefaults(pr *models.PullRequest) (err error) {
if len(f.MergeTitleField) == 0 { if len(f.MergeTitleField) == 0 {
switch f.Do { switch f.Do {
case "merge", "rebase-merge": case "merge", "rebase-merge":
f.MergeTitleField, err = pr.GetDefaultMergeMessage() f.MergeTitleField, err = pr.GetDefaultMergeMessage(ctx)
case "squash": case "squash":
f.MergeTitleField, err = pr.GetDefaultSquashMessage() f.MergeTitleField, err = pr.GetDefaultSquashMessage(ctx)
} }
} }

14
services/issue/status.go
Unescape View File

@ -5,6 +5,8 @@
package issue package issue
import ( import (
"context"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
@ -14,10 +16,16 @@ import (
// ChangeStatus changes issue status to open or closed. // ChangeStatus changes issue status to open or closed.
func ChangeStatus(issue *models.Issue, doer *user_model.User, closed bool) error { func ChangeStatus(issue *models.Issue, doer *user_model.User, closed bool) error {
comment, err := models.ChangeIssueStatus(issue, doer, closed) return changeStatusCtx(db.DefaultContext, issue, doer, closed)
}
// changeStatusCtx changes issue status to open or closed.
// TODO: if context is not db.DefaultContext we get a deadlock!!!
func changeStatusCtx(ctx context.Context, issue *models.Issue, doer *user_model.User, closed bool) error {
comment, err := models.ChangeIssueStatus(ctx, issue, doer, closed)
if err != nil { if err != nil {
if models.IsErrDependenciesLeft(err) && closed { if models.IsErrDependenciesLeft(err) && closed {
if err := models.FinishIssueStopwatchIfPossible(db.DefaultContext, doer, issue); err != nil { if err := models.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err) log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err)
} }
} }
@ -25,7 +33,7 @@ func ChangeStatus(issue *models.Issue, doer *user_model.User, closed bool) error
} }
if closed { if closed {
if err := models.FinishIssueStopwatchIfPossible(db.DefaultContext, doer, issue); err != nil { if err := models.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
return err return err
} }
} }

17
services/pull/check.go
Unescape View File

@ -14,6 +14,7 @@ import (
"strings" "strings"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
@ -59,18 +60,19 @@ func AddToTaskQueue(pr *models.PullRequest) {
} }
// CheckPullMergable check if the pull mergable based on all conditions (branch protection, merge options, ...) // CheckPullMergable check if the pull mergable based on all conditions (branch protection, merge options, ...)
func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.Permission, pr *models.PullRequest, manuallMerge, force bool) error { func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *models.Permission, pr *models.PullRequest, manuallMerge, force bool) error {
return db.WithTx(func(ctx context.Context) error {
if pr.HasMerged { if pr.HasMerged {
return ErrHasMerged return ErrHasMerged
} }
if err := pr.LoadIssue(); err != nil { if err := pr.LoadIssueCtx(ctx); err != nil {
return err return err
} else if pr.Issue.IsClosed { } else if pr.Issue.IsClosed {
return ErrIsClosed return ErrIsClosed
} }
if allowedMerge, err := IsUserAllowedToMerge(pr, *perm, doer); err != nil { if allowedMerge, err := IsUserAllowedToMerge(ctx, pr, *perm, doer); err != nil {
return err return err
} else if !allowedMerge { } else if !allowedMerge {
return ErrUserNotAllowedToMerge return ErrUserNotAllowedToMerge
@ -96,7 +98,7 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.
if err := CheckPullBranchProtections(ctx, pr, false); err != nil { if err := CheckPullBranchProtections(ctx, pr, false); err != nil {
if models.IsErrDisallowedToMerge(err) { if models.IsErrDisallowedToMerge(err) {
if force { if force {
if isRepoAdmin, err2 := models.IsUserRepoAdmin(pr.BaseRepo, doer); err2 != nil { if isRepoAdmin, err2 := models.IsUserRepoAdminCtx(ctx, pr.BaseRepo, doer); err2 != nil {
return err2 return err2
} else if !isRepoAdmin { } else if !isRepoAdmin {
return err return err
@ -111,18 +113,19 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.
return err return err
} }
if noDeps, err := models.IssueNoDependenciesLeft(pr.Issue); err != nil { if noDeps, err := models.IssueNoDependenciesLeft(ctx, pr.Issue); err != nil {
return err return err
} else if !noDeps { } else if !noDeps {
return ErrDependenciesLeft return ErrDependenciesLeft
} }
return nil return nil
}, stdCtx)
} }
// isSignedIfRequired check if merge will be signed if required // isSignedIfRequired check if merge will be signed if required
func isSignedIfRequired(ctx context.Context, pr *models.PullRequest, doer *user_model.User) (bool, error) { func isSignedIfRequired(ctx context.Context, pr *models.PullRequest, doer *user_model.User) (bool, error) {
if err := pr.LoadProtectedBranch(); err != nil { if err := pr.LoadProtectedBranchCtx(ctx); err != nil {
return false, err return false, err
} }
@ -266,7 +269,7 @@ func manuallyMerged(ctx context.Context, pr *models.PullRequest) bool {
pr.Merger = merger pr.Merger = merger
pr.MergerID = merger.ID pr.MergerID = merger.ID
if merged, err := pr.SetMerged(); err != nil { if merged, err := pr.SetMerged(ctx); err != nil {
log.Error("PullRequest[%d].setMerged : %v", pr.ID, err) log.Error("PullRequest[%d].setMerged : %v", pr.ID, err)
return false return false
} else if !merged { } else if !merged {

64
services/pull/merge.go
Unescape View File

@ -17,6 +17,7 @@ import (
"time" "time"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
@ -34,11 +35,11 @@ import (
// Merge merges pull request to base repository. // Merge merges pull request to base repository.
// Caller should check PR is ready to be merged (review and status checks) // Caller should check PR is ready to be merged (review and status checks)
// FIXME: add repoWorkingPull make sure two merges does not happen at same time. // FIXME: add repoWorkingPull make sure two merges does not happen at same time.
func Merge(ctx context.Context, pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (err error) { func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) error {
if err = pr.LoadHeadRepoCtx(ctx); err != nil { if err := pr.LoadHeadRepo(); err != nil {
log.Error("LoadHeadRepo: %v", err) log.Error("LoadHeadRepo: %v", err)
return fmt.Errorf("LoadHeadRepo: %v", err) return fmt.Errorf("LoadHeadRepo: %v", err)
} else if err = pr.LoadBaseRepoCtx(ctx); err != nil { } else if err := pr.LoadBaseRepo(); err != nil {
log.Error("LoadBaseRepo: %v", err) log.Error("LoadBaseRepo: %v", err)
return fmt.Errorf("LoadBaseRepo: %v", err) return fmt.Errorf("LoadBaseRepo: %v", err)
} }
@ -59,7 +60,9 @@ func Merge(ctx context.Context, pr *models.PullRequest, doer *user_model.User, b
go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "") go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
}() }()
pr.MergedCommitID, err = rawMerge(ctx, pr, doer, mergeStyle, expectedHeadCommitID, message) // TODO: make it able to do this in a database session
mergeCtx := context.Background()
pr.MergedCommitID, err = rawMerge(mergeCtx, pr, doer, mergeStyle, expectedHeadCommitID, message)
if err != nil { if err != nil {
return err return err
} }
@ -68,18 +71,18 @@ func Merge(ctx context.Context, pr *models.PullRequest, doer *user_model.User, b
pr.Merger = doer pr.Merger = doer
pr.MergerID = doer.ID pr.MergerID = doer.ID
if _, err := pr.SetMerged(); err != nil { if _, err := pr.SetMerged(db.DefaultContext); err != nil {
log.Error("setMerged [%d]: %v", pr.ID, err) log.Error("setMerged [%d]: %v", pr.ID, err)
} }
if err := pr.LoadIssue(); err != nil { if err := pr.LoadIssueCtx(db.DefaultContext); err != nil {
log.Error("loadIssue [%d]: %v", pr.ID, err) log.Error("loadIssue [%d]: %v", pr.ID, err)
} }
if err := pr.Issue.LoadRepo(ctx); err != nil { if err := pr.Issue.LoadRepo(db.DefaultContext); err != nil {
log.Error("loadRepo for issue [%d]: %v", pr.ID, err) log.Error("loadRepo for issue [%d]: %v", pr.ID, err)
} }
if err := pr.Issue.Repo.GetOwner(ctx); err != nil { if err := pr.Issue.Repo.GetOwner(db.DefaultContext); err != nil {
log.Error("GetOwner for issue repo [%d]: %v", pr.ID, err) log.Error("GetOwner for issue repo [%d]: %v", pr.ID, err)
} }
@ -89,17 +92,17 @@ func Merge(ctx context.Context, pr *models.PullRequest, doer *user_model.User, b
cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true))
// Resolve cross references // Resolve cross references
refs, err := pr.ResolveCrossReferences() refs, err := pr.ResolveCrossReferences(db.DefaultContext)
if err != nil { if err != nil {
log.Error("ResolveCrossReferences: %v", err) log.Error("ResolveCrossReferences: %v", err)
return nil return nil
} }
for _, ref := range refs { for _, ref := range refs {
if err = ref.LoadIssue(); err != nil { if err = ref.LoadIssueCtx(db.DefaultContext); err != nil {
return err return err
} }
if err = ref.Issue.LoadRepo(ctx); err != nil { if err = ref.Issue.LoadRepo(db.DefaultContext); err != nil {
return err return err
} }
close := ref.RefAction == references.XRefActionCloses close := ref.RefAction == references.XRefActionCloses
@ -112,7 +115,6 @@ func Merge(ctx context.Context, pr *models.PullRequest, doer *user_model.User, b
} }
} }
} }
return nil return nil
} }
@ -504,6 +506,8 @@ func rawMerge(ctx context.Context, pr *models.PullRequest, doer *user_model.User
} }
// Push back to upstream. // Push back to upstream.
// TODO: this cause an api call to "/api/internal/hook/post-receive/...",
// that prevents us from doint the whole merge in one db transaction
if err := pushCmd.Run(&git.RunOpts{ if err := pushCmd.Run(&git.RunOpts{
Env: env, Env: env,
Dir: tmpBasePath, Dir: tmpBasePath,
@ -645,17 +649,17 @@ func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (
} }
// IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections // IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections
func IsUserAllowedToMerge(pr *models.PullRequest, p models.Permission, user *user_model.User) (bool, error) { func IsUserAllowedToMerge(ctx context.Context, pr *models.PullRequest, p models.Permission, user *user_model.User) (bool, error) {
if user == nil { if user == nil {
return false, nil return false, nil
} }
err := pr.LoadProtectedBranch() err := pr.LoadProtectedBranchCtx(ctx)
if err != nil { if err != nil {
return false, err return false, err
} }
if (p.CanWrite(unit.TypeCode) && pr.ProtectedBranch == nil) || (pr.ProtectedBranch != nil && models.IsUserMergeWhitelisted(pr.ProtectedBranch, user.ID, p)) { if (p.CanWrite(unit.TypeCode) && pr.ProtectedBranch == nil) || (pr.ProtectedBranch != nil && models.IsUserMergeWhitelisted(ctx, pr.ProtectedBranch, user.ID, p)) {
return true, nil return true, nil
} }
@ -668,7 +672,7 @@ func CheckPullBranchProtections(ctx context.Context, pr *models.PullRequest, ski
return fmt.Errorf("LoadBaseRepo: %v", err) return fmt.Errorf("LoadBaseRepo: %v", err)
} }
if err = pr.LoadProtectedBranch(); err != nil { if err = pr.LoadProtectedBranchCtx(ctx); err != nil {
return fmt.Errorf("LoadProtectedBranch: %v", err) return fmt.Errorf("LoadProtectedBranch: %v", err)
} }
if pr.ProtectedBranch == nil { if pr.ProtectedBranch == nil {
@ -685,17 +689,17 @@ func CheckPullBranchProtections(ctx context.Context, pr *models.PullRequest, ski
} }
} }
if !pr.ProtectedBranch.HasEnoughApprovals(pr) { if !pr.ProtectedBranch.HasEnoughApprovals(ctx, pr) {
return models.ErrDisallowedToMerge{ return models.ErrDisallowedToMerge{
Reason: "Does not have enough approvals", Reason: "Does not have enough approvals",
} }
} }
if pr.ProtectedBranch.MergeBlockedByRejectedReview(pr) { if pr.ProtectedBranch.MergeBlockedByRejectedReview(ctx, pr) {
return models.ErrDisallowedToMerge{ return models.ErrDisallowedToMerge{
Reason: "There are requested changes", Reason: "There are requested changes",
} }
} }
if pr.ProtectedBranch.MergeBlockedByOfficialReviewRequests(pr) { if pr.ProtectedBranch.MergeBlockedByOfficialReviewRequests(ctx, pr) {
return models.ErrDisallowedToMerge{ return models.ErrDisallowedToMerge{
Reason: "There are official review requests", Reason: "There are official review requests",
} }
@ -721,10 +725,11 @@ func CheckPullBranchProtections(ctx context.Context, pr *models.PullRequest, ski
} }
// MergedManually mark pr as merged manually // MergedManually mark pr as merged manually
func MergedManually(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, commitID string) (err error) { func MergedManually(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, commitID string) error {
prUnit, err := pr.BaseRepo.GetUnit(unit.TypePullRequests) if err := db.WithTx(func(ctx context.Context) error {
prUnit, err := pr.BaseRepo.GetUnitCtx(ctx, unit.TypePullRequests)
if err != nil { if err != nil {
return return err
} }
prConfig := prUnit.PullRequestsConfig() prConfig := prUnit.PullRequestsConfig()
@ -742,12 +747,13 @@ func MergedManually(pr *models.PullRequest, doer *user_model.User, baseGitRepo *
if git.IsErrNotExist(err) { if git.IsErrNotExist(err) {
return fmt.Errorf("Wrong commit ID") return fmt.Errorf("Wrong commit ID")
} }
return return err
} }
commitID = commit.ID.String()
ok, err := baseGitRepo.IsCommitInBranch(commitID, pr.BaseBranch) ok, err := baseGitRepo.IsCommitInBranch(commitID, pr.BaseBranch)
if err != nil { if err != nil {
return return err
} }
if !ok { if !ok {
return fmt.Errorf("Wrong commit ID") return fmt.Errorf("Wrong commit ID")
@ -760,13 +766,17 @@ func MergedManually(pr *models.PullRequest, doer *user_model.User, baseGitRepo *
pr.MergerID = doer.ID pr.MergerID = doer.ID
merged := false merged := false
if merged, err = pr.SetMerged(); err != nil { if merged, err = pr.SetMerged(ctx); err != nil {
return return err
} else if !merged { } else if !merged {
return fmt.Errorf("SetMerged failed") return fmt.Errorf("SetMerged failed")
} }
return nil
}); err != nil {
return err
}
notification.NotifyMergePullRequest(pr, doer) notification.NotifyMergePullRequest(pr, doer)
log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commitID)
return nil return nil
} }

4
services/pull/update.go
Unescape View File

@ -116,13 +116,13 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *models.PullRequest, user *
return false, false, err return false, false, err
} }
mergeAllowed, err = IsUserAllowedToMerge(pr, headRepoPerm, user) mergeAllowed, err = IsUserAllowedToMerge(ctx, pr, headRepoPerm, user)
if err != nil { if err != nil {
return false, false, err return false, false, err
} }
if pull.AllowMaintainerEdit { if pull.AllowMaintainerEdit {
mergeAllowedMaintainer, err := IsUserAllowedToMerge(pr, baseRepoPerm, user) mergeAllowedMaintainer, err := IsUserAllowedToMerge(ctx, pr, baseRepoPerm, user)
if err != nil { if err != nil {
return false, false, err return false, false, err
} }

6
templates/repo/issue/view_content/pull.tmpl
Unescape View File

@ -329,7 +329,7 @@
{{.CsrfTokenHtml}} {{.CsrfTokenHtml}}
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}"> <input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
<div class="field"> <div class="field">
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}"> <input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage $.Context}}">
</div> </div>
<div class="field"> <div class="field">
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">Reviewed-on: {{$.Issue.HTMLURL}}&#13;&#10;{{$approvers}}</textarea> <textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">Reviewed-on: {{$.Issue.HTMLURL}}&#13;&#10;{{$approvers}}</textarea>
@ -375,7 +375,7 @@
{{.CsrfTokenHtml}} {{.CsrfTokenHtml}}
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}"> <input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
<div class="field"> <div class="field">
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}"> <input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage $.Context}}">
</div> </div>
<div class="field"> <div class="field">
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">Reviewed-on: {{$.Issue.HTMLURL}}&#13;&#10;{{$approvers}}</textarea> <textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">Reviewed-on: {{$.Issue.HTMLURL}}&#13;&#10;{{$approvers}}</textarea>
@ -401,7 +401,7 @@
{{.CsrfTokenHtml}} {{.CsrfTokenHtml}}
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}"> <input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
<div class="field"> <div class="field">
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultSquashMessage}}"> <input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultSquashMessage $.Context}}">
</div> </div>
<div class="field"> <div class="field">
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{.GetCommitMessages}}Reviewed-on: {{$.Issue.HTMLURL}}&#13;&#10;{{$approvers}}</textarea> <textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{.GetCommitMessages}}Reviewed-on: {{$.Issue.HTMLURL}}&#13;&#10;{{$approvers}}</textarea>

Write Preview
Loading…
Cancel
Save