Skip to content

Commit dd1b2fe

Browse files
lunny6543
authored andcommitted
Performance improvement for list pull requests (go-gitea#15447)
1 parent 802a431 commit dd1b2fe

File tree

5 files changed

+72
-29
lines changed

5 files changed

+72
-29
lines changed

models/issue_list.go

+8
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ func (issues IssueList) loadRepositories(e Engine) ([]*Repository, error) {
5353

5454
for _, issue := range issues {
5555
issue.Repo = repoMaps[issue.RepoID]
56+
if issue.PullRequest != nil {
57+
issue.PullRequest.BaseRepo = issue.Repo
58+
}
5659
}
5760
return valuesRepository(repoMaps), nil
5861
}
@@ -516,6 +519,11 @@ func (issues IssueList) LoadDiscussComments() error {
516519
return issues.loadComments(x, builder.Eq{"comment.type": CommentTypeComment})
517520
}
518521

522+
// LoadPullRequests loads pull requests
523+
func (issues IssueList) LoadPullRequests() error {
524+
return issues.loadPullRequests(x)
525+
}
526+
519527
// GetApprovalCounts returns a map of issue ID to slice of approval counts
520528
// FIXME: only returns official counts due to double counting of non-official approvals
521529
func (issues IssueList) GetApprovalCounts() (map[int64][]*ReviewCount, error) {

routers/repo/issue.go

+7-12
Original file line numberDiff line numberDiff line change
@@ -241,14 +241,13 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
241241
}
242242
}
243243

244-
approvalCounts, err := models.IssueList(issues).GetApprovalCounts()
244+
var issueList = models.IssueList(issues)
245+
approvalCounts, err := issueList.GetApprovalCounts()
245246
if err != nil {
246247
ctx.ServerError("ApprovalCounts", err)
247248
return
248249
}
249250

250-
var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
251-
252251
// Get posters.
253252
for i := range issues {
254253
// Check read status
@@ -258,16 +257,12 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
258257
ctx.ServerError("GetIsRead", err)
259258
return
260259
}
260+
}
261261

262-
if issues[i].IsPull {
263-
if err := issues[i].LoadPullRequest(); err != nil {
264-
ctx.ServerError("LoadPullRequest", err)
265-
return
266-
}
267-
268-
var statuses, _ = pull_service.GetLastCommitStatus(issues[i].PullRequest)
269-
commitStatus[issues[i].PullRequest.ID] = models.CalcCommitStatus(statuses)
270-
}
262+
commitStatus, err := pull_service.GetIssuesLastCommitStatus(issues)
263+
if err != nil {
264+
ctx.ServerError("GetIssuesLastCommitStatus", err)
265+
return
271266
}
272267

273268
ctx.Data["Issues"] = issues

routers/user/home.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -546,14 +546,14 @@ func buildIssueOverview(ctx *context.Context, unitType models.UnitType) {
546546
}
547547

548548
// maps pull request IDs to their CommitStatus. Will be posted to ctx.Data.
549-
var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
550549
for _, issue := range issues {
551550
issue.Repo = showReposMap[issue.RepoID]
551+
}
552552

553-
if isPullList {
554-
var statuses, _ = pull_service.GetLastCommitStatus(issue.PullRequest)
555-
commitStatus[issue.PullRequest.ID] = models.CalcCommitStatus(statuses)
556-
}
553+
commitStatus, err := pull_service.GetIssuesLastCommitStatus(issues)
554+
if err != nil {
555+
ctx.ServerError("GetIssuesLastCommitStatus", err)
556+
return
557557
}
558558

559559
// -------------------------------

services/pull/pull.go

+50-10
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"bufio"
99
"bytes"
1010
"context"
11-
"errors"
1211
"fmt"
1312
"strings"
1413
"time"
@@ -643,33 +642,74 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
643642
return stringBuilder.String()
644643
}
645644

645+
// GetIssuesLastCommitStatus returns a map
646+
func GetIssuesLastCommitStatus(issues models.IssueList) (map[int64]*models.CommitStatus, error) {
647+
if err := issues.LoadPullRequests(); err != nil {
648+
return nil, err
649+
}
650+
if _, err := issues.LoadRepositories(); err != nil {
651+
return nil, err
652+
}
653+
654+
var (
655+
gitRepos = make(map[int64]*git.Repository)
656+
res = make(map[int64]*models.CommitStatus)
657+
err error
658+
)
659+
defer func() {
660+
for _, gitRepo := range gitRepos {
661+
gitRepo.Close()
662+
}
663+
}()
664+
665+
for _, issue := range issues {
666+
if !issue.IsPull {
667+
continue
668+
}
669+
gitRepo, ok := gitRepos[issue.RepoID]
670+
if !ok {
671+
gitRepo, err = git.OpenRepository(issue.Repo.RepoPath())
672+
if err != nil {
673+
return nil, err
674+
}
675+
gitRepos[issue.RepoID] = gitRepo
676+
}
677+
678+
status, err := getLastCommitStatus(gitRepo, issue.PullRequest)
679+
if err != nil {
680+
return nil, err
681+
}
682+
res[issue.PullRequest.ID] = status
683+
}
684+
return res, nil
685+
}
686+
646687
// GetLastCommitStatus returns list of commit statuses for latest commit on this pull request.
647-
func GetLastCommitStatus(pr *models.PullRequest) (status []*models.CommitStatus, err error) {
688+
func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, err error) {
648689
if err = pr.LoadBaseRepo(); err != nil {
649690
return nil, err
650691
}
651-
652692
gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
653693
if err != nil {
654694
return nil, err
655695
}
656696
defer gitRepo.Close()
657697

658-
compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName())
698+
return getLastCommitStatus(gitRepo, pr)
699+
}
700+
701+
// getLastCommitStatus get pr's last commit status. PR's last commit status is the head commit id's last commit status
702+
func getLastCommitStatus(gitRepo *git.Repository, pr *models.PullRequest) (status *models.CommitStatus, err error) {
703+
sha, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
659704
if err != nil {
660705
return nil, err
661706
}
662707

663-
if compareInfo.Commits.Len() == 0 {
664-
return nil, errors.New("pull request has no commits")
665-
}
666-
667-
sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String()
668708
statusList, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, models.ListOptions{})
669709
if err != nil {
670710
return nil, err
671711
}
672-
return statusList, nil
712+
return models.CalcCommitStatus(statusList), nil
673713
}
674714

675715
// IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head

templates/shared/issuelist.tmpl

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,12 @@
6262
{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}}
6363
{{end}}
6464
{{if and .Milestone (ne $.listType "milestone")}}
65-
<a class="milestone" {{if $.RepoLink}}href="{{$.RepoLink}}/milestone/{{.Milestone.ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}/milestone/{{.Milestone.ID}}"{{end}}>
65+
<a class="milestone" {{if $.RepoLink}}href="{{$.RepoLink}}/milestone/{{.Milestone.ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.OwnerName}}/{{.Repo.Name}}/milestone/{{.Milestone.ID}}"{{end}}>
6666
{{svg "octicon-milestone" 14 "mr-2"}}{{.Milestone.Name}}
6767
</a>
6868
{{end}}
6969
{{if .Ref}}
70-
<a class="ref" {{if $.RepoLink}}href="{{$.RepoLink}}{{index $.IssueRefURLs .ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}{{index $.IssueRefURLs .ID}}"{{end}}>
70+
<a class="ref" {{if $.RepoLink}}href="{{$.RepoLink}}{{index $.IssueRefURLs .ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.OwnerName}}/{{.Repo.Name}}{{index $.IssueRefURLs .ID}}"{{end}}>
7171
{{svg "octicon-git-branch" 14 "mr-2"}}{{index $.IssueRefEndNames .ID}}
7272
</a>
7373
{{end}}

0 commit comments

Comments
 (0)