From fe09c8860ab29479b060a2e741a66a88bfa5cfde Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 15 Apr 2024 16:03:09 +0200 Subject: [PATCH] [PORT] gitea#30430: Fix rename branch 500 when the target branch is deleted but exist in database Fix https://github.com/go-gitea/gitea/issues/30428 --- Conflict resolution: trivial and move test to own subtest run directly after `Normal`. (cherrypicked commit 9466fec879f4f2c88c7c1e7a5cffba319282ab66) (cherry picked from commit db39b8f4a73415173dccff4d2c4a44d867e4c106) --- models/git/branch.go | 31 +++++-- routers/web/repo/setting/protected_branch.go | 3 + tests/integration/rename_branch_test.go | 95 ++++++++++++++++++++ 3 files changed, 123 insertions(+), 6 deletions(-) diff --git a/models/git/branch.go b/models/git/branch.go index a12cbd4dee..c905a2d368 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -301,6 +301,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str sess := db.GetEngine(ctx) + // check whether from branch exist var branch Branch exist, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repo.ID, from).Get(&branch) if err != nil { @@ -312,6 +313,24 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str } } + // check whether to branch exist or is_deleted + var dstBranch Branch + exist, err = db.GetEngine(ctx).Where("repo_id=? AND name=?", repo.ID, to).Get(&dstBranch) + if err != nil { + return err + } + if exist { + if !dstBranch.IsDeleted { + return ErrBranchAlreadyExists{ + BranchName: to, + } + } + + if _, err := db.GetEngine(ctx).ID(dstBranch.ID).NoAutoCondition().Delete(&dstBranch); err != nil { + return err + } + } + // 1. update branch in database if n, err := sess.Where("repo_id=? AND name=?", repo.ID, from).Update(&Branch{ Name: to, @@ -366,12 +385,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str return err } - // 5. do git action - if err = gitAction(ctx, isDefault); err != nil { - return err - } - - // 6. insert renamed branch record + // 5. insert renamed branch record renamedBranch := &RenamedBranch{ RepoID: repo.ID, From: from, @@ -382,6 +396,11 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str return err } + // 6. do git action + if err = gitAction(ctx, isDefault); err != nil { + return err + } + return committer.Commit() } diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index 25146779de..b2f5798a26 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -321,6 +321,9 @@ func RenameBranchPost(ctx *context.Context) { if errors.Is(err, git_model.ErrBranchIsProtected) { ctx.Flash.Error(ctx.Tr("repo.settings.rename_branch_failed_protected", form.To)) ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink)) + } else if git_model.IsErrBranchAlreadyExists(err) { + ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", form.To)) + ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink)) } else { ctx.ServerError("RenameBranch", err) } diff --git a/tests/integration/rename_branch_test.go b/tests/integration/rename_branch_test.go index b037178f90..27be77f71a 100644 --- a/tests/integration/rename_branch_test.go +++ b/tests/integration/rename_branch_test.go @@ -5,6 +5,7 @@ package integration import ( "net/http" + "net/url" "testing" git_model "code.gitea.io/gitea/models/git" @@ -17,6 +18,10 @@ import ( ) func TestRenameBranch(t *testing.T) { + onGiteaRun(t, testRenameBranch) +} + +func testRenameBranch(t *testing.T, u *url.URL) { defer tests.PrepareTestEnv(t)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) @@ -49,6 +54,96 @@ func TestRenameBranch(t *testing.T) { assert.Equal(t, "main", repo1.DefaultBranch) }) + t.Run("Database syncronization", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"), + "from": "master", + "to": "main", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + // check new branch link + req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", nil) + session.MakeRequest(t, req, http.StatusOK) + + // check old branch link + req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/master/README.md", nil) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + location := resp.Header().Get("Location") + assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location) + + // check db + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.Equal(t, "main", repo1.DefaultBranch) + + // create branch1 + csrf := GetCSRF(t, session, "/user2/repo1/src/branch/main") + + req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/_new/branch/main", map[string]string{ + "_csrf": csrf, + "new_branch_name": "branch1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + branch1 := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"}) + assert.Equal(t, "branch1", branch1.Name) + + // create branch2 + req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/_new/branch/main", map[string]string{ + "_csrf": csrf, + "new_branch_name": "branch2", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + branch2 := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"}) + assert.Equal(t, "branch2", branch2.Name) + + // rename branch2 to branch1 + req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"), + "from": "branch2", + "to": "branch1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Contains(t, flashCookie.Value, "error") + + branch2 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"}) + assert.Equal(t, "branch2", branch2.Name) + branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"}) + assert.Equal(t, "branch1", branch1.Name) + + // delete branch1 + req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/delete", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"), + "name": "branch1", + }) + session.MakeRequest(t, req, http.StatusOK) + branch2 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"}) + assert.Equal(t, "branch2", branch2.Name) + branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"}) + assert.True(t, branch1.IsDeleted) // virtual deletion + + // rename branch2 to branch1 again + req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"), + "from": "branch2", + "to": "branch1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie = session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Contains(t, flashCookie.Value, "success") + + unittest.AssertNotExistsBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"}) + branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"}) + assert.Equal(t, "branch1", branch1.Name) + }) + t.Run("Protected branch", func(t *testing.T) { defer tests.PrintCurrentTest(t)()