Added test for storing a file

pkg/files/filehandling.go

@@ -25,8 +25,7 @@ import (
 	"testing"
 )
 
-// This file handling storing and retrieving a file for different backends
-
+// This file handles storing and retrieving a file for different backends
 var fs afero.Fs
 var afs *afero.Afero
 
@@ -68,3 +67,8 @@ func TestMain(m *testing.M) {
 	InitTests()
 	os.Exit(m.Run())
 }
+
+// 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)
+}

pkg/files/files.go

@@ -21,7 +21,7 @@ import (
 	"code.vikunja.io/web"
 	"fmt"
 	"github.com/spf13/afero"
-	"mime/multipart"
+	"io"
 	"strconv"
 	"time"
 )
@@ -67,12 +67,12 @@ func (f *File) LoadFileMetaByID() (err error) {
 }
 
 // Create creates a new file from an FileHeader
-func Create(f *multipart.FileHeader, a web.Auth) (file *File, err error) {
+func Create(f io.ReadCloser, realname string, realsize int64, a web.Auth) (file *File, err error) {
 
 	// We first insert the file into the db to get it's ID
 	file = &File{
-		Name:        f.Filename,
-		Size:        f.Size,
+		Name:        realname,
+		Size:        realsize,
 		CreatedByID: a.GetID(),
 	}
 
@@ -81,15 +81,8 @@ func Create(f *multipart.FileHeader, a web.Auth) (file *File, err error) {
 		return
 	}
 
-	// Open the actual file
-	ff, err := f.Open()
-	if err != nil {
-		return
-	}
-	defer ff.Close()
-
 	// Save the file to storage with its new ID as path
-	err = afs.WriteReader(file.getFileName(), ff)
+	err = afs.WriteReader(file.getFileName(), f)
 	return
 }
 

pkg/models/task_attachment.go

@@ -19,7 +19,7 @@ package models
 import (
 	"code.vikunja.io/api/pkg/files"
 	"code.vikunja.io/web"
-	"mime/multipart"
+	"io"
 )
 
 // TaskAttachment is the definition of a task attachment
@@ -45,10 +45,11 @@ func (TaskAttachment) TableName() string {
 }
 
 // NewAttachment creates a new task attachment
-func (ta *TaskAttachment) NewAttachment(fileheader *multipart.FileHeader, a web.Auth) error {
+// Note: I'm not sure if only accepting an io.ReadCloser and not an afero.File or os.File instead is a good way of doing things.
+func (ta *TaskAttachment) NewAttachment(f io.ReadCloser, realname string, realsize int64, a web.Auth) error {
 
 	// Store the file
-	file, err := files.Create(fileheader, a)
+	file, err := files.Create(f, realname, realsize, a)
 	if err != nil {
 		return err
 	}

pkg/models/task_attachment_test.go

@@ -19,7 +19,10 @@ package models
 
 import (
 	"code.vikunja.io/api/pkg/config"
+	"code.vikunja.io/api/pkg/files"
 	"github.com/stretchr/testify/assert"
+	"io"
+	"os"
 	"testing"
 )
 
@@ -61,10 +64,39 @@ func TestTaskAttachment_ReadOne(t *testing.T) {
 	})
 }
 
+type testfile struct {
+	content []byte
+	done    bool
+}
+
+func (t *testfile) Read(p []byte) (n int, err error) {
+	if t.done {
+		return 0, io.EOF
+	}
+	for i, b := range t.content {
+		p[i] = b
+	}
+	t.done = true
+	return len(p), nil
+}
+
+func (t *testfile) Close() error {
+	return nil
+}
+
 func TestTaskAttachment_NewAttachment(t *testing.T) {
 	// Assert the file is being stored correctly
-	// Maybe needs adoption to pass an os.File or whatever comes from multipart.File.Open instead
-	// of passing multipart.File directly
+	ta := TaskAttachment{
+		TaskID: 1,
+	}
+
+	tf := &testfile{
+		content: []byte("testingstuff"),
+	}
+	err := ta.NewAttachment(tf, "testfile", 100, &User{ID: 1})
+	assert.NoError(t, err)
+	_, err = files.FileStat("/2")
+	assert.False(t, os.IsNotExist(err))
 
 	// Also assert created_by_id is correct in the files table
 

pkg/routes/api/v1/task_attachment.go

@@ -56,7 +56,14 @@ func UploadTaskAttachment(c echo.Context) error {
 		ta := models.TaskAttachment{
 			TaskID: taskAttachment.TaskID,
 		}
-		err = ta.NewAttachment(file, user)
+
+		f, err := file.Open()
+		if err != nil {
+			return handler.HandleHTTPError(err, c)
+		}
+		defer f.Close()
+
+		err = ta.NewAttachment(f, file.Filename, file.Size, user)
 		if err != nil {
 			return handler.HandleHTTPError(err, c)
 		}