Add repo_id for attachment

models/attachment.go

@@ -5,25 +5,23 @@
 package models
 
 import (
-	"bytes"
 	"fmt"
-	"io"
 	"path"
 
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/timeutil"
 
-	gouuid "github.com/google/uuid"
 	"xorm.io/xorm"
 )
 
 // Attachment represent a attachment of issue/comment/release.
 type Attachment struct {
 	ID            int64  `xorm:"pk autoincr"`
 	UUID          string `xorm:"uuid UNIQUE"`
-	IssueID       int64  `xorm:"INDEX"`
-	ReleaseID     int64  `xorm:"INDEX"`
+	RepoID        int64  `xorm:"INDEX"`           // this should not be zero
+	IssueID       int64  `xorm:"INDEX"`           // maybe zero when creating
+	ReleaseID     int64  `xorm:"INDEX"`           // maybe zero when creating
 	UploaderID    int64  `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added
 	CommentID     int64
 	Name          string
@@ -81,23 +79,6 @@ func (a *Attachment) LinkedRepository() (*Repository, UnitType, error) {
 	return nil, -1, nil
 }
 
-// NewAttachment creates a new attachment object.
-func NewAttachment(attach *Attachment, buf []byte, file io.Reader) (_ *Attachment, err error) {
-	attach.UUID = gouuid.New().String()
-
-	size, err := storage.Attachments.Save(attach.RelativePath(), io.MultiReader(bytes.NewReader(buf), file), -1)
-	if err != nil {
-		return nil, fmt.Errorf("Create: %v", err)
-	}
-	attach.Size = size
-
-	if _, err := x.Insert(attach); err != nil {
-		return nil, err
-	}
-
-	return attach, nil
-}
-
 // GetAttachmentByID returns attachment by given id
 func GetAttachmentByID(id int64) (*Attachment, error) {
 	return getAttachmentByID(x, id)

models/attachment_test.go

@@ -5,40 +5,11 @@
 package models
 
 import (
-	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestUploadAttachment(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
-
-	user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
-
-	fPath := "./attachment_test.go"
-	f, err := os.Open(fPath)
-	assert.NoError(t, err)
-	defer f.Close()
-
-	buf := make([]byte, 1024)
-	n, err := f.Read(buf)
-	assert.NoError(t, err)
-	buf = buf[:n]
-
-	attach, err := NewAttachment(&Attachment{
-		UploaderID: user.ID,
-		Name:       filepath.Base(fPath),
-	}, buf, f)
-	assert.NoError(t, err)
-
-	attachment, err := GetAttachmentByUUID(attach.UUID)
-	assert.NoError(t, err)
-	assert.EqualValues(t, user.ID, attachment.UploaderID)
-	assert.Equal(t, int64(0), attachment.DownloadCount)
-}
-
 func TestIncreaseDownloadCount(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 

models/context.go

@@ -66,3 +66,9 @@ func Iterate(ctx DBContext, tableBean interface{}, cond builder.Cond, fun func(i
 		BufferSize(setting.Database.IterateBufferSize).
 		Iterate(tableBean, fun)
 }
+
+// Insert inserts records into database
+func Insert(ctx DBContext, beans ...interface{}) error {
+	_, err := ctx.e.Insert(beans...)
+	return err
+}

models/migrations/migrations.go

@@ -338,6 +338,8 @@ var migrations = []Migration{
 	NewMigration("Alter issue/comment table TEXT fields to LONGTEXT", alterIssueAndCommentTextFieldsToLongText),
 	// v192 -> v193
 	NewMigration("RecreateIssueResourceIndexTable to have a primary key instead of an unique index", recreateIssueResourceIndexTable),
+	// v193 -> v194
+	NewMigration("Add repo id column for attachment table", addRepoIDForAttachment),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v193.go

@@ -0,0 +1,33 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"xorm.io/xorm"
+)
+
+func addRepoIDForAttachment(x *xorm.Engine) error {
+	type Attachment struct {
+		ID         int64  `xorm:"pk autoincr"`
+		UUID       string `xorm:"uuid UNIQUE"`
+		RepoID     int64  `xorm:"INDEX"` // this should not be zero
+		IssueID    int64  `xorm:"INDEX"` // maybe zero when creating
+		ReleaseID  int64  `xorm:"INDEX"` // maybe zero when creating
+		UploaderID int64  `xorm:"INDEX DEFAULT 0"`
+	}
+	if err := x.Sync2(new(Attachment)); err != nil {
+		return err
+	}
+
+	if _, err := x.Exec("UPDATE attachment set repo_id = (SELECT repo_id FROM issue WHERE issue.id = issue_id) WHERE issue_id > 0"); err != nil {
+		return err
+	}
+
+	if _, err := x.Exec("UPDATE attachment set repo_id = (SELECT repo_id FROM release WHERE release.id = release_id) WHERE release_id > 0"); err != nil {
+		return err
+	}
+
+	return nil
+}

models/migrations/v193_test.go

@@ -0,0 +1,72 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_addRepoIDForAttachment(t *testing.T) {
+	type Attachment struct {
+		ID         int64  `xorm:"pk autoincr"`
+		UUID       string `xorm:"uuid UNIQUE"`
+		RepoID     int64  `xorm:"INDEX"` // this should not be zero
+		IssueID    int64  `xorm:"INDEX"` // maybe zero when creating
+		ReleaseID  int64  `xorm:"INDEX"` // maybe zero when creating
+		UploaderID int64  `xorm:"INDEX DEFAULT 0"`
+	}
+
+	// Prepare and load the testing database
+	x, deferable := prepareTestEnv(t, 0, new(Attachment))
+	if x == nil || t.Failed() {
+		defer deferable()
+		return
+	}
+	defer deferable()
+
+	// Run the migration
+	if err := addRepoIDForAttachment(x); err != nil {
+		assert.NoError(t, err)
+		return
+	}
+
+	type Issue struct {
+		ID     int64
+		RepoID int64
+	}
+
+	var issueAttachments []*Attachment
+	err := x.Where("issue_id > 0").Find(&issueAttachments)
+	assert.NoError(t, err)
+	for _, attach := range issueAttachments {
+		assert.Greater(t, attach.RepoID, 0)
+		assert.Greater(t, attach.IssueID, 0)
+		var issue Issue
+		has, err := x.ID(attach.IssueID).Get(&issue)
+		assert.NoError(t, err)
+		assert.True(t, has)
+		assert.EqualValues(t, attach.RepoID, issue.RepoID)
+	}
+
+	type Release struct {
+		ID     int64
+		RepoID int64
+	}
+
+	var releaseAttachments []*Attachment
+	err = x.Where("release_id > 0").Find(&releaseAttachments)
+	assert.NoError(t, err)
+	for _, attach := range releaseAttachments {
+		assert.Greater(t, attach.RepoID, 0)
+		assert.Greater(t, attach.IssueID, 0)
+		var release Release
+		has, err := x.ID(attach.ReleaseID).Get(&release)
+		assert.NoError(t, err)
+		assert.True(t, has)
+		assert.EqualValues(t, attach.RepoID, release.RepoID)
+	}
+}

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)