Fixed couple of bugs in shared drives (#4703)

**Realtime not updating for the recipient**
When a recipient moved a file from one folder to another within the same
shared drive (both source and destination on the same owner instance),
the move was implemented as a copy+delete via moveFileSameStackCore.
This created a new
file ID at the destination and deleted the old ID, emitting a CREATED +
DELETED pair instead of a single UPDATED event.

**Physical file not deleted after cross-instance move**
deleteSourceFile was calling GetIndexer().DeleteFileDoc(), which only
removes the CouchDB metadata record.

Author: shepilov

web/sharings/drives_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cozy/cozy-stack/model/instance/lifecycle"
 	"github.com/cozy/cozy-stack/model/job"
 	"github.com/cozy/cozy-stack/model/sharing"
+	"github.com/cozy/cozy-stack/model/vfs"
 	"github.com/cozy/cozy-stack/pkg/assets/dynamic"
 	build "github.com/cozy/cozy-stack/pkg/config"
 	"github.com/cozy/cozy-stack/pkg/config/config"
@@ -117,12 +118,17 @@ func verifyFileMove(t *testing.T, inst *instance.Instance, fileID, expectedName,
 	require.Equal(t, expectedContent, string(content))
 }
 
-// verifyFileDeleted verifies that a file was deleted from the source instance.
-func verifyFileDeleted(t *testing.T, inst *instance.Instance, fileID string) {
+// verifyFileDeleted verifies that a file was fully deleted from the source instance: both the
+// CouchDB metadata record and the physical file content in storage. doc must be fetched before
+// the deletion so its path is available after the CouchDB record is gone.
+func verifyFileDeleted(t *testing.T, inst *instance.Instance, doc *vfs.FileDoc) {
 	t.Helper()
-	_, err := inst.VFS().FileByID(fileID)
+	_, err := inst.VFS().FileByID(doc.DocID)
 	require.Error(t, err)
 	require.True(t, os.IsNotExist(err))
+
+	_, err = inst.VFS().OpenFile(doc)
+	require.Error(t, err, "expected physical file to be deleted from storage, but it is still readable")
 }
 
 // verifyNodeDeleted verifies that a node (file or directory) was deleted from the source instance.

web/sharings/move.go

@@ -199,6 +199,21 @@ func moveDirSameStack(c echo.Context, srcInst *instance.Instance, destInst *inst
 		return files.WrapVfsError(err)
 	}
 
+	// When moving (not copying) within the same instance, use a metadata-only
+	// update via ModifyDirMetadata.
+	if srcInst.Domain == destInst.Domain && !copy {
+		patch, err := buildMovePatch(srcInst.VFS(), destDirID, srcRoot.DocName, false)
+		if err != nil {
+			return err
+		}
+		newDir, err := vfs.ModifyDirMetadata(srcInst.VFS(), srcRoot, patch)
+		if err != nil {
+			return files.WrapVfsError(err)
+		}
+		obj := files.NewDir(newDir, destSharing)
+		return jsonapi.Data(c, http.StatusCreated, obj, nil)
+	}
+
 	dirs, filesToMove, err := planLocalTree(srcInst.VFS(), srcRoot, destDirID)
 	if err != nil {
 		return files.WrapVfsError(err)
@@ -299,6 +314,21 @@ func moveFileSameStack(c echo.Context, srcInst *instance.Instance, destInst *ins
 	if err != nil {
 		return files.WrapVfsError(err)
 	}
+
+	// When moving (not copying) within the same instance, use a metadata-only
+	// update via ModifyFileMetadata.
+	if srcInst.Domain == destInst.Domain && !copy {
+		patch, err := buildMovePatch(srcInst.VFS(), dirID, srcFile.DocName, true)
+		if err != nil {
+			return err
+		}
+		newFile, err := vfs.ModifyFileMetadata(srcInst.VFS(), srcFile, patch)
+		if err != nil {
+			return files.WrapVfsError(err)
+		}
+		obj := files.NewFile(newFile, destInst, destSharing)
+		return jsonapi.Data(c, http.StatusCreated, obj, nil)
+	}
 	deleteSource := !copy
 	newFileDoc, err := moveFileSameStackCore(srcInst, destInst, srcFile, dirID, deleteSource)
 	if err != nil {
@@ -970,12 +1000,26 @@ func copyFileContent(destVFS vfs.VFS, newFileDoc *vfs.FileDoc, srcVFS vfs.VFS, s
 }
 
 func deleteSourceFile(srcVFS vfs.VFS, srcFile *vfs.FileDoc) error {
-	if err := srcVFS.GetIndexer().DeleteFileDoc(srcFile); err != nil {
+	if err := srcVFS.DestroyFile(srcFile); err != nil {
 		return files.WrapVfsError(err)
 	}
 	return nil
 }
 
+// buildMovePatch creates a DocPatch for a metadata-only move to destDirID,
+// resolving name conflicts automatically. isFile controls the conflict naming strategy.
+func buildMovePatch(fs vfs.VFS, destDirID, currentName string, isFile bool) (*vfs.DocPatch, error) {
+	uniqueName, err := ensureUniqueName(fs, destDirID, currentName, isFile)
+	if err != nil {
+		return nil, files.WrapVfsError(err)
+	}
+	patch := &vfs.DocPatch{DirID: &destDirID}
+	if uniqueName != currentName {
+		patch.Name = &uniqueName
+	}
+	return patch, nil
+}
+
 // ensureUniqueName ensures a child name under parentDirID is unique by checking the
 // indexer and applying vfs.ConflictName when needed. isFile controls the conflict
 // naming strategy on files vs directories.

web/sharings/move_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/cozy/cozy-stack/model/instance"
-
 	"github.com/cozy/cozy-stack/model/instance/lifecycle"
 	"github.com/cozy/cozy-stack/pkg/assets/dynamic"
 	build "github.com/cozy/cozy-stack/pkg/config"
@@ -206,6 +205,11 @@ func TestSharedDrivesMove(t *testing.T) {
 		_, eB, _ := env.createClients(t)
 		// Perform the move operation
 		fileToMoveSameStack := createFile(t, eB, "", "file-to-upload.txt", env.bettyToken)
+
+		// Pre-fetch the source FileDoc before the move so we can verify physical deletion later
+		srcFileDoc, err := env.betty.VFS().FileByID(fileToMoveSameStack)
+		require.NoError(t, err)
+
 		responseObj := postMove(t, eB, env.bettyToken, `{
 				  "source": {
 				    "file_id": "`+fileToMoveSameStack+`"
@@ -223,8 +227,8 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Verify the file was moved and content preserved
 		verifyFileMove(t, env.acme, movedFileID, "file-to-upload.txt", env.productDirID, "foo")
 
-		// Verify the original file was deleted
-		verifyFileDeleted(t, env.betty, fileToMoveSameStack)
+		// Verify the original file was deleted (both metadata and physical content)
+		verifyFileDeleted(t, env.betty, srcFileDoc)
 	})
 
 	// Force the cross-stack path even if instances are on the same server
@@ -233,6 +237,10 @@ func TestSharedDrivesMove(t *testing.T) {
 		fileToMoveDifferentStack := createFile(t, eB, "", "file-to-upload-diff.txt", env.bettyToken)
 		destDirInSharedDrive := createDirectory(t, eA, env.productDirID, "Dest DIR To Move", env.acmeToken)
 
+		// Pre-fetch the source FileDoc before the move so we can verify physical deletion later
+		srcFileDoc, err := env.betty.VFS().FileByID(fileToMoveDifferentStack)
+		require.NoError(t, err)
+
 		cleanup := forceCrossStack(t, env.tsA.URL)
 		defer cleanup()
 
@@ -253,8 +261,8 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Verify the file was moved and content preserved
 		verifyFileMove(t, env.acme, movedFileID, "file-to-upload-diff.txt", destDirInSharedDrive, "foo")
 
-		// Verify the original file was deleted
-		verifyFileDeleted(t, env.acme, fileToMoveDifferentStack)
+		// Verify the original file was deleted from betty (both metadata and physical content)
+		verifyFileDeleted(t, env.betty, srcFileDoc)
 	})
 
 	t.Run("SuccessfulMove_FromSharedDrive_SameStack", func(t *testing.T) {
@@ -264,6 +272,10 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Create destination directory on the target instance
 		destDirID := createRootDirectory(t, eB, "Destination Dir", env.bettyToken)
 
+		// Pre-fetch the source FileDoc before the move so we can verify physical deletion later
+		srcFileDoc, err := env.acme.VFS().FileByID(fileToMove)
+		require.NoError(t, err)
+
 		responseObj := postMove(t, eB, env.bettyToken, `{
 				  "source": {
 					"instance": "https://`+env.acme.Domain+`",
@@ -281,8 +293,8 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Verify the file was moved to the destination
 		verifyFileMove(t, env.betty, movedFileID, "file-to-move-upstream.txt", destDirID, "foo")
 
-		// Verify the original file was deleted from source
-		verifyFileDeleted(t, env.acme, fileToMove)
+		// Verify the original file was deleted (both metadata and physical content)
+		verifyFileDeleted(t, env.acme, srcFileDoc)
 	})
 
 	// Force the cross-stack path even if instances are on the same server
@@ -293,6 +305,10 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Create destination directory on the target (owner) instance
 		destDirID := createRootDirectory(t, eB, "Destination Dir Diff", env.bettyToken)
 
+		// Pre-fetch the source FileDoc before the move so we can verify physical deletion later
+		srcFileDoc, err := env.acme.VFS().FileByID(fileToDiffStack)
+		require.NoError(t, err)
+
 		cleanup := forceCrossStack(t, env.tsA.URL)
 		defer cleanup()
 
@@ -313,8 +329,8 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Verify the file was moved to the destination
 		verifyFileMove(t, env.betty, movedFileID, fileName, destDirID, "foo")
 
-		// Verify the original file was deleted from source
-		verifyFileDeleted(t, env.acme, fileToDiffStack)
+		// Verify the original file was deleted from source (both metadata and physical content)
+		verifyFileDeleted(t, env.acme, srcFileDoc)
 	})
 
 	t.Run("SuccessfulMove_BetweenSharedDrives_DifferentStack", func(t *testing.T) {
@@ -328,6 +344,10 @@ func TestSharedDrivesMove(t *testing.T) {
 		fileName := "file-to-move-between-shared-drives.txt"
 		toMove := createFile(t, eA, secondRootDirID, fileName, env.acmeToken)
 
+		// Pre-fetch the source FileDoc before the move so we can verify physical deletion later
+		srcFileDoc, err := env.acme.VFS().FileByID(toMove)
+		require.NoError(t, err)
+
 		cleanup := forceCrossStack(t, env.tsA.URL)
 		defer cleanup()
 
@@ -349,7 +369,8 @@ func TestSharedDrivesMove(t *testing.T) {
 
 		// Verify the file was moved to the destination
 		verifyFileMove(t, env.acme, movedFileID, fileName, env.meetingsDirID, "foo")
-		verifyFileDeleted(t, env.acme, toMove)
+		// Verify the original file was deleted (both metadata and physical content)
+		verifyFileDeleted(t, env.acme, srcFileDoc)
 	})
 
 	t.Run("SuccessfulMoveBetween_SharedDrives_SameStack", func(t *testing.T) {
@@ -392,11 +413,10 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Verify the response contains the new file
 		movedFileID := assertMoveResponseWithSharing(t, responseObj, fileName, meetingsID, firstSharingID)
 
-		// Verify the file was moved to the destination
+		// Verify the file was moved to the destination.
+		// Same-instance move preserves the file ID (metadata-only update via ModifyFileMetadata).
+		require.Equal(t, toMove, movedFileID, "same-instance move must preserve the file ID")
 		verifyFileMove(t, env.acme, movedFileID, fileName, meetingsID, "foo")
-
-		// Verify the original file was deleted from source
-		verifyFileDeleted(t, env.acme, toMove)
 	})
 
 	// File move with conflict resolution: moving a file between shared drives (same stack)
@@ -432,12 +452,15 @@ func TestSharedDrivesMove(t *testing.T) {
 
 		// Verify the file was moved with a renamed name (conflict resolution)
 		movedFileName := assertAutoRename(t, responseObj, conflictName)
+		movedFileID := responseObj.Path("$.data.id").String().Raw()
 
 		// Verify both files exist in destination (original + renamed)
 		verifyBothFilesExist(t, env.acme, env.meetingsDirID, conflictName, movedFileName)
 
-		// Verify source file was deleted
-		verifyFileDeleted(t, env.acme, sourceFileID)
+		// Same-instance move preserves the file ID (metadata-only update via ModifyFileMetadata).
+		// The source file was moved (not deleted): verify it now lives at the destination with the renamed name.
+		require.Equal(t, sourceFileID, movedFileID, "same-instance move must preserve the file ID")
+		verifyFileExists(t, env.acme, sourceFileID, movedFileName, env.meetingsDirID, "foo")
 	})
 
 	// File move with conflict resolution: moving a file between shared drives (same stack)
@@ -459,6 +482,10 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Create source file with the same name in the second shared drive (source)
 		sourceFileID := createFile(t, eA, secondRootDirID, conflictName, env.acmeToken)
 
+		// Pre-fetch the source FileDoc before the move so we can verify physical deletion later
+		srcFileDoc, err := env.acme.VFS().FileByID(sourceFileID)
+		require.NoError(t, err)
+
 		// Attempt to move → should succeed with auto-renaming
 		responseObj := postMove(t, eB, env.bettyToken, `{
 			  "source": {
@@ -479,8 +506,8 @@ func TestSharedDrivesMove(t *testing.T) {
 		// Verify both files exist in destination (original + renamed)
 		verifyBothFilesExist(t, env.acme, env.meetingsDirID, conflictName, movedFileName)
 
-		// Verify source file was deleted
-		verifyFileDeleted(t, env.acme, sourceFileID)
+		// Verify source file was deleted (both metadata and physical content)
+		verifyFileDeleted(t, env.acme, srcFileDoc)
 	})
 
 	// Folder move with conflict resolution: moving a directory between shared drives (same stack)