Skip to content

Commit aef1ebc

Browse files
lunnysilverwind
authored andcommitted
Also sync DB branches on push if necessary (go-gitea#28361)
Fix go-gitea#28056 This PR will check whether the repo has zero branch when pushing a branch. If that, it means this repository hasn't been synced. The reason caused that is after user upgrade from v1.20 -> v1.21, he just push branches without visit the repository user interface. Because all repositories routers will check whether a branches sync is necessary but push has not such check. For every repository, it has two states, synced or not synced. If there is zero branch for a repository, then it will be assumed as non-sync state. Otherwise, it's synced state. So if we think it's synced, we just need to update branch/insert new branch. Otherwise do a full sync. So that, for every push, there will be almost no extra load added. It's high performance than yours. For the implementation, we in fact will try to update the branch first, if updated success with affect records > 0, then all are done. Because that means the branch has been in the database. If no record is affected, that means the branch does not exist in database. So there are two possibilities. One is this is a new branch, then we just need to insert the record. Another is the branches haven't been synced, then we need to sync all the branches into database.
1 parent a053cff commit aef1ebc

File tree

9 files changed

+99
-73
lines changed

9 files changed

+99
-73
lines changed

models/git/branch.go

+14-19
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,9 @@ func DeleteBranches(ctx context.Context, repoID, doerID int64, branchIDs []int64
205205
})
206206
}
207207

208-
// UpdateBranch updates the branch information in the database. If the branch exist, it will update latest commit of this branch information
209-
// If it doest not exist, insert a new record into database
210-
func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) error {
211-
cnt, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repoID, branchName).
208+
// UpdateBranch updates the branch information in the database.
209+
func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) (int64, error) {
210+
return db.GetEngine(ctx).Where("repo_id=? AND name=?", repoID, branchName).
212211
Cols("commit_id, commit_message, pusher_id, commit_time, is_deleted, updated_unix").
213212
Update(&Branch{
214213
CommitID: commit.ID.String(),
@@ -217,21 +216,6 @@ func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string
217216
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
218217
IsDeleted: false,
219218
})
220-
if err != nil {
221-
return err
222-
}
223-
if cnt > 0 {
224-
return nil
225-
}
226-
227-
return db.Insert(ctx, &Branch{
228-
RepoID: repoID,
229-
Name: branchName,
230-
CommitID: commit.ID.String(),
231-
CommitMessage: commit.Summary(),
232-
PusherID: pusherID,
233-
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
234-
})
235219
}
236220

237221
// AddDeletedBranch adds a deleted branch to the database
@@ -308,6 +292,17 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
308292

309293
sess := db.GetEngine(ctx)
310294

295+
var branch Branch
296+
exist, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repo.ID, from).Get(&branch)
297+
if err != nil {
298+
return err
299+
} else if !exist || branch.IsDeleted {
300+
return ErrBranchNotExist{
301+
RepoID: repo.ID,
302+
BranchName: from,
303+
}
304+
}
305+
311306
// 1. update branch in database
312307
if n, err := sess.Where("repo_id=? AND name=?", repo.ID, from).Update(&Branch{
313308
Name: to,

models/git/branch_list.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ type FindBranchOptions struct {
7373
Keyword string
7474
}
7575

76-
func (opts *FindBranchOptions) Cond() builder.Cond {
76+
func (opts FindBranchOptions) ToConds() builder.Cond {
7777
cond := builder.NewCond()
7878
if opts.RepoID > 0 {
7979
cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
@@ -92,7 +92,7 @@ func (opts *FindBranchOptions) Cond() builder.Cond {
9292
}
9393

9494
func CountBranches(ctx context.Context, opts FindBranchOptions) (int64, error) {
95-
return db.GetEngine(ctx).Where(opts.Cond()).Count(&Branch{})
95+
return db.GetEngine(ctx).Where(opts.ToConds()).Count(&Branch{})
9696
}
9797

9898
func orderByBranches(sess *xorm.Session, opts FindBranchOptions) *xorm.Session {
@@ -108,7 +108,7 @@ func orderByBranches(sess *xorm.Session, opts FindBranchOptions) *xorm.Session {
108108
}
109109

110110
func FindBranches(ctx context.Context, opts FindBranchOptions) (BranchList, error) {
111-
sess := db.GetEngine(ctx).Where(opts.Cond())
111+
sess := db.GetEngine(ctx).Where(opts.ToConds())
112112
if opts.PageSize > 0 && !opts.IsListAll() {
113113
sess = db.SetSessionPagination(sess, &opts.ListOptions)
114114
}
@@ -119,7 +119,7 @@ func FindBranches(ctx context.Context, opts FindBranchOptions) (BranchList, erro
119119
}
120120

121121
func FindBranchNames(ctx context.Context, opts FindBranchOptions) ([]string, error) {
122-
sess := db.GetEngine(ctx).Select("name").Where(opts.Cond())
122+
sess := db.GetEngine(ctx).Select("name").Where(opts.ToConds())
123123
if opts.PageSize > 0 && !opts.IsListAll() {
124124
sess = db.SetSessionPagination(sess, &opts.ListOptions)
125125
}

models/git/branch_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestAddDeletedBranch(t *testing.T) {
3737
},
3838
}
3939

40-
err := git_model.UpdateBranch(db.DefaultContext, repo.ID, secondBranch.PusherID, secondBranch.Name, commit)
40+
_, err := git_model.UpdateBranch(db.DefaultContext, repo.ID, secondBranch.PusherID, secondBranch.Name, commit)
4141
assert.NoError(t, err)
4242
}
4343

routers/api/v1/repo/branch.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,11 @@ func CreateBranch(ctx *context.APIContext) {
251251
}
252252
}
253253

254-
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, oldCommit.ID.String(), opt.BranchName)
254+
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, oldCommit.ID.String(), opt.BranchName)
255255
if err != nil {
256256
if git_model.IsErrBranchNotExist(err) {
257257
ctx.Error(http.StatusNotFound, "", "The old branch does not exist")
258-
}
259-
if models.IsErrTagAlreadyExists(err) {
258+
} else if models.IsErrTagAlreadyExists(err) {
260259
ctx.Error(http.StatusConflict, "", "The branch with the same tag already exists.")
261260
} else if git_model.IsErrBranchAlreadyExists(err) || git.IsErrPushOutOfDate(err) {
262261
ctx.Error(http.StatusConflict, "", "The branch already exists.")

routers/web/repo/branch.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,9 @@ func CreateBranch(ctx *context.Context) {
191191
}
192192
err = release_service.CreateNewTag(ctx, ctx.Doer, ctx.Repo.Repository, target, form.NewBranchName, "")
193193
} else if ctx.Repo.IsViewBranch {
194-
err = repo_service.CreateNewBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName, form.NewBranchName)
194+
err = repo_service.CreateNewBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, ctx.Repo.BranchName, form.NewBranchName)
195195
} else {
196-
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.CommitID, form.NewBranchName)
196+
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, ctx.Repo.CommitID, form.NewBranchName)
197197
}
198198
if err != nil {
199199
if models.IsErrProtectedTagName(err) {

services/repository/branch.go

+65-35
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"code.gitea.io/gitea/modules/log"
2121
"code.gitea.io/gitea/modules/queue"
2222
repo_module "code.gitea.io/gitea/modules/repository"
23+
"code.gitea.io/gitea/modules/timeutil"
2324
"code.gitea.io/gitea/modules/util"
2425
notify_service "code.gitea.io/gitea/services/notify"
2526
files_service "code.gitea.io/gitea/services/repository/files"
@@ -28,35 +29,13 @@ import (
2829
)
2930

3031
// CreateNewBranch creates a new repository branch
31-
func CreateNewBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, oldBranchName, branchName string) (err error) {
32-
err = repo.MustNotBeArchived()
32+
func CreateNewBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, oldBranchName, branchName string) (err error) {
33+
branch, err := git_model.GetBranch(ctx, repo.ID, oldBranchName)
3334
if err != nil {
3435
return err
3536
}
3637

37-
// Check if branch name can be used
38-
if err := checkBranchName(ctx, repo, branchName); err != nil {
39-
return err
40-
}
41-
42-
if !git.IsBranchExist(ctx, repo.RepoPath(), oldBranchName) {
43-
return git_model.ErrBranchNotExist{
44-
BranchName: oldBranchName,
45-
}
46-
}
47-
48-
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
49-
Remote: repo.RepoPath(),
50-
Branch: fmt.Sprintf("%s%s:%s%s", git.BranchPrefix, oldBranchName, git.BranchPrefix, branchName),
51-
Env: repo_module.PushingEnvironment(doer, repo),
52-
}); err != nil {
53-
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
54-
return err
55-
}
56-
return fmt.Errorf("push: %w", err)
57-
}
58-
59-
return nil
38+
return CreateNewBranchFromCommit(ctx, doer, repo, gitRepo, branch.CommitID, branchName)
6039
}
6140

6241
// Branch contains the branch information
@@ -249,8 +228,49 @@ func checkBranchName(ctx context.Context, repo *repo_model.Repository, name stri
249228
return err
250229
}
251230

231+
// syncBranchToDB sync the branch information in the database. It will try to update the branch first,
232+
// if updated success with affect records > 0, then all are done. Because that means the branch has been in the database.
233+
// If no record is affected, that means the branch does not exist in database. So there are two possibilities.
234+
// One is this is a new branch, then we just need to insert the record. Another is the branches haven't been synced,
235+
// then we need to sync all the branches into database.
236+
func syncBranchToDB(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) error {
237+
cnt, err := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit)
238+
if err != nil {
239+
return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err)
240+
}
241+
if cnt > 0 { // This means branch does exist, so it's a normal update. It also means the branch has been synced.
242+
return nil
243+
}
244+
245+
// if user haven't visit UI but directly push to a branch after upgrading from 1.20 -> 1.21,
246+
// we cannot simply insert the branch but need to check we have branches or not
247+
hasBranch, err := db.Exist[git_model.Branch](ctx, git_model.FindBranchOptions{
248+
RepoID: repoID,
249+
IsDeletedBranch: util.OptionalBoolFalse,
250+
}.ToConds())
251+
if err != nil {
252+
return err
253+
}
254+
if !hasBranch {
255+
if _, err = repo_module.SyncRepoBranches(ctx, repoID, pusherID); err != nil {
256+
return fmt.Errorf("repo_module.SyncRepoBranches %d:%s failed: %v", repoID, branchName, err)
257+
}
258+
return nil
259+
}
260+
261+
// if database have branches but not this branch, it means this is a new branch
262+
return db.Insert(ctx, &git_model.Branch{
263+
RepoID: repoID,
264+
Name: branchName,
265+
CommitID: commit.ID.String(),
266+
CommitMessage: commit.Summary(),
267+
PusherID: pusherID,
268+
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
269+
})
270+
}
271+
252272
// CreateNewBranchFromCommit creates a new repository branch
253-
func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, commit, branchName string) (err error) {
273+
func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, commitID, branchName string) (err error) {
254274
err = repo.MustNotBeArchived()
255275
if err != nil {
256276
return err
@@ -261,18 +281,28 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo
261281
return err
262282
}
263283

264-
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
265-
Remote: repo.RepoPath(),
266-
Branch: fmt.Sprintf("%s:%s%s", commit, git.BranchPrefix, branchName),
267-
Env: repo_module.PushingEnvironment(doer, repo),
268-
}); err != nil {
269-
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
284+
return db.WithTx(ctx, func(ctx context.Context) error {
285+
commit, err := gitRepo.GetCommit(commitID)
286+
if err != nil {
287+
return err
288+
}
289+
// database operation should be done before git operation so that we can rollback if git operation failed
290+
if err := syncBranchToDB(ctx, repo.ID, doer.ID, branchName, commit); err != nil {
270291
return err
271292
}
272-
return fmt.Errorf("push: %w", err)
273-
}
274293

275-
return nil
294+
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
295+
Remote: repo.RepoPath(),
296+
Branch: fmt.Sprintf("%s:%s%s", commitID, git.BranchPrefix, branchName),
297+
Env: repo_module.PushingEnvironment(doer, repo),
298+
}); err != nil {
299+
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
300+
return err
301+
}
302+
return fmt.Errorf("push: %w", err)
303+
}
304+
return nil
305+
})
276306
}
277307

278308
// RenameBranch rename a branch

services/repository/push.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
259259
commits.Commits = commits.Commits[:setting.UI.FeedMaxCommitNum]
260260
}
261261

262-
if err = git_model.UpdateBranch(ctx, repo.ID, opts.PusherID, branch, newCommit); err != nil {
262+
if err = syncBranchToDB(ctx, repo.ID, opts.PusherID, branch, newCommit); err != nil {
263263
return fmt.Errorf("git_model.UpdateBranch %s:%s failed: %v", repo.FullName(), branch, err)
264264
}
265265

tests/integration/api_repo_get_contents_list_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,16 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
7070
session = loginUser(t, user4.Name)
7171
token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)
7272

73-
// Make a new branch in repo1
74-
newBranch := "test_branch"
75-
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
76-
assert.NoError(t, err)
7773
// Get the commit ID of the default branch
7874
gitRepo, err := git.OpenRepository(git.DefaultContext, repo1.RepoPath())
7975
assert.NoError(t, err)
8076
defer gitRepo.Close()
8177

78+
// Make a new branch in repo1
79+
newBranch := "test_branch"
80+
err = repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, gitRepo, repo1.DefaultBranch, newBranch)
81+
assert.NoError(t, err)
82+
8283
commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
8384
// Make a new tag in repo1
8485
newTag := "test_tag"

tests/integration/api_repo_get_contents_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,16 @@ func testAPIGetContents(t *testing.T, u *url.URL) {
7272
session = loginUser(t, user4.Name)
7373
token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)
7474

75-
// Make a new branch in repo1
76-
newBranch := "test_branch"
77-
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
78-
assert.NoError(t, err)
7975
// Get the commit ID of the default branch
8076
gitRepo, err := git.OpenRepository(git.DefaultContext, repo1.RepoPath())
8177
assert.NoError(t, err)
8278
defer gitRepo.Close()
8379

80+
// Make a new branch in repo1
81+
newBranch := "test_branch"
82+
err = repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, gitRepo, repo1.DefaultBranch, newBranch)
83+
assert.NoError(t, err)
84+
8485
commitID, err := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
8586
assert.NoError(t, err)
8687
// Make a new tag in repo1

0 commit comments

Comments
 (0)