fix(file-provider): Fix handling of item renamed into excluded pattern

shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift

@@ -7,6 +7,32 @@ import NextcloudFileProviderXPC
 import NextcloudKit
 
 public extension Item {
+    private func deleteRemoteItemForExcludedDestination(domain: NSFileProviderDomain?) async -> Error? {
+        let remotePath = metadata.remotePath()
+        let (_, _, error) = await remoteInterface.delete(
+            remotePath: remotePath,
+            account: account,
+            options: .init(),
+            taskHandler: { task in
+                if let domain {
+                    NSFileProviderManager(for: domain)?.register(
+                        task,
+                        forItemWithIdentifier: self.itemIdentifier,
+                        completionHandler: { _ in }
+                    )
+                }
+            }
+        )
+
+        guard error == .success || error.isNotFoundError else {
+            logger.error("Could not delete the remote item for an excluded destination.", [.item: itemIdentifier, .url: remotePath, .error: error])
+            return error.fileProviderError(handlingNoSuchItemErrorUsingItemIdentifier: itemIdentifier)
+        }
+
+        logger.info("Deleted the remote item for an excluded destination.", [.item: itemIdentifier, .url: remotePath])
+        return nil
+    }
+
     func move(
         newFileName: String,
         newRemotePath: String,
@@ -412,10 +438,9 @@ public extension Item {
             )
         }
 
-        let relativePath = (metadata.serverUrl + "/" + metadata.fileName).replacingOccurrences(of: account.davFilesUrl, with: "")
-
-        guard ignoredFiles == nil || ignoredFiles?.isExcluded(relativePath) == false else {
-            logger.info("File is in the ignore list. Will delete locally with no remote effect.", [.item: modifiedItem.itemIdentifier, .name: modifiedItem.filename])
+        let sourceRelativePath = modifiedItem.metadata.remotePath().replacingOccurrences(of: account.davFilesUrl, with: "")
+        guard ignoredFiles == nil || ignoredFiles?.isExcluded(sourceRelativePath) == false else {
+            logger.info("File is in the ignore list. Any follow-up provider deletion will have no remote effect.", [.item: modifiedItem.itemIdentifier, .name: modifiedItem.filename])
 
             guard let modifiedIgnored = await modifyUnuploaded(
                 itemTarget: itemTarget,
@@ -430,30 +455,11 @@ public extension Item {
                 progress: progress,
                 dbManager: dbManager
             ) else {
-                logger.error("Unable to modify ignored file, got nil item: \(relativePath)")
+                logger.error("Unable to modify ignored file.", [.item: modifiedItem.itemIdentifier, .name: modifiedItem.filename])
                 return (nil, NSFileProviderError(.cannotSynchronize))
             }
 
-            modifiedItem = modifiedIgnored
-            return (modifiedItem, NSFileProviderError(.excludedFromSync))
-        }
-
-        // We are handling an item that is available locally but not on the server -- so create it
-        // This can happen when a previously ignored file is no longer ignored
-        if !modifiedItem.isUploaded, modifiedItem.isDownloaded, modifiedItem.metadata.etag == "" {
-            return await modifiedItem.createUnuploaded(
-                itemTarget: itemTarget,
-                baseVersion: baseVersion,
-                changedFields: changedFields,
-                contents: newContents,
-                options: options,
-                request: request,
-                ignoredFiles: ignoredFiles,
-                domain: domain,
-                forcedChunkSize: forcedChunkSize,
-                progress: progress,
-                dbManager: dbManager
-            )
+            return (modifiedIgnored, NSFileProviderError(.excludedFromSync))
         }
 
         guard itemTarget.itemIdentifier == modifiedItem.itemIdentifier else {
@@ -494,9 +500,67 @@ public extension Item {
         }
 
         let newServerUrlFileName = newParentItemRemoteUrl + "/" + itemTarget.filename
+        let destinationRelativePath = newServerUrlFileName.replacingOccurrences(of: account.davFilesUrl, with: "")
+        let destinationIsExcluded = newServerUrlFileName.hasPrefix(account.davFilesUrl) &&
+            (ignoredFiles?.isExcluded(destinationRelativePath) ?? false)
 
         logger.debug("About to modify item.", [.item: modifiedItem])
 
+        if destinationIsExcluded {
+            logger.info("Destination is excluded from sync.", [.item: modifiedItem.itemIdentifier, .name: itemTarget.filename])
+
+            guard let modifiedIgnored = await modifyUnuploaded(
+                itemTarget: itemTarget,
+                baseVersion: baseVersion,
+                changedFields: changedFields,
+                contents: newContents,
+                options: options,
+                request: request,
+                ignoredFiles: ignoredFiles,
+                domain: domain,
+                forcedChunkSize: forcedChunkSize,
+                progress: progress,
+                dbManager: dbManager
+            ) else {
+                logger.error("Unable to modify item with an excluded destination.", [.item: modifiedItem.itemIdentifier, .name: itemTarget.filename])
+                return (nil, NSFileProviderError(.cannotSynchronize))
+            }
+
+            let hasRemoteCounterpart = modifiedItem.isUploaded || !modifiedItem.metadata.etag.isEmpty
+            if hasRemoteCounterpart, !modifiedItem.metadata.isTrashed {
+                guard let remoteDeletionError = await modifiedItem.deleteRemoteItemForExcludedDestination(domain: domain) else {
+                    guard dbManager.markItemAsExcludedFromSync(ocId: modifiedItem.metadata.ocId) else {
+                        logger.error("Unable to persist exclusion state for an excluded destination.", [.item: modifiedItem.itemIdentifier, .name: itemTarget.filename])
+                        return (nil, NSFileProviderError(.cannotSynchronize))
+                    }
+
+                    return (modifiedIgnored, NSFileProviderError(.excludedFromSync))
+                }
+
+                return (nil, remoteDeletionError)
+            }
+
+            return (modifiedIgnored, NSFileProviderError(.excludedFromSync))
+        }
+
+        // We are handling an item that is available locally but not on the server -- so create it
+        // This can happen when a previously ignored file is no longer ignored
+        if !modifiedItem.isUploaded, modifiedItem.isDownloaded, modifiedItem.metadata.etag == "" {
+            return await modifiedItem.createUnuploaded(
+                itemTarget: itemTarget,
+                baseVersion: baseVersion,
+                changedFields: changedFields,
+                contents: newContents,
+                options: options,
+                request: request,
+                ignoredFiles: ignoredFiles,
+                domain: domain,
+                forcedChunkSize: forcedChunkSize,
+                progress: progress,
+                dbManager: dbManager
+            )
+        }
+
         if changedFields.contains(.parentItemIdentifier)
             && newParentItemIdentifier == .trashContainer
             && modifiedItem.metadata.isTrashed

shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift

@@ -606,6 +606,9 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable {
     /// Use this to simulate server-side upload rejections (e.g. 404 path gone, 507 quota).
     public var uploadError: NKError?
 
+    public var deleteError: NKError?
+    public var lastDeleteRemotePath: String?
+
     /// Records the `If-Match` header the most recent upload call carried (nil if none).
     /// Lets tests assert the optimistic-concurrency precondition was sent, and with
     /// which etag. Captured before any injected `uploadError` short-circuit.
@@ -1283,6 +1286,12 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable {
         options _: NKRequestOptions = .init(),
         taskHandler _: @escaping (URLSessionTask) -> Void = { _ in }
     ) async -> (account: String, response: HTTPURLResponse?, error: NKError) {
+        lastDeleteRemotePath = remotePath
+
+        if let deleteError {
+            return (account.ncKitAccount, nil, deleteError)
+        }
+
         guard let item = item(remotePath: remotePath, account: account.ncKitAccount) else {
             return (account.ncKitAccount, nil, .urlError)
         }

shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift

@@ -1705,6 +1705,106 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase {
         XCTAssertEqual(resultItem?.metadata.fileName, "error.bak")
     }
 
+    func testModifyIntoIgnoredDestinationDeletesRemoteItemBeforeLocalExclusion() async throws {
+        let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem, rootTrashItem: rootTrashItem)
+        let ignoredMatcher = IgnoredFilesMatcher(ignoreList: ["*.blend1"], log: FileProviderLogMock())
+
+        let itemMetadata = remoteItem.toItemMetadata(account: Self.account)
+        Self.dbManager.addItemMetadata(itemMetadata)
+
+        var targetMetadata = SendableItemMetadata(value: itemMetadata)
+        targetMetadata.name = "item.blend1"
+        targetMetadata.fileName = "item.blend1"
+        targetMetadata.fileNameView = "item.blend1"
+
+        let item = Item(
+            metadata: itemMetadata,
+            parentItemIdentifier: .rootContainer,
+            account: Self.account,
+            remoteInterface: remoteInterface,
+            dbManager: Self.dbManager
+        )
+        let targetItem = Item(
+            metadata: targetMetadata,
+            parentItemIdentifier: .rootContainer,
+            account: Self.account,
+            remoteInterface: remoteInterface,
+            dbManager: Self.dbManager
+        )
+
+        let (modifiedItem, error) = await item.modify(
+            itemTarget: targetItem,
+            changedFields: [.filename],
+            contents: nil,
+            ignoredFiles: ignoredMatcher,
+            dbManager: Self.dbManager
+        )
+
+        XCTAssertEqual(error as? NSFileProviderError, NSFileProviderError(.excludedFromSync))
+        XCTAssertEqual(modifiedItem?.filename, "item.blend1")
+        XCTAssertEqual(remoteInterface.lastDeleteRemotePath, itemMetadata.remotePath())
+        XCTAssertFalse(rootItem.children.contains { $0.identifier == remoteItem.identifier })
+        XCTAssertTrue(Self.dbManager.isItemExcludedFromSync(ocId: itemMetadata.ocId))
+
+        remoteInterface.deleteError = .urlError
+        let storedMetadata = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: itemMetadata.ocId))
+        let storedItem = Item(
+            metadata: storedMetadata,
+            parentItemIdentifier: .rootContainer,
+            account: Self.account,
+            remoteInterface: remoteInterface,
+            dbManager: Self.dbManager
+        )
+
+        let deletionError = await storedItem.delete(dbManager: Self.dbManager)
+
+        XCTAssertNil(deletionError)
+        XCTAssertEqual(Self.dbManager.itemMetadata(ocId: itemMetadata.ocId)?.deleted, true)
+        XCTAssertFalse(Self.dbManager.isItemExcludedFromSync(ocId: itemMetadata.ocId))
+    }
+
+    func testModifyIntoIgnoredDestinationKeepsRemoteItemWhenCleanupFails() async {
+        let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem, rootTrashItem: rootTrashItem)
+        remoteInterface.deleteError = .urlError
+        let ignoredMatcher = IgnoredFilesMatcher(ignoreList: ["*.blend1"], log: FileProviderLogMock())
+
+        let itemMetadata = remoteItem.toItemMetadata(account: Self.account)
+        Self.dbManager.addItemMetadata(itemMetadata)
+
+        var targetMetadata = SendableItemMetadata(value: itemMetadata)
+        targetMetadata.name = "item.blend1"
+        targetMetadata.fileName = "item.blend1"
+        targetMetadata.fileNameView = "item.blend1"
+
+        let item = Item(
+            metadata: itemMetadata,
+            parentItemIdentifier: .rootContainer,
+            account: Self.account,
+            remoteInterface: remoteInterface,
+            dbManager: Self.dbManager
+        )
+        let targetItem = Item(
+            metadata: targetMetadata,
+            parentItemIdentifier: .rootContainer,
+            account: Self.account,
+            remoteInterface: remoteInterface,
+            dbManager: Self.dbManager
+        )
+
+        let (modifiedItem, error) = await item.modify(
+            itemTarget: targetItem,
+            changedFields: [.filename],
+            contents: nil,
+            ignoredFiles: ignoredMatcher,
+            dbManager: Self.dbManager
+        )
+
+        XCTAssertNil(modifiedItem)
+        XCTAssertEqual((error as? NSFileProviderError)?.code, .cannotSynchronize)
+        XCTAssertTrue(rootItem.children.contains { $0.identifier == remoteItem.identifier })
+        XCTAssertFalse(Self.dbManager.isItemExcludedFromSync(ocId: itemMetadata.ocId))
+    }
+
     func testModifyCreatesFileThatWasPreviouslyIgnoredWithContentsUrlProvided() async throws {
         let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
         let ignoredMatcher = IgnoredFilesMatcher(ignoreList: ["/logs/"], log: FileProviderLogMock())