From ff565a787f5cf1193bb60560688cabe6d6d33ef4 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Fri, 29 Dec 2023 16:47:02 +0800
Subject: [PATCH] Remove unnecessary syncbranchToDB with tests (#28624)
 (#28629)

Replace #28625

Backport #28624 by lunny

#28361 introduced `syncBranchToDB` in `CreateNewBranchFromCommit`. This
PR will revert the change because it's unnecessary. Every push will
already be checked by `syncBranchToDB`.
This PR also created a test to ensure it's right.
---
 services/repository/branch.go        | 29 +++++++---------------
 tests/integration/api_branch_test.go | 36 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/services/repository/branch.go b/services/repository/branch.go
index d56a0660c6..d8b3b81da8 100644
--- a/services/repository/branch.go
+++ b/services/repository/branch.go
@@ -281,28 +281,17 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo
 		return err
 	}
 
-	return db.WithTx(ctx, func(ctx context.Context) error {
-		commit, err := gitRepo.GetCommit(commitID)
-		if err != nil {
+	if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
+		Remote: repo.RepoPath(),
+		Branch: fmt.Sprintf("%s:%s%s", commitID, git.BranchPrefix, branchName),
+		Env:    repo_module.PushingEnvironment(doer, repo),
+	}); err != nil {
+		if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
 			return err
 		}
-		// database operation should be done before git operation so that we can rollback if git operation failed
-		if err := syncBranchToDB(ctx, repo.ID, doer.ID, branchName, commit); err != nil {
-			return err
-		}
-
-		if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
-			Remote: repo.RepoPath(),
-			Branch: fmt.Sprintf("%s:%s%s", commitID, git.BranchPrefix, branchName),
-			Env:    repo_module.PushingEnvironment(doer, repo),
-		}); err != nil {
-			if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
-				return err
-			}
-			return fmt.Errorf("push: %w", err)
-		}
-		return nil
-	})
+		return fmt.Errorf("push: %w", err)
+	}
+	return nil
 }
 
 // RenameBranch rename a branch
diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go
index bc026c117f..d90d3422dd 100644
--- a/tests/integration/api_branch_test.go
+++ b/tests/integration/api_branch_test.go
@@ -9,6 +9,8 @@ import (
 	"testing"
 
 	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/db"
+	git_model "code.gitea.io/gitea/models/git"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/tests"
 
@@ -212,3 +214,37 @@ func TestAPIBranchProtection(t *testing.T) {
 	testAPIDeleteBranch(t, "master", http.StatusForbidden)
 	testAPIDeleteBranch(t, "branch2", http.StatusNoContent)
 }
+
+func TestAPICreateBranchWithSyncBranches(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	branches, err := git_model.FindBranches(db.DefaultContext, git_model.FindBranchOptions{
+		RepoID: 1,
+	})
+	assert.NoError(t, err)
+	assert.Len(t, branches, 4)
+
+	// make a broke repository with no branch on database
+	_, err = db.DeleteByBean(db.DefaultContext, git_model.Branch{RepoID: 1})
+	assert.NoError(t, err)
+
+	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
+		giteaURL.Path = ctx.GitPath()
+
+		testAPICreateBranch(t, ctx.Session, "user2", "repo1", "", "new_branch", http.StatusCreated)
+	})
+
+	branches, err = git_model.FindBranches(db.DefaultContext, git_model.FindBranchOptions{
+		RepoID: 1,
+	})
+	assert.NoError(t, err)
+	assert.Len(t, branches, 5)
+
+	branches, err = git_model.FindBranches(db.DefaultContext, git_model.FindBranchOptions{
+		RepoID:  1,
+		Keyword: "new_branch",
+	})
+	assert.NoError(t, err)
+	assert.Len(t, branches, 1)
+}