Refactoring file locking out of tusd.Handler

* Updated filestore.FileStore implementation to use simple locking scheme. Refactored to make interface use pointers. Removed locks from tusd.Handler
* Refactored sample code to use pointer implementation of FileStore.
* Moved ErrFileLocked variable to datastore.go

cmd/tusd/main.go

@@ -39,9 +39,7 @@ func main() {
 	}
 
 	var store tusd.DataStore
-	store = filestore.FileStore{
+	store = filestore.NewFileStore(dir)
-		Path: dir,
-	}
 
 	if storeSize > 0 {
 		store = limitedstore.New(storeSize, store)

datastore.go

@@ -2,8 +2,15 @@ package tusd
 
 import (
 	"io"
+    "errors"
 )
 
+// Error indicating that the data store has locked the file for further edits.
+// This error is not a part of the official tus specification. Implementers of
+// tusd.DataStore have the option to return this error to signal a file is
+// locked for writing, and cannot be written to by another HTTP request.
+var ErrFileLocked = errors.New("file currently locked")
+
 type MetaData map[string]string
 
 type FileInfo struct {

filestore/filestore.go

@@ -7,6 +7,7 @@
 package filestore
 
 import (
+    "bytes"
 	"encoding/json"
 	"io"
 	"io/ioutil"
@@ -23,10 +24,20 @@ var defaultFilePerm = os.FileMode(0775)
 type FileStore struct {
 	// Relative or absolute path to store files in. FileStore does not check
 	// whether the path exists, you os.MkdirAll in this case on your own.
-	Path string
+	Path  string
+    locks map[string]bool
 }
 
-func (store FileStore) NewUpload(info tusd.FileInfo) (id string, err error) {
+// NewFileStore creates a new FileStore instance.
+func NewFileStore(path string) (store *FileStore) {
+    store = &FileStore{
+        Path: path,
+        locks: make(map[string]bool),
+    }
+    return
+}
+
+func (store *FileStore) NewUpload(info tusd.FileInfo) (id string, err error) {
 	id = uid.Uid()
 	info.ID = id
 
@@ -42,8 +53,13 @@ func (store FileStore) NewUpload(info tusd.FileInfo) (id string, err error) {
 	return
 }
 
-func (store FileStore) WriteChunk(id string, offset int64, src io.Reader) (int64, error) {
+func (store *FileStore) WriteChunk(id string, offset int64, src io.Reader) (int64, error) {
-	file, err := os.OpenFile(store.binPath(id), os.O_WRONLY|os.O_APPEND, defaultFilePerm)
+	if !store.getLock(id) {
+	    return 0, tusd.ErrFileLocked
+	}
+    defer store.clearLock(id)
+    
+    file, err := os.OpenFile(store.binPath(id), os.O_WRONLY|os.O_APPEND, defaultFilePerm)
 	if err != nil {
 		return 0, err
 	}
@@ -58,21 +74,28 @@ func (store FileStore) WriteChunk(id string, offset int64, src io.Reader) (int64
 	return n, err
 }
 
-func (store FileStore) GetInfo(id string) (tusd.FileInfo, error) {
+func (store *FileStore) GetInfo(id string) (info tusd.FileInfo, err error) {
-	info := tusd.FileInfo{}
+    data, err := ioutil.ReadFile(store.infoPath(id))
-	data, err := ioutil.ReadFile(store.infoPath(id))
 	if err != nil {
 		return info, err
 	}
 	err = json.Unmarshal(data, &info)
-	return info, err
+	return
 }
 
-func (store FileStore) GetReader(id string) (io.Reader, error) {
+func (store *FileStore) GetReader(id string) (io.Reader, error) {
-	return os.Open(store.binPath(id))
+	if !store.getLock(id) {
+	    return bytes.NewReader(make([]byte, 0)), tusd.ErrFileLocked
+	}
+    defer store.clearLock(id)
+    return os.Open(store.binPath(id))
 }
 
-func (store FileStore) Terminate(id string) error {
+func (store *FileStore) Terminate(id string) error {
+    if !store.getLock(id) {
+        return tusd.ErrFileLocked
+    }
+    defer store.clearLock(id)
 	if err := os.Remove(store.infoPath(id)); err != nil {
 		return err
 	}
@@ -83,17 +106,17 @@ func (store FileStore) Terminate(id string) error {
 }
 
 // Return the path to the .bin storing the binary data
-func (store FileStore) binPath(id string) string {
+func (store *FileStore) binPath(id string) string {
 	return store.Path + "/" + id + ".bin"
 }
 
 // Return the path to the .info file storing the file's info
-func (store FileStore) infoPath(id string) string {
+func (store *FileStore) infoPath(id string) string {
 	return store.Path + "/" + id + ".info"
 }
 
 // Update the entire information. Everything will be overwritten.
-func (store FileStore) writeInfo(id string, info tusd.FileInfo) error {
+func (store *FileStore) writeInfo(id string, info tusd.FileInfo) error {
 	data, err := json.Marshal(info)
 	if err != nil {
 		return err
@@ -102,7 +125,7 @@ func (store FileStore) writeInfo(id string, info tusd.FileInfo) error {
 }
 
 // Update the .info file using the new upload.
-func (store FileStore) setOffset(id string, offset int64) error {
+func (store *FileStore) setOffset(id string, offset int64) error {
 	info, err := store.GetInfo(id)
 	if err != nil {
 		return err
@@ -116,3 +139,19 @@ func (store FileStore) setOffset(id string, offset int64) error {
 	info.Offset = offset
 	return store.writeInfo(id, info)
 }
+
+// getLock obtains a lock on reading/writing data for the given file ID.
+func (store *FileStore) getLock(id string) (hasLock bool) {
+    if _, locked := store.locks[id]; locked {
+        hasLock = false
+        return
+    }
+    store.locks[id] = true
+    hasLock = true
+    return
+}
+
+// clearLock removes the lock for the given file ID.
+func (store *FileStore) clearLock(id string) {
+    delete(store.locks, id)
+}

filestore/filestore_test.go

@@ -11,7 +11,7 @@ import (
 )
 
 // Test interface implementation of Filestore
-var _ tusd.DataStore = FileStore{}
+var _ tusd.DataStore = NewFileStore("")
 
 func TestFilestore(t *testing.T) {
 	tmp, err := ioutil.TempDir("", "tusd-filestore-")
@@ -19,7 +19,7 @@ func TestFilestore(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	store := FileStore{tmp}
+	store := NewFileStore(tmp)
 
 	// Create new upload
 	id, err := store.NewUpload(tusd.FileInfo{

handler.go

@@ -25,7 +25,6 @@ var (
 	ErrInvalidUploadLength = errors.New("missing or invalid Upload-Length header")
 	ErrInvalidOffset       = errors.New("missing or invalid Upload-Offset header")
 	ErrNotFound            = errors.New("upload not found")
-	ErrFileLocked          = errors.New("file currently locked")
 	ErrIllegalOffset       = errors.New("illegal offset")
 	ErrSizeExceeded        = errors.New("resource's size exceeded")
 	ErrNotImplemented      = errors.New("feature not implemented")
@@ -72,7 +71,6 @@ type Handler struct {
 	isBasePathAbs bool
 	basePath      string
 	routeHandler  http.Handler
-	locks         map[string]bool
 
 	// For each finished upload the corresponding info object will be sent using
 	// this unbuffered channel. The NotifyCompleteUploads property in the Config
@@ -106,7 +104,6 @@ func NewHandler(config Config) (*Handler, error) {
 		basePath:        base,
 		isBasePathAbs:   uri.IsAbs(),
 		routeHandler:    mux,
-		locks:           make(map[string]bool),
 		CompleteUploads: make(chan FileInfo),
 	}
 
@@ -276,20 +273,6 @@ func (handler *Handler) headFile(w http.ResponseWriter, r *http.Request) {
 func (handler *Handler) patchFile(w http.ResponseWriter, r *http.Request) {
 	id := r.URL.Query().Get(":id")
 
-	// Ensure file is not locked
-	if _, ok := handler.locks[id]; ok {
-		handler.sendError(w, ErrFileLocked)
-		return
-	}
-
-	// Lock file for further writes (heads are allowed)
-	handler.locks[id] = true
-
-	// File will be unlocked regardless of an error or success
-	defer func() {
-		delete(handler.locks, id)
-	}()
-
 	info, err := handler.dataStore.GetInfo(id)
 	if err != nil {
 		handler.sendError(w, err)
@@ -354,20 +337,6 @@ func (handler *Handler) patchFile(w http.ResponseWriter, r *http.Request) {
 func (handler *Handler) getFile(w http.ResponseWriter, r *http.Request) {
 	id := r.URL.Query().Get(":id")
 
-	// Ensure file is not locked
-	if _, ok := handler.locks[id]; ok {
-		handler.sendError(w, ErrFileLocked)
-		return
-	}
-
-	// Lock file for further writes (heads are allowed)
-	handler.locks[id] = true
-
-	// File will be unlocked regardless of an error or success
-	defer func() {
-		delete(handler.locks, id)
-	}()
-
 	info, err := handler.dataStore.GetInfo(id)
 	if err != nil {
 		handler.sendError(w, err)
@@ -401,20 +370,6 @@ func (handler *Handler) getFile(w http.ResponseWriter, r *http.Request) {
 func (handler *Handler) delFile(w http.ResponseWriter, r *http.Request) {
 	id := r.URL.Query().Get(":id")
 
-	// Ensure file is not locked
-	if _, ok := handler.locks[id]; ok {
-		handler.sendError(w, ErrFileLocked)
-		return
-	}
-
-	// Lock file for further writes (heads are allowed)
-	handler.locks[id] = true
-
-	// File will be unlocked regardless of an error or success
-	defer func() {
-		delete(handler.locks, id)
-	}()
-
 	err := handler.dataStore.Terminate(id)
 	if err != nil {
 		handler.sendError(w, err)