Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also sync DB branches on push if necessary #28361

Merged
merged 13 commits into from
Dec 9, 2023
30 changes: 13 additions & 17 deletions models/git/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ 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
lunny marked this conversation as resolved.
Show resolved Hide resolved
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).
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 +217,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 +293,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
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
38 changes: 38 additions & 0 deletions modules/repository/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package repository

import (
"context"
"fmt"

"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
Expand All @@ -13,8 +14,45 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
)

// 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 := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That function is completely wrong here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I move the function to service layer and renamed function name to syncBranchToDB. Maybe it fixed your problem?

if err != nil {
return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err)
}
if cnt > 0 { // if branch does exist, so it's a normal update
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
totalBranches, err := db.Count[git_model.Branch](ctx, &git_model.FindBranchOptions{
RepoID: repoID,
IsDeletedBranch: util.OptionalBoolFalse,
})
if err != nil {
return err
}
if totalBranches == 0 {
lunny marked this conversation as resolved.
Show resolved Hide resolved
if _, err = SyncRepoBranches(ctx, repoID, pusherID); err != nil {
return fmt.Errorf("repo_module.SyncRepoBranches %d:%s failed: %v", repoID, branchName, err)
}
return nil
}
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()),
})
}

// SyncRepoBranches synchronizes branch table with repository branches
func SyncRepoBranches(ctx context.Context, repoID, doerID int64) (int64, error) {
repo, err := repo_model.GetRepositoryByID(ctx, repoID)
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
57 changes: 22 additions & 35 deletions services/repository/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,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 @@ -250,7 +228,7 @@ func checkBranchName(ctx context.Context, repo *repo_model.Repository, name stri
}

// 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 +239,27 @@ 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
}
if err := repo_module.UpdateBranch(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 = repo_module.UpdateBranch(ctx, repo.ID, opts.PusherID, branch, newCommit); err != nil {
lunny marked this conversation as resolved.
Show resolved Hide resolved
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