Skip to content

Use for a repo action one database transaction #19576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
May 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions integrations/pull_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,11 @@ func TestCantMergeConflict(t *testing.T) {
gitRepo, err := git.OpenRepository(git.DefaultContext, repo_model.RepoPath(user1.Name, repo1.Name))
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.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.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
gitRepo.Close()
Expand Down Expand Up @@ -342,7 +342,7 @@ func TestCantMergeUnrelated(t *testing.T) {
BaseBranch: "base",
}).(*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.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
gitRepo.Close()
Expand Down
20 changes: 10 additions & 10 deletions models/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
}

// 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 {
// Then we need to fall back on whether the user has write permission
return permissionInRepo.CanWrite(unit.TypeCode)
Expand All @@ -118,7 +118,7 @@ func IsUserMergeWhitelisted(protectBranch *ProtectedBranch, userID int64, permis
return false
}

in, err := organization.IsUserInTeams(db.DefaultContext, userID, protectBranch.MergeWhitelistTeamIDs)
in, err := organization.IsUserInTeams(ctx, userID, protectBranch.MergeWhitelistTeamIDs)
if err != nil {
log.Error("IsUserInTeams: %v", err)
return false
Expand Down Expand Up @@ -159,16 +159,16 @@ func isUserOfficialReviewer(ctx context.Context, protectBranch *ProtectedBranch,
}

// 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 {
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.
func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 {
sess := db.GetEngine(db.DefaultContext).Where("issue_id = ?", pr.IssueID).
func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(ctx context.Context, pr *PullRequest) int64 {
sess := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
And("type = ?", ReviewTypeApprove).
And("official = ?", true).
And("dismissed = ?", false)
Expand All @@ -185,11 +185,11 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest)
}

// 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 {
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("official = ?", true).
And("dismissed = ?", false).
Expand All @@ -204,11 +204,11 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque

// MergeBlockedByOfficialReviewRequests block merge because of some review request to official reviewer
// of from official review
func (protectBranch *ProtectedBranch) MergeBlockedByOfficialReviewRequests(pr *PullRequest) bool {
func (protectBranch *ProtectedBranch) MergeBlockedByOfficialReviewRequests(ctx context.Context, pr *PullRequest) bool {
if !protectBranch.BlockOnOfficialReviewRequests {
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("official = ?", true).
Exist(new(Review))
Expand Down
11 changes: 9 additions & 2 deletions models/db/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,22 @@ func WithContext(f func(ctx *Context) error) error {
}

// 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()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

if err := f(&Context{
Context: DefaultContext,
Context: parentCtx,
e: sess,
}); err != nil {
return err
Expand Down
39 changes: 8 additions & 31 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (e
}

// 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 {
return err
}
Expand Down Expand Up @@ -635,7 +635,7 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
// Check for open dependencies
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
noDeps, err := issueNoDependenciesLeft(e, issue)
noDeps, err := IssueNoDependenciesLeft(ctx, issue)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -693,30 +693,15 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
}

// ChangeIssueStatus changes issue status to open or closed.
func ChangeIssueStatus(issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
ctx, committer, err := db.TxContext()
if err != nil {
return nil, err
}
defer committer.Close()

func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
if err := issue.LoadRepo(ctx); err != nil {
return nil, err
}
if err := issue.loadPoster(db.GetEngine(ctx)); err != nil {
return nil, err
}

comment, err := 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
return changeIssueStatus(ctx, issue, doer, isClosed, false)
}

// ChangeIssueTitle changes the title of this issue, as the given user.
Expand Down Expand Up @@ -787,28 +772,20 @@ func ChangeIssueRef(issue *Issue, doer *user_model.User, oldRef string) (err err
}

// AddDeletePRBranchComment adds delete branch comment for pull request issue
func AddDeletePRBranchComment(doer *user_model.User, repo *repo_model.Repository, issueID int64, branchName string) error {
issue, err := getIssueByID(db.GetEngine(db.DefaultContext), issueID)
if err != nil {
return err
}
ctx, committer, err := db.TxContext()
func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issueID int64, branchName string) error {
issue, err := getIssueByID(db.GetEngine(ctx), issueID)
if err != nil {
return err
}
defer committer.Close()
opts := &CreateCommentOptions{
Type: CommentTypeDeleteBranch,
Doer: doer,
Repo: repo,
Issue: issue,
OldRef: branchName,
}
if _, err = CreateCommentCtx(ctx, opts); err != nil {
return err
}

return committer.Commit()
_, err = CreateCommentCtx(ctx, opts)
return err
}

// UpdateIssueAttachments update attachments by UUIDs for the issue
Expand Down
9 changes: 5 additions & 4 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,15 @@ type PushActionContent struct {

// LoadIssue loads issue from database
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 {
return nil
}
c.Issue, err = getIssueByID(e, c.IssueID)
c.Issue, err = getIssueByID(db.GetEngine(ctx), c.IssueID)
return
}

Expand Down Expand Up @@ -1126,7 +1127,7 @@ func UpdateComment(c *Comment, doer *user_model.User) error {
if _, err := sess.ID(c.ID).AllCols().Update(c); err != nil {
return err
}
if err := c.loadIssue(sess); err != nil {
if err := c.LoadIssueCtx(ctx); err != nil {
return err
}
if err := c.addCrossReferences(ctx, doer, true); err != nil {
Expand Down
10 changes: 4 additions & 6 deletions models/issue_dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package models

import (
"context"

"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/timeutil"
Expand Down Expand Up @@ -117,12 +119,8 @@ func issueDepExists(e db.Engine, issueID, depID int64) (bool, error) {
}

// IssueNoDependenciesLeft checks if issue can be closed
func IssueNoDependenciesLeft(issue *Issue) (bool, error) {
return issueNoDependenciesLeft(db.GetEngine(db.DefaultContext), issue)
}

func issueNoDependenciesLeft(e db.Engine, issue *Issue) (bool, error) {
exists, err := e.
func IssueNoDependenciesLeft(ctx context.Context, issue *Issue) (bool, error) {
exists, err := db.GetEngine(ctx).
Table("issue_dependency").
Select("issue.*").
Join("INNER", "issue", "issue.id = issue_dependency.dependency_id").
Expand Down
7 changes: 4 additions & 3 deletions models/issue_dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package models
import (
"testing"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"

Expand Down Expand Up @@ -43,15 +44,15 @@ func TestCreateIssueDependency(t *testing.T) {
_ = unittest.AssertExistsAndLoadBean(t, &Comment{Type: CommentTypeAddDependency, PosterID: user1.ID, IssueID: issue1.ID})

// Check if dependencies left is correct
left, err := IssueNoDependenciesLeft(issue1)
left, err := IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err)
assert.False(t, left)

// Close #2 and check again
_, err = ChangeIssueStatus(issue2, user1, true)
_, err = ChangeIssueStatus(db.DefaultContext, issue2, user1, true)
assert.NoError(t, err)

left, err = IssueNoDependenciesLeft(issue1)
left, err = IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err)
assert.True(t, left)

Expand Down
4 changes: 2 additions & 2 deletions models/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,12 +443,12 @@ func TestIssue_DeleteIssue(t *testing.T) {
assert.NoError(t, err)
err = CreateIssueDependency(user, issue1, issue2)
assert.NoError(t, err)
left, err := IssueNoDependenciesLeft(issue1)
left, err := IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err)
assert.False(t, left)
err = DeleteIssue(&Issue{ID: 2})
assert.NoError(t, err)
left, err = IssueNoDependenciesLeft(issue1)
left, err = IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err)
assert.True(t, left)
}
Expand Down
6 changes: 3 additions & 3 deletions models/issue_xref.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (comment *Comment) addCrossReferences(stdCtx context.Context, doer *user_mo
if comment.Type != CommentTypeCode && comment.Type != CommentTypeComment {
return nil
}
if err := comment.loadIssue(db.GetEngine(stdCtx)); err != nil {
if err := comment.LoadIssueCtx(stdCtx); err != nil {
return err
}
ctx := &crossReferencesContext{
Expand Down Expand Up @@ -340,9 +340,9 @@ func (comment *Comment) RefIssueIdent() string {
// \/ \/ |__| \/ \/

// 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)
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).
In("ref_action", []references.XRefAction{references.XRefActionCloses, references.XRefActionReopens}).
OrderBy("id").
Expand Down
4 changes: 2 additions & 2 deletions models/issue_xref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
i2 := testCreateIssue(t, 1, 2, "title2", "content2", 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)

pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))
Expand All @@ -118,7 +118,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
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)

refs, err := pr.ResolveCrossReferences()
refs, err := pr.ResolveCrossReferences(db.DefaultContext)
assert.NoError(t, err)
assert.Len(t, refs, 3)
assert.Equal(t, rp.ID, refs[0].ID, "bad ref rp: %+v", refs[0])
Expand Down
Loading