diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 7195736f923..f6cebe37b02 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -79,6 +79,14 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu return nil, err } + if err := comments.loadResolveDoers(ctx); err != nil { + return nil, err + } + + if err := comments.loadReactions(ctx, issue.Repo); err != nil { + return nil, err + } + // Find all reviews by ReviewID reviews := make(map[int64]*Review) ids := make([]int64, 0, len(comments)) @@ -107,14 +115,6 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu comments[n] = comment n++ - if err := comment.LoadResolveDoer(ctx); err != nil { - return nil, err - } - - if err := comment.LoadReactions(ctx, issue.Repo); err != nil { - return nil, err - } - var err error rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{ FootnoteContextID: strconv.FormatInt(comment.ID, 10), diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 522c066309b..610bd16f5a0 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -11,6 +11,7 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/container" "gitea.dev/modules/log" + "gitea.dev/modules/setting" ) // CommentList defines a list of comments @@ -444,6 +445,73 @@ func (comments CommentList) loadReviews(ctx context.Context) error { return nil } +// loadResolveDoers bulk-loads the resolve doer for all code comments that have one. +func (comments CommentList) loadResolveDoers(ctx context.Context) error { + resolveDoerIDs := container.FilterSlice(comments, func(c *Comment) (int64, bool) { + return c.ResolveDoerID, c.ResolveDoerID != 0 && c.Type == CommentTypeCode + }) + if len(resolveDoerIDs) == 0 { + return nil + } + + userMaps, err := user_model.GetUsersMapByIDs(ctx, resolveDoerIDs) + if err != nil { + return err + } + + for _, comment := range comments { + if comment.ResolveDoerID == 0 || comment.Type != CommentTypeCode { + continue + } + if u, ok := userMaps[comment.ResolveDoerID]; ok { + comment.ResolveDoer = u + } else { + comment.ResolveDoer = user_model.NewGhostUser() + } + } + return nil +} + +// loadReactions bulk-loads reactions for all comments in the list. +func (comments CommentList) loadReactions(ctx context.Context, repo *repo_model.Repository) error { + if len(comments) == 0 { + return nil + } + + commentIDs := container.FilterSlice(comments, func(c *Comment) (int64, bool) { + return c.ID, c.Reactions == nil + }) + if len(commentIDs) == 0 { + return nil + } + + var allReactions ReactionList + if err := db.GetEngine(ctx). + Where("`comment_id` > 0"). + In("comment_id", commentIDs). + In("`type`", setting.UI.Reactions). + Asc("issue_id", "comment_id", "created_unix", "id"). + Find(&allReactions); err != nil { + return err + } + + if _, err := allReactions.LoadUsers(ctx, repo); err != nil { + return err + } + + reactByComment := make(map[int64]ReactionList, len(commentIDs)) + for _, r := range allReactions { + reactByComment[r.CommentID] = append(reactByComment[r.CommentID], r) + } + + for _, comment := range comments { + if comment.Reactions == nil { + comment.Reactions = reactByComment[comment.ID] + } + } + return nil +} + // LoadAttributes loads attributes of the comments, except for attachments and // comments func (comments CommentList) LoadAttributes(ctx context.Context) (err error) { diff --git a/models/issues/issue_list.go b/models/issues/issue_list.go index 45e25c2da89..250344dd663 100644 --- a/models/issues/issue_list.go +++ b/models/issues/issue_list.go @@ -591,9 +591,12 @@ func (issues IssueList) GetApprovalCounts(ctx context.Context) (map[int64][]*Rev func (issues IssueList) LoadIsRead(ctx context.Context, userID int64) error { issueIDs := issues.getIssueIDs() + if len(issueIDs) == 0 { + return nil + } issueUsers := make([]*IssueUser, 0, len(issueIDs)) if err := db.GetEngine(ctx).Where("uid =?", userID). - In("issue_id"). + In("issue_id", issueIDs). Find(&issueUsers); err != nil { return err } diff --git a/models/issues/issue_list_test.go b/models/issues/issue_list_test.go index 1c9f2a1e8e7..bba864b4c54 100644 --- a/models/issues/issue_list_test.go +++ b/models/issues/issue_list_test.go @@ -11,6 +11,7 @@ import ( "gitea.dev/modules/setting" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIssueList_LoadRepositories(t *testing.T) { @@ -30,6 +31,22 @@ func TestIssueList_LoadRepositories(t *testing.T) { } } +func TestIssueList_LoadIsRead(t *testing.T) { + // Regression: In("issue_id") was missing the issueIDs argument, causing + // xorm to generate "0=1" and never mark any issue as read. + require.NoError(t, unittest.PrepareTestDatabase()) + + issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + + // Fixture: uid=1 has is_read=true on issue 1 only. + issueList := issues_model.IssueList{issue1, issue2} + require.NoError(t, issueList.LoadIsRead(t.Context(), 1)) + + assert.True(t, issue1.IsRead, "issue 1 should be marked read for user 1") + assert.False(t, issue2.IsRead, "issue 2 should not be marked read for user 1") +} + func TestIssueList_LoadAttributes(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) setting.Service.EnableTimetracking = true diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 5e267cc1133..128fcd53695 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -101,6 +101,19 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions) canReadActions := ctx.Repo.Permission.CanRead(unit.TypeActions) + // Bulk-load commit statuses for all releases in one query. + var commitStatusMap map[string][]*git_model.CommitStatus + if canReadActions && len(releases) > 0 { + shas := make([]string, 0, len(releases)) + for _, r := range releases { + shas = append(shas, r.Sha1) + } + commitStatusMap, err = git_model.GetLatestCommitStatusForRepoCommitIDs(ctx, ctx.Repo.Repository.ID, shas) + if err != nil { + return nil, err + } + } + releaseInfos := make([]*ReleaseInfo, 0, len(releases)) for _, r := range releases { if r.Publisher, ok = cacheUsers[r.PublisherID]; !ok { @@ -130,11 +143,7 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions) } if canReadActions { - statuses, err := git_model.GetLatestCommitStatus(ctx, r.Repo.ID, r.Sha1, db.ListOptionsAll) - if err != nil { - return nil, err - } - + statuses := commitStatusMap[r.Sha1] info.CommitStatus = git_model.CalcCommitStatus(statuses) info.CommitStatuses = statuses } diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 6951be45ef3..75b51151f3c 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -242,13 +242,13 @@ func Milestones(ctx *context.Context) { } sort.Sort(showRepos) + repoByID := make(map[int64]*repo_model.Repository, len(showRepos)) + for _, repo := range showRepos { + repoByID[repo.ID] = repo + } + for i := 0; i < len(milestones); { - for _, repo := range showRepos { - if milestones[i].RepoID == repo.ID { - milestones[i].Repo = repo - break - } - } + milestones[i].Repo = repoByID[milestones[i].RepoID] if milestones[i].Repo == nil { log.Warn("Cannot find milestone %d 's repository %d", milestones[i].ID, milestones[i].RepoID) milestones = append(milestones[:i], milestones[i+1:]...) diff --git a/services/git/commit.go b/services/git/commit.go index bc0516b5d6f..662e0507138 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -7,7 +7,6 @@ import ( "context" asymkey_model "gitea.dev/models/asymkey" - "gitea.dev/models/db" git_model "gitea.dev/models/git" "gitea.dev/models/gituser" repo_model "gitea.dev/models/repo" @@ -72,20 +71,27 @@ func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo // ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) ([]*git_model.SignCommitWithStatuses, error) { - newCommits := make([]*git_model.SignCommitWithStatuses, 0, len(oldCommits)) + if len(oldCommits) == 0 { + return nil, nil + } + commitIDs := make([]string, 0, len(oldCommits)) for _, c := range oldCommits { - commit := &git_model.SignCommitWithStatuses{ - SignCommit: c, - } - statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.GitCommit.ID.String(), db.ListOptionsAll) - if err != nil { - return nil, err - } + commitIDs = append(commitIDs, c.GitCommit.ID.String()) + } + statusMap, err := git_model.GetLatestCommitStatusForRepoCommitIDs(ctx, repo.ID, commitIDs) + if err != nil { + return nil, err + } - commit.Statuses = statuses - commit.Status = git_model.CalcCommitStatus(statuses) - newCommits = append(newCommits, commit) + newCommits := make([]*git_model.SignCommitWithStatuses, 0, len(oldCommits)) + for _, c := range oldCommits { + statuses := statusMap[c.GitCommit.ID.String()] + newCommits = append(newCommits, &git_model.SignCommitWithStatuses{ + SignCommit: c, + Statuses: statuses, + Status: git_model.CalcCommitStatus(statuses), + }) } return newCommits, nil }