From eb8d5f6affd4d96c375e354534ecd3faeaffaa97 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 12 Dec 2019 15:05:44 +0800 Subject: [PATCH] Fix bug that release attachment files not deleted when deleting repository (#9322) (#9329) * Fix bug that release attachment files not deleted when deleting repository * improve code * add quote * improve code --- models/repo.go | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/models/repo.go b/models/repo.go index 8875c96376..2d31184d6d 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1860,6 +1860,17 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } } + attachments := make([]*Attachment, 0, 20) + if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). + Where("`release`.repo_id = ?", repoID). + Find(&attachments); err != nil { + return err + } + releaseAttachments := make([]string, 0, len(attachments)) + for i := 0; i < len(attachments); i++ { + releaseAttachments = append(releaseAttachments, attachments[i].LocalPath()) + } + if err = deleteBeans(sess, &Access{RepoID: repo.ID}, &Action{RepoID: repo.ID}, @@ -1910,13 +1921,13 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return err } - attachmentPaths := make([]string, 0, 20) - attachments := make([]*Attachment, 0, len(attachmentPaths)) + attachments = attachments[:0] if err = sess.Join("INNER", "issue", "issue.id = attachment.issue_id"). Where("issue.repo_id = ?", repoID). Find(&attachments); err != nil { return err } + attachmentPaths := make([]string, 0, len(attachments)) for j := range attachments { attachmentPaths = append(attachmentPaths, attachments[j].LocalPath()) } @@ -1953,11 +1964,6 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return err } - // Remove attachment files. - for i := range attachmentPaths { - removeAllWithNotice(sess, "Delete attachment", attachmentPaths[i]) - } - // Remove LFS objects var lfsObjects []*LFSMetaObject if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { @@ -1997,6 +2003,8 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return fmt.Errorf("Commit: %v", err) } + sess.Close() + if org.IsOrganization() { if err = PrepareWebhooks(repo, HookEventRepository, &api.RepositoryPayload{ Action: api.HookRepoDeleted, @@ -2009,6 +2017,19 @@ func DeleteRepository(doer *User, uid, repoID int64) error { go HookQueue.Add(repo.ID) } + // 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. + + // Remove issue attachment files. + for i := range attachmentPaths { + removeAllWithNotice(x, "Delete issue attachment", attachmentPaths[i]) + } + + // Remove release attachment files. + for i := range releaseAttachments { + removeAllWithNotice(x, "Delete release attachment", releaseAttachments[i]) + } + if len(repo.Avatar) > 0 { avatarPath := repo.CustomAvatarPath() if com.IsExist(avatarPath) {