DBContext is just a Context (#17100)

* DBContext is just a Context

This PR removes some of the specialness from the DBContext and makes it context
This allows us to simplify the GetEngine code to wrap around any context in future
and means that we can change our loadRepo(e Engine) functions to simply take contexts.

Signed-off-by: Andrew Thornton <[email protected]>

* fix unit tests

Signed-off-by: Andrew Thornton <[email protected]>

* another place that needs to set the initial context

Signed-off-by: Andrew Thornton <[email protected]>

* avoid race

Signed-off-by: Andrew Thornton <[email protected]>

* change attachment error

Signed-off-by: Andrew Thornton <[email protected]>

Author: zeripath

models/access.go

@@ -116,7 +116,7 @@ func (repoAccess) TableName() string {
 
 // GetRepositoryAccesses finds all repositories with their access mode where a user has access but does not own.
 func (user *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
-	rows, err := db.DefaultContext().Engine().
+	rows, err := db.GetEngine(db.DefaultContext).
 		Join("INNER", "repository", "repository.id = access.repo_id").
 		Where("access.user_id = ?", user.ID).
 		And("repository.owner_id <> ?", user.ID).
@@ -151,7 +151,7 @@ func (user *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
 // GetAccessibleRepositories finds repositories which the user has access but does not own.
 // If limit is smaller than 1 means returns all found results.
 func (user *User) GetAccessibleRepositories(limit int) (repos []*Repository, _ error) {
-	sess := db.DefaultContext().Engine().
+	sess := db.GetEngine(db.DefaultContext).
 		Where("owner_id !=? ", user.ID).
 		Desc("updated_unix")
 	if limit > 0 {
@@ -342,5 +342,5 @@ func (repo *Repository) recalculateAccesses(e db.Engine) error {
 
 // RecalculateAccesses recalculates all accesses for repository.
 func (repo *Repository) RecalculateAccesses() error {
-	return repo.recalculateAccesses(db.DefaultContext().Engine())
+	return repo.recalculateAccesses(db.GetEngine(db.DefaultContext))
 }

models/access_test.go

@@ -127,12 +127,12 @@ func TestRepository_RecalculateAccesses(t *testing.T) {
 	repo1 := db.AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
 	assert.NoError(t, repo1.GetOwner())
 
-	_, err := db.DefaultContext().Engine().Delete(&Collaboration{UserID: 2, RepoID: 3})
+	_, err := db.GetEngine(db.DefaultContext).Delete(&Collaboration{UserID: 2, RepoID: 3})
 	assert.NoError(t, err)
 	assert.NoError(t, repo1.RecalculateAccesses())
 
 	access := &Access{UserID: 2, RepoID: 3}
-	has, err := db.DefaultContext().Engine().Get(access)
+	has, err := db.GetEngine(db.DefaultContext).Get(access)
 	assert.NoError(t, err)
 	assert.True(t, has)
 	assert.Equal(t, AccessModeOwner, access.Mode)
@@ -144,11 +144,11 @@ func TestRepository_RecalculateAccesses2(t *testing.T) {
 	repo1 := db.AssertExistsAndLoadBean(t, &Repository{ID: 4}).(*Repository)
 	assert.NoError(t, repo1.GetOwner())
 
-	_, err := db.DefaultContext().Engine().Delete(&Collaboration{UserID: 4, RepoID: 4})
+	_, err := db.GetEngine(db.DefaultContext).Delete(&Collaboration{UserID: 4, RepoID: 4})
 	assert.NoError(t, err)
 	assert.NoError(t, repo1.RecalculateAccesses())
 
-	has, err := db.DefaultContext().Engine().Get(&Access{UserID: 4, RepoID: 4})
+	has, err := db.GetEngine(db.DefaultContext).Get(&Access{UserID: 4, RepoID: 4})
 	assert.NoError(t, err)
 	assert.False(t, has)
 }
@@ -158,15 +158,15 @@ func TestRepository_RecalculateAccesses3(t *testing.T) {
 	team5 := db.AssertExistsAndLoadBean(t, &Team{ID: 5}).(*Team)
 	user29 := db.AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
 
-	has, err := db.DefaultContext().Engine().Get(&Access{UserID: 29, RepoID: 23})
+	has, err := db.GetEngine(db.DefaultContext).Get(&Access{UserID: 29, RepoID: 23})
 	assert.NoError(t, err)
 	assert.False(t, has)
 
 	// adding user29 to team5 should add an explicit access row for repo 23
 	// even though repo 23 is public
 	assert.NoError(t, AddTeamMember(team5, user29.ID))
 
-	has, err = db.DefaultContext().Engine().Get(&Access{UserID: 29, RepoID: 23})
+	has, err = db.GetEngine(db.DefaultContext).Get(&Access{UserID: 29, RepoID: 23})
 	assert.NoError(t, err)
 	assert.True(t, has)
 }

models/action.go

@@ -208,7 +208,7 @@ func GetRepositoryFromMatch(ownerName, repoName string) (*Repository, error) {
 
 // GetCommentLink returns link to action comment.
 func (a *Action) GetCommentLink() string {
-	return a.getCommentLink(db.DefaultContext().Engine())
+	return a.getCommentLink(db.GetEngine(db.DefaultContext))
 }
 
 func (a *Action) getCommentLink(e db.Engine) string {
@@ -317,7 +317,7 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) {
 
 	actions := make([]*Action, 0, setting.UI.FeedPagingNum)
 
-	if err := db.DefaultContext().Engine().Limit(setting.UI.FeedPagingNum).Desc("created_unix").Where(cond).Find(&actions); err != nil {
+	if err := db.GetEngine(db.DefaultContext).Limit(setting.UI.FeedPagingNum).Desc("created_unix").Where(cond).Find(&actions); err != nil {
 		return nil, fmt.Errorf("Find: %v", err)
 	}
 
@@ -408,6 +408,6 @@ func DeleteOldActions(olderThan time.Duration) (err error) {
 		return nil
 	}
 
-	_, err = db.DefaultContext().Engine().Where("created_unix < ?", time.Now().Add(-olderThan).Unix()).Delete(&Action{})
+	_, err = db.GetEngine(db.DefaultContext).Where("created_unix < ?", time.Now().Add(-olderThan).Unix()).Delete(&Action{})
 	return
 }

models/action_list.go

@@ -45,7 +45,7 @@ func (actions ActionList) loadUsers(e db.Engine) ([]*User, error) {
 
 // LoadUsers loads actions' all users
 func (actions ActionList) LoadUsers() ([]*User, error) {
-	return actions.loadUsers(db.DefaultContext().Engine())
+	return actions.loadUsers(db.GetEngine(db.DefaultContext))
 }
 
 func (actions ActionList) getRepoIDs() []int64 {
@@ -80,7 +80,7 @@ func (actions ActionList) loadRepositories(e db.Engine) ([]*Repository, error) {
 
 // LoadRepositories loads actions' all repositories
 func (actions ActionList) LoadRepositories() ([]*Repository, error) {
-	return actions.loadRepositories(db.DefaultContext().Engine())
+	return actions.loadRepositories(db.GetEngine(db.DefaultContext))
 }
 
 // loadAttributes loads all attributes
@@ -98,5 +98,5 @@ func (actions ActionList) loadAttributes(e db.Engine) (err error) {
 
 // LoadAttributes loads attributes of the actions
 func (actions ActionList) LoadAttributes() error {
-	return actions.loadAttributes(db.DefaultContext().Engine())
+	return actions.loadAttributes(db.GetEngine(db.DefaultContext))
 }

models/admin.go

@@ -44,7 +44,7 @@ func (n *Notice) TrStr() string {
 
 // CreateNotice creates new system notice.
 func CreateNotice(tp NoticeType, desc string, args ...interface{}) error {
-	return createNotice(db.DefaultContext().Engine(), tp, desc, args...)
+	return createNotice(db.GetEngine(db.DefaultContext), tp, desc, args...)
 }
 
 func createNotice(e db.Engine, tp NoticeType, desc string, args ...interface{}) error {
@@ -61,19 +61,19 @@ func createNotice(e db.Engine, tp NoticeType, desc string, args ...interface{})
 
 // CreateRepositoryNotice creates new system notice with type NoticeRepository.
 func CreateRepositoryNotice(desc string, args ...interface{}) error {
-	return createNotice(db.DefaultContext().Engine(), NoticeRepository, desc, args...)
+	return createNotice(db.GetEngine(db.DefaultContext), NoticeRepository, desc, args...)
 }
 
 // RemoveAllWithNotice removes all directories in given path and
 // creates a system notice when error occurs.
 func RemoveAllWithNotice(title, path string) {
-	removeAllWithNotice(db.DefaultContext().Engine(), title, path)
+	removeAllWithNotice(db.GetEngine(db.DefaultContext), title, path)
 }
 
 // RemoveStorageWithNotice removes a file from the storage and
 // creates a system notice when error occurs.
 func RemoveStorageWithNotice(bucket storage.ObjectStorage, title, path string) {
-	removeStorageWithNotice(db.DefaultContext().Engine(), bucket, title, path)
+	removeStorageWithNotice(db.GetEngine(db.DefaultContext), bucket, title, path)
 }
 
 func removeStorageWithNotice(e db.Engine, bucket storage.ObjectStorage, title, path string) {
@@ -98,33 +98,33 @@ func removeAllWithNotice(e db.Engine, title, path string) {
 
 // CountNotices returns number of notices.
 func CountNotices() int64 {
-	count, _ := db.DefaultContext().Engine().Count(new(Notice))
+	count, _ := db.GetEngine(db.DefaultContext).Count(new(Notice))
 	return count
 }
 
 // Notices returns notices in given page.
 func Notices(page, pageSize int) ([]*Notice, error) {
 	notices := make([]*Notice, 0, pageSize)
-	return notices, db.DefaultContext().Engine().
+	return notices, db.GetEngine(db.DefaultContext).
 		Limit(pageSize, (page-1)*pageSize).
 		Desc("id").
 		Find(&notices)
 }
 
 // DeleteNotice deletes a system notice by given ID.
 func DeleteNotice(id int64) error {
-	_, err := db.DefaultContext().Engine().ID(id).Delete(new(Notice))
+	_, err := db.GetEngine(db.DefaultContext).ID(id).Delete(new(Notice))
 	return err
 }
 
 // DeleteNotices deletes all notices with ID from start to end (inclusive).
 func DeleteNotices(start, end int64) error {
 	if start == 0 && end == 0 {
-		_, err := db.DefaultContext().Engine().Exec("DELETE FROM notice")
+		_, err := db.GetEngine(db.DefaultContext).Exec("DELETE FROM notice")
 		return err
 	}
 
-	sess := db.DefaultContext().Engine().Where("id >= ?", start)
+	sess := db.GetEngine(db.DefaultContext).Where("id >= ?", start)
 	if end > 0 {
 		sess.And("id <= ?", end)
 	}
@@ -137,7 +137,7 @@ func DeleteNoticesByIDs(ids []int64) error {
 	if len(ids) == 0 {
 		return nil
 	}
-	_, err := db.DefaultContext().Engine().
+	_, err := db.GetEngine(db.DefaultContext).
 		In("id", ids).
 		Delete(new(Notice))
 	return err
@@ -146,7 +146,7 @@ func DeleteNoticesByIDs(ids []int64) error {
 // GetAdminUser returns the first administrator
 func GetAdminUser() (*User, error) {
 	var admin User
-	has, err := db.DefaultContext().Engine().Where("is_admin=?", true).Get(&admin)
+	has, err := db.GetEngine(db.DefaultContext).Where("is_admin=?", true).Get(&admin)
 	if err != nil {
 		return nil, err
 	} else if !has {

models/attachment.go

@@ -5,6 +5,7 @@
 package models
 
 import (
+	"context"
 	"fmt"
 	"path"
 
@@ -38,7 +39,7 @@ func init() {
 // IncreaseDownloadCount is update download count + 1
 func (a *Attachment) IncreaseDownloadCount() error {
 	// Update download count.
-	if _, err := db.DefaultContext().Engine().Exec("UPDATE `attachment` SET download_count=download_count+1 WHERE id=?", a.ID); err != nil {
+	if _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE `attachment` SET download_count=download_count+1 WHERE id=?", a.ID); err != nil {
 		return fmt.Errorf("increase attachment count: %v", err)
 	}
 
@@ -86,7 +87,7 @@ func (a *Attachment) LinkedRepository() (*Repository, UnitType, error) {
 
 // GetAttachmentByID returns attachment by given id
 func GetAttachmentByID(id int64) (*Attachment, error) {
-	return getAttachmentByID(db.DefaultContext().Engine(), id)
+	return getAttachmentByID(db.GetEngine(db.DefaultContext), id)
 }
 
 func getAttachmentByID(e db.Engine, id int64) (*Attachment, error) {
@@ -111,8 +112,8 @@ func getAttachmentByUUID(e db.Engine, uuid string) (*Attachment, error) {
 }
 
 // GetAttachmentsByUUIDs returns attachment by given UUID list.
-func GetAttachmentsByUUIDs(ctx *db.Context, uuids []string) ([]*Attachment, error) {
-	return getAttachmentsByUUIDs(ctx.Engine(), uuids)
+func GetAttachmentsByUUIDs(ctx context.Context, uuids []string) ([]*Attachment, error) {
+	return getAttachmentsByUUIDs(db.GetEngine(ctx), uuids)
 }
 
 func getAttachmentsByUUIDs(e db.Engine, uuids []string) ([]*Attachment, error) {
@@ -127,17 +128,17 @@ func getAttachmentsByUUIDs(e db.Engine, uuids []string) ([]*Attachment, error) {
 
 // GetAttachmentByUUID returns attachment by given UUID.
 func GetAttachmentByUUID(uuid string) (*Attachment, error) {
-	return getAttachmentByUUID(db.DefaultContext().Engine(), uuid)
+	return getAttachmentByUUID(db.GetEngine(db.DefaultContext), uuid)
 }
 
 // ExistAttachmentsByUUID returns true if attachment is exist by given UUID
 func ExistAttachmentsByUUID(uuid string) (bool, error) {
-	return db.DefaultContext().Engine().Where("`uuid`=?", uuid).Exist(new(Attachment))
+	return db.GetEngine(db.DefaultContext).Where("`uuid`=?", uuid).Exist(new(Attachment))
 }
 
 // GetAttachmentByReleaseIDFileName returns attachment by given releaseId and fileName.
 func GetAttachmentByReleaseIDFileName(releaseID int64, fileName string) (*Attachment, error) {
-	return getAttachmentByReleaseIDFileName(db.DefaultContext().Engine(), releaseID, fileName)
+	return getAttachmentByReleaseIDFileName(db.GetEngine(db.DefaultContext), releaseID, fileName)
 }
 
 func getAttachmentsByIssueID(e db.Engine, issueID int64) ([]*Attachment, error) {
@@ -147,12 +148,12 @@ func getAttachmentsByIssueID(e db.Engine, issueID int64) ([]*Attachment, error)
 
 // GetAttachmentsByIssueID returns all attachments of an issue.
 func GetAttachmentsByIssueID(issueID int64) ([]*Attachment, error) {
-	return getAttachmentsByIssueID(db.DefaultContext().Engine(), issueID)
+	return getAttachmentsByIssueID(db.GetEngine(db.DefaultContext), issueID)
 }
 
 // GetAttachmentsByCommentID returns all attachments if comment by given ID.
 func GetAttachmentsByCommentID(commentID int64) ([]*Attachment, error) {
-	return getAttachmentsByCommentID(db.DefaultContext().Engine(), commentID)
+	return getAttachmentsByCommentID(db.GetEngine(db.DefaultContext), commentID)
 }
 
 func getAttachmentsByCommentID(e db.Engine, commentID int64) ([]*Attachment, error) {
@@ -174,12 +175,12 @@ func getAttachmentByReleaseIDFileName(e db.Engine, releaseID int64, fileName str
 
 // DeleteAttachment deletes the given attachment and optionally the associated file.
 func DeleteAttachment(a *Attachment, remove bool) error {
-	_, err := DeleteAttachments(db.DefaultContext(), []*Attachment{a}, remove)
+	_, err := DeleteAttachments(db.DefaultContext, []*Attachment{a}, remove)
 	return err
 }
 
 // DeleteAttachments deletes the given attachments and optionally the associated files.
-func DeleteAttachments(ctx *db.Context, attachments []*Attachment, remove bool) (int, error) {
+func DeleteAttachments(ctx context.Context, attachments []*Attachment, remove bool) (int, error) {
 	if len(attachments) == 0 {
 		return 0, nil
 	}
@@ -189,7 +190,7 @@ func DeleteAttachments(ctx *db.Context, attachments []*Attachment, remove bool)
 		ids = append(ids, a.ID)
 	}
 
-	cnt, err := ctx.Engine().In("id", ids).NoAutoCondition().Delete(attachments[0])
+	cnt, err := db.GetEngine(ctx).In("id", ids).NoAutoCondition().Delete(attachments[0])
 	if err != nil {
 		return 0, err
 	}
@@ -211,7 +212,7 @@ func DeleteAttachmentsByIssue(issueID int64, remove bool) (int, error) {
 		return 0, err
 	}
 
-	return DeleteAttachments(db.DefaultContext(), attachments, remove)
+	return DeleteAttachments(db.DefaultContext, attachments, remove)
 }
 
 // DeleteAttachmentsByComment deletes all attachments associated with the given comment.
@@ -221,20 +222,20 @@ func DeleteAttachmentsByComment(commentID int64, remove bool) (int, error) {
 		return 0, err
 	}
 
-	return DeleteAttachments(db.DefaultContext(), attachments, remove)
+	return DeleteAttachments(db.DefaultContext, attachments, remove)
 }
 
 // UpdateAttachment updates the given attachment in database
 func UpdateAttachment(atta *Attachment) error {
-	return updateAttachment(db.DefaultContext().Engine(), atta)
+	return updateAttachment(db.GetEngine(db.DefaultContext), atta)
 }
 
 // UpdateAttachmentByUUID Updates attachment via uuid
-func UpdateAttachmentByUUID(ctx *db.Context, attach *Attachment, cols ...string) error {
+func UpdateAttachmentByUUID(ctx context.Context, attach *Attachment, cols ...string) error {
 	if attach.UUID == "" {
-		return fmt.Errorf("Attachement uuid should not blank")
+		return fmt.Errorf("attachment uuid should be not blank")
 	}
-	_, err := ctx.Engine().Where("uuid=?", attach.UUID).Cols(cols...).Update(attach)
+	_, err := db.GetEngine(ctx).Where("uuid=?", attach.UUID).Cols(cols...).Update(attach)
 	return err
 }
 
@@ -252,7 +253,7 @@ func updateAttachment(e db.Engine, atta *Attachment) error {
 
 // DeleteAttachmentsByRelease deletes all attachments associated with the given release.
 func DeleteAttachmentsByRelease(releaseID int64) error {
-	_, err := db.DefaultContext().Engine().Where("release_id = ?", releaseID).Delete(&Attachment{})
+	_, err := db.GetEngine(db.DefaultContext).Where("release_id = ?", releaseID).Delete(&Attachment{})
 	return err
 }
 
@@ -262,7 +263,7 @@ func IterateAttachment(f func(attach *Attachment) error) error {
 	const batchSize = 100
 	for {
 		attachments := make([]*Attachment, 0, batchSize)
-		if err := db.DefaultContext().Engine().Limit(batchSize, start).Find(&attachments); err != nil {
+		if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&attachments); err != nil {
 			return err
 		}
 		if len(attachments) == 0 {
@@ -280,13 +281,13 @@ func IterateAttachment(f func(attach *Attachment) error) error {
 
 // CountOrphanedAttachments returns the number of bad attachments
 func CountOrphanedAttachments() (int64, error) {
-	return db.DefaultContext().Engine().Where("(issue_id > 0 and issue_id not in (select id from issue)) or (release_id > 0 and release_id not in (select id from `release`))").
+	return db.GetEngine(db.DefaultContext).Where("(issue_id > 0 and issue_id not in (select id from issue)) or (release_id > 0 and release_id not in (select id from `release`))").
 		Count(new(Attachment))
 }
 
 // DeleteOrphanedAttachments delete all bad attachments
 func DeleteOrphanedAttachments() error {
-	_, err := db.DefaultContext().Engine().Where("(issue_id > 0 and issue_id not in (select id from issue)) or (release_id > 0 and release_id not in (select id from `release`))").
+	_, err := db.GetEngine(db.DefaultContext).Where("(issue_id > 0 and issue_id not in (select id from issue)) or (release_id > 0 and release_id not in (select id from `release`))").
 		Delete(new(Attachment))
 	return err
 }

models/attachment_test.go

@@ -92,7 +92,7 @@ func TestUpdateAttachment(t *testing.T) {
 func TestGetAttachmentsByUUIDs(t *testing.T) {
 	assert.NoError(t, db.PrepareTestDatabase())
 
-	attachList, err := GetAttachmentsByUUIDs(db.DefaultContext(), []string{"a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", "not-existing-uuid"})
+	attachList, err := GetAttachmentsByUUIDs(db.DefaultContext, []string{"a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", "not-existing-uuid"})
 	assert.NoError(t, err)
 	assert.Len(t, attachList, 2)
 	assert.Equal(t, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", attachList[0].UUID)