Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some small refactors #33144

Merged
merged 8 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions models/issues/comment_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ func (comments CommentList) LoadPosters(ctx context.Context) error {
return c.PosterID, c.Poster == nil && c.PosterID > 0
})

posterMaps, err := getPostersByIDs(ctx, posterIDs)
posterMaps, err := user_model.GetUsersMapByIDs(ctx, posterIDs)
if err != nil {
return err
}

for _, comment := range comments {
if comment.Poster == nil {
comment.Poster = getPoster(comment.PosterID, posterMaps)
comment.Poster = user_model.MustGetUserFromMap(comment.PosterID, posterMaps)
}
}
return nil
}

func (comments CommentList) getLabelIDs() []int64 {
return container.FilterSlice(comments, func(comment *Comment) (int64, bool) {
return comment.LabelID, comment.LabelID > 0
return comment.LabelID, comment.LabelID > 0 && comment.Label == nil
})
}

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

labelIDs := comments.getLabelIDs()
if len(labelIDs) == 0 {
return nil
}
commentLabels := make(map[int64]*Label, len(labelIDs))
left := len(labelIDs)
for left > 0 {
Expand Down Expand Up @@ -118,8 +121,8 @@ func (comments CommentList) loadMilestones(ctx context.Context) error {
milestoneIDs = milestoneIDs[limit:]
}

for _, issue := range comments {
issue.Milestone = milestoneMaps[issue.MilestoneID]
for _, comment := range comments {
comment.Milestone = milestoneMaps[comment.MilestoneID]
}
return nil
}
Expand Down Expand Up @@ -175,6 +178,9 @@ func (comments CommentList) loadAssignees(ctx context.Context) error {
}

assigneeIDs := comments.getAssigneeIDs()
if len(assigneeIDs) == 0 {
return nil
}
assignees := make(map[int64]*user_model.User, len(assigneeIDs))
left := len(assigneeIDs)
for left > 0 {
Expand Down Expand Up @@ -301,6 +307,9 @@ func (comments CommentList) loadDependentIssues(ctx context.Context) error {

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 {
Expand Down Expand Up @@ -417,7 +426,7 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) {

func (comments CommentList) getReviewIDs() []int64 {
return container.FilterSlice(comments, func(comment *Comment) (int64, bool) {
return comment.ReviewID, comment.ReviewID > 0
return comment.ReviewID, comment.ReviewID > 0 && comment.Review == nil
})
}

Expand All @@ -427,6 +436,9 @@ func (comments CommentList) loadReviews(ctx context.Context) error {
}

reviewIDs := comments.getReviewIDs()
if len(reviewIDs) == 0 {
return nil
}
reviews := make(map[int64]*Review, len(reviewIDs))
if err := db.GetEngine(ctx).In("id", reviewIDs).Find(&reviews); err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ func (issue *Issue) loadCommentsByType(ctx context.Context, tp CommentType) (err
IssueID: issue.ID,
Type: tp,
})
for _, comment := range issue.Comments {
comment.Issue = issue
}
return err
}

Expand Down
38 changes: 2 additions & 36 deletions models/issues/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,53 +81,19 @@ func (issues IssueList) LoadPosters(ctx context.Context) error {
return issue.PosterID, issue.Poster == nil && issue.PosterID > 0
})

posterMaps, err := getPostersByIDs(ctx, posterIDs)
posterMaps, err := user_model.GetUsersMapByIDs(ctx, posterIDs)
if err != nil {
return err
}

for _, issue := range issues {
if issue.Poster == nil {
issue.Poster = getPoster(issue.PosterID, posterMaps)
issue.Poster = user_model.MustGetUserFromMap(issue.PosterID, posterMaps)
}
}
return nil
}

func getPostersByIDs(ctx context.Context, posterIDs []int64) (map[int64]*user_model.User, error) {
posterMaps := make(map[int64]*user_model.User, len(posterIDs))
left := len(posterIDs)
for left > 0 {
limit := db.DefaultMaxInSize
if left < limit {
limit = left
}
err := db.GetEngine(ctx).
In("id", posterIDs[:limit]).
Find(&posterMaps)
if err != nil {
return nil, err
}
left -= limit
posterIDs = posterIDs[limit:]
}
return posterMaps, nil
}

func getPoster(posterID int64, posterMaps map[int64]*user_model.User) *user_model.User {
if posterID == user_model.ActionsUserID {
return user_model.NewActionsUser()
}
if posterID <= 0 {
return nil
}
poster, ok := posterMaps[posterID]
if !ok {
return user_model.NewGhostUser()
}
return poster
}

func (issues IssueList) getIssueIDs() []int64 {
ids := make([]int64, 0, len(issues))
for _, issue := range issues {
Expand Down
44 changes: 44 additions & 0 deletions models/user/user_list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package user

import (
"context"

"code.gitea.io/gitea/models/db"
)

func GetUsersMapByIDs(ctx context.Context, userIDs []int64) (map[int64]*User, error) {
userMaps := make(map[int64]*User, len(userIDs))
left := len(userIDs)
for left > 0 {
limit := db.DefaultMaxInSize
if left < limit {
limit = left
}
err := db.GetEngine(ctx).
In("id", userIDs[:limit]).
Find(&userMaps)
if err != nil {
return nil, err
}
left -= limit
userIDs = userIDs[limit:]
}
return userMaps, nil
}

func MustGetUserFromMap(userID int64, usererMaps map[int64]*User) *User {
if userID == ActionsUserID {
return NewActionsUser()
}
if userID <= 0 {
return nil
}
user, ok := usererMaps[userID]
if !ok {
return NewGhostUser()
}
return user
}
30 changes: 23 additions & 7 deletions routers/web/repo/issue_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,30 @@ import (
)

// roleDescriptor returns the role descriptor for a comment in/with the given repo, poster and issue
func roleDescriptor(ctx stdCtx.Context, repo *repo_model.Repository, poster *user_model.User, issue *issues_model.Issue, hasOriginalAuthor bool) (issues_model.RoleDescriptor, error) {
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) {
roleDescriptor := issues_model.RoleDescriptor{}

if hasOriginalAuthor {
return roleDescriptor, nil
}

perm, err := access_model.GetUserRepoPermission(ctx, repo, poster)
if err != nil {
return roleDescriptor, err
var perm access_model.Permission
var err error
if permsCache != nil {
var ok bool
perm, ok = permsCache[poster.ID]
if !ok {
perm, err = access_model.GetUserRepoPermission(ctx, repo, poster)
if err != nil {
return roleDescriptor, err
}
}
permsCache[poster.ID] = perm
} else {
perm, err = access_model.GetUserRepoPermission(ctx, repo, poster)
if err != nil {
return roleDescriptor, err
}
}

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

permCache := make(map[int64]access_model.Permission)

for _, comment = range issue.Comments {
comment.Issue = issue

Expand All @@ -593,7 +609,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
continue
}

comment.ShowRole, err = roleDescriptor(ctx, issue.Repo, comment.Poster, issue, comment.HasOriginalAuthor())
comment.ShowRole, err = roleDescriptor(ctx, issue.Repo, comment.Poster, permCache, issue, comment.HasOriginalAuthor())
if err != nil {
ctx.ServerError("roleDescriptor", err)
return
Expand Down Expand Up @@ -691,7 +707,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
continue
}

c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, issue, c.HasOriginalAuthor())
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor())
if err != nil {
ctx.ServerError("roleDescriptor", err)
return
Expand Down Expand Up @@ -940,7 +956,7 @@ func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) {
ctx.ServerError("RenderString", err)
return
}
if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil {
if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, nil, issue, issue.HasOriginalAuthor()); err != nil {
ctx.ServerError("roleDescriptor", err)
return
}
Expand Down
Loading