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

internal/db/issue_mail.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/pkg/errors"
 	"github.com/unknwon/com"
 	log "unknwon.dev/clog/v2"
 
@@ -99,6 +100,8 @@ func NewMailerIssue(issue *Issue) email.Issue {
 // 1. Repository watchers, users who participated in comments and the assignee.
 // 2. Users who are not in 1. but get mentioned in current issue/comment.
 func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string) error {
+	ctx := context.TODO()
+
 	if !conf.User.EnableEmailNotification {
 		return nil
 	}
@@ -125,7 +128,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string)
 			continue
 		}
 
-		to, err := Users.GetByID(context.TODO(), watchers[i].UserID)
+		to, err := Users.GetByID(ctx, watchers[i].UserID)
 		if err != nil {
 			return fmt.Errorf("GetUserByID [%d]: %v", watchers[i].UserID, err)
 		}
@@ -156,15 +159,20 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string)
 
 	// Mail mentioned people and exclude watchers.
 	names = append(names, doer.Name)
-	tos = make([]string, 0, len(mentions)) // list of user names.
+	toUsernames := make([]string, 0, len(mentions)) // list of user names.
 	for i := range mentions {
 		if com.IsSliceContainsStr(names, mentions[i]) {
 			continue
 		}
 
-		tos = append(tos, mentions[i])
+		toUsernames = append(toUsernames, mentions[i])
+	}
+
+	tos, err = Users.GetMailableEmailsByUsernames(ctx, toUsernames)
+	if err != nil {
+		return errors.Wrap(err, "get mailable emails by usernames")
 	}
-	email.SendIssueMentionMail(NewMailerIssue(issue), NewMailerRepo(issue.Repo), NewMailerUser(doer), GetUserEmailsByNames(tos))
+	email.SendIssueMentionMail(NewMailerIssue(issue), NewMailerRepo(issue.Repo), NewMailerUser(doer), tos)
 	return nil
 }
 

internal/db/user.go

@@ -200,36 +200,12 @@ func DeleteInactivateUsers() (err error) {
 	return err
 }
 
-// GetUserEmailsByNames returns a list of e-mails corresponds to names.
-func GetUserEmailsByNames(names []string) []string {
-	mails := make([]string, 0, len(names))
-	for _, name := range names {
-		u, err := Users.GetByUsername(context.TODO(), name)
-		if err != nil {
-			continue
-		}
-		if u.IsMailable() {
-			mails = append(mails, u.Email)
-		}
-	}
-	return mails
-}
-
 // UserCommit represents a commit with validation of user.
 type UserCommit struct {
 	User *User
 	*git.Commit
 }
 
-// ValidateCommitWithEmail checks if author's e-mail of commit is corresponding to a user.
-func ValidateCommitWithEmail(c *git.Commit) *User {
-	u, err := Users.GetByEmail(context.TODO(), c.Author.Email)
-	if err != nil {
-		return nil
-	}
-	return u
-}
-
 // ValidateCommitsWithEmails checks if authors' e-mails of commits are corresponding to users.
 func ValidateCommitsWithEmails(oldCommits []*git.Commit) []*UserCommit {
 	emails := make(map[string]*User)

internal/db/users.go

@@ -73,6 +73,10 @@ type UsersStore interface {
 	// GetByKeyID returns the owner of given public key ID. It returns
 	// ErrUserNotExist when not found.
 	GetByKeyID(ctx context.Context, keyID int64) (*User, error)
+	// GetMailableEmailsByUsernames returns a list of verified primary email
+	// addresses (where email notifications are sent to) of users with given list of
+	// usernames. Non-existing usernames are ignored.
+	GetMailableEmailsByUsernames(ctx context.Context, usernames []string) ([]string, 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
@@ -509,6 +513,15 @@ func (db *users) GetByKeyID(ctx context.Context, keyID int64) (*User, error) {
 	return user, nil
 }
 
+func (db *users) GetMailableEmailsByUsernames(ctx context.Context, usernames []string) ([]string, error) {
+	emails := make([]string, 0, len(usernames))
+	return emails, db.WithContext(ctx).
+		Model(&User{}).
+		Select("email").
+		Where("lower_name IN (?) AND is_active = ?", usernames, true).
+		Find(&emails).Error
+}
+
 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)
@@ -816,11 +829,6 @@ func (u *User) IsOrganization() bool {
 	return u.Type == UserTypeOrganization
 }
 
-// IsMailable returns true if the user is eligible to receive emails.
-func (u *User) IsMailable() bool {
-	return u.IsActive
-}
-
 // APIFormat returns the API format of a user.
 func (u *User) APIFormat() *api.User {
 	return &api.User{

internal/db/users_test.go

@@ -100,6 +100,7 @@ func TestUsers(t *testing.T) {
 		{"GetByID", usersGetByID},
 		{"GetByUsername", usersGetByUsername},
 		{"GetByKeyID", usersGetByKeyID},
+		{"GetMailableEmailsByUsernames", usersGetMailableEmailsByUsernames},
 		{"HasForkedRepository", usersHasForkedRepository},
 		{"IsUsernameUsed", usersIsUsernameUsed},
 		{"List", usersList},
@@ -582,6 +583,22 @@ func usersGetByKeyID(t *testing.T, db *users) {
 	assert.Equal(t, wantErr, err)
 }
 
+func usersGetMailableEmailsByUsernames(t *testing.T, db *users) {
+	ctx := context.Background()
+
+	alice, err := db.Create(ctx, "alice", "[email protected]", CreateUserOptions{})
+	require.NoError(t, err)
+	bob, err := db.Create(ctx, "bob", "[email protected]", CreateUserOptions{Activated: true})
+	require.NoError(t, err)
+	_, err = db.Create(ctx, "cindy", "[email protected]", CreateUserOptions{Activated: true})
+	require.NoError(t, err)
+
+	got, err := db.GetMailableEmailsByUsernames(ctx, []string{alice.Name, bob.Name, "404"})
+	require.NoError(t, err)
+	want := []string{bob.Email}
+	assert.Equal(t, want, got)
+}
+
 func usersHasForkedRepository(t *testing.T, db *users) {
 	ctx := context.Background()
 

internal/route/lfs/mocks_test.go

@@ -2319,6 +2319,10 @@ type MockUsersStore struct {
 	// GetByUsernameFunc is an instance of a mock function object
 	// controlling the behavior of the method GetByUsername.
 	GetByUsernameFunc *UsersStoreGetByUsernameFunc
+	// GetMailableEmailsByUsernamesFunc is an instance of a mock function
+	// object controlling the behavior of the method
+	// GetMailableEmailsByUsernames.
+	GetMailableEmailsByUsernamesFunc *UsersStoreGetMailableEmailsByUsernamesFunc
 	// HasForkedRepositoryFunc is an instance of a mock function object
 	// controlling the behavior of the method HasForkedRepository.
 	HasForkedRepositoryFunc *UsersStoreHasForkedRepositoryFunc
@@ -2394,6 +2398,11 @@ func NewMockUsersStore() *MockUsersStore {
 				return
 			},
 		},
+		GetMailableEmailsByUsernamesFunc: &UsersStoreGetMailableEmailsByUsernamesFunc{
+			defaultHook: func(context.Context, []string) (r0 []string, r1 error) {
+				return
+			},
+		},
 		HasForkedRepositoryFunc: &UsersStoreHasForkedRepositoryFunc{
 			defaultHook: func(context.Context, int64, int64) (r0 bool) {
 				return
@@ -2486,6 +2495,11 @@ func NewStrictMockUsersStore() *MockUsersStore {
 				panic("unexpected invocation of MockUsersStore.GetByUsername")
 			},
 		},
+		GetMailableEmailsByUsernamesFunc: &UsersStoreGetMailableEmailsByUsernamesFunc{
+			defaultHook: func(context.Context, []string) ([]string, error) {
+				panic("unexpected invocation of MockUsersStore.GetMailableEmailsByUsernames")
+			},
+		},
 		HasForkedRepositoryFunc: &UsersStoreHasForkedRepositoryFunc{
 			defaultHook: func(context.Context, int64, int64) bool {
 				panic("unexpected invocation of MockUsersStore.HasForkedRepository")
@@ -2560,6 +2574,9 @@ func NewMockUsersStoreFrom(i db.UsersStore) *MockUsersStore {
 		GetByUsernameFunc: &UsersStoreGetByUsernameFunc{
 			defaultHook: i.GetByUsername,
 		},
+		GetMailableEmailsByUsernamesFunc: &UsersStoreGetMailableEmailsByUsernamesFunc{
+			defaultHook: i.GetMailableEmailsByUsernames,
+		},
 		HasForkedRepositoryFunc: &UsersStoreHasForkedRepositoryFunc{
 			defaultHook: i.HasForkedRepository,
 		},
@@ -3561,6 +3578,118 @@ func (c UsersStoreGetByUsernameFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0, c.Result1}
 }
 
+// UsersStoreGetMailableEmailsByUsernamesFunc describes the behavior when
+// the GetMailableEmailsByUsernames method of the parent MockUsersStore
+// instance is invoked.
+type UsersStoreGetMailableEmailsByUsernamesFunc struct {
+	defaultHook func(context.Context, []string) ([]string, error)
+	hooks       []func(context.Context, []string) ([]string, error)
+	history     []UsersStoreGetMailableEmailsByUsernamesFuncCall
+	mutex       sync.Mutex
+}
+
+// GetMailableEmailsByUsernames delegates to the next hook function in the
+// queue and stores the parameter and result values of this invocation.
+func (m *MockUsersStore) GetMailableEmailsByUsernames(v0 context.Context, v1 []string) ([]string, error) {
+	r0, r1 := m.GetMailableEmailsByUsernamesFunc.nextHook()(v0, v1)
+	m.GetMailableEmailsByUsernamesFunc.appendCall(UsersStoreGetMailableEmailsByUsernamesFuncCall{v0, v1, r0, r1})
+	return r0, r1
+}
+
+// SetDefaultHook sets function that is called when the
+// GetMailableEmailsByUsernames method of the parent MockUsersStore instance
+// is invoked and the hook queue is empty.
+func (f *UsersStoreGetMailableEmailsByUsernamesFunc) SetDefaultHook(hook func(context.Context, []string) ([]string, error)) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// GetMailableEmailsByUsernames 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 *UsersStoreGetMailableEmailsByUsernamesFunc) PushHook(hook func(context.Context, []string) ([]string, 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 *UsersStoreGetMailableEmailsByUsernamesFunc) SetDefaultReturn(r0 []string, r1 error) {
+	f.SetDefaultHook(func(context.Context, []string) ([]string, error) {
+		return r0, r1
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *UsersStoreGetMailableEmailsByUsernamesFunc) PushReturn(r0 []string, r1 error) {
+	f.PushHook(func(context.Context, []string) ([]string, error) {
+		return r0, r1
+	})
+}
+
+func (f *UsersStoreGetMailableEmailsByUsernamesFunc) nextHook() func(context.Context, []string) ([]string, 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 *UsersStoreGetMailableEmailsByUsernamesFunc) appendCall(r0 UsersStoreGetMailableEmailsByUsernamesFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of
+// UsersStoreGetMailableEmailsByUsernamesFuncCall objects describing the
+// invocations of this function.
+func (f *UsersStoreGetMailableEmailsByUsernamesFunc) History() []UsersStoreGetMailableEmailsByUsernamesFuncCall {
+	f.mutex.Lock()
+	history := make([]UsersStoreGetMailableEmailsByUsernamesFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// UsersStoreGetMailableEmailsByUsernamesFuncCall is an object that
+// describes an invocation of method GetMailableEmailsByUsernames on an
+// instance of MockUsersStore.
+type UsersStoreGetMailableEmailsByUsernamesFuncCall 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 []string
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 []string
+	// 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 UsersStoreGetMailableEmailsByUsernamesFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0, c.Arg1}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c UsersStoreGetMailableEmailsByUsernamesFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0, c.Result1}
+}
+
 // UsersStoreHasForkedRepositoryFunc describes the behavior when the
 // HasForkedRepository method of the parent MockUsersStore instance is
 // invoked.

internal/route/repo/commit.go

@@ -5,6 +5,7 @@
 package repo
 
 import (
+	gocontext "context"
 	"path"
 	"time"
 
@@ -110,6 +111,13 @@ func FileHistory(c *context.Context) {
 	renderCommits(c, c.Repo.TreePath)
 }
 
+// tryGetUserByEmail returns a non-nil value if the email is corresponding to an
+// existing user.
+func tryGetUserByEmail(ctx gocontext.Context, email string) *db.User {
+	user, _ := db.Users.GetByEmail(ctx, email)
+	return user
+}
+
 func Diff(c *context.Context) {
 	c.PageIs("Diff")
 	c.RequireHighlightJS()
@@ -156,7 +164,7 @@ func Diff(c *context.Context) {
 	c.Data["IsImageFile"] = commit.IsImageFile
 	c.Data["IsImageFileByIndex"] = commit.IsImageFileByIndex
 	c.Data["Commit"] = commit
-	c.Data["Author"] = db.ValidateCommitWithEmail(commit)
+	c.Data["Author"] = tryGetUserByEmail(c.Req.Context(), commit.Author.Email)
 	c.Data["Diff"] = diff
 	c.Data["Parents"] = parents
 	c.Data["DiffNotAvailable"] = diff.NumFiles() == 0

internal/route/repo/view.go

@@ -111,7 +111,7 @@ func renderDirectory(c *context.Context, treeLink string) {
 		}
 	}
 	c.Data["LatestCommit"] = latestCommit
-	c.Data["LatestCommitUser"] = db.ValidateCommitWithEmail(latestCommit)
+	c.Data["LatestCommitUser"] = tryGetUserByEmail(c.Req.Context(), latestCommit.Author.Email)
 
 	if c.Repo.CanEnableEditor() {
 		c.Data["CanAddFile"] = true