From 5ac54a0024f01bd1296c03c298e21ec18848be22 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 18 Jan 2021 16:21:45 +0100 Subject: [PATCH 1/5] add check --- modules/repository/create.go | 7 +++++++ modules/structs/repo.go | 2 +- templates/swagger/v1_json.tmpl | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/modules/repository/create.go b/modules/repository/create.go index 1f7145ee2a583..d55e5a8058592 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, diff --git a/modules/structs/repo.go b/modules/structs/repo.go index 309273d2fa337..a4eff8b162bf2 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 6650c09bb44b7..e46d37173b426 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" }, From 685e8053cf1d64995a600b26cbf8c281c3ebc194 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 18 Jan 2021 16:22:05 +0100 Subject: [PATCH 2/5] refactor --- models/issue_label.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index ea12b42ae6c05..54b286fe7e027 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)) From 94e0ec188d802088a86d58946f8506f59219b69b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 18 Jan 2021 17:40:17 +0100 Subject: [PATCH 3/5] Dont create second session on Repo Creation --- models/repo.go | 99 ++++++++++++++++++++---------------- modules/repository/create.go | 4 +- 2 files changed, 56 insertions(+), 47 deletions(-) diff --git a/models/repo.go b/models/repo.go index f2453cc5c73a9..9ccf36d3f6b34 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1512,25 +1512,47 @@ func UpdateRepositoryUnits(repo *Repository, units []RepoUnit, deleteUnitTypes [ // DeleteRepository deletes a repository for a user or organization. func DeleteRepository(doer *User, uid, repoID int64) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if err := deleteRepository(sess, doer, uid, repoID); err != nil { + return err + } + + if err := sess.Commit(); err != nil { + sess.Close() + // 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 sess.Close() +} + +// DeleteRepositoryWithContext deletes a repository for a user or organization. +func DeleteRepositoryWithContext(ctx DBContext, doer *User, uid, repoID int64) error { + return deleteRepository(ctx.e, doer, uid, repoID) +} + +func deleteRepository(e Engine, doer *User, uid, repoID int64) error { // In case is a organization. - org, err := GetUserByID(uid) + org, err := getUserByID(e, uid) if err != nil { return err } if org.IsOrganization() { - if err = org.GetTeams(&SearchTeamOptions{}); err != nil { + if err = org.getTeams(e); 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 := e.ID(repoID).Get(repo) if err != nil { return err } else if !has { @@ -1538,17 +1560,17 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } // Delete Deploy Keys - deployKeys, err := listDeployKeys(sess, repo.ID, ListOptions{}) + deployKeys, err := listDeployKeys(e, repo.ID, ListOptions{}) if err != nil { return fmt.Errorf("listDeployKeys: %v", err) } for _, dKey := range deployKeys { - if err := deleteDeployKey(sess, doer, dKey.ID); err != nil { + if err := deleteDeployKey(e, doer, dKey.ID); err != nil { return fmt.Errorf("deleteDeployKeys: %v", err) } } - if cnt, err := sess.ID(repoID).Delete(&Repository{}); err != nil { + if cnt, err := e.ID(repoID).Delete(&Repository{}); err != nil { return err } else if cnt != 1 { return ErrRepoNotExist{repoID, uid, "", ""} @@ -1556,16 +1578,16 @@ func DeleteRepository(doer *User, uid, repoID int64) error { if org.IsOrganization() { for _, t := range org.Teams { - if !t.hasRepository(sess, repoID) { + if !t.hasRepository(e, repoID) { continue - } else if err = t.removeRepository(sess, repo, false); err != nil { + } else if err = t.removeRepository(e, repo, false); err != nil { return err } } } attachments := make([]*Attachment, 0, 20) - if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). + if err = e.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). Where("`release`.repo_id = ?", repoID). Find(&attachments); err != nil { return err @@ -1575,11 +1597,11 @@ func DeleteRepository(doer *User, uid, repoID int64) error { releaseAttachments = append(releaseAttachments, attachments[i].RelativePath()) } - if _, err = sess.Exec("UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { + if _, err = e.Exec("UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { return err } - if err = deleteBeans(sess, + if err = deleteBeans(e, &Access{RepoID: repo.ID}, &Action{RepoID: repo.ID}, &Watch{RepoID: repoID}, @@ -1605,59 +1627,59 @@ func DeleteRepository(doer *User, uid, repoID int64) error { // Delete Issues and related objects var attachmentPaths []string - if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil { + if attachmentPaths, err = deleteIssuesByRepoID(e, repoID); err != nil { return err } - if _, err = sess.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil { + if _, err = e.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil { return err } if repo.IsFork { - if _, err = sess.Exec("UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { + if _, err = e.Exec("UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { return fmt.Errorf("decrease fork count: %v", err) } } - if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil { + if _, err = e.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil { return err } if len(repo.Topics) > 0 { - if err = removeTopicsFromRepo(sess, repo.ID); err != nil { + if err = removeTopicsFromRepo(e, repo.ID); err != nil { return err } } - projects, _, err := getProjects(sess, ProjectSearchOptions{ + projects, _, err := getProjects(e, ProjectSearchOptions{ RepoID: repoID, }) if err != nil { return fmt.Errorf("get projects: %v", err) } for i := range projects { - if err := deleteProjectByID(sess, projects[i].ID); err != nil { + if err := deleteProjectByID(e, projects[i].ID); err != nil { return fmt.Errorf("delete project [%d]: %v", projects[i].ID, err) } } // FIXME: Remove repository files should be executed after transaction succeed. repoPath := repo.RepoPath() - removeAllWithNotice(sess, "Delete repository files", repoPath) + removeAllWithNotice(e, "Delete repository files", repoPath) - err = repo.deleteWiki(sess) + err = repo.deleteWiki(e) if err != nil { return err } // Remove LFS objects var lfsObjects []*LFSMetaObject - if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { + if err = e.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { return err } for _, v := range lfsObjects { - count, err := sess.Count(&LFSMetaObject{Oid: v.Oid}) + count, err := e.Count(&LFSMetaObject{Oid: v.Oid}) if err != nil { return err } @@ -1665,32 +1687,19 @@ func DeleteRepository(doer *User, uid, repoID int64) error { continue } - removeStorageWithNotice(sess, storage.LFS, "Delete orphaned LFS file", v.RelativePath()) + removeStorageWithNotice(e, storage.LFS, "Delete orphaned LFS file", v.RelativePath()) } - if _, err := sess.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil { + if _, err := e.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil { return err } if repo.NumForks > 0 { - if _, err = sess.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { + if _, err = e.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { log.Error("reset 'fork_id' and 'is_fork': %v", err) } } - 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) - } - - sess.Close() - // We should always delete the files after the database transaction succeed. If // we delete the file but the database rollback, the repository will be borken. diff --git a/modules/repository/create.go b/modules/repository/create.go index d55e5a8058592..b7dc5e248a64d 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -95,7 +95,7 @@ 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 { + if errDelete := models.DeleteRepositoryWithContext(ctx, doer, u.ID, repo.ID); errDelete != nil { log.Error("Rollback deleteRepository: %v", errDelete) } return fmt.Errorf("InitializeLabels: %v", err) @@ -106,7 +106,7 @@ 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 { + if errDelete := models.DeleteRepositoryWithContext(ctx, doer, u.ID, repo.ID); errDelete != nil { log.Error("Rollback deleteRepository: %v", errDelete) } return fmt.Errorf("CreateRepository(git update-server-info): %v", err) From a570749a1dca82968d4ee8d1a9346829aa123365 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 18 Jan 2021 17:59:44 +0100 Subject: [PATCH 4/5] as per @lunny & 2nits --- models/repo.go | 11 +---------- modules/repository/create.go | 4 ++-- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/models/repo.go b/models/repo.go index 9ccf36d3f6b34..49fd3044efaea 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1522,16 +1522,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return err } - if err := sess.Commit(); err != nil { - sess.Close() - // 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 sess.Close() + return sess.Commit() } // DeleteRepositoryWithContext deletes a repository for a user or organization. diff --git a/modules/repository/create.go b/modules/repository/create.go index b7dc5e248a64d..47754ae009fcd 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -83,7 +83,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( @@ -94,7 +94,7 @@ 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 err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { if errDelete := models.DeleteRepositoryWithContext(ctx, doer, u.ID, repo.ID); errDelete != nil { log.Error("Rollback deleteRepository: %v", errDelete) } From 0f52aa862818d96cdc5efe6f5940c36e132ff5b0 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 18 Jan 2021 19:25:11 +0100 Subject: [PATCH 5/5] rollback repo on error after session closed --- models/repo.go | 70 +++++++++++++++++------------------- modules/repository/create.go | 18 ++++++---- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/models/repo.go b/models/repo.go index 49fd3044efaea..b11671e1fc09b 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1511,6 +1511,7 @@ 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() @@ -1518,32 +1519,19 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return err } - if err := deleteRepository(sess, doer, uid, repoID); err != nil { - return err - } - - return sess.Commit() -} - -// DeleteRepositoryWithContext deletes a repository for a user or organization. -func DeleteRepositoryWithContext(ctx DBContext, doer *User, uid, repoID int64) error { - return deleteRepository(ctx.e, doer, uid, repoID) -} - -func deleteRepository(e Engine, doer *User, uid, repoID int64) error { // In case is a organization. - org, err := getUserByID(e, uid) + org, err := getUserByID(sess, uid) if err != nil { return err } if org.IsOrganization() { - if err = org.getTeams(e); err != nil { + if err = org.getTeams(sess); err != nil { return err } } repo := &Repository{OwnerID: uid} - has, err := e.ID(repoID).Get(repo) + has, err := sess.ID(repoID).Get(repo) if err != nil { return err } else if !has { @@ -1551,17 +1539,17 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error { } // Delete Deploy Keys - deployKeys, err := listDeployKeys(e, repo.ID, ListOptions{}) + deployKeys, err := listDeployKeys(sess, repo.ID, ListOptions{}) if err != nil { return fmt.Errorf("listDeployKeys: %v", err) } for _, dKey := range deployKeys { - if err := deleteDeployKey(e, doer, dKey.ID); err != nil { + if err := deleteDeployKey(sess, doer, dKey.ID); err != nil { return fmt.Errorf("deleteDeployKeys: %v", err) } } - if cnt, err := e.ID(repoID).Delete(&Repository{}); err != nil { + if cnt, err := sess.ID(repoID).Delete(&Repository{}); err != nil { return err } else if cnt != 1 { return ErrRepoNotExist{repoID, uid, "", ""} @@ -1569,16 +1557,16 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error { if org.IsOrganization() { for _, t := range org.Teams { - if !t.hasRepository(e, repoID) { + if !t.hasRepository(sess, repoID) { continue - } else if err = t.removeRepository(e, repo, false); err != nil { + } else if err = t.removeRepository(sess, repo, false); err != nil { return err } } } attachments := make([]*Attachment, 0, 20) - if err = e.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). + if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). Where("`release`.repo_id = ?", repoID). Find(&attachments); err != nil { return err @@ -1588,11 +1576,11 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error { releaseAttachments = append(releaseAttachments, attachments[i].RelativePath()) } - if _, err = e.Exec("UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { + if _, err = sess.Exec("UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { return err } - if err = deleteBeans(e, + if err = deleteBeans(sess, &Access{RepoID: repo.ID}, &Action{RepoID: repo.ID}, &Watch{RepoID: repoID}, @@ -1618,59 +1606,59 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error { // Delete Issues and related objects var attachmentPaths []string - if attachmentPaths, err = deleteIssuesByRepoID(e, repoID); err != nil { + if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil { return err } - if _, err = e.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil { + if _, err = sess.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil { return err } if repo.IsFork { - if _, err = e.Exec("UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { + if _, err = sess.Exec("UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { return fmt.Errorf("decrease fork count: %v", err) } } - if _, err = e.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil { + if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil { return err } if len(repo.Topics) > 0 { - if err = removeTopicsFromRepo(e, repo.ID); err != nil { + if err = removeTopicsFromRepo(sess, repo.ID); err != nil { return err } } - projects, _, err := getProjects(e, ProjectSearchOptions{ + projects, _, err := getProjects(sess, ProjectSearchOptions{ RepoID: repoID, }) if err != nil { return fmt.Errorf("get projects: %v", err) } for i := range projects { - if err := deleteProjectByID(e, projects[i].ID); err != nil { + if err := deleteProjectByID(sess, projects[i].ID); err != nil { return fmt.Errorf("delete project [%d]: %v", projects[i].ID, err) } } // FIXME: Remove repository files should be executed after transaction succeed. repoPath := repo.RepoPath() - removeAllWithNotice(e, "Delete repository files", repoPath) + removeAllWithNotice(sess, "Delete repository files", repoPath) - err = repo.deleteWiki(e) + err = repo.deleteWiki(sess) if err != nil { return err } // Remove LFS objects var lfsObjects []*LFSMetaObject - if err = e.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { + if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { return err } for _, v := range lfsObjects { - count, err := e.Count(&LFSMetaObject{Oid: v.Oid}) + count, err := sess.Count(&LFSMetaObject{Oid: v.Oid}) if err != nil { return err } @@ -1678,19 +1666,25 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error { continue } - removeStorageWithNotice(e, storage.LFS, "Delete orphaned LFS file", v.RelativePath()) + removeStorageWithNotice(sess, storage.LFS, "Delete orphaned LFS file", v.RelativePath()) } - if _, err := e.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil { + if _, err := sess.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil { return err } if repo.NumForks > 0 { - if _, err = e.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { + if _, err = sess.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { log.Error("reset 'fork_id' and 'is_fork': %v", err) } } + if err = sess.Commit(); err != nil { + return err + } + + sess.Close() + // We should always delete the files after the database transaction succeed. If // we delete the file but the database rollback, the repository will be borken. diff --git a/modules/repository/create.go b/modules/repository/create.go index 47754ae009fcd..5eac03836e1bb 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -52,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 @@ -95,9 +97,8 @@ 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.DeleteRepositoryWithContext(ctx, doer, u.ID, repo.ID); errDelete != nil { - log.Error("Rollback deleteRepository: %v", errDelete) - } + rollbackRepo = repo + rollbackRepo.OwnerID = u.ID return fmt.Errorf("InitializeLabels: %v", err) } } @@ -106,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.DeleteRepositoryWithContext(ctx, 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 }