Skip to content

Commit

Permalink
Some small refactors (#33144)
Browse files Browse the repository at this point in the history
  • Loading branch information
lunny authored Jan 10, 2025
1 parent e5f3c16 commit d3083d2
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 48 deletions.
22 changes: 17 additions & 5 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.GetPossibleUserFromMap(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 @@ -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.GetPossibleUserFromMap(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
47 changes: 47 additions & 0 deletions models/user/user_list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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 GetPossibleUserFromMap(userID int64, usererMaps map[int64]*User) *User {
switch userID {
case GhostUserID:
return NewGhostUser()
case ActionsUserID:
return NewActionsUser()
case 0:
return nil
default:
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

0 comments on commit d3083d2

Please sign in to comment.