github/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-06-01 20:45:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Do not call "git diff" when listing PRs (#33817)

Browse Source 
Fix  #31492
This commit is contained in:
wxiaoguang 2025-03-08 15:41:51 +08:00 committed by GitHub
parent 16a332464d
commit 869ee4fc38
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 31 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
modules/structs/pull.go
View File

@ -27,9 +27,10 @@ type PullRequest struct {
Comments int `json:"comments"` Comments int `json:"comments"`
// number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR) // number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR)
ReviewComments int `json:"review_comments"` ReviewComments int `json:"review_comments"`
Additions int `json:"additions"`
Deletions int `json:"deletions"` Additions *int `json:"additions,omitempty"`
ChangedFiles int `json:"changed_files"` Deletions *int `json:"deletions,omitempty"`
ChangedFiles *int `json:"changed_files,omitempty"`
HTMLURL string `json:"html_url"` HTMLURL string `json:"html_url"`
DiffURL string `json:"diff_url"` DiffURL string `json:"diff_url"`

20
services/convert/pull.go
View File

@ -239,9 +239,11 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
// Calculate diff // Calculate diff
startCommitID = pr.MergeBase startCommitID = pr.MergeBase
apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID) diffChangedFiles, diffAdditions, diffDeletions, err := gitRepo.GetDiffShortStat(startCommitID, endCommitID)
if err != nil { if err != nil {
log.Error("GetDiffShortStat: %v", err) log.Error("GetDiffShortStat: %v", err)
} else {
apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions = &diffChangedFiles, &diffAdditions, &diffDeletions
} }
} }
@ -459,12 +461,6 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
return nil, err return nil, err
} }
// Outer scope variables to be used in diff calculation
var (
startCommitID string
endCommitID string
)
if git.IsErrBranchNotExist(err) { if git.IsErrBranchNotExist(err) {
headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref)
if err != nil && !git.IsErrNotExist(err) { if err != nil && !git.IsErrNotExist(err) {
@ -473,7 +469,6 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
} }
if err == nil { if err == nil {
apiPullRequest.Head.Sha = headCommitID apiPullRequest.Head.Sha = headCommitID
endCommitID = headCommitID
} }
} else { } else {
commit, err := headBranch.GetCommit() commit, err := headBranch.GetCommit()
@ -484,17 +479,8 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
if err == nil { if err == nil {
apiPullRequest.Head.Ref = pr.HeadBranch apiPullRequest.Head.Ref = pr.HeadBranch
apiPullRequest.Head.Sha = commit.ID.String() apiPullRequest.Head.Sha = commit.ID.String()
endCommitID = commit.ID.String()
} }
} }
// Calculate diff
startCommitID = pr.MergeBase
apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID)
if err != nil {
log.Error("GetDiffShortStat: %v", err)
}
} }
if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 { if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 {

40
tests/integration/api_pull_test.go
View File

@ -51,7 +51,6 @@ func TestAPIViewPulls(t *testing.T) {
assert.Empty(t, pull.RequestedReviewersTeams) assert.Empty(t, pull.RequestedReviewersTeams)
assert.EqualValues(t, 5, pull.RequestedReviewers[0].ID) assert.EqualValues(t, 5, pull.RequestedReviewers[0].ID)
assert.EqualValues(t, 6, pull.RequestedReviewers[1].ID) assert.EqualValues(t, 6, pull.RequestedReviewers[1].ID)
assert.EqualValues(t, 1, pull.ChangedFiles)
if assert.EqualValues(t, 5, pull.ID) { if assert.EqualValues(t, 5, pull.ID) {
resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK) resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK)
@ -59,22 +58,23 @@ func TestAPIViewPulls(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "") patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "")
assert.NoError(t, err) assert.NoError(t, err)
if assert.Len(t, patch.Files, pull.ChangedFiles) { if assert.Len(t, patch.Files, 1) {
assert.Equal(t, "File-WoW", patch.Files[0].Name) assert.Equal(t, "File-WoW", patch.Files[0].Name)
// FIXME: The old name should be empty if it's a file add type // FIXME: The old name should be empty if it's a file add type
assert.Equal(t, "File-WoW", patch.Files[0].OldName) assert.Equal(t, "File-WoW", patch.Files[0].OldName)
assert.EqualValues(t, pull.Additions, patch.Files[0].Addition) assert.EqualValues(t, 1, patch.Files[0].Addition)
assert.EqualValues(t, pull.Deletions, patch.Files[0].Deletion) assert.EqualValues(t, 0, patch.Files[0].Deletion)
assert.Equal(t, gitdiff.DiffFileAdd, patch.Files[0].Type) assert.Equal(t, gitdiff.DiffFileAdd, patch.Files[0].Type)
} }
t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID), t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID),
doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) { doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) {
if assert.Len(t, files, pull.ChangedFiles) { if assert.Len(t, files, 1) {
assert.Equal(t, "File-WoW", files[0].Filename) assert.Equal(t, "File-WoW", files[0].Filename)
assert.Empty(t, files[0].PreviousFilename) assert.Empty(t, files[0].PreviousFilename)
assert.EqualValues(t, pull.Additions, files[0].Additions) assert.EqualValues(t, 1, files[0].Additions)
assert.EqualValues(t, pull.Deletions, files[0].Deletions) assert.EqualValues(t, 1, files[0].Changes)
assert.EqualValues(t, 0, files[0].Deletions)
assert.Equal(t, "added", files[0].Status) assert.Equal(t, "added", files[0].Status)
} }
})) }))
@ -88,7 +88,6 @@ func TestAPIViewPulls(t *testing.T) {
assert.EqualValues(t, 4, pull.RequestedReviewers[1].ID) assert.EqualValues(t, 4, pull.RequestedReviewers[1].ID)
assert.EqualValues(t, 2, pull.RequestedReviewers[2].ID) assert.EqualValues(t, 2, pull.RequestedReviewers[2].ID)
assert.EqualValues(t, 5, pull.RequestedReviewers[3].ID) assert.EqualValues(t, 5, pull.RequestedReviewers[3].ID)
assert.EqualValues(t, 1, pull.ChangedFiles)
if assert.EqualValues(t, 2, pull.ID) { if assert.EqualValues(t, 2, pull.ID) {
resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK) resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK)
@ -96,45 +95,44 @@ func TestAPIViewPulls(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "") patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "")
assert.NoError(t, err) assert.NoError(t, err)
if assert.Len(t, patch.Files, pull.ChangedFiles) { if assert.Len(t, patch.Files, 1) {
assert.Equal(t, "README.md", patch.Files[0].Name) assert.Equal(t, "README.md", patch.Files[0].Name)
assert.Equal(t, "README.md", patch.Files[0].OldName) assert.Equal(t, "README.md", patch.Files[0].OldName)
assert.EqualValues(t, pull.Additions, patch.Files[0].Addition) assert.EqualValues(t, 4, patch.Files[0].Addition)
assert.EqualValues(t, pull.Deletions, patch.Files[0].Deletion) assert.EqualValues(t, 1, patch.Files[0].Deletion)
assert.Equal(t, gitdiff.DiffFileChange, patch.Files[0].Type) assert.Equal(t, gitdiff.DiffFileChange, patch.Files[0].Type)
} }
t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID), t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID),
doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) { doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) {
if assert.Len(t, files, pull.ChangedFiles) { if assert.Len(t, files, 1) {
assert.Equal(t, "README.md", files[0].Filename) assert.Equal(t, "README.md", files[0].Filename)
// FIXME: The PreviousFilename name should be the same as Filename if it's a file change // FIXME: The PreviousFilename name should be the same as Filename if it's a file change
assert.Equal(t, "", files[0].PreviousFilename) assert.Equal(t, "", files[0].PreviousFilename)
assert.EqualValues(t, pull.Additions, files[0].Additions) assert.EqualValues(t, 4, files[0].Additions)
assert.EqualValues(t, pull.Deletions, files[0].Deletions) assert.EqualValues(t, 1, files[0].Deletions)
assert.Equal(t, "changed", files[0].Status) assert.Equal(t, "changed", files[0].Status)
} }
})) }))
} }
pull = pulls[2] pull = pulls[0]
assert.EqualValues(t, 1, pull.Poster.ID) assert.EqualValues(t, 1, pull.Poster.ID)
assert.Len(t, pull.RequestedReviewers, 1) assert.Len(t, pull.RequestedReviewers, 2)
assert.Empty(t, pull.RequestedReviewersTeams) assert.Empty(t, pull.RequestedReviewersTeams)
assert.EqualValues(t, 1, pull.RequestedReviewers[0].ID) assert.EqualValues(t, 5, pull.RequestedReviewers[0].ID)
assert.EqualValues(t, 0, pull.ChangedFiles)
if assert.EqualValues(t, 1, pull.ID) { if assert.EqualValues(t, 5, pull.ID) {
resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK) resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK)
bs, err := io.ReadAll(resp.Body) bs, err := io.ReadAll(resp.Body)
assert.NoError(t, err) assert.NoError(t, err)
patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "") patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "")
assert.NoError(t, err) assert.NoError(t, err)
assert.EqualValues(t, pull.ChangedFiles, patch.NumFiles) assert.EqualValues(t, 1, patch.NumFiles)
t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID), t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID),
doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) { doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) {
assert.Len(t, files, pull.ChangedFiles) assert.Len(t, files, 1)
})) }))
} }
} }

7
tests/integration/repo_webhook_test.go
View File

@ -25,6 +25,7 @@ import (
"github.com/PuerkitoBio/goquery" "github.com/PuerkitoBio/goquery"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
func TestNewWebHookLink(t *testing.T) { func TestNewWebHookLink(t *testing.T) {
@ -378,12 +379,14 @@ func Test_WebhookPullRequest(t *testing.T) {
// 3. validate the webhook is triggered // 3. validate the webhook is triggered
assert.EqualValues(t, "pull_request", triggeredEvent) assert.EqualValues(t, "pull_request", triggeredEvent)
assert.Len(t, payloads, 1) require.Len(t, payloads, 1)
assert.EqualValues(t, "repo1", payloads[0].PullRequest.Base.Repository.Name) assert.EqualValues(t, "repo1", payloads[0].PullRequest.Base.Repository.Name)
assert.EqualValues(t, "user2/repo1", payloads[0].PullRequest.Base.Repository.FullName) assert.EqualValues(t, "user2/repo1", payloads[0].PullRequest.Base.Repository.FullName)
assert.EqualValues(t, "repo1", payloads[0].PullRequest.Head.Repository.Name) assert.EqualValues(t, "repo1", payloads[0].PullRequest.Head.Repository.Name)
assert.EqualValues(t, "user2/repo1", payloads[0].PullRequest.Head.Repository.FullName) assert.EqualValues(t, "user2/repo1", payloads[0].PullRequest.Head.Repository.FullName)
assert.EqualValues(t, 0, payloads[0].PullRequest.Additions) assert.EqualValues(t, 0, *payloads[0].PullRequest.Additions)
assert.EqualValues(t, 0, *payloads[0].PullRequest.ChangedFiles)
assert.EqualValues(t, 0, *payloads[0].PullRequest.Deletions)
}) })
} }