diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 4ef8f4b788bc5..8b2c05fc44dd0 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -359,6 +359,11 @@ func Copy(f fs.Fs, dst fs.Object, remote string, src fs.Object) (newDst fs.Objec // Move src object to dst or fdst if nil. If dst is nil then it uses // remote as the name of the new object. // +// Note that you must check the destination does not exist before +// calling this and pass it as dst. If you pass dst=nil and the +// destination does exist then this may create duplicates or return +// errors. +// // It returns the destination object if possible. Note that this may // be nil. func Move(fdst fs.Fs, dst fs.Object, remote string, src fs.Object) (newDst fs.Object, err error) { diff --git a/vfs/dir_test.go b/vfs/dir_test.go index 6ce604166637f..9ce20576afc4a 100644 --- a/vfs/dir_test.go +++ b/vfs/dir_test.go @@ -423,6 +423,8 @@ func TestDirRename(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() vfs, dir, file1 := dirCreate(t, r) + file3 := r.WriteObject("dir/file3", "file3 contents!", t1) + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1, file3}, []string{"dir"}, r.Fremote.Precision()) root, err := vfs.Root() require.NoError(t, err) @@ -434,11 +436,12 @@ func TestDirRename(t *testing.T) { err = root.Rename("dir", "dir2", root) assert.NoError(t, err) checkListing(t, root, []string{"dir2,0,true"}) - checkListing(t, dir, []string{"file1,14,false"}) + checkListing(t, dir, []string{"file1,14,false", "file3,15,false"}) // check the underlying r.Fremote file1.Path = "dir2/file1" - fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{"dir2"}, r.Fremote.Precision()) + file3.Path = "dir2/file3" + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1, file3}, []string{"dir2"}, r.Fremote.Precision()) // refetch dir node, err := vfs.Stat("dir2") @@ -449,10 +452,20 @@ func TestDirRename(t *testing.T) { err = dir.Rename("file1", "file2", root) assert.NoError(t, err) checkListing(t, root, []string{"dir2,0,true", "file2,14,false"}) - checkListing(t, dir, []string(nil)) + checkListing(t, dir, []string{"file3,15,false"}) // check the underlying r.Fremote file1.Path = "file2" + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1, file3}, []string{"dir2"}, r.Fremote.Precision()) + + // Rename a file on top of another file + err = root.Rename("file2", "file3", dir) + assert.NoError(t, err) + checkListing(t, root, []string{"dir2,0,true"}) + checkListing(t, dir, []string{"file3,14,false"}) + + // check the underlying r.Fremote + file1.Path = "dir2/file3" fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{"dir2"}, r.Fremote.Precision()) // read only check diff --git a/vfs/file.go b/vfs/file.go index c4134d067323f..966e0f4f1579c 100644 --- a/vfs/file.go +++ b/vfs/file.go @@ -9,6 +9,7 @@ import ( "github.com/ncw/rclone/fs" "github.com/ncw/rclone/fs/log" + "github.com/ncw/rclone/fs/operations" "github.com/pkg/errors" ) @@ -108,18 +109,16 @@ func (f *File) applyPendingRename() { // Otherwise it will queue the rename operation on the remote until no writers // remain. func (f *File) rename(destDir *Dir, newName string) error { - // FIXME: could Copy then Delete if Move not available - // - though care needed if case insensitive... - doMove := f.d.f.Features().Move - if doMove == nil { - err := errors.Errorf("Fs %q can't rename files (no Move)", f.d.f) + if features := f.d.f.Features(); features.Move == nil && features.Copy == nil { + err := errors.Errorf("Fs %q can't rename files (no server side Move or Copy)", f.d.f) fs.Errorf(f.Path(), "Dir.Rename error: %v", err) return err } renameCall := func() error { newPath := path.Join(destDir.path, newName) - newObject, err := doMove(f.o, newPath) + dstOverwritten, _ := f.d.f.NewObject(newPath) + newObject, err := operations.Move(f.d.f, dstOverwritten, newPath, f.o) if err != nil { fs.Errorf(f.Path(), "File.Rename error: %v", err) return err