From 5821f3c098128b5ed01f0c76d5cd17124c340c74 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Tue, 23 May 2023 08:17:26 +0200 Subject: [PATCH 01/11] add multiple file api --- modules/structs/repo_file.go | 23 + routers/api/v1/api.go | 1 + routers/api/v1/repo/file.go | 167 +++++- routers/api/v1/swagger/repo.go | 6 + routers/web/repo/editor.go | 36 +- services/repository/files/delete.go | 204 ------- services/repository/files/update.go | 530 ++++++++++-------- templates/swagger/v1_json.tmpl | 62 ++ tests/integration/api_repo_file_helpers.go | 18 +- tests/integration/pull_merge_test.go | 24 +- tests/integration/pull_update_test.go | 24 +- ...pdate_test.go => repofiles_change_test.go} | 283 +++++++--- tests/integration/repofiles_delete_test.go | 80 +-- 13 files changed, 852 insertions(+), 606 deletions(-) delete mode 100644 services/repository/files/delete.go rename tests/integration/{repofiles_update_test.go => repofiles_change_test.go} (54%) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 328d7e47c8e8..6ef4b3e36aa2 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -64,6 +64,29 @@ func (o *UpdateFileOptions) Branch() string { return o.FileOptions.BranchName } +// ChangeFileOperation for creating, updating or deleting a file +type ChangeFileOperation struct { + // required: true + // enum: create,update,delete + Operation string `json:"operation" binding:"Required"` + // path to the existing or new file + Path string `json:"path" binding:"MaxSize(500)"` + // content must be base64 encoded + // required: true + Content string `json:"content"` + // sha is the SHA for the file that already exists + SHA string `json:"sha"` + // old path of the file to move + FromPath string `json:"from_path"` +} + +// ChangeFilesOptions options for creating, updating or deleting multiple 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 ChangeFilesOptions struct { + FileOptions + Files []*ChangeFileOperation `json:"files"` +} + // FileOptionInterface provides a unified interface for the different file options type FileOptionInterface interface { Branch() string diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index fccfc5792ca7..45e36e84fe78 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1173,6 +1173,7 @@ func Routes(ctx gocontext.Context) *web.Route { m.Post("/diffpatch", reqRepoWriter(unit.TypeCode), reqToken(auth_model.AccessTokenScopeRepo), bind(api.ApplyDiffPatchFileOptions{}), repo.ApplyDiffPatch) m.Group("/contents", func() { m.Get("", repo.GetContentsList) + m.Post("", reqToken(auth_model.AccessTokenScopeRepo), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, repo.ChangeFiles) m.Get("/*", repo.GetContents) m.Group("/*", func() { m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, repo.CreateFile) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 786407827c9d..abe94e1011d4 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -407,6 +407,96 @@ func canReadFiles(r *context.Repository) bool { return r.Permission.CanRead(unit.TypeCode) } +// ChangeFiles handles API call for creating or updating multiple files +func ChangeFiles(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/contents repository repoChangeFiles + // --- + // summary: Create or update multiple files 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: body + // in: body + // required: true + // schema: + // "$ref": "#/definitions/ChangeFilesOptions" + // responses: + // "201": + // "$ref": "#/responses/FileListResponse" + // "403": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/error" + + apiOpts := web.GetForm(ctx).(*api.ChangeFilesOptions) + + if apiOpts.BranchName == "" { + apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch + } + + files := []*files_service.ChangeRepoFile{} + for _, file := range apiOpts.Files { + changeRepoFile := &files_service.ChangeRepoFile{ + Operation: file.Operation, + TreePath: file.Path, + FromTreePath: file.FromPath, + Content: file.Content, + SHA: file.SHA, + } + files = append(files, changeRepoFile) + } + + opts := &files_service.ChangeRepoFilesOptions{ + Files: files, + 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 = "Upload files over API" + } + + if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { + handleCreateOrUpdateFileError(ctx, err) + } else { + ctx.JSON(http.StatusCreated, filesResponse) + } +} + // CreateFile handles API call for creating a file func CreateFile(ctx *context.APIContext) { // swagger:operation POST /repos/{owner}/{repo}/contents/{filepath} repository repoCreateFile @@ -453,11 +543,15 @@ func CreateFile(ctx *context.APIContext) { apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch } - opts := &files_service.UpdateRepoFileOptions{ - Content: apiOpts.Content, - IsNewFile: true, + opts := &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: ctx.Params("*"), + Content: apiOpts.Content, + }, + }, Message: apiOpts.Message, - TreePath: ctx.Params("*"), OldBranch: apiOpts.BranchName, NewBranch: apiOpts.NewBranchName, Committer: &files_service.IdentityOptions{ @@ -482,13 +576,13 @@ func CreateFile(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = ctx.Tr("repo.editor.add", opts.TreePath) + opts.Message = ctx.Tr("repo.editor.add", opts.Files[0].TreePath) } - if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil { + if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - ctx.JSON(http.StatusCreated, fileResponse) + ctx.JSON(http.StatusCreated, filesResponse[0]) } } @@ -540,15 +634,19 @@ func UpdateFile(ctx *context.APIContext) { apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch } - opts := &files_service.UpdateRepoFileOptions{ - Content: apiOpts.Content, - SHA: apiOpts.SHA, - IsNewFile: false, - Message: apiOpts.Message, - FromTreePath: apiOpts.FromPath, - TreePath: ctx.Params("*"), - OldBranch: apiOpts.BranchName, - NewBranch: apiOpts.NewBranchName, + opts := &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "update", + Content: apiOpts.Content, + SHA: apiOpts.SHA, + FromTreePath: apiOpts.FromPath, + TreePath: ctx.Params("*"), + }, + }, + Message: apiOpts.Message, + OldBranch: apiOpts.BranchName, + NewBranch: apiOpts.NewBranchName, Committer: &files_service.IdentityOptions{ Name: apiOpts.Committer.Name, Email: apiOpts.Committer.Email, @@ -571,13 +669,13 @@ func UpdateFile(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = ctx.Tr("repo.editor.update", opts.TreePath) + opts.Message = ctx.Tr("repo.editor.update", opts.Files[0].TreePath) } - if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil { + if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - ctx.JSON(http.StatusOK, fileResponse) + ctx.JSON(http.StatusOK, filesResponse[0]) } } @@ -600,7 +698,7 @@ func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) { } // Called from both CreateFile or UpdateFile to handle both -func createOrUpdateFile(ctx *context.APIContext, opts *files_service.UpdateRepoFileOptions) (*api.FileResponse, error) { +func createOrUpdateFiles(ctx *context.APIContext, opts *files_service.ChangeRepoFilesOptions) ([]*api.FileResponse, error) { if !canWriteFiles(ctx, opts.OldBranch) { return nil, repo_model.ErrUserDoesNotHaveAccessToRepo{ UserID: ctx.Doer.ID, @@ -608,13 +706,15 @@ func createOrUpdateFile(ctx *context.APIContext, opts *files_service.UpdateRepoF } } - content, err := base64.StdEncoding.DecodeString(opts.Content) - if err != nil { - return nil, err + for _, file := range opts.Files { + content, err := base64.StdEncoding.DecodeString(file.Content) + if err != nil { + return nil, err + } + file.Content = string(content) } - opts.Content = string(content) - return files_service.CreateOrUpdateRepoFile(ctx, ctx.Repo.Repository, ctx.Doer, opts) + return files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts) } // DeleteFile Delete a file in a repository @@ -670,12 +770,17 @@ func DeleteFile(ctx *context.APIContext) { apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch } - opts := &files_service.DeleteRepoFileOptions{ + opts := &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "delete", + SHA: apiOpts.SHA, + TreePath: ctx.Params("*"), + }, + }, Message: apiOpts.Message, OldBranch: apiOpts.BranchName, NewBranch: apiOpts.NewBranchName, - SHA: apiOpts.SHA, - TreePath: ctx.Params("*"), Committer: &files_service.IdentityOptions{ Name: apiOpts.Committer.Name, Email: apiOpts.Committer.Email, @@ -698,10 +803,10 @@ func DeleteFile(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = ctx.Tr("repo.editor.delete", opts.TreePath) + opts.Message = ctx.Tr("repo.editor.delete", opts.Files[0].TreePath) } - if fileResponse, err := files_service.DeleteRepoFile(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { + if filesResponse, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { if git.IsErrBranchNotExist(err) || models.IsErrRepoFileDoesNotExist(err) || git.IsErrNotExist(err) { ctx.Error(http.StatusNotFound, "DeleteFile", err) return @@ -718,7 +823,7 @@ func DeleteFile(ctx *context.APIContext) { } ctx.Error(http.StatusInternalServerError, "DeleteFile", err) } else { - ctx.JSON(http.StatusOK, fileResponse) // FIXME on APIv2: return http.StatusNoContent + ctx.JSON(http.StatusOK, filesResponse[0]) // FIXME on APIv2: return http.StatusNoContent } } diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index 10056ac8cbb8..a6bdaa99930a 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -296,6 +296,12 @@ type swaggerFileResponse struct { Body api.FileResponse `json:"body"` } +// FileListResponse +// swagger:response FileListResponse +type swaggerFileListResponse struct { + Body []api.FileResponse `json:"body"` +} + // ContentsResponse // swagger:response ContentsResponse type swaggerContentsResponse struct { diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index b94aa1b7ba05..7433a0a56b16 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -272,18 +272,27 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b message += "\n\n" + form.CommitMessage } - if _, err := files_service.CreateOrUpdateRepoFile(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.UpdateRepoFileOptions{ + operation := "update" + if isNewFile { + operation = "create" + } + + if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{ LastCommitID: form.LastCommit, OldBranch: ctx.Repo.BranchName, NewBranch: branchName, - FromTreePath: ctx.Repo.TreePath, - TreePath: form.TreePath, Message: message, - Content: strings.ReplaceAll(form.Content, "\r", ""), - IsNewFile: isNewFile, - Signoff: form.Signoff, + Files: []*files_service.ChangeRepoFile{ + { + Operation: operation, + FromTreePath: ctx.Repo.TreePath, + TreePath: form.TreePath, + Content: strings.ReplaceAll(form.Content, "\r", ""), + }, + }, + Signoff: form.Signoff, }); err != nil { - // This is where we handle all the errors thrown by files_service.CreateOrUpdateRepoFile + // This is where we handle all the errors thrown by files_service.ChangeRepoFiles if git.IsErrNotExist(err) { ctx.RenderWithErr(ctx.Tr("repo.editor.file_editing_no_longer_exists", ctx.Repo.TreePath), tplEditFile, &form) } else if git_model.IsErrLFSFileLocked(err) { @@ -478,13 +487,18 @@ func DeleteFilePost(ctx *context.Context) { message += "\n\n" + form.CommitMessage } - if _, err := files_service.DeleteRepoFile(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.DeleteRepoFileOptions{ + if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{ LastCommitID: form.LastCommit, OldBranch: ctx.Repo.BranchName, NewBranch: branchName, - TreePath: ctx.Repo.TreePath, - Message: message, - Signoff: form.Signoff, + Files: []*files_service.ChangeRepoFile{ + { + Operation: "delete", + TreePath: ctx.Repo.TreePath, + }, + }, + Message: message, + Signoff: form.Signoff, }); err != nil { // This is where we handle all the errors thrown by repofiles.DeleteRepoFile if git.IsErrNotExist(err) || models.IsErrRepoFileDoesNotExist(err) { diff --git a/services/repository/files/delete.go b/services/repository/files/delete.go deleted file mode 100644 index faa60bb3bae4..000000000000 --- a/services/repository/files/delete.go +++ /dev/null @@ -1,204 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package files - -import ( - "context" - "fmt" - "strings" - - "code.gitea.io/gitea/models" - repo_model "code.gitea.io/gitea/models/repo" - user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/git" - api "code.gitea.io/gitea/modules/structs" -) - -// DeleteRepoFileOptions holds the repository delete file options -type DeleteRepoFileOptions struct { - LastCommitID string - OldBranch string - NewBranch string - TreePath string - Message string - SHA string - Author *IdentityOptions - Committer *IdentityOptions - Dates *CommitDateOptions - Signoff bool -} - -// DeleteRepoFile deletes a file in the given repository -func DeleteRepoFile(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *DeleteRepoFileOptions) (*api.FileResponse, error) { - // If no branch name is set, assume the repo's default branch - if opts.OldBranch == "" { - opts.OldBranch = repo.DefaultBranch - } - if opts.NewBranch == "" { - opts.NewBranch = opts.OldBranch - } - - gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, repo.RepoPath()) - if err != nil { - return nil, err - } - defer closer.Close() - - // oldBranch must exist for this operation - if _, err := gitRepo.GetBranch(opts.OldBranch); err != nil { - return nil, err - } - - // A NewBranch can be specified for the file to be created/updated in a new branch. - // Check to make sure the branch does not already exist, otherwise we can't proceed. - // If we aren't branching to a new branch, make sure user can commit to the given branch - if opts.NewBranch != opts.OldBranch { - newBranch, err := gitRepo.GetBranch(opts.NewBranch) - if err != nil && !git.IsErrBranchNotExist(err) { - return nil, err - } - if newBranch != nil { - return nil, models.ErrBranchAlreadyExists{ - BranchName: opts.NewBranch, - } - } - } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, opts.TreePath); err != nil { - return nil, err - } - - // Check that the path given in opts.treeName is valid (not a git path) - treePath := CleanUploadFileName(opts.TreePath) - if treePath == "" { - return nil, models.ErrFilenameInvalid{ - Path: opts.TreePath, - } - } - - message := strings.TrimSpace(opts.Message) - - author, committer := GetAuthorAndCommitterUsers(opts.Author, opts.Committer, doer) - - t, err := NewTemporaryUploadRepository(ctx, repo) - if err != nil { - return nil, err - } - defer t.Close() - if err := t.Clone(opts.OldBranch); err != nil { - return nil, err - } - if err := t.SetDefaultIndex(); err != nil { - return nil, err - } - - // Get the commit of the original branch - commit, err := t.GetBranchCommit(opts.OldBranch) - if err != nil { - return nil, err // Couldn't get a commit for the branch - } - - // Assigned LastCommitID in opts if it hasn't been set - if opts.LastCommitID == "" { - opts.LastCommitID = commit.ID.String() - } else { - lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID) - if err != nil { - return nil, fmt.Errorf("DeleteRepoFile: Invalid last commit ID: %w", err) - } - opts.LastCommitID = lastCommitID.String() - } - - // Get the files in the index - filesInIndex, err := t.LsFiles(opts.TreePath) - if err != nil { - return nil, fmt.Errorf("DeleteRepoFile: %w", err) - } - - // Find the file we want to delete in the index - inFilelist := false - for _, file := range filesInIndex { - if file == opts.TreePath { - inFilelist = true - break - } - } - if !inFilelist { - return nil, models.ErrRepoFileDoesNotExist{ - Path: opts.TreePath, - } - } - - // Get the entry of treePath and check if the SHA given is the same as the file - entry, err := commit.GetTreeEntryByPath(treePath) - if err != nil { - return nil, err - } - if opts.SHA != "" { - // If a SHA was given and the SHA given doesn't match the SHA of the fromTreePath, throw error - if opts.SHA != entry.ID.String() { - return nil, models.ErrSHADoesNotMatch{ - Path: treePath, - GivenSHA: opts.SHA, - CurrentSHA: entry.ID.String(), - } - } - } else if opts.LastCommitID != "" { - // If a lastCommitID was given and it doesn't match the commitID of the head of the branch throw - // an error, but only if we aren't creating a new branch. - if commit.ID.String() != opts.LastCommitID && opts.OldBranch == opts.NewBranch { - // CommitIDs don't match, but we don't want to throw a ErrCommitIDDoesNotMatch unless - // this specific file has been edited since opts.LastCommitID - if changed, err := commit.FileChangedSinceCommit(treePath, opts.LastCommitID); err != nil { - return nil, err - } else if changed { - return nil, models.ErrCommitIDDoesNotMatch{ - GivenCommitID: opts.LastCommitID, - CurrentCommitID: opts.LastCommitID, - } - } - // The file wasn't modified, so we are good to delete it - } - } else { - // When deleting a file, a lastCommitID or SHA needs to be given to make sure other commits haven't been - // made. We throw an error if one wasn't provided. - return nil, models.ErrSHAOrCommitIDNotProvided{} - } - - // Remove the file from the index - if err := t.RemoveFilesFromIndex(opts.TreePath); err != nil { - return nil, err - } - - // Now write the tree - treeHash, err := t.WriteTree() - if err != nil { - return nil, err - } - - // Now commit the tree - var commitHash string - if opts.Dates != nil { - commitHash, err = t.CommitTreeWithDate("HEAD", author, committer, treeHash, message, opts.Signoff, opts.Dates.Author, opts.Dates.Committer) - } else { - commitHash, err = t.CommitTree("HEAD", author, committer, treeHash, message, opts.Signoff) - } - if err != nil { - return nil, err - } - - // Then push this tree to NewBranch - if err := t.Push(doer, commitHash, opts.NewBranch); err != nil { - return nil, err - } - - commit, err = t.GetCommit(commitHash) - if err != nil { - return nil, err - } - - file, err := GetFileResponseFromCommit(ctx, repo, commit, opts.NewBranch, treePath) - if err != nil { - return nil, err - } - return file, nil -} diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 25014f441833..bea67c5d4f1f 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -41,23 +41,35 @@ type CommitDateOptions struct { Committer time.Time } -// UpdateRepoFileOptions holds the repository file update options -type UpdateRepoFileOptions struct { - LastCommitID string - OldBranch string - NewBranch string +type ChangeRepoFile struct { + Operation string TreePath string FromTreePath string - Message string Content string SHA string - IsNewFile bool +} + +// UpdateRepoFilesOptions holds the repository files update options +type ChangeRepoFilesOptions struct { + LastCommitID string + OldBranch string + NewBranch string + Message string + Files []*ChangeRepoFile Author *IdentityOptions Committer *IdentityOptions Dates *CommitDateOptions Signoff bool } +type RepoFileOptions struct { + treePath string + fromTreePath string + encoding string + bom bool + executable bool +} + func detectEncodingAndBOM(entry *git.TreeEntry, repo *repo_model.Repository) (string, bool) { reader, err := entry.Blob().DataAsync() if err != nil { @@ -125,8 +137,8 @@ func detectEncodingAndBOM(entry *git.TreeEntry, repo *repo_model.Repository) (st return encoding, false } -// CreateOrUpdateRepoFile adds or updates a file in the given repository -func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *UpdateRepoFileOptions) (*structs.FileResponse, error) { +// ChangeRepoFiles adds, updates or removes multiple files in the given repository +func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *ChangeRepoFilesOptions) ([]*structs.FileResponse, error) { // If no branch name is set, assume default branch if opts.OldBranch == "" { opts.OldBranch = repo.DefaultBranch @@ -146,6 +158,11 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do return nil, err } + treePaths := []string{} + for _, file := range opts.Files { + treePaths = append(treePaths, file.TreePath) + } + // A NewBranch can be specified for the file to be created/updated in a new branch. // Check to make sure the branch does not already exist, otherwise we can't proceed. // If we aren't branching to a new branch, make sure user can commit to the given branch @@ -159,30 +176,10 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do if err != nil && !git.IsErrBranchNotExist(err) { return nil, err } - } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, opts.TreePath); err != nil { + } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths); err != nil { return nil, err } - // If FromTreePath is not set, set it to the opts.TreePath - if opts.TreePath != "" && opts.FromTreePath == "" { - opts.FromTreePath = opts.TreePath - } - - // Check that the path given in opts.treePath is valid (not a git path) - treePath := CleanUploadFileName(opts.TreePath) - if treePath == "" { - return nil, models.ErrFilenameInvalid{ - Path: opts.TreePath, - } - } - // If there is a fromTreePath (we are copying it), also clean it up - fromTreePath := CleanUploadFileName(opts.FromTreePath) - if fromTreePath == "" && opts.FromTreePath != "" { - return nil, models.ErrFilenameInvalid{ - Path: opts.FromTreePath, - } - } - message := strings.TrimSpace(opts.Message) author, committer := GetAuthorAndCommitterUsers(opts.Author, opts.Committer, doer) @@ -192,26 +189,85 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do log.Error("%v", err) } defer t.Close() + hasOldBranch := true - if err := t.Clone(opts.OldBranch); err != nil { + err = t.Clone(opts.OldBranch) + if err != nil { if !git.IsErrBranchNotExist(err) || !repo.IsEmpty { return nil, err } if err := t.Init(); err != nil { return nil, err } - hasOldBranch = false - opts.LastCommitID = "" } - if hasOldBranch { - if err := t.SetDefaultIndex(); err != nil { - return nil, err + + filesOptions := map[string]*RepoFileOptions{} + for _, file := range opts.Files { + switch file.Operation { + case "create": + if err != nil { + hasOldBranch = false + opts.LastCommitID = "" + } + case "delete", "update": + if err != nil { + return nil, err + } + if err := t.SetDefaultIndex(); err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("Invalid file operation: %s %s", file.Operation, file.TreePath) } - } + // If FromTreePath is not set, set it to the opts.TreePath + if file.TreePath != "" && file.FromTreePath == "" { + file.FromTreePath = file.TreePath + } + + // Check that the path given in opts.treePath is valid (not a git path) + treePath := CleanUploadFileName(file.TreePath) + if treePath == "" { + return nil, models.ErrFilenameInvalid{ + Path: file.TreePath, + } + } + // If there is a fromTreePath (we are copying it), also clean it up + fromTreePath := CleanUploadFileName(file.FromTreePath) + if fromTreePath == "" && file.FromTreePath != "" { + return nil, models.ErrFilenameInvalid{ + Path: file.FromTreePath, + } + } + + filesOptions[file.TreePath] = &RepoFileOptions{ + treePath: treePath, + fromTreePath: fromTreePath, + encoding: "UTF-8", + bom: false, + executable: false, + } + if file.Operation == "delete" { + // Get the files in the index + filesInIndex, err := t.LsFiles(file.TreePath) + if err != nil { + return nil, fmt.Errorf("DeleteRepoFile: %w", err) + } - encoding := "UTF-8" - bom := false - executable := false + // Find the file we want to delete in the index + inFilelist := false + for _, indexFile := range filesInIndex { + if indexFile == file.TreePath { + inFilelist = true + break + } + } + if !inFilelist { + return nil, models.ErrRepoFileDoesNotExist{ + Path: file.TreePath, + } + } + } + } if hasOldBranch { // Get the commit of the original branch @@ -232,176 +288,215 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do } - if !opts.IsNewFile { - fromEntry, err := commit.GetTreeEntryByPath(fromTreePath) - if err != nil { - return nil, err - } - if opts.SHA != "" { - // If a SHA was given and the SHA given doesn't match the SHA of the fromTreePath, throw error - if opts.SHA != fromEntry.ID.String() { - return nil, models.ErrSHADoesNotMatch{ - Path: treePath, - GivenSHA: opts.SHA, - CurrentSHA: fromEntry.ID.String(), + for _, file := range opts.Files { + fileOptions := filesOptions[file.TreePath] + + if file.Operation == "update" || file.Operation == "delete" { + fromEntry, err := commit.GetTreeEntryByPath(fileOptions.fromTreePath) + if err != nil { + return nil, err + } + if file.SHA != "" { + // If a SHA was given and the SHA given doesn't match the SHA of the fromTreePath, throw error + if file.SHA != fromEntry.ID.String() { + return nil, models.ErrSHADoesNotMatch{ + Path: fileOptions.treePath, + GivenSHA: file.SHA, + CurrentSHA: fromEntry.ID.String(), + } } + } else if opts.LastCommitID != "" { + // If a lastCommitID was given and it doesn't match the commitID of the head of the branch throw + // an error, but only if we aren't creating a new branch. + if commit.ID.String() != opts.LastCommitID && opts.OldBranch == opts.NewBranch { + if changed, err := commit.FileChangedSinceCommit(fileOptions.treePath, opts.LastCommitID); err != nil { + return nil, err + } else if changed { + return nil, models.ErrCommitIDDoesNotMatch{ + GivenCommitID: opts.LastCommitID, + CurrentCommitID: opts.LastCommitID, + } + } + // The file wasn't modified, so we are good to delete it + } + } else { + // When updating a file, a lastCommitID or SHA needs to be given to make sure other commits + // haven't been made. We throw an error if one wasn't provided. + return nil, models.ErrSHAOrCommitIDNotProvided{} } - } else if opts.LastCommitID != "" { - // If a lastCommitID was given and it doesn't match the commitID of the head of the branch throw - // an error, but only if we aren't creating a new branch. - if commit.ID.String() != opts.LastCommitID && opts.OldBranch == opts.NewBranch { - if changed, err := commit.FileChangedSinceCommit(treePath, opts.LastCommitID); err != nil { + filesOptions[file.TreePath].encoding, filesOptions[file.TreePath].bom = detectEncodingAndBOM(fromEntry, repo) + filesOptions[file.TreePath].executable = fromEntry.IsExecutable() + } + if file.Operation == "create" || file.Operation == "update" { + // For the path where this file will be created/updated, we need to make + // sure no parts of the path are existing files or links except for the last + // item in the path which is the file name, and that shouldn't exist IF it is + // a new file OR is being moved to a new path. + treePathParts := strings.Split(fileOptions.treePath, "/") + subTreePath := "" + for index, part := range treePathParts { + subTreePath = path.Join(subTreePath, part) + entry, err := commit.GetTreeEntryByPath(subTreePath) + if err != nil { + if git.IsErrNotExist(err) { + // Means there is no item with that name, so we're good + break + } return nil, err - } else if changed { - return nil, models.ErrCommitIDDoesNotMatch{ - GivenCommitID: opts.LastCommitID, - CurrentCommitID: opts.LastCommitID, + } + if index < len(treePathParts)-1 { + if !entry.IsDir() { + return nil, models.ErrFilePathInvalid{ + Message: fmt.Sprintf("a file exists where you’re trying to create a subdirectory [path: %s]", subTreePath), + Path: subTreePath, + Name: part, + Type: git.EntryModeBlob, + } + } + } else if entry.IsLink() { + return nil, models.ErrFilePathInvalid{ + Message: fmt.Sprintf("a symbolic link exists where you’re trying to create a subdirectory [path: %s]", subTreePath), + Path: subTreePath, + Name: part, + Type: git.EntryModeSymlink, + } + } else if entry.IsDir() { + return nil, models.ErrFilePathInvalid{ + Message: fmt.Sprintf("a directory exists where you’re trying to create a file [path: %s]", subTreePath), + Path: subTreePath, + Name: part, + Type: git.EntryModeTree, + } + } else if fileOptions.fromTreePath != fileOptions.treePath || file.Operation == "create" { + // The entry shouldn't exist if we are creating new file or moving to a new path + return nil, models.ErrRepoFileAlreadyExists{ + Path: fileOptions.treePath, } } - // The file wasn't modified, so we are good to delete it + } - } else { - // When updating a file, a lastCommitID or SHA needs to be given to make sure other commits - // haven't been made. We throw an error if one wasn't provided. - return nil, models.ErrSHAOrCommitIDNotProvided{} } - encoding, bom = detectEncodingAndBOM(fromEntry, repo) - executable = fromEntry.IsExecutable() } + } - // For the path where this file will be created/updated, we need to make - // sure no parts of the path are existing files or links except for the last - // item in the path which is the file name, and that shouldn't exist IF it is - // a new file OR is being moved to a new path. - treePathParts := strings.Split(treePath, "/") - subTreePath := "" - for index, part := range treePathParts { - subTreePath = path.Join(subTreePath, part) - entry, err := commit.GetTreeEntryByPath(subTreePath) + for _, file := range opts.Files { + switch file.Operation { + case "create", "update": + fileOptions := filesOptions[file.TreePath] + + // Get the two paths (might be the same if not moving) from the index if they exist + filesInIndex, err := t.LsFiles(file.TreePath, file.FromTreePath) if err != nil { - if git.IsErrNotExist(err) { - // Means there is no item with that name, so we're good - break + return nil, fmt.Errorf("UpdateRepoFile: %w", err) + } + // If is a new file (not updating) then the given path shouldn't exist + if file.Operation == "create" { + for _, indexFile := range filesInIndex { + if indexFile == file.TreePath { + return nil, models.ErrRepoFileAlreadyExists{ + Path: file.TreePath, + } + } } - return nil, err } - if index < len(treePathParts)-1 { - if !entry.IsDir() { - return nil, models.ErrFilePathInvalid{ - Message: fmt.Sprintf("a file exists where you’re trying to create a subdirectory [path: %s]", subTreePath), - Path: subTreePath, - Name: part, - Type: git.EntryModeBlob, + + // Remove the old path from the tree + if fileOptions.fromTreePath != fileOptions.treePath && len(filesInIndex) > 0 { + for _, indexFile := range filesInIndex { + if indexFile == fileOptions.fromTreePath { + if err := t.RemoveFilesFromIndex(file.FromTreePath); err != nil { + return nil, err + } } } - } else if entry.IsLink() { - return nil, models.ErrFilePathInvalid{ - Message: fmt.Sprintf("a symbolic link exists where you’re trying to create a subdirectory [path: %s]", subTreePath), - Path: subTreePath, - Name: part, - Type: git.EntryModeSymlink, + } + + content := file.Content + if fileOptions.bom { + content = string(charset.UTF8BOM) + content + } + if fileOptions.encoding != "UTF-8" { + charsetEncoding, _ := stdcharset.Lookup(fileOptions.encoding) + if charsetEncoding != nil { + result, _, err := transform.String(charsetEncoding.NewEncoder(), content) + if err != nil { + // Look if we can't encode back in to the original we should just stick with utf-8 + log.Error("Error re-encoding %s (%s) as %s - will stay as UTF-8: %v", file.TreePath, file.FromTreePath, fileOptions.encoding, err) + result = content + } + content = result + } else { + log.Error("Unknown encoding: %s", fileOptions.encoding) } - } else if entry.IsDir() { - return nil, models.ErrFilePathInvalid{ - Message: fmt.Sprintf("a directory exists where you’re trying to create a file [path: %s]", subTreePath), - Path: subTreePath, - Name: part, - Type: git.EntryModeTree, + } + // Reset the opts.Content to our adjusted content to ensure that LFS gets the correct content + file.Content = content + var lfsMetaObject *git_model.LFSMetaObject + + if setting.LFS.StartServer && hasOldBranch { + // Check there is no way this can return multiple infos + filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ + Attributes: []string{"filter"}, + Filenames: []string{fileOptions.treePath}, + CachedOnly: true, + }) + if err != nil { + return nil, err } - } else if fromTreePath != treePath || opts.IsNewFile { - // The entry shouldn't exist if we are creating new file or moving to a new path - return nil, models.ErrRepoFileAlreadyExists{ - Path: treePath, + + if filename2attribute2info[fileOptions.treePath] != nil && filename2attribute2info[fileOptions.treePath]["filter"] == "lfs" { + // OK so we are supposed to LFS this data! + pointer, err := lfs.GeneratePointer(strings.NewReader(file.Content)) + if err != nil { + return nil, err + } + lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repo.ID} + content = pointer.StringContent() } } - } - } - - // Get the two paths (might be the same if not moving) from the index if they exist - filesInIndex, err := t.LsFiles(opts.TreePath, opts.FromTreePath) - if err != nil { - return nil, fmt.Errorf("UpdateRepoFile: %w", err) - } - // If is a new file (not updating) then the given path shouldn't exist - if opts.IsNewFile { - for _, file := range filesInIndex { - if file == opts.TreePath { - return nil, models.ErrRepoFileAlreadyExists{ - Path: opts.TreePath, - } + // Add the object to the database + objectHash, err := t.HashObject(strings.NewReader(content)) + if err != nil { + return nil, err } - } - } - // Remove the old path from the tree - if fromTreePath != treePath && len(filesInIndex) > 0 { - for _, file := range filesInIndex { - if file == fromTreePath { - if err := t.RemoveFilesFromIndex(opts.FromTreePath); err != nil { + // Add the object to the index + if fileOptions.executable { + if err := t.AddObjectToIndex("100755", objectHash, fileOptions.treePath); err != nil { + return nil, err + } + } else { + if err := t.AddObjectToIndex("100644", objectHash, fileOptions.treePath); err != nil { return nil, err } } - } - } - content := opts.Content - if bom { - content = string(charset.UTF8BOM) + content - } - if encoding != "UTF-8" { - charsetEncoding, _ := stdcharset.Lookup(encoding) - if charsetEncoding != nil { - result, _, err := transform.String(charsetEncoding.NewEncoder(), content) - if err != nil { - // Look if we can't encode back in to the original we should just stick with utf-8 - log.Error("Error re-encoding %s (%s) as %s - will stay as UTF-8: %v", opts.TreePath, opts.FromTreePath, encoding, err) - result = content + if lfsMetaObject != nil { + // We have an LFS object - create it + lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject) + if err != nil { + return nil, err + } + contentStore := lfs.NewContentStore() + exist, err := contentStore.Exists(lfsMetaObject.Pointer) + if err != nil { + return nil, err + } + if !exist { + if err := contentStore.Put(lfsMetaObject.Pointer, strings.NewReader(file.Content)); err != nil { + if _, err2 := git_model.RemoveLFSMetaObjectByOid(ctx, repo.ID, lfsMetaObject.Oid); err2 != nil { + return nil, fmt.Errorf("Error whilst removing failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, err2, err) + } + return nil, err + } + } } - content = result - } else { - log.Error("Unknown encoding: %s", encoding) - } - } - // Reset the opts.Content to our adjusted content to ensure that LFS gets the correct content - opts.Content = content - var lfsMetaObject *git_model.LFSMetaObject - - if setting.LFS.StartServer && hasOldBranch { - // Check there is no way this can return multiple infos - filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []string{"filter"}, - Filenames: []string{treePath}, - CachedOnly: true, - }) - if err != nil { - return nil, err - } - - if filename2attribute2info[treePath] != nil && filename2attribute2info[treePath]["filter"] == "lfs" { - // OK so we are supposed to LFS this data! - pointer, err := lfs.GeneratePointer(strings.NewReader(opts.Content)) - if err != nil { + case "delete": + // Remove the file from the index + if err := t.RemoveFilesFromIndex(file.TreePath); err != nil { return nil, err } - lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repo.ID} - content = pointer.StringContent() - } - } - // Add the object to the database - objectHash, err := t.HashObject(strings.NewReader(content)) - if err != nil { - return nil, err - } - - // Add the object to the index - if executable { - if err := t.AddObjectToIndex("100755", objectHash, treePath); err != nil { - return nil, err - } - } else { - if err := t.AddObjectToIndex("100644", objectHash, treePath); err != nil { - return nil, err } } @@ -422,27 +517,6 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do return nil, err } - if lfsMetaObject != nil { - // We have an LFS object - create it - lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject) - if err != nil { - return nil, err - } - contentStore := lfs.NewContentStore() - exist, err := contentStore.Exists(lfsMetaObject.Pointer) - if err != nil { - return nil, err - } - if !exist { - if err := contentStore.Put(lfsMetaObject.Pointer, strings.NewReader(opts.Content)); err != nil { - if _, err2 := git_model.RemoveLFSMetaObjectByOid(ctx, repo.ID, lfsMetaObject.Oid); err2 != nil { - return nil, fmt.Errorf("Error whilst removing failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, err2, err) - } - return nil, err - } - } - } - // Then push this tree to NewBranch if err := t.Push(doer, commitHash, opts.NewBranch); err != nil { log.Error("%T %v", err, err) @@ -454,35 +528,51 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do return nil, err } - file, err := GetFileResponseFromCommit(ctx, repo, commit, opts.NewBranch, treePath) - if err != nil { - return nil, err + files := []*structs.FileResponse{} + + for _, file := range opts.Files { + fileResponse, err := GetFileResponseFromCommit(ctx, repo, commit, opts.NewBranch, filesOptions[file.TreePath].treePath) + if err != nil { + return nil, err + } + files = append(files, fileResponse) } if repo.IsEmpty { _ = repo_model.UpdateRepositoryCols(ctx, &repo_model.Repository{ID: repo.ID, IsEmpty: false}, "is_empty") } - return file, nil + return files, nil } // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch -func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName, treePath string) error { +func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error { protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName) if err != nil { return err } if protectedBranch != nil { protectedBranch.Repo = repo - isUnprotectedFile := false - glob := protectedBranch.GetUnprotectedFilePatterns() - if len(glob) != 0 { - isUnprotectedFile = protectedBranch.IsUnprotectedFile(glob, treePath) - } - if !protectedBranch.CanUserPush(ctx, doer) && !isUnprotectedFile { - return models.ErrUserCannotCommit{ - UserName: doer.LowerName, + for _, treePath := range treePaths { + isUnprotectedFile := false + glob := protectedBranch.GetUnprotectedFilePatterns() + if len(glob) != 0 { + isUnprotectedFile = protectedBranch.IsUnprotectedFile(glob, treePath) + } + if !protectedBranch.CanUserPush(ctx, doer) && !isUnprotectedFile { + return models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + patterns := protectedBranch.GetProtectedFilePatterns() + for _, pat := range patterns { + if pat.Match(strings.ToLower(treePath)) { + return models.ErrFilePathProtected{ + Path: treePath, + } + } } + } if protectedBranch.RequireSignedCommits { _, _, _, err := asymkey_service.SignCRUDAction(ctx, repo.RepoPath(), doer, repo.RepoPath(), branchName) @@ -495,14 +585,6 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do } } } - patterns := protectedBranch.GetProtectedFilePatterns() - for _, pat := range patterns { - if pat.Match(strings.ToLower(treePath)) { - return models.ErrFilePathProtected{ - Path: treePath, - } - } - } } return nil } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 15043e465f90..a4d87b7f2efd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4063,6 +4063,57 @@ "$ref": "#/responses/notFound" } } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Create or update multiple files in a repository", + "operationId": "repoChangeFiles", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/ChangeFilesOptions" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/FileListResponse" + }, + "403": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/error" + } + } } }, "/repos/{owner}/{repo}/contents/{filepath}": { @@ -21990,6 +22041,17 @@ "$ref": "#/definitions/FileDeleteResponse" } }, + "FileListResponse": { + "description": "FileListResponse", + "headers": { + "body": { + "type": "array", + "items": { + "$ref": "#/definitions/FileResponse" + } + } + } + }, "FileResponse": { "description": "FileResponse", "schema": { diff --git a/tests/integration/api_repo_file_helpers.go b/tests/integration/api_repo_file_helpers.go index d773bcd6293c..96a6a06abc93 100644 --- a/tests/integration/api_repo_file_helpers.go +++ b/tests/integration/api_repo_file_helpers.go @@ -11,18 +11,22 @@ import ( files_service "code.gitea.io/gitea/services/repository/files" ) -func createFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName, content string) (*api.FileResponse, error) { - opts := &files_service.UpdateRepoFileOptions{ +func createFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName, content string) ([]*api.FileResponse, error) { + opts := &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: treePath, + Content: content, + }, + }, OldBranch: branchName, - TreePath: treePath, - Content: content, - IsNewFile: true, Author: nil, Committer: nil, } - return files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, user, opts) + return files_service.ChangeRepoFiles(git.DefaultContext, repo, user, opts) } -func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) (*api.FileResponse, error) { +func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) ([]*api.FileResponse, error) { return createFileInBranch(user, repo, treePath, repo.DefaultBranch, "This is a NEW file") } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 9271f25e5f0d..f6a36f60af2b 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -367,22 +367,30 @@ func TestConflictChecking(t *testing.T) { assert.NotEmpty(t, baseRepo) // create a commit on new branch. - _, err = files_service.CreateOrUpdateRepoFile(git.DefaultContext, baseRepo, user, &files_service.UpdateRepoFileOptions{ - TreePath: "important_file", + _, err = files_service.ChangeRepoFiles(git.DefaultContext, baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + Content: "Just a non-important file", + }, + }, Message: "Add a important file", - Content: "Just a non-important file", - IsNewFile: true, OldBranch: "main", NewBranch: "important-secrets", }) assert.NoError(t, err) // create a commit on main branch. - _, err = files_service.CreateOrUpdateRepoFile(git.DefaultContext, baseRepo, user, &files_service.UpdateRepoFileOptions{ - TreePath: "important_file", + _, err = files_service.ChangeRepoFiles(git.DefaultContext, baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + Content: "Not the same content :P", + }, + }, Message: "Add a important file", - Content: "Not the same content :P", - IsNewFile: true, OldBranch: "main", NewBranch: "main", }) diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index 1b66656518a3..b94731002f16 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -101,11 +101,15 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_mod assert.NotEmpty(t, headRepo) // create a commit on base Repo - _, err = files_service.CreateOrUpdateRepoFile(git.DefaultContext, baseRepo, actor, &files_service.UpdateRepoFileOptions{ - TreePath: "File_A", + _, err = files_service.ChangeRepoFiles(git.DefaultContext, baseRepo, actor, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "File_A", + Content: "File A", + }, + }, Message: "Add File A", - Content: "File A", - IsNewFile: true, OldBranch: "master", NewBranch: "master", Author: &files_service.IdentityOptions{ @@ -124,11 +128,15 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_mod assert.NoError(t, err) // create a commit on head Repo - _, err = files_service.CreateOrUpdateRepoFile(git.DefaultContext, headRepo, actor, &files_service.UpdateRepoFileOptions{ - TreePath: "File_B", + _, err = files_service.ChangeRepoFiles(git.DefaultContext, headRepo, actor, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "File_B", + Content: "File B", + }, + }, Message: "Add File on PR branch", - Content: "File B", - IsNewFile: true, OldBranch: "master", NewBranch: "newBranch", Author: &files_service.IdentityOptions{ diff --git a/tests/integration/repofiles_update_test.go b/tests/integration/repofiles_change_test.go similarity index 54% rename from tests/integration/repofiles_update_test.go rename to tests/integration/repofiles_change_test.go index 47b61c1eeb9f..222d3aeb4ea4 100644 --- a/tests/integration/repofiles_update_test.go +++ b/tests/integration/repofiles_change_test.go @@ -10,6 +10,7 @@ import ( "time" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -19,33 +20,90 @@ import ( "github.com/stretchr/testify/assert" ) -func getCreateRepoFileOptions(repo *repo_model.Repository) *files_service.UpdateRepoFileOptions { - return &files_service.UpdateRepoFileOptions{ +func getCreateRepoFilesOptions(repo *repo_model.Repository) *files_service.ChangeRepoFilesOptions { + return &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "new/file.txt", + Content: "This is a NEW file", + }, + }, OldBranch: repo.DefaultBranch, NewBranch: repo.DefaultBranch, - TreePath: "new/file.txt", Message: "Creates new/file.txt", - Content: "This is a NEW file", - IsNewFile: true, Author: nil, Committer: nil, } } -func getUpdateRepoFileOptions(repo *repo_model.Repository) *files_service.UpdateRepoFileOptions { - return &files_service.UpdateRepoFileOptions{ +func getUpdateRepoFilesOptions(repo *repo_model.Repository) *files_service.ChangeRepoFilesOptions { + return &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "update", + TreePath: "README.md", + SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", + Content: "This is UPDATED content for the README file", + }, + }, OldBranch: repo.DefaultBranch, NewBranch: repo.DefaultBranch, - TreePath: "README.md", Message: "Updates README.md", - SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", - Content: "This is UPDATED content for the README file", - IsNewFile: false, Author: nil, Committer: nil, } } +func getDeleteRepoFilesOptions(repo *repo_model.Repository) *files_service.ChangeRepoFilesOptions { + return &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "delete", + TreePath: "README.md", + SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", + }, + }, + LastCommitID: "", + OldBranch: repo.DefaultBranch, + NewBranch: repo.DefaultBranch, + Message: "Deletes README.md", + Author: &files_service.IdentityOptions{ + Name: "Bob Smith", + Email: "bob@smith.com", + }, + Committer: nil, + } +} + +func getExpectedFileResponseForRepofilesDelete(u *url.URL) *api.FileResponse { + // Just returns fields that don't change, i.e. fields with commit SHAs and dates can't be determined + return &api.FileResponse{ + Content: nil, + Commit: &api.FileCommitResponse{ + Author: &api.CommitUser{ + Identity: api.Identity{ + Name: "Bob Smith", + Email: "bob@smith.com", + }, + }, + Committer: &api.CommitUser{ + Identity: api.Identity{ + Name: "Bob Smith", + Email: "bob@smith.com", + }, + }, + Message: "Deletes README.md\n", + }, + Verification: &api.PayloadCommitVerification{ + Verified: false, + Reason: "gpg.error.not_signed_commit", + Signature: "", + Payload: "", + }, + } +} + func getExpectedFileResponseForRepofilesCreate(commitID, lastCommitSHA string) *api.FileResponse { treePath := "new/file.txt" encoding := "base64" @@ -183,7 +241,7 @@ func getExpectedFileResponseForRepofilesUpdate(commitID, filename, lastCommitSHA } } -func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { +func TestChangeRepoFilesForCreate(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { ctx := test.MockContext(t, "user2/repo1") @@ -196,10 +254,10 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { repo := ctx.Repo.Repository doer := ctx.Doer - opts := getCreateRepoFileOptions(repo) + opts := getCreateRepoFilesOptions(repo) // test - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) // asserts assert.NoError(t, err) @@ -211,16 +269,16 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { expectedFileResponse := getExpectedFileResponseForRepofilesCreate(commitID, lastCommit.ID.String()) assert.NotNil(t, expectedFileResponse) if expectedFileResponse != nil { - assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) - assert.EqualValues(t, expectedFileResponse.Commit.SHA, fileResponse.Commit.SHA) - assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name) + assert.EqualValues(t, expectedFileResponse.Content, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse[0].Commit.SHA) + assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse[0].Commit.HTMLURL) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, filesResponse[0].Commit.Author.Email) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, filesResponse[0].Commit.Author.Name) } }) } -func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { +func TestChangeRepoFilesForUpdate(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { ctx := test.MockContext(t, "user2/repo1") @@ -233,10 +291,10 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { repo := ctx.Repo.Repository doer := ctx.Doer - opts := getUpdateRepoFileOptions(repo) + opts := getUpdateRepoFilesOptions(repo) // test - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) // asserts assert.NoError(t, err) @@ -244,17 +302,17 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(opts.NewBranch) - lastCommit, _ := commit.GetCommitByPath(opts.TreePath) - expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.TreePath, lastCommit.ID.String()) - assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) - assert.EqualValues(t, expectedFileResponse.Commit.SHA, fileResponse.Commit.SHA) - assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name) + lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath) + expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String()) + assert.EqualValues(t, expectedFileResponse.Content, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse[0].Commit.SHA) + assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse[0].Commit.HTMLURL) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, filesResponse[0].Commit.Author.Email) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, filesResponse[0].Commit.Author.Name) }) } -func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { +func TestChangeRepoFilesForUpdateWithFileMove(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { ctx := test.MockContext(t, "user2/repo1") @@ -267,12 +325,12 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { repo := ctx.Repo.Repository doer := ctx.Doer - opts := getUpdateRepoFileOptions(repo) - opts.FromTreePath = "README.md" - opts.TreePath = "README_new.md" // new file name, README_new.md + opts := getUpdateRepoFilesOptions(repo) + opts.Files[0].FromTreePath = "README.md" + opts.Files[0].TreePath = "README_new.md" // new file name, README_new.md // test - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) // asserts assert.NoError(t, err) @@ -280,32 +338,32 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(opts.NewBranch) - lastCommit, _ := commit.GetCommitByPath(opts.TreePath) - expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.TreePath, lastCommit.ID.String()) + lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath) + expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String()) // assert that the old file no longer exists in the last commit of the branch - fromEntry, err := commit.GetTreeEntryByPath(opts.FromTreePath) + fromEntry, err := commit.GetTreeEntryByPath(opts.Files[0].FromTreePath) switch err.(type) { case git.ErrNotExist: // correct, continue default: t.Fatalf("expected git.ErrNotExist, got:%v", err) } - toEntry, err := commit.GetTreeEntryByPath(opts.TreePath) + toEntry, err := commit.GetTreeEntryByPath(opts.Files[0].TreePath) assert.NoError(t, err) assert.Nil(t, fromEntry) // Should no longer exist here assert.NotNil(t, toEntry) // Should exist here // assert SHA has remained the same but paths use the new file name - assert.EqualValues(t, expectedFileResponse.Content.SHA, fileResponse.Content.SHA) - assert.EqualValues(t, expectedFileResponse.Content.Name, fileResponse.Content.Name) - assert.EqualValues(t, expectedFileResponse.Content.Path, fileResponse.Content.Path) - assert.EqualValues(t, expectedFileResponse.Content.URL, fileResponse.Content.URL) - assert.EqualValues(t, expectedFileResponse.Commit.SHA, fileResponse.Commit.SHA) - assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) + assert.EqualValues(t, expectedFileResponse.Content.SHA, filesResponse[0].Content.SHA) + assert.EqualValues(t, expectedFileResponse.Content.Name, filesResponse[0].Content.Name) + assert.EqualValues(t, expectedFileResponse.Content.Path, filesResponse[0].Content.Path) + assert.EqualValues(t, expectedFileResponse.Content.URL, filesResponse[0].Content.URL) + assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse[0].Commit.SHA) + assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse[0].Commit.HTMLURL) }) } // Test opts with branch names removed, should get same results as above test -func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { +func TestChangeRepoFilesWithoutBranchNames(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { ctx := test.MockContext(t, "user2/repo1") @@ -318,12 +376,12 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { repo := ctx.Repo.Repository doer := ctx.Doer - opts := getUpdateRepoFileOptions(repo) + opts := getUpdateRepoFilesOptions(repo) opts.OldBranch = "" opts.NewBranch = "" // test - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) // asserts assert.NoError(t, err) @@ -331,13 +389,86 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(repo.DefaultBranch) - lastCommit, _ := commit.GetCommitByPath(opts.TreePath) - expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.TreePath, lastCommit.ID.String()) - assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) + lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath) + expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String()) + assert.EqualValues(t, expectedFileResponse.Content, filesResponse[0].Content) + }) +} + +func TestChangeRepoFilesForDelete(t *testing.T) { + onGiteaRun(t, testDeleteRepoFiles) +} + +func testDeleteRepoFiles(t *testing.T, u *url.URL) { + // setup + unittest.PrepareTestEnv(t) + ctx := test.MockContext(t, "user2/repo1") + ctx.SetParams(":id", "1") + test.LoadRepo(t, ctx, 1) + test.LoadRepoCommit(t, ctx) + test.LoadUser(t, ctx, 2) + test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository + doer := ctx.Doer + opts := getDeleteRepoFilesOptions(repo) + + t.Run("Delete README.md file", func(t *testing.T) { + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.NoError(t, err) + expectedFileResponse := getExpectedFileResponseForRepofilesDelete(u) + assert.NotNil(t, filesResponse) + assert.Nil(t, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) + assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) + assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) + }) + + t.Run("Verify README.md has been deleted", func(t *testing.T) { + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) + expectedError := "repository file does not exist [path: " + opts.Files[0].TreePath + "]" + assert.EqualError(t, err, expectedError) }) } -func TestCreateOrUpdateRepoFileErrors(t *testing.T) { +// Test opts with branch names removed, same results +func TestChangeRepoFilesForDeleteWithoutBranchNames(t *testing.T) { + onGiteaRun(t, testDeleteRepoFilesWithoutBranchNames) +} + +func testDeleteRepoFilesWithoutBranchNames(t *testing.T, u *url.URL) { + // setup + unittest.PrepareTestEnv(t) + ctx := test.MockContext(t, "user2/repo1") + ctx.SetParams(":id", "1") + test.LoadRepo(t, ctx, 1) + test.LoadRepoCommit(t, ctx) + test.LoadUser(t, ctx, 2) + test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + + repo := ctx.Repo.Repository + doer := ctx.Doer + opts := getDeleteRepoFilesOptions(repo) + opts.OldBranch = "" + opts.NewBranch = "" + + t.Run("Delete README.md without Branch Name", func(t *testing.T) { + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.NoError(t, err) + expectedFileResponse := getExpectedFileResponseForRepofilesDelete(u) + assert.NotNil(t, filesResponse) + assert.Nil(t, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) + assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) + assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) + }) +} + +func TestChangeRepoFilesErrors(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { ctx := test.MockContext(t, "user2/repo1") @@ -352,63 +483,63 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) { doer := ctx.Doer t.Run("bad branch", func(t *testing.T) { - opts := getUpdateRepoFileOptions(repo) + opts := getUpdateRepoFilesOptions(repo) opts.OldBranch = "bad_branch" - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) assert.Error(t, err) - assert.Nil(t, fileResponse) + assert.Nil(t, filesResponse) expectedError := "branch does not exist [name: " + opts.OldBranch + "]" assert.EqualError(t, err, expectedError) }) t.Run("bad SHA", func(t *testing.T) { - opts := getUpdateRepoFileOptions(repo) - origSHA := opts.SHA - opts.SHA = "bad_sha" - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + opts := getUpdateRepoFilesOptions(repo) + origSHA := opts.Files[0].SHA + opts.Files[0].SHA = "bad_sha" + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) - expectedError := "sha does not match [given: " + opts.SHA + ", expected: " + origSHA + "]" + expectedError := "sha does not match [given: " + opts.Files[0].SHA + ", expected: " + origSHA + "]" assert.EqualError(t, err, expectedError) }) t.Run("new branch already exists", func(t *testing.T) { - opts := getUpdateRepoFileOptions(repo) + opts := getUpdateRepoFilesOptions(repo) opts.NewBranch = "develop" - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) expectedError := "branch already exists [name: " + opts.NewBranch + "]" assert.EqualError(t, err, expectedError) }) t.Run("treePath is empty:", func(t *testing.T) { - opts := getUpdateRepoFileOptions(repo) - opts.TreePath = "" - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + opts := getUpdateRepoFilesOptions(repo) + opts.Files[0].TreePath = "" + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) expectedError := "path contains a malformed path component [path: ]" assert.EqualError(t, err, expectedError) }) t.Run("treePath is a git directory:", func(t *testing.T) { - opts := getUpdateRepoFileOptions(repo) - opts.TreePath = ".git" - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + opts := getUpdateRepoFilesOptions(repo) + opts.Files[0].TreePath = ".git" + filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) - expectedError := "path contains a malformed path component [path: " + opts.TreePath + "]" + expectedError := "path contains a malformed path component [path: " + opts.Files[0].TreePath + "]" assert.EqualError(t, err, expectedError) }) t.Run("create file that already exists", func(t *testing.T) { - opts := getCreateRepoFileOptions(repo) - opts.TreePath = "README.md" // already exists - fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts) + opts := getCreateRepoFilesOptions(repo) + opts.Files[0].TreePath = "README.md" // already exists + fileResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) assert.Nil(t, fileResponse) assert.Error(t, err) - expectedError := "repository file already exists [path: " + opts.TreePath + "]" + expectedError := "repository file already exists [path: " + opts.Files[0].TreePath + "]" assert.EqualError(t, err, expectedError) }) }) diff --git a/tests/integration/repofiles_delete_test.go b/tests/integration/repofiles_delete_test.go index 6698b280bdb5..305909fb976d 100644 --- a/tests/integration/repofiles_delete_test.go +++ b/tests/integration/repofiles_delete_test.go @@ -3,6 +3,7 @@ package integration +/* import ( "net/url" "testing" @@ -17,14 +18,18 @@ import ( "github.com/stretchr/testify/assert" ) -func getDeleteRepoFileOptions(repo *repo_model.Repository) *files_service.DeleteRepoFileOptions { - return &files_service.DeleteRepoFileOptions{ +func getDeleteRepoFileOptions(repo *repo_model.Repository) *files_service.DeleteRepoFilesOptions { + return &files_service.DeleteRepoFilesOptions{ + Files: []*files_service.DeleteRepoFile{ + { + TreePath: "README.md", + SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", + }, + }, LastCommitID: "", OldBranch: repo.DefaultBranch, NewBranch: repo.DefaultBranch, - TreePath: "README.md", Message: "Deletes README.md", - SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", Author: &files_service.IdentityOptions{ Name: "Bob Smith", Email: "bob@smith.com", @@ -80,21 +85,21 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) { opts := getDeleteRepoFileOptions(repo) t.Run("Delete README.md file", func(t *testing.T) { - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) assert.NoError(t, err) expectedFileResponse := getExpectedDeleteFileResponse(u) - assert.NotNil(t, fileResponse) - assert.Nil(t, fileResponse.Content) - assert.EqualValues(t, expectedFileResponse.Commit.Message, fileResponse.Commit.Message) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, fileResponse.Commit.Author.Identity) - assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, fileResponse.Commit.Committer.Identity) - assert.EqualValues(t, expectedFileResponse.Verification, fileResponse.Verification) + assert.NotNil(t, filesResponse) + assert.Nil(t, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) + assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) + assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) }) t.Run("Verify README.md has been deleted", func(t *testing.T) { - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) - expectedError := "repository file does not exist [path: " + opts.TreePath + "]" + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) + expectedError := "repository file does not exist [path: " + opts.Files[0].TreePath + "]" assert.EqualError(t, err, expectedError) }) } @@ -122,15 +127,15 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) { opts.NewBranch = "" t.Run("Delete README.md without Branch Name", func(t *testing.T) { - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) assert.NoError(t, err) expectedFileResponse := getExpectedDeleteFileResponse(u) - assert.NotNil(t, fileResponse) - assert.Nil(t, fileResponse.Content) - assert.EqualValues(t, expectedFileResponse.Commit.Message, fileResponse.Commit.Message) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, fileResponse.Commit.Author.Identity) - assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, fileResponse.Commit.Committer.Identity) - assert.EqualValues(t, expectedFileResponse.Verification, fileResponse.Verification) + assert.NotNil(t, filesResponse) + assert.Nil(t, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) + assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) + assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) }) } @@ -151,29 +156,29 @@ func TestDeleteRepoFileErrors(t *testing.T) { t.Run("Bad branch", func(t *testing.T) { opts := getDeleteRepoFileOptions(repo) opts.OldBranch = "bad_branch" - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) assert.Error(t, err) - assert.Nil(t, fileResponse) + assert.Nil(t, filesResponse) expectedError := "branch does not exist [name: " + opts.OldBranch + "]" assert.EqualError(t, err, expectedError) }) t.Run("Bad SHA", func(t *testing.T) { opts := getDeleteRepoFileOptions(repo) - origSHA := opts.SHA - opts.SHA = "bad_sha" - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + origSHA := opts.Files[0].SHA + opts.Files[0].SHA = "bad_sha" + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) - expectedError := "sha does not match [given: " + opts.SHA + ", expected: " + origSHA + "]" + expectedError := "sha does not match [given: " + opts.Files[0].SHA + ", expected: " + origSHA + "]" assert.EqualError(t, err, expectedError) }) t.Run("New branch already exists", func(t *testing.T) { opts := getDeleteRepoFileOptions(repo) opts.NewBranch = "develop" - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) expectedError := "branch already exists [name: " + opts.NewBranch + "]" assert.EqualError(t, err, expectedError) @@ -181,9 +186,9 @@ func TestDeleteRepoFileErrors(t *testing.T) { t.Run("TreePath is empty:", func(t *testing.T) { opts := getDeleteRepoFileOptions(repo) - opts.TreePath = "" - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + opts.Files[0].TreePath = "" + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) expectedError := "path contains a malformed path component [path: ]" assert.EqualError(t, err, expectedError) @@ -191,11 +196,12 @@ func TestDeleteRepoFileErrors(t *testing.T) { t.Run("TreePath is a git directory:", func(t *testing.T) { opts := getDeleteRepoFileOptions(repo) - opts.TreePath = ".git" - fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts) - assert.Nil(t, fileResponse) + opts.Files[0].TreePath = ".git" + filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) + assert.Nil(t, filesResponse) assert.Error(t, err) - expectedError := "path contains a malformed path component [path: " + opts.TreePath + "]" + expectedError := "path contains a malformed path component [path: " + opts.Files[0].TreePath + "]" assert.EqualError(t, err, expectedError) }) } +*/ From 30fd1c610e5500c2ca2332112c4155da92ff650b Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Tue, 23 May 2023 16:54:46 +0200 Subject: [PATCH 02/11] fix issues --- modules/structs/repo_file.go | 5 ++ routers/api/v1/swagger/options.go | 3 + routers/api/v1/swagger/repo.go | 1 + services/repository/files/update.go | 13 +++- templates/swagger/v1_json.tmpl | 93 +++++++++++++++++++++++++++-- 5 files changed, 106 insertions(+), 9 deletions(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 6ef4b3e36aa2..4dcfe62aad92 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -87,6 +87,11 @@ type ChangeFilesOptions struct { Files []*ChangeFileOperation `json:"files"` } +// Branch returns branch name +func (o *ChangeFilesOptions) Branch() string { + return o.FileOptions.BranchName +} + // FileOptionInterface provides a unified interface for the different file options type FileOptionInterface interface { Branch() string diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index 09bb1d18f3ae..353d32e2142e 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -116,6 +116,9 @@ type swaggerParameterBodies struct { // in:body EditAttachmentOptions api.EditAttachmentOptions + // in:body + ChangeFilesOptions api.ChangeFilesOptions + // in:body CreateFileOptions api.CreateFileOptions diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index a6bdaa99930a..f158101de2ea 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -299,6 +299,7 @@ type swaggerFileResponse struct { // FileListResponse // swagger:response FileListResponse type swaggerFileListResponse struct { + // in: body Body []api.FileResponse `json:"body"` } diff --git a/services/repository/files/update.go b/services/repository/files/update.go index bea67c5d4f1f..cb6aa84d3d97 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -213,12 +213,10 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use if err != nil { return nil, err } - if err := t.SetDefaultIndex(); err != nil { - return nil, err - } default: return nil, fmt.Errorf("Invalid file operation: %s %s", file.Operation, file.TreePath) } + // If FromTreePath is not set, set it to the opts.TreePath if file.TreePath != "" && file.FromTreePath == "" { file.FromTreePath = file.TreePath @@ -246,6 +244,15 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use bom: false, executable: false, } + } + + if hasOldBranch { + if err := t.SetDefaultIndex(); err != nil { + return nil, err + } + } + + for _, file := range opts.Files { if file.Operation == "delete" { // Get the files in the index filesInIndex, err := t.LsFiles(file.TreePath) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index a4d87b7f2efd..2e6723bc1fad 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -15942,6 +15942,89 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "ChangeFileOperation": { + "description": "ChangeFileOperation for creating, updating or deleting a file", + "type": "object", + "required": [ + "operation", + "content" + ], + "properties": { + "content": { + "description": "content must be base64 encoded", + "type": "string", + "x-go-name": "Content" + }, + "from_path": { + "description": "old path of the file to move", + "type": "string", + "x-go-name": "FromPath" + }, + "operation": { + "type": "string", + "enum": [ + "create", + "update", + "delete" + ], + "x-go-name": "Operation" + }, + "path": { + "description": "path to the existing or new file", + "type": "string", + "x-go-name": "Path" + }, + "sha": { + "description": "sha is the SHA for the file that already exists", + "type": "string", + "x-go-name": "SHA" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, + "ChangeFilesOptions": { + "description": "ChangeFilesOptions options for creating, updating or deleting multiple files\nNote: `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": "object", + "properties": { + "author": { + "$ref": "#/definitions/Identity" + }, + "branch": { + "description": "branch (optional) to base this file from. if not given, the default branch is used", + "type": "string", + "x-go-name": "BranchName" + }, + "committer": { + "$ref": "#/definitions/Identity" + }, + "dates": { + "$ref": "#/definitions/CommitDateOptions" + }, + "files": { + "type": "array", + "items": { + "$ref": "#/definitions/ChangeFileOperation" + }, + "x-go-name": "Files" + }, + "message": { + "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", + "type": "string", + "x-go-name": "Message" + }, + "new_branch": { + "description": "new_branch (optional) will make a new branch from `branch` before creating the file", + "type": "string", + "x-go-name": "NewBranchName" + }, + "signoff": { + "description": "Add a Signed-off-by trailer by the committer at the end of the commit log message.", + "type": "boolean", + "x-go-name": "Signoff" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "ChangedFile": { "description": "ChangedFile store information about files affected by the pull request", "type": "object", @@ -22043,12 +22126,10 @@ }, "FileListResponse": { "description": "FileListResponse", - "headers": { - "body": { - "type": "array", - "items": { - "$ref": "#/definitions/FileResponse" - } + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/FileResponse" } } }, From 162dedb56ad7b1301015ed69894516304d19c146 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Wed, 24 May 2023 06:47:45 +0200 Subject: [PATCH 03/11] remove obsolete test file --- tests/integration/repofiles_delete_test.go | 207 --------------------- 1 file changed, 207 deletions(-) delete mode 100644 tests/integration/repofiles_delete_test.go diff --git a/tests/integration/repofiles_delete_test.go b/tests/integration/repofiles_delete_test.go deleted file mode 100644 index 305909fb976d..000000000000 --- a/tests/integration/repofiles_delete_test.go +++ /dev/null @@ -1,207 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package integration - -/* -import ( - "net/url" - "testing" - - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/git" - api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/test" - files_service "code.gitea.io/gitea/services/repository/files" - - "github.com/stretchr/testify/assert" -) - -func getDeleteRepoFileOptions(repo *repo_model.Repository) *files_service.DeleteRepoFilesOptions { - return &files_service.DeleteRepoFilesOptions{ - Files: []*files_service.DeleteRepoFile{ - { - TreePath: "README.md", - SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", - }, - }, - LastCommitID: "", - OldBranch: repo.DefaultBranch, - NewBranch: repo.DefaultBranch, - Message: "Deletes README.md", - Author: &files_service.IdentityOptions{ - Name: "Bob Smith", - Email: "bob@smith.com", - }, - Committer: nil, - } -} - -func getExpectedDeleteFileResponse(u *url.URL) *api.FileResponse { - // Just returns fields that don't change, i.e. fields with commit SHAs and dates can't be determined - return &api.FileResponse{ - Content: nil, - Commit: &api.FileCommitResponse{ - Author: &api.CommitUser{ - Identity: api.Identity{ - Name: "Bob Smith", - Email: "bob@smith.com", - }, - }, - Committer: &api.CommitUser{ - Identity: api.Identity{ - Name: "Bob Smith", - Email: "bob@smith.com", - }, - }, - Message: "Deletes README.md\n", - }, - Verification: &api.PayloadCommitVerification{ - Verified: false, - Reason: "gpg.error.not_signed_commit", - Signature: "", - Payload: "", - }, - } -} - -func TestDeleteRepoFile(t *testing.T) { - onGiteaRun(t, testDeleteRepoFile) -} - -func testDeleteRepoFile(t *testing.T, u *url.URL) { - // setup - unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - test.LoadRepo(t, ctx, 1) - test.LoadRepoCommit(t, ctx) - test.LoadUser(t, ctx, 2) - test.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - repo := ctx.Repo.Repository - doer := ctx.Doer - opts := getDeleteRepoFileOptions(repo) - - t.Run("Delete README.md file", func(t *testing.T) { - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.NoError(t, err) - expectedFileResponse := getExpectedDeleteFileResponse(u) - assert.NotNil(t, filesResponse) - assert.Nil(t, filesResponse[0].Content) - assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) - assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) - assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) - }) - - t.Run("Verify README.md has been deleted", func(t *testing.T) { - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.Nil(t, filesResponse) - expectedError := "repository file does not exist [path: " + opts.Files[0].TreePath + "]" - assert.EqualError(t, err, expectedError) - }) -} - -// Test opts with branch names removed, same results -func TestDeleteRepoFileWithoutBranchNames(t *testing.T) { - onGiteaRun(t, testDeleteRepoFileWithoutBranchNames) -} - -func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) { - // setup - unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - test.LoadRepo(t, ctx, 1) - test.LoadRepoCommit(t, ctx) - test.LoadUser(t, ctx, 2) - test.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer - opts := getDeleteRepoFileOptions(repo) - opts.OldBranch = "" - opts.NewBranch = "" - - t.Run("Delete README.md without Branch Name", func(t *testing.T) { - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.NoError(t, err) - expectedFileResponse := getExpectedDeleteFileResponse(u) - assert.NotNil(t, filesResponse) - assert.Nil(t, filesResponse[0].Content) - assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) - assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) - assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) - }) -} - -func TestDeleteRepoFileErrors(t *testing.T) { - // setup - unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - test.LoadRepo(t, ctx, 1) - test.LoadRepoCommit(t, ctx) - test.LoadUser(t, ctx, 2) - test.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer - - t.Run("Bad branch", func(t *testing.T) { - opts := getDeleteRepoFileOptions(repo) - opts.OldBranch = "bad_branch" - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.Error(t, err) - assert.Nil(t, filesResponse) - expectedError := "branch does not exist [name: " + opts.OldBranch + "]" - assert.EqualError(t, err, expectedError) - }) - - t.Run("Bad SHA", func(t *testing.T) { - opts := getDeleteRepoFileOptions(repo) - origSHA := opts.Files[0].SHA - opts.Files[0].SHA = "bad_sha" - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.Nil(t, filesResponse) - assert.Error(t, err) - expectedError := "sha does not match [given: " + opts.Files[0].SHA + ", expected: " + origSHA + "]" - assert.EqualError(t, err, expectedError) - }) - - t.Run("New branch already exists", func(t *testing.T) { - opts := getDeleteRepoFileOptions(repo) - opts.NewBranch = "develop" - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.Nil(t, filesResponse) - assert.Error(t, err) - expectedError := "branch already exists [name: " + opts.NewBranch + "]" - assert.EqualError(t, err, expectedError) - }) - - t.Run("TreePath is empty:", func(t *testing.T) { - opts := getDeleteRepoFileOptions(repo) - opts.Files[0].TreePath = "" - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.Nil(t, filesResponse) - assert.Error(t, err) - expectedError := "path contains a malformed path component [path: ]" - assert.EqualError(t, err, expectedError) - }) - - t.Run("TreePath is a git directory:", func(t *testing.T) { - opts := getDeleteRepoFileOptions(repo) - opts.Files[0].TreePath = ".git" - filesResponse, err := files_service.DeleteRepoFiles(git.DefaultContext, repo, doer, opts) - assert.Nil(t, filesResponse) - assert.Error(t, err) - expectedError := "path contains a malformed path component [path: " + opts.Files[0].TreePath + "]" - assert.EqualError(t, err, expectedError) - }) -} -*/ From c5a7bebcf9a860b203741e4f68650c1a20c3a2c0 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Thu, 25 May 2023 22:02:13 +0200 Subject: [PATCH 04/11] add api test, informative commit messages --- routers/api/v1/repo/file.go | 39 ++- .../integration/api_repo_files_change_test.go | 314 ++++++++++++++++++ 2 files changed, 349 insertions(+), 4 deletions(-) create mode 100644 tests/integration/api_repo_files_change_test.go diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index abe94e1011d4..9caaf176a195 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "path" + "strings" "time" "code.gitea.io/gitea/models" @@ -487,7 +488,7 @@ func ChangeFiles(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = "Upload files over API" + opts.Message = changeFilesCommitMessage(ctx, files) } if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { @@ -576,7 +577,7 @@ func CreateFile(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = ctx.Tr("repo.editor.add", opts.Files[0].TreePath) + opts.Message = changeFilesCommitMessage(ctx, opts.Files) } if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { @@ -669,7 +670,7 @@ func UpdateFile(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = ctx.Tr("repo.editor.update", opts.Files[0].TreePath) + opts.Message = changeFilesCommitMessage(ctx, opts.Files) } if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { @@ -717,6 +718,36 @@ func createOrUpdateFiles(ctx *context.APIContext, opts *files_service.ChangeRepo return files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts) } +// format commit message if empty +func changeFilesCommitMessage(ctx *context.APIContext, files []*files_service.ChangeRepoFile) string { + var ( + createFiles []string + updateFiles []string + deleteFiles []string + ) + for _, file := range files { + switch file.Operation { + case "create": + createFiles = append(createFiles, file.TreePath) + case "update": + updateFiles = append(updateFiles, file.TreePath) + case "delete": + deleteFiles = append(deleteFiles, file.TreePath) + } + } + message := "" + if len(createFiles) != 0 { + message += ctx.Tr("repo.editor.add") + strings.Join(createFiles, ", ") + "\n" + } + if len(updateFiles) != 0 { + message += ctx.Tr("repo.editor.update") + strings.Join(updateFiles, ", ") + "\n" + } + if len(deleteFiles) != 0 { + message += ctx.Tr("repo.editor.delete") + strings.Join(deleteFiles, ", ") + "\n" + } + return message +} + // DeleteFile Delete a file in a repository func DeleteFile(ctx *context.APIContext) { // swagger:operation DELETE /repos/{owner}/{repo}/contents/{filepath} repository repoDeleteFile @@ -803,7 +834,7 @@ func DeleteFile(ctx *context.APIContext) { } if opts.Message == "" { - opts.Message = ctx.Tr("repo.editor.delete", opts.Files[0].TreePath) + opts.Message = changeFilesCommitMessage(ctx, opts.Files) } if filesResponse, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go new file mode 100644 index 000000000000..7ea0bb466dc6 --- /dev/null +++ b/tests/integration/api_repo_files_change_test.go @@ -0,0 +1,314 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + stdCtx "context" + "encoding/base64" + "fmt" + "net/http" + "net/url" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func getChangeFilesOptions() *api.ChangeFilesOptions { + newContent := "This is new text" + updateContent := "This is updated text" + newContentEncoded := base64.StdEncoding.EncodeToString([]byte(newContent)) + updateContentEncoded := base64.StdEncoding.EncodeToString([]byte(updateContent)) + return &api.ChangeFilesOptions{ + FileOptions: api.FileOptions{ + BranchName: "master", + NewBranchName: "master", + Message: "My update of new/file.txt", + Author: api.Identity{ + Name: "John Doe", + Email: "johndoe@example.com", + }, + Committer: api.Identity{ + Name: "Anne Doe", + Email: "annedoe@example.com", + }, + }, + Files: []*api.ChangeFileOperation{ + { + Operation: "create", + Content: newContentEncoded, + }, + { + Operation: "update", + Content: updateContentEncoded, + SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885", + }, + { + Operation: "delete", + SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885", + }, + }, + } +} + +func TestAPIUChangeFiles(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo1 & repo16 + user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // owner of the repo3, is an org + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // owner of neither repos + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) // public repo + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // public repo + repo16 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) // private repo + fileID := 0 + + // Get user2's token + session := loginUser(t, user2.Name) + token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeRepo) + // Get user4's token + session = loginUser(t, user4.Name) + token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeRepo) + + // Test changing files in repo1 which user2 owns, try both with branch and empty branch + for _, branch := range [...]string{ + "master", // Branch + "", // Empty branch + } { + fileID++ + createTreePath := fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath := fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath := fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + changeFilesOptions := getChangeFilesOptions() + changeFilesOptions.BranchName = branch + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url := fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo1.Name, token2) + req := NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + resp := MakeRequest(t, req, http.StatusOK) + gitRepo, _ := git.OpenRepository(stdCtx.Background(), repo1.RepoPath()) + commitID, _ := gitRepo.GetBranchCommitID(changeFilesOptions.NewBranchName) + createLasCommit, _ := gitRepo.GetCommitByPath(createTreePath) + updateLastCommit, _ := gitRepo.GetCommitByPath(updateTreePath) + expectedCreateFileResponse := getExpectedFileResponseForCreate(fmt.Sprintf("%v/%v", user2.Name, repo1.Name), commitID, createTreePath, createLasCommit.ID.String()) + expectedUpdateFileResponse := getExpectedFileResponseForUpdate(commitID, updateTreePath, updateLastCommit.ID.String()) + var fileResponse []api.FileResponse + DecodeJSON(t, resp, &fileResponse) + + // test create file + assert.EqualValues(t, expectedCreateFileResponse.Content, fileResponse[0].Content) + assert.EqualValues(t, expectedCreateFileResponse.Commit.SHA, fileResponse[0].Commit.SHA) + assert.EqualValues(t, expectedCreateFileResponse.Commit.HTMLURL, fileResponse[0].Commit.HTMLURL) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Email, fileResponse[0].Commit.Author.Email) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Name, fileResponse[0].Commit.Author.Name) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Date, fileResponse[0].Commit.Author.Date) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Email, fileResponse[0].Commit.Committer.Email) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Name, fileResponse[0].Commit.Committer.Name) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Date, fileResponse[0].Commit.Committer.Date) + + // test update file + assert.EqualValues(t, expectedUpdateFileResponse.Content, fileResponse[1].Content) + assert.EqualValues(t, expectedUpdateFileResponse.Commit.SHA, fileResponse[1].Commit.SHA) + assert.EqualValues(t, expectedUpdateFileResponse.Commit.HTMLURL, fileResponse[1].Commit.HTMLURL) + assert.EqualValues(t, expectedUpdateFileResponse.Commit.Author.Email, fileResponse[1].Commit.Author.Email) + assert.EqualValues(t, expectedUpdateFileResponse.Commit.Author.Name, fileResponse[1].Commit.Author.Name) + + // test delete file + assert.NotNil(t, fileResponse[2]) + assert.Nil(t, fileResponse[2].Content) + + gitRepo.Close() + } + + // Test changing files in a new branch + changeFilesOptions := getChangeFilesOptions() + changeFilesOptions.BranchName = repo1.DefaultBranch + changeFilesOptions.NewBranchName = "new_branch" + fileID++ + createTreePath := fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath := fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath := fmt.Sprintf("delete/file%d.txt", fileID) + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + url := fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo1.Name, token2) + req := NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + resp := MakeRequest(t, req, http.StatusOK) + var fileResponse []api.FileResponse + DecodeJSON(t, resp, &fileResponse) + expectedCreateSHA := "a635aa942442ddfdba07468cf9661c08fbdf0ebf" + expectedCreateHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/new_branch/new/file%d.txt", fileID) + expectedCreateDownloadURL := fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/new_branch/new/file%d.txt", fileID) + expectedUpdateSHA := "08bd14b2e2852529157324de9c226b3364e76136" + expectedUpdateHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/new_branch/update/file%d.txt", fileID) + expectedUpdateDownloadURL := fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/new_branch/update/file%d.txt", fileID) + assert.EqualValues(t, expectedCreateSHA, fileResponse[0].Content.SHA) + assert.EqualValues(t, expectedCreateHTMLURL, *fileResponse[0].Content.HTMLURL) + assert.EqualValues(t, expectedCreateDownloadURL, *fileResponse[0].Content.DownloadURL) + assert.EqualValues(t, changeFilesOptions.Message+"\n", fileResponse[0].Commit.Message) + assert.EqualValues(t, expectedUpdateSHA, fileResponse[1].Content.SHA) + assert.EqualValues(t, expectedUpdateHTMLURL, *fileResponse[1].Content.HTMLURL) + assert.EqualValues(t, expectedUpdateDownloadURL, *fileResponse[1].Content.DownloadURL) + assert.EqualValues(t, changeFilesOptions.Message+"\n", fileResponse[1].Commit.Message) + assert.NotNil(t, fileResponse[2]) + assert.Nil(t, fileResponse[2].Content) + assert.EqualValues(t, changeFilesOptions.Message+"\n", fileResponse[2].Commit.Message) + + // Test updating a file and renaming it + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.BranchName = repo1.DefaultBranch + fileID++ + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + createFile(user2, repo1, updateTreePath) + changeFilesOptions.Files = []*api.ChangeFileOperation{changeFilesOptions.Files[1]} + changeFilesOptions.Files[0].FromPath = updateTreePath + changeFilesOptions.Files[0].Path = "rename/" + updateTreePath + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &fileResponse) + expectedUpdateSHA = "08bd14b2e2852529157324de9c226b3364e76136" + expectedUpdateHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/master/rename/update/file%d.txt", fileID) + expectedUpdateDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/master/rename/update/file%d.txt", fileID) + assert.EqualValues(t, expectedUpdateSHA, fileResponse[0].Content.SHA) + assert.EqualValues(t, expectedUpdateHTMLURL, *fileResponse[0].Content.HTMLURL) + assert.EqualValues(t, expectedUpdateDownloadURL, *fileResponse[0].Content.DownloadURL) + + // Test updating a file without a message + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Message = "" + changeFilesOptions.BranchName = repo1.DefaultBranch + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &fileResponse) + expectedMessage := fmt.Sprintf("Create: %v\nUpdate: %v\nDelete: %v\n", createTreePath, updateTreePath, deleteTreePath) + for _, response := range fileResponse { + assert.EqualValues(t, expectedMessage, response.Commit.Message) + } + + // Test updating a file with the wrong SHA + fileID++ + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + createFile(user2, repo1, updateTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files = []*api.ChangeFileOperation{changeFilesOptions.Files[1]} + correctSHA := changeFilesOptions.Files[0].SHA + changeFilesOptions.Files[0].SHA = "badsha" + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + resp = MakeRequest(t, req, http.StatusUnprocessableEntity) + expectedAPIError := context.APIError{ + Message: "sha does not match [given: " + changeFilesOptions.Files[0].SHA + ", expected: " + correctSHA + "]", + URL: setting.API.SwaggerURL, + } + var apiError context.APIError + DecodeJSON(t, resp, &apiError) + assert.Equal(t, expectedAPIError, apiError) + + // Test creating a file in repo1 by user4 who does not have write access + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo16, updateTreePath) + createFile(user2, repo16, deleteTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo16.Name, token4) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + MakeRequest(t, req, http.StatusNotFound) + + // Tests a repo with no token given so will fail + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo16, updateTreePath) + createFile(user2, repo16, deleteTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user2.Name, repo16.Name) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + MakeRequest(t, req, http.StatusNotFound) + + // Test using access token for a private repo that the user of the token owns + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo16, updateTreePath) + createFile(user2, repo16, deleteTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo16.Name, token2) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + MakeRequest(t, req, http.StatusOK) + + // Test using org repo "user3/repo3" where user2 is a collaborator + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user3, repo3, updateTreePath) + createFile(user3, repo3, deleteTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user3.Name, repo3.Name, token2) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + MakeRequest(t, req, http.StatusOK) + + // Test using org repo "user3/repo3" with no user token + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user3, repo3, updateTreePath) + createFile(user3, repo3, deleteTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user3.Name, repo3.Name) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + MakeRequest(t, req, http.StatusNotFound) + + // Test using repo "user2/repo1" where user4 is a NOT collaborator + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo1.Name, token4) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) + MakeRequest(t, req, http.StatusForbidden) + }) +} From 164c18f34c345d526974148cc97c905de2e259b9 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Thu, 25 May 2023 22:51:06 +0200 Subject: [PATCH 05/11] fix tests --- routers/api/v1/repo/file.go | 6 ++-- .../integration/api_repo_files_change_test.go | 35 ++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 9caaf176a195..2f42ed2e10a9 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -737,13 +737,13 @@ func changeFilesCommitMessage(ctx *context.APIContext, files []*files_service.Ch } message := "" if len(createFiles) != 0 { - message += ctx.Tr("repo.editor.add") + strings.Join(createFiles, ", ") + "\n" + message += ctx.Tr("repo.editor.add", strings.Join(createFiles, ", ")+"\n") } if len(updateFiles) != 0 { - message += ctx.Tr("repo.editor.update") + strings.Join(updateFiles, ", ") + "\n" + message += ctx.Tr("repo.editor.update", strings.Join(updateFiles, ", ")+"\n") } if len(deleteFiles) != 0 { - message += ctx.Tr("repo.editor.delete") + strings.Join(deleteFiles, ", ") + "\n" + message += ctx.Tr("repo.editor.delete", strings.Join(deleteFiles, ", ")+"\n") } return message } diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go index 7ea0bb466dc6..279682f07c0a 100644 --- a/tests/integration/api_repo_files_change_test.go +++ b/tests/integration/api_repo_files_change_test.go @@ -34,13 +34,13 @@ func getChangeFilesOptions() *api.ChangeFilesOptions { NewBranchName: "master", Message: "My update of new/file.txt", Author: api.Identity{ - Name: "John Doe", - Email: "johndoe@example.com", - }, - Committer: api.Identity{ Name: "Anne Doe", Email: "annedoe@example.com", }, + Committer: api.Identity{ + Name: "John Doe", + Email: "johndoe@example.com", + }, }, Files: []*api.ChangeFileOperation{ { @@ -60,7 +60,7 @@ func getChangeFilesOptions() *api.ChangeFilesOptions { } } -func TestAPIUChangeFiles(t *testing.T) { +func TestAPIChangeFiles(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo1 & repo16 user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // owner of the repo3, is an org @@ -95,7 +95,7 @@ func TestAPIUChangeFiles(t *testing.T) { changeFilesOptions.Files[2].Path = deleteTreePath url := fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo1.Name, token2) req := NewRequestWithJSON(t, "POST", url, &changeFilesOptions) - resp := MakeRequest(t, req, http.StatusOK) + resp := MakeRequest(t, req, http.StatusCreated) gitRepo, _ := git.OpenRepository(stdCtx.Background(), repo1.RepoPath()) commitID, _ := gitRepo.GetBranchCommitID(changeFilesOptions.NewBranchName) createLasCommit, _ := gitRepo.GetCommitByPath(createTreePath) @@ -111,17 +111,14 @@ func TestAPIUChangeFiles(t *testing.T) { assert.EqualValues(t, expectedCreateFileResponse.Commit.HTMLURL, fileResponse[0].Commit.HTMLURL) assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Email, fileResponse[0].Commit.Author.Email) assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Name, fileResponse[0].Commit.Author.Name) - assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Date, fileResponse[0].Commit.Author.Date) assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Email, fileResponse[0].Commit.Committer.Email) assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Name, fileResponse[0].Commit.Committer.Name) - assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Date, fileResponse[0].Commit.Committer.Date) - // test update file assert.EqualValues(t, expectedUpdateFileResponse.Content, fileResponse[1].Content) assert.EqualValues(t, expectedUpdateFileResponse.Commit.SHA, fileResponse[1].Commit.SHA) assert.EqualValues(t, expectedUpdateFileResponse.Commit.HTMLURL, fileResponse[1].Commit.HTMLURL) - assert.EqualValues(t, expectedUpdateFileResponse.Commit.Author.Email, fileResponse[1].Commit.Author.Email) - assert.EqualValues(t, expectedUpdateFileResponse.Commit.Author.Name, fileResponse[1].Commit.Author.Name) + assert.EqualValues(t, expectedUpdateFileResponse.Commit.Committer.Email, fileResponse[1].Commit.Author.Email) + assert.EqualValues(t, expectedUpdateFileResponse.Commit.Committer.Name, fileResponse[1].Commit.Author.Name) // test delete file assert.NotNil(t, fileResponse[2]) @@ -145,7 +142,7 @@ func TestAPIUChangeFiles(t *testing.T) { createFile(user2, repo1, deleteTreePath) url := fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo1.Name, token2) req := NewRequestWithJSON(t, "POST", url, &changeFilesOptions) - resp := MakeRequest(t, req, http.StatusOK) + resp := MakeRequest(t, req, http.StatusCreated) var fileResponse []api.FileResponse DecodeJSON(t, resp, &fileResponse) expectedCreateSHA := "a635aa942442ddfdba07468cf9661c08fbdf0ebf" @@ -176,7 +173,7 @@ func TestAPIUChangeFiles(t *testing.T) { changeFilesOptions.Files[0].FromPath = updateTreePath changeFilesOptions.Files[0].Path = "rename/" + updateTreePath req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) - resp = MakeRequest(t, req, http.StatusOK) + resp = MakeRequest(t, req, http.StatusCreated) DecodeJSON(t, resp, &fileResponse) expectedUpdateSHA = "08bd14b2e2852529157324de9c226b3364e76136" expectedUpdateHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/master/rename/update/file%d.txt", fileID) @@ -193,12 +190,15 @@ func TestAPIUChangeFiles(t *testing.T) { createTreePath = fmt.Sprintf("new/file%d.txt", fileID) updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath createFile(user2, repo1, updateTreePath) createFile(user2, repo1, deleteTreePath) req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) - resp = MakeRequest(t, req, http.StatusOK) + resp = MakeRequest(t, req, http.StatusCreated) DecodeJSON(t, resp, &fileResponse) - expectedMessage := fmt.Sprintf("Create: %v\nUpdate: %v\nDelete: %v\n", createTreePath, updateTreePath, deleteTreePath) + expectedMessage := fmt.Sprintf("Add %v\nUpdate %v\nDelete %v\n", createTreePath, updateTreePath, deleteTreePath) for _, response := range fileResponse { assert.EqualValues(t, expectedMessage, response.Commit.Message) } @@ -209,6 +209,7 @@ func TestAPIUChangeFiles(t *testing.T) { createFile(user2, repo1, updateTreePath) changeFilesOptions = getChangeFilesOptions() changeFilesOptions.Files = []*api.ChangeFileOperation{changeFilesOptions.Files[1]} + changeFilesOptions.Files[0].Path = updateTreePath correctSHA := changeFilesOptions.Files[0].SHA changeFilesOptions.Files[0].SHA = "badsha" req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) @@ -264,7 +265,7 @@ func TestAPIUChangeFiles(t *testing.T) { changeFilesOptions.Files[2].Path = deleteTreePath url = fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo16.Name, token2) req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) - MakeRequest(t, req, http.StatusOK) + MakeRequest(t, req, http.StatusCreated) // Test using org repo "user3/repo3" where user2 is a collaborator fileID++ @@ -279,7 +280,7 @@ func TestAPIUChangeFiles(t *testing.T) { changeFilesOptions.Files[2].Path = deleteTreePath url = fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user3.Name, repo3.Name, token2) req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) - MakeRequest(t, req, http.StatusOK) + MakeRequest(t, req, http.StatusCreated) // Test using org repo "user3/repo3" with no user token fileID++ From 3f91092767665bc70f49282ebcff385d80f6e3fa Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 26 May 2023 07:59:46 +0200 Subject: [PATCH 06/11] trim newline in commit message --- routers/api/v1/repo/file.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 2f42ed2e10a9..1418229f3fc0 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -743,9 +743,9 @@ func changeFilesCommitMessage(ctx *context.APIContext, files []*files_service.Ch message += ctx.Tr("repo.editor.update", strings.Join(updateFiles, ", ")+"\n") } if len(deleteFiles) != 0 { - message += ctx.Tr("repo.editor.delete", strings.Join(deleteFiles, ", ")+"\n") + message += ctx.Tr("repo.editor.delete", strings.Join(deleteFiles, ", ")) } - return message + return strings.Trim(message, "\n") } // DeleteFile Delete a file in a repository From d0cfecf79775d17141e22ce0be1aca107c8e32de Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 26 May 2023 21:14:36 +0200 Subject: [PATCH 07/11] refactors, fixes --- modules/structs/repo_file.go | 7 + routers/api/v1/repo/file.go | 37 +- routers/api/v1/swagger/repo.go | 8 +- services/repository/files/file.go | 16 + services/repository/files/update.go | 522 +++++++++--------- templates/swagger/v1_json.tmpl | 37 +- tests/integration/api_repo_file_helpers.go | 4 +- .../integration/api_repo_files_change_test.go | 72 ++- tests/integration/repofiles_change_test.go | 54 +- 9 files changed, 406 insertions(+), 351 deletions(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 4dcfe62aad92..4313f93469c3 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -154,6 +154,13 @@ type FileResponse struct { Verification *PayloadCommitVerification `json:"verification"` } +// FilesResponse contains information about multiple files from a repo +type FilesResponse struct { + Files []*ContentsResponse `json:"files"` + Commit *FileCommitResponse `json:"commit"` + Verification *PayloadCommitVerification `json:"verification"` +} + // FileDeleteResponse contains information about a repo's file that was deleted type FileDeleteResponse struct { Content interface{} `json:"content"` // to be set to nil diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 1418229f3fc0..998001122f90 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -435,7 +435,7 @@ func ChangeFiles(ctx *context.APIContext) { // "$ref": "#/definitions/ChangeFilesOptions" // responses: // "201": - // "$ref": "#/responses/FileListResponse" + // "$ref": "#/responses/FilesResponse" // "403": // "$ref": "#/responses/error" // "404": @@ -583,7 +583,16 @@ func CreateFile(ctx *context.APIContext) { if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - ctx.JSON(http.StatusCreated, filesResponse[0]) + content := &api.ContentsResponse{} + if len(filesResponse.Files) > 0 { + content = filesResponse.Files[0] + } + fileResponse := &api.FileResponse{ + Content: content, + Commit: filesResponse.Commit, + Verification: filesResponse.Verification, + } + ctx.JSON(http.StatusCreated, fileResponse) } } @@ -676,7 +685,16 @@ func UpdateFile(ctx *context.APIContext) { if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - ctx.JSON(http.StatusOK, filesResponse[0]) + content := &api.ContentsResponse{} + if len(filesResponse.Files) > 0 { + content = filesResponse.Files[0] + } + fileResponse := &api.FileResponse{ + Content: content, + Commit: filesResponse.Commit, + Verification: filesResponse.Verification, + } + ctx.JSON(http.StatusOK, fileResponse) } } @@ -699,7 +717,7 @@ func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) { } // Called from both CreateFile or UpdateFile to handle both -func createOrUpdateFiles(ctx *context.APIContext, opts *files_service.ChangeRepoFilesOptions) ([]*api.FileResponse, error) { +func createOrUpdateFiles(ctx *context.APIContext, opts *files_service.ChangeRepoFilesOptions) (*api.FilesResponse, error) { if !canWriteFiles(ctx, opts.OldBranch) { return nil, repo_model.ErrUserDoesNotHaveAccessToRepo{ UserID: ctx.Doer.ID, @@ -854,7 +872,16 @@ func DeleteFile(ctx *context.APIContext) { } ctx.Error(http.StatusInternalServerError, "DeleteFile", err) } else { - ctx.JSON(http.StatusOK, filesResponse[0]) // FIXME on APIv2: return http.StatusNoContent + content := &api.ContentsResponse{} + if len(filesResponse.Files) > 0 { + content = filesResponse.Files[0] + } + fileResponse := &api.FileResponse{ + Content: content, + Commit: filesResponse.Commit, + Verification: filesResponse.Verification, + } + ctx.JSON(http.StatusOK, fileResponse) // FIXME on APIv2: return http.StatusNoContent } } diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index f158101de2ea..3e23aa4d5a5a 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -296,11 +296,11 @@ type swaggerFileResponse struct { Body api.FileResponse `json:"body"` } -// FileListResponse -// swagger:response FileListResponse -type swaggerFileListResponse struct { +// FilesResponse +// swagger:response FilesResponse +type swaggerFilesResponse struct { // in: body - Body []api.FileResponse `json:"body"` + Body api.FilesResponse `json:"body"` } // ContentsResponse diff --git a/services/repository/files/file.go b/services/repository/files/file.go index dc1e547dcdae..17b1646bb646 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -17,6 +17,22 @@ import ( "code.gitea.io/gitea/modules/util" ) +func GetFilesResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch string, treeNames []string) (*api.FilesResponse, error) { + files := []*api.ContentsResponse{} + for _, file := range treeNames { + fileContents, _ := GetContents(ctx, repo, file, branch, false) // ok if fails, then will be nil + files = append(files, fileContents) + } + fileCommitResponse, _ := GetFileCommitResponse(repo, commit) // ok if fails, then will be nil + verification := GetPayloadCommitVerification(ctx, commit) + filesResponse := &api.FilesResponse{ + Files: files, + Commit: fileCommitResponse, + Verification: verification, + } + return filesResponse, nil +} + // GetFileResponseFromCommit Constructs a FileResponse from a Commit object func GetFileResponseFromCommit(ctx context.Context, repo *repo_model.Repository, commit *git.Commit, branch, treeName string) (*api.FileResponse, error) { fileContents, _ := GetContents(ctx, repo, treeName, branch, false) // ok if fails, then will be nil diff --git a/services/repository/files/update.go b/services/repository/files/update.go index cb6aa84d3d97..2d04eab40f06 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -47,6 +47,7 @@ type ChangeRepoFile struct { FromTreePath string Content string SHA string + Options *RepoFileOptions } // UpdateRepoFilesOptions holds the repository files update options @@ -138,7 +139,7 @@ func detectEncodingAndBOM(entry *git.TreeEntry, repo *repo_model.Repository) (st } // ChangeRepoFiles adds, updates or removes multiple files in the given repository -func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *ChangeRepoFilesOptions) ([]*structs.FileResponse, error) { +func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *ChangeRepoFilesOptions) (*structs.FilesResponse, error) { // If no branch name is set, assume default branch if opts.OldBranch == "" { opts.OldBranch = repo.DefaultBranch @@ -160,7 +161,34 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use treePaths := []string{} for _, file := range opts.Files { - treePaths = append(treePaths, file.TreePath) + // If FromTreePath is not set, set it to the opts.TreePath + if file.TreePath != "" && file.FromTreePath == "" { + file.FromTreePath = file.TreePath + } + + // Check that the path given in opts.treePath is valid (not a git path) + treePath := CleanUploadFileName(file.TreePath) + if treePath == "" { + return nil, models.ErrFilenameInvalid{ + Path: file.TreePath, + } + } + // If there is a fromTreePath (we are copying it), also clean it up + fromTreePath := CleanUploadFileName(file.FromTreePath) + if fromTreePath == "" && file.FromTreePath != "" { + return nil, models.ErrFilenameInvalid{ + Path: file.FromTreePath, + } + } + + file.Options = &RepoFileOptions{ + treePath: treePath, + fromTreePath: fromTreePath, + encoding: "UTF-8", + bom: false, + executable: false, + } + treePaths = append(treePaths, treePath) } // A NewBranch can be specified for the file to be created/updated in a new branch. @@ -189,63 +217,22 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use log.Error("%v", err) } defer t.Close() - hasOldBranch := true - err = t.Clone(opts.OldBranch) - if err != nil { + if err := t.Clone(opts.OldBranch); err != nil { + for _, file := range opts.Files { + if file.Operation == "delete" { + return nil, err + } + } if !git.IsErrBranchNotExist(err) || !repo.IsEmpty { return nil, err } if err := t.Init(); err != nil { return nil, err } + hasOldBranch = false + opts.LastCommitID = "" } - - filesOptions := map[string]*RepoFileOptions{} - for _, file := range opts.Files { - switch file.Operation { - case "create": - if err != nil { - hasOldBranch = false - opts.LastCommitID = "" - } - case "delete", "update": - if err != nil { - return nil, err - } - default: - return nil, fmt.Errorf("Invalid file operation: %s %s", file.Operation, file.TreePath) - } - - // If FromTreePath is not set, set it to the opts.TreePath - if file.TreePath != "" && file.FromTreePath == "" { - file.FromTreePath = file.TreePath - } - - // Check that the path given in opts.treePath is valid (not a git path) - treePath := CleanUploadFileName(file.TreePath) - if treePath == "" { - return nil, models.ErrFilenameInvalid{ - Path: file.TreePath, - } - } - // If there is a fromTreePath (we are copying it), also clean it up - fromTreePath := CleanUploadFileName(file.FromTreePath) - if fromTreePath == "" && file.FromTreePath != "" { - return nil, models.ErrFilenameInvalid{ - Path: file.FromTreePath, - } - } - - filesOptions[file.TreePath] = &RepoFileOptions{ - treePath: treePath, - fromTreePath: fromTreePath, - encoding: "UTF-8", - bom: false, - executable: false, - } - } - if hasOldBranch { if err := t.SetDefaultIndex(); err != nil { return nil, err @@ -296,209 +283,19 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } for _, file := range opts.Files { - fileOptions := filesOptions[file.TreePath] - - if file.Operation == "update" || file.Operation == "delete" { - fromEntry, err := commit.GetTreeEntryByPath(fileOptions.fromTreePath) - if err != nil { - return nil, err - } - if file.SHA != "" { - // If a SHA was given and the SHA given doesn't match the SHA of the fromTreePath, throw error - if file.SHA != fromEntry.ID.String() { - return nil, models.ErrSHADoesNotMatch{ - Path: fileOptions.treePath, - GivenSHA: file.SHA, - CurrentSHA: fromEntry.ID.String(), - } - } - } else if opts.LastCommitID != "" { - // If a lastCommitID was given and it doesn't match the commitID of the head of the branch throw - // an error, but only if we aren't creating a new branch. - if commit.ID.String() != opts.LastCommitID && opts.OldBranch == opts.NewBranch { - if changed, err := commit.FileChangedSinceCommit(fileOptions.treePath, opts.LastCommitID); err != nil { - return nil, err - } else if changed { - return nil, models.ErrCommitIDDoesNotMatch{ - GivenCommitID: opts.LastCommitID, - CurrentCommitID: opts.LastCommitID, - } - } - // The file wasn't modified, so we are good to delete it - } - } else { - // When updating a file, a lastCommitID or SHA needs to be given to make sure other commits - // haven't been made. We throw an error if one wasn't provided. - return nil, models.ErrSHAOrCommitIDNotProvided{} - } - filesOptions[file.TreePath].encoding, filesOptions[file.TreePath].bom = detectEncodingAndBOM(fromEntry, repo) - filesOptions[file.TreePath].executable = fromEntry.IsExecutable() - } - if file.Operation == "create" || file.Operation == "update" { - // For the path where this file will be created/updated, we need to make - // sure no parts of the path are existing files or links except for the last - // item in the path which is the file name, and that shouldn't exist IF it is - // a new file OR is being moved to a new path. - treePathParts := strings.Split(fileOptions.treePath, "/") - subTreePath := "" - for index, part := range treePathParts { - subTreePath = path.Join(subTreePath, part) - entry, err := commit.GetTreeEntryByPath(subTreePath) - if err != nil { - if git.IsErrNotExist(err) { - // Means there is no item with that name, so we're good - break - } - return nil, err - } - if index < len(treePathParts)-1 { - if !entry.IsDir() { - return nil, models.ErrFilePathInvalid{ - Message: fmt.Sprintf("a file exists where you’re trying to create a subdirectory [path: %s]", subTreePath), - Path: subTreePath, - Name: part, - Type: git.EntryModeBlob, - } - } - } else if entry.IsLink() { - return nil, models.ErrFilePathInvalid{ - Message: fmt.Sprintf("a symbolic link exists where you’re trying to create a subdirectory [path: %s]", subTreePath), - Path: subTreePath, - Name: part, - Type: git.EntryModeSymlink, - } - } else if entry.IsDir() { - return nil, models.ErrFilePathInvalid{ - Message: fmt.Sprintf("a directory exists where you’re trying to create a file [path: %s]", subTreePath), - Path: subTreePath, - Name: part, - Type: git.EntryModeTree, - } - } else if fileOptions.fromTreePath != fileOptions.treePath || file.Operation == "create" { - // The entry shouldn't exist if we are creating new file or moving to a new path - return nil, models.ErrRepoFileAlreadyExists{ - Path: fileOptions.treePath, - } - } - - } + if err := handleCheckErrors(file, commit, opts, repo); err != nil { + return nil, err } } } + contentStore := lfs.NewContentStore() for _, file := range opts.Files { switch file.Operation { case "create", "update": - fileOptions := filesOptions[file.TreePath] - - // Get the two paths (might be the same if not moving) from the index if they exist - filesInIndex, err := t.LsFiles(file.TreePath, file.FromTreePath) - if err != nil { - return nil, fmt.Errorf("UpdateRepoFile: %w", err) - } - // If is a new file (not updating) then the given path shouldn't exist - if file.Operation == "create" { - for _, indexFile := range filesInIndex { - if indexFile == file.TreePath { - return nil, models.ErrRepoFileAlreadyExists{ - Path: file.TreePath, - } - } - } - } - - // Remove the old path from the tree - if fileOptions.fromTreePath != fileOptions.treePath && len(filesInIndex) > 0 { - for _, indexFile := range filesInIndex { - if indexFile == fileOptions.fromTreePath { - if err := t.RemoveFilesFromIndex(file.FromTreePath); err != nil { - return nil, err - } - } - } - } - - content := file.Content - if fileOptions.bom { - content = string(charset.UTF8BOM) + content - } - if fileOptions.encoding != "UTF-8" { - charsetEncoding, _ := stdcharset.Lookup(fileOptions.encoding) - if charsetEncoding != nil { - result, _, err := transform.String(charsetEncoding.NewEncoder(), content) - if err != nil { - // Look if we can't encode back in to the original we should just stick with utf-8 - log.Error("Error re-encoding %s (%s) as %s - will stay as UTF-8: %v", file.TreePath, file.FromTreePath, fileOptions.encoding, err) - result = content - } - content = result - } else { - log.Error("Unknown encoding: %s", fileOptions.encoding) - } - } - // Reset the opts.Content to our adjusted content to ensure that LFS gets the correct content - file.Content = content - var lfsMetaObject *git_model.LFSMetaObject - - if setting.LFS.StartServer && hasOldBranch { - // Check there is no way this can return multiple infos - filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []string{"filter"}, - Filenames: []string{fileOptions.treePath}, - CachedOnly: true, - }) - if err != nil { - return nil, err - } - - if filename2attribute2info[fileOptions.treePath] != nil && filename2attribute2info[fileOptions.treePath]["filter"] == "lfs" { - // OK so we are supposed to LFS this data! - pointer, err := lfs.GeneratePointer(strings.NewReader(file.Content)) - if err != nil { - return nil, err - } - lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repo.ID} - content = pointer.StringContent() - } - } - - // Add the object to the database - objectHash, err := t.HashObject(strings.NewReader(content)) - if err != nil { + if err := CreateOrUpdateFile(ctx, t, file, contentStore, repo.ID, hasOldBranch); err != nil { return nil, err } - - // Add the object to the index - if fileOptions.executable { - if err := t.AddObjectToIndex("100755", objectHash, fileOptions.treePath); err != nil { - return nil, err - } - } else { - if err := t.AddObjectToIndex("100644", objectHash, fileOptions.treePath); err != nil { - return nil, err - } - } - - if lfsMetaObject != nil { - // We have an LFS object - create it - lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject) - if err != nil { - return nil, err - } - contentStore := lfs.NewContentStore() - exist, err := contentStore.Exists(lfsMetaObject.Pointer) - if err != nil { - return nil, err - } - if !exist { - if err := contentStore.Put(lfsMetaObject.Pointer, strings.NewReader(file.Content)); err != nil { - if _, err2 := git_model.RemoveLFSMetaObjectByOid(ctx, repo.ID, lfsMetaObject.Oid); err2 != nil { - return nil, fmt.Errorf("Error whilst removing failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, err2, err) - } - return nil, err - } - } - } case "delete": // Remove the file from the index if err := t.RemoveFilesFromIndex(file.TreePath); err != nil { @@ -535,21 +332,220 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil, err } - files := []*structs.FileResponse{} + filesReponse, err := GetFilesResponseFromCommit(ctx, repo, commit, opts.NewBranch, treePaths) + if err != nil { + return nil, err + } + + if repo.IsEmpty { + _ = repo_model.UpdateRepositoryCols(ctx, &repo_model.Repository{ID: repo.ID, IsEmpty: false}, "is_empty") + } - for _, file := range opts.Files { - fileResponse, err := GetFileResponseFromCommit(ctx, repo, commit, opts.NewBranch, filesOptions[file.TreePath].treePath) + return filesReponse, nil +} + +// handles the check for various issues for ChangeRepoFiles +func handleCheckErrors(file *ChangeRepoFile, commit *git.Commit, opts *ChangeRepoFilesOptions, repo *repo_model.Repository) error { + if file.Operation == "update" || file.Operation == "delete" { + fromEntry, err := commit.GetTreeEntryByPath(file.Options.fromTreePath) if err != nil { - return nil, err + return err } - files = append(files, fileResponse) + if file.SHA != "" { + // If a SHA was given and the SHA given doesn't match the SHA of the fromTreePath, throw error + if file.SHA != fromEntry.ID.String() { + return models.ErrSHADoesNotMatch{ + Path: file.Options.treePath, + GivenSHA: file.SHA, + CurrentSHA: fromEntry.ID.String(), + } + } + } else if opts.LastCommitID != "" { + // If a lastCommitID was given and it doesn't match the commitID of the head of the branch throw + // an error, but only if we aren't creating a new branch. + if commit.ID.String() != opts.LastCommitID && opts.OldBranch == opts.NewBranch { + if changed, err := commit.FileChangedSinceCommit(file.Options.treePath, opts.LastCommitID); err != nil { + return err + } else if changed { + return models.ErrCommitIDDoesNotMatch{ + GivenCommitID: opts.LastCommitID, + CurrentCommitID: opts.LastCommitID, + } + } + // The file wasn't modified, so we are good to delete it + } + } else { + // When updating a file, a lastCommitID or SHA needs to be given to make sure other commits + // haven't been made. We throw an error if one wasn't provided. + return models.ErrSHAOrCommitIDNotProvided{} + } + file.Options.encoding, file.Options.bom = detectEncodingAndBOM(fromEntry, repo) + file.Options.executable = fromEntry.IsExecutable() } + if file.Operation == "create" || file.Operation == "update" { + // For the path where this file will be created/updated, we need to make + // sure no parts of the path are existing files or links except for the last + // item in the path which is the file name, and that shouldn't exist IF it is + // a new file OR is being moved to a new path. + treePathParts := strings.Split(file.Options.treePath, "/") + subTreePath := "" + for index, part := range treePathParts { + subTreePath = path.Join(subTreePath, part) + entry, err := commit.GetTreeEntryByPath(subTreePath) + if err != nil { + if git.IsErrNotExist(err) { + // Means there is no item with that name, so we're good + break + } + return err + } + if index < len(treePathParts)-1 { + if !entry.IsDir() { + return models.ErrFilePathInvalid{ + Message: fmt.Sprintf("a file exists where you’re trying to create a subdirectory [path: %s]", subTreePath), + Path: subTreePath, + Name: part, + Type: git.EntryModeBlob, + } + } + } else if entry.IsLink() { + return models.ErrFilePathInvalid{ + Message: fmt.Sprintf("a symbolic link exists where you’re trying to create a subdirectory [path: %s]", subTreePath), + Path: subTreePath, + Name: part, + Type: git.EntryModeSymlink, + } + } else if entry.IsDir() { + return models.ErrFilePathInvalid{ + Message: fmt.Sprintf("a directory exists where you’re trying to create a file [path: %s]", subTreePath), + Path: subTreePath, + Name: part, + Type: git.EntryModeTree, + } + } else if file.Options.fromTreePath != file.Options.treePath || file.Operation == "create" { + // The entry shouldn't exist if we are creating new file or moving to a new path + return models.ErrRepoFileAlreadyExists{ + Path: file.Options.treePath, + } + } - if repo.IsEmpty { - _ = repo_model.UpdateRepositoryCols(ctx, &repo_model.Repository{ID: repo.ID, IsEmpty: false}, "is_empty") + } + } + + return nil +} + +// handle creating or updating a file for ChangeRepoFiles +func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, contentStore *lfs.ContentStore, repoID int64, hasOldBranch bool) error { + // Get the two paths (might be the same if not moving) from the index if they exist + filesInIndex, err := t.LsFiles(file.TreePath, file.FromTreePath) + if err != nil { + return fmt.Errorf("UpdateRepoFile: %w", err) + } + // If is a new file (not updating) then the given path shouldn't exist + if file.Operation == "create" { + for _, indexFile := range filesInIndex { + if indexFile == file.TreePath { + return models.ErrRepoFileAlreadyExists{ + Path: file.TreePath, + } + } + } + } + + // Remove the old path from the tree + if file.Options.fromTreePath != file.Options.treePath && len(filesInIndex) > 0 { + for _, indexFile := range filesInIndex { + if indexFile == file.Options.fromTreePath { + if err := t.RemoveFilesFromIndex(file.FromTreePath); err != nil { + return err + } + } + } + } + + content := file.Content + if file.Options.bom { + content = string(charset.UTF8BOM) + content + } + if file.Options.encoding != "UTF-8" { + charsetEncoding, _ := stdcharset.Lookup(file.Options.encoding) + if charsetEncoding != nil { + result, _, err := transform.String(charsetEncoding.NewEncoder(), content) + if err != nil { + // Look if we can't encode back in to the original we should just stick with utf-8 + log.Error("Error re-encoding %s (%s) as %s - will stay as UTF-8: %v", file.TreePath, file.FromTreePath, file.Options.encoding, err) + result = content + } + content = result + } else { + log.Error("Unknown encoding: %s", file.Options.encoding) + } + } + // Reset the opts.Content to our adjusted content to ensure that LFS gets the correct content + file.Content = content + var lfsMetaObject *git_model.LFSMetaObject + + if setting.LFS.StartServer && hasOldBranch { + // Check there is no way this can return multiple infos + filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ + Attributes: []string{"filter"}, + Filenames: []string{file.Options.treePath}, + CachedOnly: true, + }) + if err != nil { + return err + } + + if filename2attribute2info[file.Options.treePath] != nil && filename2attribute2info[file.Options.treePath]["filter"] == "lfs" { + // OK so we are supposed to LFS this data! + pointer, err := lfs.GeneratePointer(strings.NewReader(file.Content)) + if err != nil { + return err + } + lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repoID} + content = pointer.StringContent() + } + } + + // Add the object to the database + objectHash, err := t.HashObject(strings.NewReader(content)) + if err != nil { + return err + } + + // Add the object to the index + if file.Options.executable { + if err := t.AddObjectToIndex("100755", objectHash, file.Options.treePath); err != nil { + return err + } + } else { + if err := t.AddObjectToIndex("100644", objectHash, file.Options.treePath); err != nil { + return err + } } - return files, nil + if lfsMetaObject != nil { + // We have an LFS object - create it + lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject) + if err != nil { + return err + } + exist, err := contentStore.Exists(lfsMetaObject.Pointer) + if err != nil { + return err + } + if !exist { + if err := contentStore.Put(lfsMetaObject.Pointer, strings.NewReader(file.Content)); err != nil { + if _, err2 := git_model.RemoveLFSMetaObjectByOid(ctx, repoID, lfsMetaObject.Oid); err2 != nil { + return fmt.Errorf("Error whilst removing failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, err2, err) + } + return err + } + } + } + + return nil } // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch @@ -560,26 +556,24 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do } if protectedBranch != nil { protectedBranch.Repo = repo + globUnprotected := protectedBranch.GetUnprotectedFilePatterns() + globProtected := protectedBranch.GetProtectedFilePatterns() + canUserPush := protectedBranch.CanUserPush(ctx, doer) for _, treePath := range treePaths { isUnprotectedFile := false - glob := protectedBranch.GetUnprotectedFilePatterns() - if len(glob) != 0 { - isUnprotectedFile = protectedBranch.IsUnprotectedFile(glob, treePath) + if len(globUnprotected) != 0 { + isUnprotectedFile = protectedBranch.IsUnprotectedFile(globUnprotected, treePath) } - if !protectedBranch.CanUserPush(ctx, doer) && !isUnprotectedFile { + if !canUserPush && !isUnprotectedFile { return models.ErrUserCannotCommit{ UserName: doer.LowerName, } } - patterns := protectedBranch.GetProtectedFilePatterns() - for _, pat := range patterns { - if pat.Match(strings.ToLower(treePath)) { - return models.ErrFilePathProtected{ - Path: treePath, - } + if protectedBranch.IsProtectedFile(globProtected, treePath) { + return models.ErrFilePathProtected{ + Path: treePath, } } - } if protectedBranch.RequireSignedCommits { _, _, _, err := asymkey_service.SignCRUDAction(ctx, repo.RepoPath(), doer, repo.RepoPath(), branchName) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 2e6723bc1fad..b65f08bc6686 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4102,7 +4102,7 @@ ], "responses": { "201": { - "$ref": "#/responses/FileListResponse" + "$ref": "#/responses/FilesResponse" }, "403": { "$ref": "#/responses/error" @@ -18460,6 +18460,26 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "FilesResponse": { + "description": "FilesResponse contains information about multiple files from a repo", + "type": "object", + "properties": { + "commit": { + "$ref": "#/definitions/FileCommitResponse" + }, + "files": { + "type": "array", + "items": { + "$ref": "#/definitions/ContentsResponse" + }, + "x-go-name": "Files" + }, + "verification": { + "$ref": "#/definitions/PayloadCommitVerification" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "GPGKey": { "description": "GPGKey a user GPG key to sign commit and tag in repository", "type": "object", @@ -22124,21 +22144,18 @@ "$ref": "#/definitions/FileDeleteResponse" } }, - "FileListResponse": { - "description": "FileListResponse", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/FileResponse" - } - } - }, "FileResponse": { "description": "FileResponse", "schema": { "$ref": "#/definitions/FileResponse" } }, + "FilesResponse": { + "description": "FilesResponse", + "schema": { + "$ref": "#/definitions/FilesResponse" + } + }, "GPGKey": { "description": "GPGKey", "schema": { diff --git a/tests/integration/api_repo_file_helpers.go b/tests/integration/api_repo_file_helpers.go index 96a6a06abc93..8e9b2bfeccf8 100644 --- a/tests/integration/api_repo_file_helpers.go +++ b/tests/integration/api_repo_file_helpers.go @@ -11,7 +11,7 @@ import ( files_service "code.gitea.io/gitea/services/repository/files" ) -func createFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName, content string) ([]*api.FileResponse, error) { +func createFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName, content string) (*api.FilesResponse, error) { opts := &files_service.ChangeRepoFilesOptions{ Files: []*files_service.ChangeRepoFile{ { @@ -27,6 +27,6 @@ func createFileInBranch(user *user_model.User, repo *repo_model.Repository, tree return files_service.ChangeRepoFiles(git.DefaultContext, repo, user, opts) } -func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) ([]*api.FileResponse, error) { +func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) (*api.FilesResponse, error) { return createFileInBranch(user, repo, treePath, repo.DefaultBranch, "This is a NEW file") } diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go index 279682f07c0a..38187ec5b9b4 100644 --- a/tests/integration/api_repo_files_change_test.go +++ b/tests/integration/api_repo_files_change_test.go @@ -102,27 +102,25 @@ func TestAPIChangeFiles(t *testing.T) { updateLastCommit, _ := gitRepo.GetCommitByPath(updateTreePath) expectedCreateFileResponse := getExpectedFileResponseForCreate(fmt.Sprintf("%v/%v", user2.Name, repo1.Name), commitID, createTreePath, createLasCommit.ID.String()) expectedUpdateFileResponse := getExpectedFileResponseForUpdate(commitID, updateTreePath, updateLastCommit.ID.String()) - var fileResponse []api.FileResponse - DecodeJSON(t, resp, &fileResponse) + var filesResponse api.FilesResponse + DecodeJSON(t, resp, &filesResponse) - // test create file - assert.EqualValues(t, expectedCreateFileResponse.Content, fileResponse[0].Content) - assert.EqualValues(t, expectedCreateFileResponse.Commit.SHA, fileResponse[0].Commit.SHA) - assert.EqualValues(t, expectedCreateFileResponse.Commit.HTMLURL, fileResponse[0].Commit.HTMLURL) - assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Email, fileResponse[0].Commit.Author.Email) - assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Name, fileResponse[0].Commit.Author.Name) - assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Email, fileResponse[0].Commit.Committer.Email) - assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Name, fileResponse[0].Commit.Committer.Name) - // test update file - assert.EqualValues(t, expectedUpdateFileResponse.Content, fileResponse[1].Content) - assert.EqualValues(t, expectedUpdateFileResponse.Commit.SHA, fileResponse[1].Commit.SHA) - assert.EqualValues(t, expectedUpdateFileResponse.Commit.HTMLURL, fileResponse[1].Commit.HTMLURL) - assert.EqualValues(t, expectedUpdateFileResponse.Commit.Committer.Email, fileResponse[1].Commit.Author.Email) - assert.EqualValues(t, expectedUpdateFileResponse.Commit.Committer.Name, fileResponse[1].Commit.Author.Name) + // check create file + assert.EqualValues(t, expectedCreateFileResponse.Content, filesResponse.Files[0]) + + // check update file + assert.EqualValues(t, expectedUpdateFileResponse.Content, filesResponse.Files[1]) + + // test commit info + assert.EqualValues(t, expectedCreateFileResponse.Commit.SHA, filesResponse.Commit.SHA) + assert.EqualValues(t, expectedCreateFileResponse.Commit.HTMLURL, filesResponse.Commit.HTMLURL) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Email, filesResponse.Commit.Author.Email) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Author.Name, filesResponse.Commit.Author.Name) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Email, filesResponse.Commit.Committer.Email) + assert.EqualValues(t, expectedCreateFileResponse.Commit.Committer.Name, filesResponse.Commit.Committer.Name) // test delete file - assert.NotNil(t, fileResponse[2]) - assert.Nil(t, fileResponse[2].Content) + assert.Nil(t, filesResponse.Files[2]) gitRepo.Close() } @@ -143,25 +141,23 @@ func TestAPIChangeFiles(t *testing.T) { url := fmt.Sprintf("/api/v1/repos/%s/%s/contents?token=%s", user2.Name, repo1.Name, token2) req := NewRequestWithJSON(t, "POST", url, &changeFilesOptions) resp := MakeRequest(t, req, http.StatusCreated) - var fileResponse []api.FileResponse - DecodeJSON(t, resp, &fileResponse) + var filesResponse api.FilesResponse + DecodeJSON(t, resp, &filesResponse) expectedCreateSHA := "a635aa942442ddfdba07468cf9661c08fbdf0ebf" expectedCreateHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/new_branch/new/file%d.txt", fileID) expectedCreateDownloadURL := fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/new_branch/new/file%d.txt", fileID) expectedUpdateSHA := "08bd14b2e2852529157324de9c226b3364e76136" expectedUpdateHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/new_branch/update/file%d.txt", fileID) expectedUpdateDownloadURL := fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/new_branch/update/file%d.txt", fileID) - assert.EqualValues(t, expectedCreateSHA, fileResponse[0].Content.SHA) - assert.EqualValues(t, expectedCreateHTMLURL, *fileResponse[0].Content.HTMLURL) - assert.EqualValues(t, expectedCreateDownloadURL, *fileResponse[0].Content.DownloadURL) - assert.EqualValues(t, changeFilesOptions.Message+"\n", fileResponse[0].Commit.Message) - assert.EqualValues(t, expectedUpdateSHA, fileResponse[1].Content.SHA) - assert.EqualValues(t, expectedUpdateHTMLURL, *fileResponse[1].Content.HTMLURL) - assert.EqualValues(t, expectedUpdateDownloadURL, *fileResponse[1].Content.DownloadURL) - assert.EqualValues(t, changeFilesOptions.Message+"\n", fileResponse[1].Commit.Message) - assert.NotNil(t, fileResponse[2]) - assert.Nil(t, fileResponse[2].Content) - assert.EqualValues(t, changeFilesOptions.Message+"\n", fileResponse[2].Commit.Message) + assert.EqualValues(t, expectedCreateSHA, filesResponse.Files[0].SHA) + assert.EqualValues(t, expectedCreateHTMLURL, *filesResponse.Files[0].HTMLURL) + assert.EqualValues(t, expectedCreateDownloadURL, *filesResponse.Files[0].DownloadURL) + assert.EqualValues(t, expectedUpdateSHA, filesResponse.Files[1].SHA) + assert.EqualValues(t, expectedUpdateHTMLURL, *filesResponse.Files[1].HTMLURL) + assert.EqualValues(t, expectedUpdateDownloadURL, *filesResponse.Files[1].DownloadURL) + assert.Nil(t, filesResponse.Files[2]) + + assert.EqualValues(t, changeFilesOptions.Message+"\n", filesResponse.Commit.Message) // Test updating a file and renaming it changeFilesOptions = getChangeFilesOptions() @@ -174,13 +170,13 @@ func TestAPIChangeFiles(t *testing.T) { changeFilesOptions.Files[0].Path = "rename/" + updateTreePath req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) resp = MakeRequest(t, req, http.StatusCreated) - DecodeJSON(t, resp, &fileResponse) + DecodeJSON(t, resp, &filesResponse) expectedUpdateSHA = "08bd14b2e2852529157324de9c226b3364e76136" expectedUpdateHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/master/rename/update/file%d.txt", fileID) expectedUpdateDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/master/rename/update/file%d.txt", fileID) - assert.EqualValues(t, expectedUpdateSHA, fileResponse[0].Content.SHA) - assert.EqualValues(t, expectedUpdateHTMLURL, *fileResponse[0].Content.HTMLURL) - assert.EqualValues(t, expectedUpdateDownloadURL, *fileResponse[0].Content.DownloadURL) + assert.EqualValues(t, expectedUpdateSHA, filesResponse.Files[0].SHA) + assert.EqualValues(t, expectedUpdateHTMLURL, *filesResponse.Files[0].HTMLURL) + assert.EqualValues(t, expectedUpdateDownloadURL, *filesResponse.Files[0].DownloadURL) // Test updating a file without a message changeFilesOptions = getChangeFilesOptions() @@ -197,11 +193,9 @@ func TestAPIChangeFiles(t *testing.T) { createFile(user2, repo1, deleteTreePath) req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions) resp = MakeRequest(t, req, http.StatusCreated) - DecodeJSON(t, resp, &fileResponse) + DecodeJSON(t, resp, &filesResponse) expectedMessage := fmt.Sprintf("Add %v\nUpdate %v\nDelete %v\n", createTreePath, updateTreePath, deleteTreePath) - for _, response := range fileResponse { - assert.EqualValues(t, expectedMessage, response.Commit.Message) - } + assert.EqualValues(t, expectedMessage, filesResponse.Commit.Message) // Test updating a file with the wrong SHA fileID++ diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index 222d3aeb4ea4..26c1e514e852 100644 --- a/tests/integration/repofiles_change_test.go +++ b/tests/integration/repofiles_change_test.go @@ -269,11 +269,11 @@ func TestChangeRepoFilesForCreate(t *testing.T) { expectedFileResponse := getExpectedFileResponseForRepofilesCreate(commitID, lastCommit.ID.String()) assert.NotNil(t, expectedFileResponse) if expectedFileResponse != nil { - assert.EqualValues(t, expectedFileResponse.Content, filesResponse[0].Content) - assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse[0].Commit.SHA) - assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse[0].Commit.HTMLURL) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, filesResponse[0].Commit.Author.Email) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, filesResponse[0].Commit.Author.Name) + assert.EqualValues(t, expectedFileResponse.Content, filesResponse.Files[0]) + assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse.Commit.SHA) + assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse.Commit.HTMLURL) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, filesResponse.Commit.Author.Email) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, filesResponse.Commit.Author.Name) } }) } @@ -304,11 +304,11 @@ func TestChangeRepoFilesForUpdate(t *testing.T) { commit, _ := gitRepo.GetBranchCommit(opts.NewBranch) lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String()) - assert.EqualValues(t, expectedFileResponse.Content, filesResponse[0].Content) - assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse[0].Commit.SHA) - assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse[0].Commit.HTMLURL) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, filesResponse[0].Commit.Author.Email) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, filesResponse[0].Commit.Author.Name) + assert.EqualValues(t, expectedFileResponse.Content, filesResponse.Files[0]) + assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse.Commit.SHA) + assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse.Commit.HTMLURL) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, filesResponse.Commit.Author.Email) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, filesResponse.Commit.Author.Name) }) } @@ -353,12 +353,12 @@ func TestChangeRepoFilesForUpdateWithFileMove(t *testing.T) { assert.Nil(t, fromEntry) // Should no longer exist here assert.NotNil(t, toEntry) // Should exist here // assert SHA has remained the same but paths use the new file name - assert.EqualValues(t, expectedFileResponse.Content.SHA, filesResponse[0].Content.SHA) - assert.EqualValues(t, expectedFileResponse.Content.Name, filesResponse[0].Content.Name) - assert.EqualValues(t, expectedFileResponse.Content.Path, filesResponse[0].Content.Path) - assert.EqualValues(t, expectedFileResponse.Content.URL, filesResponse[0].Content.URL) - assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse[0].Commit.SHA) - assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse[0].Commit.HTMLURL) + assert.EqualValues(t, expectedFileResponse.Content.SHA, filesResponse.Files[0].SHA) + assert.EqualValues(t, expectedFileResponse.Content.Name, filesResponse.Files[0].Name) + assert.EqualValues(t, expectedFileResponse.Content.Path, filesResponse.Files[0].Path) + assert.EqualValues(t, expectedFileResponse.Content.URL, filesResponse.Files[0].URL) + assert.EqualValues(t, expectedFileResponse.Commit.SHA, filesResponse.Commit.SHA) + assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, filesResponse.Commit.HTMLURL) }) } @@ -391,7 +391,7 @@ func TestChangeRepoFilesWithoutBranchNames(t *testing.T) { commit, _ := gitRepo.GetBranchCommit(repo.DefaultBranch) lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String()) - assert.EqualValues(t, expectedFileResponse.Content, filesResponse[0].Content) + assert.EqualValues(t, expectedFileResponse.Content, filesResponse.Files[0]) }) } @@ -418,11 +418,11 @@ func testDeleteRepoFiles(t *testing.T, u *url.URL) { assert.NoError(t, err) expectedFileResponse := getExpectedFileResponseForRepofilesDelete(u) assert.NotNil(t, filesResponse) - assert.Nil(t, filesResponse[0].Content) - assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) - assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) - assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) + assert.Nil(t, filesResponse.Files[0].Content) + assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse.Commit.Message) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse.Commit.Author.Identity) + assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse.Commit.Committer.Identity) + assert.EqualValues(t, expectedFileResponse.Verification, filesResponse.Verification) }) t.Run("Verify README.md has been deleted", func(t *testing.T) { @@ -460,11 +460,11 @@ func testDeleteRepoFilesWithoutBranchNames(t *testing.T, u *url.URL) { assert.NoError(t, err) expectedFileResponse := getExpectedFileResponseForRepofilesDelete(u) assert.NotNil(t, filesResponse) - assert.Nil(t, filesResponse[0].Content) - assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse[0].Commit.Message) - assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse[0].Commit.Author.Identity) - assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse[0].Commit.Committer.Identity) - assert.EqualValues(t, expectedFileResponse.Verification, filesResponse[0].Verification) + assert.Nil(t, filesResponse.Files[0]) + assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse.Commit.Message) + assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse.Commit.Author.Identity) + assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse.Commit.Committer.Identity) + assert.EqualValues(t, expectedFileResponse.Verification, filesResponse.Verification) }) } From d552c3d2215fcff61ca74b04839b453cd79329c6 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 26 May 2023 21:32:16 +0200 Subject: [PATCH 08/11] fix delete test --- tests/integration/repofiles_change_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index 26c1e514e852..a257b95a84df 100644 --- a/tests/integration/repofiles_change_test.go +++ b/tests/integration/repofiles_change_test.go @@ -418,7 +418,7 @@ func testDeleteRepoFiles(t *testing.T, u *url.URL) { assert.NoError(t, err) expectedFileResponse := getExpectedFileResponseForRepofilesDelete(u) assert.NotNil(t, filesResponse) - assert.Nil(t, filesResponse.Files[0].Content) + assert.Nil(t, filesResponse.Files[0]) assert.EqualValues(t, expectedFileResponse.Commit.Message, filesResponse.Commit.Message) assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, filesResponse.Commit.Author.Identity) assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, filesResponse.Commit.Committer.Identity) From c2840ec55db541815b19781457b1507499e9a250 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sat, 27 May 2023 07:37:39 +0200 Subject: [PATCH 09/11] extract response conversion to function --- routers/api/v1/repo/file.go | 30 +++--------------------------- services/repository/files/file.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 998001122f90..ddd529e0cd40 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -583,15 +583,7 @@ func CreateFile(ctx *context.APIContext) { if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - content := &api.ContentsResponse{} - if len(filesResponse.Files) > 0 { - content = filesResponse.Files[0] - } - fileResponse := &api.FileResponse{ - Content: content, - Commit: filesResponse.Commit, - Verification: filesResponse.Verification, - } + fileResponse := files_service.FilesResponseToSingle(filesResponse, 0) ctx.JSON(http.StatusCreated, fileResponse) } } @@ -685,15 +677,7 @@ func UpdateFile(ctx *context.APIContext) { if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - content := &api.ContentsResponse{} - if len(filesResponse.Files) > 0 { - content = filesResponse.Files[0] - } - fileResponse := &api.FileResponse{ - Content: content, - Commit: filesResponse.Commit, - Verification: filesResponse.Verification, - } + fileResponse := files_service.FilesResponseToSingle(filesResponse, 0) ctx.JSON(http.StatusOK, fileResponse) } } @@ -872,15 +856,7 @@ func DeleteFile(ctx *context.APIContext) { } ctx.Error(http.StatusInternalServerError, "DeleteFile", err) } else { - content := &api.ContentsResponse{} - if len(filesResponse.Files) > 0 { - content = filesResponse.Files[0] - } - fileResponse := &api.FileResponse{ - Content: content, - Commit: filesResponse.Commit, - Verification: filesResponse.Verification, - } + fileResponse := files_service.FilesResponseToSingle(filesResponse, 0) ctx.JSON(http.StatusOK, fileResponse) // FIXME on APIv2: return http.StatusNoContent } } diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 17b1646bb646..0be81cb00dce 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -46,6 +46,20 @@ func GetFileResponseFromCommit(ctx context.Context, repo *repo_model.Repository, return fileResponse, nil } +// constructs a FileResponse with the file at the index from FilesResponse +func FilesResponseToSingle(filesResponse *api.FilesResponse, index int) *api.FileResponse { + content := &api.ContentsResponse{} + if len(filesResponse.Files) > index { + content = filesResponse.Files[index] + } + fileResponse := &api.FileResponse{ + Content: content, + Commit: filesResponse.Commit, + Verification: filesResponse.Verification, + } + return fileResponse +} + // GetFileCommitResponse Constructs a FileCommitResponse from a Commit object func GetFileCommitResponse(repo *repo_model.Repository, commit *git.Commit) (*api.FileCommitResponse, error) { if repo == nil { From 00ef9149e7d618522893154542a17588b1c9cfa8 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sat, 27 May 2023 07:58:10 +0200 Subject: [PATCH 10/11] rename function to avoid linter warning --- routers/api/v1/repo/file.go | 6 +++--- services/repository/files/file.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index ddd529e0cd40..ae0d31c2a6d7 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -583,7 +583,7 @@ func CreateFile(ctx *context.APIContext) { if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - fileResponse := files_service.FilesResponseToSingle(filesResponse, 0) + fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0) ctx.JSON(http.StatusCreated, fileResponse) } } @@ -677,7 +677,7 @@ func UpdateFile(ctx *context.APIContext) { if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { handleCreateOrUpdateFileError(ctx, err) } else { - fileResponse := files_service.FilesResponseToSingle(filesResponse, 0) + fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0) ctx.JSON(http.StatusOK, fileResponse) } } @@ -856,7 +856,7 @@ func DeleteFile(ctx *context.APIContext) { } ctx.Error(http.StatusInternalServerError, "DeleteFile", err) } else { - fileResponse := files_service.FilesResponseToSingle(filesResponse, 0) + fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0) ctx.JSON(http.StatusOK, fileResponse) // FIXME on APIv2: return http.StatusNoContent } } diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 0be81cb00dce..16783f5b5f8a 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -47,7 +47,7 @@ func GetFileResponseFromCommit(ctx context.Context, repo *repo_model.Repository, } // constructs a FileResponse with the file at the index from FilesResponse -func FilesResponseToSingle(filesResponse *api.FilesResponse, index int) *api.FileResponse { +func GetFileResponseFromFilesResponse(filesResponse *api.FilesResponse, index int) *api.FileResponse { content := &api.ContentsResponse{} if len(filesResponse.Files) > index { content = filesResponse.Files[index] From 9da07c9fc256db1b4162c31762f9807b886f1213 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Sat, 27 May 2023 10:08:01 +0200 Subject: [PATCH 11/11] add operation description in swagger, readd error message for invalid operation --- modules/structs/repo_file.go | 3 ++- services/repository/files/update.go | 2 ++ templates/swagger/v1_json.tmpl | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 4313f93469c3..6ca0e1c10190 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -66,6 +66,7 @@ func (o *UpdateFileOptions) Branch() string { // ChangeFileOperation for creating, updating or deleting a file type ChangeFileOperation struct { + // indicates what to do with the file // required: true // enum: create,update,delete Operation string `json:"operation" binding:"Required"` @@ -74,7 +75,7 @@ type ChangeFileOperation struct { // content must be base64 encoded // required: true Content string `json:"content"` - // sha is the SHA for the file that already exists + // sha is the SHA for the file that already exists, required for update, delete SHA string `json:"sha"` // old path of the file to move FromPath string `json:"from_path"` diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 2d04eab40f06..81d5e32773a6 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -301,6 +301,8 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use if err := t.RemoveFilesFromIndex(file.TreePath); err != nil { return nil, err } + default: + return nil, fmt.Errorf("Invalid file operation: %s %s, supported operations are create, update, delete", file.Operation, file.Options.treePath) } } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index b65f08bc6686..75492ab63161 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -15961,6 +15961,7 @@ "x-go-name": "FromPath" }, "operation": { + "description": "indicates what to do with the file", "type": "string", "enum": [ "create", @@ -15975,7 +15976,7 @@ "x-go-name": "Path" }, "sha": { - "description": "sha is the SHA for the file that already exists", + "description": "sha is the SHA for the file that already exists, required for update, delete", "type": "string", "x-go-name": "SHA" }