From 4f8dba9d9dc29bfe09de951b2cc274a9a8ead73a Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Wed, 21 Oct 2015 14:28:22 -0400 Subject: [PATCH] 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 | 21 +++++++++----- filestore/filestore.go | 66 +++++++++++++++++++++++------------------- handler.go | 34 ++++++++++++++++++++-- handler_test.go | 8 +++++ 4 files changed, 91 insertions(+), 38 deletions(-) diff --git a/datastore.go b/datastore.go index f6debf5..13ea118 100644 --- a/datastore.go +++ b/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 } diff --git a/filestore/filestore.go b/filestore/filestore.go index 771405e..e0ed631 100644 --- a/filestore/filestore.go +++ b/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) -} diff --git a/handler.go b/handler.go index cdf3163..cbcb0b4 100644 --- a/handler.go +++ b/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.dataStore.Terminate(id) + err := handler.getLock(id) + if err != nil { + handler.sendError(w, err) + return + } + defer handler.releaseLock(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 { diff --git a/handler_test.go b/handler_test.go index 4187248..1c9c300 100644 --- a/handler_test.go +++ b/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