filestore: Implement new interfaces

pkg/filestore/filestore.go

@@ -6,13 +6,6 @@
 // `[id]` files without an extension contain the raw binary data uploaded.
 // No cleanup is performed so you may want to run a cronjob to ensure your disk
 // is not filled up with old and finished uploads.
-//
-// In addition, it provides an exclusive upload locking mechanism using lock files
-// which are stored on disk. Each of them stores the PID of the process which
-// acquired the lock. This allows locks to be automatically freed when a process
-// is unable to release it on its own because the process is not alive anymore.
-// For more information, consult the documentation for handler.LockerDataStore
-// interface, which is implemented by FileStore
 package filestore
 
 import (
@@ -25,8 +18,6 @@ import (
 
 	"github.com/tus/tusd/internal/uid"
 	"github.com/tus/tusd/pkg/handler"
-
-	"gopkg.in/Acconut/lockfile.v1"
 )
 
 var defaultFilePerm = os.FileMode(0664)
@@ -51,15 +42,13 @@ func New(path string) FileStore {
 // all possible extension to it.
 func (store FileStore) UseIn(composer *handler.StoreComposer) {
 	composer.UseCore(store)
-	composer.UseGetReader(store)
 	composer.UseTerminater(store)
-	composer.UseLocker(store)
 	composer.UseConcater(store)
 	composer.UseLengthDeferrer(store)
 }
 
-func (store FileStore) NewUpload(info handler.FileInfo) (id string, err error) {
+func (store FileStore) NewUpload(info handler.FileInfo) (handler.Upload, error) {
-	id = uid.Uid()
+	id := uid.Uid()
 	binPath := store.binPath(id)
 	info.ID = id
 	info.Storage = map[string]string{
@@ -73,17 +62,84 @@ func (store FileStore) NewUpload(info handler.FileInfo) (id string, err error) {
 		if os.IsNotExist(err) {
 			err = fmt.Errorf("upload directory does not exist: %s", store.Path)
 		}
-		return "", err
+		return nil, err
 	}
 	defer file.Close()
 
-	// writeInfo creates the file by itself if necessary
+	upload := &fileUpload{
-	err = store.writeInfo(id, info)
+		info:     info,
-	return
+		infoPath: store.infoPath(id),
+		binPath:  store.binPath(id),
 	}
 
-func (store FileStore) WriteChunk(id string, offset int64, src io.Reader) (int64, error) {
+	// writeInfo creates the file by itself if necessary
-	file, err := os.OpenFile(store.binPath(id), os.O_WRONLY|os.O_APPEND, defaultFilePerm)
+	err = upload.writeInfo()
+	if err != nil {
+		return nil, err
+	}
+
+	return upload, nil
+}
+
+func (store FileStore) GetUpload(id string) (handler.Upload, error) {
+	info := handler.FileInfo{}
+	data, err := ioutil.ReadFile(store.infoPath(id))
+	if err != nil {
+		return nil, err
+	}
+	if err := json.Unmarshal(data, &info); err != nil {
+		return nil, err
+	}
+
+	binPath := store.binPath(id)
+	infoPath := store.infoPath(id)
+	stat, err := os.Stat(binPath)
+	if err != nil {
+		return nil, err
+	}
+
+	info.Offset = stat.Size()
+
+	return &fileUpload{
+		info:     info,
+		binPath:  binPath,
+		infoPath: infoPath,
+	}, nil
+}
+
+func (store FileStore) AsTerminatableUpload(upload handler.Upload) handler.TerminatableUpload {
+	return upload.(*fileUpload)
+}
+
+func (store FileStore) AsLengthDeclarableUpload(upload handler.Upload) handler.LengthDeclarableUpload {
+	return upload.(*fileUpload)
+}
+
+// binPath returns the path to the file storing the binary data.
+func (store FileStore) binPath(id string) string {
+	return filepath.Join(store.Path, id)
+}
+
+// infoPath returns the path to the .info file storing the file's info.
+func (store FileStore) infoPath(id string) string {
+	return filepath.Join(store.Path, id+".info")
+}
+
+type fileUpload struct {
+	// info stores the current information about the upload
+	info handler.FileInfo
+	// infoPath is the path to the .info file
+	infoPath string
+	// binPath is the path to the binary file (which has no extension)
+	binPath string
+}
+
+func (upload *fileUpload) GetInfo() (handler.FileInfo, error) {
+	return upload.info, nil
+}
+
+func (upload *fileUpload) WriteChunk(offset int64, src io.Reader) (int64, error) {
+	file, err := os.OpenFile(upload.binPath, os.O_WRONLY|os.O_APPEND, defaultFilePerm)
 	if err != nil {
 		return 0, err
 	}
@@ -99,39 +155,20 @@ func (store FileStore) WriteChunk(id string, offset int64, src io.Reader) (int64
 		err = nil
 	}
 
+	upload.info.Offset += n
+
 	return n, err
 }
 
-func (store FileStore) GetInfo(id string) (handler.FileInfo, error) {
+func (upload *fileUpload) GetReader() (io.Reader, error) {
-	info := handler.FileInfo{}
+	return os.Open(upload.binPath)
-	data, err := ioutil.ReadFile(store.infoPath(id))
-	if err != nil {
-		return info, err
-	}
-	if err := json.Unmarshal(data, &info); err != nil {
-		return info, err
 }
 
-	binPath := store.binPath(id)
+func (upload *fileUpload) Terminate() error {
-	stat, err := os.Stat(binPath)
+	if err := os.Remove(upload.infoPath); err != nil {
-	if err != nil {
-		return info, err
-	}
-
-	info.Offset = stat.Size()
-
-	return info, nil
-}
-
-func (store FileStore) GetReader(id string) (io.Reader, error) {
-	return os.Open(store.binPath(id))
-}
-
-func (store FileStore) Terminate(id string) error {
-	if err := os.Remove(store.infoPath(id)); err != nil {
 		return err
 	}
-	if err := os.Remove(store.binPath(id)); err != nil {
+	if err := os.Remove(upload.binPath); err != nil {
 		return err
 	}
 	return nil
@@ -145,7 +182,7 @@ func (store FileStore) ConcatUploads(dest string, uploads []string) (err error)
 	defer file.Close()
 
 	for _, id := range uploads {
-		src, err := store.GetReader(id)
+		src, err := os.Open(store.binPath(id))
 		if err != nil {
 			return err
 		}
@@ -158,76 +195,21 @@ func (store FileStore) ConcatUploads(dest string, uploads []string) (err error)
 	return
 }
 
-func (store FileStore) DeclareLength(id string, length int64) error {
+func (upload *fileUpload) DeclareLength(length int64) error {
-	info, err := store.GetInfo(id)
+	upload.info.Size = length
-	if err != nil {
+	upload.info.SizeIsDeferred = false
-		return err
+	return upload.writeInfo()
-	}
-	info.Size = length
-	info.SizeIsDeferred = false
-	return store.writeInfo(id, info)
-}
-
-func (store FileStore) LockUpload(id string) error {
-	lock, err := store.newLock(id)
-	if err != nil {
-		return err
-	}
-
-	err = lock.TryLock()
-	if err == lockfile.ErrBusy {
-		return handler.ErrFileLocked
-	}
-
-	return err
-}
-
-func (store FileStore) UnlockUpload(id string) error {
-	lock, err := store.newLock(id)
-	if err != nil {
-		return err
-	}
-
-	err = lock.Unlock()
-
-	// A "no such file or directory" will be returned if no lockfile was found.
-	// Since this means that the file has never been locked, we drop the error
-	// and continue as if nothing happened.
-	if os.IsNotExist(err) {
-		err = nil
-	}
-
-	return err
-}
-
-// newLock contructs a new Lockfile instance.
-func (store FileStore) newLock(id string) (lockfile.Lockfile, error) {
-	path, err := filepath.Abs(filepath.Join(store.Path, id+".lock"))
-	if err != nil {
-		return lockfile.Lockfile(""), err
-	}
-
-	// We use Lockfile directly instead of lockfile.New to bypass the unnecessary
-	// check whether the provided path is absolute since we just resolved it
-	// on our own.
-	return lockfile.Lockfile(path), nil
-}
-
-// binPath returns the path to the file storing the binary data.
-func (store FileStore) binPath(id string) string {
-	return filepath.Join(store.Path, id)
-}
-
-// infoPath returns the path to the .info file storing the file's info.
-func (store FileStore) infoPath(id string) string {
-	return filepath.Join(store.Path, id+".info")
 }
 
 // writeInfo updates the entire information. Everything will be overwritten.
-func (store FileStore) writeInfo(id string, info handler.FileInfo) error {
+func (upload *fileUpload) writeInfo() error {
-	data, err := json.Marshal(info)
+	data, err := json.Marshal(upload.info)
 	if err != nil {
 		return err
 	}
-	return ioutil.WriteFile(store.infoPath(id), data, defaultFilePerm)
+	return ioutil.WriteFile(upload.infoPath, data, defaultFilePerm)
+}
+
+func (upload *fileUpload) FinishUpload() error {
+	return nil
 }

pkg/filestore/filestore_test.go

@@ -9,15 +9,12 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
 	"github.com/tus/tusd/pkg/handler"
 )
 
 // Test interface implementation of Filestore
 var _ handler.DataStore = FileStore{}
-var _ handler.GetReaderDataStore = FileStore{}
 var _ handler.TerminaterDataStore = FileStore{}
-var _ handler.LockerDataStore = FileStore{}
 var _ handler.ConcaterDataStore = FileStore{}
 var _ handler.LengthDeferrerDataStore = FileStore{}
 
@@ -30,38 +27,38 @@ func TestFilestore(t *testing.T) {
 	store := FileStore{tmp}
 
 	// Create new upload
-	id, err := store.NewUpload(handler.FileInfo{
+	upload, err := store.NewUpload(handler.FileInfo{
 		Size: 42,
 		MetaData: map[string]string{
 			"hello": "world",
 		},
 	})
 	a.NoError(err)
-	a.NotEqual("", id)
+	a.NotEqual(nil, upload)
 
 	// Check info without writing
-	info, err := store.GetInfo(id)
+	info, err := upload.GetInfo()
 	a.NoError(err)
 	a.EqualValues(42, info.Size)
 	a.EqualValues(0, info.Offset)
 	a.Equal(handler.MetaData{"hello": "world"}, info.MetaData)
 	a.Equal(2, len(info.Storage))
 	a.Equal("filestore", info.Storage["Type"])
-	a.Equal(filepath.Join(tmp, id), info.Storage["Path"])
+	a.Equal(filepath.Join(tmp, info.ID), info.Storage["Path"])
 
 	// Write data to upload
-	bytesWritten, err := store.WriteChunk(id, 0, strings.NewReader("hello world"))
+	bytesWritten, err := upload.WriteChunk(0, strings.NewReader("hello world"))
 	a.NoError(err)
 	a.EqualValues(len("hello world"), bytesWritten)
 
 	// Check new offset
-	info, err = store.GetInfo(id)
+	info, err = upload.GetInfo()
 	a.NoError(err)
 	a.EqualValues(42, info.Size)
 	a.EqualValues(11, info.Offset)
 
 	// Read content
-	reader, err := store.GetReader(id)
+	reader, err := upload.GetReader()
 	a.NoError(err)
 
 	content, err := ioutil.ReadAll(reader)
@@ -70,10 +67,11 @@ func TestFilestore(t *testing.T) {
 	reader.(io.Closer).Close()
 
 	// Terminate upload
-	a.NoError(store.Terminate(id))
+	a.NoError(store.AsTerminatableUpload(upload).Terminate())
 
 	// Test if upload is deleted
-	_, err = store.GetInfo(id)
+	upload, err = store.GetUpload(info.ID)
+	a.Equal(nil, upload)
 	a.True(os.IsNotExist(err))
 }
 
@@ -82,24 +80,10 @@ func TestMissingPath(t *testing.T) {
 
 	store := FileStore{"./path-that-does-not-exist"}
 
-	id, err := store.NewUpload(handler.FileInfo{})
+	upload, err := store.NewUpload(handler.FileInfo{})
 	a.Error(err)
-	a.Equal(err.Error(), "upload directory does not exist: ./path-that-does-not-exist")
+	a.Equal("upload directory does not exist: ./path-that-does-not-exist", err.Error())
-	a.Equal(id, "")
+	a.Equal(nil, upload)
-}
-
-func TestFileLocker(t *testing.T) {
-	a := assert.New(t)
-
-	dir, err := ioutil.TempDir("", "tusd-file-locker")
-	a.NoError(err)
-
-	var locker handler.LockerDataStore
-	locker = FileStore{dir}
-
-	a.NoError(locker.LockUpload("one"))
-	a.Equal(handler.ErrFileLocked, locker.LockUpload("one"))
-	a.NoError(locker.UnlockUpload("one"))
 }
 
 func TestConcatUploads(t *testing.T) {
@@ -111,9 +95,13 @@ func TestConcatUploads(t *testing.T) {
 	store := FileStore{tmp}
 
 	// Create new upload to hold concatenated upload
-	finId, err := store.NewUpload(handler.FileInfo{Size: 9})
+	finUpload, err := store.NewUpload(handler.FileInfo{Size: 9})
 	a.NoError(err)
-	a.NotEqual("", finId)
+	a.NotEqual(nil, finUpload)
+
+	finInfo, err := finUpload.GetInfo()
+	a.NoError(err)
+	finId := finInfo.ID
 
 	// Create three uploads for concatenating
 	ids := make([]string, 3)
@@ -123,27 +111,33 @@ func TestConcatUploads(t *testing.T) {
 		"ghi",
 	}
 	for i := 0; i < 3; i++ {
-		id, err := store.NewUpload(handler.FileInfo{Size: 3})
+		upload, err := store.NewUpload(handler.FileInfo{Size: 3})
 		a.NoError(err)
 
-		n, err := store.WriteChunk(id, 0, strings.NewReader(contents[i]))
+		n, err := upload.WriteChunk(0, strings.NewReader(contents[i]))
 		a.NoError(err)
 		a.EqualValues(3, n)
 
-		ids[i] = id
+		info, err := upload.GetInfo()
+		a.NoError(err)
+
+		ids[i] = info.ID
 	}
 
 	err = store.ConcatUploads(finId, ids)
 	a.NoError(err)
 
 	// Check offset
-	info, err := store.GetInfo(finId)
+	finUpload, err = store.GetUpload(finId)
+	a.NoError(err)
+
+	info, err := finUpload.GetInfo()
 	a.NoError(err)
 	a.EqualValues(9, info.Size)
 	a.EqualValues(9, info.Offset)
 
 	// Read content
-	reader, err := store.GetReader(finId)
+	reader, err := finUpload.GetReader()
 	a.NoError(err)
 
 	content, err := ioutil.ReadAll(reader)
@@ -160,19 +154,23 @@ func TestDeclareLength(t *testing.T) {
 
 	store := FileStore{tmp}
 
-	originalInfo := handler.FileInfo{Size: 0, SizeIsDeferred: true}
+	upload, err := store.NewUpload(handler.FileInfo{
-	id, err := store.NewUpload(originalInfo)
+		Size:           0,
+		SizeIsDeferred: true,
+	})
+	a.NoError(err)
+	a.NotEqual(nil, upload)
+
+	info, err := upload.GetInfo()
+	a.NoError(err)
+	a.EqualValues(0, info.Size)
+	a.Equal(true, info.SizeIsDeferred)
+
+	err = store.AsLengthDeclarableUpload(upload).DeclareLength(100)
 	a.NoError(err)
 
-	info, err := store.GetInfo(id)
+	updatedInfo, err := upload.GetInfo()
-	a.Equal(info.Size, originalInfo.Size)
-	a.Equal(info.SizeIsDeferred, originalInfo.SizeIsDeferred)
-
-	size := int64(100)
-	err = store.DeclareLength(id, size)
 	a.NoError(err)
-
+	a.EqualValues(100, updatedInfo.Size)
-	updatedInfo, err := store.GetInfo(id)
+	a.Equal(false, updatedInfo.SizeIsDeferred)
-	a.Equal(updatedInfo.Size, size)
-	a.False(updatedInfo.SizeIsDeferred)
 }