Skip to content

Commit 921df1c

Browse files
authored
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.
1 parent 4cd666d commit 921df1c

File tree

2 files changed

+45
-20
lines changed

2 files changed

+45
-20
lines changed

services/repository/branch.go

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -276,28 +276,17 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo
276276
return err
277277
}
278278

279-
return db.WithTx(ctx, func(ctx context.Context) error {
280-
commit, err := gitRepo.GetCommit(commitID)
281-
if err != nil {
282-
return err
283-
}
284-
// database operation should be done before git operation so that we can rollback if git operation failed
285-
if err := syncBranchToDB(ctx, repo.ID, doer.ID, branchName, commit); err != nil {
279+
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
280+
Remote: repo.RepoPath(),
281+
Branch: fmt.Sprintf("%s:%s%s", commitID, git.BranchPrefix, branchName),
282+
Env: repo_module.PushingEnvironment(doer, repo),
283+
}); err != nil {
284+
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
286285
return err
287286
}
288-
289-
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
290-
Remote: repo.RepoPath(),
291-
Branch: fmt.Sprintf("%s:%s%s", commitID, git.BranchPrefix, branchName),
292-
Env: repo_module.PushingEnvironment(doer, repo),
293-
}); err != nil {
294-
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
295-
return err
296-
}
297-
return fmt.Errorf("push: %w", err)
298-
}
299-
return nil
300-
})
287+
return fmt.Errorf("push: %w", err)
288+
}
289+
return nil
301290
}
302291

303292
// RenameBranch rename a branch

tests/integration/api_branch_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"testing"
1010

1111
auth_model "code.gitea.io/gitea/models/auth"
12+
"code.gitea.io/gitea/models/db"
13+
git_model "code.gitea.io/gitea/models/git"
1214
api "code.gitea.io/gitea/modules/structs"
1315
"code.gitea.io/gitea/tests"
1416

@@ -217,3 +219,37 @@ func TestAPIBranchProtection(t *testing.T) {
217219
testAPIDeleteBranch(t, "master", http.StatusForbidden)
218220
testAPIDeleteBranch(t, "branch2", http.StatusNoContent)
219221
}
222+
223+
func TestAPICreateBranchWithSyncBranches(t *testing.T) {
224+
defer tests.PrepareTestEnv(t)()
225+
226+
branches, err := db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{
227+
RepoID: 1,
228+
})
229+
assert.NoError(t, err)
230+
assert.Len(t, branches, 4)
231+
232+
// make a broke repository with no branch on database
233+
_, err = db.DeleteByBean(db.DefaultContext, git_model.Branch{RepoID: 1})
234+
assert.NoError(t, err)
235+
236+
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
237+
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
238+
giteaURL.Path = ctx.GitPath()
239+
240+
testAPICreateBranch(t, ctx.Session, "user2", "repo1", "", "new_branch", http.StatusCreated)
241+
})
242+
243+
branches, err = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{
244+
RepoID: 1,
245+
})
246+
assert.NoError(t, err)
247+
assert.Len(t, branches, 5)
248+
249+
branches, err = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{
250+
RepoID: 1,
251+
Keyword: "new_branch",
252+
})
253+
assert.NoError(t, err)
254+
assert.Len(t, branches, 1)
255+
}

0 commit comments

Comments
 (0)