From 766dabc2387b4ebc553a5ec2ae1634a876b4b811 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Thu, 1 Oct 2015 13:40:56 -0400 Subject: [PATCH] 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 | 4 +-- datastore.go | 7 ++++ filestore/filestore.go | 69 +++++++++++++++++++++++++++++-------- filestore/filestore_test.go | 4 +-- handler.go | 45 ------------------------ 5 files changed, 64 insertions(+), 65 deletions(-) diff --git a/cmd/tusd/main.go b/cmd/tusd/main.go index 6f21e27..65388f4 100644 --- a/cmd/tusd/main.go +++ b/cmd/tusd/main.go @@ -39,9 +39,7 @@ func main() { } var store tusd.DataStore - store = filestore.FileStore{ - Path: dir, - } + store = filestore.NewFileStore(dir) if storeSize > 0 { store = limitedstore.New(storeSize, store) diff --git a/datastore.go b/datastore.go index 2d2638a..71db972 100644 --- a/datastore.go +++ b/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 { diff --git a/filestore/filestore.go b/filestore/filestore.go index a3a80b6..4900179 100644 --- a/filestore/filestore.go +++ b/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) { - file, err := os.OpenFile(store.binPath(id), os.O_WRONLY|os.O_APPEND, defaultFilePerm) +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 } @@ -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) { - info := tusd.FileInfo{} - data, err := ioutil.ReadFile(store.infoPath(id)) +func (store *FileStore) GetInfo(id string) (info tusd.FileInfo, err error) { + 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) { - return os.Open(store.binPath(id)) +func (store *FileStore) GetReader(id string) (io.Reader, error) { + 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) +} diff --git a/filestore/filestore_test.go b/filestore/filestore_test.go index 74c4dfc..917334d 100644 --- a/filestore/filestore_test.go +++ b/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{ diff --git a/handler.go b/handler.go index 5935cf3..cdf3163 100644 --- a/handler.go +++ b/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)