From 59ba8f28c8d93537118dd6ba1f1dd8121a823351 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 24 Aug 2015 21:42:23 +0100 Subject: [PATCH] Implement move command - fixes #35 * Define Mover interface to move a single object * Define DirMover interface to move a directory * Implement DirMove operation * Add `rclone move` command * Tests for Dir Move To Do * Implement Move, DirMover in local, drive, dropbox * unit test for Mover * unit test for DirMover --- fs/fs.go | 27 +++++++++++++++ fs/operations.go | 79 +++++++++++++++++++++++++++++++++++++++++-- fs/operations_test.go | 77 +++++++++++++++++++++++++++++++++-------- fstest/fstest.go | 5 +++ rclone.go | 19 +++++++++-- 5 files changed, 188 insertions(+), 19 deletions(-) diff --git a/fs/fs.go b/fs/fs.go index 5b2636b32..8350bd01b 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -25,6 +25,9 @@ var ( // Error returned by NewFs if not found in config file NotFoundInConfigFile = fmt.Errorf("Didn't find section in config file") ErrorCantCopy = fmt.Errorf("Can't copy object - incompatible remotes") + ErrorCantMove = fmt.Errorf("Can't copy object - incompatible remotes") + ErrorCantDirMove = fmt.Errorf("Can't copy directory - incompatible remotes") + ErrorDirExists = fmt.Errorf("Can't copy directory - destination already exists") ) // Filesystem info @@ -163,6 +166,30 @@ type Copier interface { Copy(src Object, remote string) (Object, error) } +type Mover interface { + // Move src to this remote using server side move operations. + // + // This is stored with the remote path given + // + // It returns the destination Object and a possible error + // + // Will only be called if src.Fs().Name() == f.Name() + // + // If it isn't possible then return fs.ErrorCantMove + Move(src Object, remote string) (Object, error) +} + +type DirMover interface { + // Move src to this remote using server side move operations. + // + // Will only be called if src.Fs().Name() == f.Name() + // + // If it isn't possible then return fs.ErrorCantDirMove + // + // If destination exists then return fs.ErrorDirExists + DirMove(src Fs) error +} + // An optional interface for error as to whether the operation should be retried // // This should be returned from Update or Put methods as required diff --git a/fs/operations.go b/fs/operations.go index d03b54475..31175ed22 100644 --- a/fs/operations.go +++ b/fs/operations.go @@ -309,6 +309,35 @@ func PairCopier(in ObjectPairChan, fdst Fs, wg *sync.WaitGroup) { } } +// Read Objects on in and move them if possible, or copy them if not +func PairMover(in ObjectPairChan, fdst Fs, wg *sync.WaitGroup) { + defer wg.Done() + // See if we have Move available + fdstMover, haveMover := fdst.(Mover) + for pair := range in { + src := pair.src + dst := pair.dst + Stats.Transferring(src) + if Config.DryRun { + Debug(src, "Not moving as --dry-run") + } else if haveMover { + // Delete destination if it exists + if pair.dst != nil { + err := dst.Remove() + if err != nil { + Stats.Error() + ErrorLog(dst, "Couldn't delete: %s", err) + } + } + fdstMover.Move(src, src.Remote()) + Debug(src, "Moved") + } else { + Copy(fdst, pair.dst, src) + } + Stats.DoneTransferring(src) + } +} + // Delete all the files passed in the channel func DeleteFiles(to_be_deleted ObjectsChan) { var wg sync.WaitGroup @@ -354,7 +383,9 @@ func readFilesMap(fs Fs) map[string]Object { // Syncs fsrc into fdst // // If Delete is true then it deletes any files in fdst that aren't in fsrc -func Sync(fdst, fsrc Fs, Delete bool) error { +// +// If DoMove is true then files will be moved instead of copied +func syncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) error { err := fdst.Mkdir() if err != nil { Stats.Error() @@ -380,7 +411,11 @@ func Sync(fdst, fsrc Fs, Delete bool) error { var copierWg sync.WaitGroup copierWg.Add(Config.Transfers) for i := 0; i < Config.Transfers; i++ { - go PairCopier(to_be_uploaded, fdst, &copierWg) + if DoMove { + go PairMover(to_be_uploaded, fdst, &copierWg) + } else { + go PairCopier(to_be_uploaded, fdst, &copierWg) + } } go func() { @@ -407,7 +442,7 @@ func Sync(fdst, fsrc Fs, Delete bool) error { // Delete files if asked if Delete { if Stats.Errored() { - Log(fdst, "Not deleting files as there were IO errors") + ErrorLog(fdst, "Not deleting files as there were IO errors") return nil } @@ -424,6 +459,44 @@ func Sync(fdst, fsrc Fs, Delete bool) error { return nil } +// Syncs fsrc into fdst +func Sync(fdst, fsrc Fs) error { + return syncCopyMove(fdst, fsrc, true, false) +} + +// Copies fsrc into fdst +func CopyDir(fdst, fsrc Fs) error { + return syncCopyMove(fdst, fsrc, false, false) +} + +// Moves fsrc into fdst +func MoveDir(fdst, fsrc Fs) error { + // First attempt to use DirMover + if fdstDirMover, ok := fdst.(DirMover); ok && fsrc.Name() == fdst.Name() { + err := fdstDirMover.DirMove(fsrc) + Debug(fdst, "Using server side directory move") + switch err { + case ErrorCantDirMove, ErrorDirExists: + Debug(fdst, "Server side directory move failed - fallback to copy/delete: %v", err) + case nil: + Debug(fdst, "Server side directory move succeeded") + return nil + default: + Stats.Error() + ErrorLog(fdst, "Server side directory move failed: %v", err) + return err + } + } + + // Now move the files + err := syncCopyMove(fdst, fsrc, false, true) + if err != nil || Stats.Errored() { + ErrorLog(fdst, "Not deleting files as there were IO errors") + return err + } + return Purge(fsrc) +} + // Checks the files in fsrc and fdst according to Size and MD5SUM func Check(fdst, fsrc Fs) error { Log(fdst, "Building file list") diff --git a/fs/operations_test.go b/fs/operations_test.go index 7bbc7b0d7..da9664b9b 100644 --- a/fs/operations_test.go +++ b/fs/operations_test.go @@ -98,7 +98,7 @@ func TestCopyWithDryRun(t *testing.T) { WriteFile("sub dir/hello world", "hello world", t1) fs.Config.DryRun = true - err := fs.Sync(fremote, flocal, false) + err := fs.CopyDir(fremote, flocal) fs.Config.DryRun = false if err != nil { t.Fatalf("Copy failed: %v", err) @@ -114,7 +114,7 @@ func TestCopyWithDryRun(t *testing.T) { // Now without dry run func TestCopy(t *testing.T) { - err := fs.Sync(fremote, flocal, false) + err := fs.CopyDir(fremote, flocal) if err != nil { t.Fatalf("Copy failed: %v", err) } @@ -136,7 +136,7 @@ func TestServerSideCopy(t *testing.T) { defer finaliseCopy() t.Logf("Server side copy (if possible) %v -> %v", fremote, fremoteCopy) - err = fs.Sync(fremoteCopy, fremote, false) + err = fs.CopyDir(fremoteCopy, fremote) if err != nil { t.Fatalf("Server Side Copy failed: %v", err) } @@ -176,7 +176,7 @@ func TestCopyAfterDelete(t *testing.T) { } func TestCopyRedownload(t *testing.T) { - err := fs.Sync(flocal, fremote, false) + err := fs.CopyDir(flocal, fremote) if err != nil { t.Fatalf("Copy failed: %v", err) } @@ -206,7 +206,7 @@ func TestSyncBasedOnCheckSum(t *testing.T) { fstest.CheckListingWithPrecision(t, flocal, local_items, fs.Config.ModifyWindow) fs.Stats.ResetCounters() - err := fs.Sync(fremote, flocal, true) + err := fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Initial sync failed: %v", err) } @@ -229,7 +229,7 @@ func TestSyncBasedOnCheckSum(t *testing.T) { fstest.CheckListingWithPrecision(t, flocal, local_items, fs.Config.ModifyWindow) fs.Stats.ResetCounters() - err = fs.Sync(fremote, flocal, true) + err = fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -260,7 +260,7 @@ func TestSyncSizeOnly(t *testing.T) { fstest.CheckListingWithPrecision(t, flocal, local_items, fs.Config.ModifyWindow) fs.Stats.ResetCounters() - err := fs.Sync(fremote, flocal, true) + err := fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Initial sync failed: %v", err) } @@ -281,7 +281,7 @@ func TestSyncSizeOnly(t *testing.T) { fstest.CheckListingWithPrecision(t, flocal, local_items, fs.Config.ModifyWindow) fs.Stats.ResetCounters() - err = fs.Sync(fremote, flocal, true) + err = fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -304,7 +304,7 @@ func TestSyncAfterChangingModtimeOnly(t *testing.T) { if err != nil { t.Fatalf("Chtimes failed: %v", err) } - err = fs.Sync(fremote, flocal, true) + err = fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -317,7 +317,7 @@ func TestSyncAfterChangingModtimeOnly(t *testing.T) { func TestSyncAfterAddingAFile(t *testing.T) { WriteFile("potato", "------------------------------------------------------------", t3) - err := fs.Sync(fremote, flocal, true) + err := fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -331,7 +331,7 @@ func TestSyncAfterAddingAFile(t *testing.T) { func TestSyncAfterChangingFilesSizeOnly(t *testing.T) { WriteFile("potato", "smaller but same date", t3) - err := fs.Sync(fremote, flocal, true) + err := fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -346,7 +346,7 @@ func TestSyncAfterChangingFilesSizeOnly(t *testing.T) { // Sync after changing a file's contents, modtime but not length func TestSyncAfterChangingContentsOnly(t *testing.T) { WriteFile("potato", "SMALLER BUT SAME DATE", t2) - err := fs.Sync(fremote, flocal, true) + err := fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -366,7 +366,7 @@ func TestSyncAfterRemovingAFileAndAddingAFileDryRun(t *testing.T) { t.Fatalf("Remove failed: %v", err) } fs.Config.DryRun = true - err = fs.Sync(fremote, flocal, true) + err = fs.Sync(fremote, flocal) fs.Config.DryRun = false if err != nil { t.Fatalf("Sync failed: %v", err) @@ -386,7 +386,7 @@ func TestSyncAfterRemovingAFileAndAddingAFileDryRun(t *testing.T) { // Sync after removing a file and adding a file func TestSyncAfterRemovingAFileAndAddingAFile(t *testing.T) { - err := fs.Sync(fremote, flocal, true) + err := fs.Sync(fremote, flocal) if err != nil { t.Fatalf("Sync failed: %v", err) } @@ -398,6 +398,55 @@ func TestSyncAfterRemovingAFileAndAddingAFile(t *testing.T) { fstest.CheckListingWithPrecision(t, fremote, items, fs.Config.ModifyWindow) } +// Test a server side move if possible, or the backup path if not +func TestServerSideMove(t *testing.T) { + fremoteMove, finaliseMove, err := fstest.RandomRemote(*RemoteName, *SubDir) + if err != nil { + t.Fatalf("Failed to open remote move %q: %v", *RemoteName, err) + } + defer finaliseMove() + t.Logf("Server side move (if possible) %v -> %v", fremote, fremoteMove) + + // Start with a copy + err = fs.CopyDir(fremoteMove, fremote) + if err != nil { + t.Fatalf("Server Side Copy failed: %v", err) + } + + // Remove one file + obj := fremoteMove.NewFsObject("potato2") + if obj == nil { + t.Fatalf("Failed to find potato2") + } + err = obj.Remove() + if err != nil { + t.Fatalf("Failed to remove object: %v", err) + } + + // Do server side move + err = fs.MoveDir(fremoteMove, fremote) + if err != nil { + t.Fatalf("Server Side Move failed: %v", err) + } + + items := []fstest.Item{ + {Path: "empty space", Size: 0, ModTime: t2, Md5sum: "d41d8cd98f00b204e9800998ecf8427e"}, + {Path: "potato2", Size: 60, ModTime: t1, Md5sum: "d6548b156ea68a4e003e786df99eee76"}, + } + + fstest.CheckListingWithPrecision(t, fremote, items[:0], fs.Config.ModifyWindow) + fstest.CheckListingWithPrecision(t, fremoteMove, items, fs.Config.ModifyWindow) + + // Move it back again, dst does not exist this time + err = fs.MoveDir(fremote, fremoteMove) + if err != nil { + t.Fatalf("Server Side Move 2 failed: %v", err) + } + + fstest.CheckListingWithPrecision(t, fremote, items, fs.Config.ModifyWindow) + fstest.CheckListingWithPrecision(t, fremoteMove, items[:0], fs.Config.ModifyWindow) +} + func TestLs(t *testing.T) { var buf bytes.Buffer err := fs.List(fremote, &buf) diff --git a/fstest/fstest.go b/fstest/fstest.go index 06686bda3..0a3175223 100644 --- a/fstest/fstest.go +++ b/fstest/fstest.go @@ -108,6 +108,7 @@ func (is *Items) Done(t *testing.T) { // Checks the fs to see if it has the expected contents func CheckListingWithPrecision(t *testing.T, f fs.Fs, items []Item, precision time.Duration) { is := NewItems(items) + oldErrors := fs.Stats.GetErrors() for obj := range f.List() { if obj == nil { t.Errorf("Unexpected nil in List()") @@ -116,6 +117,10 @@ func CheckListingWithPrecision(t *testing.T, f fs.Fs, items []Item, precision ti is.Find(t, obj, precision) } is.Done(t) + // Don't notice an error when listing an empty directory + if len(items) == 0 && oldErrors == 0 && fs.Stats.GetErrors() == 1 { + fs.Stats.ResetErrors() + } } // Checks the fs to see if it has the expected contents diff --git a/rclone.go b/rclone.go index e51c1b29e..c6128a5d9 100644 --- a/rclone.go +++ b/rclone.go @@ -67,7 +67,7 @@ var Commands = []Command{ unchanged files, testing by size and modification time or MD5SUM. Doesn't delete files from the destination.`, Run: func(fdst, fsrc fs.Fs) error { - return fs.Sync(fdst, fsrc, false) + return fs.CopyDir(fdst, fsrc) }, MinArgs: 2, MaxArgs: 2, @@ -83,7 +83,22 @@ var Commands = []Command{ source, including deleting files if necessary. Since this can cause data loss, test first with the --dry-run flag.`, Run: func(fdst, fsrc fs.Fs) error { - return fs.Sync(fdst, fsrc, true) + return fs.Sync(fdst, fsrc) + }, + MinArgs: 2, + MaxArgs: 2, + Retry: true, + }, + { + Name: "move", + ArgsHelp: "source:path dest:path", + Help: ` + Moves the source to the destination. This is equivalent to a + copy followed by a purge, but may use server side operations + to speed it up. Since this can cause data loss, test first + with the --dry-run flag.`, + Run: func(fdst, fsrc fs.Fs) error { + return fs.MoveDir(fdst, fsrc) }, MinArgs: 2, MaxArgs: 2,