From 52de00c2b45d8a87ecdd68be2be86c5ebc2b96be Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Mon, 7 Apr 2025 00:17:14 +0200 Subject: [PATCH 01/31] Add multiple files API endpoint --- modules/structs/repo_file.go | 8 ++++ routers/api/v1/api.go | 1 + routers/api/v1/repo/file.go | 63 +++++++++++++++++++++++++++++++ services/repository/files/file.go | 7 +++- 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 0cd88b37038c5..1aa1e291b505b 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -176,3 +176,11 @@ type FileDeleteResponse struct { Commit *FileCommitResponse `json:"commit"` Verification *PayloadCommitVerification `json:"verification"` } + +// GetFilesOptions options for retrieving metadate and content of multiple files +type GetFilesOptions struct { + Files []string `json:"files" binding:"Required"` +} + +// FilesList contains multiple items if ContentsResponse +type FilesList []*ContentsResponse diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 5cd08a36181e2..aa7b28c489435 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1382,6 +1382,7 @@ func Routes() *web.Router { m.Group("/contents", func() { m.Get("", repo.GetContentsList) m.Post("", reqToken(), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.ChangeFiles) + m.Post("/files", bind(api.GetFilesOptions{}), repo.GetContentsFiles) m.Get("/*", repo.GetContents) m.Group("/*", func() { m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.CreateFile) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index d8e9fde2c006b..42cf3133c13f7 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -25,7 +25,9 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" pull_service "code.gitea.io/gitea/services/pull" @@ -982,3 +984,64 @@ func GetContentsList(ctx *context.APIContext) { // same as GetContents(), this function is here because swagger fails if path is empty in GetContents() interface GetContents(ctx) } + +// GetContentsFiles Get the metadata and contents of requested files +func GetContentsFiles(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/contents/files repository repoGetContentsFiles + // --- + // summary: Get the metadata and contents of requested files + // 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: ref + // in: query + // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // type: string + // required: false + // - 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 + // - name: body + // in: body + // required: true + // schema: + // "$ref": "#/definitions/GetFilesOptions" + // responses: + // "200": + // "$ref": "#/responses/FilesList" + // "404": + // "$ref": "#/responses/notFound" + + apiOpts := web.GetForm(ctx).(*api.GetFilesOptions) + + ref := ctx.FormTrim("ref") + if ref == "" { + ref = ctx.Repo.Repository.DefaultBranch + } + + files := apiOpts.Files + + filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) + count := len(filesResponse) + + listOpts := utils.GetListOptions(ctx) + filesResponse = util.PaginateSlice(filesResponse, listOpts.Page, listOpts.PageSize).([]*api.ContentsResponse) + + ctx.SetTotalCountHeader(int64(count)) + ctx.JSON(http.StatusOK, filesResponse) +} diff --git a/services/repository/files/file.go b/services/repository/files/file.go index a8ad5889cbca7..d8daf572b55e3 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -17,12 +17,17 @@ import ( "code.gitea.io/gitea/modules/util" ) -func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch string, treeNames []string) (*api.FilesResponse, error) { +func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) []*api.ContentsResponse { files := []*api.ContentsResponse{} for _, file := range treeNames { fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil files = append(files, fileContents) } + return files +} + +func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch string, treeNames []string) (*api.FilesResponse, error) { + files := GetContentsListFromTrees(ctx, repo, branch, treeNames) fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil verification := GetPayloadCommitVerification(ctx, commit) filesResponse := &api.FilesResponse{ From deacb2af9c2bf1289765037bee5abf8d7acfae88 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Mon, 7 Apr 2025 10:09:12 +0200 Subject: [PATCH 02/31] Add swagger --- modules/structs/repo_file.go | 3 -- routers/api/v1/repo/file.go | 2 +- routers/api/v1/swagger/options.go | 3 ++ templates/swagger/v1_json.tmpl | 76 +++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 1aa1e291b505b..b0e0bd979e14d 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -181,6 +181,3 @@ type FileDeleteResponse struct { type GetFilesOptions struct { Files []string `json:"files" binding:"Required"` } - -// FilesList contains multiple items if ContentsResponse -type FilesList []*ContentsResponse diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 42cf3133c13f7..945a8c824f749 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1023,7 +1023,7 @@ func GetContentsFiles(ctx *context.APIContext) { // "$ref": "#/definitions/GetFilesOptions" // responses: // "200": - // "$ref": "#/responses/FilesList" + // "$ref": "#/responses/ContentsListResponse" // "404": // "$ref": "#/responses/notFound" diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index aa5990eb38452..44919141d5f8b 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -118,6 +118,9 @@ type swaggerParameterBodies struct { // in:body EditAttachmentOptions api.EditAttachmentOptions + // in:body + GetFilesOptions api.GetFilesOptions + // in:body ChangeFilesOptions api.ChangeFilesOptions diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index d0e41e8094ea4..137ddc60fa177 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -6440,6 +6440,68 @@ } } }, + "/repos/{owner}/{repo}/contents/files": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get the metadata and contents of requested files", + "operationId": "repoGetContentsFiles", + "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": "string", + "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "name": "ref", + "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" + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/GetFilesOptions" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/ContentsListResponse" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/contents/{filepath}": { "get": { "produces": [ @@ -23018,6 +23080,20 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "GetFilesOptions": { + "description": "GetFilesOptions options for retrieving metadate and content of multiple files", + "type": "object", + "properties": { + "files": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "Files" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "GitBlobResponse": { "description": "GitBlobResponse represents a git blob", "type": "object", From b3165c66cb574f4da49bb6783e3b0034e05da7a1 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Mon, 7 Apr 2025 16:26:01 +0200 Subject: [PATCH 03/31] Remove contents prefix --- routers/api/v1/api.go | 2 +- routers/api/v1/repo/file.go | 6 +- templates/swagger/v1_json.tmpl | 124 ++++++++++++++++----------------- 3 files changed, 66 insertions(+), 66 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index aa7b28c489435..d880d0a7fa7f5 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1382,7 +1382,6 @@ func Routes() *web.Router { m.Group("/contents", func() { m.Get("", repo.GetContentsList) m.Post("", reqToken(), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.ChangeFiles) - m.Post("/files", bind(api.GetFilesOptions{}), repo.GetContentsFiles) m.Get("/*", repo.GetContents) m.Group("/*", func() { m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.CreateFile) @@ -1390,6 +1389,7 @@ func Routes() *web.Router { m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile) }, reqToken()) }, reqRepoReader(unit.TypeCode)) + m.Post("/files", bind(api.GetFilesOptions{}), reqRepoReader(unit.TypeCode), repo.GetFiles) m.Get("/signing-key.gpg", misc.SigningKey) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 945a8c824f749..4522c1bb6b084 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -985,9 +985,9 @@ func GetContentsList(ctx *context.APIContext) { GetContents(ctx) } -// GetContentsFiles Get the metadata and contents of requested files -func GetContentsFiles(ctx *context.APIContext) { - // swagger:operation POST /repos/{owner}/{repo}/contents/files repository repoGetContentsFiles +// GetFiles Get the metadata and contents of requested files +func GetFiles(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/files repository repoGetFiles // --- // summary: Get the metadata and contents of requested files // produces: diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 137ddc60fa177..3e9d898f46437 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -6440,68 +6440,6 @@ } } }, - "/repos/{owner}/{repo}/contents/files": { - "post": { - "produces": [ - "application/json" - ], - "tags": [ - "repository" - ], - "summary": "Get the metadata and contents of requested files", - "operationId": "repoGetContentsFiles", - "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": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", - "name": "ref", - "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" - }, - { - "name": "body", - "in": "body", - "required": true, - "schema": { - "$ref": "#/definitions/GetFilesOptions" - } - } - ], - "responses": { - "200": { - "$ref": "#/responses/ContentsListResponse" - }, - "404": { - "$ref": "#/responses/notFound" - } - } - } - }, "/repos/{owner}/{repo}/contents/{filepath}": { "get": { "produces": [ @@ -6833,6 +6771,68 @@ } } }, + "/repos/{owner}/{repo}/files": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get the metadata and contents of requested files", + "operationId": "repoGetFiles", + "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": "string", + "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "name": "ref", + "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" + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/GetFilesOptions" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/ContentsListResponse" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/forks": { "get": { "produces": [ From 5de2936e2f99aa5cbf20f60a672073495df450f8 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Thu, 10 Apr 2025 09:14:05 +0200 Subject: [PATCH 04/31] Add integration test for files api --- routers/api/v1/api.go | 2 +- routers/api/v1/repo/file.go | 5 + tests/integration/api_repo_files_get_test.go | 145 +++++++++++++++++++ 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 tests/integration/api_repo_files_get_test.go diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index d880d0a7fa7f5..36d2563e177a6 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1389,7 +1389,7 @@ func Routes() *web.Router { m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile) }, reqToken()) }, reqRepoReader(unit.TypeCode)) - m.Post("/files", bind(api.GetFilesOptions{}), reqRepoReader(unit.TypeCode), repo.GetFiles) + m.Post("/files", context.ReferencesGitRepo(), context.RepoRefForAPI, bind(api.GetFilesOptions{}), reqRepoReader(unit.TypeCode), repo.GetFiles) m.Get("/signing-key.gpg", misc.SigningKey) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 4522c1bb6b084..d9238597235a9 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1034,6 +1034,11 @@ func GetFiles(ctx *context.APIContext) { ref = ctx.Repo.Repository.DefaultBranch } + if !ctx.Repo.GitRepo.IsReferenceExist(ref) { + ctx.APIErrorNotFound("GetFiles", "ref does not exist") + return + } + files := apiOpts.Files filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go new file mode 100644 index 0000000000000..9ae6e02cc2145 --- /dev/null +++ b/tests/integration/api_repo_files_get_test.go @@ -0,0 +1,145 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "net/http" + "net/url" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + api "code.gitea.io/gitea/modules/structs" + repo_service "code.gitea.io/gitea/services/repository" + + "github.com/stretchr/testify/assert" +) + +func getExpectedcontentsListResponseForFiles(ref, refType, lastCommitSHA string) []*api.ContentsResponse { + return []*api.ContentsResponse{getExpectedContentsResponseForContents(ref, refType, lastCommitSHA)} +} + +func TestAPIGetRequestedFiles(t *testing.T) { + onGiteaRun(t, testAPIGetRequestedFiles) +} + +func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { + /*** SETUP ***/ + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo1 & repo16 + org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // owner of the repo3, is an org + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // owner of neither repos + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) // public repo + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // public repo + repo16 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) // private repo + filesOptions := &api.GetFilesOptions{ + Files: []string{ + "README.md", + }, + } + + // Get user2's token req.Body = + session := loginUser(t, user2.Name) + token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) //TODO: allow for a POST-request to be scope read + // Get user4's token + session = loginUser(t, user4.Name) + token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) //TODO: allow for a POST-request to be scope read + + // Get the commit ID of the default branch + gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo1) + assert.NoError(t, err) + defer gitRepo.Close() + + // Make a new branch in repo1 + newBranch := "test_branch" + err = repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, gitRepo, repo1.DefaultBranch, newBranch) + assert.NoError(t, err) + + commitID, err := gitRepo.GetBranchCommitID(repo1.DefaultBranch) + assert.NoError(t, err) + // Make a new tag in repo1 + newTag := "test_tag" + err = gitRepo.CreateTag(newTag, commitID) + assert.NoError(t, err) + /*** END SETUP ***/ + + // ref is default ref + ref := repo1.DefaultBranch + refType := "branch" + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) + resp := MakeRequest(t, req, http.StatusOK) + var contentsListResponse []*api.ContentsResponse + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + lastCommit, _ := gitRepo.GetCommitByPath("README.md") + expectedcontentsListResponse := getExpectedcontentsListResponseForFiles(ref, refType, lastCommit.ID.String()) + assert.Equal(t, expectedcontentsListResponse, contentsListResponse) + + // No ref + refType = "branch" + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo1.Name), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(repo1.DefaultBranch, refType, lastCommit.ID.String()) + assert.Equal(t, expectedcontentsListResponse, contentsListResponse) + + // ref is the branch we created above in setup + ref = newBranch + refType = "branch" + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + branchCommit, _ := gitRepo.GetBranchCommit(ref) + lastCommit, _ = branchCommit.GetCommitByPath("README.md") + expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(ref, refType, lastCommit.ID.String()) + assert.Equal(t, expectedcontentsListResponse, contentsListResponse) + + // ref is the new tag we created above in setup + ref = newTag + refType = "tag" + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + tagCommit, _ := gitRepo.GetTagCommit(ref) + lastCommit, _ = tagCommit.GetCommitByPath("README.md") + expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(ref, refType, lastCommit.ID.String()) + assert.Equal(t, expectedcontentsListResponse, contentsListResponse) + + // ref is a commit + ref = commitID + refType = "commit" + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(ref, refType, commitID) + assert.Equal(t, expectedcontentsListResponse, contentsListResponse) + + // Test file contents a file with a bad ref + ref = "badref" + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) + MakeRequest(t, req, http.StatusNotFound) + + // Test accessing private ref with user token that does not have access - should fail + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions). + AddTokenAuth(token4) + MakeRequest(t, req, http.StatusNotFound) + + // Test access private ref of owner of token + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusOK) + + // Test access of org org3 private repo file by owner user2 + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", org3.Name, repo3.Name), &filesOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusOK) +} From adfefad2b8a2f20f59d1ba2601c5f37510f323d4 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Thu, 10 Apr 2025 11:03:49 +0200 Subject: [PATCH 05/31] Format files --- tests/integration/api_repo_files_get_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index 9ae6e02cc2145..fd46c8590c69e 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -1,4 +1,4 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. +// Copyright 2025 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package integration @@ -45,10 +45,10 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { // Get user2's token req.Body = session := loginUser(t, user2.Name) - token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) //TODO: allow for a POST-request to be scope read + token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // TODO: allow for a POST-request to be scope read // Get user4's token session = loginUser(t, user4.Name) - token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) //TODO: allow for a POST-request to be scope read + token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // TODO: allow for a POST-request to be scope read // Get the commit ID of the default branch gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo1) From 4085174de7a4ba4cc0082453bf4667f173ed0704 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sun, 13 Apr 2025 20:07:01 +0200 Subject: [PATCH 06/31] Remove useless function and test --- services/repository/files/file.go | 13 ---- services/repository/files/file_test.go | 97 -------------------------- 2 files changed, 110 deletions(-) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index d8daf572b55e3..99089673b7dcb 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -38,19 +38,6 @@ func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository return filesResponse, nil } -// GetFileResponseFromCommit Constructs a FileResponse from a Commit object -func GetFileResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch, treeName string) (*api.FileResponse, error) { - fileContents, _ := GetContents(ctx, repo, treeName, branch, false) // ok if fails, then will be nil - fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil - verification := GetPayloadCommitVerification(ctx, commit) - fileResponse := &api.FileResponse{ - Content: fileContents, - Commit: fileCommitResponse, - Verification: verification, - } - return fileResponse, nil -} - // constructs a FileResponse with the file at the index from FilesResponse func GetFileResponseFromFilesResponse(filesResponse *api.FilesResponse, index int) *api.FileResponse { content := &api.ContentsResponse{} diff --git a/services/repository/files/file_test.go b/services/repository/files/file_test.go index 7cb79d7ebfdf1..169cafba0db1d 100644 --- a/services/repository/files/file_test.go +++ b/services/repository/files/file_test.go @@ -5,13 +5,6 @@ package files import ( "testing" - "time" - - "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/setting" - api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/services/contexttest" "github.com/stretchr/testify/assert" ) @@ -31,93 +24,3 @@ func TestCleanUploadFileName(t *testing.T) { assert.Equal(t, expectedCleanName, cleanName) }) } - -func getExpectedFileResponse() *api.FileResponse { - treePath := "README.md" - sha := "4b4851ad51df6a7d9f25c979345979eaeb5b349f" - encoding := "base64" - content := "IyByZXBvMQoKRGVzY3JpcHRpb24gZm9yIHJlcG8x" - selfURL := setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=master" - htmlURL := setting.AppURL + "user2/repo1/src/branch/master/" + treePath - gitURL := setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha - downloadURL := setting.AppURL + "user2/repo1/raw/branch/master/" + treePath - return &api.FileResponse{ - Content: &api.ContentsResponse{ - Name: treePath, - Path: treePath, - SHA: sha, - LastCommitSHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d", - LastCommitterDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), - LastAuthorDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), - Type: "file", - Size: 30, - Encoding: &encoding, - Content: &content, - URL: &selfURL, - HTMLURL: &htmlURL, - GitURL: &gitURL, - DownloadURL: &downloadURL, - Links: &api.FileLinksResponse{ - Self: &selfURL, - GitURL: &gitURL, - HTMLURL: &htmlURL, - }, - }, - Commit: &api.FileCommitResponse{ - CommitMeta: api.CommitMeta{ - URL: "https://try.gitea.io/api/v1/repos/user2/repo1/git/commits/65f1bf27bc3bf70f64657658635e66094edbcb4d", - SHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d", - }, - HTMLURL: "https://try.gitea.io/user2/repo1/commit/65f1bf27bc3bf70f64657658635e66094edbcb4d", - Author: &api.CommitUser{ - Identity: api.Identity{ - Name: "user1", - Email: "address1@example.com", - }, - Date: "2017-03-19T20:47:59Z", - }, - Committer: &api.CommitUser{ - Identity: api.Identity{ - Name: "Ethan Koenig", - Email: "ethantkoenig@gmail.com", - }, - Date: "2017-03-19T20:47:59Z", - }, - Parents: []*api.CommitMeta{}, - Message: "Initial commit\n", - Tree: &api.CommitMeta{ - URL: "https://try.gitea.io/api/v1/repos/user2/repo1/git/trees/2a2f1d4670728a2e10049e345bd7a276468beab6", - SHA: "2a2f1d4670728a2e10049e345bd7a276468beab6", - }, - }, - Verification: &api.PayloadCommitVerification{ - Verified: false, - Reason: "gpg.error.not_signed_commit", - Signature: "", - Payload: "", - }, - } -} - -func TestGetFileResponseFromCommit(t *testing.T) { - unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetPathParam("id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - branch := repo.DefaultBranch - treePath := "README.md" - gitRepo, _ := gitrepo.OpenRepository(ctx, repo) - defer gitRepo.Close() - commit, _ := gitRepo.GetBranchCommit(branch) - expectedFileResponse := getExpectedFileResponse() - - fileResponse, err := GetFileResponseFromCommit(ctx, repo, commit, branch, treePath) - assert.NoError(t, err) - assert.Equal(t, expectedFileResponse, fileResponse) -} From 6b08536f1794a01df2d2a891fb187e1dadc9c84e Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 13:57:45 +0200 Subject: [PATCH 07/31] Apply pagination before retreiving files --- routers/api/v1/repo/file.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index d9238597235a9..1498b80a0dca0 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1041,11 +1041,11 @@ func GetFiles(ctx *context.APIContext) { files := apiOpts.Files - filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) - count := len(filesResponse) - + count := len(files) listOpts := utils.GetListOptions(ctx) - filesResponse = util.PaginateSlice(filesResponse, listOpts.Page, listOpts.PageSize).([]*api.ContentsResponse) + files = util.PaginateSlice(files, listOpts.Page, listOpts.PageSize).([]string) + + filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) ctx.SetTotalCountHeader(int64(count)) ctx.JSON(http.StatusOK, filesResponse) From 57dc3d19ba34f83ca550e326a57934d35543441c Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 19:08:54 +0200 Subject: [PATCH 08/31] Add response size limit --- custom/conf/app.example.ini | 2 ++ modules/setting/api.go | 2 ++ modules/structs/settings.go | 1 + routers/api/v1/repo/file.go | 7 ++++++- services/context/api.go | 6 ++++++ services/repository/files/file.go | 17 ++++++++++++++--- templates/swagger/v1_json.tmpl | 19 +++++++++++++++++++ tests/integration/api_settings_test.go | 1 + 8 files changed, 51 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 8d395511687d6..565ae02c36be8 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2417,6 +2417,8 @@ LEVEL = Info ;DEFAULT_GIT_TREES_PER_PAGE = 1000 ;; Default max size of a blob returned by the blobs API (default is 10MiB) ;DEFAULT_MAX_BLOB_SIZE = 10485760 +;; Default max combined size of all blobs returned by the files API (default is 100MiB) +;DEFAULT_MAX_RESPONSE_SIZE = 104857600 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/setting/api.go b/modules/setting/api.go index c36f05cfd1c2d..cdad474cb9f65 100644 --- a/modules/setting/api.go +++ b/modules/setting/api.go @@ -18,6 +18,7 @@ var API = struct { DefaultPagingNum int DefaultGitTreesPerPage int DefaultMaxBlobSize int64 + DefaultMaxResponseSize int64 }{ EnableSwagger: true, SwaggerURL: "", @@ -25,6 +26,7 @@ var API = struct { DefaultPagingNum: 30, DefaultGitTreesPerPage: 1000, DefaultMaxBlobSize: 10485760, + DefaultMaxResponseSize: 104857600, } func loadAPIFrom(rootCfg ConfigProvider) { diff --git a/modules/structs/settings.go b/modules/structs/settings.go index e48b1a493db29..59176210e6e1f 100644 --- a/modules/structs/settings.go +++ b/modules/structs/settings.go @@ -26,6 +26,7 @@ type GeneralAPISettings struct { DefaultPagingNum int `json:"default_paging_num"` DefaultGitTreesPerPage int `json:"default_git_trees_per_page"` DefaultMaxBlobSize int64 `json:"default_max_blob_size"` + DefaultMaxResponseSize int64 `json:"default_max_response_size"` } // GeneralAttachmentSettings contains global Attachment settings exposed by API diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 1498b80a0dca0..22528d1dd6135 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1026,6 +1026,8 @@ func GetFiles(ctx *context.APIContext) { // "$ref": "#/responses/ContentsListResponse" // "404": // "$ref": "#/responses/notFound" + // "413": + // "$ref": "#/responses/contentTooLarge" apiOpts := web.GetForm(ctx).(*api.GetFilesOptions) @@ -1045,7 +1047,10 @@ func GetFiles(ctx *context.APIContext) { listOpts := utils.GetListOptions(ctx) files = util.PaginateSlice(files, listOpts.Page, listOpts.PageSize).([]string) - filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) + filesResponse, err := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) + if err != nil { + ctx.APIError(http.StatusRequestEntityTooLarge, err.Error()) + } ctx.SetTotalCountHeader(int64(count)) ctx.JSON(http.StatusOK, filesResponse) diff --git a/services/context/api.go b/services/context/api.go index 6e3b635ce366d..b7af0e5e95ce2 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -91,6 +91,12 @@ type APIForbiddenError struct { // swagger:response notFound type APINotFound struct{} +// APIContentTooLarge is a content too large error response +// swagger:response contentTooLarge +type APIContentTooLarge struct { + APIError +} + // APIConflict is a conflict empty response // swagger:response conflict type APIConflict struct{} diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 99089673b7dcb..2fe984f9de28c 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -13,21 +13,32 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" ) -func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) []*api.ContentsResponse { +func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) ([]*api.ContentsResponse, error) { files := []*api.ContentsResponse{} + var size int64 = 0 for _, file := range treeNames { fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil + if *fileContents.Content != "" { + size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size + } + if size > setting.API.DefaultMaxResponseSize { + return nil, fmt.Errorf("the combined size of the requested blobs exceeds the per-request limit set by the server administrator") + } files = append(files, fileContents) } - return files + return files, nil } func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch string, treeNames []string) (*api.FilesResponse, error) { - files := GetContentsListFromTrees(ctx, repo, branch, treeNames) + files, err := GetContentsListFromTrees(ctx, repo, branch, treeNames) + if err != nil { + return nil, err + } fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil verification := GetPayloadCommitVerification(ctx, commit) filesResponse := &api.FilesResponse{ diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3e9d898f46437..7fc887e34e01c 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -6829,6 +6829,9 @@ }, "404": { "$ref": "#/responses/notFound" + }, + "413": { + "$ref": "#/responses/contentTooLarge" } } } @@ -22914,6 +22917,11 @@ "format": "int64", "x-go-name": "DefaultMaxBlobSize" }, + "default_max_response_size": { + "type": "integer", + "format": "int64", + "x-go-name": "DefaultMaxResponseSize" + }, "default_paging_num": { "type": "integer", "format": "int64", @@ -27599,6 +27607,17 @@ "conflict": { "description": "APIConflict is a conflict empty response" }, + "contentTooLarge": { + "description": "APIContentTooLarge is a content too large error response", + "headers": { + "message": { + "type": "string" + }, + "url": { + "type": "string" + } + } + }, "empty": { "description": "APIEmpty is an empty response" }, diff --git a/tests/integration/api_settings_test.go b/tests/integration/api_settings_test.go index 6f433148f5f17..743dbb04815f8 100644 --- a/tests/integration/api_settings_test.go +++ b/tests/integration/api_settings_test.go @@ -35,6 +35,7 @@ func TestAPIExposedSettings(t *testing.T) { DefaultPagingNum: setting.API.DefaultPagingNum, DefaultGitTreesPerPage: setting.API.DefaultGitTreesPerPage, DefaultMaxBlobSize: setting.API.DefaultMaxBlobSize, + DefaultMaxResponseSize: setting.API.DefaultMaxResponseSize, }, apiSettings) repo := new(api.GeneralRepoSettings) From 4b231a3d0533609a668ba540181416737206c403 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 19:17:24 +0200 Subject: [PATCH 09/31] Fix linter --- services/repository/files/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 2fe984f9de28c..8f7c9a5deaa5b 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -27,7 +27,7 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size } if size > setting.API.DefaultMaxResponseSize { - return nil, fmt.Errorf("the combined size of the requested blobs exceeds the per-request limit set by the server administrator") + return nil, errors.New("the combined size of the requested blobs exceeds the per-request limit set by the server administrator") } files = append(files, fileContents) } From 38818285e22b8e1c01ee0f4edfb91d0ce175eb88 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 19:26:52 +0200 Subject: [PATCH 10/31] Add error if file could not be retrieved --- routers/api/v1/repo/file.go | 7 ++++++- services/repository/files/file.go | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 22528d1dd6135..1da18191343a8 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1049,7 +1049,12 @@ func GetFiles(ctx *context.APIContext) { filesResponse, err := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) if err != nil { - ctx.APIError(http.StatusRequestEntityTooLarge, err.Error()) + if err.Error() == "the combined size of the requested blobs exceeds the per-request limit set by the server administrator" { + ctx.APIError(http.StatusRequestEntityTooLarge, err.Error()) + } else { + ctx.APIErrorNotFound("GetFiles") + } + return } ctx.SetTotalCountHeader(int64(count)) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 8f7c9a5deaa5b..12fa5928b50f9 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -22,7 +22,10 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, files := []*api.ContentsResponse{} var size int64 = 0 for _, file := range treeNames { - fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil + fileContents, err := GetContents(ctx, repo, file, branch, false) + if err != nil { + return nil, err + } if *fileContents.Content != "" { size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size } From 8eaff5a6391b444fb6f0883c32bf8593914c089b Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 19:58:49 +0200 Subject: [PATCH 11/31] Fix linter --- services/repository/files/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 12fa5928b50f9..91ce3f2ce5c01 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -20,7 +20,7 @@ import ( func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) ([]*api.ContentsResponse, error) { files := []*api.ContentsResponse{} - var size int64 = 0 + var size int64 for _, file := range treeNames { fileContents, err := GetContents(ctx, repo, file, branch, false) if err != nil { From 579a0a05590838600df8fd9c78b574bbf167fd7a Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 19:59:55 +0200 Subject: [PATCH 12/31] Revert "Add error if file could not be retrieved" This reverts commit 38818285e22b8e1c01ee0f4edfb91d0ce175eb88. --- routers/api/v1/repo/file.go | 7 +------ services/repository/files/file.go | 5 +---- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 1da18191343a8..22528d1dd6135 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1049,12 +1049,7 @@ func GetFiles(ctx *context.APIContext) { filesResponse, err := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) if err != nil { - if err.Error() == "the combined size of the requested blobs exceeds the per-request limit set by the server administrator" { - ctx.APIError(http.StatusRequestEntityTooLarge, err.Error()) - } else { - ctx.APIErrorNotFound("GetFiles") - } - return + ctx.APIError(http.StatusRequestEntityTooLarge, err.Error()) } ctx.SetTotalCountHeader(int64(count)) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 91ce3f2ce5c01..b65f764b32a4a 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -22,10 +22,7 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, files := []*api.ContentsResponse{} var size int64 for _, file := range treeNames { - fileContents, err := GetContents(ctx, repo, file, branch, false) - if err != nil { - return nil, err - } + fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil if *fileContents.Content != "" { size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size } From 648a2a414a5bd135a49a0d61020994541e516305 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 18 Apr 2025 20:11:26 +0200 Subject: [PATCH 13/31] Fix for fileContents = nil --- services/repository/files/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index b65f764b32a4a..05631148f5d4a 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -23,7 +23,7 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, var size int64 for _, file := range treeNames { fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil - if *fileContents.Content != "" { + if fileContents != nil && *fileContents.Content != "" { size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size } if size > setting.API.DefaultMaxResponseSize { From 5c5577a061ad82661e79ce23d0110b372b8d181c Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sat, 19 Apr 2025 21:18:58 +0200 Subject: [PATCH 14/31] Fix api settings endpoint --- routers/api/v1/settings/settings.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/settings/settings.go b/routers/api/v1/settings/settings.go index 0ee81b96d5bb9..94fbadeab0188 100644 --- a/routers/api/v1/settings/settings.go +++ b/routers/api/v1/settings/settings.go @@ -43,6 +43,7 @@ func GetGeneralAPISettings(ctx *context.APIContext) { DefaultPagingNum: setting.API.DefaultPagingNum, DefaultGitTreesPerPage: setting.API.DefaultGitTreesPerPage, DefaultMaxBlobSize: setting.API.DefaultMaxBlobSize, + DefaultMaxResponseSize: setting.API.DefaultMaxResponseSize, }) } From 18811c7fef9ead6e97b5b4ad4c92185506cfcfc5 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sun, 20 Apr 2025 17:31:13 +0200 Subject: [PATCH 15/31] Switch pagination method and add test --- routers/api/v1/repo/file.go | 28 ++---- services/context/api.go | 6 -- services/repository/files/file.go | 14 +-- templates/swagger/v1_json.tmpl | 27 +----- tests/integration/api_repo_files_get_test.go | 94 ++++++++++++++++++++ 5 files changed, 108 insertions(+), 61 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 22528d1dd6135..2fbf789258092 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -25,9 +25,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" pull_service "code.gitea.io/gitea/services/pull" @@ -990,6 +988,11 @@ func GetFiles(ctx *context.APIContext) { // swagger:operation POST /repos/{owner}/{repo}/files repository repoGetFiles // --- // summary: Get the metadata and contents of requested files + // description: Uses automatic pagination based on default page size and + // max response size and returns the maximum allowed number of files. + // Files which could not be retrieved are null. Blobs which are too large + // are being returned with `content = ""` and `size > 0`, they can be + // requested using the `download_url`. // produces: // - application/json // parameters: @@ -1008,14 +1011,6 @@ func GetFiles(ctx *context.APIContext) { // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" // type: string // required: false - // - 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 // - name: body // in: body // required: true @@ -1026,8 +1021,6 @@ func GetFiles(ctx *context.APIContext) { // "$ref": "#/responses/ContentsListResponse" // "404": // "$ref": "#/responses/notFound" - // "413": - // "$ref": "#/responses/contentTooLarge" apiOpts := web.GetForm(ctx).(*api.GetFilesOptions) @@ -1042,16 +1035,7 @@ func GetFiles(ctx *context.APIContext) { } files := apiOpts.Files + filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) - count := len(files) - listOpts := utils.GetListOptions(ctx) - files = util.PaginateSlice(files, listOpts.Page, listOpts.PageSize).([]string) - - filesResponse, err := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) - if err != nil { - ctx.APIError(http.StatusRequestEntityTooLarge, err.Error()) - } - - ctx.SetTotalCountHeader(int64(count)) ctx.JSON(http.StatusOK, filesResponse) } diff --git a/services/context/api.go b/services/context/api.go index b7af0e5e95ce2..6e3b635ce366d 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -91,12 +91,6 @@ type APIForbiddenError struct { // swagger:response notFound type APINotFound struct{} -// APIContentTooLarge is a content too large error response -// swagger:response contentTooLarge -type APIContentTooLarge struct { - APIError -} - // APIConflict is a conflict empty response // swagger:response conflict type APIConflict struct{} diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 05631148f5d4a..3e063216236d9 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -18,7 +18,7 @@ import ( "code.gitea.io/gitea/modules/util" ) -func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) ([]*api.ContentsResponse, error) { +func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) []*api.ContentsResponse { files := []*api.ContentsResponse{} var size int64 for _, file := range treeNames { @@ -27,18 +27,18 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size } if size > setting.API.DefaultMaxResponseSize { - return nil, errors.New("the combined size of the requested blobs exceeds the per-request limit set by the server administrator") + return files // return if max page size would be exceeded } files = append(files, fileContents) + if len(files) == setting.API.DefaultPagingNum { + return files // return if paging num or max size reached + } } - return files, nil + return files } func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch string, treeNames []string) (*api.FilesResponse, error) { - files, err := GetContentsListFromTrees(ctx, repo, branch, treeNames) - if err != nil { - return nil, err - } + files := GetContentsListFromTrees(ctx, repo, branch, treeNames) fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil verification := GetPayloadCommitVerification(ctx, commit) filesResponse := &api.FilesResponse{ diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 8f7552c780eaf..4780c5d2b9f76 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7265,6 +7265,7 @@ }, "/repos/{owner}/{repo}/files": { "post": { + "description": "Uses automatic pagination based on default page size and max response size and returns the maximum allowed number of files. Files which could not be retrieved are null. Blobs which are too large are being returned with `content = \"\"` and `size \u003e 0`, they can be requested using the `download_url`.", "produces": [ "application/json" ], @@ -7294,18 +7295,6 @@ "name": "ref", "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" - }, { "name": "body", "in": "body", @@ -7321,9 +7310,6 @@ }, "404": { "$ref": "#/responses/notFound" - }, - "413": { - "$ref": "#/responses/contentTooLarge" } } } @@ -28286,17 +28272,6 @@ "conflict": { "description": "APIConflict is a conflict empty response" }, - "contentTooLarge": { - "description": "APIContentTooLarge is a content too large error response", - "headers": { - "message": { - "type": "string" - }, - "url": { - "type": "string" - } - } - }, "empty": { "description": "APIEmpty is an empty response" }, diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index fd46c8590c69e..abc8a74717cd0 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -4,17 +4,21 @@ package integration import ( + "encoding/base64" "fmt" "net/http" "net/url" "testing" + "time" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" repo_service "code.gitea.io/gitea/services/repository" @@ -142,4 +146,94 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", org3.Name, repo3.Name), &filesOptions). AddTokenAuth(token2) MakeRequest(t, req, http.StatusOK) + + // Test pagination + for i := 0; i < 40; i++ { + filesOptions.Files = append(filesOptions.Files, filesOptions.Files[0]) + } + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo1.Name), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + assert.Equal(t, setting.API.DefaultPagingNum, len(contentsListResponse)) + + // create new repo for large file tests + baseRepo, err := repo_service.CreateRepository(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{ + Name: "repo-test-files-api", + Description: "test files api", + AutoInit: true, + Gitignores: "Go", + License: "MIT", + Readme: "Default", + DefaultBranch: "main", + IsPrivate: false, + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // Test file size limit + largeFile := make([]byte, 15728640) // 15 MiB -> over max blob size + for i := range largeFile { + largeFile[i] = byte(i % 256) + } + user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository) + doAPICreateFile(user2APICtx, "large-file.txt", &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + Message: "create large-file.txt", + Author: api.Identity{ + Name: user2.Name, + Email: user2.Email, + }, + Committer: api.Identity{ + Name: user2.Name, + Email: user2.Email, + }, + Dates: api.CommitDateOptions{ + Author: time.Now(), + Committer: time.Now(), + }, + }, + ContentBase64: base64.StdEncoding.EncodeToString(largeFile), + })(t) + + filesOptions.Files = []string{"large-file.txt"} + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, baseRepo.Name), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + assert.Equal(t, int64(15728640), contentsListResponse[0].Size) + assert.Equal(t, "", *contentsListResponse[0].Content) + + // Test response size limit + smallFile := make([]byte, 5242880) // 5 MiB -> under max blob size + for i := range smallFile { + smallFile[i] = byte(i % 256) + } + doAPICreateFile(user2APICtx, "small-file.txt", &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + Message: "create small-file.txt", + Author: api.Identity{ + Name: user2.Name, + Email: user2.Email, + }, + Committer: api.Identity{ + Name: user2.Name, + Email: user2.Email, + }, + Dates: api.CommitDateOptions{ + Author: time.Now(), + Committer: time.Now(), + }, + }, + ContentBase64: base64.StdEncoding.EncodeToString(smallFile), + })(t) + filesOptions.Files = []string{"small-file.txt"} + for i := 0; i < 40; i++ { + filesOptions.Files = append(filesOptions.Files, filesOptions.Files[0]) + } + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, baseRepo.Name), &filesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &contentsListResponse) + assert.NotNil(t, contentsListResponse) + assert.Equal(t, 20, len(contentsListResponse)) } From 91f4193d2877bf50a0842dc4587bd9fb2ecbdec2 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sun, 20 Apr 2025 17:47:13 +0200 Subject: [PATCH 16/31] Fix compliance --- tests/integration/api_repo_files_get_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index abc8a74717cd0..c8ba1d21b8dda 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -155,7 +155,7 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) - assert.Equal(t, setting.API.DefaultPagingNum, len(contentsListResponse)) + assert.Len(t, contentsListResponse, setting.API.DefaultPagingNum) // create new repo for large file tests baseRepo, err := repo_service.CreateRepository(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{ @@ -202,7 +202,7 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) assert.Equal(t, int64(15728640), contentsListResponse[0].Size) - assert.Equal(t, "", *contentsListResponse[0].Content) + assert.Empty(t, *contentsListResponse[0].Content) // Test response size limit smallFile := make([]byte, 5242880) // 5 MiB -> under max blob size @@ -235,5 +235,5 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) - assert.Equal(t, 20, len(contentsListResponse)) + assert.Len(t, contentsListResponse, 20) } From 6911a9a026020fd4e837bf91cdc86c279fe8c761 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 15:52:33 +0800 Subject: [PATCH 17/31] temp --- modules/git/repo_object.go | 14 ---- routers/api/v1/repo/status.go | 38 ++++++---- routers/api/v1/utils/git.go | 101 +++++++------------------ services/context/api.go | 41 ++++------ services/repository/files/content.go | 108 ++++++++++----------------- services/repository/files/file.go | 11 ++- services/repository/files/update.go | 1 + 7 files changed, 110 insertions(+), 204 deletions(-) diff --git a/modules/git/repo_object.go b/modules/git/repo_object.go index 1d34305270676..08e0413311ebb 100644 --- a/modules/git/repo_object.go +++ b/modules/git/repo_object.go @@ -85,17 +85,3 @@ func (repo *Repository) hashObject(reader io.Reader, save bool) (string, error) } return strings.TrimSpace(stdout.String()), nil } - -// GetRefType gets the type of the ref based on the string -func (repo *Repository) GetRefType(ref string) ObjectType { - if repo.IsTagExist(ref) { - return ObjectTag - } else if repo.IsBranchExist(ref) { - return ObjectBranch - } else if repo.IsCommitExist(ref) { - return ObjectCommit - } else if _, err := repo.GetBlob(ref); err == nil { - return ObjectBlob - } - return ObjectType("invalid") -} diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go index e1dbb25865d79..38ddc15e43734 100644 --- a/routers/api/v1/repo/status.go +++ b/routers/api/v1/repo/status.go @@ -4,6 +4,8 @@ package repo import ( + "code.gitea.io/gitea/modules/util" + "errors" "fmt" "net/http" @@ -177,20 +179,19 @@ func GetCommitStatusesByRef(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - filter := utils.ResolveRefOrSha(ctx, ctx.PathParam("ref")) - if ctx.Written() { + refCommit, err := utils.ResolveRefCommit(ctx, ctx.PathParam("ref")) + if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.APIErrorNotFound(err) + } else { + ctx.APIErrorInternal(err) + } return } - - getCommitStatuses(ctx, filter) // By default filter is maybe the raw SHA + getCommitStatuses(ctx, refCommit.CommitID) } -func getCommitStatuses(ctx *context.APIContext, sha string) { - if len(sha) == 0 { - ctx.APIError(http.StatusBadRequest, nil) - return - } - sha = utils.MustConvertToSHA1(ctx.Base, ctx.Repo, sha) +func getCommitStatuses(ctx *context.APIContext, commitID string) { repo := ctx.Repo.Repository listOptions := utils.GetListOptions(ctx) @@ -198,12 +199,12 @@ func getCommitStatuses(ctx *context.APIContext, sha string) { statuses, maxResults, err := db.FindAndCount[git_model.CommitStatus](ctx, &git_model.CommitStatusOptions{ ListOptions: listOptions, RepoID: repo.ID, - SHA: sha, + SHA: commitID, SortType: ctx.FormTrim("sort"), State: ctx.FormTrim("state"), }) if err != nil { - ctx.APIErrorInternal(fmt.Errorf("GetCommitStatuses[%s, %s, %d]: %w", repo.FullName(), sha, ctx.FormInt("page"), err)) + ctx.APIErrorInternal(fmt.Errorf("GetCommitStatuses[%s, %s, %d]: %w", repo.FullName(), commitID, ctx.FormInt("page"), err)) return } @@ -257,16 +258,21 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - sha := utils.ResolveRefOrSha(ctx, ctx.PathParam("ref")) - if ctx.Written() { + refCommit, err := utils.ResolveRefCommit(ctx, ctx.PathParam("ref")) + if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.APIErrorNotFound(err) + } else { + ctx.APIErrorInternal(err) + } return } repo := ctx.Repo.Repository - statuses, count, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, utils.GetListOptions(ctx)) + statuses, count, err := git_model.GetLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String(), utils.GetListOptions(ctx)) if err != nil { - ctx.APIErrorInternal(fmt.Errorf("GetLatestCommitStatus[%s, %s]: %w", repo.FullName(), sha, err)) + ctx.APIErrorInternal(fmt.Errorf("GetLatestCommitStatus[%s, %s]: %w", repo.FullName(), refCommit.CommitID, err)) return } diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index 7d276d9d98bae..79d7945681dee 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -4,48 +4,38 @@ package utils import ( - gocontext "context" - "errors" - "fmt" - "net/http" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/services/context" + "errors" ) -// ResolveRefOrSha resolve ref to sha if exist -func ResolveRefOrSha(ctx *context.APIContext, ref string) string { - if len(ref) == 0 { - ctx.APIError(http.StatusBadRequest, nil) - return "" - } - - sha := ref - // Search branches and tags - for _, refType := range []string{"heads", "tags"} { - refSHA, lastMethodName, err := searchRefCommitByType(ctx, refType, ref) - if err != nil { - ctx.APIErrorInternal(fmt.Errorf("%s: %w", lastMethodName, err)) - return "" - } - if refSHA != "" { - sha = refSHA - break - } - } - - sha = MustConvertToSHA1(ctx, ctx.Repo, sha) - - if ctx.Repo.GitRepo != nil { - err := ctx.Repo.GitRepo.AddLastCommitCache(ctx.Repo.Repository.GetCommitsCountCacheKey(ref, ref != sha), ctx.Repo.Repository.FullName(), sha) - if err != nil { - log.Error("Unable to get commits count for %s in %s. Error: %v", sha, ctx.Repo.Repository.FullName(), err) - } - } +type RefCommit struct { + InputRef string + Ref git.RefName + Commit *git.Commit + CommitID string +} - return sha +// ResolveRefCommit resolve ref to a commit if exist +func ResolveRefCommit(ctx *context.APIContext, inputRef string) (_ *RefCommit, err error) { + var refCommit RefCommit + if gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, inputRef) { + refCommit.Ref = git.RefNameFromBranch(inputRef) + } else if gitrepo.IsTagExist(ctx, ctx.Repo.Repository, inputRef) { + refCommit.Ref = git.RefNameFromTag(inputRef) + } else if len(inputRef) == ctx.Repo.GetObjectFormat().FullLength() { + refCommit.Ref = git.RefNameFromCommit(inputRef) + } + if refCommit.Ref == "" { + return nil, git.ErrNotExist{ID: inputRef} + } + if refCommit.Commit, err = ctx.Repo.GitRepo.GetCommit(refCommit.Ref.String()); err != nil { + return nil, err + } + refCommit.InputRef = inputRef + refCommit.CommitID = refCommit.Commit.ID.String() + return &refCommit, nil } // GetGitRefs return git references based on filter @@ -59,42 +49,3 @@ func GetGitRefs(ctx *context.APIContext, filter string) ([]*git.Reference, strin refs, err := ctx.Repo.GitRepo.GetRefsFiltered(filter) return refs, "GetRefsFiltered", err } - -func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (string, string, error) { - refs, lastMethodName, err := GetGitRefs(ctx, refType+"/"+filter) // Search by type - if err != nil { - return "", lastMethodName, err - } - if len(refs) > 0 { - return refs[0].Object.String(), "", nil // Return found SHA - } - return "", "", nil -} - -// ConvertToObjectID returns a full-length SHA1 from a potential ID string -func ConvertToObjectID(ctx gocontext.Context, repo *context.Repository, commitID string) (git.ObjectID, error) { - objectFormat := repo.GetObjectFormat() - if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) { - sha, err := git.NewIDFromString(commitID) - if err == nil { - return sha, nil - } - } - - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo.Repository) - if err != nil { - return objectFormat.EmptyObjectID(), fmt.Errorf("RepositoryFromContextOrOpen: %w", err) - } - defer closer.Close() - - return gitRepo.ConvertToGitID(commitID) -} - -// MustConvertToSHA1 returns a full-length SHA1 string from a potential ID string, or returns origin input if it can't convert to SHA1 -func MustConvertToSHA1(ctx gocontext.Context, repo *context.Repository, commitID string) string { - sha, err := ConvertToObjectID(ctx, repo, commitID) - if err != nil { - return commitID - } - return sha.String() -} diff --git a/services/context/api.go b/services/context/api.go index 6e3b635ce366d..9b3c0ffa1ffd4 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -5,6 +5,7 @@ package context import ( + "code.gitea.io/gitea/modules/git" "errors" "fmt" "net/http" @@ -245,7 +246,7 @@ func APIContexter() func(http.Handler) http.Handler { // String will replace message, errors will be added to a slice func (ctx *APIContext) APIErrorNotFound(objs ...any) { message := ctx.Locale.TrString("error.not_found") - var errors []string + var errs []string for _, obj := range objs { // Ignore nil if obj == nil { @@ -253,7 +254,7 @@ func (ctx *APIContext) APIErrorNotFound(objs ...any) { } if err, ok := obj.(error); ok { - errors = append(errors, err.Error()) + errs = append(errs, err.Error()) } else { message = obj.(string) } @@ -262,7 +263,7 @@ func (ctx *APIContext) APIErrorNotFound(objs ...any) { ctx.JSON(http.StatusNotFound, map[string]any{ "message": message, "url": setting.API.SwaggerURL, - "errors": errors, + "errors": errs, }) } @@ -298,39 +299,27 @@ func RepoRefForAPI(next http.Handler) http.Handler { } if ctx.Repo.GitRepo == nil { - ctx.APIErrorInternal(errors.New("no open git repo")) + panic("no GitRepo, forgot to call the middleware?") // it is a programming error return } - refName, _, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref")) + refName, refType, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref")) var err error - - if gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, refName) { + if refType == git.RefTypeBranch { ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) - if err != nil { - ctx.APIErrorInternal(err) - return - } - ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if gitrepo.IsTagExist(ctx, ctx.Repo.Repository, refName) { + } else if refType == git.RefTypeTag { ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetTagCommit(refName) - if err != nil { - ctx.APIErrorInternal(err) - return - } - ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if len(refName) == ctx.Repo.GetObjectFormat().FullLength() { - ctx.Repo.CommitID = refName + } else if refType == git.RefTypeCommit { ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) - if err != nil { - ctx.APIErrorNotFound("GetCommit", err) - return - } - } else { + } + if ctx.Repo.Commit == nil || errors.Is(err, util.ErrNotExist) { ctx.APIErrorNotFound(fmt.Errorf("not exist: '%s'", ctx.PathParam("*"))) return + } else if err != nil { + ctx.APIErrorInternal(err) + return } - + ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() next.ServeHTTP(w, req) }) } diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 0327e7f2cebeb..6049c37a1efc3 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -4,18 +4,17 @@ package files import ( - "context" - "fmt" - "net/url" - "path" - "strings" - repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/routers/api/v1/utils" + "context" + "fmt" + "net/url" + "path" ) // ContentType repo content type @@ -23,14 +22,10 @@ type ContentType string // The string representations of different content types const ( - // ContentTypeRegular regular content type (file) - ContentTypeRegular ContentType = "file" - // ContentTypeDir dir content type (dir) - ContentTypeDir ContentType = "dir" - // ContentLink link content type (symlink) - ContentTypeLink ContentType = "symlink" - // ContentTag submodule content type (submodule) - ContentTypeSubmodule ContentType = "submodule" + ContentTypeRegular ContentType = "file" // regular content type (file) + ContentTypeDir ContentType = "dir" // dir content type (dir) + ContentTypeLink ContentType = "symlink" // link content type (symlink) + ContentTypeSubmodule ContentType = "submodule" // submodule content type (submodule) ) // String gets the string of ContentType @@ -38,16 +33,12 @@ func (ct *ContentType) String() string { return string(*ct) } -// GetContentsOrList gets the meta data of a file's contents (*ContentsResponse) if treePath not a tree +// GetContentsOrList gets the metadata of a file's contents (*ContentsResponse) if treePath not a tree // directory, otherwise a listing of file contents ([]*ContentsResponse). Ref can be a branch, commit or tag -func GetContentsOrList(ctx context.Context, repo *repo_model.Repository, treePath, ref string) (any, error) { +func GetContentsOrList(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treePath string) (any, error) { if repo.IsEmpty { return make([]any, 0), nil } - if ref == "" { - ref = repo.DefaultBranch - } - origRef := ref // Check that the path given in opts.treePath is valid (not a git path) cleanTreePath := CleanUploadFileName(treePath) @@ -58,17 +49,8 @@ func GetContentsOrList(ctx context.Context, repo *repo_model.Repository, treePat } treePath = cleanTreePath - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo) - if err != nil { - return nil, err - } - defer closer.Close() - // Get the commit object for the ref - commit, err := gitRepo.GetCommit(ref) - if err != nil { - return nil, err - } + commit := refCommit.Commit entry, err := commit.GetTreeEntryByPath(treePath) if err != nil { @@ -76,7 +58,7 @@ func GetContentsOrList(ctx context.Context, repo *repo_model.Repository, treePat } if entry.Type() != "tree" { - return GetContents(ctx, repo, treePath, origRef, false) + return GetContents(ctx, repo, refCommit, treePath, false) } // We are in a directory, so we return a list of FileContentResponse objects @@ -92,7 +74,7 @@ func GetContentsOrList(ctx context.Context, repo *repo_model.Repository, treePat } for _, e := range entries { subTreePath := path.Join(treePath, e.Name()) - fileContentResponse, err := GetContents(ctx, repo, subTreePath, origRef, true) + fileContentResponse, err := GetContents(ctx, repo, refCommit, subTreePath, true) if err != nil { return nil, err } @@ -117,13 +99,8 @@ func GetObjectTypeFromTreeEntry(entry *git.TreeEntry) ContentType { } } -// GetContents gets the meta data on a file's contents. Ref can be a branch, commit or tag -func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref string, forList bool) (*api.ContentsResponse, error) { - if ref == "" { - ref = repo.DefaultBranch - } - origRef := ref - +// GetContents gets the metadata on a file's contents. Ref can be a branch, commit or tag +func GetContents(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treePath string, forList bool) (*api.ContentsResponse, error) { // Check that the path given in opts.treePath is valid (not a git path) cleanTreePath := CleanUploadFileName(treePath) if cleanTreePath == "" && treePath != "" { @@ -139,33 +116,24 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } defer closer.Close() - // Get the commit object for the ref - commit, err := gitRepo.GetCommit(ref) - if err != nil { - return nil, err - } - commitID := commit.ID.String() - if len(ref) >= 4 && strings.HasPrefix(commitID, ref) { - ref = commit.ID.String() - } - + commit := refCommit.Commit entry, err := commit.GetTreeEntryByPath(treePath) if err != nil { return nil, err } - refType := gitRepo.GetRefType(ref) - if refType == "invalid" { - return nil, fmt.Errorf("no commit found for the ref [ref: %s]", ref) + refType := refCommit.Ref.RefType() + if refType != git.RefTypeBranch && refType != git.RefTypeTag && refType != git.RefTypeCommit { + return nil, fmt.Errorf("no commit found for the ref [ref: %s]", refCommit.Ref) } - selfURL, err := url.Parse(repo.APIURL() + "/contents/" + util.PathEscapeSegments(treePath) + "?ref=" + url.QueryEscape(origRef)) + selfURL, err := url.Parse(repo.APIURL() + "/contents/" + util.PathEscapeSegments(treePath) + "?ref=" + url.QueryEscape(refCommit.InputRef)) if err != nil { return nil, err } selfURLString := selfURL.String() - err = gitRepo.AddLastCommitCache(repo.GetCommitsCountCacheKey(ref, refType != git.ObjectCommit), repo.FullName(), commitID) + err = gitRepo.AddLastCommitCache(repo.GetCommitsCountCacheKey(refCommit.InputRef, refType != git.RefTypeCommit), repo.FullName(), refCommit.CommitID) if err != nil { return nil, err } @@ -196,13 +164,16 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref if lastCommit.Author != nil { contentsResponse.LastAuthorDate = lastCommit.Author.When } + // Now populate the rest of the ContentsResponse based on entry type if entry.IsRegular() || entry.IsExecutable() { contentsResponse.Type = string(ContentTypeRegular) - if blobResponse, err := GetBlobBySHA(ctx, repo, gitRepo, entry.ID.String()); err != nil { - return nil, err - } else if !forList { - // We don't show the content if we are getting a list of FileContentResponses + // if it is listing the repo root dir, don't waste system resources on reading content + if !forList { + blobResponse, err := GetBlobBySHA(ctx, repo, gitRepo, entry.ID.String()) + if err != nil { + return nil, err + } contentsResponse.Encoding = &blobResponse.Encoding contentsResponse.Content = &blobResponse.Content } @@ -228,7 +199,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } // Handle links if entry.IsRegular() || entry.IsLink() || entry.IsExecutable() { - downloadURL, err := url.Parse(repo.HTMLURL() + "/raw/" + url.PathEscape(string(refType)) + "/" + util.PathEscapeSegments(ref) + "/" + util.PathEscapeSegments(treePath)) + downloadURL, err := url.Parse(repo.HTMLURL() + "/raw/" + refCommit.Ref.RefWebLinkPath() + "/" + util.PathEscapeSegments(treePath)) if err != nil { return nil, err } @@ -236,7 +207,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref contentsResponse.DownloadURL = &downloadURLString } if !entry.IsSubModule() { - htmlURL, err := url.Parse(repo.HTMLURL() + "/src/" + url.PathEscape(string(refType)) + "/" + util.PathEscapeSegments(ref) + "/" + util.PathEscapeSegments(treePath)) + htmlURL, err := url.Parse(repo.HTMLURL() + "/src/" + refCommit.Ref.RefWebLinkPath() + "/" + util.PathEscapeSegments(treePath)) if err != nil { return nil, err } @@ -262,18 +233,17 @@ func GetBlobBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git if err != nil { return nil, err } - content := "" + ret := &api.GitBlobResponse{ + SHA: gitBlob.ID.String(), + URL: repo.APIURL() + "/git/blobs/" + url.PathEscape(gitBlob.ID.String()), + Size: gitBlob.Size(), + } if gitBlob.Size() <= setting.API.DefaultMaxBlobSize { - content, err = gitBlob.GetBlobContentBase64() + ret.Encoding = "base64" + ret.Content, err = gitBlob.GetBlobContentBase64() if err != nil { return nil, err } } - return &api.GitBlobResponse{ - SHA: gitBlob.ID.String(), - URL: repo.APIURL() + "/git/blobs/" + url.PathEscape(gitBlob.ID.String()), - Size: gitBlob.Size(), - Encoding: "base64", - Content: content, - }, nil + return ret, nil } diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 3e063216236d9..0cdc64f78babc 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -4,6 +4,7 @@ package files import ( + "code.gitea.io/gitea/routers/api/v1/utils" "context" "errors" "fmt" @@ -18,13 +19,15 @@ import ( "code.gitea.io/gitea/modules/util" ) -func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, branch string, treeNames []string) []*api.ContentsResponse { - files := []*api.ContentsResponse{} +func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treeNames []string) []*api.ContentsResponse { + var files []*api.ContentsResponse var size int64 for _, file := range treeNames { - fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil + fileContents, _ := GetContents(ctx, repo, refCommit, file, false) // ok if fails, then will be nil if fileContents != nil && *fileContents.Content != "" { - size += fileContents.Size // if content isn't empty (e. g. due to the single blob being too large), add file size to response size + // if content isn't empty (e.g. due to the single blob being too large), add file size to response size + // the content is base64 encoded + size += fileContents.Size * 4 / 3 } if size > setting.API.DefaultMaxResponseSize { return files // return if max page size would be exceeded diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 75ede4976f9a2..daa91972978f0 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -296,6 +296,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil, err } + // FIXME: this call seems not right, why it needs to read the file content again filesResponse, err := GetFilesResponseFromCommit(ctx, repo, commit, opts.NewBranch, treePaths) if err != nil { return nil, err From 114d73f89b928b22d2fe056cb137268384670aac Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 16:05:28 +0800 Subject: [PATCH 18/31] temp --- routers/api/v1/repo/file.go | 43 ++++++++++++++-------------- routers/api/v1/repo/status.go | 2 +- routers/api/v1/utils/git.go | 7 ++++- services/context/api.go | 2 +- services/repository/files/content.go | 9 +++--- services/repository/files/file.go | 10 +++---- services/repository/files/update.go | 3 +- 7 files changed, 41 insertions(+), 35 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 2fbf789258092..c8011de315597 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -25,7 +25,9 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" pull_service "code.gitea.io/gitea/services/pull" @@ -894,6 +896,17 @@ func DeleteFile(ctx *context.APIContext) { } } +func resolveRefCommit(ctx *context.APIContext, ref string) *utils.RefCommit { + ref = util.IfZero(ref, ctx.Repo.Repository.DefaultBranch) + refCommit, err := utils.ResolveRefCommit(ctx, ref) + if errors.Is(err, util.ErrNotExist) { + ctx.APIErrorNotFound(err) + } else if err != nil { + ctx.APIErrorInternal(err) + } + return refCommit +} + // GetContents Get the metadata and contents (if a file) of an entry in a repository, or a list of entries if a dir func GetContents(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/contents/{filepath} repository repoGetContents @@ -928,18 +941,13 @@ func GetContents(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - if !canReadFiles(ctx.Repo) { - ctx.APIErrorInternal(repo_model.ErrUserDoesNotHaveAccessToRepo{ - UserID: ctx.Doer.ID, - RepoName: ctx.Repo.Repository.LowerName, - }) + treePath := ctx.PathParam("*") + refCommit := resolveRefCommit(ctx, ctx.FormTrim("ref")) + if ctx.Written() { return } - treePath := ctx.PathParam("*") - ref := ctx.FormTrim("ref") - - if fileList, err := files_service.GetContentsOrList(ctx, ctx.Repo.Repository, treePath, ref); err != nil { + if fileList, err := files_service.GetContentsOrList(ctx, ctx.Repo.Repository, refCommit, treePath); err != nil { if git.IsErrNotExist(err) { ctx.APIErrorNotFound("GetContentsOrList", err) return @@ -1023,19 +1031,10 @@ func GetFiles(ctx *context.APIContext) { // "$ref": "#/responses/notFound" apiOpts := web.GetForm(ctx).(*api.GetFilesOptions) - - ref := ctx.FormTrim("ref") - if ref == "" { - ref = ctx.Repo.Repository.DefaultBranch - } - - if !ctx.Repo.GitRepo.IsReferenceExist(ref) { - ctx.APIErrorNotFound("GetFiles", "ref does not exist") + refCommit := resolveRefCommit(ctx, ctx.FormTrim("ref")) + if ctx.Written() { return } - - files := apiOpts.Files - filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, ref, files) - - ctx.JSON(http.StatusOK, filesResponse) + filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, refCommit, apiOpts.Files) + ctx.JSON(http.StatusOK, util.SliceNilAsEmpty(filesResponse)) } diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go index 38ddc15e43734..18a4615d09276 100644 --- a/routers/api/v1/repo/status.go +++ b/routers/api/v1/repo/status.go @@ -4,7 +4,6 @@ package repo import ( - "code.gitea.io/gitea/modules/util" "errors" "fmt" "net/http" @@ -12,6 +11,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/context" diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index 79d7945681dee..beb315ba4d60e 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -4,10 +4,11 @@ package utils import ( + "errors" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/services/context" - "errors" ) type RefCommit struct { @@ -38,6 +39,10 @@ func ResolveRefCommit(ctx *context.APIContext, inputRef string) (_ *RefCommit, e return &refCommit, nil } +func NewRefCommit(refName git.RefName, commit *git.Commit) *RefCommit { + return &RefCommit{InputRef: refName.ShortName(), Ref: refName, Commit: commit, CommitID: commit.ID.String()} +} + // GetGitRefs return git references based on filter func GetGitRefs(ctx *context.APIContext, filter string) ([]*git.Reference, string, error) { if ctx.Repo.GitRepo == nil { diff --git a/services/context/api.go b/services/context/api.go index 9b3c0ffa1ffd4..98a8fc0a24b1f 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -5,7 +5,6 @@ package context import ( - "code.gitea.io/gitea/modules/git" "errors" "fmt" "net/http" @@ -16,6 +15,7 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/log" diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 6049c37a1efc3..3ca0b4aeabe01 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -4,6 +4,11 @@ package files import ( + "context" + "fmt" + "net/url" + "path" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" @@ -11,10 +16,6 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/api/v1/utils" - "context" - "fmt" - "net/url" - "path" ) // ContentType repo content type diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 0cdc64f78babc..6699e8348cbb6 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -4,7 +4,6 @@ package files import ( - "code.gitea.io/gitea/routers/api/v1/utils" "context" "errors" "fmt" @@ -17,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/routers/api/v1/utils" ) func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treeNames []string) []*api.ContentsResponse { @@ -40,10 +40,10 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, return files } -func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch string, treeNames []string) (*api.FilesResponse, error) { - files := GetContentsListFromTrees(ctx, repo, branch, treeNames) - fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil - verification := GetPayloadCommitVerification(ctx, commit) +func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treeNames []string) (*api.FilesResponse, error) { + files := GetContentsListFromTrees(ctx, repo, refCommit, treeNames) + fileCommitResponse, _ := GetFileCommitResponse(repo, refCommit.Commit) // ok if fails, then will be nil + verification := GetPayloadCommitVerification(ctx, refCommit.Commit) filesResponse := &api.FilesResponse{ Files: files, Commit: fileCommitResponse, diff --git a/services/repository/files/update.go b/services/repository/files/update.go index daa91972978f0..574c76b1ba759 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/routers/api/v1/utils" asymkey_service "code.gitea.io/gitea/services/asymkey" pull_service "code.gitea.io/gitea/services/pull" ) @@ -297,7 +298,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } // FIXME: this call seems not right, why it needs to read the file content again - filesResponse, err := GetFilesResponseFromCommit(ctx, repo, commit, opts.NewBranch, treePaths) + filesResponse, err := GetFilesResponseFromCommit(ctx, repo, utils.NewRefCommit(git.RefNameFromCommit(commit.ID.String()), commit), treePaths) if err != nil { return nil, err } From d173d4eb6fa28ccd6e34ffa9354c5554a8f91e67 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 16:07:41 +0800 Subject: [PATCH 19/31] temp --- routers/api/v1/repo/status.go | 20 ++++---------------- routers/api/v1/utils/git.go | 3 +-- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go index 18a4615d09276..6c4bbb440d7d2 100644 --- a/routers/api/v1/repo/status.go +++ b/routers/api/v1/repo/status.go @@ -4,14 +4,12 @@ package repo import ( - "errors" "fmt" "net/http" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/context" @@ -179,13 +177,8 @@ func GetCommitStatusesByRef(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - refCommit, err := utils.ResolveRefCommit(ctx, ctx.PathParam("ref")) - if err != nil { - if errors.Is(err, util.ErrNotExist) { - ctx.APIErrorNotFound(err) - } else { - ctx.APIErrorInternal(err) - } + refCommit := resolveRefCommit(ctx, ctx.PathParam("ref")) + if ctx.Written() { return } getCommitStatuses(ctx, refCommit.CommitID) @@ -258,13 +251,8 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - refCommit, err := utils.ResolveRefCommit(ctx, ctx.PathParam("ref")) - if err != nil { - if errors.Is(err, util.ErrNotExist) { - ctx.APIErrorNotFound(err) - } else { - ctx.APIErrorInternal(err) - } + refCommit := resolveRefCommit(ctx, ctx.PathParam("ref")) + if ctx.Written() { return } diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index beb315ba4d60e..bb54eab579330 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -20,7 +20,7 @@ type RefCommit struct { // ResolveRefCommit resolve ref to a commit if exist func ResolveRefCommit(ctx *context.APIContext, inputRef string) (_ *RefCommit, err error) { - var refCommit RefCommit + refCommit := RefCommit{InputRef: inputRef} if gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, inputRef) { refCommit.Ref = git.RefNameFromBranch(inputRef) } else if gitrepo.IsTagExist(ctx, ctx.Repo.Repository, inputRef) { @@ -34,7 +34,6 @@ func ResolveRefCommit(ctx *context.APIContext, inputRef string) (_ *RefCommit, e if refCommit.Commit, err = ctx.Repo.GitRepo.GetCommit(refCommit.Ref.String()); err != nil { return nil, err } - refCommit.InputRef = inputRef refCommit.CommitID = refCommit.Commit.ID.String() return &refCommit, nil } From f6d949be496def2b0b1bc54990be712b641eff14 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 17:12:08 +0800 Subject: [PATCH 20/31] fix tests --- routers/api/v1/api.go | 6 +- routers/api/v1/repo/file.go | 2 +- routers/api/v1/utils/git.go | 16 +++-- services/contexttest/context_tests.go | 15 ++++- services/repository/files/content_test.go | 81 +++++------------------ services/repository/files/update.go | 3 +- 6 files changed, 46 insertions(+), 77 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index bf2b6e85f4426..53fd2cb7a96b1 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1389,15 +1389,15 @@ func Routes() *web.Router { m.Post("/diffpatch", reqRepoWriter(unit.TypeCode), reqToken(), bind(api.ApplyDiffPatchFileOptions{}), mustNotBeArchived, repo.ApplyDiffPatch) m.Group("/contents", func() { m.Get("", repo.GetContentsList) - m.Post("", reqToken(), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.ChangeFiles) m.Get("/*", repo.GetContents) + m.Post("", reqToken(), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.ChangeFiles) m.Group("/*", func() { m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.CreateFile) m.Put("", bind(api.UpdateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.UpdateFile) m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile) }, reqToken()) - }, reqRepoReader(unit.TypeCode)) - m.Post("/files", context.ReferencesGitRepo(), context.RepoRefForAPI, bind(api.GetFilesOptions{}), reqRepoReader(unit.TypeCode), repo.GetFiles) + }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) + m.Post("/files", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(), context.RepoRefForAPI, bind(api.GetFilesOptions{}), repo.GetFiles) m.Get("/signing-key.gpg", misc.SigningKey) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index c8011de315597..b79c5491b0e0e 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -898,7 +898,7 @@ func DeleteFile(ctx *context.APIContext) { func resolveRefCommit(ctx *context.APIContext, ref string) *utils.RefCommit { ref = util.IfZero(ref, ctx.Repo.Repository.DefaultBranch) - refCommit, err := utils.ResolveRefCommit(ctx, ref) + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ref) if errors.Is(err, util.ErrNotExist) { ctx.APIErrorNotFound(err) } else if err != nil { diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index bb54eab579330..dbb889b888458 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -6,8 +6,10 @@ package utils import ( "errors" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/reqctx" "code.gitea.io/gitea/services/context" ) @@ -19,19 +21,23 @@ type RefCommit struct { } // ResolveRefCommit resolve ref to a commit if exist -func ResolveRefCommit(ctx *context.APIContext, inputRef string) (_ *RefCommit, err error) { +func ResolveRefCommit(ctx reqctx.RequestContext, repo *repo_model.Repository, inputRef string) (_ *RefCommit, err error) { + gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) + if err != nil { + return nil, err + } refCommit := RefCommit{InputRef: inputRef} - if gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, inputRef) { + if gitrepo.IsBranchExist(ctx, repo, inputRef) { refCommit.Ref = git.RefNameFromBranch(inputRef) - } else if gitrepo.IsTagExist(ctx, ctx.Repo.Repository, inputRef) { + } else if gitrepo.IsTagExist(ctx, repo, inputRef) { refCommit.Ref = git.RefNameFromTag(inputRef) - } else if len(inputRef) == ctx.Repo.GetObjectFormat().FullLength() { + } else if len(inputRef) == git.ObjectFormatFromName(repo.ObjectFormatName).FullLength() { refCommit.Ref = git.RefNameFromCommit(inputRef) } if refCommit.Ref == "" { return nil, git.ErrNotExist{ID: inputRef} } - if refCommit.Commit, err = ctx.Repo.GitRepo.GetCommit(refCommit.Ref.String()); err != nil { + if refCommit.Commit, err = gitRepo.GetCommit(refCommit.Ref.String()); err != nil { return nil, err } refCommit.CommitID = refCommit.Commit.ID.String() diff --git a/services/contexttest/context_tests.go b/services/contexttest/context_tests.go index c895de3569730..b54023897b072 100644 --- a/services/contexttest/context_tests.go +++ b/services/contexttest/context_tests.go @@ -170,10 +170,19 @@ func LoadUser(t *testing.T, ctx gocontext.Context, userID int64) { // LoadGitRepo load a git repo into a test context. Requires that ctx.Repo has // already been populated. -func LoadGitRepo(t *testing.T, ctx *context.Context) { - assert.NoError(t, ctx.Repo.Repository.LoadOwner(ctx)) +func LoadGitRepo(t *testing.T, ctx gocontext.Context) { + var repo *context.Repository + switch ctx := any(ctx).(type) { + case *context.Context: + repo = ctx.Repo + case *context.APIContext: + repo = ctx.Repo + default: + assert.FailNow(t, "context is not *context.Context or *context.APIContext") + } + assert.NoError(t, repo.Repository.LoadOwner(ctx)) var err error - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + repo.GitRepo, err = gitrepo.OpenRepository(ctx, repo.Repository) assert.NoError(t, err) } diff --git a/services/repository/files/content_test.go b/services/repository/files/content_test.go index e066f30cbafd6..cb6ed583cafc9 100644 --- a/services/repository/files/content_test.go +++ b/services/repository/files/content_test.go @@ -10,11 +10,13 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/gitrepo" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/contexttest" _ "code.gitea.io/gitea/models/actions" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMain(m *testing.M) { @@ -64,18 +66,13 @@ func TestGetContents(t *testing.T) { defer ctx.Repo.GitRepo.Close() treePath := "README.md" - ref := ctx.Repo.Repository.DefaultBranch + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch) + require.NoError(t, err) expectedContentsResponse := getExpectedReadmeContentsResponse() t.Run("Get README.md contents with GetContents(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContents(ctx, ctx.Repo.Repository, treePath, ref, false) - assert.Equal(t, expectedContentsResponse, fileContentResponse) - assert.NoError(t, err) - }) - - t.Run("Get README.md contents with ref as empty string (should then use the repo's default branch) with GetContents(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContents(ctx, ctx.Repo.Repository, treePath, "", false) + fileContentResponse, err := GetContents(ctx, ctx.Repo.Repository, refCommit, treePath, false) assert.Equal(t, expectedContentsResponse, fileContentResponse) assert.NoError(t, err) }) @@ -92,7 +89,8 @@ func TestGetContentsOrListForDir(t *testing.T) { defer ctx.Repo.GitRepo.Close() treePath := "" // root dir - ref := ctx.Repo.Repository.DefaultBranch + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch) + require.NoError(t, err) readmeContentsResponse := getExpectedReadmeContentsResponse() // because will be in a list, doesn't have encoding and content @@ -104,13 +102,7 @@ func TestGetContentsOrListForDir(t *testing.T) { } t.Run("Get root dir contents with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, ref) - assert.EqualValues(t, expectedContentsListResponse, fileContentResponse) - assert.NoError(t, err) - }) - - t.Run("Get root dir contents with ref as empty string (should then use the repo's default branch) with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, "") + fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, refCommit, treePath) assert.EqualValues(t, expectedContentsListResponse, fileContentResponse) assert.NoError(t, err) }) @@ -127,18 +119,13 @@ func TestGetContentsOrListForFile(t *testing.T) { defer ctx.Repo.GitRepo.Close() treePath := "README.md" - ref := ctx.Repo.Repository.DefaultBranch + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch) + require.NoError(t, err) expectedContentsResponse := getExpectedReadmeContentsResponse() t.Run("Get README.md contents with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, ref) - assert.EqualValues(t, expectedContentsResponse, fileContentResponse) - assert.NoError(t, err) - }) - - t.Run("Get README.md contents with ref as empty string (should then use the repo's default branch) with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, "") + fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, refCommit, treePath) assert.EqualValues(t, expectedContentsResponse, fileContentResponse) assert.NoError(t, err) }) @@ -155,24 +142,16 @@ func TestGetContentsErrors(t *testing.T) { defer ctx.Repo.GitRepo.Close() repo := ctx.Repo.Repository - treePath := "README.md" - ref := repo.DefaultBranch + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch) + require.NoError(t, err) t.Run("bad treePath", func(t *testing.T) { badTreePath := "bad/tree.md" - fileContentResponse, err := GetContents(ctx, repo, badTreePath, ref, false) + fileContentResponse, err := GetContents(ctx, repo, refCommit, badTreePath, false) assert.Error(t, err) assert.EqualError(t, err, "object does not exist [id: , rel_path: bad]") assert.Nil(t, fileContentResponse) }) - - t.Run("bad ref", func(t *testing.T) { - badRef := "bad_ref" - fileContentResponse, err := GetContents(ctx, repo, treePath, badRef, false) - assert.Error(t, err) - assert.EqualError(t, err, "object does not exist [id: "+badRef+", rel_path: ]") - assert.Nil(t, fileContentResponse) - }) } func TestGetContentsOrListErrors(t *testing.T) { @@ -186,42 +165,16 @@ func TestGetContentsOrListErrors(t *testing.T) { defer ctx.Repo.GitRepo.Close() repo := ctx.Repo.Repository - treePath := "README.md" - ref := repo.DefaultBranch + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch) + require.NoError(t, err) t.Run("bad treePath", func(t *testing.T) { badTreePath := "bad/tree.md" - fileContentResponse, err := GetContentsOrList(ctx, repo, badTreePath, ref) + fileContentResponse, err := GetContentsOrList(ctx, repo, refCommit, badTreePath) assert.Error(t, err) assert.EqualError(t, err, "object does not exist [id: , rel_path: bad]") assert.Nil(t, fileContentResponse) }) - - t.Run("bad ref", func(t *testing.T) { - badRef := "bad_ref" - fileContentResponse, err := GetContentsOrList(ctx, repo, treePath, badRef) - assert.Error(t, err) - assert.EqualError(t, err, "object does not exist [id: "+badRef+", rel_path: ]") - assert.Nil(t, fileContentResponse) - }) -} - -func TestGetContentsOrListOfEmptyRepos(t *testing.T) { - unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user30/empty") - ctx.SetPathParam("id", "52") - contexttest.LoadRepo(t, ctx, 52) - contexttest.LoadUser(t, ctx, 30) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - - t.Run("empty repo", func(t *testing.T) { - contents, err := GetContentsOrList(ctx, repo, "", "") - assert.NoError(t, err) - assert.Empty(t, contents) - }) } func TestGetBlobBySHA(t *testing.T) { diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 574c76b1ba759..fbf59c40edb97 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -298,7 +298,8 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } // FIXME: this call seems not right, why it needs to read the file content again - filesResponse, err := GetFilesResponseFromCommit(ctx, repo, utils.NewRefCommit(git.RefNameFromCommit(commit.ID.String()), commit), treePaths) + // FIXME: why it uses the NewBranch as "ref", it should use the commit ID because the response is only for this commit + filesResponse, err := GetFilesResponseFromCommit(ctx, repo, utils.NewRefCommit(git.RefNameFromBranch(opts.NewBranch), commit), treePaths) if err != nil { return nil, err } From aa2cc662b94db5cd1414600dc91431e954228e4c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 17:21:56 +0800 Subject: [PATCH 21/31] fix lint & test --- routers/api/v1/api.go | 2 +- routers/api/v1/repo/file.go | 4 ++-- routers/api/v1/repo/status.go | 4 ++-- routers/api/v1/utils/git.go | 18 +++++++++--------- services/context/api.go | 8 ++++---- services/repository/files/content.go | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 53fd2cb7a96b1..3f4639fc8641e 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1397,7 +1397,7 @@ func Routes() *web.Router { m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile) }, reqToken()) }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) - m.Post("/files", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(), context.RepoRefForAPI, bind(api.GetFilesOptions{}), repo.GetFiles) + m.Post("/files", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(), bind(api.GetFilesOptions{}), repo.GetFiles) m.Get("/signing-key.gpg", misc.SigningKey) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index b79c5491b0e0e..adcc26d95a384 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -896,9 +896,9 @@ func DeleteFile(ctx *context.APIContext) { } } -func resolveRefCommit(ctx *context.APIContext, ref string) *utils.RefCommit { +func resolveRefCommit(ctx *context.APIContext, ref string, minCommitIDLen ...int) *utils.RefCommit { ref = util.IfZero(ref, ctx.Repo.Repository.DefaultBranch) - refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ref) + refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ref, minCommitIDLen...) if errors.Is(err, util.ErrNotExist) { ctx.APIErrorNotFound(err) } else if err != nil { diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go index 6c4bbb440d7d2..756adcf3a380e 100644 --- a/routers/api/v1/repo/status.go +++ b/routers/api/v1/repo/status.go @@ -177,7 +177,7 @@ func GetCommitStatusesByRef(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - refCommit := resolveRefCommit(ctx, ctx.PathParam("ref")) + refCommit := resolveRefCommit(ctx, ctx.PathParam("ref"), 7) if ctx.Written() { return } @@ -251,7 +251,7 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - refCommit := resolveRefCommit(ctx, ctx.PathParam("ref")) + refCommit := resolveRefCommit(ctx, ctx.PathParam("ref"), 7) if ctx.Written() { return } diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index dbb889b888458..1cfe01a639903 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -15,29 +15,29 @@ import ( type RefCommit struct { InputRef string - Ref git.RefName + RefName git.RefName Commit *git.Commit CommitID string } // ResolveRefCommit resolve ref to a commit if exist -func ResolveRefCommit(ctx reqctx.RequestContext, repo *repo_model.Repository, inputRef string) (_ *RefCommit, err error) { +func ResolveRefCommit(ctx reqctx.RequestContext, repo *repo_model.Repository, inputRef string, minCommitIDLen ...int) (_ *RefCommit, err error) { gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) if err != nil { return nil, err } refCommit := RefCommit{InputRef: inputRef} if gitrepo.IsBranchExist(ctx, repo, inputRef) { - refCommit.Ref = git.RefNameFromBranch(inputRef) + refCommit.RefName = git.RefNameFromBranch(inputRef) } else if gitrepo.IsTagExist(ctx, repo, inputRef) { - refCommit.Ref = git.RefNameFromTag(inputRef) - } else if len(inputRef) == git.ObjectFormatFromName(repo.ObjectFormatName).FullLength() { - refCommit.Ref = git.RefNameFromCommit(inputRef) + refCommit.RefName = git.RefNameFromTag(inputRef) + } else if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.ObjectFormatName), inputRef, minCommitIDLen...) { + refCommit.RefName = git.RefNameFromCommit(inputRef) } - if refCommit.Ref == "" { + if refCommit.RefName == "" { return nil, git.ErrNotExist{ID: inputRef} } - if refCommit.Commit, err = gitRepo.GetCommit(refCommit.Ref.String()); err != nil { + if refCommit.Commit, err = gitRepo.GetCommit(refCommit.RefName.String()); err != nil { return nil, err } refCommit.CommitID = refCommit.Commit.ID.String() @@ -45,7 +45,7 @@ func ResolveRefCommit(ctx reqctx.RequestContext, repo *repo_model.Repository, in } func NewRefCommit(refName git.RefName, commit *git.Commit) *RefCommit { - return &RefCommit{InputRef: refName.ShortName(), Ref: refName, Commit: commit, CommitID: commit.ID.String()} + return &RefCommit{InputRef: refName.ShortName(), RefName: refName, Commit: commit, CommitID: commit.ID.String()} } // GetGitRefs return git references based on filter diff --git a/services/context/api.go b/services/context/api.go index 98a8fc0a24b1f..c79fdcb1006a9 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -300,16 +300,16 @@ func RepoRefForAPI(next http.Handler) http.Handler { if ctx.Repo.GitRepo == nil { panic("no GitRepo, forgot to call the middleware?") // it is a programming error - return } refName, refType, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref")) var err error - if refType == git.RefTypeBranch { + switch { + case refType == git.RefTypeBranch: ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) - } else if refType == git.RefTypeTag { + case refType == git.RefTypeTag: ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetTagCommit(refName) - } else if refType == git.RefTypeCommit { + case refType == git.RefTypeCommit: ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) } if ctx.Repo.Commit == nil || errors.Is(err, util.ErrNotExist) { diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 3ca0b4aeabe01..c2d02c4541c11 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -123,9 +123,9 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, refCommit *ut return nil, err } - refType := refCommit.Ref.RefType() + refType := refCommit.RefName.RefType() if refType != git.RefTypeBranch && refType != git.RefTypeTag && refType != git.RefTypeCommit { - return nil, fmt.Errorf("no commit found for the ref [ref: %s]", refCommit.Ref) + return nil, fmt.Errorf("no commit found for the ref [ref: %s]", refCommit.RefName) } selfURL, err := url.Parse(repo.APIURL() + "/contents/" + util.PathEscapeSegments(treePath) + "?ref=" + url.QueryEscape(refCommit.InputRef)) @@ -200,7 +200,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, refCommit *ut } // Handle links if entry.IsRegular() || entry.IsLink() || entry.IsExecutable() { - downloadURL, err := url.Parse(repo.HTMLURL() + "/raw/" + refCommit.Ref.RefWebLinkPath() + "/" + util.PathEscapeSegments(treePath)) + downloadURL, err := url.Parse(repo.HTMLURL() + "/raw/" + refCommit.RefName.RefWebLinkPath() + "/" + util.PathEscapeSegments(treePath)) if err != nil { return nil, err } @@ -208,7 +208,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, refCommit *ut contentsResponse.DownloadURL = &downloadURLString } if !entry.IsSubModule() { - htmlURL, err := url.Parse(repo.HTMLURL() + "/src/" + refCommit.Ref.RefWebLinkPath() + "/" + util.PathEscapeSegments(treePath)) + htmlURL, err := url.Parse(repo.HTMLURL() + "/src/" + refCommit.RefName.RefWebLinkPath() + "/" + util.PathEscapeSegments(treePath)) if err != nil { return nil, err } From 388058d096880d4f444cf38ce5e5a8b9ad6581a1 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Mon, 21 Apr 2025 12:29:26 +0200 Subject: [PATCH 22/31] Adapt tests for 4/3 size increase --- services/repository/files/file.go | 2 +- tests/integration/api_repo_files_get_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 6699e8348cbb6..e3d745e89faeb 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -26,7 +26,7 @@ func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, fileContents, _ := GetContents(ctx, repo, refCommit, file, false) // ok if fails, then will be nil if fileContents != nil && *fileContents.Content != "" { // if content isn't empty (e.g. due to the single blob being too large), add file size to response size - // the content is base64 encoded + // the content is base64 encoded, so it's size increases to around 4/3 of the original size size += fileContents.Size * 4 / 3 } if size > setting.API.DefaultMaxResponseSize { diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index c8ba1d21b8dda..46dad021c3db6 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -235,5 +235,5 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) - assert.Len(t, contentsListResponse, 20) + assert.Len(t, contentsListResponse, 15) // base64-encoded content is around 4/3 the size of the original content } From 907118759ed089fec1bd6f75936eff706a75e634 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 17:45:27 +0800 Subject: [PATCH 23/31] fix lint & test --- routers/api/v1/repo/file.go | 6 ------ services/context/api.go | 8 ++++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index adcc26d95a384..d475163e16b1c 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -16,7 +16,6 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" @@ -412,11 +411,6 @@ func canWriteFiles(ctx *context.APIContext, branch string) bool { !ctx.Repo.Repository.IsArchived } -// canReadFiles returns true if repository is readable and user has proper access level. -func canReadFiles(r *context.Repository) bool { - return r.Permission.CanRead(unit.TypeCode) -} - func base64Reader(s string) (io.ReadSeeker, error) { b, err := base64.StdEncoding.DecodeString(s) if err != nil { diff --git a/services/context/api.go b/services/context/api.go index c79fdcb1006a9..041e69ee64b52 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -304,12 +304,12 @@ func RepoRefForAPI(next http.Handler) http.Handler { refName, refType, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref")) var err error - switch { - case refType == git.RefTypeBranch: + switch refType { + case git.RefTypeBranch: ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) - case refType == git.RefTypeTag: + case git.RefTypeTag: ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetTagCommit(refName) - case refType == git.RefTypeCommit: + case git.RefTypeCommit: ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) } if ctx.Repo.Commit == nil || errors.Is(err, util.ErrNotExist) { From 8a2f9f324bd14ad6825bf096ea770818d4456c24 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 19:49:25 +0800 Subject: [PATCH 24/31] fine tune --- tests/integration/api_repo_files_get_test.go | 157 ++++++------------ .../integration/api_repo_get_contents_test.go | 15 +- 2 files changed, 60 insertions(+), 112 deletions(-) diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index 46dad021c3db6..cb65961d96624 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -4,13 +4,6 @@ package integration import ( - "encoding/base64" - "fmt" - "net/http" - "net/url" - "testing" - "time" - auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -20,15 +13,17 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" repo_service "code.gitea.io/gitea/services/repository" - + "encoding/base64" + "fmt" "github.com/stretchr/testify/assert" + "net/http" + "net/url" + "testing" + "time" ) -func getExpectedcontentsListResponseForFiles(ref, refType, lastCommitSHA string) []*api.ContentsResponse { - return []*api.ContentsResponse{getExpectedContentsResponseForContents(ref, refType, lastCommitSHA)} -} - func TestAPIGetRequestedFiles(t *testing.T) { onGiteaRun(t, testAPIGetRequestedFiles) } @@ -41,112 +36,68 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) // public repo repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // public repo repo16 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) // private repo - filesOptions := &api.GetFilesOptions{ - Files: []string{ - "README.md", - }, - } - // Get user2's token req.Body = + // Get user2's token session := loginUser(t, user2.Name) token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // TODO: allow for a POST-request to be scope read // Get user4's token session = loginUser(t, user4.Name) token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // TODO: allow for a POST-request to be scope read - // Get the commit ID of the default branch gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo1) assert.NoError(t, err) defer gitRepo.Close() - - // Make a new branch in repo1 - newBranch := "test_branch" - err = repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, gitRepo, repo1.DefaultBranch, newBranch) - assert.NoError(t, err) - - commitID, err := gitRepo.GetBranchCommitID(repo1.DefaultBranch) - assert.NoError(t, err) - // Make a new tag in repo1 - newTag := "test_tag" - err = gitRepo.CreateTag(newTag, commitID) - assert.NoError(t, err) - /*** END SETUP ***/ - - // ref is default ref - ref := repo1.DefaultBranch - refType := "branch" - req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) - resp := MakeRequest(t, req, http.StatusOK) - var contentsListResponse []*api.ContentsResponse - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) lastCommit, _ := gitRepo.GetCommitByPath("README.md") - expectedcontentsListResponse := getExpectedcontentsListResponseForFiles(ref, refType, lastCommit.ID.String()) - assert.Equal(t, expectedcontentsListResponse, contentsListResponse) - // No ref - refType = "branch" - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo1.Name), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(repo1.DefaultBranch, refType, lastCommit.ID.String()) - assert.Equal(t, expectedcontentsListResponse, contentsListResponse) - - // ref is the branch we created above in setup - ref = newBranch - refType = "branch" - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - branchCommit, _ := gitRepo.GetBranchCommit(ref) - lastCommit, _ = branchCommit.GetCommitByPath("README.md") - expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(ref, refType, lastCommit.ID.String()) - assert.Equal(t, expectedcontentsListResponse, contentsListResponse) - - // ref is the new tag we created above in setup - ref = newTag - refType = "tag" - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - tagCommit, _ := gitRepo.GetTagCommit(ref) - lastCommit, _ = tagCommit.GetCommitByPath("README.md") - expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(ref, refType, lastCommit.ID.String()) - assert.Equal(t, expectedcontentsListResponse, contentsListResponse) - - // ref is a commit - ref = commitID - refType = "commit" - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - expectedcontentsListResponse = getExpectedcontentsListResponseForFiles(ref, refType, commitID) - assert.Equal(t, expectedcontentsListResponse, contentsListResponse) - - // Test file contents a file with a bad ref - ref = "badref" - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files?ref=%s", user2.Name, repo1.Name, ref), &filesOptions) - MakeRequest(t, req, http.StatusNotFound) - - // Test accessing private ref with user token that does not have access - should fail - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions). - AddTokenAuth(token4) - MakeRequest(t, req, http.StatusNotFound) + requestFiles := func(t *testing.T, url string, files []string, expectedStatusCode ...int) (ret []*api.ContentsResponse) { + req := NewRequestWithJSON(t, "POST", url, &api.GetFilesOptions{Files: files}) + resp := MakeRequest(t, req, util.OptionalArg(expectedStatusCode, http.StatusOK)) + if resp.Code != http.StatusOK { + return nil + } + DecodeJSON(t, resp, &ret) + return ret + } - // Test access private ref of owner of token - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusOK) + t.Run("User2NoRef", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/files", []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) + t.Run("User2RefBranch", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=master", []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) + t.Run("User2RefTag", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=v1.1", []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("v1.1", "tag", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) + t.Run("User2RefCommit", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=65f1bf27bc3bf70f64657658635e66094edbcb4d", []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("65f1bf27bc3bf70f64657658635e66094edbcb4d", "commit", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) + t.Run("User2RefNotExist", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=not-exist", []string{"README.md"}, http.StatusNotFound) + assert.Empty(t, ret) + }) - // Test access of org org3 private repo file by owner user2 - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", org3.Name, repo3.Name), &filesOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusOK) + t.Run("PermissionCheck", func(t *testing.T) { + filesOptions := &api.GetFilesOptions{Files: []string{"README.md"}} + // Test accessing private ref with user token that does not have access - should fail + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions).AddTokenAuth(token4) + MakeRequest(t, req, http.StatusNotFound) + // Test access private ref of owner of token + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions).AddTokenAuth(token2) + MakeRequest(t, req, http.StatusOK) + // Test access of org org3 private repo file by owner user2 + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", org3.Name, repo3.Name), &filesOptions).AddTokenAuth(token2) + MakeRequest(t, req, http.StatusOK) + }) + // TODO: use mocked config to test without creating new files (to speed up the test) // Test pagination for i := 0; i < 40; i++ { filesOptions.Files = append(filesOptions.Files, filesOptions.Files[0]) diff --git a/tests/integration/api_repo_get_contents_test.go b/tests/integration/api_repo_get_contents_test.go index 1571787886cce..e2fd1d4cbcba6 100644 --- a/tests/integration/api_repo_get_contents_test.go +++ b/tests/integration/api_repo_get_contents_test.go @@ -4,6 +4,7 @@ package integration import ( + "code.gitea.io/gitea/modules/util" "io" "net/http" "net/url" @@ -26,28 +27,24 @@ import ( func getExpectedContentsResponseForContents(ref, refType, lastCommitSHA string) *api.ContentsResponse { treePath := "README.md" - sha := "4b4851ad51df6a7d9f25c979345979eaeb5b349f" - encoding := "base64" - content := "IyByZXBvMQoKRGVzY3JpcHRpb24gZm9yIHJlcG8x" selfURL := setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=" + ref htmlURL := setting.AppURL + "user2/repo1/src/" + refType + "/" + ref + "/" + treePath - gitURL := setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha - downloadURL := setting.AppURL + "user2/repo1/raw/" + refType + "/" + ref + "/" + treePath + gitURL := setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/4b4851ad51df6a7d9f25c979345979eaeb5b349f" return &api.ContentsResponse{ Name: treePath, Path: treePath, - SHA: sha, + SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", LastCommitSHA: lastCommitSHA, LastCommitterDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), LastAuthorDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), Type: "file", Size: 30, - Encoding: &encoding, - Content: &content, + Encoding: util.ToPointer("base64"), + Content: util.ToPointer("IyByZXBvMQoKRGVzY3JpcHRpb24gZm9yIHJlcG8x"), URL: &selfURL, HTMLURL: &htmlURL, GitURL: &gitURL, - DownloadURL: &downloadURL, + DownloadURL: util.ToPointer(setting.AppURL + "user2/repo1/raw/" + refType + "/" + ref + "/" + treePath), Links: &api.FileLinksResponse{ Self: &selfURL, GitURL: &gitURL, From 8a5b4f82d33a04321ded662056794d8b8aabe3a3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 20:40:36 +0800 Subject: [PATCH 25/31] fine tune tests --- tests/integration/api_repo_files_get_test.go | 163 +++++++----------- .../integration/api_repo_get_contents_test.go | 2 +- 2 files changed, 61 insertions(+), 104 deletions(-) diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index cb65961d96624..de5b1ac4c009e 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -4,8 +4,11 @@ package integration import ( + "fmt" + "net/http" + "testing" + auth_model "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -13,23 +16,17 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" - repo_service "code.gitea.io/gitea/services/repository" - "encoding/base64" - "fmt" + "code.gitea.io/gitea/tests" + "github.com/stretchr/testify/assert" - "net/http" - "net/url" - "testing" - "time" + "github.com/stretchr/testify/require" ) func TestAPIGetRequestedFiles(t *testing.T) { - onGiteaRun(t, testAPIGetRequestedFiles) -} + defer tests.PrepareTestEnv(t)() -func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { - /*** SETUP ***/ user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo1 & repo16 org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // owner of the repo3, is an org user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // owner of neither repos @@ -37,12 +34,14 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // public repo repo16 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) // private repo + // TODO: add "GET" support + // Get user2's token session := loginUser(t, user2.Name) - token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // TODO: allow for a POST-request to be scope read + token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // Get user4's token session = loginUser(t, user4.Name) - token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // TODO: allow for a POST-request to be scope read + token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo1) assert.NoError(t, err) @@ -97,94 +96,52 @@ func testAPIGetRequestedFiles(t *testing.T, u *url.URL) { MakeRequest(t, req, http.StatusOK) }) - // TODO: use mocked config to test without creating new files (to speed up the test) - // Test pagination - for i := 0; i < 40; i++ { - filesOptions.Files = append(filesOptions.Files, filesOptions.Files[0]) - } - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo1.Name), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - assert.Len(t, contentsListResponse, setting.API.DefaultPagingNum) - - // create new repo for large file tests - baseRepo, err := repo_service.CreateRepository(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{ - Name: "repo-test-files-api", - Description: "test files api", - AutoInit: true, - Gitignores: "Go", - License: "MIT", - Readme: "Default", - DefaultBranch: "main", - IsPrivate: false, - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) + t.Run("ResponseList", func(t *testing.T) { + defer test.MockVariableValue(&setting.API.DefaultPagingNum)() + defer test.MockVariableValue(&setting.API.DefaultMaxBlobSize)() + defer test.MockVariableValue(&setting.API.DefaultMaxResponseSize)() - // Test file size limit - largeFile := make([]byte, 15728640) // 15 MiB -> over max blob size - for i := range largeFile { - largeFile[i] = byte(i % 256) - } - user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository) - doAPICreateFile(user2APICtx, "large-file.txt", &api.CreateFileOptions{ - FileOptions: api.FileOptions{ - Message: "create large-file.txt", - Author: api.Identity{ - Name: user2.Name, - Email: user2.Email, - }, - Committer: api.Identity{ - Name: user2.Name, - Email: user2.Email, - }, - Dates: api.CommitDateOptions{ - Author: time.Now(), - Committer: time.Now(), - }, - }, - ContentBase64: base64.StdEncoding.EncodeToString(largeFile), - })(t) - - filesOptions.Files = []string{"large-file.txt"} - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, baseRepo.Name), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - assert.Equal(t, int64(15728640), contentsListResponse[0].Size) - assert.Empty(t, *contentsListResponse[0].Content) - - // Test response size limit - smallFile := make([]byte, 5242880) // 5 MiB -> under max blob size - for i := range smallFile { - smallFile[i] = byte(i % 256) - } - doAPICreateFile(user2APICtx, "small-file.txt", &api.CreateFileOptions{ - FileOptions: api.FileOptions{ - Message: "create small-file.txt", - Author: api.Identity{ - Name: user2.Name, - Email: user2.Email, - }, - Committer: api.Identity{ - Name: user2.Name, - Email: user2.Email, - }, - Dates: api.CommitDateOptions{ - Author: time.Now(), - Committer: time.Now(), - }, - }, - ContentBase64: base64.StdEncoding.EncodeToString(smallFile), - })(t) - filesOptions.Files = []string{"small-file.txt"} - for i := 0; i < 40; i++ { - filesOptions.Files = append(filesOptions.Files, filesOptions.Files[0]) - } - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, baseRepo.Name), &filesOptions) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &contentsListResponse) - assert.NotNil(t, contentsListResponse) - assert.Len(t, contentsListResponse, 15) // base64-encoded content is around 4/3 the size of the original content + type expected struct { + Name string + HasContent bool + } + assertResponse := func(t *testing.T, expected []*expected, ret []*api.ContentsResponse) { + require.Len(t, ret, len(expected)) + for i, e := range expected { + if e == nil { + assert.Nil(t, ret[i], "item %d", i) + continue + } + assert.Equal(t, e.Name, ret[i].Name, "item %d name", i) + if e.HasContent { + require.NotNil(t, ret[i].Content, "item %d content", i) + assert.NotEmpty(t, *ret[i].Content, "item %d content", i) + } else { + assert.Nil(t, ret[i].Content, "item %d content", i) + } + } + } + + // repo1 "DefaultBranch" has 2 files: LICENSE (1064 bytes), README.md (30 bytes) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + assertResponse(t, []*expected{nil, {"LICENSE", true}, {"README.md", true}}, ret) + + // the returned file list is limited by the DefaultPagingNum + setting.API.DefaultPagingNum = 2 + ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + assertResponse(t, []*expected{nil, {"LICENSE", true}}, ret) + setting.API.DefaultPagingNum = 100 + + // if a file exceeds the DefaultMaxBlobSize, the content is not returned + setting.API.DefaultMaxBlobSize = 200 + ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + assertResponse(t, []*expected{nil, {"LICENSE", false}, {"README.md", true}}, ret) + setting.API.DefaultMaxBlobSize = 20000 + + // if the total response size would exceed the DefaultMaxResponseSize, then the list stops + setting.API.DefaultMaxResponseSize = 1064*4/3 + 1 + ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + assertResponse(t, []*expected{nil, {"LICENSE", true}}, ret) + setting.API.DefaultMaxBlobSize = 20000 + }) } diff --git a/tests/integration/api_repo_get_contents_test.go b/tests/integration/api_repo_get_contents_test.go index e2fd1d4cbcba6..a7de97c052880 100644 --- a/tests/integration/api_repo_get_contents_test.go +++ b/tests/integration/api_repo_get_contents_test.go @@ -4,7 +4,6 @@ package integration import ( - "code.gitea.io/gitea/modules/util" "io" "net/http" "net/url" @@ -19,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" From ff4a1050e744cb56b202b27db4d28fb027d27d38 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 21:26:34 +0800 Subject: [PATCH 26/31] fix --- modules/structs/git_blob.go | 10 +++++----- routers/api/v1/repo/file.go | 2 +- services/repository/files/content.go | 8 ++++---- services/repository/files/content_test.go | 5 +++-- services/repository/files/file.go | 14 +++++++------- tests/integration/api_repo_files_get_test.go | 2 +- tests/integration/api_repo_git_blobs_test.go | 2 +- 7 files changed, 22 insertions(+), 21 deletions(-) diff --git a/modules/structs/git_blob.go b/modules/structs/git_blob.go index 96c7a271a9234..96770cc62e210 100644 --- a/modules/structs/git_blob.go +++ b/modules/structs/git_blob.go @@ -5,9 +5,9 @@ package structs // GitBlobResponse represents a git blob type GitBlobResponse struct { - Content string `json:"content"` - Encoding string `json:"encoding"` - URL string `json:"url"` - SHA string `json:"sha"` - Size int64 `json:"size"` + Content *string `json:"content"` + Encoding *string `json:"encoding"` + URL string `json:"url"` + SHA string `json:"sha"` + Size int64 `json:"size"` } diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index d475163e16b1c..36f68dfa34db9 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1029,6 +1029,6 @@ func GetFiles(ctx *context.APIContext) { if ctx.Written() { return } - filesResponse := files_service.GetContentsListFromTrees(ctx, ctx.Repo.Repository, refCommit, apiOpts.Files) + filesResponse := files_service.GetContentsListFromTreePaths(ctx, ctx.Repo.Repository, refCommit, apiOpts.Files) ctx.JSON(http.StatusOK, util.SliceNilAsEmpty(filesResponse)) } diff --git a/services/repository/files/content.go b/services/repository/files/content.go index c2d02c4541c11..7a07a0ddca431 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -175,8 +175,8 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, refCommit *ut if err != nil { return nil, err } - contentsResponse.Encoding = &blobResponse.Encoding - contentsResponse.Content = &blobResponse.Content + contentsResponse.Encoding = blobResponse.Encoding + contentsResponse.Content = blobResponse.Content } } else if entry.IsDir() { contentsResponse.Type = string(ContentTypeDir) @@ -240,11 +240,11 @@ func GetBlobBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git Size: gitBlob.Size(), } if gitBlob.Size() <= setting.API.DefaultMaxBlobSize { - ret.Encoding = "base64" - ret.Content, err = gitBlob.GetBlobContentBase64() + content, err := gitBlob.GetBlobContentBase64() if err != nil { return nil, err } + ret.Encoding, ret.Content = util.ToPointer("base64"), &content } return ret, nil } diff --git a/services/repository/files/content_test.go b/services/repository/files/content_test.go index cb6ed583cafc9..eb10f5c9b1188 100644 --- a/services/repository/files/content_test.go +++ b/services/repository/files/content_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/gitrepo" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/contexttest" @@ -197,8 +198,8 @@ func TestGetBlobBySHA(t *testing.T) { gbr, err := GetBlobBySHA(ctx, ctx.Repo.Repository, gitRepo, ctx.PathParam("sha")) expectedGBR := &api.GitBlobResponse{ - Content: "dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK", - Encoding: "base64", + Content: util.ToPointer("dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK"), + Encoding: util.ToPointer("base64"), URL: "https://try.gitea.io/api/v1/repos/user2/repo1/git/blobs/65f1bf27bc3bf70f64657658635e66094edbcb4d", SHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d", Size: 180, diff --git a/services/repository/files/file.go b/services/repository/files/file.go index e3d745e89faeb..c75a3ed28a267 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -19,29 +19,29 @@ import ( "code.gitea.io/gitea/routers/api/v1/utils" ) -func GetContentsListFromTrees(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treeNames []string) []*api.ContentsResponse { +func GetContentsListFromTreePaths(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treePaths []string) []*api.ContentsResponse { var files []*api.ContentsResponse var size int64 - for _, file := range treeNames { - fileContents, _ := GetContents(ctx, repo, refCommit, file, false) // ok if fails, then will be nil - if fileContents != nil && *fileContents.Content != "" { + for _, treePath := range treePaths { + fileContents, _ := GetContents(ctx, repo, refCommit, treePath, false) // ok if fails, then will be nil + if fileContents != nil && fileContents.Content != nil && *fileContents.Content != "" { // if content isn't empty (e.g. due to the single blob being too large), add file size to response size // the content is base64 encoded, so it's size increases to around 4/3 of the original size size += fileContents.Size * 4 / 3 } if size > setting.API.DefaultMaxResponseSize { - return files // return if max page size would be exceeded + break // stop if max page size would be exceeded } files = append(files, fileContents) if len(files) == setting.API.DefaultPagingNum { - return files // return if paging num or max size reached + break // stop if paging num or max size reached } } return files } func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treeNames []string) (*api.FilesResponse, error) { - files := GetContentsListFromTrees(ctx, repo, refCommit, treeNames) + files := GetContentsListFromTreePaths(ctx, repo, refCommit, treeNames) fileCommitResponse, _ := GetFileCommitResponse(repo, refCommit.Commit) // ok if fails, then will be nil verification := GetPayloadCommitVerification(ctx, refCommit.Commit) filesResponse := &api.FilesResponse{ diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index de5b1ac4c009e..d1a1987e3062f 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -139,7 +139,7 @@ func TestAPIGetRequestedFiles(t *testing.T) { setting.API.DefaultMaxBlobSize = 20000 // if the total response size would exceed the DefaultMaxResponseSize, then the list stops - setting.API.DefaultMaxResponseSize = 1064*4/3 + 1 + setting.API.DefaultMaxResponseSize = ret[1].Size*4/3 + 1 ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) assertResponse(t, []*expected{nil, {"LICENSE", true}}, ret) setting.API.DefaultMaxBlobSize = 20000 diff --git a/tests/integration/api_repo_git_blobs_test.go b/tests/integration/api_repo_git_blobs_test.go index 9c4be31396798..d4274bdb4042c 100644 --- a/tests/integration/api_repo_git_blobs_test.go +++ b/tests/integration/api_repo_git_blobs_test.go @@ -41,7 +41,7 @@ func TestAPIReposGitBlobs(t *testing.T) { DecodeJSON(t, resp, &gitBlobResponse) assert.NotNil(t, gitBlobResponse) expectedContent := "dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK" - assert.Equal(t, expectedContent, gitBlobResponse.Content) + assert.Equal(t, expectedContent, *gitBlobResponse.Content) // Tests a private repo with no token so will fail req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/git/blobs/%s", user2.Name, repo16.Name, repo16ReadmeSHA) From d867f3336312addc4a8ced026b7140a619c59f20 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 22:03:54 +0800 Subject: [PATCH 27/31] add GET method support --- routers/api/v1/api.go | 4 +- routers/api/v1/repo/file.go | 64 +++++++++++++++++--- templates/swagger/v1_json.tmpl | 54 ++++++++++++++++- tests/integration/api_repo_files_get_test.go | 38 +++++++----- 4 files changed, 134 insertions(+), 26 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 3f4639fc8641e..738c1ff666bc9 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1397,7 +1397,9 @@ func Routes() *web.Router { m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile) }, reqToken()) }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) - m.Post("/files", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(), bind(api.GetFilesOptions{}), repo.GetFiles) + m.Combo("/file-contents", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()). + Get(repo.GetFileContentsGet). + Post(bind(api.GetFilesOptions{}), repo.GetFileContentsPost) // POST method requires "write" permission, so we also support "GET" method above m.Get("/signing-key.gpg", misc.SigningKey) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 36f68dfa34db9..095c08a39f5a3 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -985,16 +986,53 @@ func GetContentsList(ctx *context.APIContext) { GetContents(ctx) } -// GetFiles Get the metadata and contents of requested files -func GetFiles(ctx *context.APIContext) { - // swagger:operation POST /repos/{owner}/{repo}/files repository repoGetFiles +func GetFileContentsGet(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/file-contents repository repoGetFileContents + // --- + // summary: Get the metadata and contents of requested files + // description: See the POST method. This GET method supports to use JSON encoded request body in parameter. + // 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: ref + // in: query + // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // type: string + // required: false + // - name: body + // in: query + // description: "The JSON encoded body (see the POST request): {files: [\"filename1\", \"filename2\"]}" + // type: string + // required: true + // responses: + // "200": + // "$ref": "#/responses/ContentsListResponse" + // "404": + // "$ref": "#/responses/notFound" + + // POST method requires "write" permission, so we also support this "GET" method + handleGetFileContents(ctx) +} + +func GetFileContentsPost(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/file-contents repository repoGetFileContentsPost // --- // summary: Get the metadata and contents of requested files // description: Uses automatic pagination based on default page size and // max response size and returns the maximum allowed number of files. - // Files which could not be retrieved are null. Blobs which are too large - // are being returned with `content = ""` and `size > 0`, they can be - // requested using the `download_url`. + // Files which could not be retrieved are null. Files which are too large + // are being returned with `encoding == null`, `content == null` and `size > 0`, + // they can be requested separately by using the `download_url`. // produces: // - application/json // parameters: @@ -1023,12 +1061,22 @@ func GetFiles(ctx *context.APIContext) { // "$ref": "#/responses/ContentsListResponse" // "404": // "$ref": "#/responses/notFound" + handleGetFileContents(ctx) +} - apiOpts := web.GetForm(ctx).(*api.GetFilesOptions) +func handleGetFileContents(ctx *context.APIContext) { + opts, ok := web.GetForm(ctx).(*api.GetFilesOptions) + if !ok { + err := json.Unmarshal(util.UnsafeStringToBytes(ctx.FormString("body")), &opts) + if err != nil { + ctx.APIError(http.StatusBadRequest, "invalid body parameter") + return + } + } refCommit := resolveRefCommit(ctx, ctx.FormTrim("ref")) if ctx.Written() { return } - filesResponse := files_service.GetContentsListFromTreePaths(ctx, ctx.Repo.Repository, refCommit, apiOpts.Files) + filesResponse := files_service.GetContentsListFromTreePaths(ctx, ctx.Repo.Repository, refCommit, opts.Files) ctx.JSON(http.StatusOK, util.SliceNilAsEmpty(filesResponse)) } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4780c5d2b9f76..dd7d6d6702fa9 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7263,9 +7263,57 @@ } } }, - "/repos/{owner}/{repo}/files": { + "/repos/{owner}/{repo}/file-contents": { + "get": { + "description": "See the POST method. This GET method supports to use JSON encoded request body in parameter.", + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get the metadata and contents of requested files", + "operationId": "repoGetFileContents", + "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": "string", + "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "name": "ref", + "in": "query" + }, + { + "type": "string", + "description": "The JSON encoded body (see the POST request): {files: [\"filename1\", \"filename2\"]}", + "name": "body", + "in": "query", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/ContentsListResponse" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + }, "post": { - "description": "Uses automatic pagination based on default page size and max response size and returns the maximum allowed number of files. Files which could not be retrieved are null. Blobs which are too large are being returned with `content = \"\"` and `size \u003e 0`, they can be requested using the `download_url`.", + "description": "Uses automatic pagination based on default page size and max response size and returns the maximum allowed number of files. Files which could not be retrieved are null. Files which are too large are being returned with `encoding == null`, `content == null` and `size \u003e 0`, they can be requested separately by using the `download_url`.", "produces": [ "application/json" ], @@ -7273,7 +7321,7 @@ "repository" ], "summary": "Get the metadata and contents of requested files", - "operationId": "repoGetFiles", + "operationId": "repoGetFileContentsPost", "parameters": [ { "type": "string", diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index d1a1987e3062f..7e85a013bd2d4 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -6,6 +6,7 @@ package integration import ( "fmt" "net/http" + "net/url" "testing" auth_model "code.gitea.io/gitea/models/auth" @@ -14,6 +15,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" @@ -34,8 +36,6 @@ func TestAPIGetRequestedFiles(t *testing.T) { repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // public repo repo16 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) // private repo - // TODO: add "GET" support - // Get user2's token session := loginUser(t, user2.Name) token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) @@ -58,41 +58,51 @@ func TestAPIGetRequestedFiles(t *testing.T) { return ret } + t.Run("User2Get", func(t *testing.T) { + reqBodyOpt := &api.GetFilesOptions{Files: []string{"README.md"}} + reqBodyParam, _ := json.Marshal(reqBodyOpt) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/user2/repo1/file-contents?body=%s", url.QueryEscape(string(reqBodyParam)))) + resp := MakeRequest(t, req, http.StatusOK) + var ret []*api.ContentsResponse + DecodeJSON(t, resp, &ret) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) t.Run("User2NoRef", func(t *testing.T) { - ret := requestFiles(t, "/api/v1/repos/user2/repo1/files", []string{"README.md"}) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents", []string{"README.md"}) expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefBranch", func(t *testing.T) { - ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=master", []string{"README.md"}) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=master", []string{"README.md"}) expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefTag", func(t *testing.T) { - ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=v1.1", []string{"README.md"}) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=v1.1", []string{"README.md"}) expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("v1.1", "tag", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefCommit", func(t *testing.T) { - ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=65f1bf27bc3bf70f64657658635e66094edbcb4d", []string{"README.md"}) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=65f1bf27bc3bf70f64657658635e66094edbcb4d", []string{"README.md"}) expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("65f1bf27bc3bf70f64657658635e66094edbcb4d", "commit", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefNotExist", func(t *testing.T) { - ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=not-exist", []string{"README.md"}, http.StatusNotFound) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=not-exist", []string{"README.md"}, http.StatusNotFound) assert.Empty(t, ret) }) t.Run("PermissionCheck", func(t *testing.T) { filesOptions := &api.GetFilesOptions{Files: []string{"README.md"}} // Test accessing private ref with user token that does not have access - should fail - req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions).AddTokenAuth(token4) + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/file-contents", user2.Name, repo16.Name), &filesOptions).AddTokenAuth(token4) MakeRequest(t, req, http.StatusNotFound) // Test access private ref of owner of token - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", user2.Name, repo16.Name), &filesOptions).AddTokenAuth(token2) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/file-contents", user2.Name, repo16.Name), &filesOptions).AddTokenAuth(token2) MakeRequest(t, req, http.StatusOK) // Test access of org org3 private repo file by owner user2 - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/files", org3.Name, repo3.Name), &filesOptions).AddTokenAuth(token2) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/file-contents", org3.Name, repo3.Name), &filesOptions).AddTokenAuth(token2) MakeRequest(t, req, http.StatusOK) }) @@ -123,24 +133,24 @@ func TestAPIGetRequestedFiles(t *testing.T) { } // repo1 "DefaultBranch" has 2 files: LICENSE (1064 bytes), README.md (30 bytes) - ret := requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) assertResponse(t, []*expected{nil, {"LICENSE", true}, {"README.md", true}}, ret) // the returned file list is limited by the DefaultPagingNum setting.API.DefaultPagingNum = 2 - ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + ret = requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) assertResponse(t, []*expected{nil, {"LICENSE", true}}, ret) setting.API.DefaultPagingNum = 100 // if a file exceeds the DefaultMaxBlobSize, the content is not returned setting.API.DefaultMaxBlobSize = 200 - ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + ret = requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) assertResponse(t, []*expected{nil, {"LICENSE", false}, {"README.md", true}}, ret) setting.API.DefaultMaxBlobSize = 20000 // if the total response size would exceed the DefaultMaxResponseSize, then the list stops setting.API.DefaultMaxResponseSize = ret[1].Size*4/3 + 1 - ret = requestFiles(t, "/api/v1/repos/user2/repo1/files?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) + ret = requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) assertResponse(t, []*expected{nil, {"LICENSE", true}}, ret) setting.API.DefaultMaxBlobSize = 20000 }) From d236e7505f6ed407985b9a25e6cf12c2be56ad73 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 22:13:38 +0800 Subject: [PATCH 28/31] fix lint --- tests/integration/api_repo_files_get_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index 7e85a013bd2d4..00e9c70429544 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -61,7 +61,7 @@ func TestAPIGetRequestedFiles(t *testing.T) { t.Run("User2Get", func(t *testing.T) { reqBodyOpt := &api.GetFilesOptions{Files: []string{"README.md"}} reqBodyParam, _ := json.Marshal(reqBodyOpt) - req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/user2/repo1/file-contents?body=%s", url.QueryEscape(string(reqBodyParam)))) + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/file-contents?body="+url.QueryEscape(string(reqBodyParam))) resp := MakeRequest(t, req, http.StatusOK) var ret []*api.ContentsResponse DecodeJSON(t, resp, &ret) From c23f4b819fc8a7a9055dba06d5f680ac05627792 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 23:00:58 +0800 Subject: [PATCH 29/31] update swagger doc --- routers/api/v1/repo/file.go | 4 ++-- templates/swagger/v1_json.tmpl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 095c08a39f5a3..054679fa5fe47 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -990,7 +990,7 @@ func GetFileContentsGet(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/file-contents repository repoGetFileContents // --- // summary: Get the metadata and contents of requested files - // description: See the POST method. This GET method supports to use JSON encoded request body in parameter. + // description: See the POST method. This GET method supports to use JSON encoded request body in query parameter. // produces: // - application/json // parameters: @@ -1011,7 +1011,7 @@ func GetFileContentsGet(ctx *context.APIContext) { // required: false // - name: body // in: query - // description: "The JSON encoded body (see the POST request): {files: [\"filename1\", \"filename2\"]}" + // description: "The JSON encoded body (see the POST request): {\"files\": [\"filename1\", \"filename2\"]}" // type: string // required: true // responses: diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index c4f6472997b06..08930989875a7 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7265,7 +7265,7 @@ }, "/repos/{owner}/{repo}/file-contents": { "get": { - "description": "See the POST method. This GET method supports to use JSON encoded request body in parameter.", + "description": "See the POST method. This GET method supports to use JSON encoded request body in query parameter.", "produces": [ "application/json" ], @@ -7297,7 +7297,7 @@ }, { "type": "string", - "description": "The JSON encoded body (see the POST request): {files: [\"filename1\", \"filename2\"]}", + "description": "The JSON encoded body (see the POST request): {\"files\": [\"filename1\", \"filename2\"]}", "name": "body", "in": "query", "required": true From 57ed5e0717853829bb8689f1a08a5aa610efa3c0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 23:15:19 +0800 Subject: [PATCH 30/31] fine tune comments --- routers/api/v1/repo/file.go | 4 ++++ services/repository/files/file.go | 10 ++++------ tests/integration/api_repo_files_get_test.go | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 054679fa5fe47..66811e6df7bc9 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -1061,6 +1061,10 @@ func GetFileContentsPost(ctx *context.APIContext) { // "$ref": "#/responses/ContentsListResponse" // "404": // "$ref": "#/responses/notFound" + + // This is actually a "read" request, but we need to accept a "files" list, then POST method seems easy to use. + // But the permission system requires that the caller must have "write" permission to use POST method. + // At the moment there is no other way to get around the permission check, so there is a "GET" workaround method above. handleGetFileContents(ctx) } diff --git a/services/repository/files/file.go b/services/repository/files/file.go index c75a3ed28a267..c4991b458d473 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -19,22 +19,20 @@ import ( "code.gitea.io/gitea/routers/api/v1/utils" ) -func GetContentsListFromTreePaths(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treePaths []string) []*api.ContentsResponse { - var files []*api.ContentsResponse +func GetContentsListFromTreePaths(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treePaths []string) (files []*api.ContentsResponse) { var size int64 for _, treePath := range treePaths { fileContents, _ := GetContents(ctx, repo, refCommit, treePath, false) // ok if fails, then will be nil if fileContents != nil && fileContents.Content != nil && *fileContents.Content != "" { // if content isn't empty (e.g. due to the single blob being too large), add file size to response size - // the content is base64 encoded, so it's size increases to around 4/3 of the original size - size += fileContents.Size * 4 / 3 + size += int64(len(*fileContents.Content)) } if size > setting.API.DefaultMaxResponseSize { - break // stop if max page size would be exceeded + break // stop if max response size would be exceeded } files = append(files, fileContents) if len(files) == setting.API.DefaultPagingNum { - break // stop if paging num or max size reached + break // stop if paging num reached } } return files diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index 00e9c70429544..a4ded7da3f80c 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -149,7 +149,7 @@ func TestAPIGetRequestedFiles(t *testing.T) { setting.API.DefaultMaxBlobSize = 20000 // if the total response size would exceed the DefaultMaxResponseSize, then the list stops - setting.API.DefaultMaxResponseSize = ret[1].Size*4/3 + 1 + setting.API.DefaultMaxResponseSize = ret[1].Size*4/3 + 10 ret = requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=DefaultBranch", []string{"no-such.txt", "LICENSE", "README.md"}) assertResponse(t, []*expected{nil, {"LICENSE", true}}, ret) setting.API.DefaultMaxBlobSize = 20000 From b6c7465b984c2bd21ec756c33f9a32c6fc6403e8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 23:18:21 +0800 Subject: [PATCH 31/31] fix swagger doc, "master" is no longer "usually" --- routers/api/v1/repo/file.go | 10 +++++----- templates/swagger/v1_json.tmpl | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 66811e6df7bc9..f40d39a2517f6 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -377,7 +377,7 @@ func GetEditorconfig(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch." // type: string // required: false // responses: @@ -927,7 +927,7 @@ func GetContents(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch." // type: string // required: false // responses: @@ -973,7 +973,7 @@ func GetContentsList(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch." // type: string // required: false // responses: @@ -1006,7 +1006,7 @@ func GetFileContentsGet(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch." // type: string // required: false // - name: body @@ -1048,7 +1048,7 @@ func GetFileContentsPost(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch." // type: string // required: false // - name: body diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 08930989875a7..0a9432fc9d124 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -6863,7 +6863,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch.", "name": "ref", "in": "query" } @@ -6966,7 +6966,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch.", "name": "ref", "in": "query" } @@ -7248,7 +7248,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch.", "name": "ref", "in": "query" } @@ -7291,7 +7291,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch.", "name": "ref", "in": "query" }, @@ -7339,7 +7339,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch.", "name": "ref", "in": "query" },