diff --git a/backend/union/union.go b/backend/union/union.go index 5009ee28c..966f8ccc1 100644 --- a/backend/union/union.go +++ b/backend/union/union.go @@ -254,7 +254,7 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, return nil, err } for _, e := range entries { - if e.UpstreamFs().Features().Move == nil { + if !operations.CanServerSideMove(e.UpstreamFs()) { return nil, fs.ErrorCantMove } } @@ -277,12 +277,27 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, errs[i] = errors.Wrap(fs.ErrorCantMove, su.Name()+":"+remote) return } - mo, err := du.Features().Move(ctx, o.UnWrap(), remote) - if err != nil || mo == nil { + srcObj := o.UnWrap() + duFeatures := du.Features() + do := duFeatures.Move + if duFeatures.Move == nil { + do = duFeatures.Copy + } + // Do the Move or Copy + dstObj, err := do(ctx, srcObj, remote) + if err != nil || dstObj == nil { errs[i] = errors.Wrap(err, su.Name()) return } - objs[i] = du.WrapObject(mo) + objs[i] = du.WrapObject(dstObj) + // Delete the source object if Copy + if duFeatures.Move == nil { + err = srcObj.Remove(ctx) + if err != nil { + errs[i] = errors.Wrap(err, su.Name()) + return + } + } }) var en []upstream.Entry for _, o := range objs { @@ -848,8 +863,16 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e SetTier: true, GetTier: true, }).Fill(ctx, f) + canMove := true for _, f := range upstreams { features = features.Mask(ctx, f) // Mask all upstream fs + if !operations.CanServerSideMove(f) { + canMove = false + } + } + // We can move if all remotes support Move or Copy + if canMove { + features.Move = f.Move } // Enable ListR when upstreams either support ListR or is local diff --git a/backend/union/union_internal_test.go b/backend/union/union_internal_test.go index db5367561..8171f593f 100644 --- a/backend/union/union_internal_test.go +++ b/backend/union/union_internal_test.go @@ -3,10 +3,15 @@ package union import ( "bytes" "context" + "fmt" + "io/ioutil" + "os" "testing" "time" + "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/object" + "github.com/rclone/rclone/fs/operations" "github.com/rclone/rclone/fstest" "github.com/rclone/rclone/fstest/fstests" "github.com/rclone/rclone/lib/random" @@ -14,6 +19,22 @@ import ( "github.com/stretchr/testify/require" ) +// MakeTestDirs makes directories in /tmp for testing +func MakeTestDirs(t *testing.T, n int) (dirs []string, clean func()) { + for i := 1; i <= n; i++ { + dir, err := ioutil.TempDir("", fmt.Sprintf("rclone-union-test-%d", n)) + require.NoError(t, err) + dirs = append(dirs, dir) + } + clean = func() { + for _, dir := range dirs { + err := os.RemoveAll(dir) + assert.NoError(t, err) + } + } + return dirs, clean +} + func (f *Fs) TestInternalReadOnly(t *testing.T) { if f.name != "TestUnionRO" { t.Skip("Only on RO union") @@ -65,3 +86,83 @@ func (f *Fs) InternalTest(t *testing.T) { } var _ fstests.InternalTester = (*Fs)(nil) + +// This specifically tests a union of local which can Move but not +// Copy and :memory: which can Copy but not Move to makes sure that +// the resulting union can Move +func TestMoveCopy(t *testing.T) { + if *fstest.RemoteName != "" { + t.Skip("Skipping as -remote set") + } + ctx := context.Background() + dirs, clean := MakeTestDirs(t, 1) + defer clean() + fsString := fmt.Sprintf(":union,upstreams='%s :memory:bucket':", dirs[0]) + f, err := fs.NewFs(ctx, fsString) + require.NoError(t, err) + + unionFs := f.(*Fs) + fLocal := unionFs.upstreams[0].Fs + fMemory := unionFs.upstreams[1].Fs + + t.Run("Features", func(t *testing.T) { + assert.NotNil(t, f.Features().Move) + assert.Nil(t, f.Features().Copy) + + // Check underlying are as we are expect + assert.NotNil(t, fLocal.Features().Move) + assert.Nil(t, fLocal.Features().Copy) + assert.Nil(t, fMemory.Features().Move) + assert.NotNil(t, fMemory.Features().Copy) + }) + + // Put a file onto the local fs + contentsLocal := random.String(50) + fileLocal := fstest.NewItem("local.txt", contentsLocal, time.Now()) + _, _ = fstests.PutTestContents(ctx, t, fLocal, &fileLocal, contentsLocal, true) + objLocal, err := f.NewObject(ctx, fileLocal.Path) + require.NoError(t, err) + + // Put a file onto the memory fs + contentsMemory := random.String(60) + fileMemory := fstest.NewItem("memory.txt", contentsMemory, time.Now()) + _, _ = fstests.PutTestContents(ctx, t, fMemory, &fileMemory, contentsMemory, true) + objMemory, err := f.NewObject(ctx, fileMemory.Path) + require.NoError(t, err) + + fstest.CheckListing(t, f, []fstest.Item{fileLocal, fileMemory}) + + t.Run("MoveLocal", func(t *testing.T) { + fileLocal.Path = "local-renamed.txt" + _, err := operations.Move(ctx, f, nil, fileLocal.Path, objLocal) + require.NoError(t, err) + fstest.CheckListing(t, f, []fstest.Item{fileLocal, fileMemory}) + + // Check can retrieve object from union + obj, err := f.NewObject(ctx, fileLocal.Path) + require.NoError(t, err) + assert.Equal(t, fileLocal.Size, obj.Size()) + + // Check can retrieve object from underlying + obj, err = fLocal.NewObject(ctx, fileLocal.Path) + require.NoError(t, err) + assert.Equal(t, fileLocal.Size, obj.Size()) + + t.Run("MoveMemory", func(t *testing.T) { + fileMemory.Path = "memory-renamed.txt" + _, err := operations.Move(ctx, f, nil, fileMemory.Path, objMemory) + require.NoError(t, err) + fstest.CheckListing(t, f, []fstest.Item{fileLocal, fileMemory}) + + // Check can retrieve object from union + obj, err := f.NewObject(ctx, fileMemory.Path) + require.NoError(t, err) + assert.Equal(t, fileMemory.Size, obj.Size()) + + // Check can retrieve object from underlying + obj, err = fMemory.NewObject(ctx, fileMemory.Path) + require.NoError(t, err) + assert.Equal(t, fileMemory.Size, obj.Size()) + }) + }) +} diff --git a/backend/union/union_test.go b/backend/union/union_test.go index 07b3bb230..c58936e9d 100644 --- a/backend/union/union_test.go +++ b/backend/union/union_test.go @@ -2,16 +2,13 @@ package union_test import ( - "fmt" - "io/ioutil" - "os" "testing" _ "github.com/rclone/rclone/backend/local" + _ "github.com/rclone/rclone/backend/memory" + "github.com/rclone/rclone/backend/union" "github.com/rclone/rclone/fstest" "github.com/rclone/rclone/fstest/fstests" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestIntegration runs integration tests against the remote @@ -26,26 +23,11 @@ func TestIntegration(t *testing.T) { }) } -func makeTestDirs(t *testing.T, n int) (dirs []string, clean func()) { - for i := 1; i <= n; i++ { - dir, err := ioutil.TempDir("", fmt.Sprintf("rclone-union-test-%d", n)) - require.NoError(t, err) - dirs = append(dirs, dir) - } - clean = func() { - for _, dir := range dirs { - err := os.RemoveAll(dir) - assert.NoError(t, err) - } - } - return dirs, clean -} - func TestStandard(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skipping as -remote set") } - dirs, clean := makeTestDirs(t, 3) + dirs, clean := union.MakeTestDirs(t, 3) defer clean() upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2] name := "TestUnion" @@ -67,7 +49,7 @@ func TestRO(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skipping as -remote set") } - dirs, clean := makeTestDirs(t, 3) + dirs, clean := union.MakeTestDirs(t, 3) defer clean() upstreams := dirs[0] + " " + dirs[1] + ":ro " + dirs[2] + ":ro" name := "TestUnionRO" @@ -89,7 +71,7 @@ func TestNC(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skipping as -remote set") } - dirs, clean := makeTestDirs(t, 3) + dirs, clean := union.MakeTestDirs(t, 3) defer clean() upstreams := dirs[0] + " " + dirs[1] + ":nc " + dirs[2] + ":nc" name := "TestUnionNC" @@ -111,7 +93,7 @@ func TestPolicy1(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skipping as -remote set") } - dirs, clean := makeTestDirs(t, 3) + dirs, clean := union.MakeTestDirs(t, 3) defer clean() upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2] name := "TestUnionPolicy1" @@ -133,7 +115,7 @@ func TestPolicy2(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skipping as -remote set") } - dirs, clean := makeTestDirs(t, 3) + dirs, clean := union.MakeTestDirs(t, 3) defer clean() upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2] name := "TestUnionPolicy2" @@ -155,7 +137,7 @@ func TestPolicy3(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skipping as -remote set") } - dirs, clean := makeTestDirs(t, 3) + dirs, clean := union.MakeTestDirs(t, 3) defer clean() upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2] name := "TestUnionPolicy3"