From 921df1cbad83dcba37ff12cba88c7d3a69f8588b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 28 Dec 2023 15:28:57 +0800 Subject: [PATCH] Remove unnecessary syncbranchToDB with tests (#28624) #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 dca938444..725477876 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -276,28 +276,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 28e690b35..103f8f707 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" @@ -217,3 +219,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 := db.Find[git_model.Branch](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 = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{ + RepoID: 1, + }) + assert.NoError(t, err) + assert.Len(t, branches, 5) + + branches, err = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{ + RepoID: 1, + Keyword: "new_branch", + }) + assert.NoError(t, err) + assert.Len(t, branches, 1) +}