Skip to content

Add API endpoint to get changed files of a PR #18228

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 deletions modules/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"code.gitea.io/gitea/modules/log"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff"
webhook_service "code.gitea.io/gitea/services/webhook"
)

Expand Down Expand Up @@ -372,3 +373,10 @@ func ToLFSLock(l *models.LFSLock) *api.LFSLock {
},
}
}

// ToChangedFile convert a gitdiff.DiffFile to api.ChangedFile
func ToChangedFile(f *gitdiff.DiffFile) *api.ChangedFile {
return &api.ChangedFile{
Filename: f.Name,
}
}
5 changes: 5 additions & 0 deletions modules/structs/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,8 @@ type EditPullRequestOption struct {
Deadline *time.Time `json:"due_date"`
RemoveDeadline *bool `json:"unset_due_date"`
}

// ChangedFile store information about files affected by the pull request
type ChangedFile struct {
Filename string `json:"filename"`
}
1 change: 1 addition & 0 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,7 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
m.Get(".{diffType:diff|patch}", repo.DownloadPullDiffOrPatch)
m.Post("/update", reqToken(), repo.UpdatePullRequest)
m.Get("/commits", repo.GetPullRequestCommits)
m.Get("/files", repo.GetPullRequestFiles)
Copy link
Member

Choose a reason for hiding this comment

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

just use context.ReferencesGitRepo(false), to get gitRepo injected into ctx

m.Combo("/merge").Get(repo.IsPullRequestMerged).
Post(reqToken(), mustNotBeArchived, bind(forms.MergePullRequestForm{}), repo.MergePullRequest)
m.Group("/reviews", func() {
Expand Down
127 changes: 127 additions & 0 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/notification"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/api/v1/utils"
asymkey_service "code.gitea.io/gitea/services/asymkey"
"code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/gitdiff"
issue_service "code.gitea.io/gitea/services/issue"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
Expand Down Expand Up @@ -1255,3 +1257,128 @@ func GetPullRequestCommits(ctx *context.APIContext) {

ctx.JSON(http.StatusOK, &apiCommits)
}

// GetPullRequestFiles gets all changed files associated with a given PR
func GetPullRequestFiles(ctx *context.APIContext) {
// swagger:operation GET /repos/{owner}/{repo}/pulls/{index}/files repository repoGetPullRequestFiles
// ---
// summary: Get changed files for a pull request
// produces:
// - application/json
// parameters:
// - name: owner
// in: path
// description: owner of the repo
// type: string
// required: true
// - name: repo
// in: path
// description: name of the repo
// type: string
// required: true
// - name: index
// in: path
// description: index of the pull request to get
// type: integer
// format: int64
// required: true
// - name: page
// in: query
// description: page number of results to return (1-based)
// type: integer
// - name: limit
// in: query
// description: page size of results
// type: integer
// responses:
// "200":
// "$ref": "#/responses/ChangedFileList"
// "404":
// "$ref": "#/responses/notFound"

pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrPullRequestNotExist(err) {
ctx.NotFound()
} else {
ctx.Error(http.StatusInternalServerError, "GetPullRequestByIndex", err)
}
return
}

if err := pr.LoadBaseRepo(); err != nil {
ctx.InternalServerError(err)
return
}

var prInfo *git.CompareInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move this to Line 1321(after defer baseGitRepo.Close())

baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
ctx.ServerError("OpenRepository", err)
return
}
defer baseGitRepo.Close()
if pr.HasMerged {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), true, false)
} else {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true, false)
}
if err != nil {
ctx.ServerError("GetCompareInfo", err)
return
}

headCommitID, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName())
if err != nil {
ctx.ServerError("GetRefCommitID", err)
return
}

startCommitID := prInfo.MergeBase
endCommitID := headCommitID

maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles

diff, err := gitdiff.GetDiff(baseGitRepo,
&gitdiff.DiffOptions{
BeforeCommitID: startCommitID,
AfterCommitID: endCommitID,
SkipTo: ctx.FormString("skip-to"),
Copy link
Contributor

Choose a reason for hiding this comment

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

skip-to isn't mentioned as query in the swagger description.

MaxLines: maxLines,
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
MaxFiles: maxFiles,
WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
Copy link
Contributor

Choose a reason for hiding this comment

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

This will always be empty as this ctx.Data is set by SetWhitespaceBehavior middleware.

}, ctx.FormStrings("files")...)
if err != nil {
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
ctx.ServerError("GetDiff", err)

return
}

listOptions := utils.GetListOptions(ctx)

totalNumberOfFiles := diff.NumFiles
totalNumberOfPages := int(math.Ceil(float64(totalNumberOfFiles) / float64(listOptions.PageSize)))

start, end := listOptions.GetStartEnd()

if end > totalNumberOfFiles {
end = totalNumberOfFiles
Copy link
Contributor

Choose a reason for hiding this comment

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

This could cause that it becomes. start > end In which a panic will be caused on line 1368 panic: runtime error: makeslice: cap out of range. Maybe add a check if start is still smaller than end?

}

apiFiles := make([]*api.ChangedFile, 0, end-start)
for i := start; i < end; i++ {
apiFile := convert.ToChangedFile(diff.Files[i])
apiFiles = append(apiFiles, apiFile)
Comment on lines +1370 to +1371
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
apiFile := convert.ToChangedFile(diff.Files[i])
apiFiles = append(apiFiles, apiFile)
apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i]))

}

ctx.SetLinkHeader(totalNumberOfFiles, listOptions.PageSize)
ctx.SetTotalCountHeader(int64(totalNumberOfFiles))

ctx.RespHeader().Set("X-Page", strconv.Itoa(listOptions.Page))
ctx.RespHeader().Set("X-PerPage", strconv.Itoa(listOptions.PageSize))
ctx.RespHeader().Set("X-PageCount", strconv.Itoa(totalNumberOfPages))
ctx.RespHeader().Set("X-HasMore", strconv.FormatBool(listOptions.Page < totalNumberOfPages))
ctx.AppendAccessControlExposeHeaders("X-Page", "X-PerPage", "X-PageCount", "X-HasMore")

ctx.JSON(http.StatusOK, &apiFiles)
}
22 changes: 22 additions & 0 deletions routers/api/v1/swagger/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,28 @@ type swaggerCommitList struct {
Body []api.Commit `json:"body"`
}

// ChangedFileList
// swagger:response ChangedFileList
type swaggerChangedFileList struct {
// The current page
Page int `json:"X-Page"`

// Commits per page
PerPage int `json:"X-PerPage"`

// Total commit count
Total int `json:"X-Total"`

// Total number of pages
PageCount int `json:"X-PageCount"`

// True if there is another page
HasMore bool `json:"X-HasMore"`

// in: body
Files []api.ChangedFile `json:"files"`
}

// Note
// swagger:response Note
type swaggerNote struct {
Expand Down
102 changes: 102 additions & 0 deletions templates/swagger/v1_json.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -7567,6 +7567,62 @@
}
}
},
"/repos/{owner}/{repo}/pulls/{index}/files": {
"get": {
"produces": [
"application/json"
],
"tags": [
"repository"
],
"summary": "Get changed files for a pull request",
"operationId": "repoGetPullRequestFiles",
"parameters": [
{
"type": "string",
"description": "owner of the repo",
"name": "owner",
"in": "path",
"required": true
},
{
"type": "string",
"description": "name of the repo",
"name": "repo",
"in": "path",
"required": true
},
{
"type": "integer",
"format": "int64",
"description": "index of the pull request to get",
"name": "index",
"in": "path",
"required": true
},
{
"type": "integer",
"description": "page number of results to return (1-based)",
"name": "page",
"in": "query"
},
{
"type": "integer",
"description": "page size of results",
"name": "limit",
"in": "query"
}
],
"responses": {
"200": {
"$ref": "#/responses/ChangedFileList"
},
"404": {
"$ref": "#/responses/notFound"
}
}
}
},
"/repos/{owner}/{repo}/pulls/{index}/merge": {
"get": {
"produces": [
Expand Down Expand Up @@ -12914,6 +12970,17 @@
},
"x-go-package": "code.gitea.io/gitea/modules/structs"
},
"ChangedFile": {
"description": "ChangedFile store information about files affected by the pull request",
"type": "object",
"properties": {
"filename": {
"type": "string",
"x-go-name": "Filename"
}
},
"x-go-package": "code.gitea.io/gitea/modules/structs"
},
"CombinedStatus": {
"description": "CombinedStatus holds the combined state of several statuses for a single commit",
"type": "object",
Expand Down Expand Up @@ -18186,6 +18253,41 @@
}
}
},
"ChangedFileList": {
"description": "ChangedFileList",
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/ChangedFile"
}
},
"headers": {
"X-HasMore": {
"type": "boolean",
"description": "True if there is another page"
},
"X-Page": {
"type": "integer",
"format": "int64",
"description": "The current page"
},
"X-PageCount": {
"type": "integer",
"format": "int64",
"description": "Total number of pages"
},
"X-PerPage": {
"type": "integer",
"format": "int64",
"description": "Commits per page"
},
"X-Total": {
"type": "integer",
"format": "int64",
"description": "Total commit count"
}
}
},
"CombinedStatus": {
"description": "CombinedStatus",
"schema": {
Expand Down