diff --git a/models/activities/notification_list.go b/models/activities/notification_list.go index 9353ff5d5f6..89a0c62ce54 100644 --- a/models/activities/notification_list.go +++ b/models/activities/notification_list.go @@ -228,30 +228,15 @@ func (nl NotificationList) LoadRepos(ctx context.Context) (repo_model.Repository repoIDs := nl.getPendingRepoIDs() repos := make(map[int64]*repo_model.Repository, len(repoIDs)) - left := len(repoIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", repoIDs[:limit]). - Rows(new(repo_model.Repository)) - if err != nil { - return nil, nil, err - } - - for rows.Next() { - var repo repo_model.Repository - err = rows.Scan(&repo) - if err != nil { - rows.Close() - return nil, nil, err - } - - repos[repo.ID] = &repo - } - _ = rows.Close() - - left -= limit - repoIDs = repoIDs[limit:] + err := db.QueryAndProcessByChunk(repoIDs, func(chunk []int64) (out []*repo_model.Repository, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(repo *repo_model.Repository) error { + repos[repo.ID] = repo + return nil + }) + if err != nil { + return nil, nil, err } failed := []int{} @@ -299,30 +284,15 @@ func (nl NotificationList) LoadIssues(ctx context.Context) ([]int, error) { issueIDs := nl.getPendingIssueIDs() issues := make(map[int64]*issues_model.Issue, len(issueIDs)) - left := len(issueIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", issueIDs[:limit]). - Rows(new(issues_model.Issue)) - if err != nil { - return nil, err - } - - for rows.Next() { - var issue issues_model.Issue - err = rows.Scan(&issue) - if err != nil { - rows.Close() - return nil, err - } - - issues[issue.ID] = &issue - } - _ = rows.Close() - - left -= limit - issueIDs = issueIDs[limit:] + err := db.QueryAndProcessByChunk(issueIDs, func(chunk []int64) (out []*issues_model.Issue, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(issue *issues_model.Issue) error { + issues[issue.ID] = issue + return nil + }) + if err != nil { + return nil, err } failures := []int{} @@ -391,30 +361,15 @@ func (nl NotificationList) LoadUsers(ctx context.Context) ([]int, error) { userIDs := nl.getUserIDs() users := make(map[int64]*user_model.User, len(userIDs)) - left := len(userIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", userIDs[:limit]). - Rows(new(user_model.User)) - if err != nil { - return nil, err - } - - for rows.Next() { - var user user_model.User - err = rows.Scan(&user) - if err != nil { - rows.Close() - return nil, err - } - - users[user.ID] = &user - } - _ = rows.Close() - - left -= limit - userIDs = userIDs[limit:] + err := db.QueryAndProcessByChunk(userIDs, func(chunk []int64) (out []*user_model.User, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(user *user_model.User) error { + users[user.ID] = user + return nil + }) + if err != nil { + return nil, err } failures := []int{} @@ -439,30 +394,15 @@ func (nl NotificationList) LoadComments(ctx context.Context) ([]int, error) { commentIDs := nl.getPendingCommentIDs() comments := make(map[int64]*issues_model.Comment, len(commentIDs)) - left := len(commentIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", commentIDs[:limit]). - Rows(new(issues_model.Comment)) - if err != nil { - return nil, err - } - - for rows.Next() { - var comment issues_model.Comment - err = rows.Scan(&comment) - if err != nil { - rows.Close() - return nil, err - } - - comments[comment.ID] = &comment - } - _ = rows.Close() - - left -= limit - commentIDs = commentIDs[limit:] + err := db.QueryAndProcessByChunk(commentIDs, func(chunk []int64) (out []*issues_model.Comment, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(comment *issues_model.Comment) error { + comments[comment.ID] = comment + return nil + }) + if err != nil { + return nil, err } failures := []int{} diff --git a/models/activities/notification_test.go b/models/activities/notification_test.go index 74838d8671f..beb404d0351 100644 --- a/models/activities/notification_test.go +++ b/models/activities/notification_test.go @@ -15,6 +15,7 @@ import ( user_model "gitea.dev/models/user" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCreateOrUpdateIssueNotifications(t *testing.T) { @@ -109,6 +110,71 @@ func TestNotificationsForUser(t *testing.T) { } } +func TestNotificationListLoadAttributes(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + first := &activities_model.Notification{RepoID: 1, IssueID: 1, UserID: 1, CommentID: 2} + last := &activities_model.Notification{RepoID: 2, IssueID: 4, UserID: 2, CommentID: 8} + loaded := &activities_model.Notification{ + RepoID: 1, IssueID: 1, UserID: 1, CommentID: 2, + Repository: &repo_model.Repository{ID: 1}, + Issue: &issues_model.Issue{ID: 1}, + User: &user_model.User{ID: 1}, + Comment: &issues_model.Comment{ID: 2}, + } + saved := *loaded + list := activities_model.NotificationList{first} + var wantFailures []int + for i := range db.DefaultMaxInSize { + id := unittest.NonexistentID - int64(i) + wantFailures = append(wantFailures, len(list)) + list = append(list, &activities_model.Notification{RepoID: id, IssueID: id, UserID: id, CommentID: id}) + } + withoutIssue := &activities_model.Notification{RepoID: 1} + list = append(list, last, loaded, withoutIssue) + + repos, failures, err := list.LoadRepos(t.Context()) + require.NoError(t, err) + assert.Equal(t, wantFailures, failures) + require.Len(t, repos, 2) + assert.Equal(t, int64(1), repos[0].ID) + assert.Equal(t, int64(2), repos[1].ID) + failures, err = list.LoadIssues(t.Context()) + require.NoError(t, err) + assert.Equal(t, wantFailures, failures) + failures, err = list.LoadUsers(t.Context()) + require.NoError(t, err) + assert.Empty(t, failures) + failures, err = list.LoadComments(t.Context()) + require.NoError(t, err) + assert.Empty(t, failures) + + for _, notification := range []*activities_model.Notification{first, last} { + require.NotNil(t, notification.Repository) + assert.Equal(t, notification.RepoID, notification.Repository.ID) + require.NotNil(t, notification.Issue) + assert.Equal(t, notification.IssueID, notification.Issue.ID) + assert.Same(t, notification.Repository, notification.Issue.Repo) + require.NotNil(t, notification.User) + assert.Equal(t, notification.UserID, notification.User.ID) + require.NotNil(t, notification.Comment) + assert.Equal(t, notification.CommentID, notification.Comment.ID) + assert.Same(t, notification.Issue, notification.Comment.Issue) + } + assert.Same(t, saved.Repository, loaded.Repository) + assert.Same(t, saved.Issue, loaded.Issue) + assert.Same(t, saved.User, loaded.User) + assert.Same(t, saved.Comment, loaded.Comment) + for _, notification := range list[1 : db.DefaultMaxInSize+1] { + assert.Nil(t, notification.Repository) + assert.Nil(t, notification.Issue) + assert.Nil(t, notification.User) + assert.Nil(t, notification.Comment) + } + assert.Nil(t, withoutIssue.Issue) + assert.Nil(t, withoutIssue.User) + assert.Nil(t, withoutIssue.Comment) +} + func TestNotification_GetRepo(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) notf := unittest.AssertExistsAndLoadBean(t, &activities_model.Notification{RepoID: 1}) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 5a018922d31..95a527fcdf7 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -25,7 +25,6 @@ import ( "golang.org/x/crypto/bcrypt" "golang.org/x/oauth2" "xorm.io/builder" - "xorm.io/xorm" ) // Authorization codes should expire within 10 minutes per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2 @@ -587,22 +586,17 @@ func GetOAuth2GrantsByUserID(ctx context.Context, uid int64) ([]*OAuth2Grant, er Grant *OAuth2Grant `xorm:"extends"` Application *OAuth2Application `xorm:"extends"` } - var results *xorm.Rows - var err error - if results, err = db.GetEngine(ctx). + var joinedOAuth2Grants []*joinedOAuth2Grant + err := db.GetEngine(ctx). Table("oauth2_grant"). Where("user_id = ?", uid). Join("INNER", "oauth2_application", "application_id = oauth2_application.id"). - Rows(new(joinedOAuth2Grant)); err != nil { + Find(&joinedOAuth2Grants) + if err != nil { return nil, err } - defer results.Close() grants := make([]*OAuth2Grant, 0) - for results.Next() { - joinedGrant := new(joinedOAuth2Grant) - if err := results.Scan(joinedGrant); err != nil { - return nil, err - } + for _, joinedGrant := range joinedOAuth2Grants { joinedGrant.Grant.Application = joinedGrant.Application grants = append(grants, joinedGrant.Grant) } diff --git a/models/db/list.go b/models/db/list.go index 64a823addd2..6d1e89bb8b7 100644 --- a/models/db/list.go +++ b/models/db/list.go @@ -5,6 +5,7 @@ package db import ( "context" + "slices" "gitea.dev/modules/setting" @@ -198,3 +199,18 @@ func FindAndCount[T any](ctx context.Context, opts FindOptions) ([]*T, int64, er } return objects, cnt, nil } + +func QueryAndProcessByChunk[IN, OUT any](in []IN, fnIn func(chunk []IN) ([]OUT, error), fnOut func(out OUT) error) error { + for chunkIn := range slices.Chunk(in, DefaultMaxInSize) { + chunkOut, err := fnIn(chunkIn) + if err != nil { + return err + } + for _, out := range chunkOut { + if err := fnOut(out); err != nil { + return err + } + } + } + return nil +} diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 24b28234d0e..88405c02155 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -56,28 +56,15 @@ func (comments CommentList) loadLabels(ctx context.Context) error { return nil } commentLabels := make(map[int64]*Label, len(labelIDs)) - left := len(labelIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", labelIDs[:limit]). - Rows(new(Label)) - if err != nil { - return err - } - - for rows.Next() { - var label Label - err = rows.Scan(&label) - if err != nil { - _ = rows.Close() - return err - } - commentLabels[label.ID] = &label - } - _ = rows.Close() - left -= limit - labelIDs = labelIDs[limit:] + err := db.QueryAndProcessByChunk(labelIDs, func(chunk []int64) (out []*Label, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(label *Label) error { + commentLabels[label.ID] = label + return nil + }) + if err != nil { + return err } for _, comment := range comments { @@ -103,17 +90,15 @@ func (comments CommentList) loadMilestones(ctx context.Context) error { } milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) - left := len(milestoneIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - err := db.GetEngine(ctx). - In("id", milestoneIDs[:limit]). - Find(&milestoneMaps) - if err != nil { - return err - } - left -= limit - milestoneIDs = milestoneIDs[limit:] + err := db.QueryAndProcessByChunk(milestoneIDs, func(chunk []int64) (out []*Milestone, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(milestone *Milestone) error { + milestoneMaps[milestone.ID] = milestone + return nil + }) + if err != nil { + return err } for _, comment := range comments { @@ -139,17 +124,15 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error { } milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) - left := len(milestoneIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - err := db.GetEngine(ctx). - In("id", milestoneIDs[:limit]). - Find(&milestoneMaps) - if err != nil { - return err - } - left -= limit - milestoneIDs = milestoneIDs[limit:] + err := db.QueryAndProcessByChunk(milestoneIDs, func(chunk []int64) (out []*Milestone, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(milestone *Milestone) error { + milestoneMaps[milestone.ID] = milestone + return nil + }) + if err != nil { + return err } for _, issue := range comments { @@ -174,30 +157,15 @@ func (comments CommentList) loadAssignees(ctx context.Context) error { return nil } assignees := make(map[int64]*user_model.User, len(assigneeIDs)) - left := len(assigneeIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", assigneeIDs[:limit]). - Rows(new(user_model.User)) - if err != nil { - return err - } - - for rows.Next() { - var user user_model.User - err = rows.Scan(&user) - if err != nil { - rows.Close() - return err - } - - assignees[user.ID] = &user - } - _ = rows.Close() - - left -= limit - assigneeIDs = assigneeIDs[limit:] + err := db.QueryAndProcessByChunk(assigneeIDs, func(chunk []int64) (out []*user_model.User, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(user *user_model.User) error { + assignees[user.ID] = user + return nil + }) + if err != nil { + return err } for _, comment := range comments { @@ -246,30 +214,15 @@ func (comments CommentList) LoadIssues(ctx context.Context) error { issueIDs := comments.getIssueIDs() issues := make(map[int64]*Issue, len(issueIDs)) - left := len(issueIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("id", issueIDs[:limit]). - Rows(new(Issue)) - if err != nil { - return err - } - - for rows.Next() { - var issue Issue - err = rows.Scan(&issue) - if err != nil { - rows.Close() - return err - } - - issues[issue.ID] = &issue - } - _ = rows.Close() - - left -= limit - issueIDs = issueIDs[limit:] + err := db.QueryAndProcessByChunk(issueIDs, func(chunk []int64) (out []*Issue, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(issue *Issue) error { + issues[issue.ID] = issue + return nil + }) + if err != nil { + return err } for _, comment := range comments { @@ -294,36 +247,20 @@ func (comments CommentList) loadDependentIssues(ctx context.Context) error { return nil } - e := db.GetEngine(ctx) issueIDs := comments.getDependentIssueIDs() if len(issueIDs) == 0 { return nil } issues := make(map[int64]*Issue, len(issueIDs)) - left := len(issueIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := e. - In("id", issueIDs[:limit]). - Rows(new(Issue)) - if err != nil { - return err - } - - for rows.Next() { - var issue Issue - err = rows.Scan(&issue) - if err != nil { - _ = rows.Close() - return err - } - - issues[issue.ID] = &issue - } - _ = rows.Close() - - left -= limit - issueIDs = issueIDs[limit:] + err := db.QueryAndProcessByChunk(issueIDs, func(chunk []int64) (out []*Issue, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(issue *Issue) error { + issues[issue.ID] = issue + return nil + }) + if err != nil { + return err } for _, comment := range comments { @@ -376,29 +313,15 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) { attachments := make(map[int64][]*repo_model.Attachment, len(comments)) commentsIDs := comments.getAttachmentCommentIDs() - left := len(commentsIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("comment_id", commentsIDs[:limit]). - Rows(new(repo_model.Attachment)) - if err != nil { - return err - } - - for rows.Next() { - var attachment repo_model.Attachment - err = rows.Scan(&attachment) - if err != nil { - _ = rows.Close() - return err - } - attachments[attachment.CommentID] = append(attachments[attachment.CommentID], &attachment) - } - - _ = rows.Close() - left -= limit - commentsIDs = commentsIDs[limit:] + err = db.QueryAndProcessByChunk(commentsIDs, func(chunk []int64) (out []*repo_model.Attachment, err error) { + err = db.GetEngine(ctx).In("comment_id", chunk).Find(&out) + return out, err + }, func(attachment *repo_model.Attachment) error { + attachments[attachment.CommentID] = append(attachments[attachment.CommentID], attachment) + return nil + }) + if err != nil { + return err } for _, comment := range comments { diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index 1364cdbec63..49408f67775 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -14,6 +14,7 @@ import ( user_model "gitea.dev/models/user" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCreateComment(t *testing.T) { @@ -59,17 +60,62 @@ func TestLoadAssigneeUserAndTeam_DeletedTeamBecomesGhostTeam(t *testing.T) { } func TestCommentListLoadAttributesMixedAssignees(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) + require.NoError(t, unittest.PrepareTestDatabase()) + first := &issues_model.Comment{ + ID: 1, PosterID: 1, IssueID: 1, LabelID: 1, MilestoneID: 1, OldMilestoneID: 1, AssigneeID: 1, DependentIssueID: 2, + } + last := &issues_model.Comment{ + ID: 2, PosterID: 2, IssueID: 4, LabelID: 2, MilestoneID: 3, OldMilestoneID: 3, AssigneeID: 2, DependentIssueID: 1, + } comments := issues_model.CommentList{ {AssigneeTeamID: 8}, {AssigneeID: 999999}, + first, } + for i := range db.DefaultMaxInSize { + id := unittest.NonexistentID - int64(i) + comments = append(comments, &issues_model.Comment{ + ID: id, PosterID: id, IssueID: id, LabelID: id, MilestoneID: id, OldMilestoneID: id, AssigneeID: id, DependentIssueID: id, + }) + } + comments = append(comments, last) - assert.NoError(t, comments.LoadAttributes(t.Context())) + require.NoError(t, comments.LoadAttributes(t.Context())) assert.Nil(t, comments[0].Assignee) assert.Zero(t, comments[0].AssigneeID) assert.Equal(t, user_model.GhostUserID, comments[1].AssigneeID) assert.Equal(t, user_model.GhostUserID, comments[1].Assignee.ID) + for _, comment := range []*issues_model.Comment{first, last} { + require.NotNil(t, comment.Poster) + assert.Equal(t, comment.PosterID, comment.Poster.ID) + require.NotNil(t, comment.Issue) + assert.Equal(t, comment.IssueID, comment.Issue.ID) + require.NotNil(t, comment.Label) + assert.Equal(t, comment.LabelID, comment.Label.ID) + require.NotNil(t, comment.Milestone) + assert.Equal(t, comment.MilestoneID, comment.Milestone.ID) + require.NotNil(t, comment.OldMilestone) + assert.Equal(t, comment.OldMilestoneID, comment.OldMilestone.ID) + require.NotNil(t, comment.Assignee) + assert.Equal(t, comment.AssigneeID, comment.Assignee.ID) + require.NotNil(t, comment.DependentIssue) + assert.Equal(t, comment.DependentIssueID, comment.DependentIssue.ID) + require.NotNil(t, comment.DependentIssue.Repo) + assert.Equal(t, comment.DependentIssue.RepoID, comment.DependentIssue.Repo.ID) + require.Len(t, comment.Attachments, 2) + for _, attachment := range comment.Attachments { + assert.Equal(t, comment.ID, attachment.CommentID) + } + } + missing := comments[3] + assert.Equal(t, user_model.GhostUserID, missing.Poster.ID) + assert.Equal(t, user_model.GhostUserID, missing.Assignee.ID) + assert.Nil(t, missing.Issue) + assert.Nil(t, missing.Label) + assert.Nil(t, missing.Milestone) + assert.Nil(t, missing.OldMilestone) + assert.Nil(t, missing.DependentIssue) + assert.Empty(t, missing.Attachments) } func Test_UpdateCommentAttachment(t *testing.T) { diff --git a/models/issues/issue_list.go b/models/issues/issue_list.go index 250344dd663..b879a733492 100644 --- a/models/issues/issue_list.go +++ b/models/issues/issue_list.go @@ -40,17 +40,15 @@ func (issues IssueList) LoadRepositories(ctx context.Context) (repo_model.Reposi repoIDs := issues.getRepoIDs() repoMaps := make(map[int64]*repo_model.Repository, len(repoIDs)) - left := len(repoIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - err := db.GetEngine(ctx). - In("id", repoIDs[:limit]). - Find(&repoMaps) - if err != nil { - return nil, fmt.Errorf("find repository: %w", err) - } - left -= limit - repoIDs = repoIDs[limit:] + err := db.QueryAndProcessByChunk(repoIDs, func(chunk []int64) (out []*repo_model.Repository, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(repo *repo_model.Repository) error { + repoMaps[repo.ID] = repo + return nil + }) + if err != nil { + return nil, fmt.Errorf("find repository: %w", err) } for _, issue := range issues { @@ -111,36 +109,20 @@ func (issues IssueList) LoadLabels(ctx context.Context) error { issueLabels := make(map[int64][]*Label, len(issues)*3) issueIDs := issues.getIssueIDs() - left := len(issueIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx).Table("label"). - Join("LEFT", "issue_label", "issue_label.label_id = label.id"). - In("issue_label.issue_id", issueIDs[:limit]). - Asc("label.name"). - Rows(new(LabelIssue)) - if err != nil { - return err - } - for rows.Next() { - var labelIssue LabelIssue - err = rows.Scan(&labelIssue) - if err != nil { - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1) - } - return err - } - issueLabels[labelIssue.IssueLabel.IssueID] = append(issueLabels[labelIssue.IssueLabel.IssueID], labelIssue.Label) - } - // When there are no rows left and we try to close it. - // Since that is not relevant for us, we can safely ignore it. - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1) - } - left -= limit - issueIDs = issueIDs[limit:] + err := db.QueryAndProcessByChunk(issueIDs, func(chunk []int64) (out []*LabelIssue, err error) { + err = db.GetEngine(ctx).Table("label"). + Join("LEFT", "issue_label", "issue_label.label_id = label.id"). + In("issue_label.issue_id", chunk). + Asc("label.name"). + Find(&out) + return out, err + }, func(out *LabelIssue) error { + issueLabels[out.IssueLabel.IssueID] = append(issueLabels[out.IssueLabel.IssueID], out.Label) + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.LoadLabels: %w", err) } for _, issue := range issues { @@ -163,17 +145,15 @@ func (issues IssueList) LoadMilestones(ctx context.Context) error { } milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) - left := len(milestoneIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - err := db.GetEngine(ctx). - In("id", milestoneIDs[:limit]). - Find(&milestoneMaps) - if err != nil { - return err - } - left -= limit - milestoneIDs = milestoneIDs[limit:] + err := db.QueryAndProcessByChunk(milestoneIDs, func(chunk []int64) (out []*Milestone, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(milestone *Milestone) error { + milestoneMaps[milestone.ID] = milestone + return nil + }) + if err != nil { + return err } for _, issue := range issues { @@ -186,32 +166,27 @@ func (issues IssueList) LoadMilestones(ctx context.Context) error { func (issues IssueList) LoadProjects(ctx context.Context) error { issueIDs := issues.getIssueIDs() issueProjectMaps := make(map[int64][]*project_model.Project, len(issues)) - left := len(issueIDs) type projectWithIssueID struct { *project_model.Project `xorm:"extends"` IssueID int64 } - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - - projects := make([]*projectWithIssueID, 0, limit) - err := db.GetEngine(ctx). + err := db.QueryAndProcessByChunk(issueIDs, func(chunk []int64) (out []*projectWithIssueID, err error) { + err = db.GetEngine(ctx). Table("project"). Select("project.*, project_issue.issue_id"). Join("INNER", "project_issue", "project.id = project_issue.project_id"). - In("project_issue.issue_id", issueIDs[:limit]). + In("project_issue.issue_id", chunk). OrderBy("project_issue.issue_id ASC, project.id ASC"). - Find(&projects) - if err != nil { - return err - } - for _, project := range projects { - issueProjectMaps[project.IssueID] = append(issueProjectMaps[project.IssueID], project.Project) - } - left -= limit - issueIDs = issueIDs[limit:] + Find(&out) + return out, err + }, func(project *projectWithIssueID) error { + issueProjectMaps[project.IssueID] = append(issueProjectMaps[project.IssueID], project.Project) + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.LoadProjects: %w", err) } for _, issue := range issues { @@ -233,34 +208,18 @@ func (issues IssueList) LoadAssignees(ctx context.Context) error { assignees := make(map[int64][]*user_model.User, len(issues)) issueIDs := issues.getIssueIDs() - left := len(issueIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx).Table("issue_assignees"). + err := db.QueryAndProcessByChunk(issueIDs, func(chunk []int64) (out []*AssigneeIssue, err error) { + err = db.GetEngine(ctx).Table("issue_assignees"). Join("INNER", "`user`", "`user`.id = `issue_assignees`.assignee_id"). - In("`issue_assignees`.issue_id", issueIDs[:limit]).OrderBy(user_model.GetOrderByName()). - Rows(new(AssigneeIssue)) - if err != nil { - return err - } - - for rows.Next() { - var assigneeIssue AssigneeIssue - err = rows.Scan(&assigneeIssue) - if err != nil { - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadAssignees: Close: %w", err1) - } - return err - } - - assignees[assigneeIssue.IssueAssignee.IssueID] = append(assignees[assigneeIssue.IssueAssignee.IssueID], assigneeIssue.Assignee) - } - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadAssignees: Close: %w", err1) - } - left -= limit - issueIDs = issueIDs[limit:] + In("`issue_assignees`.issue_id", chunk).OrderBy(user_model.GetOrderByName()). + Find(&out) + return out, err + }, func(assigneeIssue *AssigneeIssue) error { + assignees[assigneeIssue.IssueAssignee.IssueID] = append(assignees[assigneeIssue.IssueAssignee.IssueID], assigneeIssue.Assignee) + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.LoadAssignees: %w", err) } for _, issue := range issues { @@ -291,32 +250,17 @@ func (issues IssueList) LoadPullRequests(ctx context.Context) error { } pullRequestMaps := make(map[int64]*PullRequest, len(issuesIDs)) - left := len(issuesIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("issue_id", issuesIDs[:limit]). - Rows(new(PullRequest)) - if err != nil { - return err - } - - for rows.Next() { - var pr PullRequest - err = rows.Scan(&pr) - if err != nil { - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadPullRequests: Close: %w", err1) - } - return err - } - pullRequestMaps[pr.IssueID] = &pr - } - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadPullRequests: Close: %w", err1) - } - left -= limit - issuesIDs = issuesIDs[limit:] + err := db.QueryAndProcessByChunk(issuesIDs, func(chunk []int64) (out []*PullRequest, err error) { + err = db.GetEngine(ctx). + In("issue_id", chunk). + Find(&out) + return out, err + }, func(pr *PullRequest) error { + pullRequestMaps[pr.IssueID] = pr + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.LoadPullRequests: %w", err) } for _, issue := range issues { @@ -336,32 +280,15 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) { attachments := make(map[int64][]*repo_model.Attachment, len(issues)) issuesIDs := issues.getIssueIDs() - left := len(issuesIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx). - In("issue_id", issuesIDs[:limit]). - Rows(new(repo_model.Attachment)) - if err != nil { - return err - } - - for rows.Next() { - var attachment repo_model.Attachment - err = rows.Scan(&attachment) - if err != nil { - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadAttachments: Close: %w", err1) - } - return err - } - attachments[attachment.IssueID] = append(attachments[attachment.IssueID], &attachment) - } - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadAttachments: Close: %w", err1) - } - left -= limit - issuesIDs = issuesIDs[limit:] + err = db.QueryAndProcessByChunk(issuesIDs, func(chunk []int64) (out []*repo_model.Attachment, err error) { + err = db.GetEngine(ctx).In("issue_id", chunk).Find(&out) + return out, err + }, func(attachment *repo_model.Attachment) error { + attachments[attachment.IssueID] = append(attachments[attachment.IssueID], attachment) + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.LoadAttachments: %w", err) } for _, issue := range issues { @@ -378,35 +305,20 @@ func (issues IssueList) loadComments(ctx context.Context, cond builder.Cond) (er comments := make(map[int64][]*Comment, len(issues)) issuesIDs := issues.getIssueIDs() - left := len(issuesIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - rows, err := db.GetEngine(ctx).Table("comment"). + err = db.QueryAndProcessByChunk(issuesIDs, func(chunk []int64) (out []*Comment, err error) { + err = db.GetEngine(ctx).Table("comment"). Join("INNER", "issue", "issue.id = comment.issue_id"). - In("issue.id", issuesIDs[:limit]). + In("issue.id", chunk). Where(cond). NoAutoCondition(). - Rows(new(Comment)) - if err != nil { - return err - } - - for rows.Next() { - var comment Comment - err = rows.Scan(&comment) - if err != nil { - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadComments: Close: %w", err1) - } - return err - } - comments[comment.IssueID] = append(comments[comment.IssueID], &comment) - } - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadComments: Close: %w", err1) - } - left -= limit - issuesIDs = issuesIDs[limit:] + Find(&out) + return out, err + }, func(comment *Comment) error { + comments[comment.IssueID] = append(comments[comment.IssueID], comment) + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.loadComments: %w", err) } for _, issue := range issues { @@ -442,37 +354,21 @@ func (issues IssueList) loadTotalTrackedTimes(ctx context.Context) (err error) { } } - left := len(ids) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - + err = db.QueryAndProcessByChunk(ids, func(chunk []int64) (out []*totalTimesByIssue, err error) { // select issue_id, sum(time) from tracked_time where issue_id in () group by issue_id - rows, err := db.GetEngine(ctx).Table("tracked_time"). + err = db.GetEngine(ctx).Table("tracked_time"). Where("deleted = ?", false). Select("issue_id, sum(time) as time"). - In("issue_id", ids[:limit]). + In("issue_id", chunk). GroupBy("issue_id"). - Rows(new(totalTimesByIssue)) - if err != nil { - return err - } - - for rows.Next() { - var totalTime totalTimesByIssue - err = rows.Scan(&totalTime) - if err != nil { - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadTotalTrackedTimes: Close: %w", err1) - } - return err - } - trackedTimes[totalTime.IssueID] = totalTime.Time - } - if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadTotalTrackedTimes: Close: %w", err1) - } - left -= limit - ids = ids[limit:] + Find(&out) + return out, err + }, func(totalTime *totalTimesByIssue) error { + trackedTimes[totalTime.IssueID] = totalTime.Time + return nil + }) + if err != nil { + return fmt.Errorf("IssueList.loadTotalTrackedTimes: %w", err) } for _, issue := range issues { diff --git a/models/issues/issue_list_test.go b/models/issues/issue_list_test.go index bba864b4c54..1d8f8febd62 100644 --- a/models/issues/issue_list_test.go +++ b/models/issues/issue_list_test.go @@ -6,9 +6,11 @@ package issues_test import ( "testing" + "gitea.dev/models/db" issues_model "gitea.dev/models/issues" "gitea.dev/models/unittest" "gitea.dev/modules/setting" + "gitea.dev/modules/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -48,18 +50,36 @@ func TestIssueList_LoadIsRead(t *testing.T) { } func TestIssueList_LoadAttributes(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - setting.Service.EnableTimetracking = true - issueList := issues_model.IssueList{ + require.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.Service.EnableTimetracking, true)() + fixtureIssues := issues_model.IssueList{ unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}), + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}), unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 4}), } + issueList := issues_model.IssueList{fixtureIssues[0]} + for i := range db.DefaultMaxInSize { + issueList = append(issueList, &issues_model.Issue{ID: int64(1000 + i), RepoID: 1, IsPull: true}) + } + issueList = append(issueList, fixtureIssues[1:]...) - assert.NoError(t, issueList.LoadAttributes(t.Context())) - for _, issue := range issueList { + require.NoError(t, issueList.LoadAttributes(t.Context())) + require.NoError(t, issueList.LoadAttachments(t.Context())) + require.NoError(t, issueList.LoadComments(t.Context())) + assert.Len(t, fixtureIssues[0].Labels, 1) + assert.Len(t, fixtureIssues[1].Labels, 2) + assert.Len(t, fixtureIssues[0].Assignees, 1) + assert.Empty(t, fixtureIssues[1].Assignees) + assert.Len(t, fixtureIssues[0].Attachments, 1) + assert.Len(t, fixtureIssues[1].Attachments, 1) + assert.Len(t, fixtureIssues[2].Attachments, 2) + assert.Len(t, fixtureIssues[0].Comments, 3) + assert.Len(t, fixtureIssues[1].Comments, 4) + assert.Len(t, fixtureIssues[2].Comments, 1) + for _, issue := range fixtureIssues { assert.Equal(t, issue.RepoID, issue.Repo.ID) for _, label := range issue.Labels { - assert.Equal(t, issue.RepoID, label.RepoID) + assert.Equal(t, unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: label.ID}), label) unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: label.ID}) } if issue.PosterID > 0 { @@ -72,7 +92,9 @@ func TestIssueList_LoadAttributes(t *testing.T) { assert.Equal(t, issue.MilestoneID, issue.Milestone.ID) } if issue.IsPull { + require.NotNil(t, issue.PullRequest) assert.Equal(t, issue.ID, issue.PullRequest.IssueID) + assert.Same(t, issue, issue.PullRequest.Issue) } for _, attachment := range issue.Attachments { assert.Equal(t, issue.ID, attachment.IssueID) @@ -82,10 +104,24 @@ func TestIssueList_LoadAttributes(t *testing.T) { } if issue.ID == int64(1) { assert.Equal(t, int64(400), issue.TotalTrackedTime) - assert.NotEmpty(t, issue.Projects) + } + if issue.ID == 1 || issue.ID == 2 { + require.Len(t, issue.Projects, 1) assert.Equal(t, int64(1), issue.Projects[0].ID) } else { assert.Empty(t, issue.Projects) } } + assert.Equal(t, int64(3682), fixtureIssues[1].TotalTrackedTime) + + require.NoError(t, issueList.LoadDiscussComments(t.Context())) + assert.Len(t, fixtureIssues[0].Comments, 2) + assert.Empty(t, fixtureIssues[1].Comments) + assert.Len(t, fixtureIssues[2].Comments, 1) + for _, issue := range fixtureIssues { + for _, comment := range issue.Comments { + assert.Equal(t, issue.ID, comment.IssueID) + assert.Equal(t, issues_model.CommentTypeComment, comment.Type) + } + } } diff --git a/models/issues/milestone_list.go b/models/issues/milestone_list.go index 3b4bdc6e9f6..8e5d48396bb 100644 --- a/models/issues/milestone_list.go +++ b/models/issues/milestone_list.go @@ -113,36 +113,29 @@ func (milestones MilestoneList) LoadTotalTrackedTimes(ctx context.Context) error trackedTimes := make(map[int64]int64, len(milestones)) // Get total tracked time by milestone_id - rows, err := db.GetEngine(ctx).Table("issue"). + var totalTimesByMilestones []*totalTimesByMilestone + err := db.GetEngine(ctx).Table("issue"). Join("INNER", "milestone", "issue.milestone_id = milestone.id"). Join("LEFT", "tracked_time", "tracked_time.issue_id = issue.id"). Where("tracked_time.deleted = ?", false). Select("milestone_id, sum(time) as time"). In("milestone_id", milestones.getMilestoneIDs()). GroupBy("milestone_id"). - Rows(new(totalTimesByMilestone)) + Find(&totalTimesByMilestones) if err != nil { return err } - defer rows.Close() - - for rows.Next() { - var totalTime totalTimesByMilestone - err = rows.Scan(&totalTime) - if err != nil { - return err - } + for _, totalTime := range totalTimesByMilestones { trackedTimes[totalTime.MilestoneID] = totalTime.Time } - for _, milestone := range milestones { milestone.TotalTrackedTime = trackedTimes[milestone.ID] } return nil } -// CountMilestonesByRepoCondAndKw map from repo conditions and the keyword of milestones' name to number of milestones matching the options` +// CountMilestonesMap map from repo conditions and the keyword of milestones' name to number of milestones matching the options` func CountMilestonesMap(ctx context.Context, opts FindMilestoneOptions) (map[int64]int64, error) { sess := db.GetEngine(ctx).Where(opts.ToConds()) diff --git a/models/user/user_list.go b/models/user/user_list.go index f639a1f71a6..7c26ded6516 100644 --- a/models/user/user_list.go +++ b/models/user/user_list.go @@ -15,17 +15,15 @@ func GetUsersMapByIDs(ctx context.Context, userIDs []int64) (map[int64]*User, er return userMaps, nil } - left := len(userIDs) - for left > 0 { - limit := min(left, db.DefaultMaxInSize) - err := db.GetEngine(ctx). - In("id", userIDs[:limit]). - Find(&userMaps) - if err != nil { - return nil, err - } - left -= limit - userIDs = userIDs[limit:] + err := db.QueryAndProcessByChunk(userIDs, func(chunk []int64) (out []*User, err error) { + err = db.GetEngine(ctx).In("id", chunk).Find(&out) + return out, err + }, func(user *User) error { + userMaps[user.ID] = user + return nil + }) + if err != nil { + return nil, err } return userMaps, nil }