Skip to content

Commit

Permalink
Check PR changed files count directly from get pull request endpoint (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dblinkhorn authored Nov 19, 2024
1 parent 3987077 commit 8e8a129
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 3 deletions.
13 changes: 10 additions & 3 deletions pull/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func (loc Locator) IsComplete() bool {
case loc.Value.GetHead().GetRepo().GetID() == 0:
case loc.Value.GetHead().GetRepo().GetName() == "":
case loc.Value.GetHead().GetRepo().GetOwner().GetLogin() == "":
case loc.Value.GetChangedFiles() == 0:
default:
return true
}
Expand Down Expand Up @@ -108,6 +109,7 @@ func (loc Locator) toV4(ctx context.Context, client *githubv4.Client) (*v4PullRe
v4.BaseRefName = loc.Value.GetBase().GetRef()
v4.BaseRepository.DatabaseID = loc.Value.GetBase().GetRepo().GetID()
v4.IsDraft = loc.Value.GetDraft()
v4.ChangedFiles = loc.Value.GetChangedFiles()
return &v4, nil
}

Expand Down Expand Up @@ -270,6 +272,11 @@ func (ghc *GitHubContext) Branches() (base string, head string) {
}

func (ghc *GitHubContext) ChangedFiles() ([]*File, error) {
// Check if changed files exceeds the limit
if ghc.pr.ChangedFiles > MaxPullRequestFiles {
return nil, errors.Errorf("number of changed files (%d) exceeds limit (%d)", ghc.pr.ChangedFiles, MaxPullRequestFiles)
}

if ghc.files == nil {
opt := github.ListOptions{
PerPage: 100,
Expand Down Expand Up @@ -318,9 +325,7 @@ func (ghc *GitHubContext) ChangedFiles() ([]*File, error) {
})
}
}
if len(ghc.files) >= MaxPullRequestFiles {
return nil, errors.Errorf("too many files in pull request, maximum is %d", MaxPullRequestFiles)
}

return ghc.files, nil
}

Expand Down Expand Up @@ -1109,6 +1114,8 @@ type v4PullRequest struct {
BaseRepository struct {
DatabaseID int64
}

ChangedFiles int
}

type v4PageInfo struct {
Expand Down
22 changes: 22 additions & 0 deletions pull/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,27 @@ func TestChangedFiles(t *testing.T) {
assert.Equal(t, 2, filesRule.Count, "cached files were not used")
}

func TestChangedFilesExceedsLimit(t *testing.T) {
rp := &ResponsePlayer{}
rp.AddRule(
ExactPathMatcher("/repos/testorg/testrepo/pulls/123/files"),
"testdata/responses/pull_files.yml",
)
rp.AddRule(
GraphQLNodePrefixMatcher("repository.pullRequest.changedFiles"),
"testdata/responses/pull_changed_files_exceeds_limit.yml",
)

pr := defaultTestPR()
*pr.ChangedFiles = 3001

ctx := makeContext(t, rp, pr, nil)

_, err := ctx.ChangedFiles()
assert.Equal(t, 3001, pr.GetChangedFiles())
assert.Contains(t, err.Error(), "number of changed files (3001) exceeds limit (3000)")
}

func TestChangedFilesNoFiles(t *testing.T) {
rp := &ResponsePlayer{}
filesRule := rp.AddRule(
Expand Down Expand Up @@ -723,6 +744,7 @@ func defaultTestPR() *github.PullRequest {
Name: github.String("testrepo"),
},
},
ChangedFiles: github.Int(1),
}
}

Expand Down
18 changes: 18 additions & 0 deletions pull/testdata/responses/pull_changed_files_exceeds_limit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
- status: 200
body: |
{
"errors": [],
"data": {
"repository": {
"pullRequest": {
"body": "/no-platform",
"createdAt": "2018-01-12T00:11:50Z",
"lastEditedAt": "2018-01-12T00:11:50Z",
"author": {
"login": "agirlnamedsophia"
},
"changedFiles": 3001
}
}
}
}

0 comments on commit 8e8a129

Please sign in to comment.