community
/
gitea
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Performance improvements for pull request list API (#30490) 

Fix #30483

---------

Co-authored-by: yp05327 <576951401@qq.com>
Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
Lunny Xiao 2024-05-31 20:10:11 +08:00 committed by GitHub
parent 972f807ee7
commit 352a2cae24
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 243 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
models/issues/assignees.go
View File

@ -27,23 +27,27 @@ func init() {
// LoadAssignees load assignees of this issue. // LoadAssignees load assignees of this issue.
func (issue *Issue) LoadAssignees(ctx context.Context) (err error) { func (issue *Issue) LoadAssignees(ctx context.Context) (err error) {
if issue.isAssigneeLoaded || len(issue.Assignees) > 0 {
return nil
}
// Reset maybe preexisting assignees // Reset maybe preexisting assignees
issue.Assignees = []*user_model.User{} issue.Assignees = []*user_model.User{}
issue.Assignee = nil issue.Assignee = nil
err = db.GetEngine(ctx).Table("`user`"). if err = db.GetEngine(ctx).Table("`user`").
Join("INNER", "issue_assignees", "assignee_id = `user`.id"). Join("INNER", "issue_assignees", "assignee_id = `user`.id").
Where("issue_assignees.issue_id = ?", issue.ID). Where("issue_assignees.issue_id = ?", issue.ID).
Find(&issue.Assignees) Find(&issue.Assignees); err != nil {
if err != nil {
return err return err
} }
issue.isAssigneeLoaded = true
// Check if we have at least one assignee and if yes put it in as `Assignee` // Check if we have at least one assignee and if yes put it in as `Assignee`
if len(issue.Assignees) > 0 { if len(issue.Assignees) > 0 {
issue.Assignee = issue.Assignees[0] issue.Assignee = issue.Assignees[0]
} }
return err return nil
} }
// GetAssigneeIDsByIssue returns the IDs of users assigned to an issue // GetAssigneeIDsByIssue returns the IDs of users assigned to an issue

12
models/issues/comment_list.go
View File

@ -16,19 +16,17 @@ import (
// CommentList defines a list of comments // CommentList defines a list of comments
type CommentList []*Comment type CommentList []*Comment
func (comments CommentList) getPosterIDs() []int64 {
return container.FilterSlice(comments, func(c *Comment) (int64, bool) {
return c.PosterID, c.PosterID > 0
})
}
// LoadPosters loads posters // LoadPosters loads posters
func (comments CommentList) LoadPosters(ctx context.Context) error { func (comments CommentList) LoadPosters(ctx context.Context) error {
if len(comments) == 0 { if len(comments) == 0 {
return nil return nil
} }
posterMaps, err := getPosters(ctx, comments.getPosterIDs()) posterIDs := container.FilterSlice(comments, func(c *Comment) (int64, bool) {
return c.PosterID, c.Poster == nil && c.PosterID > 0
})
posterMaps, err := getPostersByIDs(ctx, posterIDs)
if err != nil { if err != nil {
return err return err
} }

96
models/issues/issue.go
View File

@ -98,32 +98,35 @@ var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already
// Issue represents an issue or pull request of repository. // Issue represents an issue or pull request of repository.
type Issue struct { type Issue struct {
ID int64 `xorm:"pk autoincr"` ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"` RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"`
Repo *repo_model.Repository `xorm:"-"` Repo *repo_model.Repository `xorm:"-"`
Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository. Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository.
PosterID int64 `xorm:"INDEX"` PosterID int64 `xorm:"INDEX"`
Poster *user_model.User `xorm:"-"` Poster *user_model.User `xorm:"-"`
OriginalAuthor string OriginalAuthor string
OriginalAuthorID int64 `xorm:"index"` OriginalAuthorID int64 `xorm:"index"`
Title string `xorm:"name"` Title string `xorm:"name"`
Content string `xorm:"LONGTEXT"` Content string `xorm:"LONGTEXT"`
RenderedContent template.HTML `xorm:"-"` RenderedContent template.HTML `xorm:"-"`
ContentVersion int `xorm:"NOT NULL DEFAULT 0"` ContentVersion int `xorm:"NOT NULL DEFAULT 0"`
Labels []*Label `xorm:"-"` Labels []*Label `xorm:"-"`
MilestoneID int64 `xorm:"INDEX"` isLabelsLoaded bool `xorm:"-"`
Milestone *Milestone `xorm:"-"` MilestoneID int64 `xorm:"INDEX"`
Project *project_model.Project `xorm:"-"` Milestone *Milestone `xorm:"-"`
Priority int isMilestoneLoaded bool `xorm:"-"`
AssigneeID int64 `xorm:"-"` Project *project_model.Project `xorm:"-"`
Assignee *user_model.User `xorm:"-"` Priority int
IsClosed bool `xorm:"INDEX"` AssigneeID int64 `xorm:"-"`
IsRead bool `xorm:"-"` Assignee *user_model.User `xorm:"-"`
IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not. isAssigneeLoaded bool `xorm:"-"`
PullRequest *PullRequest `xorm:"-"` IsClosed bool `xorm:"INDEX"`
NumComments int IsRead bool `xorm:"-"`
Ref string IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not.
PinOrder int `xorm:"DEFAULT 0"` PullRequest *PullRequest `xorm:"-"`
NumComments int
Ref string
PinOrder int `xorm:"DEFAULT 0"`
DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
@ -131,11 +134,12 @@ type Issue struct {
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
ClosedUnix timeutil.TimeStamp `xorm:"INDEX"` ClosedUnix timeutil.TimeStamp `xorm:"INDEX"`
Attachments []*repo_model.Attachment `xorm:"-"` Attachments []*repo_model.Attachment `xorm:"-"`
Comments CommentList `xorm:"-"` isAttachmentsLoaded bool `xorm:"-"`
Reactions ReactionList `xorm:"-"` Comments CommentList `xorm:"-"`
TotalTrackedTime int64 `xorm:"-"` Reactions ReactionList `xorm:"-"`
Assignees []*user_model.User `xorm:"-"` TotalTrackedTime int64 `xorm:"-"`
Assignees []*user_model.User `xorm:"-"`
// IsLocked limits commenting abilities to users on an issue // IsLocked limits commenting abilities to users on an issue
// with write access // with write access
@ -187,6 +191,19 @@ func (issue *Issue) LoadRepo(ctx context.Context) (err error) {
return nil return nil
} }
func (issue *Issue) LoadAttachments(ctx context.Context) (err error) {
if issue.isAttachmentsLoaded || issue.Attachments != nil {
return nil
}
issue.Attachments, err = repo_model.GetAttachmentsByIssueID(ctx, issue.ID)
if err != nil {
return fmt.Errorf("getAttachmentsByIssueID [%d]: %w", issue.ID, err)
}
issue.isAttachmentsLoaded = true
return nil
}
// IsTimetrackerEnabled returns true if the repo enables timetracking // IsTimetrackerEnabled returns true if the repo enables timetracking
func (issue *Issue) IsTimetrackerEnabled(ctx context.Context) bool { func (issue *Issue) IsTimetrackerEnabled(ctx context.Context) bool {
if err := issue.LoadRepo(ctx); err != nil { if err := issue.LoadRepo(ctx); err != nil {
@ -287,11 +304,12 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) {
// LoadMilestone load milestone of this issue. // LoadMilestone load milestone of this issue.
func (issue *Issue) LoadMilestone(ctx context.Context) (err error) { func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
if (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 { if !issue.isMilestoneLoaded && (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 {
issue.Milestone, err = GetMilestoneByRepoID(ctx, issue.RepoID, issue.MilestoneID) issue.Milestone, err = GetMilestoneByRepoID(ctx, issue.RepoID, issue.MilestoneID)
if err != nil && !IsErrMilestoneNotExist(err) { if err != nil && !IsErrMilestoneNotExist(err) {
return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %w", issue.RepoID, issue.MilestoneID, err) return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %w", issue.RepoID, issue.MilestoneID, err)
} }
issue.isMilestoneLoaded = true
} }
return nil return nil
} }
@ -327,11 +345,8 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
return err return err
} }
if issue.Attachments == nil { if err = issue.LoadAttachments(ctx); err != nil {
issue.Attachments, err = repo_model.GetAttachmentsByIssueID(ctx, issue.ID) return err
if err != nil {
return fmt.Errorf("getAttachmentsByIssueID [%d]: %w", issue.ID, err)
}
} }
if err = issue.loadComments(ctx); err != nil { if err = issue.loadComments(ctx); err != nil {
@ -350,6 +365,13 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
return issue.loadReactions(ctx) return issue.loadReactions(ctx)
} }
func (issue *Issue) ResetAttributesLoaded() {
issue.isLabelsLoaded = false
issue.isMilestoneLoaded = false
issue.isAttachmentsLoaded = false
issue.isAssigneeLoaded = false
}
// GetIsRead load the `IsRead` field of the issue // GetIsRead load the `IsRead` field of the issue
func (issue *Issue) GetIsRead(ctx context.Context, userID int64) error { func (issue *Issue) GetIsRead(ctx context.Context, userID int64) error {
issueUser := &IssueUser{IssueID: issue.ID, UID: userID} issueUser := &IssueUser{IssueID: issue.ID, UID: userID}

6
models/issues/issue_label.go
View File

@ -111,6 +111,7 @@ func NewIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m
return err return err
} }
issue.isLabelsLoaded = false
issue.Labels = nil issue.Labels = nil
if err = issue.LoadLabels(ctx); err != nil { if err = issue.LoadLabels(ctx); err != nil {
return err return err
@ -160,6 +161,8 @@ func NewIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer *us
return err return err
} }
// reload all labels
issue.isLabelsLoaded = false
issue.Labels = nil issue.Labels = nil
if err = issue.LoadLabels(ctx); err != nil { if err = issue.LoadLabels(ctx); err != nil {
return err return err
@ -325,11 +328,12 @@ func FixIssueLabelWithOutsideLabels(ctx context.Context) (int64, error) {
// LoadLabels loads labels // LoadLabels loads labels
func (issue *Issue) LoadLabels(ctx context.Context) (err error) { func (issue *Issue) LoadLabels(ctx context.Context) (err error) {
if issue.Labels == nil && issue.ID != 0 { if !issue.isLabelsLoaded && issue.Labels == nil && issue.ID != 0 {
issue.Labels, err = GetLabelsByIssueID(ctx, issue.ID) issue.Labels, err = GetLabelsByIssueID(ctx, issue.ID)
if err != nil { if err != nil {
return fmt.Errorf("getLabelsByIssueID [%d]: %w", issue.ID, err) return fmt.Errorf("getLabelsByIssueID [%d]: %w", issue.ID, err)
} }
issue.isLabelsLoaded = true
} }
return nil return nil
} }

47
models/issues/issue_list.go
View File

@ -72,18 +72,16 @@ func (issues IssueList) LoadRepositories(ctx context.Context) (repo_model.Reposi
return repo_model.ValuesRepository(repoMaps), nil return repo_model.ValuesRepository(repoMaps), nil
} }
func (issues IssueList) getPosterIDs() []int64 { func (issues IssueList) LoadPosters(ctx context.Context) error {
return container.FilterSlice(issues, func(issue *Issue) (int64, bool) {
return issue.PosterID, true
})
}
func (issues IssueList) loadPosters(ctx context.Context) error {
if len(issues) == 0 { if len(issues) == 0 {
return nil return nil
} }
posterMaps, err := getPosters(ctx, issues.getPosterIDs()) posterIDs := container.FilterSlice(issues, func(issue *Issue) (int64, bool) {
return issue.PosterID, issue.Poster == nil && issue.PosterID > 0
})
posterMaps, err := getPostersByIDs(ctx, posterIDs)
if err != nil { if err != nil {
return err return err
} }
@ -94,7 +92,7 @@ func (issues IssueList) loadPosters(ctx context.Context) error {
return nil return nil
} }
func getPosters(ctx context.Context, posterIDs []int64) (map[int64]*user_model.User, error) { func getPostersByIDs(ctx context.Context, posterIDs []int64) (map[int64]*user_model.User, error) {
posterMaps := make(map[int64]*user_model.User, len(posterIDs)) posterMaps := make(map[int64]*user_model.User, len(posterIDs))
left := len(posterIDs) left := len(posterIDs)
for left > 0 { for left > 0 {
@ -136,7 +134,7 @@ func (issues IssueList) getIssueIDs() []int64 {
return ids return ids
} }
func (issues IssueList) loadLabels(ctx context.Context) error { func (issues IssueList) LoadLabels(ctx context.Context) error {
if len(issues) == 0 { if len(issues) == 0 {
return nil return nil
} }
@ -168,7 +166,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error {
err = rows.Scan(&labelIssue) err = rows.Scan(&labelIssue)
if err != nil { if err != nil {
if err1 := rows.Close(); err1 != nil { if err1 := rows.Close(); err1 != nil {
return fmt.Errorf("IssueList.loadLabels: Close: %w", err1) return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1)
} }
return err return err
} }
@ -177,7 +175,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error {
// When there are no rows left and we try to close it. // When there are no rows left and we try to close it.
// Since that is not relevant for us, we can safely ignore it. // Since that is not relevant for us, we can safely ignore it.
if err1 := rows.Close(); err1 != nil { if err1 := rows.Close(); err1 != nil {
return fmt.Errorf("IssueList.loadLabels: Close: %w", err1) return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1)
} }
left -= limit left -= limit
issueIDs = issueIDs[limit:] issueIDs = issueIDs[limit:]
@ -185,6 +183,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error {
for _, issue := range issues { for _, issue := range issues {
issue.Labels = issueLabels[issue.ID] issue.Labels = issueLabels[issue.ID]
issue.isLabelsLoaded = true
} }
return nil return nil
} }
@ -195,7 +194,7 @@ func (issues IssueList) getMilestoneIDs() []int64 {
}) })
} }
func (issues IssueList) loadMilestones(ctx context.Context) error { func (issues IssueList) LoadMilestones(ctx context.Context) error {
milestoneIDs := issues.getMilestoneIDs() milestoneIDs := issues.getMilestoneIDs()
if len(milestoneIDs) == 0 { if len(milestoneIDs) == 0 {
return nil return nil
@ -220,6 +219,7 @@ func (issues IssueList) loadMilestones(ctx context.Context) error {
for _, issue := range issues { for _, issue := range issues {
issue.Milestone = milestoneMaps[issue.MilestoneID] issue.Milestone = milestoneMaps[issue.MilestoneID]
issue.isMilestoneLoaded = true
} }
return nil return nil
} }
@ -263,7 +263,7 @@ func (issues IssueList) LoadProjects(ctx context.Context) error {
return nil return nil
} }
func (issues IssueList) loadAssignees(ctx context.Context) error { func (issues IssueList) LoadAssignees(ctx context.Context) error {
if len(issues) == 0 { if len(issues) == 0 {
return nil return nil
} }
@ -310,6 +310,10 @@ func (issues IssueList) loadAssignees(ctx context.Context) error {
for _, issue := range issues { for _, issue := range issues {
issue.Assignees = assignees[issue.ID] issue.Assignees = assignees[issue.ID]
if len(issue.Assignees) > 0 {
issue.Assignee = issue.Assignees[0]
}
issue.isAssigneeLoaded = true
} }
return nil return nil
} }
@ -413,6 +417,7 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) {
for _, issue := range issues { for _, issue := range issues {
issue.Attachments = attachments[issue.ID] issue.Attachments = attachments[issue.ID]
issue.isAttachmentsLoaded = true
} }
return nil return nil
} }
@ -538,23 +543,23 @@ func (issues IssueList) LoadAttributes(ctx context.Context) error {
return fmt.Errorf("issue.loadAttributes: LoadRepositories: %w", err) return fmt.Errorf("issue.loadAttributes: LoadRepositories: %w", err)
} }
if err := issues.loadPosters(ctx); err != nil { if err := issues.LoadPosters(ctx); err != nil {
return fmt.Errorf("issue.loadAttributes: loadPosters: %w", err) return fmt.Errorf("issue.loadAttributes: LoadPosters: %w", err)
} }
if err := issues.loadLabels(ctx); err != nil { if err := issues.LoadLabels(ctx); err != nil {
return fmt.Errorf("issue.loadAttributes: loadLabels: %w", err) return fmt.Errorf("issue.loadAttributes: LoadLabels: %w", err)
} }
if err := issues.loadMilestones(ctx); err != nil { if err := issues.LoadMilestones(ctx); err != nil {
return fmt.Errorf("issue.loadAttributes: loadMilestones: %w", err) return fmt.Errorf("issue.loadAttributes: LoadMilestones: %w", err)
} }
if err := issues.LoadProjects(ctx); err != nil { if err := issues.LoadProjects(ctx); err != nil {
return fmt.Errorf("issue.loadAttributes: loadProjects: %w", err) return fmt.Errorf("issue.loadAttributes: loadProjects: %w", err)
} }
if err := issues.loadAssignees(ctx); err != nil { if err := issues.LoadAssignees(ctx); err != nil {
return fmt.Errorf("issue.loadAttributes: loadAssignees: %w", err) return fmt.Errorf("issue.loadAttributes: loadAssignees: %w", err)
} }

13
models/issues/pull.go
View File

@ -159,10 +159,11 @@ type PullRequest struct {
ChangedProtectedFiles []string `xorm:"TEXT JSON"` ChangedProtectedFiles []string `xorm:"TEXT JSON"`
IssueID int64 `xorm:"INDEX"` IssueID int64 `xorm:"INDEX"`
Issue *Issue `xorm:"-"` Issue *Issue `xorm:"-"`
Index int64 Index int64
RequestedReviewers []*user_model.User `xorm:"-"` RequestedReviewers []*user_model.User `xorm:"-"`
isRequestedReviewersLoaded bool `xorm:"-"`
HeadRepoID int64 `xorm:"INDEX"` HeadRepoID int64 `xorm:"INDEX"`
HeadRepo *repo_model.Repository `xorm:"-"` HeadRepo *repo_model.Repository `xorm:"-"`
@ -289,7 +290,7 @@ func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) {
// LoadRequestedReviewers loads the requested reviewers. // LoadRequestedReviewers loads the requested reviewers.
func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error { func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error {
if len(pr.RequestedReviewers) > 0 { if pr.isRequestedReviewersLoaded || len(pr.RequestedReviewers) > 0 {
return nil return nil
} }
@ -297,10 +298,10 @@ func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error {
if err != nil { if err != nil {
return err return err
} }
if err = reviews.LoadReviewers(ctx); err != nil { if err = reviews.LoadReviewers(ctx); err != nil {
return err return err
} }
pr.isRequestedReviewersLoaded = true
for _, review := range reviews { for _, review := range reviews {
pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer) pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer)
} }

111
models/issues/pull_list.go
View File

@ -9,8 +9,10 @@ import (
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
access_model "code.gitea.io/gitea/models/perm/access" access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
@ -123,7 +125,7 @@ func GetPullRequestIDsByCheckStatus(ctx context.Context, status PullRequestStatu
} }
// PullRequests returns all pull requests for a base Repo by the given conditions // PullRequests returns all pull requests for a base Repo by the given conditions
func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptions) ([]*PullRequest, int64, error) { func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptions) (PullRequestList, int64, error) {
if opts.Page <= 0 { if opts.Page <= 0 {
opts.Page = 1 opts.Page = 1
} }
@ -153,50 +155,93 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio
// PullRequestList defines a list of pull requests // PullRequestList defines a list of pull requests
type PullRequestList []*PullRequest type PullRequestList []*PullRequest
func (prs PullRequestList) LoadAttributes(ctx context.Context) error { func (prs PullRequestList) getRepositoryIDs() []int64 {
if len(prs) == 0 { repoIDs := make(container.Set[int64])
return nil for _, pr := range prs {
if pr.BaseRepo == nil && pr.BaseRepoID > 0 {
repoIDs.Add(pr.BaseRepoID)
}
if pr.HeadRepo == nil && pr.HeadRepoID > 0 {
repoIDs.Add(pr.HeadRepoID)
}
} }
return repoIDs.Values()
}
// Load issues. func (prs PullRequestList) LoadRepositories(ctx context.Context) error {
issueIDs := prs.GetIssueIDs() repoIDs := prs.getRepositoryIDs()
issues := make([]*Issue, 0, len(issueIDs)) reposMap := make(map[int64]*repo_model.Repository, len(repoIDs))
if err := db.GetEngine(ctx). if err := db.GetEngine(ctx).
Where("id > 0"). In("id", repoIDs).
In("id", issueIDs). Find(&reposMap); err != nil {
Find(&issues); err != nil { return fmt.Errorf("find repos: %w", err)
return fmt.Errorf("find issues: %w", err)
}
set := make(map[int64]*Issue)
for i := range issues {
set[issues[i].ID] = issues[i]
} }
for _, pr := range prs { for _, pr := range prs {
pr.Issue = set[pr.IssueID] if pr.BaseRepo == nil {
/* pr.BaseRepo = reposMap[pr.BaseRepoID]
Old code: }
pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync if pr.HeadRepo == nil {
pr.HeadRepo = reposMap[pr.HeadRepoID]
It's worth panic because it's almost impossible to happen under normal use. pr.isHeadRepoLoaded = true
But in integration testing, an asynchronous task could read a database that has been reset.
So returning an error would make more sense, let the caller has a choice to ignore it.
*/
if pr.Issue == nil {
return fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist)
} }
pr.Issue.PullRequest = pr
} }
return nil return nil
} }
func (prs PullRequestList) LoadAttributes(ctx context.Context) error {
if _, err := prs.LoadIssues(ctx); err != nil {
return err
}
return nil
}
func (prs PullRequestList) LoadIssues(ctx context.Context) (IssueList, error) {
if len(prs) == 0 {
return nil, nil
}
// Load issues.
issueIDs := prs.GetIssueIDs()
issues := make(map[int64]*Issue, len(issueIDs))
if err := db.GetEngine(ctx).
In("id", issueIDs).
Find(&issues); err != nil {
return nil, fmt.Errorf("find issues: %w", err)
}
issueList := make(IssueList, 0, len(prs))
for _, pr := range prs {
if pr.Issue == nil {
pr.Issue = issues[pr.IssueID]
/*
Old code:
pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync
It's worth panic because it's almost impossible to happen under normal use.
But in integration testing, an asynchronous task could read a database that has been reset.
So returning an error would make more sense, let the caller has a choice to ignore it.
*/
if pr.Issue == nil {
return nil, fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist)
}
}
pr.Issue.PullRequest = pr
if pr.Issue.Repo == nil {
pr.Issue.Repo = pr.BaseRepo
}
issueList = append(issueList, pr.Issue)
}
return issueList, nil
}
// GetIssueIDs returns all issue ids // GetIssueIDs returns all issue ids
func (prs PullRequestList) GetIssueIDs() []int64 { func (prs PullRequestList) GetIssueIDs() []int64 {
issueIDs := make([]int64, 0, len(prs)) return container.FilterSlice(prs, func(pr *PullRequest) (int64, bool) {
for i := range prs { if pr.Issue == nil {
issueIDs = append(issueIDs, prs[i].IssueID) return pr.IssueID, pr.IssueID > 0
} }
return issueIDs return 0, false
})
} }
// HasMergedPullRequestInRepo returns whether the user(poster) has merged pull-request in the repo // HasMergedPullRequestInRepo returns whether the user(poster) has merged pull-request in the repo

4
models/user/user.go
View File

@ -856,6 +856,10 @@ func GetUserByID(ctx context.Context, id int64) (*User, error) {
// GetUserByIDs returns the user objects by given IDs if exists. // GetUserByIDs returns the user objects by given IDs if exists.
func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) { func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
if len(ids) == 0 {
return nil, nil
}
users := make([]*User, 0, len(ids)) users := make([]*User, 0, len(ids))
err := db.GetEngine(ctx).In("id", ids). err := db.GetEngine(ctx).In("id", ids).
Table("user"). Table("user").

48
routers/api/v1/repo/pull.go
View File

@ -116,23 +116,39 @@ func ListPullRequests(ctx *context.APIContext) {
} }
apiPrs := make([]*api.PullRequest, len(prs)) 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)
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)
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 { for i := range prs {
if err = prs[i].LoadIssue(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
return
}
if err = prs[i].LoadAttributes(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
return
}
if err = prs[i].LoadBaseRepo(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadBaseRepo", err)
return
}
if err = prs[i].LoadHeadRepo(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
return
}
apiPrs[i] = convert.ToAPIPullRequest(ctx, prs[i], ctx.Doer) apiPrs[i] = convert.ToAPIPullRequest(ctx, prs[i], ctx.Doer)
} }

9
services/convert/issue.go
View File

@ -31,15 +31,15 @@ func ToAPIIssue(ctx context.Context, doer *user_model.User, issue *issues_model.
} }
func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, getDownloadURL func(repo *repo_model.Repository, attach *repo_model.Attachment) string) *api.Issue { func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, getDownloadURL func(repo *repo_model.Repository, attach *repo_model.Attachment) string) *api.Issue {
if err := issue.LoadLabels(ctx); err != nil {
return &api.Issue{}
}
if err := issue.LoadPoster(ctx); err != nil { if err := issue.LoadPoster(ctx); err != nil {
return &api.Issue{} return &api.Issue{}
} }
if err := issue.LoadRepo(ctx); err != nil { if err := issue.LoadRepo(ctx); err != nil {
return &api.Issue{} return &api.Issue{}
} }
if err := issue.LoadAttachments(ctx); err != nil {
return &api.Issue{}
}
apiIssue := &api.Issue{ apiIssue := &api.Issue{
ID: issue.ID, ID: issue.ID,
@ -63,6 +63,9 @@ func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Iss
} }
apiIssue.URL = issue.APIURL(ctx) apiIssue.URL = issue.APIURL(ctx)
apiIssue.HTMLURL = issue.HTMLURL() apiIssue.HTMLURL = issue.HTMLURL()
if err := issue.LoadLabels(ctx); err != nil {
return &api.Issue{}
}
apiIssue.Labels = ToLabelList(issue.Labels, issue.Repo, issue.Repo.Owner) apiIssue.Labels = ToLabelList(issue.Labels, issue.Repo, issue.Repo.Owner)
apiIssue.Repo = &api.RepositoryMeta{ apiIssue.Repo = &api.RepositoryMeta{
ID: issue.Repo.ID, ID: issue.Repo.ID,

12
services/convert/pull.go
View File

@ -11,6 +11,7 @@ import (
"code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access" access_model "code.gitea.io/gitea/models/perm/access"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
@ -44,7 +45,16 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
return nil return nil
} }
p, err := access_model.GetUserRepoPermission(ctx, pr.BaseRepo, doer) var doerID int64
if doer != nil {
doerID = doer.ID
}
const repoDoerPermCacheKey = "repo_doer_perm_cache"
p, err := cache.GetWithContextCache(ctx, repoDoerPermCacheKey, fmt.Sprintf("%d_%d", pr.BaseRepoID, doerID),
func() (access_model.Permission, error) {
return access_model.GetUserRepoPermission(ctx, pr.BaseRepo, doer)
})
if err != nil { if err != nil {
log.Error("GetUserRepoPermission[%d]: %v", pr.BaseRepoID, err) log.Error("GetUserRepoPermission[%d]: %v", pr.BaseRepoID, err)
p.AccessMode = perm.AccessModeNone p.AccessMode = perm.AccessModeNone

3
services/issue/assignee_test.go
View File

@ -39,7 +39,8 @@ func TestDeleteNotPassedAssignee(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.Empty(t, issue.Assignees) assert.Empty(t, issue.Assignees)
// Check they're gone // Reload to check they're gone
issue.ResetAttributesLoaded()
assert.NoError(t, issue.LoadAssignees(db.DefaultContext)) assert.NoError(t, issue.LoadAssignees(db.DefaultContext))
assert.Empty(t, issue.Assignees) assert.Empty(t, issue.Assignees)
assert.Empty(t, issue.Assignee) assert.Empty(t, issue.Assignee)