From c919d68997fd69243a118c66b3ad4440fd115cf5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 29 Feb 2024 15:43:59 +0800 Subject: [PATCH 1/3] Fix comment attachments haven't updated the database column --- models/issues/comment.go | 6 ++++-- models/issues/issue_update.go | 9 ++++----- services/repository/repository.go | 7 +++++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index c7b22f3cca073..fdc49c09294ed 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -870,6 +870,9 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } fallthrough case CommentTypeComment: + if err = updateAttachments(ctx, opts, comment); err != nil { + return err + } if _, err = db.Exec(ctx, "UPDATE `issue` SET num_comments=num_comments+1 WHERE id=?", opts.Issue.ID); err != nil { return err } @@ -895,8 +898,7 @@ func updateAttachments(ctx context.Context, opts *CreateCommentOptions, comment for i := range attachments { attachments[i].IssueID = opts.Issue.ID attachments[i].CommentID = comment.ID - // No assign value could be 0, so ignore AllCols(). - if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil { + if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Cols("issue_id", "comment_id").Update(attachments[i]); err != nil { return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err) } } diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index b258dc882cb2c..43d26fcb741fa 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -282,7 +282,6 @@ type NewIssueOptions struct { // NewIssueWithIndex creates issue with given index func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssueOptions) (err error) { - e := db.GetEngine(ctx) opts.Issue.Title = strings.TrimSpace(opts.Issue.Title) if opts.Issue.MilestoneID > 0 { @@ -306,7 +305,7 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue return fmt.Errorf("issue exist") } - if _, err := e.Insert(opts.Issue); err != nil { + if err := db.Insert(ctx, opts.Issue); err != nil { return err } @@ -336,7 +335,7 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue // During the session, SQLite3 driver cannot handle retrieve objects after update something. // So we have to get all needed labels first. labels := make([]*Label, 0, len(opts.LabelIDs)) - if err = e.In("id", opts.LabelIDs).Find(&labels); err != nil { + if err = db.GetEngine(ctx).In("id", opts.LabelIDs).Find(&labels); err != nil { return fmt.Errorf("find all labels [label_ids: %v]: %w", opts.LabelIDs, err) } @@ -368,8 +367,8 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue for i := 0; i < len(attachments); i++ { attachments[i].IssueID = opts.Issue.ID - if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Cols("issue_id").Update(attachments[i]); err != nil { + return fmt.Errorf("update attachment issue_id [id: %d]: %w", attachments[i].ID, err) } } } diff --git a/services/repository/repository.go b/services/repository/repository.go index d28200c0adc5d..a4c8d23487e4f 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -124,7 +124,7 @@ func UpdateRepository(ctx context.Context, repo *repo_model.Repository, visibili // LinkedRepository returns the linked repo if any func LinkedRepository(ctx context.Context, a *repo_model.Attachment) (*repo_model.Repository, unit.Type, error) { - if a.IssueID != 0 { + if a.IssueID != 0 { // attachment for issues or comments iss, err := issues_model.GetIssueByID(ctx, a.IssueID) if err != nil { return nil, unit.TypeIssues, err @@ -135,13 +135,16 @@ func LinkedRepository(ctx context.Context, a *repo_model.Attachment) (*repo_mode unitType = unit.TypePullRequests } return repo, unitType, err - } else if a.ReleaseID != 0 { + } else if a.ReleaseID != 0 { // attachment for releases rel, err := repo_model.GetReleaseByID(ctx, a.ReleaseID) if err != nil { return nil, unit.TypeReleases, err } repo, err := repo_model.GetRepositoryByID(ctx, rel.RepoID) return repo, unit.TypeReleases, err + } else if a.RepoID > 0 { // attachment for repository upload + repo, err := repo_model.GetRepositoryByID(ctx, a.RepoID) + return repo, unit.TypeCode, err } return nil, -1, nil } From fc32838a2acead6bd66b615d245c2547253d8208 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 4 Mar 2024 11:55:01 +0800 Subject: [PATCH 2/3] parse attachments from markdown text --- models/issues/comment.go | 23 +------- models/issues/comment_attach.go | 44 +++++++++++++++ modules/markup/markdown/attachment.go | 65 ++++++++++++++++++++++ modules/markup/markdown/attachment_test.go | 30 ++++++++++ routers/web/repo/issue.go | 12 +--- services/issue/comments.go | 65 +++++++++++++++++++--- 6 files changed, 199 insertions(+), 40 deletions(-) create mode 100644 models/issues/comment_attach.go create mode 100644 modules/markup/markdown/attachment.go create mode 100644 modules/markup/markdown/attachment_test.go diff --git a/models/issues/comment.go b/models/issues/comment.go index 6a4d2088589bc..99d893f21fccc 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -576,28 +576,6 @@ func (c *Comment) LoadAttachments(ctx context.Context) error { return nil } -// UpdateAttachments update attachments by UUIDs for the comment -func (c *Comment) UpdateAttachments(ctx context.Context, uuids []string) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) - } - for i := 0; i < len(attachments); i++ { - attachments[i].IssueID = c.IssueID - attachments[i].CommentID = c.ID - if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) - } - } - return committer.Commit() -} - // LoadAssigneeUserAndTeam if comment.Type is CommentTypeAssignees, then load assignees func (c *Comment) LoadAssigneeUserAndTeam(ctx context.Context) error { var err error @@ -1127,6 +1105,7 @@ func UpdateComment(ctx context.Context, c *Comment, doer *user_model.User) error if _, err := sess.ID(c.ID).AllCols().Update(c); err != nil { return err } + if err := c.LoadIssue(ctx); err != nil { return err } diff --git a/models/issues/comment_attach.go b/models/issues/comment_attach.go new file mode 100644 index 0000000000000..7ac0f176a6713 --- /dev/null +++ b/models/issues/comment_attach.go @@ -0,0 +1,44 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" +) + +// UpdateAttachments update attachments by UUIDs for the comment +func UpdateAttachments(ctx context.Context, c *Comment, uuids []string) error { + ctx, committer, err := db.TxContext(ctx) + if err != nil { + return err + } + defer committer.Close() + + attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) + if err != nil { + return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) + } + for i := 0; i < len(attachments); i++ { + attachments[i].IssueID = c.IssueID + attachments[i].CommentID = c.ID + if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { + return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + } + } + return committer.Commit() +} + +// ClearCommentAttaches clear attachments' comment information by UUIDs for the comment +func ClearCommentAttaches(ctx context.Context, comment *Comment, uuids []string) error { + _, err := db.GetEngine(ctx). + Where("issue_id = ?", comment.IssueID). + And("comment_id = ?", comment.ID). + In("uuid", uuids). + Cols("issue_id", "comment_id").Update(&repo_model.Attachment{}) + return err +} diff --git a/modules/markup/markdown/attachment.go b/modules/markup/markdown/attachment.go new file mode 100644 index 0000000000000..228775d9f81f9 --- /dev/null +++ b/modules/markup/markdown/attachment.go @@ -0,0 +1,65 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markdown + +import ( + "strings" + + "code.gitea.io/gitea/modules/container" + + "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" + "github.com/yuin/goldmark/text" + "golang.org/x/net/html" +) + +func ParseAttachments(content string) (container.Set[string], error) { + parser := goldmark.DefaultParser() + rd := text.NewReader([]byte(content)) + doc := parser.Parse(rd) + var attachments []string + if err := ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { + if !entering { + return ast.WalkContinue, nil + } + switch b := n.(type) { + case *ast.HTMLBlock: + var tag string + for i := 0; i < b.Lines().Len(); i++ { + p := content[b.Lines().At(i).Start:b.Lines().At(i).Stop] + tag += p + } + doc, err := html.Parse(strings.NewReader(strings.TrimSpace(tag))) + if err != nil { + return ast.WalkStop, err + } + + var processAllImgs func(*html.Node) + processAllImgs = func(n *html.Node) { + if n.Type == html.ElementNode && n.Data == "img" { + for _, a := range n.Attr { + if a.Key == "src" && strings.HasPrefix(a.Val, "/attachments/") { + attachments = append(attachments, strings.TrimPrefix(a.Val, "/attachments/")) + return + } + } + } + // traverse the child nodes + for c := n.FirstChild; c != nil; c = c.NextSibling { + processAllImgs(c) + } + } + // make a recursive call to your function + processAllImgs(doc) + case *ast.Image: + if strings.HasPrefix(string(b.Destination), "/attachments/") { + attachments = append(attachments, strings.TrimPrefix(string(b.Destination), "/attachments/")) + } + } + return ast.WalkContinue, nil + }); err != nil { + return nil, err + } + return container.SetOf(attachments...), nil +} diff --git a/modules/markup/markdown/attachment_test.go b/modules/markup/markdown/attachment_test.go new file mode 100644 index 0000000000000..2880964a2532f --- /dev/null +++ b/modules/markup/markdown/attachment_test.go @@ -0,0 +1,30 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markdown + +import ( + "testing" + + "code.gitea.io/gitea/modules/container" + + "github.com/stretchr/testify/assert" +) + +func Test_ParseAttachments(t *testing.T) { + attachments, err := ParseAttachments(` +# This is a test +![test](/attachments/08058e33-d916-432a-88b9-5f616cfe9f01) + +## free + +image +`) + if err != nil { + t.Fatal(err) + } + assert.EqualValues(t, container.Set[string]{ + "08058e33-d916-432a-88b9-5f616cfe9f01": struct{}{}, + "08058e33-d916-432a-88b9-5f616cfe9f00": struct{}{}, + }, attachments) +} diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index ebaa955ac8057..ba70111b4c9aa 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -3143,7 +3143,7 @@ func UpdateCommentContent(ctx *context.Context) { }) return } - if err = issue_service.UpdateComment(ctx, comment, ctx.Doer, oldContent); err != nil { + if err = issue_service.UpdateComment(ctx, comment, ctx.Doer, oldContent, ctx.FormStrings("files[]"), ctx.FormBool("ignore_attachments")); err != nil { ctx.ServerError("UpdateComment", err) return } @@ -3153,14 +3153,6 @@ func UpdateCommentContent(ctx *context.Context) { return } - // when the update request doesn't intend to update attachments (eg: change checkbox state), ignore attachment updates - if !ctx.FormBool("ignore_attachments") { - if err := updateAttachments(ctx, comment, ctx.FormStrings("files[]")); err != nil { - ctx.ServerError("UpdateAttachments", err) - return - } - } - content, err := markdown.RenderString(&markup.RenderContext{ Links: markup.Links{ Base: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? @@ -3527,7 +3519,7 @@ func updateAttachments(ctx *context.Context, item any, files []string) error { case *issues_model.Issue: err = issues_model.UpdateIssueAttachments(ctx, content.ID, files) case *issues_model.Comment: - err = content.UpdateAttachments(ctx, files) + err = issues_model.UpdateAttachments(ctx, content, files) default: return fmt.Errorf("unknown Type: %T", content) } diff --git a/services/issue/comments.go b/services/issue/comments.go index 8d8c575c14039..db0b662de5db6 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -11,6 +11,8 @@ import ( issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" + "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/timeutil" notify_service "code.gitea.io/gitea/services/notify" ) @@ -68,8 +70,22 @@ func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_m return comment, nil } +func getDiffAttaches(oldAttaches, newAttaches container.Set[string]) (addedAttaches, delAttaches container.Set[string]) { + for attach := range newAttaches { + if !oldAttaches.Contains(attach) { + addedAttaches.Add(attach) + } + } + for attach := range oldAttaches { + if !newAttaches.Contains(attach) { + delAttaches.Add(attach) + } + } + return +} + // UpdateComment updates information of comment. -func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_model.User, oldContent string) error { +func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_model.User, oldContent string, attachments []string, ignoreUpdateAttachments bool) error { needsContentHistory := c.Content != oldContent && c.Type.HasContentSupport() if needsContentHistory { hasContentHistory, err := issues_model.HasIssueContentHistory(ctx, c.IssueID, c.ID) @@ -84,15 +100,48 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_mode } } - if err := issues_model.UpdateComment(ctx, c, doer); err != nil { - return err - } - - if needsContentHistory { - err := issues_model.SaveIssueContentHistory(ctx, doer.ID, c.IssueID, c.ID, timeutil.TimeStampNow(), c.Content, false) - if err != nil { + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := issues_model.UpdateComment(ctx, c, doer); err != nil { return err } + + if !ignoreUpdateAttachments { + oldAttaches, err := markdown.ParseAttachments(oldContent) + if err != nil { + return err + } + + newAttaches, err := markdown.ParseAttachments(c.Content) + if err != nil { + return err + } + + addedAttaches, delAttaches := getDiffAttaches(oldAttaches, newAttaches) + for _, attach := range addedAttaches.Values() { + attachments = append(attachments, attach) + } + + if len(delAttaches) > 0 { + if err := issues_model.ClearCommentAttaches(ctx, c, delAttaches.Values()); err != nil { + return err + } + } + + // when the update request doesn't intend to update attachments (eg: change checkbox state), ignore attachment updates + if err := updateAttachments(ctx, c, attachments); err != nil { + return err + } + } + + if needsContentHistory { + err := issues_model.SaveIssueContentHistory(ctx, doer.ID, c.IssueID, c.ID, timeutil.TimeStampNow(), c.Content, false) + if err != nil { + return err + } + } + return nil + }); err != nil { + return err } notify_service.UpdateComment(ctx, doer, c, oldContent) From 4225fdfb5431d0e121ec298238b6e9a0533c8c58 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 4 Mar 2024 16:22:10 +0800 Subject: [PATCH 3/3] Fix bug --- routers/api/v1/repo/issue_comment.go | 2 +- routers/api/v1/repo/issue_comment_attachment.go | 2 +- services/issue/comments.go | 16 +++++++++++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 763419b7a2520..f06fb30889047 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -609,7 +609,7 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) oldContent := comment.Content comment.Content = form.Body - if err := issue_service.UpdateComment(ctx, comment, ctx.Doer, oldContent); err != nil { + if err := issue_service.UpdateComment(ctx, comment, ctx.Doer, oldContent, nil, true); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateComment", err) return } diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index e7436db7982a7..6ef5cd8ebea35 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -198,7 +198,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { return } - if err = issue_service.UpdateComment(ctx, comment, ctx.Doer, comment.Content); err != nil { + if err = issue_service.UpdateComment(ctx, comment, ctx.Doer, comment.Content, []string{attachment.UUID}, false); err != nil { ctx.ServerError("UpdateComment", err) return } diff --git a/services/issue/comments.go b/services/issue/comments.go index db0b662de5db6..2ead446dbe4f7 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -81,7 +81,7 @@ func getDiffAttaches(oldAttaches, newAttaches container.Set[string]) (addedAttac delAttaches.Add(attach) } } - return + return addedAttaches, delAttaches } // UpdateComment updates information of comment. @@ -105,7 +105,7 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_mode return err } - if !ignoreUpdateAttachments { + if ignoreUpdateAttachments { oldAttaches, err := markdown.ParseAttachments(oldContent) if err != nil { return err @@ -117,7 +117,7 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_mode } addedAttaches, delAttaches := getDiffAttaches(oldAttaches, newAttaches) - for _, attach := range addedAttaches.Values() { + for attach := range addedAttaches { attachments = append(attachments, attach) } @@ -127,8 +127,14 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_mode } } - // when the update request doesn't intend to update attachments (eg: change checkbox state), ignore attachment updates - if err := updateAttachments(ctx, c, attachments); err != nil { + // update all these attachments's issue_id or comment_id if it's zero + // warning don't update those attachments with issue_id and comment_id not zero + // because those maybe copied from other respositories. + if _, err := db.GetEngine(ctx).Where("issue_id = 0 AND comment_id = 0"). + In("uuid", attachments).Cols("issue_id, comment_id").Update(&repo_model.Attachment{ + IssueID: c.IssueID, + CommentID: c.ID, + }); err != nil { return err } }