Skip to content

Commit 0f52aa8

Browse files
65436543
committed
rollback repo on error after session closed
1 parent a570749 commit 0f52aa8

File tree

2 files changed

+44
-44
lines changed

2 files changed

+44
-44
lines changed

‎models/repo.go

Lines changed: 32 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,74 +1511,62 @@ 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 {
15151516
sess := x.NewSession()
15161517
defer sess.Close()
15171518
if err := sess.Begin(); err != nil {
15181519
return err
15191520
}
15201521

1521-
if err := deleteRepository(sess, doer, uid, repoID); err != nil {
1522-
return err
1523-
}
1524-
1525-
return sess.Commit()
1526-
}
1527-
1528-
// DeleteRepositoryWithContext deletes a repository for a user or organization.
1529-
func DeleteRepositoryWithContext(ctx DBContext, doer *User, uid, repoID int64) error {
1530-
return deleteRepository(ctx.e, doer, uid, repoID)
1531-
}
1532-
1533-
func deleteRepository(e Engine, doer *User, uid, repoID int64) error {
15341522
// In case is a organization.
1535-
org, err := getUserByID(e, uid)
1523+
org, err := getUserByID(sess, uid)
15361524
if err != nil {
15371525
return err
15381526
}
15391527
if org.IsOrganization() {
1540-
if err = org.getTeams(e); err != nil {
1528+
if err = org.getTeams(sess); err != nil {
15411529
return err
15421530
}
15431531
}
15441532

15451533
repo := &Repository{OwnerID: uid}
1546-
has, err := e.ID(repoID).Get(repo)
1534+
has, err := sess.ID(repoID).Get(repo)
15471535
if err != nil {
15481536
return err
15491537
} else if !has {
15501538
return ErrRepoNotExist{repoID, uid, "", ""}
15511539
}
15521540

15531541
// Delete Deploy Keys
1554-
deployKeys, err := listDeployKeys(e, repo.ID, ListOptions{})
1542+
deployKeys, err := listDeployKeys(sess, repo.ID, ListOptions{})
15551543
if err != nil {
15561544
return fmt.Errorf("listDeployKeys: %v", err)
15571545
}
15581546
for _, dKey := range deployKeys {
1559-
if err := deleteDeployKey(e, doer, dKey.ID); err != nil {
1547+
if err := deleteDeployKey(sess, doer, dKey.ID); err != nil {
15601548
return fmt.Errorf("deleteDeployKeys: %v", err)
15611549
}
15621550
}
15631551

1564-
if cnt, err := e.ID(repoID).Delete(&Repository{}); err != nil {
1552+
if cnt, err := sess.ID(repoID).Delete(&Repository{}); err != nil {
15651553
return err
15661554
} else if cnt != 1 {
15671555
return ErrRepoNotExist{repoID, uid, "", ""}
15681556
}
15691557

15701558
if org.IsOrganization() {
15711559
for _, t := range org.Teams {
1572-
if !t.hasRepository(e, repoID) {
1560+
if !t.hasRepository(sess, repoID) {
15731561
continue
1574-
} else if err = t.removeRepository(e, repo, false); err != nil {
1562+
} else if err = t.removeRepository(sess, repo, false); err != nil {
15751563
return err
15761564
}
15771565
}
15781566
}
15791567

15801568
attachments := make([]*Attachment, 0, 20)
1581-
if err = e.Join("INNER", "`release`", "`release`.id = `attachment`.release_id").
1569+
if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id").
15821570
Where("`release`.repo_id = ?", repoID).
15831571
Find(&attachments); err != nil {
15841572
return err
@@ -1588,11 +1576,11 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error {
15881576
releaseAttachments = append(releaseAttachments, attachments[i].RelativePath())
15891577
}
15901578

1591-
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 {
1579+
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 {
15921580
return err
15931581
}
15941582

1595-
if err = deleteBeans(e,
1583+
if err = deleteBeans(sess,
15961584
&Access{RepoID: repo.ID},
15971585
&Action{RepoID: repo.ID},
15981586
&Watch{RepoID: repoID},
@@ -1618,79 +1606,85 @@ func deleteRepository(e Engine, doer *User, uid, repoID int64) error {
16181606

16191607
// Delete Issues and related objects
16201608
var attachmentPaths []string
1621-
if attachmentPaths, err = deleteIssuesByRepoID(e, repoID); err != nil {
1609+
if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil {
16221610
return err
16231611
}
16241612

1625-
if _, err = e.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil {
1613+
if _, err = sess.Where("repo_id = ?", repoID).Delete(new(RepoUnit)); err != nil {
16261614
return err
16271615
}
16281616

16291617
if repo.IsFork {
1630-
if _, err = e.Exec("UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil {
1618+
if _, err = sess.Exec("UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil {
16311619
return fmt.Errorf("decrease fork count: %v", err)
16321620
}
16331621
}
16341622

1635-
if _, err = e.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil {
1623+
if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil {
16361624
return err
16371625
}
16381626

16391627
if len(repo.Topics) > 0 {
1640-
if err = removeTopicsFromRepo(e, repo.ID); err != nil {
1628+
if err = removeTopicsFromRepo(sess, repo.ID); err != nil {
16411629
return err
16421630
}
16431631
}
16441632

1645-
projects, _, err := getProjects(e, ProjectSearchOptions{
1633+
projects, _, err := getProjects(sess, ProjectSearchOptions{
16461634
RepoID: repoID,
16471635
})
16481636
if err != nil {
16491637
return fmt.Errorf("get projects: %v", err)
16501638
}
16511639
for i := range projects {
1652-
if err := deleteProjectByID(e, projects[i].ID); err != nil {
1640+
if err := deleteProjectByID(sess, projects[i].ID); err != nil {
16531641
return fmt.Errorf("delete project [%d]: %v", projects[i].ID, err)
16541642
}
16551643
}
16561644

16571645
// FIXME: Remove repository files should be executed after transaction succeed.
16581646
repoPath := repo.RepoPath()
1659-
removeAllWithNotice(e, "Delete repository files", repoPath)
1647+
removeAllWithNotice(sess, "Delete repository files", repoPath)
16601648

1661-
err = repo.deleteWiki(e)
1649+
err = repo.deleteWiki(sess)
16621650
if err != nil {
16631651
return err
16641652
}
16651653

16661654
// Remove LFS objects
16671655
var lfsObjects []*LFSMetaObject
1668-
if err = e.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil {
1656+
if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil {
16691657
return err
16701658
}
16711659

16721660
for _, v := range lfsObjects {
1673-
count, err := e.Count(&LFSMetaObject{Oid: v.Oid})
1661+
count, err := sess.Count(&LFSMetaObject{Oid: v.Oid})
16741662
if err != nil {
16751663
return err
16761664
}
16771665
if count > 1 {
16781666
continue
16791667
}
16801668

1681-
removeStorageWithNotice(e, storage.LFS, "Delete orphaned LFS file", v.RelativePath())
1669+
removeStorageWithNotice(sess, storage.LFS, "Delete orphaned LFS file", v.RelativePath())
16821670
}
16831671

1684-
if _, err := e.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil {
1672+
if _, err := sess.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil {
16851673
return err
16861674
}
16871675

16881676
if repo.NumForks > 0 {
1689-
if _, err = e.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil {
1677+
if _, err = sess.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil {
16901678
log.Error("reset 'fork_id' and 'is_fork': %v", err)
16911679
}
16921680
}
16931681

1682+
if err = sess.Commit(); err != nil {
1683+
return err
1684+
}
1685+
1686+
sess.Close()
1687+
16941688
// We should always delete the files after the database transaction succeed. If
16951689
// we delete the file but the database rollback, the repository will be borken.
16961690

‎modules/repository/create.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
5252
TrustModel: opts.TrustModel,
5353
}
5454

55+
var rollbackRepo *models.Repository
56+
5557
if err := models.WithTx(func(ctx models.DBContext) error {
5658
if err := models.CreateRepository(ctx, doer, u, repo, false); err != nil {
5759
return err
@@ -95,9 +97,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
9597
// Initialize Issue Labels if selected
9698
if len(opts.IssueLabels) > 0 {
9799
if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil {
98-
if errDelete := models.DeleteRepositoryWithContext(ctx, doer, u.ID, repo.ID); errDelete != nil {
99-
log.Error("Rollback deleteRepository: %v", errDelete)
100-
}
100+
rollbackRepo = repo
101+
rollbackRepo.OwnerID = u.ID
101102
return fmt.Errorf("InitializeLabels: %v", err)
102103
}
103104
}
@@ -106,13 +107,18 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod
106107
SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)).
107108
RunInDir(repoPath); err != nil {
108109
log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
109-
if errDelete := models.DeleteRepositoryWithContext(ctx, doer, u.ID, repo.ID); errDelete != nil {
110-
log.Error("Rollback deleteRepository: %v", errDelete)
111-
}
110+
rollbackRepo = repo
111+
rollbackRepo.OwnerID = u.ID
112112
return fmt.Errorf("CreateRepository(git update-server-info): %v", err)
113113
}
114114
return nil
115115
}); 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+
116122
return nil, err
117123
}
118124

0 commit comments

Comments
 (0)