From df1092ef33416e562ea22050640c94f38fa2df0f Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sun, 12 Jun 2016 15:06:27 +0100 Subject: [PATCH] Change Fs.Put so that it must cope with existing files This should fix duplicate files on drive and 409 errors on amazonclouddrive however it will slow down the upload slightly as another roundtrip will be needed. None of the other Fses needed adjusting. Fixes #483 --- amazonclouddrive/amazonclouddrive.go | 11 ++++ amazonclouddrive/amazonclouddrive_test.go | 1 + b2/b2_test.go | 1 + drive/drive.go | 60 ++++++++++++------- drive/drive_test.go | 1 + dropbox/dropbox_test.go | 1 + fs/fs.go | 13 ++++ fs/operations_test.go | 57 +++++++++++------- fstest/fstests/fstests.go | 7 +++ googlecloudstorage/googlecloudstorage_test.go | 1 + hubic/hubic_test.go | 1 + local/local_test.go | 1 + onedrive/onedrive_test.go | 1 + s3/s3_test.go | 1 + swift/swift_test.go | 1 + yandex/yandex_test.go | 1 + 16 files changed, 114 insertions(+), 45 deletions(-) diff --git a/amazonclouddrive/amazonclouddrive.go b/amazonclouddrive/amazonclouddrive.go index 94154ce5c..750f6d138 100644 --- a/amazonclouddrive/amazonclouddrive.go +++ b/amazonclouddrive/amazonclouddrive.go @@ -425,6 +425,17 @@ func (f *Fs) Put(in io.Reader, src fs.ObjectInfo) (fs.Object, error) { fs: f, remote: remote, } + // Check if object already exists + err := o.readMetaData() + switch err { + case nil: + return o, o.Update(in, src) + case fs.ErrorDirNotFound, acd.ErrorNodeNotFound: + // Not found so create it + default: + return nil, err + } + // If not create it leaf, directoryID, err := f.dirCache.FindPath(remote, true) if err != nil { return nil, err diff --git a/amazonclouddrive/amazonclouddrive_test.go b/amazonclouddrive/amazonclouddrive_test.go index 86a17b8ab..75fcc26b6 100644 --- a/amazonclouddrive/amazonclouddrive_test.go +++ b/amazonclouddrive/amazonclouddrive_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/b2/b2_test.go b/b2/b2_test.go index b96d30cfa..3c1c25de4 100644 --- a/b2/b2_test.go +++ b/b2/b2_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/drive/drive.go b/drive/drive.go index 3e75cc4e1..2016e2f54 100644 --- a/drive/drive.go +++ b/drive/drive.go @@ -15,17 +15,16 @@ import ( "strings" "time" - "golang.org/x/oauth2" - "golang.org/x/oauth2/google" - "google.golang.org/api/drive/v2" - "google.golang.org/api/googleapi" - "github.com/ncw/rclone/dircache" "github.com/ncw/rclone/fs" "github.com/ncw/rclone/oauthutil" "github.com/ncw/rclone/pacer" "github.com/pkg/errors" "github.com/spf13/pflag" + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" + "google.golang.org/api/drive/v2" + "google.golang.org/api/googleapi" ) // Constants @@ -81,6 +80,7 @@ var ( "text/plain": "txt", } extensionToMimeType map[string]string + errorObjectNotFound = errors.New("Object not found") ) // Register with Fs @@ -351,29 +351,29 @@ func NewFs(name, path string) (fs.Fs, error) { // Return an FsObject from a path func (f *Fs) newFsObjectWithInfoErr(remote string, info *drive.File) (fs.Object, error) { - fs := &Object{ + o := &Object{ fs: f, remote: remote, } if info != nil { - fs.setMetaData(info) + o.setMetaData(info) } else { - err := fs.readMetaData() // reads info and meta, returning an error + err := o.readMetaData() // reads info and meta, returning an error if err != nil { // logged already fs.Debug("Failed to read info: %s", err) return nil, err } } - return fs, nil + return o, nil } // Return an FsObject from a path // // May return nil if an error occurred func (f *Fs) newFsObjectWithInfo(remote string, info *drive.File) fs.Object { - fs, _ := f.newFsObjectWithInfoErr(remote, info) + o, _ := f.newFsObjectWithInfoErr(remote, info) // Errors have already been logged - return fs + return o } // NewFsObject returns an FsObject from a path @@ -542,14 +542,27 @@ func (f *Fs) createFileInfo(remote string, modTime time.Time, size int64) (*Obje // Put the object // -// This assumes that the object doesn't not already exists - if you -// call it when it does exist then it will create a duplicate. Call -// object.Update() in this case. -// // Copy the reader in to the new object which is returned // // The new object may have been created if an error is returned func (f *Fs) Put(in io.Reader, src fs.ObjectInfo) (fs.Object, error) { + exisitingObj, err := f.newFsObjectWithInfoErr(src.Remote(), nil) + switch err { + case nil: + return exisitingObj, exisitingObj.Update(in, src) + case fs.ErrorDirNotFound, errorObjectNotFound: + // Not found so create it + return f.PutUnchecked(in, src) + default: + return nil, err + } +} + +// PutUnchecked uploads the object +// +// This will create a duplicate if we upload a new file without +// checking to see if there is one already - use Put() for that. +func (f *Fs) PutUnchecked(in io.Reader, src fs.ObjectInfo) (fs.Object, error) { remote := src.Remote() size := src.Size() modTime := src.ModTime() @@ -857,8 +870,8 @@ func (o *Object) readMetaData() (err error) { return err } if !found { - fs.Debug(o, "Couldn't find object") - return errors.New("couldn't find object") + fs.Debug(o, "%v", errorObjectNotFound) + return errorObjectNotFound } return nil } @@ -1042,10 +1055,11 @@ func (o *Object) Remove() error { // Check the interfaces are satisfied var ( - _ fs.Fs = (*Fs)(nil) - _ fs.Purger = (*Fs)(nil) - _ fs.Copier = (*Fs)(nil) - _ fs.Mover = (*Fs)(nil) - _ fs.DirMover = (*Fs)(nil) - _ fs.Object = (*Object)(nil) + _ fs.Fs = (*Fs)(nil) + _ fs.Purger = (*Fs)(nil) + _ fs.Copier = (*Fs)(nil) + _ fs.Mover = (*Fs)(nil) + _ fs.DirMover = (*Fs)(nil) + _ fs.PutUncheckeder = (*Fs)(nil) + _ fs.Object = (*Object)(nil) ) diff --git a/drive/drive_test.go b/drive/drive_test.go index 7c9dd3b97..8fed660ff 100644 --- a/drive/drive_test.go +++ b/drive/drive_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/dropbox/dropbox_test.go b/dropbox/dropbox_test.go index 56a62e00b..3979bef9d 100644 --- a/dropbox/dropbox_test.go +++ b/dropbox/dropbox_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/fs/fs.go b/fs/fs.go index b43171874..2d8206816 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -253,6 +253,19 @@ type UnWrapper interface { UnWrap() Fs } +// PutUncheckeder is an optional interface for Fs +type PutUncheckeder interface { + // Put in to the remote path with the modTime given of the given size + // + // May create the object even if it returns an error - if so + // will return the object and the error, otherwise will return + // nil and the error + // + // May create duplicates or return errors if src already + // exists. + PutUnchecked(in io.Reader, src ObjectInfo) (Object, error) +} + // ObjectsChan is a channel of Objects type ObjectsChan chan Object diff --git a/fs/operations_test.go b/fs/operations_test.go index b4f850851..4cf521105 100644 --- a/fs/operations_test.go +++ b/fs/operations_test.go @@ -187,7 +187,15 @@ func (r *Run) WriteFile(filePath, content string, t time.Time) fstest.Item { } // WriteObjectTo writes an object to the fs, remote passed in -func (r *Run) WriteObjectTo(f fs.Fs, remote, content string, modTime time.Time) fstest.Item { +func (r *Run) WriteObjectTo(f fs.Fs, remote, content string, modTime time.Time, useUnchecked bool) fstest.Item { + put := f.Put + if useUnchecked { + if fPutUnchecked, ok := f.(fs.PutUncheckeder); ok { + put = fPutUnchecked.PutUnchecked + } else { + r.Fatalf("Fs doesn't support PutUnchecked") + } + } const maxTries = 5 if !r.mkdir[f.String()] { err := f.Mkdir() @@ -199,7 +207,7 @@ func (r *Run) WriteObjectTo(f fs.Fs, remote, content string, modTime time.Time) for tries := 1; ; tries++ { in := bytes.NewBufferString(content) objinfo := fs.NewStaticObjectInfo(remote, modTime, int64(len(content)), true, nil, nil) - _, err := f.Put(in, objinfo) + _, err := put(in, objinfo) if err == nil { break } @@ -215,7 +223,12 @@ func (r *Run) WriteObjectTo(f fs.Fs, remote, content string, modTime time.Time) // WriteObject writes an object to the remote func (r *Run) WriteObject(remote, content string, modTime time.Time) fstest.Item { - return r.WriteObjectTo(r.fremote, remote, content, modTime) + return r.WriteObjectTo(r.fremote, remote, content, modTime, false) +} + +// WriteUncheckedObject writes an object to the remote not checking for duplicates +func (r *Run) WriteUncheckedObject(remote, content string, modTime time.Time) fstest.Item { + return r.WriteObjectTo(r.fremote, remote, content, modTime, true) } // WriteBoth calls WriteObject and WriteFile with the same arguments @@ -817,7 +830,7 @@ func TestServerSideMove(t *testing.T) { t.Logf("Server side move (if possible) %v -> %v", r.fremote, fremoteMove) // Write just one file in the new remote - r.WriteObjectTo(fremoteMove, "empty space", "", t2) + r.WriteObjectTo(fremoteMove, "empty space", "", t2, false) fstest.CheckItems(t, fremoteMove, file2) // Do server side move @@ -1087,9 +1100,9 @@ func TestDeduplicateInteractive(t *testing.T) { r := NewRun(t) defer r.Finalise() - file1 := r.WriteObject("one", "This is one", t1) - file2 := r.WriteObject("one", "This is one", t1) - file3 := r.WriteObject("one", "This is one", t1) + file1 := r.WriteUncheckedObject("one", "This is one", t1) + file2 := r.WriteUncheckedObject("one", "This is one", t1) + file3 := r.WriteUncheckedObject("one", "This is one", t1) r.checkWithDuplicates(t, file1, file2, file3) err := fs.Deduplicate(r.fremote, fs.DeduplicateInteractive) @@ -1107,9 +1120,9 @@ func TestDeduplicateSkip(t *testing.T) { r := NewRun(t) defer r.Finalise() - file1 := r.WriteObject("one", "This is one", t1) - file2 := r.WriteObject("one", "This is one", t1) - file3 := r.WriteObject("one", "This is another one", t1) + file1 := r.WriteUncheckedObject("one", "This is one", t1) + file2 := r.WriteUncheckedObject("one", "This is one", t1) + file3 := r.WriteUncheckedObject("one", "This is another one", t1) r.checkWithDuplicates(t, file1, file2, file3) err := fs.Deduplicate(r.fremote, fs.DeduplicateSkip) @@ -1127,9 +1140,9 @@ func TestDeduplicateFirst(t *testing.T) { r := NewRun(t) defer r.Finalise() - file1 := r.WriteObject("one", "This is one", t1) - file2 := r.WriteObject("one", "This is one A", t1) - file3 := r.WriteObject("one", "This is one BB", t1) + file1 := r.WriteUncheckedObject("one", "This is one", t1) + file2 := r.WriteUncheckedObject("one", "This is one A", t1) + file3 := r.WriteUncheckedObject("one", "This is one BB", t1) r.checkWithDuplicates(t, file1, file2, file3) err := fs.Deduplicate(r.fremote, fs.DeduplicateFirst) @@ -1156,9 +1169,9 @@ func TestDeduplicateNewest(t *testing.T) { r := NewRun(t) defer r.Finalise() - file1 := r.WriteObject("one", "This is one", t1) - file2 := r.WriteObject("one", "This is one too", t2) - file3 := r.WriteObject("one", "This is another one", t3) + file1 := r.WriteUncheckedObject("one", "This is one", t1) + file2 := r.WriteUncheckedObject("one", "This is one too", t2) + file3 := r.WriteUncheckedObject("one", "This is another one", t3) r.checkWithDuplicates(t, file1, file2, file3) err := fs.Deduplicate(r.fremote, fs.DeduplicateNewest) @@ -1176,9 +1189,9 @@ func TestDeduplicateOldest(t *testing.T) { r := NewRun(t) defer r.Finalise() - file1 := r.WriteObject("one", "This is one", t1) - file2 := r.WriteObject("one", "This is one too", t2) - file3 := r.WriteObject("one", "This is another one", t3) + file1 := r.WriteUncheckedObject("one", "This is one", t1) + file2 := r.WriteUncheckedObject("one", "This is one too", t2) + file3 := r.WriteUncheckedObject("one", "This is another one", t3) r.checkWithDuplicates(t, file1, file2, file3) err := fs.Deduplicate(r.fremote, fs.DeduplicateOldest) @@ -1196,9 +1209,9 @@ func TestDeduplicateRename(t *testing.T) { r := NewRun(t) defer r.Finalise() - file1 := r.WriteObject("one.txt", "This is one", t1) - file2 := r.WriteObject("one.txt", "This is one too", t2) - file3 := r.WriteObject("one.txt", "This is another one", t3) + file1 := r.WriteUncheckedObject("one.txt", "This is one", t1) + file2 := r.WriteUncheckedObject("one.txt", "This is one too", t2) + file3 := r.WriteUncheckedObject("one.txt", "This is another one", t3) r.checkWithDuplicates(t, file1, file2, file3) err := fs.Deduplicate(r.fremote, fs.DeduplicateRename) diff --git a/fstest/fstests/fstests.go b/fstest/fstests/fstests.go index 0192c08af..f1f4e7f5f 100644 --- a/fstest/fstests/fstests.go +++ b/fstest/fstests/fstests.go @@ -232,6 +232,13 @@ func TestFsPutFile2(t *testing.T) { testPut(t, &file2) } +// TestFsUpdateFile1 tests updating file1 with new contents +func TestFsUpdateFile1(t *testing.T) { + skipIfNotOk(t) + testPut(t, &file1) + // Note that the next test will check there are no duplicated file names +} + // TestFsListDirFile2 tests the files are correctly uploaded func TestFsListDirFile2(t *testing.T) { skipIfNotOk(t) diff --git a/googlecloudstorage/googlecloudstorage_test.go b/googlecloudstorage/googlecloudstorage_test.go index 7fe017f8b..7b4e114e1 100644 --- a/googlecloudstorage/googlecloudstorage_test.go +++ b/googlecloudstorage/googlecloudstorage_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/hubic/hubic_test.go b/hubic/hubic_test.go index 943fb0fe7..929d923ea 100644 --- a/hubic/hubic_test.go +++ b/hubic/hubic_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/local/local_test.go b/local/local_test.go index b11d30208..8e8d6e004 100644 --- a/local/local_test.go +++ b/local/local_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/onedrive/onedrive_test.go b/onedrive/onedrive_test.go index d44849348..9d0e7461b 100644 --- a/onedrive/onedrive_test.go +++ b/onedrive/onedrive_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/s3/s3_test.go b/s3/s3_test.go index 3301fe165..a7a6f8197 100644 --- a/s3/s3_test.go +++ b/s3/s3_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/swift/swift_test.go b/swift/swift_test.go index 2507831c5..b33dba5e2 100644 --- a/swift/swift_test.go +++ b/swift/swift_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) } diff --git a/yandex/yandex_test.go b/yandex/yandex_test.go index 59b320928..83ff9d6b0 100644 --- a/yandex/yandex_test.go +++ b/yandex/yandex_test.go @@ -28,6 +28,7 @@ func TestFsListDirEmpty(t *testing.T) { fstests.TestFsListDirEmpty(t) } func TestFsNewFsObjectNotFound(t *testing.T) { fstests.TestFsNewFsObjectNotFound(t) } func TestFsPutFile1(t *testing.T) { fstests.TestFsPutFile1(t) } func TestFsPutFile2(t *testing.T) { fstests.TestFsPutFile2(t) } +func TestFsUpdateFile1(t *testing.T) { fstests.TestFsUpdateFile1(t) } func TestFsListDirFile2(t *testing.T) { fstests.TestFsListDirFile2(t) } func TestFsListDirRoot(t *testing.T) { fstests.TestFsListDirRoot(t) } func TestFsListSubdir(t *testing.T) { fstests.TestFsListSubdir(t) }