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

Move git diff codes from models to services/gitdiff #7889

Merged
merged 7 commits into from
Sep 6, 2019
Merged
Show file tree
Hide file tree
Changes from 4 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
81 changes: 0 additions & 81 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@
package models

import (
"bytes"
"fmt"
"strings"

"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"

Expand Down Expand Up @@ -488,32 +486,6 @@ func (c *Comment) UnsignedLine() uint64 {
return uint64(c.Line)
}

// AsDiff returns c.Patch as *Diff
func (c *Comment) AsDiff() (*Diff, error) {
diff, err := ParsePatch(setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch))
if err != nil {
return nil, err
}
if len(diff.Files) == 0 {
return nil, fmt.Errorf("no file found for comment ID: %d", c.ID)
}
secs := diff.Files[0].Sections
if len(secs) == 0 {
return nil, fmt.Errorf("no sections found for comment ID: %d", c.ID)
}
return diff, nil
}

// MustAsDiff executes AsDiff and logs the error instead of returning
func (c *Comment) MustAsDiff() *Diff {
diff, err := c.AsDiff()
if err != nil {
log.Warn("MustAsDiff: %v", err)
}
return diff
}

// CodeCommentURL returns the url to a comment in code
func (c *Comment) CodeCommentURL() string {
err := c.LoadIssue()
Expand Down Expand Up @@ -873,59 +845,6 @@ func CreateIssueComment(doer *User, repo *Repository, issue *Issue, content stri
return comment, nil
}

// CreateCodeComment creates a plain code comment at the specified line / path
func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, treePath string, line, reviewID int64) (*Comment, error) {
var commitID, patch string
pr, err := GetPullRequestByIssueID(issue.ID)
if err != nil {
return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err)
}
if err := pr.GetBaseRepo(); err != nil {
return nil, fmt.Errorf("GetHeadRepo: %v", err)
}
gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
return nil, fmt.Errorf("OpenRepository: %v", err)
}

// FIXME validate treePath
// Get latest commit referencing the commented line
// No need for get commit for base branch changes
if line > 0 {
commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
if err == nil {
commitID = commit.ID.String()
} else if !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") {
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
}
}

// Only fetch diff if comment is review comment
if reviewID != 0 {
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
if err != nil {
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
}
patchBuf := new(bytes.Buffer)
if err := GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, RawDiffNormal, treePath, patchBuf); err != nil {
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
}
patch = CutDiffAroundLine(patchBuf, int64((&Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
}
return CreateComment(&CreateCommentOptions{
Type: CommentTypeCode,
Doer: doer,
Repo: repo,
Issue: issue,
Content: content,
LineNum: line,
TreePath: treePath,
CommitSHA: commitID,
ReviewID: reviewID,
Patch: patch,
})
}

// CreateRefComment creates a commit reference comment to issue.
func CreateRefComment(doer *User, repo *Repository, issue *Issue, content, commitSHA string) error {
if len(commitSHA) == 0 {
Expand Down
5 changes: 5 additions & 0 deletions models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,3 +250,8 @@ func MaxBatchInsertSize(bean interface{}) int {
t := x.TableInfo(bean)
return 999 / len(t.ColumnsSeq())
}

// Count returns records number according struct's fields as database query conditions
func Count(bean interface{}) (int64, error) {
return x.Count(bean)
}
3 changes: 2 additions & 1 deletion modules/repofiles/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import (
"strings"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/services/gitdiff"
)

// GetDiffPreview produces and returns diff result of a file which is not yet committed.
func GetDiffPreview(repo *models.Repository, branch, treePath, content string) (*models.Diff, error) {
func GetDiffPreview(repo *models.Repository, branch, treePath, content string) (*gitdiff.Diff, error) {
if branch == "" {
branch = repo.DefaultBranch
}
Expand Down
9 changes: 5 additions & 4 deletions modules/repofiles/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/services/gitdiff"

"github.com/stretchr/testify/assert"
)
Expand All @@ -25,10 +26,10 @@ func TestGetDiffPreview(t *testing.T) {
treePath := "README.md"
content := "# repo1\n\nDescription for repo1\nthis is a new line"

expectedDiff := &models.Diff{
expectedDiff := &gitdiff.Diff{
TotalAddition: 2,
TotalDeletion: 1,
Files: []*models.DiffFile{
Files: []*gitdiff.DiffFile{
{
Name: "README.md",
OldName: "README.md",
Expand All @@ -42,10 +43,10 @@ func TestGetDiffPreview(t *testing.T) {
IsLFSFile: false,
IsRenamed: false,
IsSubmodule: false,
Sections: []*models.DiffSection{
Sections: []*gitdiff.DiffSection{
{
Name: "",
Lines: []*models.DiffLine{
Lines: []*gitdiff.DiffLine{
{
LeftIdx: 0,
RightIdx: 0,
Expand Down
5 changes: 3 additions & 2 deletions modules/repofiles/temp_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/gitdiff"
)

// TemporaryUploadRepository is a type to wrap our upload repositories as a shallow clone
Expand Down Expand Up @@ -290,7 +291,7 @@ func (t *TemporaryUploadRepository) Push(doer *models.User, commitHash string, b
}

// DiffIndex returns a Diff of the current index to the head
func (t *TemporaryUploadRepository) DiffIndex() (diff *models.Diff, err error) {
func (t *TemporaryUploadRepository) DiffIndex() (diff *gitdiff.Diff, err error) {
timeout := 5 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
Expand All @@ -313,7 +314,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (diff *models.Diff, err error) {
pid := process.GetManager().Add(fmt.Sprintf("diffIndex [repo_path: %s]", t.repo.RepoPath()), cmd)
defer process.GetManager().Remove(pid)

diff, err = models.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdout)
diff, err = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdout)
if err != nil {
return nil, fmt.Errorf("ParsePatch: %v", err)
}
Expand Down
2 changes: 2 additions & 0 deletions modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff"

"gopkg.in/editorconfig/editorconfig-core-go.v1"
)
Expand Down Expand Up @@ -230,6 +231,7 @@ func NewFuncMap() []template.FuncMap {
}
return float32(n) * 100 / float32(sum)
},
"CommentMustAsDiff": gitdiff.CommentMustAsDiff,
}}
}

Expand Down
7 changes: 4 additions & 3 deletions routers/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/gitdiff"
)

const (
Expand Down Expand Up @@ -217,7 +218,7 @@ func Diff(ctx *context.Context) {

ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses)

diff, err := models.GetDiffCommit(models.RepoPath(userName, repoName),
diff, err := gitdiff.GetDiffCommit(models.RepoPath(userName, repoName),
commitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
if err != nil {
Expand Down Expand Up @@ -269,10 +270,10 @@ func Diff(ctx *context.Context) {

// RawDiff dumps diff results of repository in given commit ID to io.Writer
func RawDiff(ctx *context.Context) {
if err := models.GetRawDiff(
if err := gitdiff.GetRawDiff(
models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name),
ctx.Params(":sha"),
models.RawDiffType(ctx.Params(":ext")),
gitdiff.RawDiffType(ctx.Params(":ext")),
ctx.Resp,
); err != nil {
ctx.ServerError("GetRawDiff", err)
Expand Down
3 changes: 2 additions & 1 deletion routers/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/gitdiff"
)

const (
Expand Down Expand Up @@ -230,7 +231,7 @@ func PrepareCompareDiff(
return true
}

diff, err := models.GetDiffRange(models.RepoPath(headUser.Name, headRepo.Name),
diff, err := gitdiff.GetDiffRange(models.RepoPath(headUser.Name, headRepo.Name),
compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"code.gitea.io/gitea/modules/pull"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff"

"github.com/unknwon/com"
)
Expand Down Expand Up @@ -517,7 +518,7 @@ func ViewPullFiles(ctx *context.Context) {
ctx.Data["Reponame"] = pull.HeadRepo.Name
}

diff, err := models.GetDiffRangeWithWhitespaceBehavior(diffRepoPath,
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(diffRepoPath,
startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
whitespaceFlags[ctx.Data["WhitespaceBehavior"].(string)])
Expand Down
3 changes: 2 additions & 1 deletion routers/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/notification"
pull_service "code.gitea.io/gitea/modules/pull"
comment_service "code.gitea.io/gitea/services/comments"
)

// CreateCodeComment will create a code comment including an pending review if required
Expand Down Expand Up @@ -69,7 +70,7 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) {
review.ID = form.Reply
}
//FIXME check if line, commit and treepath exist
comment, err := models.CreateCodeComment(
comment, err := comment_service.CreateCodeComment(
ctx.User,
issue.Repo,
issue,
Expand Down
69 changes: 69 additions & 0 deletions services/comments/comments.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package comments

import (
"bytes"
"fmt"
"strings"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/gitdiff"
)

// CreateCodeComment creates a plain code comment at the specified line / path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeComment is a bit too general. It could mean comments in the source code at first glance.
Maybe ReviewComment if there are no other types of comments?
If renaming it would involve extensive changes beyond this PR, no need to bother now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to leave it for another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny By the way, after realizing it's already there and this is just a refactor. I think it is not worth the effort to change the name at this point.

func CreateCodeComment(doer *models.User, repo *models.Repository, issue *models.Issue, content, treePath string, line, reviewID int64) (*models.Comment, error) {
var commitID, patch string
pr, err := models.GetPullRequestByIssueID(issue.ID)
if err != nil {
return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err)
}
if err := pr.GetBaseRepo(); err != nil {
return nil, fmt.Errorf("GetHeadRepo: %v", err)
}
gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
return nil, fmt.Errorf("OpenRepository: %v", err)
}

// FIXME validate treePath
// Get latest commit referencing the commented line
// No need for get commit for base branch changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'No need to get ... or 'No need of getting...'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move them and haven't see it's correct.

if line > 0 {
commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
if err == nil {
commitID = commit.ID.String()
} else if !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") {
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
}
}

// Only fetch diff if comment is review comment
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what circumstances reviewID would be 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move them and haven't see it's correct.

if reviewID != 0 {
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
if err != nil {
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
}
patchBuf := new(bytes.Buffer)
if err := gitdiff.GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, gitdiff.RawDiffNormal, treePath, patchBuf); err != nil {
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
}
patch = gitdiff.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
}
return models.CreateComment(&models.CreateCommentOptions{
Type: models.CommentTypeCode,
Doer: doer,
Repo: repo,
Issue: issue,
Content: content,
LineNum: line,
TreePath: treePath,
CommitSHA: commitID,
ReviewID: reviewID,
Patch: patch,
})
}
Loading