Fix rename branch 500 when the target branch is deleted but exist in database (#30430...

Backport #30430 by @lunny

Fix #30428

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

diff --git a/models/git/branch.go b/models/git/branch.go
index fa0781fed1d5728a8bd54958f45bd01ca55e4f72..2979dff3d211e91300a1b0d9b6aedf4e6dfc516c 100644 (file)
--- a/models/git/branch.go
+++ b/models/git/branch.go
@@ -297,6 +297,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 {
@@ -308,6 +309,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,
@@ -362,12 +381,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,
@@ -378,6 +392,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 b30dc3b0614f2997a235c135cb4db1b9e81fb9a0..4bab3f897a20a7c77e4393655ecbfdb5d2e61e0d 100644 (file)
--- a/routers/web/repo/setting/protected_branch.go
+++ b/routers/web/repo/setting/protected_branch.go
@@ -313,7 +313,13 @@ func RenameBranchPost(ctx *context.Context) {
 
        msg, err := repository.RenameBranch(ctx, ctx.Repo.Repository, ctx.Doer, ctx.Repo.GitRepo, form.From, form.To)
        if err != nil {
-               ctx.ServerError("RenameBranch", err)
+               switch {
+               case 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))
+               default:
+                       ctx.ServerError("RenameBranch", err)
+               }
                return
        }
 

diff --git a/tests/integration/rename_branch_test.go b/tests/integration/rename_branch_test.go
index 703fc243a4d3e382e4fef83504d21dc13a1777c4..13f6cf204b539351e68cbd2a9d774a09be006ae1 100644 (file)
--- a/tests/integration/rename_branch_test.go
+++ b/tests/integration/rename_branch_test.go
@@ -5,17 +5,23 @@ package integration
 
 import (
        "net/http"
+       "net/url"
        "testing"
 
        git_model "code.gitea.io/gitea/models/git"
        repo_model "code.gitea.io/gitea/models/repo"
        "code.gitea.io/gitea/models/unittest"
+       gitea_context "code.gitea.io/gitea/services/context"
        "code.gitea.io/gitea/tests"
 
        "github.com/stretchr/testify/assert"
 )
 
 func TestRenameBranch(t *testing.T) {
+       onGiteaRun(t, testRenameBranch)
+}
+
+func testRenameBranch(t *testing.T, u *url.URL) {
        defer tests.PrepareTestEnv(t)()
 
        unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: 1, Name: "master"})
@@ -26,20 +32,19 @@ func TestRenameBranch(t *testing.T) {
        resp := session.MakeRequest(t, req, http.StatusOK)
        htmlDoc := NewHTMLParser(t, resp.Body)
 
-       postData := map[string]string{
+       req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{
                "_csrf": htmlDoc.GetCSRF(),
                "from":  "master",
                "to":    "main",
-       }
-       req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", postData)
+       })
        session.MakeRequest(t, req, http.StatusSeeOther)
 
        // check new branch link
-       req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", postData)
+       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", postData)
+       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)
@@ -47,4 +52,69 @@ func TestRenameBranch(t *testing.T) {
        // 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": htmlDoc.GetCSRF(),
+               "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": htmlDoc.GetCSRF(),
+               "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": htmlDoc.GetCSRF(),
+               "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)
 }