Skip to content

Commit

Permalink
Remove unnecessary syncbranchToDB with tests (go-gitea#28624)
Browse files Browse the repository at this point in the history
go-gitea#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.
  • Loading branch information
lunny authored and silverwind committed Feb 20, 2024
1 parent e9f4061 commit a3ead63
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 20 deletions.
29 changes: 9 additions & 20 deletions services/repository/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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 {
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
}

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
Expand Down
36 changes: 36 additions & 0 deletions tests/integration/api_branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}

0 comments on commit a3ead63

Please sign in to comment.