Reimplement as PATCH /branches/* endpoint

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

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 {
+type UpdateBranchRepoOption struct {
-	// Old branch name
-	//
-	// required: true
-	// unique: true
-	OldName string `json:"old_name" binding:"Required;GitRefName;MaxSize(100)"`
 	// 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

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)

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.
+// UpdateBranch renames a repository's branch.
-func RenameBranch(ctx *context.APIContext) {
+func UpdateBranch(ctx *context.APIContext) {
-	// swagger:operation POST /repos/{owner}/{repo}/branches/rename repository repoRenameBranch
+	// 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)
+	branchName := opt.Name
-	if err != nil {
+	if branchName != "" {
-		ctx.Error(http.StatusInternalServerError, "RenameBranch", err)
+		msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, oldName, branchName)
-		return
+		if err != nil {
-	}
+			ctx.Error(http.StatusInternalServerError, "RenameBranch", err)
-	if msg != "" {
+			return
-		ctx.Error(http.StatusUnprocessableEntity, "", msg)
+		}
-		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

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

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",

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) {
+		t.Run("UpdateBranchWithEmptyRepo", func(t *testing.T) {
-			testAPIRenameBranch(t, "user10", "repo6", "master", "test", http.StatusNotFound)
+			testAPIUpdateBranch(t, "user10", "repo6", "master", "test", http.StatusNotFound)
 		})
-		t.Run("RenameBranchWithSameBranchNames", func(t *testing.T) {
+		t.Run("UpdateBranchWithSameBranchNames", func(t *testing.T) {
-			testAPIRenameBranch(t, "user2", "repo1", "master", "master", http.StatusUnprocessableEntity)
+			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) {
+		t.Run("UpdateBranchThatAlreadyExists", func(t *testing.T) {
-			testAPIRenameBranch(t, "user2", "repo1", "master", "branch2", http.StatusUnprocessableEntity)
+			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) {
+		t.Run("UpdateBranchWithNonExistentBranch", func(t *testing.T) {
-			testAPIRenameBranch(t, "user2", "repo1", "i-dont-exist", "branch2", http.StatusUnprocessableEntity)
+			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{
+	req := NewRequestWithJSON(t, "PATCH", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.UpdateBranchRepoOption{
-		OldName: from,
+		Name: to,
-		NewName: to,
 	}).AddTokenAuth(token)
-	MakeRequest(t, req, expectedHTTPStatus)
+	return MakeRequest(t, req, expectedHTTPStatus)
 }
 
 func TestAPIBranchProtection(t *testing.T) {