Skip to content

Commit

Permalink
Use batch database operations instead of one by one to optimze api pulls
Browse files Browse the repository at this point in the history
  • Loading branch information
lunny committed Dec 4, 2024
1 parent c9e582c commit 5760487
Show file tree
Hide file tree
Showing 5 changed files with 330 additions and 41 deletions.
60 changes: 60 additions & 0 deletions models/issues/pull_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
"xorm.io/xorm"
)

Expand Down Expand Up @@ -240,6 +241,65 @@ func (prs PullRequestList) GetIssueIDs() []int64 {
})
}

func (prs PullRequestList) LoadReviewCommentsCounts(ctx context.Context) (map[int64]int, error) {
issueIDs := prs.GetIssueIDs()
countsMap := make(map[int64]int, len(issueIDs))
counts := make([]struct {
IssueID int64
Count int
}, 0, len(issueIDs))
if err := db.GetEngine(ctx).Select("issue_id, count(*) as count").
Table("comment").In("issue_id", issueIDs).And("type = ?", CommentTypeReview).
GroupBy("issue_id").Find(&counts); err != nil {
return nil, err
}
for _, c := range counts {
countsMap[c.IssueID] = c.Count
}
return countsMap, nil
}

func (prs PullRequestList) LoadReviews(ctx context.Context) (ReviewList, error) {
issueIDs := prs.GetIssueIDs()
reviews := make([]*Review, 0, len(issueIDs))

subQuery := builder.Select("max(id) as id").
From("review").
Where(builder.In("issue_id", issueIDs)).
And(builder.In("`type`", ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest)).
And(builder.Eq{
"dismissed": false,
"original_author_id": 0,
"reviewer_team_id": 0,
}).
GroupBy("issue_id, reviewer_id")
// Get latest review of each reviewer, sorted in order they were made
if err := db.GetEngine(ctx).In("id", subQuery).OrderBy("review.updated_unix ASC").Find(&reviews); err != nil {
return nil, err
}

teamReviewRequests := make([]*Review, 0, 5)
subQueryTeam := builder.Select("max(id) as id").
From("review").
Where(builder.In("issue_id", issueIDs)).
And(builder.Eq{
"original_author_id": 0,
}).And(builder.Neq{
"reviewer_team_id": 0,
}).
GroupBy("issue_id, reviewer_team_id").
OrderBy("review.updated_unix ASC")
if err := db.GetEngine(ctx).In("id", subQueryTeam).Find(&teamReviewRequests); err != nil {
return nil, err
}

if len(teamReviewRequests) > 0 {
reviews = append(reviews, teamReviewRequests...)
}

return reviews, nil
}

// HasMergedPullRequestInRepo returns whether the user(poster) has merged pull-request in the repo
func HasMergedPullRequestInRepo(ctx context.Context, repoID, posterID int64) (bool, error) {
return db.GetEngine(ctx).
Expand Down
11 changes: 3 additions & 8 deletions models/issues/review_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,9 @@ func (reviews ReviewList) LoadReviewersTeams(ctx context.Context) error {
}
}

teamsMap := make(map[int64]*organization_model.Team, 0)
for _, teamID := range reviewersTeamsIDs {
team, err := organization_model.GetTeamByID(ctx, teamID)
if err != nil {
return err
}

teamsMap[teamID] = team
teamsMap, err := organization_model.GetTeamsByIDs(ctx, reviewersTeamsIDs)
if err != nil {
return err
}

for _, review := range reviews {
Expand Down
5 changes: 5 additions & 0 deletions models/organization/team_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,8 @@ func GetTeamsByOrgIDs(ctx context.Context, orgIDs []int64) (TeamList, error) {
teams := make([]*Team, 0, 10)
return teams, db.GetEngine(ctx).Where(builder.In("org_id", orgIDs)).Find(&teams)
}

func GetTeamsByIDs(ctx context.Context, teamIDs []int64) (map[int64]*Team, error) {
teams := make(map[int64]*Team, len(teamIDs))
return teams, db.GetEngine(ctx).Where(builder.In("`id`", teamIDs)).Find(&teams)
}
35 changes: 2 additions & 33 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,42 +139,11 @@ func ListPullRequests(ctx *context.APIContext) {
return
}

apiPrs := make([]*api.PullRequest, len(prs))
// NOTE: load repository first, so that issue.Repo will be filled with pr.BaseRepo
if err := prs.LoadRepositories(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadRepositories", err)
return
}
issueList, err := prs.LoadIssues(ctx)
apiPrs, err := convert.ToAPIPullRequests(ctx, ctx.Repo.Repository, prs, ctx.Doer)
if err != nil {
ctx.Error(http.StatusInternalServerError, "LoadIssues", err)
return
}

if err := issueList.LoadLabels(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadLabels", err)
return
}
if err := issueList.LoadPosters(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadPoster", err)
ctx.Error(http.StatusInternalServerError, "ToAPIPullRequests", err)
return
}
if err := issueList.LoadAttachments(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
return
}
if err := issueList.LoadMilestones(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadMilestones", err)
return
}
if err := issueList.LoadAssignees(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadAssignees", err)
return
}

for i := range prs {
apiPrs[i] = convert.ToAPIPullRequest(ctx, prs[i], ctx.Doer)
}

ctx.SetLinkHeader(int(maxResults), listOptions.PageSize)
ctx.SetTotalCountHeader(maxResults)
Expand Down
Loading

0 comments on commit 5760487

Please sign in to comment.