Skip to content

Commit 70327d6

Browse files
lunnylunny
authored
Improve commits list performance to reduce unnecessary database queries (#33528)
When listing commits, Gitea attempts to retrieve the actual user based on the commit email. Querying users one by one from the database is inefficient. This PR optimizes the process by batch querying users by email, reducing the number of database queries.
1 parent f232d8f commit 70327d6

File tree

10 files changed

+207
-42
lines changed

10 files changed

+207
-42
lines changed

‎models/asymkey/gpg_key_commit_verification.go‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models/db"
1313
repo_model "code.gitea.io/gitea/models/repo"
1414
user_model "code.gitea.io/gitea/models/user"
15+
"code.gitea.io/gitea/modules/container"
1516
"code.gitea.io/gitea/modules/git"
1617
"code.gitea.io/gitea/modules/log"
1718
"code.gitea.io/gitea/modules/setting"
@@ -71,21 +72,41 @@ const (
7172
)
7273

7374
// ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys.
74-
func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error)) []*SignCommit {
75+
func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error)) ([]*SignCommit, error) {
7576
newCommits := make([]*SignCommit, 0, len(oldCommits))
7677
keyMap := map[string]bool{}
7778

79+
emails := make(container.Set[string])
7880
for _, c := range oldCommits {
81+
if c.Committer != nil {
82+
emails.Add(c.Committer.Email)
83+
}
84+
}
85+
86+
emailUsers, err := user_model.GetUsersByEmails(ctx, emails.Values())
87+
if err != nil {
88+
return nil, err
89+
}
90+
91+
for _, c := range oldCommits {
92+
committer, ok := emailUsers[c.Committer.Email]
93+
if !ok && c.Committer != nil {
94+
committer = &user_model.User{
95+
Name: c.Committer.Name,
96+
Email: c.Committer.Email,
97+
}
98+
}
99+
79100
signCommit := &SignCommit{
80101
UserCommit: c,
81-
Verification: ParseCommitWithSignature(ctx, c.Commit),
102+
Verification: parseCommitWithSignatureCommitter(ctx, c.Commit, committer),
82103
}
83104

84105
_ = CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap)
85106

86107
newCommits = append(newCommits, signCommit)
87108
}
88-
return newCommits
109+
return newCommits, nil
89110
}
90111

91112
// ParseCommitWithSignature check if signature is good against keystore.
@@ -113,6 +134,10 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *CommitVerific
113134
}
114135
}
115136

137+
return parseCommitWithSignatureCommitter(ctx, c, committer)
138+
}
139+
140+
func parseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *CommitVerification {
116141
// If no signature just report the committer
117142
if c.Signature == nil {
118143
return &CommitVerification{

‎models/git/commit_status.go‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ type SignCommitWithStatuses struct {
497497
}
498498

499499
// ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state
500-
func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) []*SignCommitWithStatuses {
500+
func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) ([]*SignCommitWithStatuses, error) {
501501
newCommits := make([]*SignCommitWithStatuses, 0, len(oldCommits))
502502

503503
for _, c := range oldCommits {
@@ -506,15 +506,14 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig
506506
}
507507
statuses, _, err := GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptions{})
508508
if err != nil {
509-
log.Error("GetLatestCommitStatus: %v", err)
510-
} else {
511-
commit.Statuses = statuses
512-
commit.Status = CalcCommitStatus(statuses)
509+
return nil, err
513510
}
514511

512+
commit.Statuses = statuses
513+
commit.Status = CalcCommitStatus(statuses)
515514
newCommits = append(newCommits, commit)
516515
}
517-
return newCommits
516+
return newCommits, nil
518517
}
519518

520519
// hashCommitStatusContext hash context
@@ -523,18 +522,23 @@ func hashCommitStatusContext(context string) string {
523522
}
524523

525524
// ConvertFromGitCommit converts git commits into SignCommitWithStatuses
526-
func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) []*SignCommitWithStatuses {
527-
return ParseCommitsWithStatus(ctx,
528-
asymkey_model.ParseCommitsWithSignature(
529-
ctx,
530-
user_model.ValidateCommitsWithEmails(ctx, commits),
531-
repo.GetTrustModel(),
532-
func(user *user_model.User) (bool, error) {
533-
return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID)
534-
},
535-
),
536-
repo,
525+
func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) ([]*SignCommitWithStatuses, error) {
526+
validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, commits)
527+
if err != nil {
528+
return nil, err
529+
}
530+
signedCommits, err := asymkey_model.ParseCommitsWithSignature(
531+
ctx,
532+
validatedCommits,
533+
repo.GetTrustModel(),
534+
func(user *user_model.User) (bool, error) {
535+
return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID)
536+
},
537537
)
538+
if err != nil {
539+
return nil, err
540+
}
541+
return ParseCommitsWithStatus(ctx, signedCommits, repo)
538542
}
539543

540544
// CommitStatusesHideActionsURL hide Gitea Actions urls

‎models/issues/comment.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,10 @@ func (c *Comment) LoadPushCommits(ctx context.Context) (err error) {
802802
}
803803
defer closer.Close()
804804

805-
c.Commits = git_model.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
805+
c.Commits, err = git_model.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
806+
if err != nil {
807+
return err
808+
}
806809
c.CommitsNum = int64(len(c.Commits))
807810
}
808811

‎models/user/user.go‎

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,28 +1129,85 @@ func ValidateCommitWithEmail(ctx context.Context, c *git.Commit) *User {
11291129
}
11301130

11311131
// ValidateCommitsWithEmails checks if authors' e-mails of commits are corresponding to users.
1132-
func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) []*UserCommit {
1132+
func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) ([]*UserCommit, error) {
11331133
var (
1134-
emails = make(map[string]*User)
11351134
newCommits = make([]*UserCommit, 0, len(oldCommits))
1135+
emailSet = make(container.Set[string])
11361136
)
11371137
for _, c := range oldCommits {
1138-
var u *User
11391138
if c.Author != nil {
1140-
if v, ok := emails[c.Author.Email]; !ok {
1141-
u, _ = GetUserByEmail(ctx, c.Author.Email)
1142-
emails[c.Author.Email] = u
1143-
} else {
1144-
u = v
1145-
}
1139+
emailSet.Add(c.Author.Email)
11461140
}
1141+
}
1142+
1143+
emailUserMap, err := GetUsersByEmails(ctx, emailSet.Values())
1144+
if err != nil {
1145+
return nil, err
1146+
}
11471147

1148+
for _, c := range oldCommits {
1149+
user, ok := emailUserMap[c.Author.Email]
1150+
if !ok {
1151+
user = &User{
1152+
Name: c.Author.Name,
1153+
Email: c.Author.Email,
1154+
}
1155+
}
11481156
newCommits = append(newCommits, &UserCommit{
1149-
User: u,
1157+
User: user,
11501158
Commit: c,
11511159
})
11521160
}
1153-
return newCommits
1161+
return newCommits, nil
1162+
}
1163+
1164+
func GetUsersByEmails(ctx context.Context, emails []string) (map[string]*User, error) {
1165+
if len(emails) == 0 {
1166+
return nil, nil
1167+
}
1168+
1169+
needCheckEmails := make(container.Set[string])
1170+
needCheckUserNames := make(container.Set[string])
1171+
for _, email := range emails {
1172+
if strings.HasSuffix(email, fmt.Sprintf("@%s", setting.Service.NoReplyAddress)) {
1173+
username := strings.TrimSuffix(email, fmt.Sprintf("@%s", setting.Service.NoReplyAddress))
1174+
needCheckUserNames.Add(username)
1175+
} else {
1176+
needCheckEmails.Add(strings.ToLower(email))
1177+
}
1178+
}
1179+
1180+
emailAddresses := make([]*EmailAddress, 0, len(needCheckEmails))
1181+
if err := db.GetEngine(ctx).In("lower_email", needCheckEmails.Values()).
1182+
And("is_activated=?", true).
1183+
Find(&emailAddresses); err != nil {
1184+
return nil, err
1185+
}
1186+
userIDs := make(container.Set[int64])
1187+
for _, email := range emailAddresses {
1188+
userIDs.Add(email.UID)
1189+
}
1190+
users, err := GetUsersByIDs(ctx, userIDs.Values())
1191+
if err != nil {
1192+
return nil, err
1193+
}
1194+
1195+
results := make(map[string]*User, len(emails))
1196+
for _, user := range users {
1197+
if user.KeepEmailPrivate {
1198+
results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user
1199+
} else {
1200+
results[user.Email] = user
1201+
}
1202+
}
1203+
users = make([]*User, 0, len(needCheckUserNames))
1204+
if err := db.GetEngine(ctx).In("lower_name", needCheckUserNames.Values()).Find(&users); err != nil {
1205+
return nil, err
1206+
}
1207+
for _, user := range users {
1208+
results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user
1209+
}
1210+
return results, nil
11541211
}
11551212

11561213
// GetUserByEmail returns the user object by given e-mail if exists.

‎routers/web/repo/blame.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,12 @@ func processBlameParts(ctx *context.Context, blameParts []*git.BlamePart) map[st
234234
}
235235

236236
// populate commit email addresses to later look up avatars.
237-
for _, c := range user_model.ValidateCommitsWithEmails(ctx, commits) {
237+
validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, commits)
238+
if err != nil {
239+
ctx.ServerError("ValidateCommitsWithEmails", err)
240+
return nil
241+
}
242+
for _, c := range validatedCommits {
238243
commitNames[c.ID.String()] = c
239244
}
240245

‎routers/web/repo/commit.go‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ func Commits(ctx *context.Context) {
8080
ctx.ServerError("CommitsByRange", err)
8181
return
8282
}
83-
ctx.Data["Commits"] = processGitCommits(ctx, commits)
83+
ctx.Data["Commits"], err = processGitCommits(ctx, commits)
84+
if err != nil {
85+
ctx.ServerError("processGitCommits", err)
86+
return
87+
}
8488
commitIDs := make([]string, 0, len(commits))
8589
for _, c := range commits {
8690
commitIDs = append(commitIDs, c.ID.String())
@@ -192,7 +196,11 @@ func SearchCommits(ctx *context.Context) {
192196
return
193197
}
194198
ctx.Data["CommitCount"] = len(commits)
195-
ctx.Data["Commits"] = processGitCommits(ctx, commits)
199+
ctx.Data["Commits"], err = processGitCommits(ctx, commits)
200+
if err != nil {
201+
ctx.ServerError("processGitCommits", err)
202+
return
203+
}
196204

197205
ctx.Data["Keyword"] = query
198206
if all {
@@ -235,7 +243,11 @@ func FileHistory(ctx *context.Context) {
235243
ctx.ServerError("CommitsByFileAndRange", err)
236244
return
237245
}
238-
ctx.Data["Commits"] = processGitCommits(ctx, commits)
246+
ctx.Data["Commits"], err = processGitCommits(ctx, commits)
247+
if err != nil {
248+
ctx.ServerError("processGitCommits", err)
249+
return
250+
}
239251

240252
ctx.Data["Username"] = ctx.Repo.Owner.Name
241253
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
@@ -416,13 +428,16 @@ func RawDiff(ctx *context.Context) {
416428
}
417429
}
418430

419-
func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) []*git_model.SignCommitWithStatuses {
420-
commits := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository)
431+
func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) ([]*git_model.SignCommitWithStatuses, error) {
432+
commits, err := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository)
433+
if err != nil {
434+
return nil, err
435+
}
421436
if !ctx.Repo.CanRead(unit_model.TypeActions) {
422437
for _, commit := range commits {
423438
commit.Status.HideActionsURL(ctx)
424439
git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses)
425440
}
426441
}
427-
return commits
442+
return commits, nil
428443
}

‎routers/web/repo/compare.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,11 @@ func PrepareCompareDiff(
647647
return false
648648
}
649649

650-
commits := processGitCommits(ctx, ci.CompareInfo.Commits)
650+
commits, err := processGitCommits(ctx, ci.CompareInfo.Commits)
651+
if err != nil {
652+
ctx.ServerError("processGitCommits", err)
653+
return false
654+
}
651655
ctx.Data["Commits"] = commits
652656
ctx.Data["CommitCount"] = len(commits)
653657

‎routers/web/repo/pull.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,11 @@ func ViewPullCommits(ctx *context.Context) {
631631
ctx.Data["Username"] = ctx.Repo.Owner.Name
632632
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
633633

634-
commits := processGitCommits(ctx, prInfo.Commits)
634+
commits, err := processGitCommits(ctx, prInfo.Commits)
635+
if err != nil {
636+
ctx.ServerError("processGitCommits", err)
637+
return
638+
}
635639
ctx.Data["Commits"] = commits
636640
ctx.Data["CommitCount"] = len(commits)
637641

‎routers/web/repo/wiki.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,14 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
437437
ctx.ServerError("CommitsByFileAndRange", err)
438438
return nil, nil
439439
}
440-
ctx.Data["Commits"] = git_model.ConvertFromGitCommit(ctx, commitsHistory, ctx.Repo.Repository)
440+
ctx.Data["Commits"], err = git_model.ConvertFromGitCommit(ctx, commitsHistory, ctx.Repo.Repository)
441+
if err != nil {
442+
if wikiRepo != nil {
443+
wikiRepo.Close()
444+
}
445+
ctx.ServerError("ConvertFromGitCommit", err)
446+
return nil, nil
447+
}
441448

442449
pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5)
443450
pager.AddParamFromRequest(ctx.Req)

0 commit comments

Comments
 (0)