Skip to content

Merge api on Create or update file contents #29419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Merge api on Create or update file contents #29419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
052737a
Merge api on Create or update file contents
smallfish06 Feb 26, 2024
3f3cc09
resolve conflict with main branch
smallfish06 Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions modules/structs/repo_file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type FileOptions struct {
}

// CreateFileOptions options for creating files
// Deprecated: Use CreateOrUpdateFileOptions instead
// 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
Expand Down Expand Up @@ -48,10 +49,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"`
Expand All @@ -60,7 +64,7 @@ type UpdateFileOptions struct {
}

// Branch returns branch name
func (o *UpdateFileOptions) Branch() string {
func (o *CreateOrUpdateFileOptions) Branch() string {
return o.FileOptions.BranchName
}

Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,7 @@ func Routes() *web.Route {
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))
Expand Down
62 changes: 40 additions & 22 deletions routers/api/v1/repo/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ func ChangeFiles(ctx *context.APIContext) {
}

// CreateFile handles API call for creating a file
// Deprecated: Use CreateOrUpdateFileOptions instead
func CreateFile(ctx *context.APIContext) {
// swagger:operation POST /repos/{owner}/{repo}/contents/{filepath} repository repoCreateFile
// ---
Expand Down Expand Up @@ -611,11 +612,11 @@ func CreateFile(ctx *context.APIContext) {
}
}

// UpdateFile handles API call for updating a file
func UpdateFile(ctx *context.APIContext) {
// swagger:operation PUT /repos/{owner}/{repo}/contents/{filepath} repository repoUpdateFile
// 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: Update a file in a repository
// summary: Create or update a file in a repository
// consumes:
// - application/json
// produces:
Expand All @@ -633,17 +634,19 @@ func UpdateFile(ctx *context.APIContext) {
// required: true
// - name: filepath
// in: path
// description: path of the file to update
// description: path of the file to create or 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":
Expand All @@ -652,10 +655,7 @@ func UpdateFile(ctx *context.APIContext) {
// "$ref": "#/responses/error"
// "423":
// "$ref": "#/responses/repoArchivedError"
apiOpts := web.GetForm(ctx).(*api.UpdateFileOptions)
if ctx.Repo.Repository.IsEmpty {
ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
}
apiOpts := web.GetForm(ctx).(*api.CreateOrUpdateFileOptions)

if apiOpts.BranchName == "" {
apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch
Expand All @@ -667,16 +667,30 @@ 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 {
if ctx.Repo.Repository.IsEmpty {
ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
return
}

changeRepoFile = files_service.ChangeRepoFile{
Operation: "update",
TreePath: ctx.Params("*"),
ContentReader: contentReader,
SHA: apiOpts.SHA,
FromTreePath: apiOpts.FromPath,
}
}

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,
Expand Down Expand Up @@ -709,7 +723,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.StatusCreated, fileResponse)
} else {
ctx.JSON(http.StatusOK, fileResponse)
}
Comment on lines +726 to +730
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part looks bit awkward to use SHA as flag of response. If you have any idea for this please let me know

I tried using file_service.GetContents to make clear for this but couldn't get more understandable code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using fragile "SHA" field, using a new "operation" is better.

Now:

  • create: for non-existing file
  • update: for existing file
  • upload: create or update

}
}

Expand All @@ -728,10 +746,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{
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func ApplyDiffPatch(ctx *context.APIContext) {
// in: body
// required: true
// schema:
// "$ref": "#/definitions/UpdateFileOptions"
// "$ref": "#/definitions/ApplyDiffPatchFileOptions"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swagger doc was using wrong schema

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by Fix diffpatch API endpoint (#35610)

// responses:
// "200":
// "$ref": "#/responses/FileResponse"
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/swagger/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ type swaggerParameterBodies struct {
CreateFileOptions api.CreateFileOptions

// in:body
UpdateFileOptions api.UpdateFileOptions
CreateOrUpdateFileOptions api.CreateOrUpdateFileOptions

// in:body
DeleteFileOptions api.DeleteFileOptions
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/api_helper_for_declarative_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading