Repository: rclone/rclone (language: go)

Issue:
Server side copy - Box - item_name_in_use 409 error with existing files
Linked from [forum posting](https://forum.rclone.org/t/server-side-copy-box-item-name-in-use-409-error-with-existing-files/11365).

If I attempt a server side copy from one box path to another, any file existing in both paths that has been updated in the src path will not be updated in the dst path. Instead, an error is thrown:

    DEBUG : : Sizes differ (src 32 vs dst 25)
    ERROR : : Failed to copy: Error "item_name_in_use" (409): Item with the same name already exists

The command is something like:

    rclone copy boxRemote:path1 boxRemote:path2

where path2 is intended to have a copy of all files that exist in path1.

The first time this copy is called, everything works because there are no existing files of path1 in path2. However, if later, a file is updated in path1 and then the above copy command is attempted, this `item_name_in_use` error is thrown.

This is with rclone 1.48 on Ubuntu x64 16.04.

I suspect this is not desired behavior, because the same types of copy commands work just fine if src is local instead of a box remote.


Discussion / hints:
I reproduced this like this with the latest beta

```
$ date -Is | rclone rcat boxcom:test1/file1.txt
$ date -Is | rclone rcat boxcom:test2/file1.txt
$ rclone copy -vv --retries 1 --low-level-retries 1 boxcom:test2 boxcom:test1
2019/09/05 13:03:01 DEBUG : rclone: Version "v1.49.0-024-g5dad88ae-fix-3506-names-beta" starting with parameters ["rclone" "copy" "-vv" "--retries" "1" "--low-level-retries" "1" "boxcom:test2" "boxcom:test1"]
2019/09/05 13:03:01 DEBUG : Using config file from "/home/ncw/.rclone.conf"
2019/09/05 13:03:02 DEBUG : file1.txt: Modification times differ by -8s: 2019-09-05 05:02:38 -0700 -0700, 2019-09-05 05:02:30 -0700 -0700
2019/09/05 13:03:02 INFO  : box root 'test1': Waiting for checks to finish
2019/09/05 13:03:02 DEBUG : file1.txt: SHA-1 = b28a9df4b94db8bc1fb4ad48a3b0137d8d6f3348 (box root 'test2')
2019/09/05 13:03:02 DEBUG : file1.txt: SHA-1 = f261a573e4a89950c5e8d5d22367570274a987d8 (box root 'test1')
2019/09/05 13:03:02 DEBUG : file1.txt: SHA-1 differ
2019/09/05 13:03:02 INFO  : box root 'test1': Waiting for transfers to finish
2019/09/05 13:03:02 ERROR : file1.txt: Failed to copy: Error "item_name_in_use" (409): Item with the same name already exists
2019/09/05 13:03:02 ERROR : Attempt 1/1 failed with 3 errors and: Error "item_name_in_use" (409): Item with the same name already exists
2019/09/05 13:03:02 Failed to copy with 3 errors: last error was: Error "item_name_in_use" (409): Item with the same name already exists
```

I suspect we need to add an integration test for
- Copy over existing
- Move over existing

and fix any backends that have a problem starting with box!
Same problem for me.
Same problem as well. I just end up rclone copying/moving to a new temp folder

Fix patch:
diff --git a/backend/box/box.go b/backend/box/box.go
index 4f38955eb017c..7183f35e56409 100644
--- a/backend/box/box.go
+++ b/backend/box/box.go
@@ -43,6 +43,7 @@ import (
 	"github.com/rclone/rclone/lib/jwtutil"
 	"github.com/rclone/rclone/lib/oauthutil"
 	"github.com/rclone/rclone/lib/pacer"
+	"github.com/rclone/rclone/lib/random"
 	"github.com/rclone/rclone/lib/rest"
 	"github.com/youmark/pkcs8"
 	"golang.org/x/oauth2"
@@ -256,7 +257,6 @@ func getQueryParams(boxConfig *api.ConfigJSON) map[string]string {
 }
 
 func getDecryptedPrivateKey(boxConfig *api.ConfigJSON) (key *rsa.PrivateKey, err error) {
-
 	block, rest := pem.Decode([]byte(boxConfig.BoxAppSettings.AppAuth.PrivateKey))
 	if len(rest) > 0 {
 		return nil, fmt.Errorf("box: extra data included in private key: %w", err)
@@ -619,7 +619,7 @@ func (f *Fs) CreateDir(ctx context.Context, pathID, leaf string) (newID string,
 		return shouldRetry(ctx, resp, err)
 	})
 	if err != nil {
-		//fmt.Printf("...Error %v\n", err)
+		// fmt.Printf("...Error %v\n", err)
 		return "", err
 	}
 	// fmt.Printf("...Id %q\n", *info.Id)
@@ -966,6 +966,26 @@ func (f *Fs) Copy(ctx context.Context, src fs.Object, remote string) (fs.Object,
 		return nil, err
 	}
 
+	// check if dest already exists
+	item, err := f.preUploadCheck(ctx, leaf, directoryID, src.Size())
+	if err != nil {
+		return nil, err
+	}
+	if item != nil { // dest already exists, need to copy to temp name and then move
+		tempSuffix := "-rclone-copy-" + random.String(8)
+		fs.Debugf(remote, "dst already exists, copying to temp name %v", remote+tempSuffix)
+		tempObj, err := f.Copy(ctx, src, remote+tempSuffix)
+		if err != nil {
+			return nil, err
+		}
+		fs.Debugf(remote+tempSuffix, "moving to real name %v", remote)
+		err = f.deleteObject(ctx, item.ID)
+		if err != nil {
+			return nil, err
+		}
+		return f.Move(ctx, tempObj, remote)
+	}
+
 	// Copy the object
 	opts := rest.Opts{
 		Method:     "POST",


Test patch:
diff --git a/cmd/bisync/bisync_test.go b/cmd/bisync/bisync_test.go
index bc359bbd3923b..da89c3e20e10c 100644
--- a/cmd/bisync/bisync_test.go
+++ b/cmd/bisync/bisync_test.go
@@ -15,6 +15,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"runtime"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -207,15 +208,16 @@ type bisyncTest struct {
 	parent1  fs.Fs
 	parent2  fs.Fs
 	// global flags
-	argRemote1    string
-	argRemote2    string
-	noCompare     bool
-	noCleanup     bool
-	golden        bool
-	debug         bool
-	stopAt        int
-	TestFn        bisync.TestFunc
-	ignoreModtime bool // ignore modtimes when comparing final listings, for backends without support
+	argRemote1      string
+	argRemote2      string
+	noCompare       bool
+	noCleanup       bool
+	golden          bool
+	debug           bool
+	stopAt          int
+	TestFn          bisync.TestFunc
+	ignoreModtime   bool // ignore modtimes when comparing final listings, for backends without support
+	ignoreBlankHash bool // ignore blank hashes for backends where we allow them to be blank
 }
 
 var color = bisync.Color
@@ -946,6 +948,10 @@ func (b *bisyncTest) checkPreReqs(ctx context.Context, opt *bisync.Options) (con
 	if (!b.fs1.Features().CanHaveEmptyDirectories || !b.fs2.Features().CanHaveEmptyDirectories) && (b.testCase == "createemptysrcdirs" || b.testCase == "rmdirs") {
 		b.t.Skip("skipping test as remote does not support empty dirs")
 	}
+	ignoreHashBackends := []string{"TestWebdavNextcloud", "TestWebdavOwncloud", "TestAzureFiles"} // backends that support hashes but allow them to be blank
+	if slices.ContainsFunc(ignoreHashBackends, func(prefix string) bool { return strings.HasPrefix(b.fs1.Name(), prefix) }) || slices.ContainsFunc(ignoreHashBackends, func(prefix string) bool { return strings.HasPrefix(b.fs2.Name(), prefix) }) {
+		b.ignoreBlankHash = true
+	}
 	if b.fs1.Precision() == fs.ModTimeNotSupported || b.fs2.Precision() == fs.ModTimeNotSupported {
 		if b.testCase != "nomodtime" {
 			b.t.Skip("skipping test as at least one remote does not support setting modtime")
@@ -1551,6 +1557,12 @@ func (b *bisyncTest) mangleResult(dir, file string, golden bool) string {
 	if b.fs1.Hashes() == hash.Set(hash.None) || b.fs2.Hashes() == hash.Set(hash.None) {
 		logReplacements = append(logReplacements, `^.*{hashtype} differ.*$`, dropMe)
 	}
+	if b.ignoreBlankHash {
+		logReplacements = append(logReplacements,
+			`^.*hash is missing.*$`, dropMe,
+			`^.*not equal on recheck.*$`, dropMe,
+		)
+	}
 	rep := logReplacements
 	if b.testCase == "dry_run" {
 		rep = append(rep, dryrunReplacements...)
diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go
index f1c07921e7788..288616cd514c6 100644
--- a/fs/sync/sync_test.go
+++ b/fs/sync/sync_test.go
@@ -597,6 +597,108 @@ func TestServerSideCopy(t *testing.T) {
 	fstest.CheckItems(t, FremoteCopy, file1)
 }
 
+// Test copying a file over itself
+func TestCopyOverSelf(t *testing.T) {
+	ctx := context.Background()
+	r := fstest.NewRun(t)
+	file1 := r.WriteObject(ctx, "sub dir/hello world", "hello world", t1)
+	r.CheckRemoteItems(t, file1)
+	file2 := r.WriteFile("sub dir/hello world", "hello world again", t2)
+	r.CheckLocalItems(t, file2)
+
+	ctx = predictDstFromLogger(ctx)
+	err := CopyDir(ctx, r.Fremote, r.Flocal, false)
+	require.NoError(t, err)
+	testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t)
+	r.CheckRemoteItems(t, file2)
+}
+
+// Test server-side copying a file over itself
+func TestServerSideCopyOverSelf(t *testing.T) {
+	ctx := context.Background()
+	r := fstest.NewRun(t)
+	file1 := r.WriteObject(ctx, "sub dir/hello world", "hello world", t1)
+	r.CheckRemoteItems(t, file1)
+
+	FremoteCopy, _, finaliseCopy, err := fstest.RandomRemote()
+	require.NoError(t, err)
+	defer finaliseCopy()
+	t.Logf("Server side copy (if possible) %v -> %v", r.Fremote, FremoteCopy)
+
+	ctx = predictDstFromLogger(ctx)
+	err = CopyDir(ctx, FremoteCopy, r.Fremote, false)
+	require.NoError(t, err)
+	testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t)
+	fstest.CheckItems(t, FremoteCopy, file1)
+
+	file2 := r.WriteObject(ctx, "sub dir/hello world", "hello world again", t2)
+	r.CheckRemoteItems(t, file2)
+
+	ctx = predictDstFromLogger(ctx)
+	err = CopyDir(ctx, FremoteCopy, r.Fremote, false)
+	require.NoError(t, err)
+	testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t)
+	fstest.CheckItems(t, FremoteCopy, file2)
+}
+
+// Test moving a file over itself
+func TestMoveOverSelf(t *testing.T) {
+	ctx := context.Background()
+	r := fstest.NewRun(t)
+	file1 := r.WriteObject(ctx, "sub dir/hello world", "hello world", t1)
+	r.CheckRemoteItems(t, file1)
+	file2 := r.WriteFile("sub dir/hello world", "hello world again", t2)
+	r.CheckLocalItems(t, file2)
+
+	ctx = predictDstFromLogger(ctx)
+	err := MoveDir(ctx, r.Fremote, r.Flocal, false, false)
+	require.NoError(t, err)
+	testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t)
+	r.CheckLocalItems(t)
+	r.CheckRemoteItems(t, file2)
+}
+
+// Test server-side moving a file over itself
+func TestServerSideMoveOverSelf(t *testing.T) {
+	ctx := context.Background()
+	r := fstest.NewRun(t)
+	file1 := r.WriteObject(ctx, "sub dir/hello world", "hello world", t1)
+	r.CheckRemoteItems(t, file1)
+
+	FremoteCopy, _, finaliseCopy, err := fstest.RandomRemote()
+	require.NoError(t, err)
+	defer finaliseCopy()
+	t.Logf("Server side copy (if possible) %v -> %v", r.Fremote, FremoteCopy)
+
+	ctx = predictDstFromLogger(ctx)
+	err = CopyDir(ctx, FremoteCopy, r.Fremote, false)
+	require.NoError(t, err)
+	testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t)
+	fstest.CheckItems(t, FremoteCopy, file1)
+
+	file2 := r.WriteObject(ctx, "sub dir/hello world", "hello world again", t2)
+	r.CheckRemoteItems(t, file2)
+
+	// ctx = predictDstFromLogger(ctx)
+	err = MoveDir(ctx, FremoteCopy, r.Fremote, false, false)
+	require.NoError(t, err)
+	// testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t) // not currently supported
+	r.CheckRemoteItems(t)
+	fstest.CheckItems(t, FremoteCopy, file2)
+
+	// check that individual file moves also work without MoveDir
+	file3 := r.WriteObject(ctx, "sub dir/hello world", "hello world a third time", t3)
+	r.CheckRemoteItems(t, file3)
+
+	ctx = predictDstFromLogger(ctx)
+	fs.Debugf(nil, "testing file moves")
+	err = moveDir(ctx, FremoteCopy, r.Fremote, false, false)
+	require.NoError(t, err)
+	testLoggerVsLsf(ctx, FremoteCopy, operations.GetLoggerOpt(ctx).JSON, t)
+	r.CheckRemoteItems(t)
+	fstest.CheckItems(t, FremoteCopy, file3)
+}
+
 // Check that if the local file doesn't exist when we copy it up,
 // nothing happens to the remote file
 func TestCopyAfterDelete(t *testing.T) {
@@ -2320,15 +2422,19 @@ func testSyncBackupDir(t *testing.T, backupDir string, suffix string, suffixKeep
 
 	r.CheckRemoteItems(t, file1b, file2, file3a, file1a)
 }
+
 func TestSyncBackupDir(t *testing.T) {
 	testSyncBackupDir(t, "backup", "", false)
 }
+
 func TestSyncBackupDirWithSuffix(t *testing.T) {
 	testSyncBackupDir(t, "backup", ".bak", false)
 }
+
 func TestSyncBackupDirWithSuffixKeepExtension(t *testing.T) {
 	testSyncBackupDir(t, "backup", "-2019-01-01", true)
 }
+
 func TestSyncBackupDirSuffixOnly(t *testing.T) {
 	testSyncBackupDir(t, "", ".bak", false)
 }
@@ -2806,7 +2912,7 @@ func predictDstFromLogger(ctx context.Context) context.Context {
 }
 
 func DstLsf(ctx context.Context, Fremote fs.Fs) *bytes.Buffer {
-	var opt = operations.ListJSONOpt{
+	opt := operations.ListJSONOpt{
 		NoModTime:  false,
 		NoMimeType: true,
 		DirsOnly:   false,
diff --git a/fstest/test_all/config.yaml b/fstest/test_all/config.yaml
index 66d60d993552f..7c5ed0feb8b92 100644
--- a/fstest/test_all/config.yaml
+++ b/fstest/test_all/config.yaml
@@ -395,6 +395,10 @@ backends:
  - backend:  "cache"
    remote:   "TestCache:"
    fastlist: false
+   ignoretests:
+     - TestBisyncLocalRemote
+     - TestBisyncRemoteLocal
+     - TestBisyncRemoteRemote
  - backend:  "mega"
    remote:   "TestMega:"
    fastlist: false