Handle expected errors in FileCreate & FileUpdate API (#11643) (#11718)

as title needed for #11641
go-gitea · Jun 1, 2020 · 70739c3 · 70739c3
1 parent 249e22b
commit 70739c3
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -2746,6 +2746,15 @@
         "responses": {
           "200": {
             "$ref": "#/responses/FileResponse"
+          },
+          "403": {
+            "$ref": "#/responses/error"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
+          },
+          "422": {
+            "$ref": "#/responses/error"
           }
         }
       },
@@ -2795,6 +2804,15 @@
         "responses": {
           "201": {
             "$ref": "#/responses/FileResponse"
+          },
+          "403": {
+            "$ref": "#/responses/error"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
+          },
+          "422": {
+            "$ref": "#/responses/error"
           }
         }
       },