Skip to content

Commit

Permalink
Also sync DB branches on push if necessary (go-gitea#28361)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lunny authored and silverwind committed Feb 20, 2024
1 parent a053cff commit aef1ebc
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 73 deletions.
33 changes: 14 additions & 19 deletions models/git/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,9 @@ func DeleteBranches(ctx context.Context, repoID, doerID int64, branchIDs []int64
})
}

// UpdateBranch updates the branch information in the database. If the branch exist, it will update latest commit of this branch information
// If it doest not exist, insert a new record into database
func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) error {
cnt, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repoID, branchName).
// UpdateBranch updates the branch information in the database.
func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) (int64, error) {
return db.GetEngine(ctx).Where("repo_id=? AND name=?", repoID, branchName).
Cols("commit_id, commit_message, pusher_id, commit_time, is_deleted, updated_unix").
Update(&Branch{
CommitID: commit.ID.String(),
Expand All @@ -217,21 +216,6 @@ func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
IsDeleted: false,
})
if err != nil {
return err
}
if cnt > 0 {
return nil
}

return db.Insert(ctx, &Branch{
RepoID: repoID,
Name: branchName,
CommitID: commit.ID.String(),
CommitMessage: commit.Summary(),
PusherID: pusherID,
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
})
}

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

sess := db.GetEngine(ctx)

var branch Branch
exist, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repo.ID, from).Get(&branch)
if err != nil {
return err
} else if !exist || branch.IsDeleted {
return ErrBranchNotExist{
RepoID: repo.ID,
BranchName: from,
}
}

// 1. update branch in database
if n, err := sess.Where("repo_id=? AND name=?", repo.ID, from).Update(&Branch{
Name: to,
Expand Down
8 changes: 4 additions & 4 deletions models/git/branch_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type FindBranchOptions struct {
Keyword string
}

func (opts *FindBranchOptions) Cond() builder.Cond {
func (opts FindBranchOptions) ToConds() builder.Cond {
cond := builder.NewCond()
if opts.RepoID > 0 {
cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
Expand All @@ -92,7 +92,7 @@ func (opts *FindBranchOptions) Cond() builder.Cond {
}

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

func orderByBranches(sess *xorm.Session, opts FindBranchOptions) *xorm.Session {
Expand All @@ -108,7 +108,7 @@ func orderByBranches(sess *xorm.Session, opts FindBranchOptions) *xorm.Session {
}

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

func FindBranchNames(ctx context.Context, opts FindBranchOptions) ([]string, error) {
sess := db.GetEngine(ctx).Select("name").Where(opts.Cond())
sess := db.GetEngine(ctx).Select("name").Where(opts.ToConds())
if opts.PageSize > 0 && !opts.IsListAll() {
sess = db.SetSessionPagination(sess, &opts.ListOptions)
}
Expand Down
2 changes: 1 addition & 1 deletion models/git/branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestAddDeletedBranch(t *testing.T) {
},
}

err := git_model.UpdateBranch(db.DefaultContext, repo.ID, secondBranch.PusherID, secondBranch.Name, commit)
_, err := git_model.UpdateBranch(db.DefaultContext, repo.ID, secondBranch.PusherID, secondBranch.Name, commit)
assert.NoError(t, err)
}

Expand Down
5 changes: 2 additions & 3 deletions routers/api/v1/repo/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,11 @@ func CreateBranch(ctx *context.APIContext) {
}
}

err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, oldCommit.ID.String(), opt.BranchName)
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, oldCommit.ID.String(), opt.BranchName)
if err != nil {
if git_model.IsErrBranchNotExist(err) {
ctx.Error(http.StatusNotFound, "", "The old branch does not exist")
}
if models.IsErrTagAlreadyExists(err) {
} else if models.IsErrTagAlreadyExists(err) {
ctx.Error(http.StatusConflict, "", "The branch with the same tag already exists.")
} else if git_model.IsErrBranchAlreadyExists(err) || git.IsErrPushOutOfDate(err) {
ctx.Error(http.StatusConflict, "", "The branch already exists.")
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ func CreateBranch(ctx *context.Context) {
}
err = release_service.CreateNewTag(ctx, ctx.Doer, ctx.Repo.Repository, target, form.NewBranchName, "")
} else if ctx.Repo.IsViewBranch {
err = repo_service.CreateNewBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName, form.NewBranchName)
err = repo_service.CreateNewBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, ctx.Repo.BranchName, form.NewBranchName)
} else {
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.CommitID, form.NewBranchName)
err = repo_service.CreateNewBranchFromCommit(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, ctx.Repo.CommitID, form.NewBranchName)
}
if err != nil {
if models.IsErrProtectedTagName(err) {
Expand Down
100 changes: 65 additions & 35 deletions services/repository/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/queue"
repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
notify_service "code.gitea.io/gitea/services/notify"
files_service "code.gitea.io/gitea/services/repository/files"
Expand All @@ -28,35 +29,13 @@ import (
)

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

// Check if branch name can be used
if err := checkBranchName(ctx, repo, branchName); err != nil {
return err
}

if !git.IsBranchExist(ctx, repo.RepoPath(), oldBranchName) {
return git_model.ErrBranchNotExist{
BranchName: oldBranchName,
}
}

if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
Remote: repo.RepoPath(),
Branch: fmt.Sprintf("%s%s:%s%s", git.BranchPrefix, oldBranchName, 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 CreateNewBranchFromCommit(ctx, doer, repo, gitRepo, branch.CommitID, branchName)
}

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

// syncBranchToDB sync the branch information in the database. It 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.
func syncBranchToDB(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) error {
cnt, err := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit)
if err != nil {
return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err)
}
if cnt > 0 { // This means branch does exist, so it's a normal update. It also means the branch has been synced.
return nil
}

// if user haven't visit UI but directly push to a branch after upgrading from 1.20 -> 1.21,
// we cannot simply insert the branch but need to check we have branches or not
hasBranch, err := db.Exist[git_model.Branch](ctx, git_model.FindBranchOptions{
RepoID: repoID,
IsDeletedBranch: util.OptionalBoolFalse,
}.ToConds())
if err != nil {
return err
}
if !hasBranch {
if _, err = repo_module.SyncRepoBranches(ctx, repoID, pusherID); err != nil {
return fmt.Errorf("repo_module.SyncRepoBranches %d:%s failed: %v", repoID, branchName, err)
}
return nil
}

// if database have branches but not this branch, it means this is a new branch
return db.Insert(ctx, &git_model.Branch{
RepoID: repoID,
Name: branchName,
CommitID: commit.ID.String(),
CommitMessage: commit.Summary(),
PusherID: pusherID,
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
})
}

// CreateNewBranchFromCommit creates a new repository branch
func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, commit, branchName string) (err error) {
func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, commitID, branchName string) (err error) {
err = repo.MustNotBeArchived()
if err != nil {
return err
Expand All @@ -261,18 +281,28 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo
return err
}

if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
Remote: repo.RepoPath(),
Branch: fmt.Sprintf("%s:%s%s", commit, git.BranchPrefix, branchName),
Env: repo_module.PushingEnvironment(doer, repo),
}); err != nil {
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(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 {
return err
}
return fmt.Errorf("push: %w", err)
}

return 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
}
return fmt.Errorf("push: %w", err)
}
return nil
})
}

// RenameBranch rename a branch
Expand Down
2 changes: 1 addition & 1 deletion services/repository/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
commits.Commits = commits.Commits[:setting.UI.FeedMaxCommitNum]
}

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

Expand Down
9 changes: 5 additions & 4 deletions tests/integration/api_repo_get_contents_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,16 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
session = loginUser(t, user4.Name)
token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)

// Make a new branch in repo1
newBranch := "test_branch"
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
assert.NoError(t, err)
// Get the commit ID of the default branch
gitRepo, err := git.OpenRepository(git.DefaultContext, repo1.RepoPath())
assert.NoError(t, err)
defer gitRepo.Close()

// Make a new branch in repo1
newBranch := "test_branch"
err = repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, gitRepo, repo1.DefaultBranch, newBranch)
assert.NoError(t, err)

commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
// Make a new tag in repo1
newTag := "test_tag"
Expand Down
9 changes: 5 additions & 4 deletions tests/integration/api_repo_get_contents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,16 @@ func testAPIGetContents(t *testing.T, u *url.URL) {
session = loginUser(t, user4.Name)
token4 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)

// Make a new branch in repo1
newBranch := "test_branch"
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
assert.NoError(t, err)
// Get the commit ID of the default branch
gitRepo, err := git.OpenRepository(git.DefaultContext, repo1.RepoPath())
assert.NoError(t, err)
defer gitRepo.Close()

// Make a new branch in repo1
newBranch := "test_branch"
err = repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, gitRepo, repo1.DefaultBranch, newBranch)
assert.NoError(t, err)

commitID, err := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
assert.NoError(t, err)
// Make a new tag in repo1
Expand Down

0 comments on commit aef1ebc

Please sign in to comment.