Skip to content

Commit 9466fec

Browse files
lunnylunny
authored
Fix rename branch 500 when the target branch is deleted but exist in database (#30430)
Fix #30428
1 parent f9fdac9 commit 9466fec

File tree

3 files changed

+107
-12
lines changed

3 files changed

+107
-12
lines changed

‎models/git/branch.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
297297

298298
sess := db.GetEngine(ctx)
299299

300+
// check whether from branch exist
300301
var branch Branch
301302
exist, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repo.ID, from).Get(&branch)
302303
if err != nil {
@@ -308,6 +309,24 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
308309
}
309310
}
310311

312+
// check whether to branch exist or is_deleted
313+
var dstBranch Branch
314+
exist, err = db.GetEngine(ctx).Where("repo_id=? AND name=?", repo.ID, to).Get(&dstBranch)
315+
if err != nil {
316+
return err
317+
}
318+
if exist {
319+
if !dstBranch.IsDeleted {
320+
return ErrBranchAlreadyExists{
321+
BranchName: to,
322+
}
323+
}
324+
325+
if _, err := db.GetEngine(ctx).ID(dstBranch.ID).NoAutoCondition().Delete(&dstBranch); err != nil {
326+
return err
327+
}
328+
}
329+
311330
// 1. update branch in database
312331
if n, err := sess.Where("repo_id=? AND name=?", repo.ID, from).Update(&Branch{
313332
Name: to,
@@ -362,12 +381,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
362381
return err
363382
}
364383

365-
// 5. do git action
366-
if err = gitAction(ctx, isDefault); err != nil {
367-
return err
368-
}
369-
370-
// 6. insert renamed branch record
384+
// 5. insert renamed branch record
371385
renamedBranch := &RenamedBranch{
372386
RepoID: repo.ID,
373387
From: from,
@@ -378,6 +392,11 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
378392
return err
379393
}
380394

395+
// 6. do git action
396+
if err = gitAction(ctx, isDefault); err != nil {
397+
return err
398+
}
399+
381400
return committer.Commit()
382401
}
383402

‎routers/web/repo/setting/protected_branch.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,13 @@ func RenameBranchPost(ctx *context.Context) {
313313

314314
msg, err := repository.RenameBranch(ctx, ctx.Repo.Repository, ctx.Doer, ctx.Repo.GitRepo, form.From, form.To)
315315
if err != nil {
316-
ctx.ServerError("RenameBranch", err)
316+
switch {
317+
case git_model.IsErrBranchAlreadyExists(err):
318+
ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", form.To))
319+
ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink))
320+
default:
321+
ctx.ServerError("RenameBranch", err)
322+
}
317323
return
318324
}
319325

‎tests/integration/rename_branch_test.go

Lines changed: 75 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,23 @@ package integration
55

66
import (
77
"net/http"
8+
"net/url"
89
"testing"
910

1011
git_model "code.gitea.io/gitea/models/git"
1112
repo_model "code.gitea.io/gitea/models/repo"
1213
"code.gitea.io/gitea/models/unittest"
14+
gitea_context "code.gitea.io/gitea/services/context"
1315
"code.gitea.io/gitea/tests"
1416

1517
"github.com/stretchr/testify/assert"
1618
)
1719

1820
func TestRenameBranch(t *testing.T) {
21+
onGiteaRun(t, testRenameBranch)
22+
}
23+
24+
func testRenameBranch(t *testing.T, u *url.URL) {
1925
defer tests.PrepareTestEnv(t)()
2026

2127
unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: 1, Name: "master"})
@@ -26,25 +32,89 @@ func TestRenameBranch(t *testing.T) {
2632
resp := session.MakeRequest(t, req, http.StatusOK)
2733
htmlDoc := NewHTMLParser(t, resp.Body)
2834

29-
postData := map[string]string{
35+
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{
3036
"_csrf": htmlDoc.GetCSRF(),
3137
"from": "master",
3238
"to": "main",
33-
}
34-
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", postData)
39+
})
3540
session.MakeRequest(t, req, http.StatusSeeOther)
3641

3742
// check new branch link
38-
req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", postData)
43+
req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", nil)
3944
session.MakeRequest(t, req, http.StatusOK)
4045

4146
// check old branch link
42-
req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/master/README.md", postData)
47+
req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/master/README.md", nil)
4348
resp = session.MakeRequest(t, req, http.StatusSeeOther)
4449
location := resp.Header().Get("Location")
4550
assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location)
4651

4752
// check db
4853
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
4954
assert.Equal(t, "main", repo1.DefaultBranch)
55+
56+
// create branch1
57+
csrf := GetCSRF(t, session, "/user2/repo1/src/branch/main")
58+
59+
req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/_new/branch/main", map[string]string{
60+
"_csrf": csrf,
61+
"new_branch_name": "branch1",
62+
})
63+
session.MakeRequest(t, req, http.StatusSeeOther)
64+
65+
branch1 := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"})
66+
assert.Equal(t, "branch1", branch1.Name)
67+
68+
// create branch2
69+
req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/_new/branch/main", map[string]string{
70+
"_csrf": csrf,
71+
"new_branch_name": "branch2",
72+
})
73+
session.MakeRequest(t, req, http.StatusSeeOther)
74+
75+
branch2 := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"})
76+
assert.Equal(t, "branch2", branch2.Name)
77+
78+
// rename branch2 to branch1
79+
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{
80+
"_csrf": htmlDoc.GetCSRF(),
81+
"from": "branch2",
82+
"to": "branch1",
83+
})
84+
session.MakeRequest(t, req, http.StatusSeeOther)
85+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
86+
assert.NotNil(t, flashCookie)
87+
assert.Contains(t, flashCookie.Value, "error")
88+
89+
branch2 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"})
90+
assert.Equal(t, "branch2", branch2.Name)
91+
branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"})
92+
assert.Equal(t, "branch1", branch1.Name)
93+
94+
// delete branch1
95+
req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/delete", map[string]string{
96+
"_csrf": htmlDoc.GetCSRF(),
97+
"name": "branch1",
98+
})
99+
session.MakeRequest(t, req, http.StatusOK)
100+
branch2 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"})
101+
assert.Equal(t, "branch2", branch2.Name)
102+
branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"})
103+
assert.True(t, branch1.IsDeleted) // virtual deletion
104+
105+
// rename branch2 to branch1 again
106+
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{
107+
"_csrf": htmlDoc.GetCSRF(),
108+
"from": "branch2",
109+
"to": "branch1",
110+
})
111+
session.MakeRequest(t, req, http.StatusSeeOther)
112+
113+
flashCookie = session.GetCookie(gitea_context.CookieNameFlash)
114+
assert.NotNil(t, flashCookie)
115+
assert.Contains(t, flashCookie.Value, "success")
116+
117+
unittest.AssertNotExistsBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"})
118+
branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"})
119+
assert.Equal(t, "branch1", branch1.Name)
50120
}

0 commit comments

Comments
 (0)