Merge api on Create or update file contents

Author: smallfish06

modules/structs/repo_file.go

@@ -20,20 +20,6 @@ type FileOptions struct {
 	Signoff bool `json:"signoff"`
 }
 
-// CreateFileOptions options for creating files
-// Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)
-type CreateFileOptions struct {
-	FileOptions
-	// content must be base64 encoded
-	// required: true
-	ContentBase64 string `json:"content"`
-}
-
-// Branch returns branch name
-func (o *CreateFileOptions) Branch() string {
-	return o.FileOptions.BranchName
-}
-
 // DeleteFileOptions options for deleting files (used for other File structs below)
 // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)
 type DeleteFileOptions struct {
@@ -48,10 +34,13 @@ func (o *DeleteFileOptions) Branch() string {
 	return o.FileOptions.BranchName
 }
 
-// UpdateFileOptions options for updating files
+// CreateOrUpdateFileOptions options for creating or updating files
 // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)
-type UpdateFileOptions struct {
-	DeleteFileOptions
+type CreateOrUpdateFileOptions struct {
+	FileOptions
+	// sha is the SHA for the file that already exists
+	// required only for updating files
+	SHA string `json:"sha"`
 	// content must be base64 encoded
 	// required: true
 	ContentBase64 string `json:"content"`
@@ -60,7 +49,7 @@ type UpdateFileOptions struct {
 }
 
 // Branch returns branch name
-func (o *UpdateFileOptions) Branch() string {
+func (o *CreateOrUpdateFileOptions) Branch() string {
 	return o.FileOptions.BranchName
 }
 

routers/api/v1/api.go

@@ -1257,8 +1257,7 @@ func Routes() *web.Route {
 					m.Post("", reqToken(), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.ChangeFiles)
 					m.Get("/*", repo.GetContents)
 					m.Group("/*", func() {
-						m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.CreateFile)
-						m.Put("", bind(api.UpdateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.UpdateFile)
+						m.Put("", bind(api.CreateOrUpdateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.CreateOrUpdateFile)
 						m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile)
 					}, reqToken())
 				}, reqRepoReader(unit.TypeCode))

routers/api/v1/repo/file.go

@@ -513,11 +513,11 @@ func ChangeFiles(ctx *context.APIContext) {
 	}
 }
 
-// CreateFile handles API call for creating a file
-func CreateFile(ctx *context.APIContext) {
-	// swagger:operation POST /repos/{owner}/{repo}/contents/{filepath} repository repoCreateFile
+// CreateOrUpdateFile handles API call for creating or updating a file
+func CreateOrUpdateFile(ctx *context.APIContext) {
+	// swagger:operation PUT /repos/{owner}/{repo}/contents/{filepath} repository repoCreateOrUpdateFile
 	// ---
-	// summary: Create a file in a repository
+	// summary: Create or update a file in a repository
 	// consumes:
 	// - application/json
 	// produces:
@@ -535,115 +535,19 @@ func CreateFile(ctx *context.APIContext) {
 	//   required: true
 	// - name: filepath
 	//   in: path
-	//   description: path of the file to create
+	//   description: path of the file to create or update
 	//   type: string
 	//   required: true
 	// - name: body
 	//   in: body
 	//   required: true
 	//   schema:
-	//     "$ref": "#/definitions/CreateFileOptions"
-	// responses:
-	//   "201":
-	//     "$ref": "#/responses/FileResponse"
-	//   "403":
-	//     "$ref": "#/responses/error"
-	//   "404":
-	//     "$ref": "#/responses/notFound"
-	//   "422":
-	//     "$ref": "#/responses/error"
-	//   "423":
-	//     "$ref": "#/responses/repoArchivedError"
-
-	apiOpts := web.GetForm(ctx).(*api.CreateFileOptions)
-
-	if apiOpts.BranchName == "" {
-		apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch
-	}
-
-	contentReader, err := base64Reader(apiOpts.ContentBase64)
-	if err != nil {
-		ctx.Error(http.StatusUnprocessableEntity, "Invalid base64 content", err)
-		return
-	}
-
-	opts := &files_service.ChangeRepoFilesOptions{
-		Files: []*files_service.ChangeRepoFile{
-			{
-				Operation:     "create",
-				TreePath:      ctx.Params("*"),
-				ContentReader: contentReader,
-			},
-		},
-		Message:   apiOpts.Message,
-		OldBranch: apiOpts.BranchName,
-		NewBranch: apiOpts.NewBranchName,
-		Committer: &files_service.IdentityOptions{
-			Name:  apiOpts.Committer.Name,
-			Email: apiOpts.Committer.Email,
-		},
-		Author: &files_service.IdentityOptions{
-			Name:  apiOpts.Author.Name,
-			Email: apiOpts.Author.Email,
-		},
-		Dates: &files_service.CommitDateOptions{
-			Author:    apiOpts.Dates.Author,
-			Committer: apiOpts.Dates.Committer,
-		},
-		Signoff: apiOpts.Signoff,
-	}
-	if opts.Dates.Author.IsZero() {
-		opts.Dates.Author = time.Now()
-	}
-	if opts.Dates.Committer.IsZero() {
-		opts.Dates.Committer = time.Now()
-	}
-
-	if opts.Message == "" {
-		opts.Message = changeFilesCommitMessage(ctx, opts.Files)
-	}
-
-	if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil {
-		handleCreateOrUpdateFileError(ctx, err)
-	} else {
-		fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0)
-		ctx.JSON(http.StatusCreated, fileResponse)
-	}
-}
-
-// UpdateFile handles API call for updating a file
-func UpdateFile(ctx *context.APIContext) {
-	// swagger:operation PUT /repos/{owner}/{repo}/contents/{filepath} repository repoUpdateFile
-	// ---
-	// summary: Update a file in a repository
-	// consumes:
-	// - application/json
-	// 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: filepath
-	//   in: path
-	//   description: path of the file to update
-	//   type: string
-	//   required: true
-	// - name: body
-	//   in: body
-	//   required: true
-	//   schema:
-	//     "$ref": "#/definitions/UpdateFileOptions"
+	//     "$ref": "#/definitions/CreateOrUpdateFileOptions"
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/FileResponse"
+	//   "201":
+	//     "$ref": "#/responses/FileResponse"
 	//   "403":
 	//     "$ref": "#/responses/error"
 	//   "404":
@@ -652,7 +556,7 @@ func UpdateFile(ctx *context.APIContext) {
 	//     "$ref": "#/responses/error"
 	//   "423":
 	//     "$ref": "#/responses/repoArchivedError"
-	apiOpts := web.GetForm(ctx).(*api.UpdateFileOptions)
+	apiOpts := web.GetForm(ctx).(*api.CreateOrUpdateFileOptions)
 	if ctx.Repo.Repository.IsEmpty {
 		ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
 	}
@@ -667,16 +571,25 @@ func UpdateFile(ctx *context.APIContext) {
 		return
 	}
 
+	var changeRepoFile files_service.ChangeRepoFile
+	if apiOpts.SHA == "" {
+		changeRepoFile = files_service.ChangeRepoFile{
+			Operation:     "create",
+			TreePath:      ctx.Params("*"),
+			ContentReader: contentReader,
+		}
+	} else {
+		changeRepoFile = files_service.ChangeRepoFile{
+			Operation:     "update",
+			ContentReader: contentReader,
+			SHA:           apiOpts.SHA,
+			FromTreePath:  apiOpts.FromPath,
+			TreePath:      ctx.Params("*"),
+		}
+	}
+
 	opts := &files_service.ChangeRepoFilesOptions{
-		Files: []*files_service.ChangeRepoFile{
-			{
-				Operation:     "update",
-				ContentReader: contentReader,
-				SHA:           apiOpts.SHA,
-				FromTreePath:  apiOpts.FromPath,
-				TreePath:      ctx.Params("*"),
-			},
-		},
+		Files:     []*files_service.ChangeRepoFile{&changeRepoFile},
 		Message:   apiOpts.Message,
 		OldBranch: apiOpts.BranchName,
 		NewBranch: apiOpts.NewBranchName,
@@ -709,7 +622,11 @@ func UpdateFile(ctx *context.APIContext) {
 		handleCreateOrUpdateFileError(ctx, err)
 	} else {
 		fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0)
-		ctx.JSON(http.StatusOK, fileResponse)
+		if apiOpts.SHA == "" {
+			ctx.JSON(http.StatusOK, fileResponse)
+		} else {
+			ctx.JSON(http.StatusCreated, fileResponse)
+		}
 	}
 }
 
@@ -728,10 +645,10 @@ func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) {
 		return
 	}
 
-	ctx.Error(http.StatusInternalServerError, "UpdateFile", err)
+	ctx.Error(http.StatusInternalServerError, "CreateOrUpdateFile", err)
 }
 
-// Called from both CreateFile or UpdateFile to handle both
+// Called from both CreateFile or CreateOrUpdateFile to handle both
 func createOrUpdateFiles(ctx *context.APIContext, opts *files_service.ChangeRepoFilesOptions) (*api.FilesResponse, error) {
 	if !canWriteFiles(ctx, opts.OldBranch) {
 		return nil, repo_model.ErrUserDoesNotHaveAccessToRepo{

routers/api/v1/swagger/options.go

@@ -120,10 +120,7 @@ type swaggerParameterBodies struct {
 	ChangeFilesOptions api.ChangeFilesOptions
 
 	// in:body
-	CreateFileOptions api.CreateFileOptions
-
-	// in:body
-	UpdateFileOptions api.UpdateFileOptions
+	CreateOrUpdateFileOptions api.CreateOrUpdateFileOptions
 
 	// in:body
 	DeleteFileOptions api.DeleteFileOptions

tests/integration/api_helper_for_declarative_test.go

@@ -353,9 +353,9 @@ func doAPIGetBranch(ctx APITestContext, branch string, callback ...func(*testing
 	}
 }
 
-func doAPICreateFile(ctx APITestContext, treepath string, options *api.CreateFileOptions, callback ...func(*testing.T, api.FileResponse)) func(*testing.T) {
+func doAPICreateFile(ctx APITestContext, treepath string, options *api.CreateOrUpdateFileOptions, callback ...func(*testing.T, api.FileResponse)) func(*testing.T) {
 	return func(t *testing.T) {
-		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", ctx.Username, ctx.Reponame, treepath), &options).
+		req := NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", ctx.Username, ctx.Reponame, treepath), &options).
 			AddTokenAuth(ctx.Token)
 		if ctx.ExpectedCode != 0 {
 			ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)