all: unwrap database.LFSStore interface (#7692)

internal/database/database.go

@@ -124,7 +124,6 @@ func NewConnection(w logger.Writer) (*gorm.DB, error) {
 
 	// Initialize stores, sorted in alphabetical order.
 	LoginSources = &loginSourcesStore{DB: db, files: sourceFiles}
-	LFS = &lfsStore{DB: db}
 	Notices = NewNoticesStore(db)
 	Orgs = NewOrgsStore(db)
 	Perms = NewPermsStore(db)
@@ -163,3 +162,7 @@ func (db *DB) AccessTokens() *AccessTokensStore {
 func (db *DB) Actions() *ActionsStore {
 	return newActionsStore(db.db)
 }
+
+func (db *DB) LFS() *LFSStore {
+	return newLFSStore(db.db)
+}

internal/database/lfs.go

@@ -6,6 +6,7 @@ package database
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -15,20 +16,6 @@ import (
 	"gogs.io/gogs/internal/lfsutil"
 )
 
-// LFSStore is the persistent interface for LFS objects.
-type LFSStore interface {
-	// CreateObject creates a LFS object record in database.
-	CreateObject(ctx context.Context, repoID int64, oid lfsutil.OID, size int64, storage lfsutil.Storage) error
-	// GetObjectByOID returns the LFS object with given OID. It returns
-	// ErrLFSObjectNotExist when not found.
-	GetObjectByOID(ctx context.Context, repoID int64, oid lfsutil.OID) (*LFSObject, error)
-	// GetObjectsByOIDs returns LFS objects found within "oids". The returned list
-	// could have less elements if some oids were not found.
-	GetObjectsByOIDs(ctx context.Context, repoID int64, oids ...lfsutil.OID) ([]*LFSObject, error)
-}
-
-var LFS LFSStore
-
 // LFSObject is the relation between an LFS object and a repository.
 type LFSObject struct {
 	RepoID    int64           `gorm:"primaryKey;auto_increment:false"`
@@ -38,29 +25,32 @@ type LFSObject struct {
 	CreatedAt time.Time       `gorm:"not null"`
 }
 
-var _ LFSStore = (*lfsStore)(nil)
+// LFSStore is the storage layer for LFS objects.
+type LFSStore struct {
+	db *gorm.DB
+}
 
-type lfsStore struct {
-	*gorm.DB
+func newLFSStore(db *gorm.DB) *LFSStore {
+	return &LFSStore{db: db}
 }
 
-func (s *lfsStore) CreateObject(ctx context.Context, repoID int64, oid lfsutil.OID, size int64, storage lfsutil.Storage) error {
+// CreateObject creates an LFS object record in database.
+func (s *LFSStore) CreateObject(ctx context.Context, repoID int64, oid lfsutil.OID, size int64, storage lfsutil.Storage) error {
 	object := &LFSObject{
 		RepoID:  repoID,
 		OID:     oid,
 		Size:    size,
 		Storage: storage,
 	}
-	return s.WithContext(ctx).Create(object).Error
+	return s.db.WithContext(ctx).Create(object).Error
 }
 
 type ErrLFSObjectNotExist struct {
 	args errutil.Args
 }
 
 func IsErrLFSObjectNotExist(err error) bool {
-	_, ok := err.(ErrLFSObjectNotExist)
-	return ok
+	return errors.As(err, &ErrLFSObjectNotExist{})
 }
 
 func (err ErrLFSObjectNotExist) Error() string {
@@ -71,26 +61,30 @@ func (ErrLFSObjectNotExist) NotFound() bool {
 	return true
 }
 
-func (s *lfsStore) GetObjectByOID(ctx context.Context, repoID int64, oid lfsutil.OID) (*LFSObject, error) {
+// GetObjectByOID returns the LFS object with given OID. It returns
+// ErrLFSObjectNotExist when not found.
+func (s *LFSStore) GetObjectByOID(ctx context.Context, repoID int64, oid lfsutil.OID) (*LFSObject, error) {
 	object := new(LFSObject)
-	err := s.WithContext(ctx).Where("repo_id = ? AND oid = ?", repoID, oid).First(object).Error
+	err := s.db.WithContext(ctx).Where("repo_id = ? AND oid = ?", repoID, oid).First(object).Error
 	if err != nil {
-		if err == gorm.ErrRecordNotFound {
+		if errors.Is(err, gorm.ErrRecordNotFound) {
 			return nil, ErrLFSObjectNotExist{args: errutil.Args{"repoID": repoID, "oid": oid}}
 		}
 		return nil, err
 	}
 	return object, err
 }
 
-func (s *lfsStore) GetObjectsByOIDs(ctx context.Context, repoID int64, oids ...lfsutil.OID) ([]*LFSObject, error) {
+// GetObjectsByOIDs returns LFS objects found within "oids". The returned list
+// could have fewer elements if some oids were not found.
+func (s *LFSStore) GetObjectsByOIDs(ctx context.Context, repoID int64, oids ...lfsutil.OID) ([]*LFSObject, error) {
 	if len(oids) == 0 {
 		return []*LFSObject{}, nil
 	}
 
 	objects := make([]*LFSObject, 0, len(oids))
-	err := s.WithContext(ctx).Where("repo_id = ? AND oid IN (?)", repoID, oids).Find(&objects).Error
-	if err != nil && err != gorm.ErrRecordNotFound {
+	err := s.db.WithContext(ctx).Where("repo_id = ? AND oid IN (?)", repoID, oids).Find(&objects).Error
+	if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		return nil, err
 	}
 	return objects, nil

internal/database/lfs_test.go

@@ -23,77 +23,77 @@ func TestLFS(t *testing.T) {
 	t.Parallel()
 
 	ctx := context.Background()
-	db := &lfsStore{
-		DB: newTestDB(t, "lfsStore"),
+	s := &LFSStore{
+		db: newTestDB(t, "LFSStore"),
 	}
 
 	for _, tc := range []struct {
 		name string
-		test func(t *testing.T, ctx context.Context, db *lfsStore)
+		test func(t *testing.T, ctx context.Context, s *LFSStore)
 	}{
 		{"CreateObject", lfsCreateObject},
 		{"GetObjectByOID", lfsGetObjectByOID},
 		{"GetObjectsByOIDs", lfsGetObjectsByOIDs},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Cleanup(func() {
-				err := clearTables(t, db.DB)
+				err := clearTables(t, s.db)
 				require.NoError(t, err)
 			})
-			tc.test(t, ctx, db)
+			tc.test(t, ctx, s)
 		})
 		if t.Failed() {
 			break
 		}
 	}
 }
 
-func lfsCreateObject(t *testing.T, ctx context.Context, db *lfsStore) {
+func lfsCreateObject(t *testing.T, ctx context.Context, s *LFSStore) {
 	// Create first LFS object
 	repoID := int64(1)
 	oid := lfsutil.OID("ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f")
-	err := db.CreateObject(ctx, repoID, oid, 12, lfsutil.StorageLocal)
+	err := s.CreateObject(ctx, repoID, oid, 12, lfsutil.StorageLocal)
 	require.NoError(t, err)
 
 	// Get it back and check the CreatedAt field
-	object, err := db.GetObjectByOID(ctx, repoID, oid)
+	object, err := s.GetObjectByOID(ctx, repoID, oid)
 	require.NoError(t, err)
-	assert.Equal(t, db.NowFunc().Format(time.RFC3339), object.CreatedAt.UTC().Format(time.RFC3339))
+	assert.Equal(t, s.db.NowFunc().Format(time.RFC3339), object.CreatedAt.UTC().Format(time.RFC3339))
 
-	// Try create second LFS object with same oid should fail
-	err = db.CreateObject(ctx, repoID, oid, 12, lfsutil.StorageLocal)
+	// Try to create second LFS object with same oid should fail
+	err = s.CreateObject(ctx, repoID, oid, 12, lfsutil.StorageLocal)
 	assert.Error(t, err)
 }
 
-func lfsGetObjectByOID(t *testing.T, ctx context.Context, db *lfsStore) {
+func lfsGetObjectByOID(t *testing.T, ctx context.Context, s *LFSStore) {
 	// Create a LFS object
 	repoID := int64(1)
 	oid := lfsutil.OID("ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f")
-	err := db.CreateObject(ctx, repoID, oid, 12, lfsutil.StorageLocal)
+	err := s.CreateObject(ctx, repoID, oid, 12, lfsutil.StorageLocal)
 	require.NoError(t, err)
 
 	// We should be able to get it back
-	_, err = db.GetObjectByOID(ctx, repoID, oid)
+	_, err = s.GetObjectByOID(ctx, repoID, oid)
 	require.NoError(t, err)
 
 	// Try to get a non-existent object
-	_, err = db.GetObjectByOID(ctx, repoID, "bad_oid")
+	_, err = s.GetObjectByOID(ctx, repoID, "bad_oid")
 	expErr := ErrLFSObjectNotExist{args: errutil.Args{"repoID": repoID, "oid": lfsutil.OID("bad_oid")}}
 	assert.Equal(t, expErr, err)
 }
 
-func lfsGetObjectsByOIDs(t *testing.T, ctx context.Context, db *lfsStore) {
+func lfsGetObjectsByOIDs(t *testing.T, ctx context.Context, s *LFSStore) {
 	// Create two LFS objects
 	repoID := int64(1)
 	oid1 := lfsutil.OID("ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f")
 	oid2 := lfsutil.OID("ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64g")
-	err := db.CreateObject(ctx, repoID, oid1, 12, lfsutil.StorageLocal)
+	err := s.CreateObject(ctx, repoID, oid1, 12, lfsutil.StorageLocal)
 	require.NoError(t, err)
-	err = db.CreateObject(ctx, repoID, oid2, 12, lfsutil.StorageLocal)
+	err = s.CreateObject(ctx, repoID, oid2, 12, lfsutil.StorageLocal)
 	require.NoError(t, err)
 
 	// We should be able to get them back and ignore non-existent ones
-	objects, err := db.GetObjectsByOIDs(ctx, repoID, oid1, oid2, "bad_oid")
+	objects, err := s.GetObjectsByOIDs(ctx, repoID, oid1, oid2, "bad_oid")
 	require.NoError(t, err)
 	assert.Equal(t, 2, len(objects), "number of objects")
 

internal/database/mocks.go

@@ -8,14 +8,6 @@ import (
 	"testing"
 )
 
-func SetMockLFSStore(t *testing.T, mock LFSStore) {
-	before := LFS
-	LFS = mock
-	t.Cleanup(func() {
-		LFS = before
-	})
-}
-
 func setMockLoginSourcesStore(t *testing.T, mock LoginSourcesStore) {
 	before := LoginSources
 	LoginSources = mock

internal/route/lfs/basic.go

@@ -25,6 +25,7 @@ const (
 )
 
 type basicHandler struct {
+	store Store
 	// The default storage backend for uploading new objects.
 	defaultStorage lfsutil.Storage
 	// The list of available storage backends to access objects.
@@ -43,7 +44,7 @@ func (h *basicHandler) Storager(storage lfsutil.Storage) lfsutil.Storager {
 
 // GET /{owner}/{repo}.git/info/lfs/object/basic/{oid}
 func (h *basicHandler) serveDownload(c *macaron.Context, repo *database.Repository, oid lfsutil.OID) {
-	object, err := database.LFS.GetObjectByOID(c.Req.Context(), repo.ID, oid)
+	object, err := h.store.GetLFSObjectByOID(c.Req.Context(), repo.ID, oid)
 	if err != nil {
 		if database.IsErrLFSObjectNotExist(err) {
 			responseJSON(c.Resp, http.StatusNotFound, responseError{
@@ -78,7 +79,7 @@ func (h *basicHandler) serveDownload(c *macaron.Context, repo *database.Reposito
 func (h *basicHandler) serveUpload(c *macaron.Context, repo *database.Repository, oid lfsutil.OID) {
 	// NOTE: LFS client will retry upload the same object if there was a partial failure,
 	// therefore we would like to skip ones that already exist.
-	_, err := database.LFS.GetObjectByOID(c.Req.Context(), repo.ID, oid)
+	_, err := h.store.GetLFSObjectByOID(c.Req.Context(), repo.ID, oid)
 	if err == nil {
 		// Object exists, drain the request body and we're good.
 		_, _ = io.Copy(io.Discard, c.Req.Request.Body)
@@ -105,7 +106,7 @@ func (h *basicHandler) serveUpload(c *macaron.Context, repo *database.Repository
 		return
 	}
 
-	err = database.LFS.CreateObject(c.Req.Context(), repo.ID, oid, written, s.Storage())
+	err = h.store.CreateLFSObject(c.Req.Context(), repo.ID, oid, written, s.Storage())
 	if err != nil {
 		// NOTE: It is OK to leave the file when the whole operation failed
 		// with a DB error, a retry on client side can safely overwrite the
@@ -120,7 +121,7 @@ func (h *basicHandler) serveUpload(c *macaron.Context, repo *database.Repository
 }
 
 // POST /{owner}/{repo}.git/info/lfs/object/basic/verify
-func (*basicHandler) serveVerify(c *macaron.Context, repo *database.Repository) {
+func (h *basicHandler) serveVerify(c *macaron.Context, repo *database.Repository) {
 	var request basicVerifyRequest
 	defer func() { _ = c.Req.Request.Body.Close() }()
 
@@ -139,7 +140,7 @@ func (*basicHandler) serveVerify(c *macaron.Context, repo *database.Repository)
 		return
 	}
 
-	object, err := database.LFS.GetObjectByOID(c.Req.Context(), repo.ID, request.Oid)
+	object, err := h.store.GetLFSObjectByOID(c.Req.Context(), repo.ID, request.Oid)
 	if err != nil {
 		if database.IsErrLFSObjectNotExist(err) {
 			responseJSON(c.Resp, http.StatusNotFound, responseError{