Skip to content
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

Use for a repo action one database transaction #19576

Merged
merged 17 commits into from
May 3, 2022
Merged
Show file tree
Hide file tree
Changes from 12 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
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
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
29 changes: 10 additions & 19 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,23 +227,23 @@ func (pr *PullRequest) LoadProtectedBranchCtx(ctx context.Context) (err error) {
}

// 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 {
var err error
pr.HeadRepo, err = repo_model.GetRepositoryByID(pr.HeadRepoID)
pr.HeadRepo, err = repo_model.GetRepositoryByIDCtx(ctx, pr.HeadRepoID)
if err != nil {
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)
}
if err := pr.LoadBaseRepo(); err != nil {
if err := pr.LoadBaseRepoCtx(ctx); err != nil {
return "", fmt.Errorf("LoadBaseRepo: %v", err)
}

issueReference := "#"
if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) {
if pr.BaseRepo.UnitEnabledCtx(ctx, unit.TypeExternalTracker) {
issueReference = "!"
}

Expand Down Expand Up @@ -337,14 +337,14 @@ func (pr *PullRequest) getReviewedByLines(writer io.Writer) error {
}

// GetDefaultSquashMessage returns default message used when squash and merging pull request
func (pr *PullRequest) GetDefaultSquashMessage() (string, error) {
if err := pr.LoadIssue(); err != nil {
func (pr *PullRequest) GetDefaultSquashMessage(ctx context.Context) (string, error) {
if err := pr.LoadIssueCtx(ctx); err != nil {
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)
}
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
Expand All @@ -371,7 +371,7 @@ func (pr *PullRequest) IsEmpty() bool {
}

// 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 {
return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index)
}
Expand All @@ -380,12 +380,6 @@ func (pr *PullRequest) SetMerged() (bool, error) {
}

pr.HasMerged = true

ctx, committer, err := db.TxContext()
if err != nil {
return false, err
}
defer committer.Close()
sess := db.GetEngine(ctx)

if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
Expand Down Expand Up @@ -432,9 +426,6 @@ func (pr *PullRequest) SetMerged() (bool, error) {
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
}

Expand Down
4 changes: 2 additions & 2 deletions models/pull_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ func GetUnmergedPullRequestsByHeadInfo(repoID int64, branch string) ([]*PullRequ

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