mirror of
https://github.com/go-gitea/gitea
synced 2024-12-25 21:34:49 +01:00
Contents API should return 404 on not exist (#10323)
* Return 404 on not exist * swagger update and use git.IsErrNotExist * Handle delete too * Handle delete too x2 * Fix pr 10323 (#3) * fix TESTS * leafe a note for fututre * placate golangci-lint Signed-off-by: Andrew Thornton <art27@cantab.net> * Update integrations/api_repo_file_delete_test.go Co-Authored-By: 6543 <6543@obermui.de> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com> Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
This commit is contained in:
parent
7c8e116987
commit
2e85ad665a
5 changed files with 48 additions and 31 deletions
|
@ -11,8 +11,6 @@ import (
|
|||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models"
|
||||
"code.gitea.io/gitea/modules/context"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
api "code.gitea.io/gitea/modules/structs"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
@ -109,18 +107,10 @@ func TestAPIDeleteFile(t *testing.T) {
|
|||
treePath = fmt.Sprintf("delete/file%d.txt", fileID)
|
||||
createFile(user2, repo1, treePath)
|
||||
deleteFileOptions = getDeleteFileOptions()
|
||||
correctSHA := deleteFileOptions.SHA
|
||||
deleteFileOptions.SHA = "badsha"
|
||||
url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2)
|
||||
req = NewRequestWithJSON(t, "DELETE", url, &deleteFileOptions)
|
||||
resp = session.MakeRequest(t, req, http.StatusInternalServerError)
|
||||
expectedAPIError := context.APIError{
|
||||
Message: "sha does not match [given: " + deleteFileOptions.SHA + ", expected: " + correctSHA + "]",
|
||||
URL: setting.API.SwaggerURL,
|
||||
}
|
||||
var apiError context.APIError
|
||||
DecodeJSON(t, resp, &apiError)
|
||||
assert.Equal(t, expectedAPIError, apiError)
|
||||
resp = session.MakeRequest(t, req, http.StatusBadRequest)
|
||||
|
||||
// Test creating a file in repo16 by user4 who does not have write access
|
||||
fileID++
|
||||
|
|
|
@ -11,7 +11,6 @@ import (
|
|||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models"
|
||||
"code.gitea.io/gitea/modules/context"
|
||||
"code.gitea.io/gitea/modules/git"
|
||||
repo_module "code.gitea.io/gitea/modules/repository"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
|
@ -139,14 +138,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
|
|||
// Test file contents a file with a bad ref
|
||||
ref = "badref"
|
||||
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
|
||||
resp = session.MakeRequest(t, req, http.StatusInternalServerError)
|
||||
expectedAPIError := context.APIError{
|
||||
Message: "object does not exist [id: " + ref + ", rel_path: ]",
|
||||
URL: setting.API.SwaggerURL,
|
||||
}
|
||||
var apiError context.APIError
|
||||
DecodeJSON(t, resp, &apiError)
|
||||
assert.Equal(t, expectedAPIError, apiError)
|
||||
resp = session.MakeRequest(t, req, http.StatusNotFound)
|
||||
|
||||
// Test accessing private ref with user token that does not have access - should fail
|
||||
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo16.Name, treePath, token4)
|
||||
|
|
|
@ -10,7 +10,6 @@ import (
|
|||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models"
|
||||
"code.gitea.io/gitea/modules/context"
|
||||
"code.gitea.io/gitea/modules/git"
|
||||
repo_module "code.gitea.io/gitea/modules/repository"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
|
@ -141,14 +140,7 @@ func testAPIGetContents(t *testing.T, u *url.URL) {
|
|||
// Test file contents a file with a bad ref
|
||||
ref = "badref"
|
||||
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
|
||||
resp = session.MakeRequest(t, req, http.StatusInternalServerError)
|
||||
expectedAPIError := context.APIError{
|
||||
Message: "object does not exist [id: " + ref + ", rel_path: ]",
|
||||
URL: setting.API.SwaggerURL,
|
||||
}
|
||||
var apiError context.APIError
|
||||
DecodeJSON(t, resp, &apiError)
|
||||
assert.Equal(t, expectedAPIError, apiError)
|
||||
resp = session.MakeRequest(t, req, http.StatusNotFound)
|
||||
|
||||
// Test accessing private ref with user token that does not have access - should fail
|
||||
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo16.Name, treePath, token4)
|
||||
|
|
|
@ -362,9 +362,15 @@ func DeleteFile(ctx *context.APIContext, apiOpts api.DeleteFileOptions) {
|
|||
// responses:
|
||||
// "200":
|
||||
// "$ref": "#/responses/FileDeleteResponse"
|
||||
// "400":
|
||||
// "$ref": "#/responses/error"
|
||||
// "403":
|
||||
// "$ref": "#/responses/error"
|
||||
// "404":
|
||||
// "$ref": "#/responses/error"
|
||||
|
||||
if !CanWriteFiles(ctx.Repo) {
|
||||
ctx.Error(http.StatusInternalServerError, "DeleteFile", models.ErrUserDoesNotHaveAccessToRepo{
|
||||
ctx.Error(http.StatusForbidden, "DeleteFile", models.ErrUserDoesNotHaveAccessToRepo{
|
||||
UserID: ctx.User.ID,
|
||||
RepoName: ctx.Repo.Repository.LowerName,
|
||||
})
|
||||
|
@ -402,9 +408,23 @@ func DeleteFile(ctx *context.APIContext, apiOpts api.DeleteFileOptions) {
|
|||
}
|
||||
|
||||
if fileResponse, err := repofiles.DeleteRepoFile(ctx.Repo.Repository, ctx.User, opts); err != nil {
|
||||
if git.IsErrBranchNotExist(err) || models.IsErrRepoFileDoesNotExist(err) || git.IsErrNotExist(err) {
|
||||
ctx.Error(http.StatusNotFound, "DeleteFile", err)
|
||||
return
|
||||
} else if models.IsErrBranchAlreadyExists(err) ||
|
||||
models.IsErrFilenameInvalid(err) ||
|
||||
models.IsErrSHADoesNotMatch(err) ||
|
||||
models.IsErrCommitIDDoesNotMatch(err) ||
|
||||
models.IsErrSHAOrCommitIDNotProvided(err) {
|
||||
ctx.Error(http.StatusBadRequest, "DeleteFile", err)
|
||||
return
|
||||
} else if models.IsErrUserCannotCommit(err) {
|
||||
ctx.Error(http.StatusForbidden, "DeleteFile", err)
|
||||
return
|
||||
}
|
||||
ctx.Error(http.StatusInternalServerError, "DeleteFile", err)
|
||||
} else {
|
||||
ctx.JSON(http.StatusOK, fileResponse)
|
||||
ctx.JSON(http.StatusOK, fileResponse) // FIXME on APIv2: return http.StatusNoContent
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -439,6 +459,8 @@ func GetContents(ctx *context.APIContext) {
|
|||
// responses:
|
||||
// "200":
|
||||
// "$ref": "#/responses/ContentsResponse"
|
||||
// "404":
|
||||
// "$ref": "#/responses/notFound"
|
||||
|
||||
if !CanReadFiles(ctx.Repo) {
|
||||
ctx.Error(http.StatusInternalServerError, "GetContentsOrList", models.ErrUserDoesNotHaveAccessToRepo{
|
||||
|
@ -452,6 +474,10 @@ func GetContents(ctx *context.APIContext) {
|
|||
ref := ctx.QueryTrim("ref")
|
||||
|
||||
if fileList, err := repofiles.GetContentsOrList(ctx.Repo.Repository, treePath, ref); err != nil {
|
||||
if git.IsErrNotExist(err) {
|
||||
ctx.NotFound("GetContentsOrList", err)
|
||||
return
|
||||
}
|
||||
ctx.Error(http.StatusInternalServerError, "GetContentsOrList", err)
|
||||
} else {
|
||||
ctx.JSON(http.StatusOK, fileList)
|
||||
|
@ -484,6 +510,8 @@ func GetContentsList(ctx *context.APIContext) {
|
|||
// responses:
|
||||
// "200":
|
||||
// "$ref": "#/responses/ContentsListResponse"
|
||||
// "404":
|
||||
// "$ref": "#/responses/notFound"
|
||||
|
||||
// same as GetContents(), this function is here because swagger fails if path is empty in GetContents() interface
|
||||
GetContents(ctx)
|
||||
|
|
|
@ -2638,6 +2638,9 @@
|
|||
"responses": {
|
||||
"200": {
|
||||
"$ref": "#/responses/ContentsListResponse"
|
||||
},
|
||||
"404": {
|
||||
"$ref": "#/responses/notFound"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -2684,6 +2687,9 @@
|
|||
"responses": {
|
||||
"200": {
|
||||
"$ref": "#/responses/ContentsResponse"
|
||||
},
|
||||
"404": {
|
||||
"$ref": "#/responses/notFound"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
@ -2831,6 +2837,15 @@
|
|||
"responses": {
|
||||
"200": {
|
||||
"$ref": "#/responses/FileDeleteResponse"
|
||||
},
|
||||
"400": {
|
||||
"$ref": "#/responses/error"
|
||||
},
|
||||
"403": {
|
||||
"$ref": "#/responses/error"
|
||||
},
|
||||
"404": {
|
||||
"$ref": "#/responses/error"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue