Improve commits list performance to reduce unnecessary database queries

models/asymkey/gpg_key_commit_verification.go

@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/container"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -71,21 +72,41 @@ const (
 )
 
 // ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys.
-func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error)) []*SignCommit {
+func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error)) ([]*SignCommit, error) {
 	newCommits := make([]*SignCommit, 0, len(oldCommits))
 	keyMap := map[string]bool{}
 
+	emails := make(container.Set[string])
 	for _, c := range oldCommits {
+		if c.Committer != nil {
+			emails.Add(c.Committer.Email)
+		}
+	}
+
+	emailUsers, err := user_model.GetUsersByEmails(ctx, emails.Values())
+	if err != nil {
+		return nil, err
+	}
+
+	for _, c := range oldCommits {
+		committer, ok := emailUsers[c.Committer.Email]
+		if !ok && c.Committer != nil {
+			committer = &user_model.User{
+				Name:  c.Committer.Name,
+				Email: c.Committer.Email,
+			}
+		}
+
 		signCommit := &SignCommit{
 			UserCommit:   c,
-			Verification: ParseCommitWithSignature(ctx, c.Commit),
+			Verification: parseCommitWithSignatureCommitter(ctx, c.Commit, committer),
 		}
 
 		_ = CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap)
 
 		newCommits = append(newCommits, signCommit)
 	}
-	return newCommits
+	return newCommits, nil
 }
 
 // ParseCommitWithSignature check if signature is good against keystore.
@@ -113,6 +134,10 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *CommitVerific
 		}
 	}
 
+	return parseCommitWithSignatureCommitter(ctx, c, committer)
+}
+
+func parseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *CommitVerification {
 	// If no signature just report the committer
 	if c.Signature == nil {
 		return &CommitVerification{

models/git/commit_status.go

@@ -497,7 +497,7 @@ type SignCommitWithStatuses struct {
 }
 
 // ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state
-func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) []*SignCommitWithStatuses {
+func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) ([]*SignCommitWithStatuses, error) {
 	newCommits := make([]*SignCommitWithStatuses, 0, len(oldCommits))
 
 	for _, c := range oldCommits {
@@ -506,15 +506,14 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig
 		}
 		statuses, _, err := GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptions{})
 		if err != nil {
-			log.Error("GetLatestCommitStatus: %v", err)
-		} else {
-			commit.Statuses = statuses
-			commit.Status = CalcCommitStatus(statuses)
+			return nil, err
 		}
 
+		commit.Statuses = statuses
+		commit.Status = CalcCommitStatus(statuses)
 		newCommits = append(newCommits, commit)
 	}
-	return newCommits
+	return newCommits, nil
 }
 
 // hashCommitStatusContext hash context
@@ -523,18 +522,23 @@ func hashCommitStatusContext(context string) string {
 }
 
 // ConvertFromGitCommit converts git commits into SignCommitWithStatuses
-func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) []*SignCommitWithStatuses {
-	return ParseCommitsWithStatus(ctx,
-		asymkey_model.ParseCommitsWithSignature(
-			ctx,
-			user_model.ValidateCommitsWithEmails(ctx, commits),
-			repo.GetTrustModel(),
-			func(user *user_model.User) (bool, error) {
-				return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID)
-			},
-		),
-		repo,
+func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) ([]*SignCommitWithStatuses, error) {
+	validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, commits)
+	if err != nil {
+		return nil, err
+	}
+	signedCommits, err := asymkey_model.ParseCommitsWithSignature(
+		ctx,
+		validatedCommits,
+		repo.GetTrustModel(),
+		func(user *user_model.User) (bool, error) {
+			return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID)
+		},
 	)
+	if err != nil {
+		return nil, err
+	}
+	return ParseCommitsWithStatus(ctx, signedCommits, repo)
 }
 
 // CommitStatusesHideActionsURL hide Gitea Actions urls

models/issues/comment.go

@@ -802,7 +802,10 @@ func (c *Comment) LoadPushCommits(ctx context.Context) (err error) {
 		}
 		defer closer.Close()
 
-		c.Commits = git_model.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
+		c.Commits, err = git_model.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
+		if err != nil {
+			return err
+		}
 		c.CommitsNum = int64(len(c.Commits))
 	}
 

models/user/user.go

@@ -1129,28 +1129,85 @@ func ValidateCommitWithEmail(ctx context.Context, c *git.Commit) *User {
 }
 
 // ValidateCommitsWithEmails checks if authors' e-mails of commits are corresponding to users.
-func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) []*UserCommit {
+func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) ([]*UserCommit, error) {
 	var (
-		emails     = make(map[string]*User)
 		newCommits = make([]*UserCommit, 0, len(oldCommits))
+		emailSet   = make(container.Set[string])
 	)
 	for _, c := range oldCommits {
-		var u *User
 		if c.Author != nil {
-			if v, ok := emails[c.Author.Email]; !ok {
-				u, _ = GetUserByEmail(ctx, c.Author.Email)
-				emails[c.Author.Email] = u
-			} else {
-				u = v
-			}
+			emailSet.Add(c.Author.Email)
 		}
+	}
+
+	emailUserMap, err := GetUsersByEmails(ctx, emailSet.Values())
+	if err != nil {
+		return nil, err
+	}
 
+	for _, c := range oldCommits {
+		user, ok := emailUserMap[c.Author.Email]
+		if !ok {
+			user = &User{
+				Name:  c.Author.Name,
+				Email: c.Author.Email,
+			}
+		}
 		newCommits = append(newCommits, &UserCommit{
-			User:   u,
+			User:   user,
 			Commit: c,
 		})
 	}
-	return newCommits
+	return newCommits, nil
+}
+
+func GetUsersByEmails(ctx context.Context, emails []string) (map[string]*User, error) {
+	if len(emails) == 0 {
+		return nil, nil
+	}
+
+	needCheckEmails := make(container.Set[string])
+	needCheckUserNames := make(container.Set[string])
+	for _, email := range emails {
+		if strings.HasSuffix(email, fmt.Sprintf("@%s", setting.Service.NoReplyAddress)) {
+			username := strings.TrimSuffix(email, fmt.Sprintf("@%s", setting.Service.NoReplyAddress))
+			needCheckUserNames.Add(username)
+		} else {
+			needCheckEmails.Add(strings.ToLower(email))
+		}
+	}
+
+	emailAddresses := make([]*EmailAddress, 0, len(needCheckEmails))
+	if err := db.GetEngine(ctx).In("lower_email", needCheckEmails.Values()).
+		And("is_activated=?", true).
+		Find(&emailAddresses); err != nil {
+		return nil, err
+	}
+	userIDs := make(container.Set[int64])
+	for _, email := range emailAddresses {
+		userIDs.Add(email.UID)
+	}
+	users, err := GetUsersByIDs(ctx, userIDs.Values())
+	if err != nil {
+		return nil, err
+	}
+
+	results := make(map[string]*User, len(emails))
+	for _, user := range users {
+		if user.KeepEmailPrivate {
+			results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user
+		} else {
+			results[user.Email] = user
+		}
+	}
+	users = make([]*User, 0, len(needCheckUserNames))
+	if err := db.GetEngine(ctx).In("lower_name", needCheckUserNames.Values()).Find(&users); err != nil {
+		return nil, err
+	}
+	for _, user := range users {
+		results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user
+	}
+	return results, nil
 }
 
 // GetUserByEmail returns the user object by given e-mail if exists.

routers/web/repo/blame.go

@@ -234,7 +234,12 @@ func processBlameParts(ctx *context.Context, blameParts []*git.BlamePart) map[st
 	}
 
 	// populate commit email addresses to later look up avatars.
-	for _, c := range user_model.ValidateCommitsWithEmails(ctx, commits) {
+	validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, commits)
+	if err != nil {
+		ctx.ServerError("ValidateCommitsWithEmails", err)
+		return nil
+	}
+	for _, c := range validatedCommits {
 		commitNames[c.ID.String()] = c
 	}
 

routers/web/repo/commit.go

@@ -80,7 +80,11 @@ func Commits(ctx *context.Context) {
 		ctx.ServerError("CommitsByRange", err)
 		return
 	}
-	ctx.Data["Commits"] = processGitCommits(ctx, commits)
+	ctx.Data["Commits"], err = processGitCommits(ctx, commits)
+	if err != nil {
+		ctx.ServerError("processGitCommits", err)
+		return
+	}
 	commitIDs := make([]string, 0, len(commits))
 	for _, c := range commits {
 		commitIDs = append(commitIDs, c.ID.String())
@@ -192,7 +196,11 @@ func SearchCommits(ctx *context.Context) {
 		return
 	}
 	ctx.Data["CommitCount"] = len(commits)
-	ctx.Data["Commits"] = processGitCommits(ctx, commits)
+	ctx.Data["Commits"], err = processGitCommits(ctx, commits)
+	if err != nil {
+		ctx.ServerError("processGitCommits", err)
+		return
+	}
 
 	ctx.Data["Keyword"] = query
 	if all {
@@ -235,7 +243,11 @@ func FileHistory(ctx *context.Context) {
 		ctx.ServerError("CommitsByFileAndRange", err)
 		return
 	}
-	ctx.Data["Commits"] = processGitCommits(ctx, commits)
+	ctx.Data["Commits"], err = processGitCommits(ctx, commits)
+	if err != nil {
+		ctx.ServerError("processGitCommits", err)
+		return
+	}
 
 	ctx.Data["Username"] = ctx.Repo.Owner.Name
 	ctx.Data["Reponame"] = ctx.Repo.Repository.Name
@@ -416,13 +428,16 @@ func RawDiff(ctx *context.Context) {
 	}
 }
 
-func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) []*git_model.SignCommitWithStatuses {
-	commits := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository)
+func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) ([]*git_model.SignCommitWithStatuses, error) {
+	commits, err := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository)
+	if err != nil {
+		return nil, err
+	}
 	if !ctx.Repo.CanRead(unit_model.TypeActions) {
 		for _, commit := range commits {
 			commit.Status.HideActionsURL(ctx)
 			git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses)
 		}
 	}
-	return commits
+	return commits, nil
 }

routers/web/repo/compare.go

@@ -647,7 +647,11 @@ func PrepareCompareDiff(
 		return false
 	}
 
-	commits := processGitCommits(ctx, ci.CompareInfo.Commits)
+	commits, err := processGitCommits(ctx, ci.CompareInfo.Commits)
+	if err != nil {
+		ctx.ServerError("processGitCommits", err)
+		return false
+	}
 	ctx.Data["Commits"] = commits
 	ctx.Data["CommitCount"] = len(commits)
 

routers/web/repo/pull.go

@@ -631,7 +631,11 @@ func ViewPullCommits(ctx *context.Context) {
 	ctx.Data["Username"] = ctx.Repo.Owner.Name
 	ctx.Data["Reponame"] = ctx.Repo.Repository.Name
 
-	commits := processGitCommits(ctx, prInfo.Commits)
+	commits, err := processGitCommits(ctx, prInfo.Commits)
+	if err != nil {
+		ctx.ServerError("processGitCommits", err)
+		return
+	}
 	ctx.Data["Commits"] = commits
 	ctx.Data["CommitCount"] = len(commits)
 

routers/web/repo/wiki.go

@@ -437,7 +437,14 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
 		ctx.ServerError("CommitsByFileAndRange", err)
 		return nil, nil
 	}
-	ctx.Data["Commits"] = git_model.ConvertFromGitCommit(ctx, commitsHistory, ctx.Repo.Repository)
+	ctx.Data["Commits"], err = git_model.ConvertFromGitCommit(ctx, commitsHistory, ctx.Repo.Repository)
+	if err != nil {
+		if wikiRepo != nil {
+			wikiRepo.Close()
+		}
+		ctx.ServerError("ConvertFromGitCommit", err)
+		return nil, nil
+	}
 
 	pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5)
 	pager.AddParamFromRequest(ctx.Req)