From 74436281edab9727ceafbe58b71bf41de62bb33d Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 29 May 2026 17:13:39 +0100 Subject: [PATCH] drive: fix server-side move failing on shared drives with duplicate dirs - fixes #9472 When moving a file rclone removed the file from its old parent by looking the parent ID up from the path using the directory cache. When the source contained two directories with the same name and path, the cache could resolve to the wrong duplicate, so the removeParents request didn't match the file's real parent. This left the file with both its old and new parent, which fails on shared drives with: A shared drive item must have exactly one parent., teamDrivesParentLimit This uses the source object's actual parent ID instead when it is known, falling back to the path lookup only when the object has zero or multiple parents. --- backend/drive/drive.go | 15 ++++-- backend/drive/drive_internal_test.go | 78 ++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/backend/drive/drive.go b/backend/drive/drive.go index 712286014..f3dad792d 100644 --- a/backend/drive/drive.go +++ b/backend/drive/drive.go @@ -3034,11 +3034,18 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, remote = remote[:len(remote)-len(ext)] } - _, srcParentID, err := srcObj.fs.dirCache.FindPath(ctx, src.Remote(), false) - if err != nil { - return nil, err + // Find the ID of the parent to remove the file from. + var srcParentID string + if len(srcObj.parents) == 1 { + srcParentID = srcObj.parents[0] + } else { + var err error + _, srcParentID, err = srcObj.fs.dirCache.FindPath(ctx, src.Remote(), false) + if err != nil { + return nil, err + } + srcParentID = actualID(srcParentID) } - srcParentID = actualID(srcParentID) // Temporary Object under construction dstInfo, err := f.createFileInfo(ctx, remote, src.ModTime(ctx)) diff --git a/backend/drive/drive_internal_test.go b/backend/drive/drive_internal_test.go index 743ce7e59..d5c2aff26 100644 --- a/backend/drive/drive_internal_test.go +++ b/backend/drive/drive_internal_test.go @@ -681,6 +681,83 @@ func (f *Fs) InternalTestSingleQuoteFolder(t *testing.T) { assert.NoError(t, err) } +// TestIntegration/FsMkdir/FsPutFiles/Internal/MoveDuplicateParent +// +// Check that moving a file removes it from its real parent even when there +// are two directories with the same name and the directory cache resolves the +// path to the wrong duplicate. Otherwise the file ends up with two parents +// which fails on shared drives with a teamDrivesParentLimit error (#9472). +func (f *Fs) InternalTestMoveDuplicateParent(t *testing.T) { + ctx := context.Background() + if f.Features().Move == nil { + t.Skip("Move not supported") + } + + rootID, err := f.dirCache.RootID(ctx, false) + require.NoError(t, err) + + const dupName = "dup9472" + const dstName = "dst9472" + const fileName = "michael.txt" + + // Create the first "dup9472" directory - this is the one the directory + // cache will resolve the path to. + dirA, err := f.createDir(ctx, rootID, dupName, nil) + require.NoError(t, err) + f.dirCache.Put(dupName, dirA.Id) + + // Create a second directory with the SAME name and parent: a duplicate + // the directory cache does not know about. + dirB, err := f.createDir(ctx, rootID, dupName, nil) + require.NoError(t, err) + + // Destination directory + dirDst, err := f.createDir(ctx, rootID, dstName, nil) + require.NoError(t, err) + + defer func() { + _ = f.delete(ctx, dirA.Id, false) + _ = f.delete(ctx, dirB.Id, false) + _ = f.delete(ctx, dirDst.Id, false) + f.dirCache.Flush() + }() + + // Upload a file directly into dirB (the duplicate the cache doesn't know). + createInfo := &drive.File{ + Name: fileName, + Parents: []string{dirB.Id}, + } + var fileInfo *drive.File + err = f.pacer.Call(func() (bool, error) { + fileInfo, err = f.svc.Files.Create(createInfo). + Media(strings.NewReader("9472 duplicate parent test")). + Fields(f.getFileFields(ctx)). + SupportsAllDrives(true). + Context(ctx).Do() + return f.shouldRetry(ctx, err) + }) + require.NoError(t, err) + require.Equal(t, []string{dirB.Id}, fileInfo.Parents) + + // Sanity check: the directory cache resolves the path to dirA, not the + // dirB the file actually lives in. + gotDirID, err := f.dirCache.FindDir(ctx, dupName, false) + require.NoError(t, err) + require.Equal(t, dirA.Id, gotDirID, "directory cache should resolve to the wrong duplicate") + + // Build the source object pointing at the file in dirB and move it. + srcObj, err := f.newObjectWithInfo(ctx, dupName+"/"+fileName, fileInfo) + require.NoError(t, err) + + dstObj, err := f.Move(ctx, srcObj, dstName+"/"+fileName) + require.NoError(t, err) + + // The moved file must have exactly one parent: the destination. + movedInfo, err := f.getFile(ctx, dstObj.(fs.IDer).ID(), "id,parents") + require.NoError(t, err) + assert.Equal(t, []string{dirDst.Id}, movedInfo.Parents) +} + func (f *Fs) InternalTest(t *testing.T) { // These tests all depend on each other so run them as nested tests t.Run("DocumentImport", func(t *testing.T) { @@ -701,6 +778,7 @@ func (f *Fs) InternalTest(t *testing.T) { t.Run("Query", f.InternalTestQuery) t.Run("AgeQuery", f.InternalTestAgeQuery) t.Run("SingleQuoteFolder", f.InternalTestSingleQuoteFolder) + t.Run("MoveDuplicateParent", f.InternalTestMoveDuplicateParent) t.Run("ShouldRetry", f.InternalTestShouldRetry) }