community
/
rclone
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

union: fix rename not working with union of local disk and bucket based remote 

Before this change the union's feature flags were a strict AND of the
underlying remotes. This means that a union of a local disk (which can
Move but not Copy) and a bucket based remote (which can Copy but not
Move) could neither Move nor Copy.

This fix advertises Move in the union if all the remotes can Move or
Copy. It also implements Move as Copy+Delete (like rclone does
normally) if the underlying union does not support Move.

This enables renames to work with unions of local disk and bucket
based remotes expected.

Fixes #5632
This commit is contained in:
Nick Craig-Wood 2021-09-30 11:11:46 +01:00
parent b389b84685
commit bb0c4ad2d8
3 changed files with 136 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
backend/union/union.go
View File

@ -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

101
backend/union/union_internal_test.go
View File

@ -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())
})
})
}

34
backend/union/union_test.go
View File

@ -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"