Skip to content

Commit 46fa3f5

Browse files
lunnyGiteaBot
authored andcommitted
Fix bug when API get pull changed files for deleted head repository (go-gitea#34333)
1 parent 6c5f0af commit 46fa3f5

File tree

2 files changed

+101
-1
lines changed

2 files changed

+101
-1
lines changed

routers/api/v1/repo/pull.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1632,7 +1632,9 @@ func GetPullRequestFiles(ctx *context.APIContext) {
16321632

16331633
apiFiles := make([]*api.ChangedFile, 0, limit)
16341634
for i := start; i < start+limit; i++ {
1635-
apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i], pr.HeadRepo, endCommitID))
1635+
// refs/pull/1/head stores the HEAD commit ID, allowing all related commits to be found in the base repository.
1636+
// The head repository might have been deleted, so we should not rely on it here.
1637+
apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i], pr.BaseRepo, endCommitID))
16361638
}
16371639

16381640
ctx.SetLinkHeader(totalNumberOfFiles, listOptions.PageSize)

tests/integration/api_pull_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import (
88
"fmt"
99
"io"
1010
"net/http"
11+
"net/url"
12+
"strings"
1113
"testing"
14+
"time"
1215

1316
auth_model "code.gitea.io/gitea/models/auth"
1417
"code.gitea.io/gitea/models/db"
@@ -17,11 +20,15 @@ import (
1720
repo_model "code.gitea.io/gitea/models/repo"
1821
"code.gitea.io/gitea/models/unittest"
1922
user_model "code.gitea.io/gitea/models/user"
23+
"code.gitea.io/gitea/modules/git"
2024
"code.gitea.io/gitea/modules/setting"
2125
api "code.gitea.io/gitea/modules/structs"
26+
"code.gitea.io/gitea/services/convert"
2227
"code.gitea.io/gitea/services/forms"
2328
"code.gitea.io/gitea/services/gitdiff"
2429
issue_service "code.gitea.io/gitea/services/issue"
30+
pull_service "code.gitea.io/gitea/services/pull"
31+
files_service "code.gitea.io/gitea/services/repository/files"
2532
"code.gitea.io/gitea/tests"
2633

2734
"github.com/stretchr/testify/assert"
@@ -424,3 +431,94 @@ func TestAPICommitPullRequest(t *testing.T) {
424431
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/commits/%s/pull", owner.Name, repo.Name, invalidCommitSHA).AddTokenAuth(ctx.Token)
425432
ctx.Session.MakeRequest(t, req, http.StatusNotFound)
426433
}
434+
435+
func TestAPIViewPullFilesWithHeadRepoDeleted(t *testing.T) {
436+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
437+
baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
438+
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
439+
440+
ctx := NewAPITestContext(t, "user1", baseRepo.Name, auth_model.AccessTokenScopeAll)
441+
442+
doAPIForkRepository(ctx, "user2")(t)
443+
444+
forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ForkID: baseRepo.ID, OwnerName: "user1"})
445+
446+
// add a new file to the forked repo
447+
addFileToForkedResp, err := files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user1, &files_service.ChangeRepoFilesOptions{
448+
Files: []*files_service.ChangeRepoFile{
449+
{
450+
Operation: "create",
451+
TreePath: "file_1.txt",
452+
ContentReader: strings.NewReader("file1"),
453+
},
454+
},
455+
Message: "add file1",
456+
OldBranch: "master",
457+
NewBranch: "fork-branch-1",
458+
Author: &files_service.IdentityOptions{
459+
GitUserName: user1.Name,
460+
GitUserEmail: user1.Email,
461+
},
462+
Committer: &files_service.IdentityOptions{
463+
GitUserName: user1.Name,
464+
GitUserEmail: user1.Email,
465+
},
466+
Dates: &files_service.CommitDateOptions{
467+
Author: time.Now(),
468+
Committer: time.Now(),
469+
},
470+
})
471+
assert.NoError(t, err)
472+
assert.NotEmpty(t, addFileToForkedResp)
473+
474+
// create Pull
475+
pullIssue := &issues_model.Issue{
476+
RepoID: baseRepo.ID,
477+
Title: "Test pull-request-target-event",
478+
PosterID: user1.ID,
479+
Poster: user1,
480+
IsPull: true,
481+
}
482+
pullRequest := &issues_model.PullRequest{
483+
HeadRepoID: forkedRepo.ID,
484+
BaseRepoID: baseRepo.ID,
485+
HeadBranch: "fork-branch-1",
486+
BaseBranch: "master",
487+
HeadRepo: forkedRepo,
488+
BaseRepo: baseRepo,
489+
Type: issues_model.PullRequestGitea,
490+
}
491+
492+
prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest}
493+
err = pull_service.NewPullRequest(git.DefaultContext, prOpts)
494+
assert.NoError(t, err)
495+
pr := convert.ToAPIPullRequest(t.Context(), pullRequest, user1)
496+
497+
ctx = NewAPITestContext(t, "user2", baseRepo.Name, auth_model.AccessTokenScopeAll)
498+
doAPIGetPullFiles(ctx, pr, func(t *testing.T, files []*api.ChangedFile) {
499+
if assert.Len(t, files, 1) {
500+
assert.Equal(t, "file_1.txt", files[0].Filename)
501+
assert.Empty(t, files[0].PreviousFilename)
502+
assert.Equal(t, 1, files[0].Additions)
503+
assert.Equal(t, 1, files[0].Changes)
504+
assert.Equal(t, 0, files[0].Deletions)
505+
assert.Equal(t, "added", files[0].Status)
506+
}
507+
})(t)
508+
509+
// delete the head repository of the pull request
510+
forkCtx := NewAPITestContext(t, "user1", forkedRepo.Name, auth_model.AccessTokenScopeAll)
511+
doAPIDeleteRepository(forkCtx)(t)
512+
513+
doAPIGetPullFiles(ctx, pr, func(t *testing.T, files []*api.ChangedFile) {
514+
if assert.Len(t, files, 1) {
515+
assert.Equal(t, "file_1.txt", files[0].Filename)
516+
assert.Empty(t, files[0].PreviousFilename)
517+
assert.Equal(t, 1, files[0].Additions)
518+
assert.Equal(t, 1, files[0].Changes)
519+
assert.Equal(t, 0, files[0].Deletions)
520+
assert.Equal(t, "added", files[0].Status)
521+
}
522+
})(t)
523+
})
524+
}

0 commit comments

Comments
 (0)