Skip to content

Commit

Permalink
Use context parameter in models/git (#22367)
Browse files Browse the repository at this point in the history 
After #22362, we can feel free to use transactions without
`db.DefaultContext`.

And there are still lots of models using `db.DefaultContext`, I think we
should refactor them carefully and one by one.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
@wolfogre @lunny
wolfogre and lunny authored Jan 9, 2023
1 parent b878155 commit 7adc2de
Show file tree
Hide file tree
Showing 41 changed files with 179 additions and 176 deletions.
52 changes: 26 additions & 26 deletions models/git/branches.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,19 @@ func (protectBranch *ProtectedBranch) IsProtected() bool {
}

// CanUserPush returns if some user could push to this protected branch
func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
func (protectBranch *ProtectedBranch) CanUserPush(ctx context.Context, userID int64) bool {
if !protectBranch.CanPush {
return false
}

if !protectBranch.EnableWhitelist {
if user, err := user_model.GetUserByID(db.DefaultContext, userID); err != nil {
if user, err := user_model.GetUserByID(ctx, userID); err != nil {
log.Error("GetUserByID: %v", err)
return false
} else if repo, err := repo_model.GetRepositoryByID(db.DefaultContext, protectBranch.RepoID); err != nil {
} else if repo, err := repo_model.GetRepositoryByID(ctx, protectBranch.RepoID); err != nil {
log.Error("repo_model.GetRepositoryByID: %v", err)
return false
} else if writeAccess, err := access_model.HasAccessUnit(db.DefaultContext, user, repo, unit.TypeCode, perm.AccessModeWrite); err != nil {
} else if writeAccess, err := access_model.HasAccessUnit(ctx, user, repo, unit.TypeCode, perm.AccessModeWrite); err != nil {
log.Error("HasAccessUnit: %v", err)
return false
} else {
Expand All @@ -95,7 +95,7 @@ func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
return false
}

in, err := organization.IsUserInTeams(db.DefaultContext, userID, protectBranch.WhitelistTeamIDs)
in, err := organization.IsUserInTeams(ctx, userID, protectBranch.WhitelistTeamIDs)
if err != nil {
log.Error("IsUserInTeams: %v", err)
return false
Expand Down Expand Up @@ -320,19 +320,19 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote
}

// GetProtectedBranches get all protected branches
func GetProtectedBranches(repoID int64) ([]*ProtectedBranch, error) {
func GetProtectedBranches(ctx context.Context, repoID int64) ([]*ProtectedBranch, error) {
protectedBranches := make([]*ProtectedBranch, 0)
return protectedBranches, db.GetEngine(db.DefaultContext).Find(&protectedBranches, &ProtectedBranch{RepoID: repoID})
return protectedBranches, db.GetEngine(ctx).Find(&protectedBranches, &ProtectedBranch{RepoID: repoID})
}

// IsProtectedBranch checks if branch is protected
func IsProtectedBranch(repoID int64, branchName string) (bool, error) {
func IsProtectedBranch(ctx context.Context, repoID int64, branchName string) (bool, error) {
protectedBranch := &ProtectedBranch{
RepoID: repoID,
BranchName: branchName,
}

has, err := db.GetEngine(db.DefaultContext).Exist(protectedBranch)
has, err := db.GetEngine(ctx).Exist(protectedBranch)
if err != nil {
return true, err
}
Expand Down Expand Up @@ -413,13 +413,13 @@ func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, curre
}

// DeleteProtectedBranch removes ProtectedBranch relation between the user and repository.
func DeleteProtectedBranch(repoID, id int64) (err error) {
func DeleteProtectedBranch(ctx context.Context, repoID, id int64) (err error) {
protectedBranch := &ProtectedBranch{
RepoID: repoID,
ID: id,
}

if affected, err := db.GetEngine(db.DefaultContext).Delete(protectedBranch); err != nil {
if affected, err := db.GetEngine(ctx).Delete(protectedBranch); err != nil {
return err
} else if affected != 1 {
return fmt.Errorf("delete protected branch ID(%v) failed", id)
Expand All @@ -440,28 +440,28 @@ type DeletedBranch struct {
}

// AddDeletedBranch adds a deleted branch to the database
func AddDeletedBranch(repoID int64, branchName, commit string, deletedByID int64) error {
func AddDeletedBranch(ctx context.Context, repoID int64, branchName, commit string, deletedByID int64) error {
deletedBranch := &DeletedBranch{
RepoID: repoID,
Name: branchName,
Commit: commit,
DeletedByID: deletedByID,
}

_, err := db.GetEngine(db.DefaultContext).Insert(deletedBranch)
_, err := db.GetEngine(ctx).Insert(deletedBranch)
return err
}

// GetDeletedBranches returns all the deleted branches
func GetDeletedBranches(repoID int64) ([]*DeletedBranch, error) {
func GetDeletedBranches(ctx context.Context, repoID int64) ([]*DeletedBranch, error) {
deletedBranches := make([]*DeletedBranch, 0)
return deletedBranches, db.GetEngine(db.DefaultContext).Where("repo_id = ?", repoID).Desc("deleted_unix").Find(&deletedBranches)
return deletedBranches, db.GetEngine(ctx).Where("repo_id = ?", repoID).Desc("deleted_unix").Find(&deletedBranches)
}

// GetDeletedBranchByID get a deleted branch by its ID
func GetDeletedBranchByID(repoID, id int64) (*DeletedBranch, error) {
func GetDeletedBranchByID(ctx context.Context, repoID, id int64) (*DeletedBranch, error) {
deletedBranch := &DeletedBranch{}
has, err := db.GetEngine(db.DefaultContext).Where("repo_id = ?", repoID).And("id = ?", id).Get(deletedBranch)
has, err := db.GetEngine(ctx).Where("repo_id = ?", repoID).And("id = ?", id).Get(deletedBranch)
if err != nil {
return nil, err
}
Expand All @@ -472,13 +472,13 @@ func GetDeletedBranchByID(repoID, id int64) (*DeletedBranch, error) {
}

// RemoveDeletedBranchByID removes a deleted branch from the database
func RemoveDeletedBranchByID(repoID, id int64) (err error) {
func RemoveDeletedBranchByID(ctx context.Context, repoID, id int64) (err error) {
deletedBranch := &DeletedBranch{
RepoID: repoID,
ID: id,
}

if affected, err := db.GetEngine(db.DefaultContext).Delete(deletedBranch); err != nil {
if affected, err := db.GetEngine(ctx).Delete(deletedBranch); err != nil {
return err
} else if affected != 1 {
return fmt.Errorf("remove deleted branch ID(%v) failed", id)
Expand All @@ -498,8 +498,8 @@ func (deletedBranch *DeletedBranch) LoadUser(ctx context.Context) {
}

// RemoveDeletedBranchByName removes all deleted branches
func RemoveDeletedBranchByName(repoID int64, branch string) error {
_, err := db.GetEngine(db.DefaultContext).Where("repo_id=? AND name=?", repoID, branch).Delete(new(DeletedBranch))
func RemoveDeletedBranchByName(ctx context.Context, repoID int64, branch string) error {
_, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repoID, branch).Delete(new(DeletedBranch))
return err
}

Expand All @@ -509,7 +509,7 @@ func RemoveOldDeletedBranches(ctx context.Context, olderThan time.Duration) {
log.Trace("Doing: DeletedBranchesCleanup")

deleteBefore := time.Now().Add(-olderThan)
_, err := db.GetEngine(db.DefaultContext).Where("deleted_unix < ?", deleteBefore.Unix()).Delete(new(DeletedBranch))
_, err := db.GetEngine(ctx).Where("deleted_unix < ?", deleteBefore.Unix()).Delete(new(DeletedBranch))
if err != nil {
log.Error("DeletedBranchesCleanup: %v", err)
}
Expand All @@ -526,19 +526,19 @@ type RenamedBranch struct {
}

// FindRenamedBranch check if a branch was renamed
func FindRenamedBranch(repoID int64, from string) (branch *RenamedBranch, exist bool, err error) {
func FindRenamedBranch(ctx context.Context, repoID int64, from string) (branch *RenamedBranch, exist bool, err error) {
branch = &RenamedBranch{
RepoID: repoID,
From: from,
}
exist, err = db.GetEngine(db.DefaultContext).Get(branch)
exist, err = db.GetEngine(ctx).Get(branch)

return branch, exist, err
}

// RenameBranch rename a branch
func RenameBranch(repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) {
ctx, committer, err := db.TxContext(db.DefaultContext)
func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return err
}
Expand Down
20 changes: 10 additions & 10 deletions models/git/branches_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ func TestAddDeletedBranch(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
firstBranch := unittest.AssertExistsAndLoadBean(t, &git_model.DeletedBranch{ID: 1})

assert.Error(t, git_model.AddDeletedBranch(repo.ID, firstBranch.Name, firstBranch.Commit, firstBranch.DeletedByID))
assert.NoError(t, git_model.AddDeletedBranch(repo.ID, "test", "5655464564554545466464656", int64(1)))
assert.Error(t, git_model.AddDeletedBranch(db.DefaultContext, repo.ID, firstBranch.Name, firstBranch.Commit, firstBranch.DeletedByID))
assert.NoError(t, git_model.AddDeletedBranch(db.DefaultContext, repo.ID, "test", "5655464564554545466464656", int64(1)))
}

func TestGetDeletedBranches(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})

branches, err := git_model.GetDeletedBranches(repo.ID)
branches, err := git_model.GetDeletedBranches(db.DefaultContext, repo.ID)
assert.NoError(t, err)
assert.Len(t, branches, 2)
}
Expand Down Expand Up @@ -65,7 +65,7 @@ func TestRemoveDeletedBranch(t *testing.T) {

firstBranch := unittest.AssertExistsAndLoadBean(t, &git_model.DeletedBranch{ID: 1})

err := git_model.RemoveDeletedBranchByID(repo.ID, 1)
err := git_model.RemoveDeletedBranchByID(db.DefaultContext, repo.ID, 1)
assert.NoError(t, err)
unittest.AssertNotExistsBean(t, firstBranch)
unittest.AssertExistsAndLoadBean(t, &git_model.DeletedBranch{ID: 2})
Expand All @@ -74,7 +74,7 @@ func TestRemoveDeletedBranch(t *testing.T) {
func getDeletedBranch(t *testing.T, branch *git_model.DeletedBranch) *git_model.DeletedBranch {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})

deletedBranch, err := git_model.GetDeletedBranchByID(repo.ID, branch.ID)
deletedBranch, err := git_model.GetDeletedBranchByID(db.DefaultContext, repo.ID, branch.ID)
assert.NoError(t, err)
assert.Equal(t, branch.ID, deletedBranch.ID)
assert.Equal(t, branch.Name, deletedBranch.Name)
Expand All @@ -86,12 +86,12 @@ func getDeletedBranch(t *testing.T, branch *git_model.DeletedBranch) *git_model.

func TestFindRenamedBranch(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
branch, exist, err := git_model.FindRenamedBranch(1, "dev")
branch, exist, err := git_model.FindRenamedBranch(db.DefaultContext, 1, "dev")
assert.NoError(t, err)
assert.Equal(t, true, exist)
assert.Equal(t, "master", branch.To)

_, exist, err = git_model.FindRenamedBranch(1, "unknow")
_, exist, err = git_model.FindRenamedBranch(db.DefaultContext, 1, "unknow")
assert.NoError(t, err)
assert.Equal(t, false, exist)
}
Expand All @@ -110,7 +110,7 @@ func TestRenameBranch(t *testing.T) {
}, git_model.WhitelistOptions{}))
assert.NoError(t, committer.Commit())

assert.NoError(t, git_model.RenameBranch(repo1, "master", "main", func(isDefault bool) error {
assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(isDefault bool) error {
_isDefault = isDefault
return nil
}))
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestOnlyGetDeletedBranchOnCorrectRepo(t *testing.T) {
// is actually on repo with ID 1.
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})

deletedBranch, err := git_model.GetDeletedBranchByID(repo2.ID, 1)
deletedBranch, err := git_model.GetDeletedBranchByID(db.DefaultContext, repo2.ID, 1)

// Expect no error, and the returned branch is nil.
assert.NoError(t, err)
Expand All @@ -154,7 +154,7 @@ func TestOnlyGetDeletedBranchOnCorrectRepo(t *testing.T) {
// This should return the deletedBranch.
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})

deletedBranch, err = git_model.GetDeletedBranchByID(repo1.ID, 1)
deletedBranch, err = git_model.GetDeletedBranchByID(db.DefaultContext, repo1.ID, 1)

// Expect no error, and the returned branch to be not nil.
assert.NoError(t, err)
Expand Down
32 changes: 16 additions & 16 deletions models/git/commit_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ func (status *CommitStatus) loadAttributes(ctx context.Context) (err error) {
}

// APIURL returns the absolute APIURL to this commit-status.
func (status *CommitStatus) APIURL() string {
_ = status.loadAttributes(db.DefaultContext)
func (status *CommitStatus) APIURL(ctx context.Context) string {
_ = status.loadAttributes(ctx)
return status.Repo.APIURL() + "/statuses/" + url.PathEscape(status.SHA)
}

Expand Down Expand Up @@ -162,15 +162,15 @@ type CommitStatusOptions struct {
}

// GetCommitStatuses returns all statuses for a given commit.
func GetCommitStatuses(repo *repo_model.Repository, sha string, opts *CommitStatusOptions) ([]*CommitStatus, int64, error) {
func GetCommitStatuses(ctx context.Context, repo *repo_model.Repository, sha string, opts *CommitStatusOptions) ([]*CommitStatus, int64, error) {
if opts.Page <= 0 {
opts.Page = 1
}
if opts.PageSize <= 0 {
opts.Page = setting.ItemsPerPage
}

countSession := listCommitStatusesStatement(repo, sha, opts)
countSession := listCommitStatusesStatement(ctx, repo, sha, opts)
countSession = db.SetSessionPagination(countSession, opts)
maxResults, err := countSession.Count(new(CommitStatus))
if err != nil {
Expand All @@ -179,14 +179,14 @@ func GetCommitStatuses(repo *repo_model.Repository, sha string, opts *CommitStat
}

statuses := make([]*CommitStatus, 0, opts.PageSize)
findSession := listCommitStatusesStatement(repo, sha, opts)
findSession := listCommitStatusesStatement(ctx, repo, sha, opts)
findSession = db.SetSessionPagination(findSession, opts)
sortCommitStatusesSession(findSession, opts.SortType)
return statuses, maxResults, findSession.Find(&statuses)
}

func listCommitStatusesStatement(repo *repo_model.Repository, sha string, opts *CommitStatusOptions) *xorm.Session {
sess := db.GetEngine(db.DefaultContext).Where("repo_id = ?", repo.ID).And("sha = ?", sha)
func listCommitStatusesStatement(ctx context.Context, repo *repo_model.Repository, sha string, opts *CommitStatusOptions) *xorm.Session {
sess := db.GetEngine(ctx).Where("repo_id = ?", repo.ID).And("sha = ?", sha)
switch opts.State {
case "pending", "success", "error", "failure", "warning":
sess.And("state = ?", opts.State)
Expand Down Expand Up @@ -241,10 +241,10 @@ func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOp
}

// FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts
func FindRepoRecentCommitStatusContexts(repoID int64, before time.Duration) ([]string, error) {
func FindRepoRecentCommitStatusContexts(ctx context.Context, repoID int64, before time.Duration) ([]string, error) {
start := timeutil.TimeStampNow().AddDuration(-before)
ids := make([]int64, 0, 10)
if err := db.GetEngine(db.DefaultContext).Table("commit_status").
if err := db.GetEngine(ctx).Table("commit_status").
Where("repo_id = ?", repoID).
And("updated_unix >= ?", start).
Select("max( id ) as id").
Expand All @@ -257,7 +257,7 @@ func FindRepoRecentCommitStatusContexts(repoID int64, before time.Duration) ([]s
if len(ids) == 0 {
return contexts, nil
}
return contexts, db.GetEngine(db.DefaultContext).Select("context").Table("commit_status").In("id", ids).Find(&contexts)
return contexts, db.GetEngine(ctx).Select("context").Table("commit_status").In("id", ids).Find(&contexts)
}

// NewCommitStatusOptions holds options for creating a CommitStatus
Expand All @@ -269,7 +269,7 @@ type NewCommitStatusOptions struct {
}

// NewCommitStatus save commit statuses into database
func NewCommitStatus(opts NewCommitStatusOptions) error {
func NewCommitStatus(ctx context.Context, opts NewCommitStatusOptions) error {
if opts.Repo == nil {
return fmt.Errorf("NewCommitStatus[nil, %s]: no repository specified", opts.SHA)
}
Expand All @@ -283,7 +283,7 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
return fmt.Errorf("NewCommitStatus[%s, %s]: invalid sha: %w", repoPath, opts.SHA, err)
}

ctx, committer, err := db.TxContext(db.DefaultContext)
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err)
}
Expand Down Expand Up @@ -322,14 +322,14 @@ type SignCommitWithStatuses struct {
}

// ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state
func ParseCommitsWithStatus(oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) []*SignCommitWithStatuses {
func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) []*SignCommitWithStatuses {
newCommits := make([]*SignCommitWithStatuses, 0, len(oldCommits))

for _, c := range oldCommits {
commit := &SignCommitWithStatuses{
SignCommit: c,
}
statuses, _, err := GetLatestCommitStatus(db.DefaultContext, repo.ID, commit.ID.String(), db.ListOptions{})
statuses, _, err := GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptions{})
if err != nil {
log.Error("GetLatestCommitStatus: %v", err)
} else {
Expand All @@ -348,8 +348,8 @@ func hashCommitStatusContext(context string) string {
}

// ConvertFromGitCommit converts git commits into SignCommitWithStatuses
func ConvertFromGitCommit(commits []*git.Commit, repo *repo_model.Repository) []*SignCommitWithStatuses {
return ParseCommitsWithStatus(
func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) []*SignCommitWithStatuses {
return ParseCommitsWithStatus(ctx,
asymkey_model.ParseCommitsWithSignature(
user_model.ValidateCommitsWithEmails(commits),
repo.GetTrustModel(),
Expand Down
12 changes: 6 additions & 6 deletions models/git/commit_status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,28 @@ func TestGetCommitStatuses(t *testing.T) {

sha1 := "1234123412341234123412341234123412341234"

statuses, maxResults, err := git_model.GetCommitStatuses(repo1, sha1, &git_model.CommitStatusOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 50}})
statuses, maxResults, err := git_model.GetCommitStatuses(db.DefaultContext, repo1, sha1, &git_model.CommitStatusOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 50}})
assert.NoError(t, err)
assert.Equal(t, int(maxResults), 5)
assert.Len(t, statuses, 5)

assert.Equal(t, "ci/awesomeness", statuses[0].Context)
assert.Equal(t, structs.CommitStatusPending, statuses[0].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL())
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL(db.DefaultContext))

assert.Equal(t, "cov/awesomeness", statuses[1].Context)
assert.Equal(t, structs.CommitStatusWarning, statuses[1].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL())
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL(db.DefaultContext))

assert.Equal(t, "cov/awesomeness", statuses[2].Context)
assert.Equal(t, structs.CommitStatusSuccess, statuses[2].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL())
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL(db.DefaultContext))

assert.Equal(t, "ci/awesomeness", statuses[3].Context)
assert.Equal(t, structs.CommitStatusFailure, statuses[3].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL())
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL(db.DefaultContext))

assert.Equal(t, "deploy/awesomeness", statuses[4].Context)
assert.Equal(t, structs.CommitStatusError, statuses[4].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL())
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL(db.DefaultContext))
}
Loading

0 comments on commit 7adc2de

Please sign in to comment.