From 2686e6bbbe0e2cbb58292732b740a6944ce09c8a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 18 Jan 2021 21:00:50 +0100 Subject: [PATCH] Check if label template exist first (#14384) * add check * refactor * rollback repo on error after session closed --- models/issue_label.go | 10 +++++----- models/repo.go | 30 ++++++++++++------------------ modules/repository/create.go | 29 +++++++++++++++++++++-------- modules/structs/repo.go | 2 +- templates/swagger/v1_json.tmpl | 2 +- 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index ea12b42ae..54b286fe7 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -47,7 +47,7 @@ type Label struct { func GetLabelTemplateFile(name string) ([][3]string, error) { data, err := GetRepoInitFile("label", name) if err != nil { - return nil, fmt.Errorf("GetRepoInitFile: %v", err) + return nil, ErrIssueLabelTemplateLoad{name, fmt.Errorf("GetRepoInitFile: %v", err)} } lines := strings.Split(string(data), "\n") @@ -62,7 +62,7 @@ func GetLabelTemplateFile(name string) ([][3]string, error) { fields := strings.SplitN(parts[0], " ", 2) if len(fields) != 2 { - return nil, fmt.Errorf("line is malformed: %s", line) + return nil, ErrIssueLabelTemplateLoad{name, fmt.Errorf("line is malformed: %s", line)} } color := strings.Trim(fields[0], " ") @@ -70,7 +70,7 @@ func GetLabelTemplateFile(name string) ([][3]string, error) { color = "#" + color } if !LabelColorPattern.MatchString(color) { - return nil, fmt.Errorf("bad HTML color code in line: %s", line) + return nil, ErrIssueLabelTemplateLoad{name, fmt.Errorf("bad HTML color code in line: %s", line)} } var description string @@ -167,7 +167,7 @@ func (label *Label) ForegroundColor() template.CSS { func loadLabels(labelTemplate string) ([]string, error) { list, err := GetLabelTemplateFile(labelTemplate) if err != nil { - return nil, ErrIssueLabelTemplateLoad{labelTemplate, err} + return nil, err } labels := make([]string, len(list)) @@ -186,7 +186,7 @@ func LoadLabelsFormatted(labelTemplate string) (string, error) { func initializeLabels(e Engine, id int64, labelTemplate string, isOrg bool) error { list, err := GetLabelTemplateFile(labelTemplate) if err != nil { - return ErrIssueLabelTemplateLoad{labelTemplate, err} + return err } labels := make([]*Label, len(list)) diff --git a/models/repo.go b/models/repo.go index f2453cc5c..b11671e1f 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1511,26 +1511,27 @@ func UpdateRepositoryUnits(repo *Repository, units []RepoUnit, deleteUnitTypes [ } // DeleteRepository deletes a repository for a user or organization. +// make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock) func DeleteRepository(doer *User, uid, repoID int64) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + // In case is a organization. - org, err := GetUserByID(uid) + org, err := getUserByID(sess, uid) if err != nil { return err } if org.IsOrganization() { - if err = org.GetTeams(&SearchTeamOptions{}); err != nil { + if err = org.getTeams(sess); err != nil { return err } } - sess := x.NewSession() - defer sess.Close() - if err = sess.Begin(); err != nil { - return err - } - - repo := &Repository{ID: repoID, OwnerID: uid} - has, err := sess.Get(repo) + repo := &Repository{OwnerID: uid} + has, err := sess.ID(repoID).Get(repo) if err != nil { return err } else if !has { @@ -1679,14 +1680,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } if err = sess.Commit(); err != nil { - sess.Close() - if len(deployKeys) > 0 { - // We need to rewrite the public keys because the commit failed - if err2 := RewriteAllPublicKeys(); err2 != nil { - return fmt.Errorf("Commit: %v SSH Keys: %v", err, err2) - } - } - return fmt.Errorf("Commit: %v", err) + return err } sess.Close() diff --git a/modules/repository/create.go b/modules/repository/create.go index 1f7145ee2..5eac03836 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -27,6 +27,13 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod opts.DefaultBranch = setting.Repository.DefaultBranch } + // Check if label template exist + if len(opts.IssueLabels) > 0 { + if _, err := models.GetLabelTemplateFile(opts.IssueLabels); err != nil { + return nil, err + } + } + repo := &models.Repository{ OwnerID: u.ID, Owner: u, @@ -45,6 +52,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod TrustModel: opts.TrustModel, } + var rollbackRepo *models.Repository + if err := models.WithTx(func(ctx models.DBContext) error { if err := models.CreateRepository(ctx, doer, u, repo, false); err != nil { return err @@ -76,7 +85,7 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod } } - if err := initRepository(ctx, repoPath, doer, repo, opts); err != nil { + if err = initRepository(ctx, repoPath, doer, repo, opts); err != nil { if err2 := util.RemoveAll(repoPath); err2 != nil { log.Error("initRepository: %v", err) return fmt.Errorf( @@ -87,10 +96,9 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod // Initialize Issue Labels if selected if len(opts.IssueLabels) > 0 { - if err := models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { - if errDelete := models.DeleteRepository(doer, u.ID, repo.ID); errDelete != nil { - log.Error("Rollback deleteRepository: %v", errDelete) - } + if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { + rollbackRepo = repo + rollbackRepo.OwnerID = u.ID return fmt.Errorf("InitializeLabels: %v", err) } } @@ -99,13 +107,18 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)). RunInDir(repoPath); err != nil { log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) - if errDelete := models.DeleteRepository(doer, u.ID, repo.ID); errDelete != nil { - log.Error("Rollback deleteRepository: %v", errDelete) - } + rollbackRepo = repo + rollbackRepo.OwnerID = u.ID return fmt.Errorf("CreateRepository(git update-server-info): %v", err) } return nil }); err != nil { + if rollbackRepo != nil { + if errDelete := models.DeleteRepository(doer, rollbackRepo.OwnerID, rollbackRepo.ID); errDelete != nil { + log.Error("Rollback deleteRepository: %v", errDelete) + } + } + return nil, err } diff --git a/modules/structs/repo.go b/modules/structs/repo.go index 309273d2f..a4eff8b16 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -106,7 +106,7 @@ type CreateRepoOption struct { Description string `json:"description" binding:"MaxSize(255)"` // Whether the repository is private Private bool `json:"private"` - // Issue Label set to use + // Label-Set to use IssueLabels string `json:"issue_labels"` // Whether the repository should be auto-intialized? AutoInit bool `json:"auto_init"` diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6650c09bb..e46d37173 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -12488,7 +12488,7 @@ "x-go-name": "Gitignores" }, "issue_labels": { - "description": "Issue Label set to use", + "description": "Label-Set to use", "type": "string", "x-go-name": "IssueLabels" },