From 4445c70698a5a29d5bc25f5f5de9d1f20b8e4b65 Mon Sep 17 00:00:00 2001
From: Kemal Zebari <kemalzebra@gmail.com>
Date: Mon, 18 Nov 2024 22:07:59 -0800
Subject: [PATCH] Make the `name` PATCH field required

---
 modules/structs/repo.go              |  3 ++-
 routers/api/v1/repo/branch.go        | 39 ++++++++++------------------
 templates/swagger/v1_json.tmpl       |  3 +++
 tests/integration/api_branch_test.go | 18 -------------
 4 files changed, 19 insertions(+), 44 deletions(-)

diff --git a/modules/structs/repo.go b/modules/structs/repo.go
index 239b1e22d8..fb784bd8b3 100644
--- a/modules/structs/repo.go
+++ b/modules/structs/repo.go
@@ -283,8 +283,9 @@ type CreateBranchRepoOption struct {
 type UpdateBranchRepoOption struct {
 	// New branch name
 	//
+	// required: true
 	// unique: true
-	Name string `json:"name" binding:"GitRefName;MaxSize(100)"`
+	Name string `json:"name" binding:"Required;GitRefName;MaxSize(100)"`
 }
 
 // TransferRepoOption options when transfer a repository's ownership
diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index 62cebaec62..5967d0bbfc 100644
--- a/routers/api/v1/repo/branch.go
+++ b/routers/api/v1/repo/branch.go
@@ -450,33 +450,22 @@ func UpdateBranch(ctx *context.APIContext) {
 		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.StatusNotFound, "", "Branch doesn't exist.")
-			return
-		}
-	} else {
-		branchName = oldName
+	msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, oldName, opt.Name)
+	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.StatusNotFound, "", "Branch doesn't exist.")
+		return
 	}
 
-	branch, err := ctx.Repo.GitRepo.GetBranch(branchName)
+	branch, err := ctx.Repo.GitRepo.GetBranch(opt.Name)
 	if err != nil {
-		if git.IsErrBranchNotExist(err) {
-			// This could occur if the client passes a non-existent branch and we
-			// skip executing the branch that contains the RenameBranch() call.
-			ctx.Error(http.StatusNotFound, "", "Branch doesn't exist.")
-			return
-		}
 		ctx.Error(http.StatusInternalServerError, "GetBranch", err)
 		return
 	}
@@ -495,7 +484,7 @@ func UpdateBranch(ctx *context.APIContext) {
 
 	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)
+		ctx.Error(http.StatusInternalServerError, "ToBranch", err)
 		return
 	}
 
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index b8006258c4..2961348b92 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -24946,6 +24946,9 @@
     "UpdateBranchRepoOption": {
       "description": "UpdateBranchRepoOption options when updating a branch in a repository",
       "type": "object",
+      "required": [
+        "name"
+      ],
       "properties": {
         "name": {
           "description": "New branch name",
diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go
index a2b71dcf24..18e9019d39 100644
--- a/tests/integration/api_branch_test.go
+++ b/tests/integration/api_branch_test.go
@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"slices"
 	"testing"
 
 	auth_model "code.gitea.io/gitea/models/auth"
@@ -204,23 +203,6 @@ func TestAPIUpdateBranch(t *testing.T) {
 			resp := testAPIUpdateBranch(t, "user2", "repo1", "i-dont-exist", "new-branch-name", http.StatusNotFound)
 			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("UpdateBranchWithNonExistentBranchAndNewNameIsTheEmptyString", func(t *testing.T) {
-			testAPIUpdateBranch(t, "user2", "repo1", "i-dont-exist", "", http.StatusNotFound)
-		})
 		t.Run("RenameBranchNormalScenario", func(t *testing.T) {
 			resp := testAPIUpdateBranch(t, "user2", "repo1", "branch2", "new-branch-name", http.StatusOK)
 			var branch api.Branch