Add repo_id for attachment (#16958)

When create a new issue or comment and paste/upload an attachment/image, it will not assign an issue id before submit. So if user give up the creating, the attachments will lost key feature and become dirty content. We don't know if we need to delete the attachment even if the repository deleted.

This PR add a repo_id in attachment table so that even if a new upload attachment with no issue_id or release_id but should have repo_id. When deleting a repository, they could also be deleted.

Co-authored-by: 6543 <6543@obermui.de>

1 files changed, 47 insertions, 31 deletions

diff --git a/models/repo.go b/models/repo.go
index 94af318789..23287067e2 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -524,21 +524,6 @@ func (repo *Repository) ComposeDocumentMetas() map[string]string {
 	return repo.DocumentRenderingMetas
 }
 
-// DeleteWiki removes the actual and local copy of repository wiki.
-func (repo *Repository) DeleteWiki() error {
-	return repo.deleteWiki(x)
-}
-
-func (repo *Repository) deleteWiki(e Engine) error {
-	wikiPaths := []string{repo.WikiPath()}
-	for _, wikiPath := range wikiPaths {
-		removeAllWithNotice(e, "Delete repository wiki", wikiPath)
-	}
-
-	_, err := e.Where("repo_id = ?", repo.ID).And("type = ?", UnitTypeWiki).Delete(new(RepoUnit))
-	return err
-}
-
 func (repo *Repository) getAssignees(e Engine) (_ []*User, err error) {
 	if err = repo.getOwner(e); err != nil {
 		return nil, err
@@ -1497,11 +1482,6 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		releaseAttachments = append(releaseAttachments, attachments[i].RelativePath())
 	}
 
-	if _, err = sess.In("release_id", builder.Select("id").From("`release`").Where(builder.Eq{"`release`.repo_id": repoID})).
-		Delete(&Attachment{}); err != nil {
-		return err
-	}
-
 	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
 	}
@@ -1579,21 +1559,13 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		}
 	}
 
-	// FIXME: Remove repository files should be executed after transaction succeed.
-	repoPath := repo.RepoPath()
-	removeAllWithNotice(sess, "Delete repository files", repoPath)
-
-	err = repo.deleteWiki(sess)
-	if err != nil {
-		return err
-	}
-
 	// Remove LFS objects
 	var lfsObjects []*LFSMetaObject
 	if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil {
 		return err
 	}
 
+	var lfsPaths = make([]string, 0, len(lfsObjects))
 	for _, v := range lfsObjects {
 		count, err := sess.Count(&LFSMetaObject{Pointer: lfs.Pointer{Oid: v.Oid}})
 		if err != nil {
@@ -1603,7 +1575,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 			continue
 		}
 
-		removeStorageWithNotice(sess, storage.LFS, "Delete orphaned LFS file", v.RelativePath())
+		lfsPaths = append(lfsPaths, v.RelativePath())
 	}
 
 	if _, err := sess.Delete(&LFSMetaObject{RepositoryID: repoID}); err != nil {
@@ -1616,10 +1588,11 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		return err
 	}
 
+	var archivePaths = make([]string, 0, len(archives))
 	for _, v := range archives {
 		v.Repo = repo
 		p, _ := v.RelativePath()
-		removeStorageWithNotice(sess, storage.RepoArchives, "Delete repo archive file", p)
+		archivePaths = append(archivePaths, p)
 	}
 
 	if _, err := sess.Delete(&RepoArchiver{RepoID: repoID}); err != nil {
@@ -1632,6 +1605,25 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		}
 	}
 
+	// Get all attachments with both issue_id and release_id are zero
+	var newAttachments []*Attachment
+	if err := sess.Where(builder.Eq{
+		"repo_id":    repo.ID,
+		"issue_id":   0,
+		"release_id": 0,
+	}).Find(&newAttachments); err != nil {
+		return err
+	}
+
+	var newAttachmentPaths = make([]string, 0, len(newAttachments))
+	for _, attach := range newAttachments {
+		newAttachmentPaths = append(newAttachmentPaths, attach.RelativePath())
+	}
+
+	if _, err := sess.Where("repo_id=?", repo.ID).Delete(new(Attachment)); err != nil {
+		return err
+	}
+
 	if err = sess.Commit(); err != nil {
 		return err
 	}
@@ -1641,6 +1633,25 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 	// We should always delete the files after the database transaction succeed. If
 	// we delete the file but the database rollback, the repository will be broken.
 
+	// Remove repository files.
+	repoPath := repo.RepoPath()
+	removeAllWithNotice(x, "Delete repository files", repoPath)
+
+	// Remove wiki files
+	if repo.HasWiki() {
+		removeAllWithNotice(x, "Delete repository wiki", repo.WikiPath())
+	}
+
+	// Remove archives
+	for i := range archivePaths {
+		removeStorageWithNotice(x, storage.RepoArchives, "Delete repo archive file", archivePaths[i])
+	}
+
+	// Remove lfs objects
+	for i := range lfsPaths {
+		removeStorageWithNotice(x, storage.LFS, "Delete orphaned LFS file", lfsPaths[i])
+	}
+
 	// Remove issue attachment files.
 	for i := range attachmentPaths {
 		RemoveStorageWithNotice(storage.Attachments, "Delete issue attachment", attachmentPaths[i])
@@ -1651,6 +1662,11 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		RemoveStorageWithNotice(storage.Attachments, "Delete release attachment", releaseAttachments[i])
 	}
 
+	// Remove attachment with no issue_id and release_id.
+	for i := range newAttachmentPaths {
+		RemoveStorageWithNotice(storage.Attachments, "Delete issue attachment", attachmentPaths[i])
+	}
+
 	if len(repo.Avatar) > 0 {
 		if err := storage.RepoAvatars.Delete(repo.CustomAvatarRelativePath()); err != nil {
 			return fmt.Errorf("Failed to remove %s: %v", repo.Avatar, err)