diff --git a/modules/structs/repo.go b/modules/structs/repo.go index c06044e95e..239b1e22d8 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -278,19 +278,13 @@ type CreateBranchRepoOption struct { OldRefName string `json:"old_ref_name" binding:"GitRefName;MaxSize(100)"` } -// RenameBranchOption options when rename a branch in a repository +// UpdateBranchRepoOption options when updating a branch in a repository // swagger:model -type RenameBranchRepoOption struct { - // Old branch name - // - // required: true - // unique: true - OldName string `json:"old_name" binding:"Required;GitRefName;MaxSize(100)"` +type UpdateBranchRepoOption struct { // New branch name // - // required: true // unique: true - NewName string `json:"new_name" binding:"Required;GitRefName;MaxSize(100)"` + Name string `json:"name" binding:"GitRefName;MaxSize(100)"` } // TransferRepoOption options when transfer a repository's ownership diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 5ed01493d8..986c4cf992 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1195,7 +1195,7 @@ func Routes() *web.Router { m.Get("/*", repo.GetBranch) m.Delete("/*", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, repo.DeleteBranch) m.Post("", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.CreateBranchRepoOption{}), repo.CreateBranch) - m.Post("/rename", tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository), reqRepoWriter(unit.TypeCode), bind(api.RenameBranchRepoOption{}), repo.RenameBranch) + m.Patch("/*", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.UpdateBranchRepoOption{}), repo.UpdateBranch) }, context.ReferencesGitRepo(), reqRepoReader(unit.TypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 5dcc5e08ae..1e51226074 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -396,11 +396,11 @@ func ListBranches(ctx *context.APIContext) { ctx.JSON(http.StatusOK, apiBranches) } -// RenameBranch renames a repository's branch. -func RenameBranch(ctx *context.APIContext) { - // swagger:operation POST /repos/{owner}/{repo}/branches/rename repository repoRenameBranch +// UpdateBranch renames a repository's branch. +func UpdateBranch(ctx *context.APIContext) { + // swagger:operation PATCH /repos/{owner}/{repo}/branches/{branch} repository repoUpdateBranch // --- - // summary: Rename a branch + // summary: Update a branch // consumes: // - application/json // produces: @@ -416,12 +416,16 @@ func RenameBranch(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: branch + // in: path + // description: name of the branch + // type: string // - name: body // in: body // schema: - // "$ref": "#/definitions/RenameBranchRepoOption" + // "$ref": "#/definitions/UpdateBranchRepoOption" // responses: - // "201": + // "200": // "$ref": "#/responses/Branch" // "403": // "$ref": "#/responses/forbidden" @@ -430,7 +434,9 @@ func RenameBranch(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" - opt := web.GetForm(ctx).(*api.RenameBranchRepoOption) + opt := web.GetForm(ctx).(*api.UpdateBranchRepoOption) + + oldName := ctx.PathParam("*") repo := ctx.Repo.Repository if repo.IsEmpty { @@ -443,17 +449,26 @@ func RenameBranch(ctx *context.APIContext) { return } - msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, opt.OldName, opt.NewName) - if err != nil { - ctx.Error(http.StatusInternalServerError, "RenameBranch", err) - return - } - if msg != "" { - ctx.Error(http.StatusUnprocessableEntity, "", msg) - return + branchName := opt.Name + if branchName != "" { + msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, oldName, branchName) + if err != nil { + ctx.Error(http.StatusInternalServerError, "RenameBranch", err) + return + } + if msg == "target_exist" { + ctx.Error(http.StatusUnprocessableEntity, "", "Cannot rename a branch using the same name or rename to a branch that already exists.") + return + } + if msg == "from_not_exist" { + ctx.Error(http.StatusUnprocessableEntity, "", "Branch doesn't exist.") + return + } + } else { + branchName = oldName } - branch, err := ctx.Repo.GitRepo.GetBranch(opt.NewName) + branch, err := ctx.Repo.GitRepo.GetBranch(branchName) if err != nil { ctx.Error(http.StatusInternalServerError, "GetBranch", err) return @@ -471,13 +486,13 @@ func RenameBranch(ctx *context.APIContext) { return } - br, err := convert.ToBranch(ctx, repo, opt.NewName, commit, pb, ctx.Doer, ctx.Repo.IsAdmin()) + br, err := convert.ToBranch(ctx, repo, branch.Name, commit, pb, ctx.Doer, ctx.Repo.IsAdmin()) if err != nil { ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err) return } - ctx.JSON(http.StatusCreated, br) + ctx.JSON(http.StatusOK, br) } // GetBranchProtection gets a branch protection diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index ad4bed3258..62ae808edd 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -90,7 +90,7 @@ type swaggerParameterBodies struct { // in:body EditRepoOption api.EditRepoOption // in:body - RenameBranchReopOption api.RenameBranchRepoOption + UpdateBranchRepoOption api.UpdateBranchRepoOption // in:body TransferRepoOption api.TransferRepoOption // in:body diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fd486c1ea6..0cd377b2e6 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4905,58 +4905,6 @@ } } }, - "/repos/{owner}/{repo}/branches/rename": { - "post": { - "consumes": [ - "application/json" - ], - "produces": [ - "application/json" - ], - "tags": [ - "repository" - ], - "summary": "Rename a branch", - "operationId": "repoRenameBranch", - "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", - "schema": { - "$ref": "#/definitions/RenameBranchRepoOption" - } - } - ], - "responses": { - "201": { - "$ref": "#/responses/Branch" - }, - "403": { - "$ref": "#/responses/forbidden" - }, - "404": { - "$ref": "#/responses/notFound" - }, - "422": { - "$ref": "#/responses/validationError" - } - } - } - }, "/repos/{owner}/{repo}/branches/{branch}": { "get": { "produces": [ @@ -5045,6 +4993,62 @@ "$ref": "#/responses/repoArchivedError" } } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Update a branch", + "operationId": "repoUpdateBranch", + "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 + }, + { + "type": "string", + "description": "name of the branch", + "name": "branch", + "in": "path" + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/UpdateBranchRepoOption" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/Branch" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } } }, "/repos/{owner}/{repo}/collaborators": { @@ -24057,29 +24061,6 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, - "RenameBranchRepoOption": { - "description": "RenameBranchOption options when rename a branch in a repository", - "type": "object", - "required": [ - "old_name", - "new_name" - ], - "properties": { - "new_name": { - "description": "New branch name", - "type": "string", - "uniqueItems": true, - "x-go-name": "NewName" - }, - "old_name": { - "description": "Old branch name", - "type": "string", - "uniqueItems": true, - "x-go-name": "OldName" - } - }, - "x-go-package": "code.gitea.io/gitea/modules/structs" - }, "RenameUserOption": { "description": "RenameUserOption options when renaming a user", "type": "object", @@ -24961,6 +24942,19 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "UpdateBranchRepoOption": { + "description": "UpdateBranchRepoOption options when updating a branch in a repository", + "type": "object", + "properties": { + "name": { + "description": "New branch name", + "type": "string", + "uniqueItems": true, + "x-go-name": "Name" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "UpdateFileOptions": { "description": "UpdateFileOptions options for updating 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", diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 12cf3150f1..80a9a66301 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -5,7 +5,9 @@ package integration import ( "net/http" + "net/http/httptest" "net/url" + "slices" "testing" auth_model "code.gitea.io/gitea/models/auth" @@ -185,33 +187,52 @@ func testAPICreateBranch(t testing.TB, session *TestSession, user, repo, oldBran return resp.Result().StatusCode == status } -func TestAPIRenameBranch(t *testing.T) { +func TestAPIUpdateBranch(t *testing.T) { onGiteaRun(t, func(t *testing.T, _ *url.URL) { - t.Run("RenameBranchWithEmptyRepo", func(t *testing.T) { - testAPIRenameBranch(t, "user10", "repo6", "master", "test", http.StatusNotFound) + t.Run("UpdateBranchWithEmptyRepo", func(t *testing.T) { + testAPIUpdateBranch(t, "user10", "repo6", "master", "test", http.StatusNotFound) }) - t.Run("RenameBranchWithSameBranchNames", func(t *testing.T) { - testAPIRenameBranch(t, "user2", "repo1", "master", "master", http.StatusUnprocessableEntity) + t.Run("UpdateBranchWithSameBranchNames", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user2", "repo1", "master", "master", http.StatusUnprocessableEntity) + assert.Contains(t, resp.Body.String(), "Cannot rename a branch using the same name or rename to a branch that already exists.") }) - t.Run("RenameBranchWithBranchThatAlreadyExists", func(t *testing.T) { - testAPIRenameBranch(t, "user2", "repo1", "master", "branch2", http.StatusUnprocessableEntity) + t.Run("UpdateBranchThatAlreadyExists", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user2", "repo1", "master", "branch2", http.StatusUnprocessableEntity) + assert.Contains(t, resp.Body.String(), "Cannot rename a branch using the same name or rename to a branch that already exists.") }) - t.Run("RenameBranchWithNonExistentBranch", func(t *testing.T) { - testAPIRenameBranch(t, "user2", "repo1", "i-dont-exist", "branch2", http.StatusUnprocessableEntity) + t.Run("UpdateBranchWithNonExistentBranch", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user2", "repo1", "i-dont-exist", "new-branch-name", http.StatusUnprocessableEntity) + assert.Contains(t, resp.Body.String(), "Branch doesn't exist.") + }) + t.Run("UpdateBranchWithEmptyStringAsNewName", func(t *testing.T) { + resp := testAPIUpdateBranch(t, "user13", "repo11", "master", "", http.StatusOK) + var branch api.Branch + DecodeJSON(t, resp, &branch) + assert.EqualValues(t, "master", branch.Name) + + // Make sure the branch name did not change in the db. + branches, err := db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{ + RepoID: 11, + }) + assert.NoError(t, err) + branchWasUnchanged := slices.ContainsFunc(branches, func(b *git_model.Branch) bool { return b.Name == "master" }) + assert.True(t, branchWasUnchanged, "master branch shouldn't have been renamed") }) t.Run("RenameBranchNormalScenario", func(t *testing.T) { - testAPIRenameBranch(t, "user2", "repo1", "branch2", "new-branch-name", http.StatusCreated) + resp := testAPIUpdateBranch(t, "user2", "repo1", "branch2", "new-branch-name", http.StatusOK) + var branch api.Branch + DecodeJSON(t, resp, &branch) + assert.EqualValues(t, "new-branch-name", branch.Name) }) }) } -func testAPIRenameBranch(t *testing.T, ownerName, repoName, from, to string, expectedHTTPStatus int) { +func testAPIUpdateBranch(t *testing.T, ownerName, repoName, from, to string, expectedHTTPStatus int) *httptest.ResponseRecorder { token := getUserToken(t, ownerName, auth_model.AccessTokenScopeWriteRepository) - req := NewRequestWithJSON(t, "POST", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/rename", &api.RenameBranchRepoOption{ - OldName: from, - NewName: to, + req := NewRequestWithJSON(t, "PATCH", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.UpdateBranchRepoOption{ + Name: to, }).AddTokenAuth(token) - MakeRequest(t, req, expectedHTTPStatus) + return MakeRequest(t, req, expectedHTTPStatus) } func TestAPIBranchProtection(t *testing.T) {