From 59e14c25df69c96a6d801999c8ac6589f2934c3a Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Thu, 20 Sep 2018 10:46:44 +0100
Subject: [PATCH] vfs: enable rename for nearly all remotes using server side
 Move or Copy

Before this change remotes without server side Move (eg swift, s3,
gcs) would not be able to rename files.

After it means nearly all remotes will be able to rename files on
rclone mount with the notable exceptions of b2 and yandex.

This changes checks to see if the remote can do Move or Copy then
calls `operations.Move` to do the actual move.  This will do a server
side Move or Copy but won't download and re-upload the file.

It also checks to see if the destination exists first which avoids
conflicts or duplicates.

Fixes #1965
Fixes #2569
---
 fs/operations/operations.go |  5 +++++
 vfs/dir_test.go             | 19 ++++++++++++++++---
 vfs/file.go                 | 11 +++++------
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/fs/operations/operations.go b/fs/operations/operations.go
index 4ef8f4b78..8b2c05fc4 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 6ce604166..9ce20576a 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 c4134d067..966e0f4f1 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