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

internal/db/user.go

@@ -5,7 +5,6 @@
 package db
 
 import (
-	"context"
 	"fmt"
 	_ "image/jpeg"
 	"os"
@@ -14,8 +13,6 @@ import (
 	log "unknwon.dev/clog/v2"
 	"xorm.io/xorm"
 
-	"github.com/gogs/git-module"
-
 	"gogs.io/gogs/internal/repoutil"
 	"gogs.io/gogs/internal/userutil"
 )
@@ -200,33 +197,6 @@ func DeleteInactivateUsers() (err error) {
 	return err
 }
 
-// UserCommit represents a commit with validation of user.
-type UserCommit struct {
-	User *User
-	*git.Commit
-}
-
-// ValidateCommitsWithEmails checks if authors' e-mails of commits are corresponding to users.
-func ValidateCommitsWithEmails(oldCommits []*git.Commit) []*UserCommit {
-	emails := make(map[string]*User)
-	newCommits := make([]*UserCommit, len(oldCommits))
-	for i := range oldCommits {
-		var u *User
-		if v, ok := emails[oldCommits[i].Author.Email]; !ok {
-			u, _ = Users.GetByEmail(context.TODO(), oldCommits[i].Author.Email)
-			emails[oldCommits[i].Author.Email] = u
-		} else {
-			u = v
-		}
-
-		newCommits[i] = &UserCommit{
-			User:   u,
-			Commit: oldCommits[i],
-		}
-	}
-	return newCommits
-}
-
 // GetRepositoryAccesses finds all repositories with their access mode where a user has access but does not own.
 func (u *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
 	accesses := make([]*Access, 0, 10)

internal/db/users.go

@@ -450,27 +450,35 @@ func (db *users) GetByEmail(ctx context.Context, email string) (*User, error) {
 	}
 	email = strings.ToLower(email)
 
-	// First try to find the user by primary email
+	/*
+		Equivalent SQL for PostgreSQL:
+
+		SELECT * FROM "user"
+		LEFT JOIN email_address ON email_address.uid = "user".id
+		WHERE
+			"user".type = @userType
+		AND (
+				"user".email = @email AND "user".is_active = TRUE
+			OR  email_address.email = @email AND email_address.is_activated = TRUE
+		)
+	*/
 	user := new(User)
 	err := db.WithContext(ctx).
-		Where("email = ? AND type = ? AND is_active = ?", email, UserTypeIndividual, true).
+		Joins(dbutil.Quote("LEFT JOIN email_address ON email_address.uid = %s.id", "user"), true).
-		First(user).
+		Where(dbutil.Quote("%s.type = ?", "user"), UserTypeIndividual).
+		Where(db.
+			Where(dbutil.Quote("%[1]s.email = ? AND %[1]s.is_active = ?", "user"), email, true).
+			Or("email_address.email = ? AND email_address.is_activated = ?", email, true),
+		).
+		First(&user).
 		Error
-	if err == nil {
-		return user, nil
-	} else if err != gorm.ErrRecordNotFound {
-		return nil, err
-	}
-
-	// Otherwise, check activated email addresses
-	emailAddress, err := NewEmailAddressesStore(db.DB).GetByEmail(ctx, email, true)
 	if err != nil {
-		if IsErrEmailAddressNotExist(err) {
+		if err == gorm.ErrRecordNotFound {
 			return nil, ErrUserNotExist{args: errutil.Args{"email": email}}
 		}
 		return nil, err
 	}
-	return db.GetByID(ctx, emailAddress.UserID)
+	return user, nil
 }
 
 func (db *users) GetByID(ctx context.Context, id int64) (*User, error) {

internal/db/users_test.go

@@ -593,7 +593,7 @@ func usersGetMailableEmailsByUsernames(t *testing.T, db *users) {
 	_, err = db.Create(ctx, "cindy", "cindy@exmaple.com", CreateUserOptions{Activated: true})
 	require.NoError(t, err)
 
-	got, err := db.GetMailableEmailsByUsernames(ctx, []string{alice.Name, bob.Name, "404"})
+	got, err := db.GetMailableEmailsByUsernames(ctx, []string{alice.Name, bob.Name, "ignore-non-exist"})
 	require.NoError(t, err)
 	want := []string{bob.Email}
 	assert.Equal(t, want, got)

internal/route/repo/commit.go

@@ -61,7 +61,7 @@ func renderCommits(c *context.Context, filename string) {
 	}
 
 	commits = RenderIssueLinks(commits, c.Repo.RepoLink)
-	c.Data["Commits"] = db.ValidateCommitsWithEmails(commits)
+	c.Data["Commits"] = matchUsersWithCommitEmails(c.Req.Context(), commits)
 
 	if page > 1 {
 		c.Data["HasPrevious"] = true
@@ -98,7 +98,7 @@ func SearchCommits(c *context.Context) {
 	}
 
 	commits = RenderIssueLinks(commits, c.Repo.RepoLink)
-	c.Data["Commits"] = db.ValidateCommitsWithEmails(commits)
+	c.Data["Commits"] = matchUsersWithCommitEmails(c.Req.Context(), commits)
 
 	c.Data["Keyword"] = keyword
 	c.Data["Username"] = c.Repo.Owner.Name
@@ -188,6 +188,31 @@ func RawDiff(c *context.Context) {
 	}
 }
 
+type userCommit struct {
+	User *db.User
+	*git.Commit
+}
+
+// matchUsersWithCommitEmails matches existing users using commit author emails.
+func matchUsersWithCommitEmails(ctx gocontext.Context, oldCommits []*git.Commit) []*userCommit {
+	emailToUsers := make(map[string]*db.User)
+	newCommits := make([]*userCommit, len(oldCommits))
+	for i := range oldCommits {
+		var u *db.User
+		if v, ok := emailToUsers[oldCommits[i].Author.Email]; !ok {
+			emailToUsers[oldCommits[i].Author.Email], _ = db.Users.GetByEmail(ctx, oldCommits[i].Author.Email)
+		} else {
+			u = v
+		}
+
+		newCommits[i] = &userCommit{
+			User:   u,
+			Commit: oldCommits[i],
+		}
+	}
+	return newCommits
+}
+
 func CompareDiff(c *context.Context) {
 	c.Data["IsDiffCompare"] = true
 	userName := c.Repo.Owner.Name
@@ -218,7 +243,7 @@ func CompareDiff(c *context.Context) {
 
 	c.Data["IsSplitStyle"] = c.Query("style") == "split"
 	c.Data["CommitRepoLink"] = c.Repo.RepoLink
-	c.Data["Commits"] = db.ValidateCommitsWithEmails(commits)
+	c.Data["Commits"] = matchUsersWithCommitEmails(c.Req.Context(), commits)
 	c.Data["CommitsCount"] = len(commits)
 	c.Data["BeforeCommitID"] = beforeCommitID
 	c.Data["AfterCommitID"] = afterCommitID

internal/route/repo/pull.go

@@ -298,7 +298,7 @@ func ViewPullCommits(c *context.Context) {
 		commits = prInfo.Commits
 	}
 
-	c.Data["Commits"] = db.ValidateCommitsWithEmails(commits)
+	c.Data["Commits"] = matchUsersWithCommitEmails(c.Req.Context(), commits)
 	c.Data["CommitsCount"] = len(commits)
 
 	c.Success(PULL_COMMITS)
@@ -606,7 +606,7 @@ func PrepareCompareDiff(
 		return false
 	}
 
-	c.Data["Commits"] = db.ValidateCommitsWithEmails(meta.Commits)
+	c.Data["Commits"] = matchUsersWithCommitEmails(c.Req.Context(), meta.Commits)
 	c.Data["CommitCount"] = len(meta.Commits)
 	c.Data["Username"] = headUser.Name
 	c.Data["Reponame"] = headRepo.Name