fix(files): use absolute path everywhere

This commit is contained in:
kolaente 2024-09-06 12:55:35 +02:00
parent cf94cc8cab
commit 68636f27da
Signed by untrusted user: konrad
GPG Key ID: F40E70337AB24C9B
11 changed files with 45 additions and 31 deletions

View File

@ -20,7 +20,6 @@ import (
"os"
"testing"
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/modules/keyvalue"
@ -54,9 +53,8 @@ func initFixtures(t *testing.T) {
// InitTestFileFixtures initializes file fixtures
func InitTestFileFixtures(t *testing.T) {
// Init fixture files
filename := config.FilesBasePath.GetString() + "/1"
err := afero.WriteFile(afs, filename, []byte("testfile1"), 0644)
testfile := &File{ID: 1}
err := afero.WriteFile(afs, testfile.getAbsoluteFilePath(), []byte("testfile1"), 0644)
require.NoError(t, err)
}
@ -84,6 +82,6 @@ func InitTests() {
}
// FileStat stats a file. This is an exported function to be able to test this from outide of the package
func FileStat(filename string) (os.FileInfo, error) {
return afs.Stat(filename)
func FileStat(file *File) (os.FileInfo, error) {
return afs.Stat(file.getAbsoluteFilePath())
}

View File

@ -128,11 +128,8 @@ func CreateWithMimeAndSession(s *xorm.Session, f io.Reader, realname string, rea
}
// Delete removes a file from the DB and the file system
func (f *File) Delete() (err error) {
s := db.NewSession()
defer s.Close()
deleted, err := s.Where("id = ?", f.ID).Delete(f)
func (f *File) Delete(s *xorm.Session) (err error) {
deleted, err := s.Where("id = ?", f.ID).Delete(&File{})
if err != nil {
_ = s.Rollback()
return err

View File

@ -21,6 +21,8 @@ import (
"os"
"testing"
"code.vikunja.io/api/pkg/db"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -84,15 +86,20 @@ func TestCreate(t *testing.T) {
func TestFile_Delete(t *testing.T) {
t.Run("Normal", func(t *testing.T) {
s := db.NewSession()
defer s.Close()
initFixtures(t)
f := &File{ID: 1}
err := f.Delete()
err := f.Delete(s)
require.NoError(t, err)
})
t.Run("Nonexisting", func(t *testing.T) {
s := db.NewSession()
defer s.Close()
initFixtures(t)
f := &File{ID: 9999}
err := f.Delete()
err := f.Delete(s)
require.Error(t, err)
assert.True(t, IsErrFileDoesNotExist(err))
})

View File

@ -398,7 +398,7 @@ func RegisterOldExportCleanupCron() {
log.Debugf(logPrefix+"Removing %d old user data exports...", len(fs))
for _, f := range fs {
err = f.Delete()
err = f.Delete(s)
if err != nil {
log.Errorf(logPrefix+"Could not remove user export file %d: %s", f.ID, err)
return

View File

@ -1204,8 +1204,11 @@ func (p *Project) DeleteBackgroundFileIfExists() (err error) {
return
}
s := db.NewSession()
defer s.Close()
file := files.File{ID: p.BackgroundFileID}
err = file.Delete()
err = file.Delete(s)
if err != nil && files.IsErrFileDoesNotExist(err) {
return nil
}

View File

@ -77,7 +77,7 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realna
ta.CreatedBy, err = GetUserOrLinkShareUser(s, a)
if err != nil {
// remove the uploaded file if adding it to the db fails
if err2 := file.Delete(); err2 != nil {
if err2 := file.Delete(s); err2 != nil {
return err2
}
return err
@ -87,7 +87,7 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realna
_, err = s.Insert(ta)
if err != nil {
// remove the uploaded file if adding it to the db fails
if err2 := file.Delete(); err2 != nil {
if err2 := file.Delete(s); err2 != nil {
return err2
}
return err
@ -326,7 +326,7 @@ func (ta *TaskAttachment) Delete(s *xorm.Session, a web.Auth) error {
}
// Delete the underlying file
err = ta.File.Delete()
err = ta.File.Delete(s)
// If the file does not exist, we don't want to error out
if err != nil && files.IsErrFileDoesNotExist(err) {
return nil

View File

@ -19,7 +19,7 @@ package models
import (
"io"
"os"
"strconv"
"path/filepath"
"testing"
"code.vikunja.io/api/pkg/config"
@ -51,7 +51,7 @@ func TestTaskAttachment_ReadOne(t *testing.T) {
// Load the actual attachment file and check its content
err = ta.File.LoadFileByID()
require.NoError(t, err)
assert.Equal(t, config.FilesBasePath.GetString()+"/1", ta.File.File.Name())
assert.Equal(t, filepath.Join(config.ServiceRootpath.GetString(), config.FilesBasePath.GetString(), "/1"), ta.File.File.Name())
content := make([]byte, 9)
read, err := ta.File.File.Read(content)
require.NoError(t, err)
@ -122,7 +122,7 @@ func TestTaskAttachment_NewAttachment(t *testing.T) {
err := ta.NewAttachment(s, tf, "testfile", 100, testuser)
require.NoError(t, err)
assert.NotEqual(t, 0, ta.FileID)
_, err = files.FileStat("files/" + strconv.FormatInt(ta.FileID, 10))
_, err = files.FileStat(ta.File)
require.NoError(t, err)
assert.False(t, os.IsNotExist(err))
assert.Equal(t, testuser.ID, ta.CreatedByID)
@ -158,23 +158,28 @@ func TestTaskAttachment_ReadAll(t *testing.T) {
}
func TestTaskAttachment_Delete(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
u := &user.User{ID: 1}
files.InitTestFileFixtures(t)
t.Run("Normal", func(t *testing.T) {
files.InitTestFileFixtures(t)
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
ta := &TaskAttachment{ID: 1}
err := ta.Delete(s, u)
require.NoError(t, err)
// Check if the file itself was deleted
_, err = files.FileStat("/1") // The new file has the id 2 since it's the second attachment
_, err = files.FileStat(ta.File) // The new file has the id 2 since it's the second attachment
require.Error(t, err)
assert.True(t, os.IsNotExist(err))
})
t.Run("Nonexisting", func(t *testing.T) {
files.InitTestFileFixtures(t)
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
ta := &TaskAttachment{ID: 9999}
err := ta.Delete(s, u)
require.Error(t, err)
@ -182,6 +187,10 @@ func TestTaskAttachment_Delete(t *testing.T) {
})
t.Run("Existing attachment, nonexisting file", func(t *testing.T) {
files.InitTestFileFixtures(t)
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
ta := &TaskAttachment{ID: 2}
err := ta.Delete(s, u)
require.NoError(t, err)

View File

@ -79,7 +79,7 @@ type Task struct {
EndDate time.Time `xorm:"DATETIME INDEX null 'end_date'" json:"end_date" query:"-"`
// An array of users who are assigned to this task
Assignees []*user.User `xorm:"-" json:"assignees"`
// An array of labels which are associated with this task. This property is read-only, you must use the seperate endpoint to add labels to a task.
// An array of labels which are associated with this task. This property is read-only, you must use the separate endpoint to add labels to a task.
Labels []*Label `xorm:"-" json:"labels"`
// The task color in hex
HexColor string `xorm:"varchar(6) null" json:"hex_color" valid:"runelength(0|7)" maxLength:"7"`

View File

@ -291,7 +291,7 @@ func (p *Provider) Set(s *xorm.Session, image *background.Image, project *models
// Remove the old background if one exists
if project.BackgroundFileID != 0 {
file := files.File{ID: project.BackgroundFileID}
err = file.Delete()
err = file.Delete(s)
if err != nil && !files.IsErrFileDoesNotExist(err) {
return err
}

View File

@ -56,7 +56,7 @@ func (p *Provider) Set(s *xorm.Session, img *background.Image, project *models.P
// Remove the old background if one exists
if project.BackgroundFileID != 0 {
file := files.File{ID: project.BackgroundFileID}
err := file.Delete()
err := file.Delete(s)
if err != nil && !files.IsErrFileDoesNotExist(err) {
return err
}

View File

@ -167,7 +167,7 @@ func UploadAvatar(c echo.Context) (err error) {
// Remove the old file if one exists
if u.AvatarFileID != 0 {
f := &files.File{ID: u.AvatarFileID}
if err := f.Delete(); err != nil {
if err := f.Delete(s); err != nil {
if !files.IsErrFileDoesNotExist(err) {
_ = s.Rollback()
return handler.HandleHTTPError(err)