Nulo/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Add repo_id for attachment (#16958)

Browse source 
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>
This commit is contained in:
Lunny Xiao 2021-09-08 23:19:30 +08:00 committed by GitHub
parent f55cd033ed
commit ddc709ff7f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 306 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
models/attachment.go
View file

@ -5,16 +5,13 @@
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"
)
@ -22,8 +19,9 @@ import (
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)

29
models/attachment_test.go
View file

@ -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())

6
models/context.go
View file

@ -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
}

11
models/fixtures/attachment.yml
View file

@ -1,6 +1,7 @@
-
id: 1
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11
repo_id: 1
issue_id: 1
comment_id: 0
name: attach1
@ -10,6 +11,7 @@
-
id: 2
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12
repo_id: 2
issue_id: 4
comment_id: 0
name: attach2
@ -19,6 +21,7 @@
-
id: 3
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a13
repo_id: 1
issue_id: 2
comment_id: 1
name: attach1
@ -28,6 +31,7 @@
-
id: 4
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14
repo_id: 1
issue_id: 3
comment_id: 1
name: attach2
@ -37,6 +41,7 @@
-
id: 5
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a15
repo_id: 2
issue_id: 4
comment_id: 0
name: attach1
@ -46,6 +51,7 @@
-
id: 6
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a16
repo_id: 1
issue_id: 5
comment_id: 2
name: attach1
@ -55,6 +61,7 @@
-
id: 7
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17
repo_id: 1
issue_id: 5
comment_id: 2
name: attach1
@ -64,6 +71,7 @@
-
id: 8
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18
repo_id: 3
issue_id: 6
comment_id: 0
name: attach1
@ -73,6 +81,7 @@
-
id: 9
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a19
repo_id: 1
release_id: 1
name: attach1
download_count: 0
@ -81,6 +90,7 @@
-
id: 10
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20
repo_id: 0 # TestGetAttachment/NotLinked
uploader_id: 8
name: attach1
download_count: 0
@ -89,6 +99,7 @@
-
id: 11
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a21
repo_id: 40
release_id: 2
name: attach1
download_count: 0

2
models/migrations/migrations.go
View file

@ -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

33
models/migrations/v193.go Normal file
View file

@ -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 = `attachment`.issue_id) WHERE `attachment`.issue_id > 0"); err != nil {
return err
}
if _, err := x.Exec("UPDATE `attachment` set repo_id = (SELECT repo_id FROM `release` WHERE `release`.id = `attachment`.release_id) WHERE `attachment`.release_id > 0"); err != nil {
return err
}
return nil
}

71
models/migrations/v193_test.go Normal file
View file

@ -0,0 +1,71 @@
// 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"`
}
type Issue struct {
ID int64
RepoID int64
}
type Release struct {
ID int64
RepoID int64
}
// Prepare and load the testing database
x, deferrable := prepareTestEnv(t, 0, new(Attachment), new(Issue), new(Release))
defer deferrable()
if x == nil || t.Failed() {
return
}
// Run the migration
if err := addRepoIDForAttachment(x); err != nil {
assert.NoError(t, err)
return
}
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)
}
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)
}
}

78
models/repo.go
View file

@ -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)

24
routers/api/v1/repo/release_attachment.go
View file

@ -15,6 +15,7 @@ import (
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/attachment"
)
// GetReleaseAttachment gets a single attachment of the release
@ -176,31 +177,18 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
}
defer file.Close()
buf := make([]byte, 1024)
n, _ := file.Read(buf)
if n > 0 {
buf = buf[:n]
}
// Check if the filetype is allowed by the settings
err = upload.Verify(buf, header.Filename, setting.Repository.Release.AllowedTypes)
if err != nil {
ctx.Error(http.StatusBadRequest, "DetectContentType", err)
return
}
var filename = header.Filename
if query := ctx.FormString("name"); query != "" {
filename = query
}
// Create a new attachment and save the file
attach, err := models.NewAttachment(&models.Attachment{
UploaderID: ctx.User.ID,
Name: filename,
ReleaseID: release.ID,
}, buf, file)
attach, err := attachment.UploadAttachment(file, ctx.User.ID, release.RepoID, releaseID, filename, setting.Repository.Release.AllowedTypes)
if err != nil {
if upload.IsErrFileTypeForbidden(err) {
ctx.Error(http.StatusBadRequest, "DetectContentType", err)
return
}
ctx.Error(http.StatusInternalServerError, "NewAttachment", err)
return
}

22
routers/web/repo/attachment.go
View file

@ -16,20 +16,21 @@ import (
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/routers/common"
"code.gitea.io/gitea/services/attachment"
)
// UploadIssueAttachment response for Issue/PR attachments
func UploadIssueAttachment(ctx *context.Context) {
uploadAttachment(ctx, setting.Attachment.AllowedTypes)
uploadAttachment(ctx, ctx.Repo.Repository.ID, setting.Attachment.AllowedTypes)
}
// UploadReleaseAttachment response for uploading release attachments
func UploadReleaseAttachment(ctx *context.Context) {
uploadAttachment(ctx, setting.Repository.Release.AllowedTypes)
uploadAttachment(ctx, ctx.Repo.Repository.ID, setting.Repository.Release.AllowedTypes)
}
// UploadAttachment response for uploading attachments
func uploadAttachment(ctx *context.Context, allowedTypes string) {
func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) {
if !setting.Attachment.Enabled {
ctx.Error(http.StatusNotFound, "attachment is not enabled")
return
@ -42,23 +43,12 @@ func uploadAttachment(ctx *context.Context, allowedTypes string) {
}
defer file.Close()
buf := make([]byte, 1024)
n, _ := file.Read(buf)
if n > 0 {
buf = buf[:n]
}
err = upload.Verify(buf, header.Filename, allowedTypes)
attach, err := attachment.UploadAttachment(file, ctx.User.ID, repoID, 0, header.Filename, allowedTypes)
if err != nil {
if upload.IsErrFileTypeForbidden(err) {
ctx.Error(http.StatusBadRequest, err.Error())
return
}
attach, err := models.NewAttachment(&models.Attachment{
UploaderID: ctx.User.ID,
Name: header.Filename,
}, buf, file)
if err != nil {
ctx.Error(http.StatusInternalServerError, fmt.Sprintf("NewAttachment: %v", err))
return
}

3
routers/web/repo/setting.go
View file

@ -34,6 +34,7 @@ import (
"code.gitea.io/gitea/services/mailer"
mirror_service "code.gitea.io/gitea/services/mirror"
repo_service "code.gitea.io/gitea/services/repository"
wiki_service "code.gitea.io/gitea/services/wiki"
)
const (
@ -682,7 +683,7 @@ func SettingsPost(ctx *context.Context) {
return
}
err := repo.DeleteWiki()
err := wiki_service.DeleteWiki(repo)
if err != nil {
log.Error("Delete Wiki: %v", err.Error())
}

57
services/attachment/attachment.go Normal file
View file

@ -0,0 +1,57 @@
// 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 attachment
import (
"bytes"
"fmt"
"io"
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/upload"
"github.com/google/uuid"
)
// NewAttachment creates a new attachment object, but do not verify.
func NewAttachment(attach *models.Attachment, file io.Reader) (*models.Attachment, error) {
if attach.RepoID == 0 {
return nil, fmt.Errorf("attachment %s should belong to a repository", attach.Name)
}
err := models.WithTx(func(ctx models.DBContext) error {
attach.UUID = uuid.New().String()
size, err := storage.Attachments.Save(attach.RelativePath(), file, -1)
if err != nil {
return fmt.Errorf("Create: %v", err)
}
attach.Size = size
return models.Insert(ctx, attach)
})
return attach, err
}
// UploadAttachment upload new attachment into storage and update database
func UploadAttachment(file io.Reader, actorID, repoID, releaseID int64, fileName string, allowedTypes string) (*models.Attachment, error) {
buf := make([]byte, 1024)
n, _ := file.Read(buf)
if n > 0 {
buf = buf[:n]
}
if err := upload.Verify(buf, fileName, allowedTypes); err != nil {
return nil, err
}
return NewAttachment(&models.Attachment{
RepoID: repoID,
UploaderID: actorID,
ReleaseID: releaseID,
Name: fileName,
}, io.MultiReader(bytes.NewReader(buf), file))
}

42
services/attachment/attachment_test.go Normal file
View file

@ -0,0 +1,42 @@
// 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 attachment
import (
"os"
"path/filepath"
"testing"
"code.gitea.io/gitea/models"
"github.com/stretchr/testify/assert"
)
func TestMain(m *testing.M) {
models.MainTest(m, filepath.Join("..", ".."))
}
func TestUploadAttachment(t *testing.T) {
assert.NoError(t, models.PrepareTestDatabase())
user := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User)
fPath := "./attachment_test.go"
f, err := os.Open(fPath)
assert.NoError(t, err)
defer f.Close()
attach, err := NewAttachment(&models.Attachment{
RepoID: 1,
UploaderID: user.ID,
Name: filepath.Base(fPath),
}, f)
assert.NoError(t, err)
attachment, err := models.GetAttachmentByUUID(attach.UUID)
assert.NoError(t, err)
assert.EqualValues(t, user.ID, attachment.UploaderID)
assert.Equal(t, int64(0), attachment.DownloadCount)
}

11
services/release/release_test.go
View file

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/services/attachment"
"github.com/stretchr/testify/assert"
)
@ -101,10 +102,11 @@ func TestRelease_Create(t *testing.T) {
IsTag: false,
}, nil, ""))
attach, err := models.NewAttachment(&models.Attachment{
attach, err := attachment.NewAttachment(&models.Attachment{
RepoID: repo.ID,
UploaderID: user.ID,
Name: "test.txt",
}, []byte{}, strings.NewReader("testtest"))
}, strings.NewReader("testtest"))
assert.NoError(t, err)
var release = models.Release{
@ -233,10 +235,11 @@ func TestRelease_Update(t *testing.T) {
assert.Equal(t, tagName, release.TagName)
// Add new attachments
attach, err := models.NewAttachment(&models.Attachment{
attach, err := attachment.NewAttachment(&models.Attachment{
RepoID: repo.ID,
UploaderID: user.ID,
Name: "test.txt",
}, []byte{}, strings.NewReader("testtest"))
}, strings.NewReader("testtest"))
assert.NoError(t, err)
assert.NoError(t, UpdateRelease(user, gitRepo, release, []string{attach.UUID}, nil, nil))

10
services/wiki/wiki.go
View file

@ -366,3 +366,13 @@ func DeleteWikiPage(doer *models.User, repo *models.Repository, wikiName string)
return nil
}
// DeleteWiki removes the actual and local copy of repository wiki.
func DeleteWiki(repo *models.Repository) error {
if err := models.UpdateRepositoryUnits(repo, nil, []models.UnitType{models.UnitTypeWiki}); err != nil {
return err
}
models.RemoveAllWithNotice("Delete repository wiki", repo.WikiPath())
return nil
}