fix(models): fix incorrect or unnecessary "scan rows" (#39516)

The six batched loaders in `IssueList` iterate their rows and never call
`rows.Err()`.

Refactored the code by a new helper function.

---------

Signed-off-by: Calvin Tjoaquinn <calvintjoa23@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

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{}

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})

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)
 	}

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
+}

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 {

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) {

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 (<issue ids in current page>) 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 {

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)
+		}
+	}
 }

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())
 

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
 }