refactor(db): migrate methods off `user.go` (#7329)

8 files changed, 202 insertions, 65 deletions

diff --git a/internal/cmd/serv.go b/internal/cmd/serv.go
index 518a52c0..f4ef7020 100644
--- a/internal/cmd/serv.go
+++ b/internal/cmd/serv.go
@@ -133,6 +133,7 @@ var allowedCommands = map[string]db.AccessMode{
 }
 
 func runServ(c *cli.Context) error {
+	ctx := context.Background()
 	setup(c, "serv.log", true)
 
 	if conf.SSH.Disabled {
@@ -161,7 +162,7 @@ func runServ(c *cli.Context) error {
 	repoName := strings.TrimSuffix(strings.ToLower(repoFields[1]), ".git")
 	repoName = strings.TrimSuffix(repoName, ".wiki")
 
-	owner, err := db.Users.GetByUsername(context.Background(), ownerName)
+	owner, err := db.Users.GetByUsername(ctx, ownerName)
 	if err != nil {
 		if db.IsErrUserNotExist(err) {
 			fail("Repository owner does not exist", "Unregistered owner: %s", ownerName)
@@ -204,12 +205,12 @@ func runServ(c *cli.Context) error {
 			}
 			checkDeployKey(key, repo)
 		} else {
-			user, err = db.GetUserByKeyID(key.ID)
+			user, err = db.Users.GetByKeyID(ctx, key.ID)
 			if err != nil {
 				fail("Internal error", "Failed to get user by key ID '%d': %v", key.ID, err)
 			}
 
-			mode := db.Perms.AccessMode(context.Background(), user.ID, repo.ID,
+			mode := db.Perms.AccessMode(ctx, user.ID, repo.ID,
 				db.AccessModeOptions{
 					OwnerID: repo.OwnerID,
 					Private: repo.IsPrivate,
diff --git a/internal/db/errors/user.go b/internal/db/errors/user.go
deleted file mode 100644
index 09572af9..00000000
--- a/internal/db/errors/user.go
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright 2017 The Gogs Authors. All rights reserved.
-// Use of this source code is governed by a MIT-style
-// license that can be found in the LICENSE file.
-
-package errors
-
-import (
-	"fmt"
-)
-
-type UserNotKeyOwner struct {
-	KeyID int64
-}
-
-func IsUserNotKeyOwner(err error) bool {
-	_, ok := err.(UserNotKeyOwner)
-	return ok
-}
-
-func (err UserNotKeyOwner) Error() string {
-	return fmt.Sprintf("user is not the owner of public key [key_id: %d]", err.KeyID)
-}
diff --git a/internal/db/repo.go b/internal/db/repo.go
index 896c39b8..4d1c3e82 100644
--- a/internal/db/repo.go
+++ b/internal/db/repo.go
@@ -524,7 +524,20 @@ func (repo *Repository) GetAssignees() (_ []*User, err error) {
 
 // GetAssigneeByID returns the user that has write access of repository by given ID.
 func (repo *Repository) GetAssigneeByID(userID int64) (*User, error) {
-	return GetAssigneeByID(repo, userID)
+	ctx := context.TODO()
+	if !Perms.Authorize(
+		ctx,
+		userID,
+		repo.ID,
+		AccessModeRead,
+		AccessModeOptions{
+			OwnerID: repo.OwnerID,
+			Private: repo.IsPrivate,
+		},
+	) {
+		return nil, ErrUserNotExist{args: errutil.Args{"userID": userID}}
+	}
+	return Users.GetByID(ctx, userID)
 }
 
 // GetWriters returns all users that have write access to the repository.
diff --git a/internal/db/ssh_key.go b/internal/db/ssh_key.go
index d99a0dc6..4bf5b73e 100644
--- a/internal/db/ssh_key.go
+++ b/internal/db/ssh_key.go
@@ -44,20 +44,20 @@ const (
 
 // PublicKey represents a user or deploy SSH public key.
 type PublicKey struct {
-	ID          int64
-	OwnerID     int64      `xorm:"INDEX NOT NULL"`
-	Name        string     `xorm:"NOT NULL"`
-	Fingerprint string     `xorm:"NOT NULL"`
-	Content     string     `xorm:"TEXT NOT NULL"`
-	Mode        AccessMode `xorm:"NOT NULL DEFAULT 2"`
-	Type        KeyType    `xorm:"NOT NULL DEFAULT 1"`
-
-	Created           time.Time `xorm:"-" json:"-"`
+	ID          int64      `gorm:"primaryKey"`
+	OwnerID     int64      `xorm:"INDEX NOT NULL" gorm:"index;not null"`
+	Name        string     `xorm:"NOT NULL" gorm:"not null"`
+	Fingerprint string     `xorm:"NOT NULL" gorm:"not null"`
+	Content     string     `xorm:"TEXT NOT NULL" gorm:"type:TEXT;not null"`
+	Mode        AccessMode `xorm:"NOT NULL DEFAULT 2" gorm:"not null;default:2"`
+	Type        KeyType    `xorm:"NOT NULL DEFAULT 1" gorm:"not null;default:1"`
+
+	Created           time.Time `xorm:"-" json:"-" gorm:"-"`
 	CreatedUnix       int64
-	Updated           time.Time `xorm:"-" json:"-"` // Note: Updated must below Created for AfterSet.
+	Updated           time.Time `xorm:"-" json:"-" gorm:"-"` // Note: Updated must below Created for AfterSet.
 	UpdatedUnix       int64
-	HasRecentActivity bool `xorm:"-" json:"-"`
-	HasUsed           bool `xorm:"-" json:"-"`
+	HasRecentActivity bool `xorm:"-" json:"-" gorm:"-"`
+	HasUsed           bool `xorm:"-" json:"-" gorm:"-"`
 }
 
 func (k *PublicKey) BeforeInsert() {
diff --git a/internal/db/user.go b/internal/db/user.go
index c2091e6f..50b59630 100644
--- a/internal/db/user.go
+++ b/internal/db/user.go
@@ -18,7 +18,6 @@ import (
 	"github.com/gogs/git-module"
 
 	"gogs.io/gogs/internal/conf"
-	"gogs.io/gogs/internal/db/errors"
 	"gogs.io/gogs/internal/repoutil"
 	"gogs.io/gogs/internal/userutil"
 )
@@ -203,17 +202,6 @@ func DeleteInactivateUsers() (err error) {
 	return err
 }
 
-func GetUserByKeyID(keyID int64) (*User, error) {
-	user := new(User)
-	has, err := x.SQL("SELECT a.* FROM `user` AS a, public_key AS b WHERE a.id = b.owner_id AND b.id=?", keyID).Get(user)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, errors.UserNotKeyOwner{KeyID: keyID}
-	}
-	return user, nil
-}
-
 func getUserByID(e Engine, id int64) (*User, error) {
 	u := new(User)
 	has, err := e.ID(id).Get(u)
@@ -225,20 +213,6 @@ func getUserByID(e Engine, id int64) (*User, error) {
 	return u, nil
 }
 
-// GetAssigneeByID returns the user with read access of repository by given ID.
-func GetAssigneeByID(repo *Repository, userID int64) (*User, error) {
-	ctx := context.TODO()
-	if !Perms.Authorize(ctx, userID, repo.ID, AccessModeRead,
-		AccessModeOptions{
-			OwnerID: repo.OwnerID,
-			Private: repo.IsPrivate,
-		},
-	) {
-		return nil, ErrUserNotExist{args: map[string]interface{}{"userID": userID}}
-	}
-	return Users.GetByID(ctx, userID)
-}
-
 // GetUserEmailsByNames returns a list of e-mails corresponds to names.
 func GetUserEmailsByNames(names []string) []string {
 	mails := make([]string, 0, len(names))
diff --git a/internal/db/users.go b/internal/db/users.go
index 65326a95..80b124ea 100644
--- a/internal/db/users.go
+++ b/internal/db/users.go
@@ -70,6 +70,9 @@ type UsersStore interface {
 	// GetByUsername returns the user with given username. It returns
 	// ErrUserNotExist when not found.
 	GetByUsername(ctx context.Context, username string) (*User, error)
+	// GetByKeyID returns the owner of given public key ID. It returns
+	// ErrUserNotExist when not found.
+	GetByKeyID(ctx context.Context, keyID int64) (*User, error)
 	// HasForkedRepository returns true if the user has forked given repository.
 	HasForkedRepository(ctx context.Context, userID, repoID int64) bool
 	// IsUsernameUsed returns true if the given username has been used other than
@@ -484,6 +487,22 @@ func (db *users) GetByUsername(ctx context.Context, username string) (*User, err
 	return user, nil
 }
 
+func (db *users) GetByKeyID(ctx context.Context, keyID int64) (*User, error) {
+	user := new(User)
+	err := db.WithContext(ctx).
+		Joins(dbutil.Quote("JOIN public_key ON public_key.owner_id = %s.id", "user")).
+		Where("public_key.id = ?", keyID).
+		First(user).
+		Error
+	if err != nil {
+		if err == gorm.ErrRecordNotFound {
+			return nil, ErrUserNotExist{args: errutil.Args{"keyID": keyID}}
+		}
+		return nil, err
+	}
+	return user, nil
+}
+
 func (db *users) HasForkedRepository(ctx context.Context, userID, repoID int64) bool {
 	var count int64
 	db.WithContext(ctx).Model(new(Repository)).Where("owner_id = ? AND fork_id = ?", userID, repoID).Count(&count)
diff --git a/internal/db/users_test.go b/internal/db/users_test.go
index 4cf5aaf4..6a63c5aa 100644
--- a/internal/db/users_test.go
+++ b/internal/db/users_test.go
@@ -82,7 +82,7 @@ func TestUsers(t *testing.T) {
 	}
 	t.Parallel()
 
-	tables := []interface{}{new(User), new(EmailAddress), new(Repository), new(Follow), new(PullRequest)}
+	tables := []interface{}{new(User), new(EmailAddress), new(Repository), new(Follow), new(PullRequest), new(PublicKey)}
 	db := &users{
 		DB: dbtest.NewDB(t, "users", tables...),
 	}
@@ -99,6 +99,7 @@ func TestUsers(t *testing.T) {
 		{"GetByEmail", usersGetByEmail},
 		{"GetByID", usersGetByID},
 		{"GetByUsername", usersGetByUsername},
+		{"GetByKeyID", usersGetByKeyID},
 		{"HasForkedRepository", usersHasForkedRepository},
 		{"IsUsernameUsed", usersIsUsernameUsed},
 		{"List", usersList},
@@ -553,6 +554,33 @@ func usersGetByUsername(t *testing.T, db *users) {
 	assert.Equal(t, wantErr, err)
 }
 
+func usersGetByKeyID(t *testing.T, db *users) {
+	ctx := context.Background()
+
+	alice, err := db.Create(ctx, "alice", "alice@exmaple.com", CreateUserOptions{})
+	require.NoError(t, err)
+
+	// TODO: Use PublicKeys.Create to replace SQL hack when the method is available.
+	publicKey := &PublicKey{
+		OwnerID:     alice.ID,
+		Name:        "test-key",
+		Fingerprint: "12:f8:7e:78:61:b4:bf:e2:de:24:15:96:4e:d4:72:53",
+		Content:     "test-key-content",
+		CreatedUnix: db.NowFunc().Unix(),
+		UpdatedUnix: db.NowFunc().Unix(),
+	}
+	err = db.WithContext(ctx).Create(publicKey).Error
+	require.NoError(t, err)
+
+	user, err := db.GetByKeyID(ctx, publicKey.ID)
+	require.NoError(t, err)
+	assert.Equal(t, alice.Name, user.Name)
+
+	_, err = db.GetByKeyID(ctx, publicKey.ID+1)
+	wantErr := ErrUserNotExist{args: errutil.Args{"keyID": publicKey.ID + 1}}
+	assert.Equal(t, wantErr, err)
+}
+
 func usersHasForkedRepository(t *testing.T, db *users) {
 	ctx := context.Background()
 
diff --git a/internal/route/lfs/mocks_test.go b/internal/route/lfs/mocks_test.go
index 07dfbecb..5a82bb74 100644
--- a/internal/route/lfs/mocks_test.go
+++ b/internal/route/lfs/mocks_test.go
@@ -2313,6 +2313,9 @@ type MockUsersStore struct {
 	// GetByIDFunc is an instance of a mock function object controlling the
 	// behavior of the method GetByID.
 	GetByIDFunc *UsersStoreGetByIDFunc
+	// GetByKeyIDFunc is an instance of a mock function object controlling
+	// the behavior of the method GetByKeyID.
+	GetByKeyIDFunc *UsersStoreGetByKeyIDFunc
 	// GetByUsernameFunc is an instance of a mock function object
 	// controlling the behavior of the method GetByUsername.
 	GetByUsernameFunc *UsersStoreGetByUsernameFunc
@@ -2378,6 +2381,11 @@ func NewMockUsersStore() *MockUsersStore {
 				return
 			},
 		},
+		GetByKeyIDFunc: &UsersStoreGetByKeyIDFunc{
+			defaultHook: func(context.Context, int64) (r0 *db.User, r1 error) {
+				return
+			},
+		},
 		GetByUsernameFunc: &UsersStoreGetByUsernameFunc{
 			defaultHook: func(context.Context, string) (r0 *db.User, r1 error) {
 				return
@@ -2460,6 +2468,11 @@ func NewStrictMockUsersStore() *MockUsersStore {
 				panic("unexpected invocation of MockUsersStore.GetByID")
 			},
 		},
+		GetByKeyIDFunc: &UsersStoreGetByKeyIDFunc{
+			defaultHook: func(context.Context, int64) (*db.User, error) {
+				panic("unexpected invocation of MockUsersStore.GetByKeyID")
+			},
+		},
 		GetByUsernameFunc: &UsersStoreGetByUsernameFunc{
 			defaultHook: func(context.Context, string) (*db.User, error) {
 				panic("unexpected invocation of MockUsersStore.GetByUsername")
@@ -2528,6 +2541,9 @@ func NewMockUsersStoreFrom(i db.UsersStore) *MockUsersStore {
 		GetByIDFunc: &UsersStoreGetByIDFunc{
 			defaultHook: i.GetByID,
 		},
+		GetByKeyIDFunc: &UsersStoreGetByKeyIDFunc{
+			defaultHook: i.GetByKeyID,
+		},
 		GetByUsernameFunc: &UsersStoreGetByUsernameFunc{
 			defaultHook: i.GetByUsername,
 		},
@@ -3313,6 +3329,114 @@ func (c UsersStoreGetByIDFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0, c.Result1}
 }
 
+// UsersStoreGetByKeyIDFunc describes the behavior when the GetByKeyID
+// method of the parent MockUsersStore instance is invoked.
+type UsersStoreGetByKeyIDFunc struct {
+	defaultHook func(context.Context, int64) (*db.User, error)
+	hooks       []func(context.Context, int64) (*db.User, error)
+	history     []UsersStoreGetByKeyIDFuncCall
+	mutex       sync.Mutex
+}
+
+// GetByKeyID delegates to the next hook function in the queue and stores
+// the parameter and result values of this invocation.
+func (m *MockUsersStore) GetByKeyID(v0 context.Context, v1 int64) (*db.User, error) {
+	r0, r1 := m.GetByKeyIDFunc.nextHook()(v0, v1)
+	m.GetByKeyIDFunc.appendCall(UsersStoreGetByKeyIDFuncCall{v0, v1, r0, r1})
+	return r0, r1
+}
+
+// SetDefaultHook sets function that is called when the GetByKeyID method of
+// the parent MockUsersStore instance is invoked and the hook queue is
+// empty.
+func (f *UsersStoreGetByKeyIDFunc) SetDefaultHook(hook func(context.Context, int64) (*db.User, error)) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// GetByKeyID method of the parent MockUsersStore instance invokes the hook
+// at the front of the queue and discards it. After the queue is empty, the
+// default hook function is invoked for any future action.
+func (f *UsersStoreGetByKeyIDFunc) PushHook(hook func(context.Context, int64) (*db.User, error)) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *UsersStoreGetByKeyIDFunc) SetDefaultReturn(r0 *db.User, r1 error) {
+	f.SetDefaultHook(func(context.Context, int64) (*db.User, error) {
+		return r0, r1
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *UsersStoreGetByKeyIDFunc) PushReturn(r0 *db.User, r1 error) {
+	f.PushHook(func(context.Context, int64) (*db.User, error) {
+		return r0, r1
+	})
+}
+
+func (f *UsersStoreGetByKeyIDFunc) nextHook() func(context.Context, int64) (*db.User, error) {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *UsersStoreGetByKeyIDFunc) appendCall(r0 UsersStoreGetByKeyIDFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of UsersStoreGetByKeyIDFuncCall objects
+// describing the invocations of this function.
+func (f *UsersStoreGetByKeyIDFunc) History() []UsersStoreGetByKeyIDFuncCall {
+	f.mutex.Lock()
+	history := make([]UsersStoreGetByKeyIDFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// UsersStoreGetByKeyIDFuncCall is an object that describes an invocation of
+// method GetByKeyID on an instance of MockUsersStore.
+type UsersStoreGetByKeyIDFuncCall struct {
+	// Arg0 is the value of the 1st argument passed to this method
+	// invocation.
+	Arg0 context.Context
+	// Arg1 is the value of the 2nd argument passed to this method
+	// invocation.
+	Arg1 int64
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 *db.User
+	// Result1 is the value of the 2nd result returned from this method
+	// invocation.
+	Result1 error
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c UsersStoreGetByKeyIDFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0, c.Arg1}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c UsersStoreGetByKeyIDFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0, c.Result1}
+}
+
 // UsersStoreGetByUsernameFunc describes the behavior when the GetByUsername
 // method of the parent MockUsersStore instance is invoked.
 type UsersStoreGetByUsernameFunc struct {