From 431524445eaf3b767e7127fc1eb7f5d2fdc7af2d Mon Sep 17 00:00:00 2001 From: nielash Date: Wed, 6 Mar 2024 13:26:50 -0500 Subject: [PATCH] combine: fix operations.DirMove across upstreams - fixes #7661 Before this change, operations.DirMove would fail when moving a directory, if the src and dest were on different upstreams of a combine remote. The issue only affected operations.DirMove, and not sync.MoveDir, because they checked for server-side-move support in different ways. MoveDir checks by just trying it and seeing what error comes back. This works fine for combine because combine returns fs.ErrorCantDirMove which MoveDir understands what to do with. DirMove, however, only checked whether the function pointer is nil. This is an unreliable way to check for combine, because combine does advertise support for DirMove, despite not always being able to do it. This change fixes the issue by checking the returned error in a manner similar to sync.MoveDir and falling back to individual file moves (copy + delete) depending on which error was returned. --- fs/operations/operations.go | 5 ++++- fs/operations/operations_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 116f291e8..1d950e40c 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -2310,7 +2310,10 @@ func DirMove(ctx context.Context, f fs.Fs, srcRemote, dstRemote string) (err err if err == nil { accounting.Stats(ctx).Renames(1) } - return err + if err != fs.ErrorCantDirMove && err != fs.ErrorDirExists { + return err + } + fs.Infof(f, "Can't DirMove - falling back to file moves: %v", err) } // Load the directory tree into memory diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index 4f4cf4cfd..98676ae26 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -1405,6 +1405,33 @@ func TestDirMove(t *testing.T) { fs.GetModifyWindow(ctx, r.Fremote), ) + // Try with a DirMove method that exists but returns fs.ErrorCantDirMove (ex. combine moving across upstreams) + // Should fall back to manual move (copy + delete) + + features.DirMove = func(ctx context.Context, src fs.Fs, srcRemote string, dstRemote string) error { + return fs.ErrorCantDirMove + } + + assert.NoError(t, operations.DirMove(ctx, r.Fremote, "A3", "A4")) + + for i := range files { + files[i].Path = strings.ReplaceAll(files[i].Path, "A3/", "A4/") + } + + fstest.CheckListingWithPrecision( + t, + r.Fremote, + files, + []string{ + "A4", + "A4/B1", + "A4/B2", + "A4/B1/C1", + "A4/B1/C2", + "A4/B1/C3", + }, + fs.GetModifyWindow(ctx, r.Fremote), + ) } func TestGetFsInfo(t *testing.T) {