From 7986d6ed22f7b08d1937f7d0a21a304b2db26d30 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sun, 13 May 2018 13:24:19 +0200 Subject: [PATCH] Add support for invalidating comments Signed-off-by: Jonas Franz --- models/git_diff_test.go | 2 +- models/issue_comment.go | 84 +++++++++++++++++++++++--- models/pull.go | 59 ++++++++++++++++-- modules/auth/repo_form.go | 11 ++-- routers/repo/pull_review.go | 5 +- vendor/code.gitea.io/git/repo_blame.go | 14 +++++ vendor/vendor.json | 7 ++- 7 files changed, 154 insertions(+), 28 deletions(-) diff --git a/models/git_diff_test.go b/models/git_diff_test.go index e0a2cab735..e4f2b7fcd8 100644 --- a/models/git_diff_test.go +++ b/models/git_diff_test.go @@ -48,7 +48,7 @@ func TestDiff_LoadComments(t *testing.T) { { Lines: []*DiffLine{ { - LeftIdx: 4, + LeftIdx: 4, RightIdx: 4, }, }, diff --git a/models/issue_comment.go b/models/issue_comment.go index 8d3d0c3064..144a2dda22 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -6,8 +6,10 @@ package models import ( "fmt" + "path" "strings" + "code.gitea.io/git" "code.gitea.io/gitea/modules/markup/markdown" "github.com/Unknwon/com" "github.com/go-xorm/builder" @@ -122,8 +124,9 @@ type Comment struct { // For view issue page. ShowTag CommentTag `xorm:"-"` - Review *Review `xorm:"-"` - ReviewID int64 + Review *Review `xorm:"-"` + ReviewID int64 + Invalidated bool } // AfterLoad is invoked from XORM after setting the values of all fields of this object. @@ -339,6 +342,40 @@ func (c *Comment) LoadReview() error { return c.loadReview(x) } +func (c *Comment) getPathAndFile(repoPath string) (string, string) { + p := path.Dir(c.TreePath) + if p == "." { + p = "" + } + p = fmt.Sprintf("%s/%s", repoPath, p) + file := path.Base(c.TreePath) + return p, file +} + +func (c *Comment) checkInvalidation(e Engine, repo *git.Repository, branch string) error { + p, file := c.getPathAndFile(repo.Path) + // FIXME differentiate between previous and proposed line + var line = c.Line + if line < 0 { + line *= -1 + } + commit, err := repo.LineBlame(branch, p, file, uint(line)) + if err != nil { + return err + } + if c.CommitSHA != commit.ID.String() { + c.Invalidated = true + return UpdateComment(c) + } + return nil +} + +// CheckInvalidation checks if the line of code comment got changed by another commit. +// If the line got changed the comment is going to be invalidated. +func (c *Comment) CheckInvalidation(repo *git.Repository, branch string) error { + return c.checkInvalidation(x, repo, branch) +} + func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { var LabelID int64 if opts.Label != nil { @@ -622,7 +659,29 @@ func CreateIssueComment(doer *User, repo *Repository, issue *Issue, content stri } // CreateCodeComment creates a plain code comment at the specified line / path -func CreateCodeComment(doer *User, repo *Repository, issue *Issue, commitSHA, content, treePath string, line, reviewID int64) (*Comment, error) { +func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, treePath string, line, reviewID int64) (*Comment, error) { + pr, err := GetPullRequestByIssueID(issue.ID) + if err != nil { + return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err) + } + if err := pr.GetHeadRepo(); err != nil { + return nil, fmt.Errorf("GetHeadRepo: %v", err) + } + gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + return nil, fmt.Errorf("OpenRepository: %v", err) + } + dummyComment := &Comment{Line: line, TreePath: treePath} + p, file := dummyComment.getPathAndFile(gitRepo.Path) + // FIXME differentiate between previous and proposed line + var gitLine = line + if gitLine < 0 { + gitLine *= -1 + } + commit, err := gitRepo.LineBlame(pr.HeadBranch, p, file, uint(gitLine)) + if err != nil { + return nil, err + } return CreateComment(&CreateCommentOptions{ Type: CommentTypeCode, Doer: doer, @@ -631,7 +690,7 @@ func CreateCodeComment(doer *User, repo *Repository, issue *Issue, commitSHA, co Content: content, LineNum: line, TreePath: treePath, - CommitSHA: commitSHA, + CommitSHA: commit.ID.String(), ReviewID: reviewID, }) } @@ -792,14 +851,21 @@ func DeleteComment(comment *Comment) error { func fetchCodeComments(e Engine, issue *Issue, currentUser *User) (map[string]map[int64][]*Comment, error) { pathToLineToComment := make(map[string]map[int64][]*Comment) - comments, err := findComments(e, FindCommentsOptions{ + + //Find comments + opts := FindCommentsOptions{ Type: CommentTypeCode, IssueID: issue.ID, - }) - if err != nil { + } + var comments []*Comment + if err := e.Where(opts.toConds().And(builder.Eq{"invalidated": false})). + Asc("comment.created_unix"). + Asc("comment.id"). + Find(&comments); err != nil { return nil, err } - if err = issue.loadRepo(e); err != nil { + + if err := issue.loadRepo(e); err != nil { return nil, err } // Find all reviews by ReviewID @@ -810,7 +876,7 @@ func fetchCodeComments(e Engine, issue *Issue, currentUser *User) (map[string]ma ids = append(ids, comment.ReviewID) } } - if err = e.In("id", ids).Find(&reviews); err != nil { + if err := e.In("id", ids).Find(&reviews); err != nil { return nil, err } for _, comment := range comments { diff --git a/models/pull.go b/models/pull.go index 5f4a6e2054..23b3888aec 100644 --- a/models/pull.go +++ b/models/pull.go @@ -1063,10 +1063,7 @@ func (prs PullRequestList) loadAttributes(e Engine) error { } // Load issues. - issueIDs := make([]int64, 0, len(prs)) - for i := range prs { - issueIDs = append(issueIDs, prs[i].IssueID) - } + issueIDs := prs.getIssueIDs() issues := make([]*Issue, 0, len(issueIDs)) if err := e. Where("id > 0"). @@ -1085,11 +1082,44 @@ func (prs PullRequestList) loadAttributes(e Engine) error { return nil } +func (prs PullRequestList) getIssueIDs() []int64 { + issueIDs := make([]int64, 0, len(prs)) + for i := range prs { + issueIDs = append(issueIDs, prs[i].IssueID) + } + return issueIDs +} + // LoadAttributes load all the prs attributes func (prs PullRequestList) LoadAttributes() error { return prs.loadAttributes(x) } +func (prs PullRequestList) invalidateCodeComments(e Engine, repo *git.Repository, branch string) error { + if len(prs) == 0 { + return nil + } + issueIDs := prs.getIssueIDs() + var codeComments []*Comment + if err := e. + Where("type = ? and invalidated = ?", CommentTypeCode, false). + In("issue_id", issueIDs). + Find(&codeComments); err != nil { + return fmt.Errorf("find code comments: %v", err) + } + for _, comment := range codeComments { + if err := comment.CheckInvalidation(repo, branch); err != nil { + return err + } + } + return nil +} + +// InvalidateCodeComments will lookup the prs for code comments which got invalidated by change +func (prs PullRequestList) InvalidateCodeComments(repo *git.Repository, branch string) error { + return prs.invalidateCodeComments(x, repo, branch) +} + func addHeadRepoTasks(prs []*PullRequest) { for _, pr := range prs { log.Trace("addHeadRepoTasks[%d]: composing new test task", pr.ID) @@ -1116,10 +1146,29 @@ func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool } if isSync { - if err = PullRequestList(prs).LoadAttributes(); err != nil { + requests := PullRequestList(prs) + if err = requests.LoadAttributes(); err != nil { log.Error(4, "PullRequestList.LoadAttributes: %v", err) } + var gitRepo *git.Repository + repo, err := GetRepositoryByID(repoID) + if err != nil { + log.Error(4, "GetRepositoryByID: %v", err) + goto REQUIRED_PROCEDURE + } + gitRepo, err = git.OpenRepository(repo.RepoPath()) + if err != nil { + log.Error(4, "git.OpenRepository: %v", err) + goto REQUIRED_PROCEDURE + } + go func() { + err := requests.InvalidateCodeComments(gitRepo, branch) + if err != nil { + log.Error(4, "PullRequestList.InvalidateCodeComments: %v", err) + } + }() + REQUIRED_PROCEDURE: if err == nil { for _, pr := range prs { pr.Issue.PullRequest = pr diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 80ae8bbaa9..4d433484d8 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -358,12 +358,11 @@ func (f *MergePullRequestForm) Validate(ctx *macaron.Context, errs binding.Error // CodeCommentForm form for adding code comments for PRs type CodeCommentForm struct { - Content string `binding:"Required"` - Side string `binding:"Required;In(previous,proposed)"` - Line int64 - TreePath string `form:"path" binding:"Required"` - CommitSHA string `form:"commit_id" binding:"Required"` - IsReview bool `form:"is_review"` + Content string `binding:"Required"` + Side string `binding:"Required;In(previous,proposed)"` + Line int64 + TreePath string `form:"path" binding:"Required"` + IsReview bool `form:"is_review"` } // Validate validates the fields diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go index a7c6ef3724..2dad56c060 100644 --- a/routers/repo/pull_review.go +++ b/routers/repo/pull_review.go @@ -63,14 +63,11 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) { } } } - //FIXME check if line, commit and treepath exist - var err error - comment, err = models.CreateCodeComment( + comment, err := models.CreateCodeComment( ctx.User, issue.Repo, issue, - form.CommitSHA, form.Content, form.TreePath, signedLine, diff --git a/vendor/code.gitea.io/git/repo_blame.go b/vendor/code.gitea.io/git/repo_blame.go index b48cbeea6c..43c6a21c62 100644 --- a/vendor/code.gitea.io/git/repo_blame.go +++ b/vendor/code.gitea.io/git/repo_blame.go @@ -4,7 +4,21 @@ package git +import "fmt" + // FileBlame return the Blame object of file func (repo *Repository) FileBlame(revision, path, file string) ([]byte, error) { return NewCommand("blame", "--root", file).RunInDirBytes(path) } + +// LineBlame returns the latest commit at the given line +func (repo *Repository) LineBlame(revision, path, file string, line uint) (*Commit, error) { + res, err := NewCommand("blame", fmt.Sprintf("-L %d,%d", line, line), "-p", revision, file).RunInDir(path) + if err != nil { + return nil, err + } + if len(res) < 40 { + return nil, fmt.Errorf("invalid result of blame: %s", res) + } + return repo.GetCommit(string(res[:40])) +} diff --git a/vendor/vendor.json b/vendor/vendor.json index e4f9dd5c92..d829618110 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -3,10 +3,11 @@ "ignore": "test appengine", "package": [ { - "checksumSHA1": "BfL4Z7P1alyUUNspKJu7Q4GPCNs=", + "checksumSHA1": "jkAY8qJRd3N2isGPpoCMoq+QkBc=", + "origin": "github.com/JonasFranzDEV/git", "path": "code.gitea.io/git", - "revision": "f1ecc138bebcffed32be1a574ed0c2701b33733f", - "revisionTime": "2018-04-21T01:08:19Z" + "revision": "575c3983fb275c7e87906a781ace9d97e8f4071d", + "revisionTime": "2018-05-13T11:02:42Z" }, { "checksumSHA1": "WMD6+Qh2+5hd9uiq910pF/Ihylw=",