Adding LockFile and UnlockFile to server API (per comments on issue #22)

* Added Locked member to tusd.FileInfo object
* Pushed locking mechanisms down to the JSON files themselves.

datastore.go

@@ -1,16 +1,9 @@
 package tusd
 
 import (
-	"errors"
 	"io"
 )
 
-// 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 {
@@ -30,6 +23,11 @@ type FileInfo struct {
 	// ordered slice containing the ids of the uploads of which the final upload
 	// will consist after concatenation.
 	PartialUploads []string
+    // Indicates that a write operation is currently pending for this file.
+    // This field is not needed to implement the tus specification.
+    // Implementations of tusd.DataStore may use this to prevent write
+    // collisions or race conditions.
+    Locked bool
 }
 
 type DataStore interface {
@@ -61,4 +59,13 @@ type DataStore interface {
 	// Terminate an upload so any further requests to the resource, both reading
 	// and writing, must return os.ErrNotExist or similar.
 	Terminate(id string) error
+	// Lock the upload file for writing. This feature is not part of the
+	// official TUS specification. Handlers should call this prior to writing
+	// chunks or terminating files. Returns true if the file lock has been
+	// acquired.
+	LockFile(id string) (bool, error)
+	// Unlock the upload file for writing. This feature is not part of the
+	// official TUS specification. Handlers should defer calls to this after
+	// acquiring a lock.
+	UnlockFile(id string) error
 }

filestore/filestore.go

@@ -25,14 +25,12 @@ 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
-	locks map[string]bool
 }
 
 // NewFileStore creates a new FileStore instance.
 func NewFileStore(path string) (store *FileStore) {
 	store = &FileStore{
 		Path:  path,
-		locks: make(map[string]bool),
 	}
 	return
 }
@@ -54,11 +52,6 @@ func (store *FileStore) NewUpload(info tusd.FileInfo) (id string, err error) {
 }
 
 func (store *FileStore) WriteChunk(id string, offset int64, src io.Reader) (int64, error) {
-	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
@@ -84,18 +77,18 @@ func (store *FileStore) GetInfo(id string) (info tusd.FileInfo, err error) {
 }
 
 func (store *FileStore) GetReader(id string) (io.Reader, error) {
-	if !store.getLock(id) {
+	hasLock, err := store.LockFile(id)
+	if err != nil {
+		return bytes.NewReader(make([]byte, 0)), err
+	}
+	if !hasLock {
 		return bytes.NewReader(make([]byte, 0)), tusd.ErrFileLocked
 	}
-	defer store.clearLock(id)
+	defer store.UnlockFile(id)
 	return os.Open(store.binPath(id))
 }
 
 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
 	}
@@ -105,6 +98,37 @@ func (store *FileStore) Terminate(id string) error {
 	return nil
 }
 
+func (store *FileStore) LockFile(id string) (hasLock bool, err) {
+	info, err := store.GetInfo(id)
+	if err != nil {
+		hasLock = false
+		return
+	}
+	if info.Locked {
+		// Cannot acquire lock if something else has the lock.
+		hasLock = false
+		return
+	}
+	info.Locked = true
+	err = writeInfo(id, info)
+	if err != nil {
+		hasLock = false
+		return
+	}
+	hasLock = true
+	return
+}
+
+func (store *FileStore) UnlockFile(id string) (err error) {
+	info, err := store.GetInfo(id)
+	if err != nil {
+		return
+	}
+	info.Locked = false
+	err = writeInfo(info)
+	return
+}
+
 // Return the path to the .bin storing the binary data
 func (store *FileStore) binPath(id string) string {
 	return store.Path + "/" + id + ".bin"
@@ -139,19 +163,3 @@ 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)
-}

handler.go

@@ -25,6 +25,7 @@ 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")
@@ -272,6 +273,11 @@ func (handler *Handler) headFile(w http.ResponseWriter, r *http.Request) {
 // space is left.
 func (handler *Handler) patchFile(w http.ResponseWriter, r *http.Request) {
 	id := r.URL.Query().Get(":id")
+	err := handler.getLock(id)
+	if err != nil {
+		handler.sendError(w, err)
+	}
+	defer handler.releaseLock(id)
 
 	info, err := handler.dataStore.GetInfo(id)
 	if err != nil {
@@ -369,8 +375,14 @@ func (handler *Handler) getFile(w http.ResponseWriter, r *http.Request) {
 // Terminate an upload permanently.
 func (handler *Handler) delFile(w http.ResponseWriter, r *http.Request) {
 	id := r.URL.Query().Get(":id")
+	err := handler.getLock(id)
+	if err != nil {
+		handler.sendError(w, err)
+		return
+	}
+	defer handler.releaseLock(id)
 	
-	err := handler.dataStore.Terminate(id)
+	err = handler.dataStore.Terminate(id)
 	if err != nil {
 		handler.sendError(w, err)
 		return
@@ -454,6 +466,24 @@ func (handler *Handler) fillFinalUpload(id string, uploads []string) error {
 	return err
 }
 
+// Get the lock from the data store, returning an error if a true error occurred
+// or if the file could not be locked.
+func (handler *Handler) getLock(id string) (error) {
+	hasLock, err := handler.dataStore.LockFile(id)
+	if err != nil {
+		return err
+	}
+	if !hasLock {
+		return ErrFileLocked
+	}
+	return nil
+}
+
+// Release the lock from the data store
+func (handler *Handler) releaseLock(id string) (error) {
+	return handler.dataStore.UnlockFile(id)
+}
+
 // Parse the Upload-Metadata header as defined in the File Creation extension.
 // e.g. Upload-Metadata: name bHVucmpzLnBuZw==,type aW1hZ2UvcG5n
 func parseMeta(header string) map[string]string {

handler_test.go

@@ -28,6 +28,14 @@ func (store zeroStore) Terminate(id string) error {
 	return ErrNotImplemented
 }
 
+func (store zeroStore) LockFile(id string) (bool, error) {
+	return true, nil
+}
+
+func (store zeroStore) UnlockFile(id string) (error) {
+	return nil
+}
+
 type httpTest struct {
 	Name string