Skip to content

Commit 2686e6b

Browse files
authored
Check if label template exist first (#14384)
* add check * refactor * rollback repo on error after session closed
1 parent 127907c commit 2686e6b

File tree

5 files changed

+40
-33
lines changed

5 files changed

+40
-33
lines changed

models/issue_label.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ type Label struct {
4747
func GetLabelTemplateFile(name string) ([][3]string, error) {
4848
data, err := GetRepoInitFile("label", name)
4949
if err != nil {
50-
return nil, fmt.Errorf("GetRepoInitFile: %v", err)
50+
return nil, ErrIssueLabelTemplateLoad{name, fmt.Errorf("GetRepoInitFile: %v", err)}
5151
}
5252

5353
lines := strings.Split(string(data), "\n")
@@ -62,15 +62,15 @@ func GetLabelTemplateFile(name string) ([][3]string, error) {
6262

6363
fields := strings.SplitN(parts[0], " ", 2)
6464
if len(fields) != 2 {
65-
return nil, fmt.Errorf("line is malformed: %s", line)
65+
return nil, ErrIssueLabelTemplateLoad{name, fmt.Errorf("line is malformed: %s", line)}
6666
}
6767

6868
color := strings.Trim(fields[0], " ")
6969
if len(color) == 6 {
7070
color = "#" + color
7171
}
7272
if !LabelColorPattern.MatchString(color) {
73-
return nil, fmt.Errorf("bad HTML color code in line: %s", line)
73+
return nil, ErrIssueLabelTemplateLoad{name, fmt.Errorf("bad HTML color code in line: %s", line)}
7474
}
7575

7676
var description string
@@ -167,7 +167,7 @@ func (label *Label) ForegroundColor() template.CSS {
167167
func loadLabels(labelTemplate string) ([]string, error) {
168168
list, err := GetLabelTemplateFile(labelTemplate)
169169
if err != nil {
170-
return nil, ErrIssueLabelTemplateLoad{labelTemplate, err}
170+
return nil, err
171171
}
172172

173173
labels := make([]string, len(list))
@@ -186,7 +186,7 @@ func LoadLabelsFormatted(labelTemplate string) (string, error) {
186186
func initializeLabels(e Engine, id int64, labelTemplate string, isOrg bool) error {
187187
list, err := GetLabelTemplateFile(labelTemplate)
188188
if err != nil {
189-
return ErrIssueLabelTemplateLoad{labelTemplate, err}
189+
return err
190190
}
191191

192192
labels := make([]*Label, len(list))

models/repo.go

+12-18
Original file line numberDiff line numberDiff line change
@@ -1511,26 +1511,27 @@ func UpdateRepositoryUnits(repo *Repository, units []RepoUnit, deleteUnitTypes [
15111511
}
15121512

15131513
// DeleteRepository deletes a repository for a user or organization.
1514+
// make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock)
15141515
func DeleteRepository(doer *User, uid, repoID int64) error {
1516+
sess := x.NewSession()
1517+
defer sess.Close()
1518+
if err := sess.Begin(); err != nil {
1519+
return err
1520+
}
1521+
15151522
// In case is a organization.
1516-
org, err := GetUserByID(uid)
1523+
org, err := getUserByID(sess, uid)
15171524
if err != nil {
15181525
return err
15191526
}
15201527
if org.IsOrganization() {
1521-
if err = org.GetTeams(&SearchTeamOptions{}); err != nil {
1528+
if err = org.getTeams(sess); err != nil {
15221529
return err
15231530
}
15241531
}
15251532

1526-
sess := x.NewSession()
1527-
defer sess.Close()
1528-
if err = sess.Begin(); err != nil {
1529-
return err
1530-
}
1531-
1532-
repo := &Repository{ID: repoID, OwnerID: uid}
1533-
has, err := sess.Get(repo)
1533+
repo := &Repository{OwnerID: uid}
1534+
has, err := sess.ID(repoID).Get(repo)
15341535
if err != nil {
15351536
return err
15361537
} else if !has {
@@ -1679,14 +1680,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
16791680
}
16801681

16811682
if err = sess.Commit(); err != nil {
1682-
sess.Close()
1683-
if len(deployKeys) > 0 {
1684-
// We need to rewrite the public keys because the commit failed
1685-
if err2 := RewriteAllPublicKeys(); err2 != nil {
1686-
return fmt.Errorf("Commit: %v SSH Keys: %v", err, err2)
1687-
}
1688-
}
1689-
return fmt.Errorf("Commit: %v", err)
1683+
return err
16901684
}
16911685

16921686
sess.Close()

modules/repository/create.go

+21-8
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
2727
opts.DefaultBranch = setting.Repository.DefaultBranch
2828
}
2929

30+
// Check if label template exist
31+
if len(opts.IssueLabels) > 0 {
32+
if _, err := models.GetLabelTemplateFile(opts.IssueLabels); err != nil {
33+
return nil, err
34+
}
35+
}
36+
3037
repo := &models.Repository{
3138
OwnerID: u.ID,
3239
Owner: u,
@@ -45,6 +52,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
4552
TrustModel: opts.TrustModel,
4653
}
4754

55+
var rollbackRepo *models.Repository
56+
4857
if err := models.WithTx(func(ctx models.DBContext) error {
4958
if err := models.CreateRepository(ctx, doer, u, repo, false); err != nil {
5059
return err
@@ -76,7 +85,7 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
7685
}
7786
}
7887

79-
if err := initRepository(ctx, repoPath, doer, repo, opts); err != nil {
88+
if err = initRepository(ctx, repoPath, doer, repo, opts); err != nil {
8089
if err2 := util.RemoveAll(repoPath); err2 != nil {
8190
log.Error("initRepository: %v", err)
8291
return fmt.Errorf(
@@ -87,10 +96,9 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
8796

8897
// Initialize Issue Labels if selected
8998
if len(opts.IssueLabels) > 0 {
90-
if err := models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil {
91-
if errDelete := models.DeleteRepository(doer, u.ID, repo.ID); errDelete != nil {
92-
log.Error("Rollback deleteRepository: %v", errDelete)
93-
}
99+
if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil {
100+
rollbackRepo = repo
101+
rollbackRepo.OwnerID = u.ID
94102
return fmt.Errorf("InitializeLabels: %v", err)
95103
}
96104
}
@@ -99,13 +107,18 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
99107
SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)).
100108
RunInDir(repoPath); err != nil {
101109
log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
102-
if errDelete := models.DeleteRepository(doer, u.ID, repo.ID); errDelete != nil {
103-
log.Error("Rollback deleteRepository: %v", errDelete)
104-
}
110+
rollbackRepo = repo
111+
rollbackRepo.OwnerID = u.ID
105112
return fmt.Errorf("CreateRepository(git update-server-info): %v", err)
106113
}
107114
return nil
108115
}); err != nil {
116+
if rollbackRepo != nil {
117+
if errDelete := models.DeleteRepository(doer, rollbackRepo.OwnerID, rollbackRepo.ID); errDelete != nil {
118+
log.Error("Rollback deleteRepository: %v", errDelete)
119+
}
120+
}
121+
109122
return nil, err
110123
}
111124

modules/structs/repo.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ type CreateRepoOption struct {
106106
Description string `json:"description" binding:"MaxSize(255)"`
107107
// Whether the repository is private
108108
Private bool `json:"private"`
109-
// Issue Label set to use
109+
// Label-Set to use
110110
IssueLabels string `json:"issue_labels"`
111111
// Whether the repository should be auto-intialized?
112112
AutoInit bool `json:"auto_init"`

templates/swagger/v1_json.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -12488,7 +12488,7 @@
1248812488
"x-go-name": "Gitignores"
1248912489
},
1249012490
"issue_labels": {
12491-
"description": "Issue Label set to use",
12491+
"description": "Label-Set to use",
1249212492
"type": "string",
1249312493
"x-go-name": "IssueLabels"
1249412494
},

0 commit comments

Comments
 (0)