From 4b178e12df769f4be09ab89e860bace7d40d06f0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 11 Dec 2019 14:32:18 +0800 Subject: [PATCH 1/4] Fix bug that release attachment files not deleted when deleting repository --- models/repo.go | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/models/repo.go b/models/repo.go index e809bafa309f1..aaef588bb6529 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1992,6 +1992,17 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } } + releaseAttachments := make([]string, 0, 20) + attachments := make([]*Attachment, 0, len(releaseAttachments)) + if err = sess.Join("INNER", "release", "release.id = attachment.release_id"). + Where("release.repo_id = ?", repoID). + Find(&attachments); err != nil { + return err + } + 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}, @@ -2043,7 +2054,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } attachmentPaths := make([]string, 0, 20) - attachments := make([]*Attachment, 0, len(attachmentPaths)) + attachments = make([]*Attachment, 0, len(attachmentPaths)) if err = sess.Join("INNER", "issue", "issue.id = attachment.issue_id"). Where("issue.repo_id = ?", repoID). Find(&attachments); err != nil { @@ -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,23 @@ 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. + if len(releaseAttachments) > 0 { + for i := range releaseAttachments { + removeAllWithNotice(x, "Delete release attachment", releaseAttachments[i]) + } + } + if len(repo.Avatar) > 0 { avatarPath := repo.CustomAvatarPath() if com.IsExist(avatarPath) { From b4a936b4aea45d6c0a6f418c51dd9dac42302973 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 11 Dec 2019 21:42:46 +0800 Subject: [PATCH 2/4] improve code --- models/repo.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/models/repo.go b/models/repo.go index aaef588bb6529..b9d2309a7de8e 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1993,7 +1993,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } releaseAttachments := make([]string, 0, 20) - attachments := make([]*Attachment, 0, len(releaseAttachments)) + attachments := make([]*Attachment, 0, cap(releaseAttachments)) if err = sess.Join("INNER", "release", "release.id = attachment.release_id"). Where("release.repo_id = ?", repoID). Find(&attachments); err != nil { @@ -2054,7 +2054,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } 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 { @@ -2146,10 +2146,8 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } // Remove release attachment files. - if len(releaseAttachments) > 0 { - for i := range releaseAttachments { - removeAllWithNotice(x, "Delete release attachment", releaseAttachments[i]) - } + for i := range releaseAttachments { + removeAllWithNotice(x, "Delete release attachment", releaseAttachments[i]) } if len(repo.Avatar) > 0 { From ec6ffebf20e5f990ffa1f227bf1048cd9258ddb2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 12 Dec 2019 08:43:26 +0800 Subject: [PATCH 3/4] add quote --- models/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/repo.go b/models/repo.go index b9d2309a7de8e..44743c03057c2 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1994,8 +1994,8 @@ func DeleteRepository(doer *User, uid, repoID int64) error { releaseAttachments := make([]string, 0, 20) attachments := make([]*Attachment, 0, cap(releaseAttachments)) - if err = sess.Join("INNER", "release", "release.id = attachment.release_id"). - Where("release.repo_id = ?", repoID). + if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). + Where("`release`.repo_id = ?", repoID). Find(&attachments); err != nil { return err } From 33990739fb390294db19563e4a70324a51b4b761 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 12 Dec 2019 09:09:16 +0800 Subject: [PATCH 4/4] improve code --- models/repo.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/repo.go b/models/repo.go index 44743c03057c2..d5ea29c5015f1 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1992,13 +1992,13 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } } - releaseAttachments := make([]string, 0, 20) - attachments := make([]*Attachment, 0, cap(releaseAttachments)) + 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()) } @@ -2053,13 +2053,13 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return err } - attachmentPaths := make([]string, 0, 20) 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()) }