more context for models (#19511)

make more usage of context, to have more db transaction in one session (make diff of #9307 smaller)
go-gitea · Apr 28, 2022 · 06e4687 · 06e4687
1 parent 332b2ec
commit 06e4687
Show file tree

Hide file tree

Showing 54 changed files with 274 additions and 244 deletions.
diff --git a/models/action.go b/models/action.go
@@ -508,7 +508,7 @@ func notifyWatchers(ctx context.Context, actions ...*Action) error {
 					permPR[i] = false
 					continue
 				}
-				perm, err := getUserRepoPermission(ctx, repo, user)
+				perm, err := GetUserRepoPermission(ctx, repo, user)
 				if err != nil {
 					permCode[i] = false
 					permIssue[i] = false

diff --git a/models/branches.go b/models/branches.go
@@ -337,57 +337,57 @@ type WhitelistOptions struct {
 // If ID is 0, it creates a new record. Otherwise, updates existing record.
 // This function also performs check if whitelist user and team's IDs have been changed
 // to avoid unnecessary whitelist delete and regenerate.
-func UpdateProtectBranch(repo *repo_model.Repository, protectBranch *ProtectedBranch, opts WhitelistOptions) (err error) {
-	if err = repo.GetOwner(db.DefaultContext); err != nil {
+func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, protectBranch *ProtectedBranch, opts WhitelistOptions) (err error) {
+	if err = repo.GetOwner(ctx); err != nil {
 		return fmt.Errorf("GetOwner: %v", err)
 	}
 
-	whitelist, err := updateUserWhitelist(repo, protectBranch.WhitelistUserIDs, opts.UserIDs)
+	whitelist, err := updateUserWhitelist(ctx, repo, protectBranch.WhitelistUserIDs, opts.UserIDs)
 	if err != nil {
 		return err
 	}
 	protectBranch.WhitelistUserIDs = whitelist
 
-	whitelist, err = updateUserWhitelist(repo, protectBranch.MergeWhitelistUserIDs, opts.MergeUserIDs)
+	whitelist, err = updateUserWhitelist(ctx, repo, protectBranch.MergeWhitelistUserIDs, opts.MergeUserIDs)
 	if err != nil {
 		return err
 	}
 	protectBranch.MergeWhitelistUserIDs = whitelist
 
-	whitelist, err = updateApprovalWhitelist(repo, protectBranch.ApprovalsWhitelistUserIDs, opts.ApprovalsUserIDs)
+	whitelist, err = updateApprovalWhitelist(ctx, repo, protectBranch.ApprovalsWhitelistUserIDs, opts.ApprovalsUserIDs)
 	if err != nil {
 		return err
 	}
 	protectBranch.ApprovalsWhitelistUserIDs = whitelist
 
 	// if the repo is in an organization
-	whitelist, err = updateTeamWhitelist(repo, protectBranch.WhitelistTeamIDs, opts.TeamIDs)
+	whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.WhitelistTeamIDs, opts.TeamIDs)
 	if err != nil {
 		return err
 	}
 	protectBranch.WhitelistTeamIDs = whitelist
 
-	whitelist, err = updateTeamWhitelist(repo, protectBranch.MergeWhitelistTeamIDs, opts.MergeTeamIDs)
+	whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.MergeWhitelistTeamIDs, opts.MergeTeamIDs)
 	if err != nil {
 		return err
 	}
 	protectBranch.MergeWhitelistTeamIDs = whitelist
 
-	whitelist, err = updateTeamWhitelist(repo, protectBranch.ApprovalsWhitelistTeamIDs, opts.ApprovalsTeamIDs)
+	whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.ApprovalsWhitelistTeamIDs, opts.ApprovalsTeamIDs)
 	if err != nil {
 		return err
 	}
 	protectBranch.ApprovalsWhitelistTeamIDs = whitelist
 
 	// Make sure protectBranch.ID is not 0 for whitelists
 	if protectBranch.ID == 0 {
-		if _, err = db.GetEngine(db.DefaultContext).Insert(protectBranch); err != nil {
+		if _, err = db.GetEngine(ctx).Insert(protectBranch); err != nil {
 			return fmt.Errorf("Insert: %v", err)
 		}
 		return nil
 	}
 
-	if _, err = db.GetEngine(db.DefaultContext).ID(protectBranch.ID).AllCols().Update(protectBranch); err != nil {
+	if _, err = db.GetEngine(ctx).ID(protectBranch.ID).AllCols().Update(protectBranch); err != nil {
 		return fmt.Errorf("Update: %v", err)
 	}
 
@@ -416,15 +416,15 @@ func IsProtectedBranch(repoID int64, branchName string) (bool, error) {
 
 // updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
 // the users from newWhitelist which have explicit read or write access to the repo.
-func updateApprovalWhitelist(repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
+func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
 	hasUsersChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
 	if !hasUsersChanged {
 		return currentWhitelist, nil
 	}
 
 	whitelist = make([]int64, 0, len(newWhitelist))
 	for _, userID := range newWhitelist {
-		if reader, err := IsRepoReader(repo, userID); err != nil {
+		if reader, err := IsRepoReader(ctx, repo, userID); err != nil {
 			return nil, err
 		} else if !reader {
 			continue
@@ -437,19 +437,19 @@ func updateApprovalWhitelist(repo *repo_model.Repository, currentWhitelist, newW
 
 // updateUserWhitelist checks whether the user whitelist changed and returns a whitelist with
 // the users from newWhitelist which have write access to the repo.
-func updateUserWhitelist(repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
+func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
 	hasUsersChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
 	if !hasUsersChanged {
 		return currentWhitelist, nil
 	}
 
 	whitelist = make([]int64, 0, len(newWhitelist))
 	for _, userID := range newWhitelist {
-		user, err := user_model.GetUserByID(userID)
+		user, err := user_model.GetUserByIDCtx(ctx, userID)
 		if err != nil {
 			return nil, fmt.Errorf("GetUserByID [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
 		}
-		perm, err := GetUserRepoPermission(repo, user)
+		perm, err := GetUserRepoPermission(ctx, repo, user)
 		if err != nil {
 			return nil, fmt.Errorf("GetUserRepoPermission [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
 		}
@@ -466,13 +466,13 @@ func updateUserWhitelist(repo *repo_model.Repository, currentWhitelist, newWhite
 
 // updateTeamWhitelist checks whether the team whitelist changed and returns a whitelist with
 // the teams from newWhitelist which have write access to the repo.
-func updateTeamWhitelist(repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
+func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
 	hasTeamsChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
 	if !hasTeamsChanged {
 		return currentWhitelist, nil
 	}
 
-	teams, err := organization.GetTeamsWithAccessToRepo(repo.OwnerID, repo.ID, perm.AccessModeRead)
+	teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead)
 	if err != nil {
 		return nil, fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err)
 	}

diff --git a/models/branches_test.go b/models/branches_test.go
@@ -7,6 +7,7 @@ package models
 import (
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 
@@ -99,11 +100,14 @@ func TestRenameBranch(t *testing.T) {
 	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}).(*repo_model.Repository)
 	_isDefault := false
 
-	err := UpdateProtectBranch(repo1, &ProtectedBranch{
+	ctx, committer, err := db.TxContext()
+	defer committer.Close()
+	assert.NoError(t, err)
+	assert.NoError(t, UpdateProtectBranch(ctx, repo1, &ProtectedBranch{
 		RepoID:     repo1.ID,
 		BranchName: "master",
-	}, WhitelistOptions{})
-	assert.NoError(t, err)
+	}, WhitelistOptions{}))
+	assert.NoError(t, committer.Commit())
 
 	assert.NoError(t, RenameBranch(repo1, "master", "main", func(isDefault bool) error {
 		_isDefault = isDefault

diff --git a/models/commit_status.go b/models/commit_status.go
@@ -232,12 +232,13 @@ type CommitStatusIndex struct {
 
 // GetLatestCommitStatus returns all statuses with a unique context for a given commit.
 func GetLatestCommitStatus(repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
-	return getLatestCommitStatus(db.GetEngine(db.DefaultContext), repoID, sha, listOptions)
+	return GetLatestCommitStatusCtx(db.DefaultContext, repoID, sha, listOptions)
 }
 
-func getLatestCommitStatus(e db.Engine, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
+// GetLatestCommitStatusCtx returns all statuses with a unique context for a given commit.
+func GetLatestCommitStatusCtx(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
 	ids := make([]int64, 0, 10)
-	sess := e.Table(&CommitStatus{}).
+	sess := db.GetEngine(ctx).Table(&CommitStatus{}).
 		Where("repo_id = ?", repoID).And("sha = ?", sha).
 		Select("max( id ) as id").
 		GroupBy("context_hash").OrderBy("max( id ) desc")
@@ -252,7 +253,7 @@ func getLatestCommitStatus(e db.Engine, repoID int64, sha string, listOptions db
 	if len(ids) == 0 {
 		return statuses, count, nil
 	}
-	return statuses, count, e.In("id", ids).Find(&statuses)
+	return statuses, count, db.GetEngine(ctx).In("id", ids).Find(&statuses)
 }
 
 // FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts

diff --git a/models/issue.go b/models/issue.go
@@ -162,13 +162,9 @@ func (issue *Issue) GetPullRequest() (pr *PullRequest, err error) {
 }
 
 // LoadLabels loads labels
-func (issue *Issue) LoadLabels() error {
-	return issue.loadLabels(db.GetEngine(db.DefaultContext))
-}
-
-func (issue *Issue) loadLabels(e db.Engine) (err error) {
+func (issue *Issue) LoadLabels(ctx context.Context) (err error) {
 	if issue.Labels == nil {
-		issue.Labels, err = getLabelsByIssueID(e, issue.ID)
+		issue.Labels, err = getLabelsByIssueID(db.GetEngine(ctx), issue.ID)
 		if err != nil {
 			return fmt.Errorf("getLabelsByIssueID [%d]: %v", issue.ID, err)
 		}
@@ -313,7 +309,7 @@ func (issue *Issue) loadAttributes(ctx context.Context) (err error) {
 		return
 	}
 
-	if err = issue.loadLabels(e); err != nil {
+	if err = issue.LoadLabels(ctx); err != nil {
 		return
 	}
 
@@ -493,7 +489,7 @@ func ClearIssueLabels(issue *Issue, doer *user_model.User) (err error) {
 		return err
 	}
 
-	perm, err := getUserRepoPermission(ctx, issue.Repo, doer)
+	perm, err := GetUserRepoPermission(ctx, issue.Repo, doer)
 	if err != nil {
 		return err
 	}
@@ -539,7 +535,7 @@ func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (e
 		return err
 	}
 
-	if err = issue.loadLabels(db.GetEngine(ctx)); err != nil {
+	if err = issue.LoadLabels(ctx); err != nil {
 		return err
 	}
 
@@ -587,7 +583,7 @@ func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (e
 	}
 
 	issue.Labels = nil
-	if err = issue.loadLabels(db.GetEngine(ctx)); err != nil {
+	if err = issue.LoadLabels(ctx); err != nil {
 		return err
 	}
 
@@ -2341,9 +2337,9 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
 			continue
 		}
 		// Normal users must have read access to the referencing issue
-		perm, err := getUserRepoPermission(ctx, issue.Repo, user)
+		perm, err := GetUserRepoPermission(ctx, issue.Repo, user)
 		if err != nil {
-			return nil, fmt.Errorf("getUserRepoPermission [%d]: %v", user.ID, err)
+			return nil, fmt.Errorf("GetUserRepoPermission [%d]: %v", user.ID, err)
 		}
 		if !perm.CanReadIssuesOrPulls(issue.IsPull) {
 			continue

diff --git a/models/issue_label.go b/models/issue_label.go
@@ -613,7 +613,6 @@ func NewIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error
 		return err
 	}
 	defer committer.Close()
-	sess := db.GetEngine(ctx)
 
 	if err = issue.LoadRepo(ctx); err != nil {
 		return err
@@ -629,7 +628,7 @@ func NewIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error
 	}
 
 	issue.Labels = nil
-	if err = issue.loadLabels(sess); err != nil {
+	if err = issue.LoadLabels(ctx); err != nil {
 		return err
 	}
 
@@ -670,7 +669,7 @@ func NewIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (err e
 	}
 
 	issue.Labels = nil
-	if err = issue.loadLabels(db.GetEngine(ctx)); err != nil {
+	if err = issue.LoadLabels(ctx); err != nil {
 		return err
 	}
 
@@ -707,23 +706,13 @@ func deleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *use
 }
 
 // DeleteIssueLabel deletes issue-label relation.
-func DeleteIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error) {
-	ctx, committer, err := db.TxContext()
-	if err != nil {
-		return err
-	}
-	defer committer.Close()
-
-	if err = deleteIssueLabel(ctx, issue, label, doer); err != nil {
+func DeleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_model.User) error {
+	if err := deleteIssueLabel(ctx, issue, label, doer); err != nil {
 		return err
 	}
 
 	issue.Labels = nil
-	if err = issue.loadLabels(db.GetEngine(ctx)); err != nil {
-		return err
-	}
-
-	return committer.Commit()
+	return issue.LoadLabels(ctx)
 }
 
 func deleteLabelsByRepoID(sess db.Engine, repoID int64) error {

diff --git a/models/issue_label_test.go b/models/issue_label_test.go
@@ -369,7 +369,12 @@ func TestDeleteIssueLabel(t *testing.T) {
 			}
 		}
 
-		assert.NoError(t, DeleteIssueLabel(issue, label, doer))
+		ctx, committer, err := db.TxContext()
+		defer committer.Close()
+		assert.NoError(t, err)
+		assert.NoError(t, DeleteIssueLabel(ctx, issue, label, doer))
+		assert.NoError(t, committer.Commit())
+
 		unittest.AssertNotExistsBean(t, &IssueLabel{IssueID: issueID, LabelID: labelID})
 		unittest.AssertExistsAndLoadBean(t, &Comment{
 			Type:     CommentTypeLabel,

diff --git a/models/issue_xref.go b/models/issue_xref.go
@@ -215,7 +215,7 @@ func (issue *Issue) verifyReferencedIssue(stdCtx context.Context, ctx *crossRefe
 
 	// Check doer permissions; set action to None if the doer can't change the destination
 	if refIssue.RepoID != ctx.OrigIssue.RepoID || ref.Action != references.XRefActionNone {
-		perm, err := getUserRepoPermission(stdCtx, refIssue.Repo, ctx.Doer)
+		perm, err := GetUserRepoPermission(stdCtx, refIssue.Repo, ctx.Doer)
 		if err != nil {
 			return nil, references.XRefActionNone, err
 		}