Skip to content

Commit 39eaf76

Browse files
committed
Fix bug when API get pull changed files for deleted head repository
1 parent e67f74e commit 39eaf76

File tree

2 files changed

+102
-1
lines changed

2 files changed

+102
-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: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@ package integration
55

66
import (
77
"bytes"
8+
"context"
89
"fmt"
910
"io"
1011
"net/http"
12+
"net/url"
13+
"strings"
1114
"testing"
15+
"time"
1216

1317
auth_model "code.gitea.io/gitea/models/auth"
1418
"code.gitea.io/gitea/models/db"
@@ -17,11 +21,15 @@ import (
1721
repo_model "code.gitea.io/gitea/models/repo"
1822
"code.gitea.io/gitea/models/unittest"
1923
user_model "code.gitea.io/gitea/models/user"
24+
"code.gitea.io/gitea/modules/git"
2025
"code.gitea.io/gitea/modules/setting"
2126
api "code.gitea.io/gitea/modules/structs"
27+
"code.gitea.io/gitea/services/convert"
2228
"code.gitea.io/gitea/services/forms"
2329
"code.gitea.io/gitea/services/gitdiff"
2430
issue_service "code.gitea.io/gitea/services/issue"
31+
pull_service "code.gitea.io/gitea/services/pull"
32+
files_service "code.gitea.io/gitea/services/repository/files"
2533
"code.gitea.io/gitea/tests"
2634

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

0 commit comments

Comments
 (0)