fix(files): use absolute path everywhere

(cherry picked from commit 68636f27da707f3ee87ba0e4f1ff100504486608)

pkg/files/filehandling.go

@@ -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
+	testfile := &File{ID: 1}
-	filename := config.FilesBasePath.GetString() + "/1"
+	err := afero.WriteFile(afs, testfile.getAbsoluteFilePath(), []byte("testfile1"), 0644)
-	err := afero.WriteFile(afs, filename, []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) {
+func FileStat(file *File) (os.FileInfo, error) {
-	return afs.Stat(filename)
+	return afs.Stat(file.getAbsoluteFilePath())
 }

pkg/files/files.go

@@ -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) {
+func (f *File) Delete(s *xorm.Session) (err error) {
-	s := db.NewSession()
+	deleted, err := s.Where("id = ?", f.ID).Delete(&File{})
-	defer s.Close()
-
-	deleted, err := s.Where("id = ?", f.ID).Delete(f)
 	if err != nil {
 		_ = s.Rollback()
 		return err

pkg/files/files_test.go

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

pkg/models/export.go

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

pkg/models/project.go

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

pkg/models/task_attachment.go

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

pkg/models/task_attachment_test.go

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

pkg/models/tasks.go

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

pkg/modules/background/unsplash/unsplash.go

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

pkg/modules/background/upload/upload.go

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

pkg/routes/api/v1/avatar.go

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