Skip to content

Commit d3083d2

Browse files
lunnylunny
authored
Some small refactors (#33144)
1 parent e5f3c16 commit d3083d2

File tree

5 files changed

+92
-48
lines changed

5 files changed

+92
-48
lines changed

Diff for: ‎models/issues/comment_list.go

+17-5
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ func (comments CommentList) LoadPosters(ctx context.Context) error {
2626
return c.PosterID, c.Poster == nil && c.PosterID > 0
2727
})
2828

29-
posterMaps, err := getPostersByIDs(ctx, posterIDs)
29+
posterMaps, err := user_model.GetUsersMapByIDs(ctx, posterIDs)
3030
if err != nil {
3131
return err
3232
}
3333

3434
for _, comment := range comments {
3535
if comment.Poster == nil {
36-
comment.Poster = getPoster(comment.PosterID, posterMaps)
36+
comment.Poster = user_model.GetPossibleUserFromMap(comment.PosterID, posterMaps)
3737
}
3838
}
3939
return nil
4040
}
4141

4242
func (comments CommentList) getLabelIDs() []int64 {
4343
return container.FilterSlice(comments, func(comment *Comment) (int64, bool) {
44-
return comment.LabelID, comment.LabelID > 0
44+
return comment.LabelID, comment.LabelID > 0 && comment.Label == nil
4545
})
4646
}
4747

@@ -51,6 +51,9 @@ func (comments CommentList) loadLabels(ctx context.Context) error {
5151
}
5252

5353
labelIDs := comments.getLabelIDs()
54+
if len(labelIDs) == 0 {
55+
return nil
56+
}
5457
commentLabels := make(map[int64]*Label, len(labelIDs))
5558
left := len(labelIDs)
5659
for left > 0 {
@@ -118,8 +121,8 @@ func (comments CommentList) loadMilestones(ctx context.Context) error {
118121
milestoneIDs = milestoneIDs[limit:]
119122
}
120123

121-
for _, issue := range comments {
122-
issue.Milestone = milestoneMaps[issue.MilestoneID]
124+
for _, comment := range comments {
125+
comment.Milestone = milestoneMaps[comment.MilestoneID]
123126
}
124127
return nil
125128
}
@@ -175,6 +178,9 @@ func (comments CommentList) loadAssignees(ctx context.Context) error {
175178
}
176179

177180
assigneeIDs := comments.getAssigneeIDs()
181+
if len(assigneeIDs) == 0 {
182+
return nil
183+
}
178184
assignees := make(map[int64]*user_model.User, len(assigneeIDs))
179185
left := len(assigneeIDs)
180186
for left > 0 {
@@ -301,6 +307,9 @@ func (comments CommentList) loadDependentIssues(ctx context.Context) error {
301307

302308
e := db.GetEngine(ctx)
303309
issueIDs := comments.getDependentIssueIDs()
310+
if len(issueIDs) == 0 {
311+
return nil
312+
}
304313
issues := make(map[int64]*Issue, len(issueIDs))
305314
left := len(issueIDs)
306315
for left > 0 {
@@ -427,6 +436,9 @@ func (comments CommentList) loadReviews(ctx context.Context) error {
427436
}
428437

429438
reviewIDs := comments.getReviewIDs()
439+
if len(reviewIDs) == 0 {
440+
return nil
441+
}
430442
reviews := make(map[int64]*Review, len(reviewIDs))
431443
if err := db.GetEngine(ctx).In("id", reviewIDs).Find(&reviews); err != nil {
432444
return err

Diff for: ‎models/issues/issue.go

+3
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ func (issue *Issue) loadCommentsByType(ctx context.Context, tp CommentType) (err
238238
IssueID: issue.ID,
239239
Type: tp,
240240
})
241+
for _, comment := range issue.Comments {
242+
comment.Issue = issue
243+
}
241244
return err
242245
}
243246

Diff for: ‎models/issues/issue_list.go

+2-36
Original file line numberDiff line numberDiff line change
@@ -81,53 +81,19 @@ func (issues IssueList) LoadPosters(ctx context.Context) error {
8181
return issue.PosterID, issue.Poster == nil && issue.PosterID > 0
8282
})
8383

84-
posterMaps, err := getPostersByIDs(ctx, posterIDs)
84+
posterMaps, err := user_model.GetUsersMapByIDs(ctx, posterIDs)
8585
if err != nil {
8686
return err
8787
}
8888

8989
for _, issue := range issues {
9090
if issue.Poster == nil {
91-
issue.Poster = getPoster(issue.PosterID, posterMaps)
91+
issue.Poster = user_model.GetPossibleUserFromMap(issue.PosterID, posterMaps)
9292
}
9393
}
9494
return nil
9595
}
9696

97-
func getPostersByIDs(ctx context.Context, posterIDs []int64) (map[int64]*user_model.User, error) {
98-
posterMaps := make(map[int64]*user_model.User, len(posterIDs))
99-
left := len(posterIDs)
100-
for left > 0 {
101-
limit := db.DefaultMaxInSize
102-
if left < limit {
103-
limit = left
104-
}
105-
err := db.GetEngine(ctx).
106-
In("id", posterIDs[:limit]).
107-
Find(&posterMaps)
108-
if err != nil {
109-
return nil, err
110-
}
111-
left -= limit
112-
posterIDs = posterIDs[limit:]
113-
}
114-
return posterMaps, nil
115-
}
116-
117-
func getPoster(posterID int64, posterMaps map[int64]*user_model.User) *user_model.User {
118-
if posterID == user_model.ActionsUserID {
119-
return user_model.NewActionsUser()
120-
}
121-
if posterID <= 0 {
122-
return nil
123-
}
124-
poster, ok := posterMaps[posterID]
125-
if !ok {
126-
return user_model.NewGhostUser()
127-
}
128-
return poster
129-
}
130-
13197
func (issues IssueList) getIssueIDs() []int64 {
13298
ids := make([]int64, 0, len(issues))
13399
for _, issue := range issues {

Diff for: ‎models/user/user_list.go

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package user
5+
6+
import (
7+
"context"
8+
9+
"code.gitea.io/gitea/models/db"
10+
)
11+
12+
func GetUsersMapByIDs(ctx context.Context, userIDs []int64) (map[int64]*User, error) {
13+
userMaps := make(map[int64]*User, len(userIDs))
14+
left := len(userIDs)
15+
for left > 0 {
16+
limit := db.DefaultMaxInSize
17+
if left < limit {
18+
limit = left
19+
}
20+
err := db.GetEngine(ctx).
21+
In("id", userIDs[:limit]).
22+
Find(&userMaps)
23+
if err != nil {
24+
return nil, err
25+
}
26+
left -= limit
27+
userIDs = userIDs[limit:]
28+
}
29+
return userMaps, nil
30+
}
31+
32+
func GetPossibleUserFromMap(userID int64, usererMaps map[int64]*User) *User {
33+
switch userID {
34+
case GhostUserID:
35+
return NewGhostUser()
36+
case ActionsUserID:
37+
return NewActionsUser()
38+
case 0:
39+
return nil
40+
default:
41+
user, ok := usererMaps[userID]
42+
if !ok {
43+
return NewGhostUser()
44+
}
45+
return user
46+
}
47+
}

Diff for: ‎routers/web/repo/issue_view.go

+23-7
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,30 @@ import (
4040
)
4141

4242
// roleDescriptor returns the role descriptor for a comment in/with the given repo, poster and issue
43-
func roleDescriptor(ctx stdCtx.Context, repo *repo_model.Repository, poster *user_model.User, issue *issues_model.Issue, hasOriginalAuthor bool) (issues_model.RoleDescriptor, error) {
43+
func roleDescriptor(ctx stdCtx.Context, repo *repo_model.Repository, poster *user_model.User, permsCache map[int64]access_model.Permission, issue *issues_model.Issue, hasOriginalAuthor bool) (issues_model.RoleDescriptor, error) {
4444
roleDescriptor := issues_model.RoleDescriptor{}
4545

4646
if hasOriginalAuthor {
4747
return roleDescriptor, nil
4848
}
4949

50-
perm, err := access_model.GetUserRepoPermission(ctx, repo, poster)
51-
if err != nil {
52-
return roleDescriptor, err
50+
var perm access_model.Permission
51+
var err error
52+
if permsCache != nil {
53+
var ok bool
54+
perm, ok = permsCache[poster.ID]
55+
if !ok {
56+
perm, err = access_model.GetUserRepoPermission(ctx, repo, poster)
57+
if err != nil {
58+
return roleDescriptor, err
59+
}
60+
}
61+
permsCache[poster.ID] = perm
62+
} else {
63+
perm, err = access_model.GetUserRepoPermission(ctx, repo, poster)
64+
if err != nil {
65+
return roleDescriptor, err
66+
}
5367
}
5468

5569
// If the poster is the actual poster of the issue, enable Poster role.
@@ -576,6 +590,8 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
576590
return
577591
}
578592

593+
permCache := make(map[int64]access_model.Permission)
594+
579595
for _, comment = range issue.Comments {
580596
comment.Issue = issue
581597

@@ -593,7 +609,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
593609
continue
594610
}
595611

596-
comment.ShowRole, err = roleDescriptor(ctx, issue.Repo, comment.Poster, issue, comment.HasOriginalAuthor())
612+
comment.ShowRole, err = roleDescriptor(ctx, issue.Repo, comment.Poster, permCache, issue, comment.HasOriginalAuthor())
597613
if err != nil {
598614
ctx.ServerError("roleDescriptor", err)
599615
return
@@ -691,7 +707,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
691707
continue
692708
}
693709

694-
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, issue, c.HasOriginalAuthor())
710+
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor())
695711
if err != nil {
696712
ctx.ServerError("roleDescriptor", err)
697713
return
@@ -940,7 +956,7 @@ func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) {
940956
ctx.ServerError("RenderString", err)
941957
return
942958
}
943-
if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil {
959+
if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, nil, issue, issue.HasOriginalAuthor()); err != nil {
944960
ctx.ServerError("roleDescriptor", err)
945961
return
946962
}

0 commit comments

Comments
 (0)