From 751cfb805ddddbb4242583db2a71d8d3ed00f9d7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 12 Dec 2019 13:31:05 +0800 Subject: [PATCH] Fix bug that release attachment files not deleted when deleting repository (#9322) * 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 e809bafa309..d5ea29c5015 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1992,6 +1992,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}, @@ -2042,13 +2053,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()) } @@ -2085,11 +2096,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 { @@ -2129,6 +2135,21 @@ func DeleteRepository(doer *User, uid, repoID int64) error { 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. + + // 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) {