[GITEA] Remove redundant syncBranchToDB

- The transaction in combination with Git push was causing deadlocks if
you had the `push_update` queue set to `immediate`. This was the root
cause of slow integration tests in CI.
- Remove the sync branch code as this is already being done in the Git
post-receive hook.
- Add tests to proof the branch models are in sync even with this code
removed.

(cherry picked from commit 90110e1f44)
(cherry picked from commit a064065cb9)
This commit is contained in:
Gusted 2023-12-17 22:49:58 +01:00 committed by Earl Warren
parent 30bd28eed3
commit 7713e558eb
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
3 changed files with 65 additions and 78 deletions

View file

@ -276,28 +276,18 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo
return err return err
} }
return db.WithTx(ctx, func(ctx context.Context) error { if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
commit, err := gitRepo.GetCommit(commitID) Remote: repo.RepoPath(),
if err != nil { Branch: fmt.Sprintf("%s:%s%s", commitID, git.BranchPrefix, branchName),
return err Env: repo_module.PushingEnvironment(doer, repo),
} }); err != nil {
// database operation should be done before git operation so that we can rollback if git operation failed if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
if err := syncBranchToDB(ctx, repo.ID, doer.ID, branchName, commit); err != nil {
return err return err
} }
return fmt.Errorf("push: %w", err)
}
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{ return nil
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 // RenameBranch rename a branch

View file

@ -4,72 +4,55 @@
package integration package integration
import ( import (
"fmt"
"net/http" "net/http"
"net/url" "net/url"
"testing" "testing"
"code.gitea.io/gitea/modules/translation" git_model "code.gitea.io/gitea/models/git"
"code.gitea.io/gitea/tests" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
gitea_context "code.gitea.io/gitea/modules/context"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
func TestViewBranches(t *testing.T) { func TestBranchActions(t *testing.T) {
defer tests.PrepareTestEnv(t)()
req := NewRequest(t, "GET", "/user2/repo1/branches")
resp := MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
_, exists := htmlDoc.doc.Find(".delete-branch-button").Attr("data-url")
assert.False(t, exists, "The template has changed")
}
func TestDeleteBranch(t *testing.T) {
defer tests.PrepareTestEnv(t)()
deleteBranch(t)
}
func TestUndoDeleteBranch(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) { onGiteaRun(t, func(t *testing.T, u *url.URL) {
deleteBranch(t) session := loginUser(t, "user2")
htmlDoc, name := branchAction(t, ".restore-branch-button") repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
assert.Contains(t, branch3 := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 3, RepoID: repo1.ID})
htmlDoc.doc.Find(".ui.positive.message").Text(), branchesLink := repo1.FullName() + "/branches"
translation.NewLocale("en-US").Tr("repo.branch.restore_success", name),
) t.Run("View", func(t *testing.T) {
req := NewRequest(t, "GET", branchesLink)
MakeRequest(t, req, http.StatusOK)
})
t.Run("Delete branch", func(t *testing.T) {
link := fmt.Sprintf("/%s/branches/delete?name=%s", repo1.FullName(), branch3.Name)
req := NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": GetCSRF(t, session, branchesLink),
})
session.MakeRequest(t, req, http.StatusOK)
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.Contains(t, flashCookie.Value, "success%3DBranch%2B%2522branch2%2522%2Bhas%2Bbeen%2Bdeleted.")
assert.True(t, unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 3, RepoID: repo1.ID}).IsDeleted)
})
t.Run("Restore branch", func(t *testing.T) {
link := fmt.Sprintf("/%s/branches/restore?branch_id=%d&name=%s", repo1.FullName(), branch3.ID, branch3.Name)
req := NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": GetCSRF(t, session, branchesLink),
})
session.MakeRequest(t, req, http.StatusOK)
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.Contains(t, flashCookie.Value, "success%3DBranch%2B%2522branch2%2522%2Bhas%2Bbeen%2Brestored")
assert.False(t, unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 3, RepoID: repo1.ID}).IsDeleted)
})
}) })
} }
func deleteBranch(t *testing.T) {
htmlDoc, name := branchAction(t, ".delete-branch-button")
assert.Contains(t,
htmlDoc.doc.Find(".ui.positive.message").Text(),
translation.NewLocale("en-US").Tr("repo.branch.deletion_success", name),
)
}
func branchAction(t *testing.T, button string) (*HTMLDoc, string) {
session := loginUser(t, "user2")
req := NewRequest(t, "GET", "/user2/repo1/branches")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
link, exists := htmlDoc.doc.Find(button).Attr("data-url")
if !assert.True(t, exists, "The template has changed") {
t.Skip()
}
req = NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": htmlDoc.GetCSRF(),
})
session.MakeRequest(t, req, http.StatusOK)
url, err := url.Parse(link)
assert.NoError(t, err)
req = NewRequest(t, "GET", "/user2/repo1/branches")
resp = session.MakeRequest(t, req, http.StatusOK)
return NewHTMLParser(t, resp.Body), url.Query().Get("name")
}

View file

@ -10,6 +10,8 @@ import (
"strings" "strings"
"testing" "testing"
git_model "code.gitea.io/gitea/models/git"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/translation"
@ -47,12 +49,14 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) {
CreateRelease string CreateRelease string
FlashMessage string FlashMessage string
ExpectedStatus int ExpectedStatus int
CheckBranch bool
}{ }{
{ {
OldRefSubURL: "branch/master", OldRefSubURL: "branch/master",
NewBranch: "feature/test1", NewBranch: "feature/test1",
ExpectedStatus: http.StatusSeeOther, ExpectedStatus: http.StatusSeeOther,
FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test1"), FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test1"),
CheckBranch: true,
}, },
{ {
OldRefSubURL: "branch/master", OldRefSubURL: "branch/master",
@ -65,6 +69,7 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) {
NewBranch: "feature=test1", NewBranch: "feature=test1",
ExpectedStatus: http.StatusSeeOther, ExpectedStatus: http.StatusSeeOther,
FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature=test1"), FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature=test1"),
CheckBranch: true,
}, },
{ {
OldRefSubURL: "branch/master", OldRefSubURL: "branch/master",
@ -94,6 +99,7 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) {
NewBranch: "feature/test3", NewBranch: "feature/test3",
ExpectedStatus: http.StatusSeeOther, ExpectedStatus: http.StatusSeeOther,
FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test3"), FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test3"),
CheckBranch: true,
}, },
{ {
OldRefSubURL: "branch/master", OldRefSubURL: "branch/master",
@ -108,10 +114,15 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) {
CreateRelease: "v1.0.1", CreateRelease: "v1.0.1",
ExpectedStatus: http.StatusSeeOther, ExpectedStatus: http.StatusSeeOther,
FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test4"), FlashMessage: translation.NewLocale("en-US").Tr("repo.branch.create_success", "feature/test4"),
CheckBranch: true,
}, },
} }
session := loginUser(t, "user2")
for _, test := range tests { for _, test := range tests {
session := loginUser(t, "user2") if test.CheckBranch {
unittest.AssertNotExistsBean(t, &git_model.Branch{RepoID: 1, Name: test.NewBranch})
}
if test.CreateRelease != "" { if test.CreateRelease != "" {
createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false) createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false)
} }
@ -125,6 +136,9 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) {
test.FlashMessage, test.FlashMessage,
) )
} }
if test.CheckBranch {
unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: 1, Name: test.NewBranch})
}
} }
} }