Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main'
Browse files Browse the repository at this point in the history
* upstream/main:
  Add more stats tables (go-gitea#29730)
  Use Get but not Post to get actions artifacts (go-gitea#29734)
  Do some performance optimize for issues list and view issue/pull (go-gitea#29515)
  • Loading branch information
zjjhot committed Mar 12, 2024
2 parents 81d1696 + e5e2b2f commit 7b9bd02
Show file tree
Hide file tree
Showing 13 changed files with 130 additions and 65 deletions.
7 changes: 6 additions & 1 deletion models/activities/statistic.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
asymkey_model "code.gitea.io/gitea/models/asymkey"
"code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
Expand All @@ -29,7 +30,8 @@ type Statistic struct {
Mirror, Release, AuthSource, Webhook,
Milestone, Label, HookTask,
Team, UpdateTask, Project,
ProjectBoard, Attachment int64
ProjectBoard, Attachment,
Branches, Tags, CommitStatus int64
IssueByLabel []IssueByLabelCount
IssueByRepository []IssueByRepositoryCount
}
Expand Down Expand Up @@ -58,6 +60,9 @@ func GetStatistic(ctx context.Context) (stats Statistic) {
stats.Counter.Watch, _ = e.Count(new(repo_model.Watch))
stats.Counter.Star, _ = e.Count(new(repo_model.Star))
stats.Counter.Access, _ = e.Count(new(access_model.Access))
stats.Counter.Branches, _ = e.Count(new(git_model.Branch))
stats.Counter.Tags, _ = e.Where("is_draft=?", false).Count(new(repo_model.Release))
stats.Counter.CommitStatus, _ = e.Count(new(git_model.CommitStatus))

type IssueCount struct {
Count int64
Expand Down
8 changes: 2 additions & 6 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,8 @@ func (c *Comment) LoadTime(ctx context.Context) error {
return err
}

func (c *Comment) loadReactions(ctx context.Context, repo *repo_model.Repository) (err error) {
// LoadReactions loads comment reactions
func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository) (err error) {
if c.Reactions != nil {
return nil
}
Expand All @@ -691,11 +692,6 @@ func (c *Comment) loadReactions(ctx context.Context, repo *repo_model.Repository
return nil
}

// LoadReactions loads comment reactions
func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository) error {
return c.loadReactions(ctx, repo)
}

func (c *Comment) loadReview(ctx context.Context) (err error) {
if c.ReviewID == 0 {
return nil
Expand Down
2 changes: 1 addition & 1 deletion models/issues/comment_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
}

// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) ([]*Comment, error) {
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) (CommentList, error) {
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: issue.ID,
Expand Down
78 changes: 59 additions & 19 deletions models/issues/comment_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ type CommentList []*Comment
func (comments CommentList) getPosterIDs() []int64 {
posterIDs := make(container.Set[int64], len(comments))
for _, comment := range comments {
posterIDs.Add(comment.PosterID)
if comment.PosterID > 0 {
posterIDs.Add(comment.PosterID)
}
}
return posterIDs.Values()
}
Expand All @@ -41,18 +43,12 @@ func (comments CommentList) LoadPosters(ctx context.Context) error {
return nil
}

func (comments CommentList) getCommentIDs() []int64 {
ids := make([]int64, 0, len(comments))
for _, comment := range comments {
ids = append(ids, comment.ID)
}
return ids
}

func (comments CommentList) getLabelIDs() []int64 {
ids := make(container.Set[int64], len(comments))
for _, comment := range comments {
ids.Add(comment.LabelID)
if comment.LabelID > 0 {
ids.Add(comment.LabelID)
}
}
return ids.Values()
}
Expand Down Expand Up @@ -100,7 +96,9 @@ func (comments CommentList) loadLabels(ctx context.Context) error {
func (comments CommentList) getMilestoneIDs() []int64 {
ids := make(container.Set[int64], len(comments))
for _, comment := range comments {
ids.Add(comment.MilestoneID)
if comment.MilestoneID > 0 {
ids.Add(comment.MilestoneID)
}
}
return ids.Values()
}
Expand Down Expand Up @@ -141,7 +139,9 @@ func (comments CommentList) loadMilestones(ctx context.Context) error {
func (comments CommentList) getOldMilestoneIDs() []int64 {
ids := make(container.Set[int64], len(comments))
for _, comment := range comments {
ids.Add(comment.OldMilestoneID)
if comment.OldMilestoneID > 0 {
ids.Add(comment.OldMilestoneID)
}
}
return ids.Values()
}
Expand Down Expand Up @@ -182,7 +182,9 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error {
func (comments CommentList) getAssigneeIDs() []int64 {
ids := make(container.Set[int64], len(comments))
for _, comment := range comments {
ids.Add(comment.AssigneeID)
if comment.AssigneeID > 0 {
ids.Add(comment.AssigneeID)
}
}
return ids.Values()
}
Expand Down Expand Up @@ -314,7 +316,9 @@ func (comments CommentList) getDependentIssueIDs() []int64 {
if comment.DependentIssue != nil {
continue
}
ids.Add(comment.DependentIssueID)
if comment.DependentIssueID > 0 {
ids.Add(comment.DependentIssueID)
}
}
return ids.Values()
}
Expand Down Expand Up @@ -369,23 +373,57 @@ func (comments CommentList) loadDependentIssues(ctx context.Context) error {
return nil
}

// getAttachmentCommentIDs only return the comment ids which possibly has attachments
func (comments CommentList) getAttachmentCommentIDs() []int64 {
ids := make(container.Set[int64], len(comments))
for _, comment := range comments {
if comment.Type == CommentTypeComment ||
comment.Type == CommentTypeReview ||
comment.Type == CommentTypeCode {
ids.Add(comment.ID)
}
}
return ids.Values()
}

// LoadAttachmentsByIssue loads attachments by issue id
func (comments CommentList) LoadAttachmentsByIssue(ctx context.Context) error {
if len(comments) == 0 {
return nil
}

attachments := make([]*repo_model.Attachment, 0, len(comments)/2)
if err := db.GetEngine(ctx).Where("issue_id=? AND comment_id>0", comments[0].IssueID).Find(&attachments); err != nil {
return err
}

commentAttachmentsMap := make(map[int64][]*repo_model.Attachment, len(comments))
for _, attach := range attachments {
commentAttachmentsMap[attach.CommentID] = append(commentAttachmentsMap[attach.CommentID], attach)
}

for _, comment := range comments {
comment.Attachments = commentAttachmentsMap[comment.ID]
}
return nil
}

// LoadAttachments loads attachments
func (comments CommentList) LoadAttachments(ctx context.Context) (err error) {
if len(comments) == 0 {
return nil
}

attachments := make(map[int64][]*repo_model.Attachment, len(comments))
commentsIDs := comments.getCommentIDs()
commentsIDs := comments.getAttachmentCommentIDs()
left := len(commentsIDs)
for left > 0 {
limit := db.DefaultMaxInSize
if left < limit {
limit = left
}
rows, err := db.GetEngine(ctx).Table("attachment").
Join("INNER", "comment", "comment.id = attachment.comment_id").
In("comment.id", commentsIDs[:limit]).
rows, err := db.GetEngine(ctx).
In("comment_id", commentsIDs[:limit]).
Rows(new(repo_model.Attachment))
if err != nil {
return err
Expand Down Expand Up @@ -415,7 +453,9 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) {
func (comments CommentList) getReviewIDs() []int64 {
ids := make(container.Set[int64], len(comments))
for _, comment := range comments {
ids.Add(comment.ReviewID)
if comment.ReviewID > 0 {
ids.Add(comment.ReviewID)
}
}
return ids.Values()
}
Expand Down
25 changes: 22 additions & 3 deletions models/issues/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,8 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) {
if left < limit {
limit = left
}
rows, err := db.GetEngine(ctx).Table("attachment").
Join("INNER", "issue", "issue.id = attachment.issue_id").
In("issue.id", issuesIDs[:limit]).
rows, err := db.GetEngine(ctx).
In("issue_id", issuesIDs[:limit]).
Rows(new(repo_model.Attachment))
if err != nil {
return err
Expand Down Expand Up @@ -609,3 +608,23 @@ func (issues IssueList) GetApprovalCounts(ctx context.Context) (map[int64][]*Rev

return approvalCountMap, nil
}

func (issues IssueList) LoadIsRead(ctx context.Context, userID int64) error {
issueIDs := issues.getIssueIDs()
issueUsers := make([]*IssueUser, 0, len(issueIDs))
if err := db.GetEngine(ctx).Where("uid =?", userID).
In("issue_id").
Find(&issueUsers); err != nil {
return err
}

for _, issueUser := range issueUsers {
for _, issue := range issues {
if issue.ID == issueUser.IssueID {
issue.IsRead = issueUser.IsRead
}
}
}

return nil
}
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,8 @@ var migrations = []Migration{
NewMigration("Add default_wiki_branch to repository table", v1_22.AddDefaultWikiBranch),
// v290 -> v291
NewMigration("Add PayloadVersion to HookTask", v1_22.AddPayloadVersionToHookTaskTable),
// v291 -> v292
NewMigration("Add Index to attachment.comment_id", v1_22.AddCommentIDIndexofAttachment),
}

// GetCurrentDBVersion returns the current db version
Expand Down
14 changes: 14 additions & 0 deletions models/migrations/v1_22/v291.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_22 //nolint

import "xorm.io/xorm"

func AddCommentIDIndexofAttachment(x *xorm.Engine) error {
type Attachment struct {
CommentID int64 `xorm:"INDEX"`
}

return x.Sync(&Attachment{})
}
2 changes: 1 addition & 1 deletion models/repo/attachment.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Attachment struct {
IssueID int64 `xorm:"INDEX"` // maybe zero when creating
ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating
UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added
CommentID int64
CommentID int64 `xorm:"INDEX"`
Name string
DownloadCount int64 `xorm:"DEFAULT 0"`
Size int64 `xorm:"DEFAULT 0"`
Expand Down
4 changes: 0 additions & 4 deletions routers/api/v1/repo/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,6 @@ func ListRepoIssueComments(ctx *context.APIContext) {
ctx.Error(http.StatusInternalServerError, "LoadIssues", err)
return
}
if err := comments.LoadPosters(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadPosters", err)
return
}
if err := comments.LoadAttachments(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
return
Expand Down
39 changes: 17 additions & 22 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,15 +324,15 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
return
}

// Get posters.
for i := range issues {
// Check read status
if !ctx.IsSigned {
issues[i].IsRead = true
} else if err = issues[i].GetIsRead(ctx, ctx.Doer.ID); err != nil {
ctx.ServerError("GetIsRead", err)
if ctx.IsSigned {
if err := issues.LoadIsRead(ctx, ctx.Doer.ID); err != nil {
ctx.ServerError("LoadIsRead", err)
return
}
} else {
for i := range issues {
issues[i].IsRead = true
}
}

commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(ctx, issues)
Expand Down Expand Up @@ -1604,20 +1604,20 @@ func ViewIssue(ctx *context.Context) {

// Render comments and and fetch participants.
participants[0] = issue.Poster

if err := issue.Comments.LoadAttachmentsByIssue(ctx); err != nil {
ctx.ServerError("LoadAttachmentsByIssue", err)
return
}
if err := issue.Comments.LoadPosters(ctx); err != nil {
ctx.ServerError("LoadPosters", err)
return
}

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

if err := comment.LoadPoster(ctx); err != nil {
ctx.ServerError("LoadPoster", err)
return
}

if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview {
if err := comment.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}

comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{
Links: markup.Links{
Base: ctx.Repo.RepoLink,
Expand Down Expand Up @@ -1665,7 +1665,6 @@ func ViewIssue(ctx *context.Context) {
comment.Milestone = ghostMilestone
}
} else if comment.Type == issues_model.CommentTypeProject {

if err = comment.LoadProject(ctx); err != nil {
ctx.ServerError("LoadProject", err)
return
Expand Down Expand Up @@ -1731,10 +1730,6 @@ func ViewIssue(ctx *context.Context) {
for _, codeComments := range comment.Review.CodeComments {
for _, lineComments := range codeComments {
for _, c := range lineComments {
if err := c.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
// Check tag.
role, ok = marked[c.PosterID]
if ok {
Expand Down
8 changes: 3 additions & 5 deletions routers/web/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,9 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
return
}

for _, c := range comments {
if err := c.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}
if err := comments.LoadAttachments(ctx); err != nil {
ctx.ServerError("LoadAttachments", err)
return
}

ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
Expand Down
2 changes: 1 addition & 1 deletion routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ func registerRoutes(m *web.Route) {
})
m.Post("/cancel", reqRepoActionsWriter, actions.Cancel)
m.Post("/approve", reqRepoActionsWriter, actions.Approve)
m.Post("/artifacts", actions.ArtifactsView)
m.Get("/artifacts", actions.ArtifactsView)
m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView)
m.Delete("/artifacts/{artifact_name}", actions.ArtifactsDeleteView)
m.Post("/rerun", reqRepoActionsWriter, actions.Rerun)
Expand Down
Loading

0 comments on commit 7b9bd02

Please sign in to comment.