From 1dfa28ffa557720d164a351783be64c9a47a0adb Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Thu, 29 Sep 2022 04:27:20 +0200 Subject: [PATCH] Add API endpoint to get changed files of a PR (#21177) This adds an api endpoint `/files` to PRs that allows to get a list of changed files. built upon #18228, reviews there are included closes https://github.com/go-gitea/gitea/issues/654 Co-authored-by: Anton Bracke Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang --- models/fixtures/pull_request.yml | 4 +- modules/convert/convert.go | 34 +++++++ modules/structs/pull.go | 13 +++ routers/api/v1/api.go | 1 + routers/api/v1/repo/pull.go | 136 +++++++++++++++++++++++++ routers/api/v1/swagger/repo.go | 22 ++++ templates/swagger/v1_json.tmpl | 155 +++++++++++++++++++++++++++++ tests/integration/api_pull_test.go | 48 ++++++++- 8 files changed, 407 insertions(+), 6 deletions(-) diff --git a/models/fixtures/pull_request.yml b/models/fixtures/pull_request.yml index d45baa711c..165437f032 100644 --- a/models/fixtures/pull_request.yml +++ b/models/fixtures/pull_request.yml @@ -60,8 +60,8 @@ head_repo_id: 1 base_repo_id: 1 head_branch: pr-to-update - base_branch: branch1 - merge_base: 1234567890abcdef + base_branch: branch2 + merge_base: 985f0301dba5e7b34be866819cd15ad3d8f508ee has_merged: false - diff --git a/modules/convert/convert.go b/modules/convert/convert.go index c62b4303ed..0e67563077 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -27,6 +27,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" ) @@ -414,3 +415,36 @@ func ToLFSLock(l *git_model.LFSLock) *api.LFSLock { }, } } + +// ToChangedFile convert a gitdiff.DiffFile to api.ChangedFile +func ToChangedFile(f *gitdiff.DiffFile, repo *repo_model.Repository, commit string) *api.ChangedFile { + status := "changed" + if f.IsDeleted { + status = "deleted" + } else if f.IsCreated { + status = "added" + } else if f.IsRenamed && f.Type == gitdiff.DiffFileCopy { + status = "copied" + } else if f.IsRenamed && f.Type == gitdiff.DiffFileRename { + status = "renamed" + } else if f.Addition == 0 && f.Deletion == 0 { + status = "unchanged" + } + + file := &api.ChangedFile{ + Filename: f.GetDiffFileName(), + Status: status, + Additions: f.Addition, + Deletions: f.Deletion, + Changes: f.Addition + f.Deletion, + HTMLURL: fmt.Sprint(repo.HTMLURL(), "/src/commit/", commit, "/", util.PathEscapeSegments(f.GetDiffFileName())), + ContentsURL: fmt.Sprint(repo.APIURL(), "/contents/", util.PathEscapeSegments(f.GetDiffFileName()), "?ref=", commit), + RawURL: fmt.Sprint(repo.HTMLURL(), "/raw/commit/", commit, "/", util.PathEscapeSegments(f.GetDiffFileName())), + } + + if status == "rename" { + file.PreviousFilename = f.OldName + } + + return file +} diff --git a/modules/structs/pull.go b/modules/structs/pull.go index b63b3edfd3..f627241b26 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -95,3 +95,16 @@ type EditPullRequestOption struct { RemoveDeadline *bool `json:"unset_due_date"` AllowMaintainerEdit *bool `json:"allow_maintainer_edit"` } + +// ChangedFile store information about files affected by the pull request +type ChangedFile struct { + Filename string `json:"filename"` + PreviousFilename string `json:"previous_filename,omitempty"` + Status string `json:"status"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + Changes int `json:"changes"` + HTMLURL string `json:"html_url,omitempty"` + ContentsURL string `json:"contents_url,omitempty"` + RawURL string `json:"raw_url,omitempty"` +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 6a98121b73..0d11674aa9 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1002,6 +1002,7 @@ func Routes(ctx gocontext.Context) *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) m.Combo("/merge").Get(repo.IsPullRequestMerged). Post(reqToken(), mustNotBeArchived, bind(forms.MergePullRequestForm{}), repo.MergePullRequest). Delete(reqToken(), mustNotBeArchived, repo.CancelScheduledAutoMerge) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index dda05203df..2cf30e7c47 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -26,6 +26,7 @@ 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" @@ -33,6 +34,7 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/automerge" "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" @@ -1323,3 +1325,137 @@ 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: skip-to + // in: query + // description: skip to given file + // type: string + // - name: whitespace + // in: query + // description: whitespace behavior + // type: string + // enum: [ignore-all, ignore-change, ignore-eol, show-all] + // - 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 := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if issues_model.IsErrPullRequestNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetPullRequestByIndex", err) + } + return + } + + if err := pr.LoadBaseRepo(); err != nil { + ctx.InternalServerError(err) + return + } + + if err := pr.LoadHeadRepo(); err != nil { + ctx.InternalServerError(err) + return + } + + baseGitRepo := ctx.Repo.GitRepo + + var prInfo *git.CompareInfo + 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"), + MaxLines: maxLines, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.FormString("whitespace")), + }) + if err != nil { + 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 + } + + apiFiles := make([]*api.ChangedFile, 0, end-start) + for i := start; i < end; i++ { + apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i], pr.HeadRepo, endCommitID)) + } + + 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) +} diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index 3522e24276..642b1b7b91 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -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 + Body []api.ChangedFile `json:"body"` +} + // Note // swagger:response Note type swaggerNote struct { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 47100450f9..52553e2e89 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -8019,6 +8019,80 @@ } } }, + "/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": "string", + "description": "skip to given file", + "name": "skip-to", + "in": "query" + }, + { + "enum": [ + "ignore-all", + "ignore-change", + "ignore-eol", + "show-all" + ], + "type": "string", + "description": "whitespace behavior", + "name": "whitespace", + "in": "query" + }, + { + "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": [ @@ -13715,6 +13789,52 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "ChangedFile": { + "description": "ChangedFile store information about files affected by the pull request", + "type": "object", + "properties": { + "additions": { + "type": "integer", + "format": "int64", + "x-go-name": "Additions" + }, + "changes": { + "type": "integer", + "format": "int64", + "x-go-name": "Changes" + }, + "contents_url": { + "type": "string", + "x-go-name": "ContentsURL" + }, + "deletions": { + "type": "integer", + "format": "int64", + "x-go-name": "Deletions" + }, + "filename": { + "type": "string", + "x-go-name": "Filename" + }, + "html_url": { + "type": "string", + "x-go-name": "HTMLURL" + }, + "previous_filename": { + "type": "string", + "x-go-name": "PreviousFilename" + }, + "raw_url": { + "type": "string", + "x-go-name": "RawURL" + }, + "status": { + "type": "string", + "x-go-name": "Status" + } + }, + "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", @@ -19173,6 +19293,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": { diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go index 032912a073..8ce92f3d4a 100644 --- a/tests/integration/api_pull_test.go +++ b/tests/integration/api_pull_test.go @@ -6,6 +6,7 @@ package integration import ( "fmt" + "io/ioutil" "net/http" "testing" @@ -27,15 +28,35 @@ func TestAPIViewPulls(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) - session := loginUser(t, "user2") - token := getTokenForLoggedInUser(t, session) - req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/pulls?state=all&token="+token, owner.Name, repo.Name) - resp := session.MakeRequest(t, req, http.StatusOK) + ctx := NewAPITestContext(t, "user2", repo.Name) + + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/pulls?state=all&token="+ctx.Token, owner.Name, repo.Name) + resp := ctx.Session.MakeRequest(t, req, http.StatusOK) var pulls []*api.PullRequest DecodeJSON(t, resp, &pulls) expectedLen := unittest.GetCount(t, &issues_model.Issue{RepoID: repo.ID}, unittest.Cond("is_pull = ?", true)) assert.Len(t, pulls, expectedLen) + + pull := pulls[0] + if assert.EqualValues(t, 5, pull.ID) { + resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK) + _, err := ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + // TODO: use diff to generate stats to test against + + t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID), + doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) { + if assert.Len(t, files, 1) { + assert.EqualValues(t, "File-WoW", files[0].Filename) + assert.EqualValues(t, "", files[0].PreviousFilename) + assert.EqualValues(t, 1, files[0].Additions) + assert.EqualValues(t, 1, files[0].Changes) + assert.EqualValues(t, 0, files[0].Deletions) + assert.EqualValues(t, "added", files[0].Status) + } + })) + } } // TestAPIMergePullWIP ensures that we can't merge a WIP pull request @@ -183,3 +204,22 @@ func TestAPIEditPull(t *testing.T) { }) session.MakeRequest(t, req, http.StatusNotFound) } + +func doAPIGetPullFiles(ctx APITestContext, pr *api.PullRequest, callback func(*testing.T, []*api.ChangedFile)) func(*testing.T) { + return func(t *testing.T) { + url := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/files?token=%s", ctx.Username, ctx.Reponame, pr.Index, ctx.Token) + + req := NewRequest(t, http.MethodGet, url) + if ctx.ExpectedCode == 0 { + ctx.ExpectedCode = http.StatusOK + } + resp := ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + + files := make([]*api.ChangedFile, 0, 1) + DecodeJSON(t, resp, &files) + + if callback != nil { + callback(t, files) + } + } +}