From b636cd6fd40ae05f22e732574da10af060273f9f Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Sun, 31 May 2020 22:59:34 +0200
Subject: [PATCH] Handle expected errors in FileCreate & FileUpdate API
(#11643)
as title
needed for #11641
---
integrations/api_repo_file_create_test.go | 2 +-
integrations/api_repo_file_update_test.go | 2 +-
routers/api/v1/repo/file.go | 39 +++++++++++++++++++++--
templates/swagger/v1_json.tmpl | 18 +++++++++++
4 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/integrations/api_repo_file_create_test.go b/integrations/api_repo_file_create_test.go
index 3c8b50d5d1..853224f091 100644
--- a/integrations/api_repo_file_create_test.go
+++ b/integrations/api_repo_file_create_test.go
@@ -189,7 +189,7 @@ func TestAPICreateFile(t *testing.T) {
treePath = "README.md"
url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2)
req = NewRequestWithJSON(t, "POST", url, &createFileOptions)
- resp = session.MakeRequest(t, req, http.StatusInternalServerError)
+ resp = session.MakeRequest(t, req, http.StatusUnprocessableEntity)
expectedAPIError := context.APIError{
Message: "repository file already exists [path: " + treePath + "]",
URL: setting.API.SwaggerURL,
diff --git a/integrations/api_repo_file_update_test.go b/integrations/api_repo_file_update_test.go
index 369e48d646..69a9463774 100644
--- a/integrations/api_repo_file_update_test.go
+++ b/integrations/api_repo_file_update_test.go
@@ -208,7 +208,7 @@ func TestAPIUpdateFile(t *testing.T) {
updateFileOptions.SHA = "badsha"
url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2)
req = NewRequestWithJSON(t, "PUT", url, &updateFileOptions)
- resp = session.MakeRequest(t, req, http.StatusInternalServerError)
+ resp = session.MakeRequest(t, req, http.StatusUnprocessableEntity)
expectedAPIError := context.APIError{
Message: "sha does not match [given: " + updateFileOptions.SHA + ", expected: " + correctSHA + "]",
URL: setting.API.SwaggerURL,
diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go
index 02a7de9b58..73b8e31a44 100644
--- a/routers/api/v1/repo/file.go
+++ b/routers/api/v1/repo/file.go
@@ -7,6 +7,7 @@ package repo
import (
"encoding/base64"
+ "fmt"
"net/http"
"time"
@@ -198,6 +199,16 @@ func CreateFile(ctx *context.APIContext, apiOpts api.CreateFileOptions) {
// responses:
// "201":
// "$ref": "#/responses/FileResponse"
+ // "403":
+ // "$ref": "#/responses/error"
+ // "404":
+ // "$ref": "#/responses/notFound"
+ // "422":
+ // "$ref": "#/responses/error"
+
+ if ctx.Repo.Repository.IsEmpty {
+ ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
+ }
if apiOpts.BranchName == "" {
apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch
@@ -235,7 +246,7 @@ func CreateFile(ctx *context.APIContext, apiOpts api.CreateFileOptions) {
}
if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil {
- ctx.Error(http.StatusInternalServerError, "CreateFile", err)
+ handleCreateOrUpdateFileError(ctx, err)
} else {
ctx.JSON(http.StatusCreated, fileResponse)
}
@@ -274,6 +285,16 @@ func UpdateFile(ctx *context.APIContext, apiOpts api.UpdateFileOptions) {
// responses:
// "200":
// "$ref": "#/responses/FileResponse"
+ // "403":
+ // "$ref": "#/responses/error"
+ // "404":
+ // "$ref": "#/responses/notFound"
+ // "422":
+ // "$ref": "#/responses/error"
+
+ if ctx.Repo.Repository.IsEmpty {
+ ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
+ }
if apiOpts.BranchName == "" {
apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch
@@ -313,12 +334,26 @@ func UpdateFile(ctx *context.APIContext, apiOpts api.UpdateFileOptions) {
}
if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil {
- ctx.Error(http.StatusInternalServerError, "UpdateFile", err)
+ handleCreateOrUpdateFileError(ctx, err)
} else {
ctx.JSON(http.StatusOK, fileResponse)
}
}
+func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) {
+ if models.IsErrUserCannotCommit(err) || models.IsErrFilePathProtected(err) {
+ ctx.Error(http.StatusForbidden, "Access", err)
+ return
+ }
+ if models.IsErrBranchAlreadyExists(err) || models.IsErrFilenameInvalid(err) || models.IsErrSHADoesNotMatch(err) ||
+ models.IsErrFilePathInvalid(err) || models.IsErrRepoFileAlreadyExists(err) {
+ ctx.Error(http.StatusUnprocessableEntity, "Invalid", err)
+ return
+ }
+
+ ctx.Error(http.StatusInternalServerError, "UpdateFile", err)
+}
+
// Called from both CreateFile or UpdateFile to handle both
func createOrUpdateFile(ctx *context.APIContext, opts *repofiles.UpdateRepoFileOptions) (*api.FileResponse, error) {
if !canWriteFiles(ctx.Repo) {
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index 70f12b083f..4f9c458326 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -2793,6 +2793,15 @@
"responses": {
"200": {
"$ref": "#/responses/FileResponse"
+ },
+ "403": {
+ "$ref": "#/responses/error"
+ },
+ "404": {
+ "$ref": "#/responses/notFound"
+ },
+ "422": {
+ "$ref": "#/responses/error"
}
}
},
@@ -2842,6 +2851,15 @@
"responses": {
"201": {
"$ref": "#/responses/FileResponse"
+ },
+ "403": {
+ "$ref": "#/responses/error"
+ },
+ "404": {
+ "$ref": "#/responses/notFound"
+ },
+ "422": {
+ "$ref": "#/responses/error"
}
}
},