From 53a3aad196432d5cef83df81c58bed98d3f78d62 Mon Sep 17 00:00:00 2001 From: Iva Horn Date: Tue, 13 Jan 2026 16:22:53 +0100 Subject: [PATCH 1/5] fix: Trash enumerates only locally trashed items. - Introduced new schema property wasTrashedLocally on SendableItemMetadata. - Added corresponding schema version and migration. - Extended related data models accordingly. - Updated Enumerator to no longer enumerate remote trash items based on wasTrashedLocally. Signed-off-by: Iva Horn --- .../Database/FilesDatabaseManager.swift | 13 ++- .../Database/SchemaVersion.swift | 1 + .../Enumeration/Enumerator+Trash.swift | 107 ++++-------------- .../Enumeration/Enumerator.swift | 57 +--------- .../Extensions/NKFile+Extensions.swift | 5 +- .../Extensions/NKTrash+Extensions.swift | 8 +- .../Item/Item+Create.swift | 3 +- .../Item/Item+Ignored.swift | 3 +- .../Item/Item+LockFile.swift | 3 +- .../Item/Item+Modify.swift | 27 ++--- .../Item/Item+Trash.swift | 80 +++++-------- .../NextcloudFileProviderKit/Item/Item.swift | 6 +- .../Metadata/ItemMetadata.swift | 1 + .../Metadata/RealmItemMetadata.swift | 1 + .../Metadata/SendableItemMetadata.swift | 13 ++- Tests/Interface/ItemMetadata+Init.swift | 3 +- Tests/Interface/MockRemoteItem.swift | 3 +- .../EnumeratorTests.swift | 75 +----------- .../RemoteChangeObserverTests.swift | 23 ++-- 19 files changed, 127 insertions(+), 305 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index 47622e1e..425c537b 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -40,7 +40,7 @@ public final class FilesDatabaseManager: Sendable { ) } - private static let schemaVersion = SchemaVersion.addedIsLockFileOfLocalOriginToRealmItemMetadata + private static let schemaVersion = SchemaVersion.addedWasTrashedLocallyToRealmItemMetadata let logger: FileProviderLogger let account: Account @@ -134,6 +134,15 @@ public final class FilesDatabaseManager: Sendable { } } + if oldSchemaVersion == SchemaVersion.addedIsLockFileOfLocalOriginToRealmItemMetadata.rawValue { + migration.enumerateObjects(ofType: RealmItemMetadata.className()) { _, newObject in + guard let newObject else { + return + } + + newObject["wasTrashedLocally"] = false + } + } }, objectTypes: [RealmItemMetadata.self, RemoteFileChunk.self] ) @@ -520,7 +529,7 @@ public final class FilesDatabaseManager: Sendable { do { try database.write { database.add(RealmItemMetadata(value: metadata), update: .all) - logger.debug("Added item metadata.", [.item: metadata.ocId, .name: metadata.name, .url: metadata.serverUrl]) + logger.debug("Added item metadata.", [.item: metadata.ocId, .name: metadata.fileName, .url: metadata.serverUrl]) } } catch { logger.error("Failed to add item metadata.", [.item: metadata.ocId, .name: metadata.name, .url: metadata.serverUrl, .error: error]) diff --git a/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift b/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift index 8019e210..db699f5d 100644 --- a/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift +++ b/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift @@ -9,4 +9,5 @@ enum SchemaVersion: UInt64 { case deletedLocalFileMetadata = 200 case addedLockTokenPropertyToRealmItemMetadata = 201 case addedIsLockFileOfLocalOriginToRealmItemMetadata = 202 + case addedWasTrashedLocallyToRealmItemMetadata = 203 } diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+Trash.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+Trash.swift index d868a3d3..51130273 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+Trash.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+Trash.swift @@ -5,96 +5,29 @@ import NextcloudKit extension Enumerator { - static func completeEnumerationObserver( - _ observer: NSFileProviderEnumerationObserver, - account: Account, - remoteInterface: RemoteInterface, - dbManager: FilesDatabaseManager, - numPage: Int, - trashItems: [NKTrash], - log: any FileProviderLogging - ) { - var metadatas = [SendableItemMetadata]() - for trashItem in trashItems { - let metadata = trashItem.toItemMetadata(account: account) - dbManager.addItemMetadata(metadata) - metadatas.append(metadata) - } - - Task { [metadatas] in - let logger = FileProviderLogger(category: "Enumerator", log: log) - - do { - let items = try await metadatas.toFileProviderItems(account: account, remoteInterface: remoteInterface, dbManager: dbManager, log: log) - - Task { @MainActor in - observer.didEnumerate(items) - logger.info("Did enumerate \(items.count) trash items.") - observer.finishEnumerating(upTo: fileProviderPageforNumPage(numPage)) - } - } catch { - logger.error("Finishing enumeration with error.") - Task { @MainActor in observer.finishEnumeratingWithError(error) } - } - } - } - - static func completeChangesObserver( - _ observer: NSFileProviderChangeObserver, - anchor: NSFileProviderSyncAnchor, - account: Account, - remoteInterface: RemoteInterface, - dbManager: FilesDatabaseManager, - trashItems: [NKTrash], - log: any FileProviderLogging - ) async { + /// + /// Change enumeration completion. + /// + /// `NKTrash` items do not have an ETag. We assume they cannot be modified while they are in the trash. So we will just check by their `ocId`. + /// Newly added items by deletion on the server side or another client are not of interested and we do not want to display them in the local trash. + /// In the end, only the remotely and permanently deleted items are of interest. + /// + static func completeChangesObserver(_ observer: NSFileProviderChangeObserver, anchor: NSFileProviderSyncAnchor, account: Account, dbManager: FilesDatabaseManager, remoteTrashItems: [NKTrash], log: any FileProviderLogging) async { let logger = FileProviderLogger(category: "Enumerator", log: log) - var newTrashedItems = [NSFileProviderItem]() - - // NKTrash items do not have an etag ; we assume they cannot be modified while they are in - // the trash, so we will just check by ocId - var existingTrashedItems = dbManager.trashedItemMetadatas(account: account) - - for trashItem in trashItems { - if let existingTrashItemIndex = existingTrashedItems.firstIndex( - where: { $0.ocId == trashItem.ocId } - ) { - existingTrashedItems.remove(at: existingTrashItemIndex) - continue - } - - let metadata = trashItem.toItemMetadata(account: account) - dbManager.addItemMetadata(metadata) - - let item = await Item( - metadata: metadata, - parentItemIdentifier: .trashContainer, - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - remoteSupportsTrash: remoteInterface.supportsTrash(account: account), - log: log - ) - newTrashedItems.append(item) - - logger.debug("Will enumerate changed trash item.", [.item: metadata.ocId, .name: metadata.fileName]) - } - - let deletedTrashedItemsIdentifiers = existingTrashedItems.map { - NSFileProviderItemIdentifier($0.ocId) - } - if !deletedTrashedItemsIdentifiers.isEmpty { - for itemIdentifier in deletedTrashedItemsIdentifiers { - dbManager.deleteItemMetadata(ocId: itemIdentifier.rawValue) - } - - logger.debug("Will enumerate deleted trashed items: \(deletedTrashedItemsIdentifiers)") - observer.didDeleteItems(withIdentifiers: deletedTrashedItemsIdentifiers) + let localIdentifiers = dbManager.trashedItemMetadatas(account: account).map(\.ocId) + let localSet = Set(localIdentifiers) + let remoteIdentifiers = remoteTrashItems.map(\.ocId) + let remoteSet = Set(remoteIdentifiers) + let orphanedSet = localSet.subtracting(remoteSet) + let orphanedIdentifiers = orphanedSet.map { NSFileProviderItemIdentifier($0) } + + for identifier in orphanedSet { + logger.info("Permanently deleting remote trash item which could not be matched with a local one.", [.item: identifier]) + dbManager.deleteItemMetadata(ocId: identifier) } - if !newTrashedItems.isEmpty { - observer.didUpdate(newTrashedItems) - } + observer.didDeleteItems(withIdentifiers: orphanedIdentifiers) observer.finishEnumeratingChanges(upTo: anchor, moreComing: false) + logger.debug("Finished enumerating remote changes in trash.") } } diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index 99cbaa76..e8820c97 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -76,21 +76,8 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable { public func enumerateItems(for observer: NSFileProviderEnumerationObserver, startingAt page: NSFileProviderPage) { logger.info("Received enumerate items request for enumerator with user", [.account: account.ncKitAccount, .url: serverUrl]) - /* - - inspect the page to determine whether this is an initial or a follow-up request (TODO) - - If this is an enumerator for a directory, the root container or all directories: - - perform a server request to fetch directory contents - If this is an enumerator for the working set: - - perform a server request to update your local database - - fetch the working set from your local database - - - inform the observer about the items returned by the server (possibly multiple times) - - inform the observer that you are finished with this page - */ - if enumeratedItemIdentifier == .trashContainer { - logger.info("Enumerating trash.", [.account: account.ncKitAccount, .url: serverUrl]) + logger.info("Enumerating items in trash.", [.account: account.ncKitAccount, .url: serverUrl]) Task { [weak self] in guard let self else { @@ -111,42 +98,11 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable { return } - let domain = domain - let enumeratedItemIdentifier = enumeratedItemIdentifier - - let (_, trashedItems, _, trashReadError) = await remoteInterface.listingTrashAsync( - filename: nil, - showHiddenFiles: true, - account: account.ncKitAccount, - options: .init(), - taskHandler: { task in - if let domain { - NSFileProviderManager(for: domain)?.register( - task, - forItemWithIdentifier: enumeratedItemIdentifier, - completionHandler: { _ in } - ) - } - } - ) - - guard trashReadError == .success else { - let error = trashReadError.fileProviderError(handlingNoSuchItemErrorUsingItemIdentifier: enumeratedItemIdentifier) ?? NSFileProviderError(.cannotSynchronize) - observer.finishEnumeratingWithError(error) - return - } - - Self.completeEnumerationObserver( - observer, - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - numPage: 1, - trashItems: trashedItems ?? [], - log: logger.log - ) + // We only want to list items deleted on the local device. + // That cannot happen before the initial content enumeration for a file provider domain because the latter does not exist yet. + // Hence the initial trash content enumeration can be finished with an empty set. + observer.finishEnumerating(upTo: Self.fileProviderPageforNumPage(1)) } - return } @@ -334,9 +290,8 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable { observer, anchor: anchor, account: account, - remoteInterface: remoteInterface, dbManager: dbManager, - trashItems: trashedItems ?? [], + remoteTrashItems: trashedItems ?? [], log: logger.log ) } diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index 73ff8e4b..cf0fc4fc 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -15,7 +15,7 @@ extension NKFile { return fileUrl == urlString } - func toItemMetadata(uploaded: Bool = true) -> SendableItemMetadata { + func toItemMetadata(uploaded: Bool = true, wasTrashedLocally: Bool = false) -> SendableItemMetadata { let creationDate = creationDate ?? date let uploadDate = uploadDate ?? date @@ -88,7 +88,8 @@ extension NKFile { uploadDate: uploadDate as Date, urlBase: urlBase, user: user, - userId: userId + userId: userId, + wasTrashedLocally: wasTrashedLocally ) } } diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKTrash+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKTrash+Extensions.swift index 19ca9c68..62710b48 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKTrash+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKTrash+Extensions.swift @@ -5,7 +5,10 @@ import Foundation import NextcloudKit extension NKTrash { - func toItemMetadata(account: Account) -> SendableItemMetadata { + /// + /// Convert a trashed item representation into sendable item metadata. + /// + func toItemMetadata(account: Account, wasTrashedLocally: Bool = false) -> SendableItemMetadata { SendableItemMetadata( ocId: ocId, account: account.ncKitAccount, @@ -35,7 +38,8 @@ extension NKTrash { trashbinDeletionTime: trashbinDeletionTime, urlBase: account.serverUrl, user: account.username, - userId: account.id + userId: account.id, + wasTrashedLocally: wasTrashedLocally ) } } diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Create.swift b/Sources/NextcloudFileProviderKit/Item/Item+Create.swift index 05f6ae3e..6c457ee9 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Create.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Create.swift @@ -213,7 +213,8 @@ public extension Item { uploaded: true, urlBase: account.serverUrl, user: account.username, - userId: account.id + userId: account.id, + wasTrashedLocally: false ) dbManager.addItemMetadata(newMetadata) diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Ignored.swift b/Sources/NextcloudFileProviderKit/Item/Item+Ignored.swift index 8f30b6d7..3f93aa4e 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Ignored.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Ignored.swift @@ -52,7 +52,8 @@ extension Item { uploaded: false, urlBase: account.serverUrl, user: account.username, - userId: account.id + userId: account.id, + wasTrashedLocally: false ) dbManager.addItemMetadata(metadata) diff --git a/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift b/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift index 8c5b993c..2624e881 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+LockFile.swift @@ -126,7 +126,8 @@ extension Item { uploaded: false, urlBase: account.serverUrl, user: account.username, - userId: account.id + userId: account.id, + wasTrashedLocally: false ) dbManager.addItemMetadata(metadata) diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift b/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift index 749aadf6..c4dfb4a2 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift @@ -656,13 +656,13 @@ public extension Item { return (modifiedItem, nil) } else if changedFields.contains(.parentItemIdentifier) && newParentItemIdentifier == .trashContainer { - let (_, capabilities, _, error) = await remoteInterface.currentCapabilities( - account: account, options: .init(), taskHandler: { _ in } - ) + let (_, capabilities, _, error) = await remoteInterface.currentCapabilities(account: account, options: .init(), taskHandler: { _ in }) + guard let capabilities, error == .success else { logger.error("Could not acquire capabilities during item move to trash, won't proceed.", [.item: modifiedItem, .error: error]) return (nil, error.fileProviderError) } + guard capabilities.files?.undelete == true else { logger.error("Cannot delete item as server does not support trashing.", [.item: modifiedItem]) return (nil, NSError(domain: NSCocoaErrorDomain, code: NSFeatureUnsupportedError)) @@ -672,15 +672,8 @@ public extension Item { // Rename the item if necessary before doing the trashing procedures if changedFields.contains(.filename) { let currentParentItemRemotePath = modifiedItem.metadata.serverUrl - let preTrashingRenamedRemotePath = - currentParentItemRemotePath + "/" + itemTarget.filename - let (renameModifiedItem, renameError) = await modifiedItem.move( - newFileName: itemTarget.filename, - newRemotePath: preTrashingRenamedRemotePath, - newParentItemIdentifier: modifiedItem.parentItemIdentifier, - newParentItemRemotePath: currentParentItemRemotePath, - dbManager: dbManager - ) + let preTrashingRenamedRemotePath = currentParentItemRemotePath + "/" + itemTarget.filename + let (renameModifiedItem, renameError) = await modifiedItem.move(newFileName: itemTarget.filename, newRemotePath: preTrashingRenamedRemotePath, newParentItemIdentifier: modifiedItem.parentItemIdentifier, newParentItemRemotePath: currentParentItemRemotePath, dbManager: dbManager) guard renameError == nil, let renameModifiedItem else { logger.error("Could not rename pre-trash item.", [.item: modifiedItem.itemIdentifier, .error: error]) @@ -690,10 +683,12 @@ public extension Item { modifiedItem = renameModifiedItem } - let (trashedItem, trashingError) = await Self.trash( - modifiedItem, account: account, dbManager: dbManager, domain: domain, log: logger.log - ) - guard trashingError == nil else { return (modifiedItem, trashingError) } + let (trashedItem, trashingError) = await Self.trash(modifiedItem, account: account, dbManager: dbManager, domain: domain, log: logger.log) + + guard trashingError == nil else { + return (modifiedItem, trashingError) + } + modifiedItem = trashedItem } else if changedFields.contains(.filename) || changedFields.contains(.parentItemIdentifier) { // Recover the item first diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Trash.swift b/Sources/NextcloudFileProviderKit/Item/Item+Trash.swift index 63cc4c01..cfa9dfc9 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Trash.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Trash.swift @@ -23,16 +23,14 @@ extension Item { } let ocId = modifiedItem.itemIdentifier.rawValue + guard let dirtyMetadata = dbManager.itemMetadata(ocId: ocId) else { - logger.error( - """ - Could not correctly process trashing results, dirty metadata not found. - \(modifiedItem.filename) \(ocId) - """ - ) + logger.error("Could not correctly process trashing results, dirty metadata not found.", [.item: ocId, .name: modifiedItem.filename]) return (modifiedItem, NSFileProviderError(.cannotSynchronize)) } + let dirtyChildren = dbManager.childItems(directoryMetadata: dirtyMetadata) + let dirtyItem = await Item( metadata: dirtyMetadata, parentItemIdentifier: .trashContainer, @@ -83,7 +81,7 @@ extension Item { } } - var postDeleteMetadata = targetItemNKTrash.toItemMetadata(account: account) + var postDeleteMetadata = targetItemNKTrash.toItemMetadata(account: account, wasTrashedLocally: true) postDeleteMetadata.ocId = modifiedItem.itemIdentifier.rawValue dbManager.addItemMetadata(postDeleteMetadata) @@ -124,23 +122,21 @@ extension Item { // Update state of child files childFiles.removeFirst() // This is the target path, already scanned + for file in childFiles { var metadata = file.toItemMetadata() + guard let original = dirtyChildren .filter({ $0.ocId == metadata.ocId || $0.fileId == metadata.fileId }) .first else { - logger.info( - """ - Skipping post-trash child item metadata: \(metadata.fileName) - Could not find matching existing item in database, cannot do ocId correction - """ - ) + logger.info("Skipping post-trash child item metadata. Could not find matching existing item in database, cannot do ocId correction.", [.name: metadata.fileName]) continue } + metadata.ocId = original.ocId // Give original id back dbManager.addItemMetadata(metadata) - logger.info("Note: that was a post-trash child item metadata") + logger.info("The previous addition was a post-trash child item metadata.") } return (postDeleteItem, nil) @@ -158,7 +154,8 @@ extension Item { let logger = FileProviderLogger(category: "Item", log: log) func finaliseRestore(target: NKFile) async -> (Item, Error?) { - let restoredItemMetadata = target.toItemMetadata() + let restoredItemMetadata = target.toItemMetadata(wasTrashedLocally: false) + guard let parentItemIdentifier = await dbManager.parentItemIdentifierWithRemoteFallback( fromMetadata: restoredItemMetadata, remoteInterface: remoteInterface, @@ -175,6 +172,7 @@ extension Item { newFileName: restoredItemMetadata.fileName ) } + dbManager.addItemMetadata(restoredItemMetadata) return await (Item( @@ -194,29 +192,24 @@ extension Item { options: .init(), taskHandler: { _ in } ) + guard restoreError == .success else { - logger.error( - """ - Could not restore item \(modifiedItem.filename) from trash - Received error: \(restoreError.errorDescription) - """ - ) + logger.error("Could not restore item from trash.", [.name: modifiedItem.filename, .error: restoreError.errorDescription]) + return (modifiedItem, restoreError.fileProviderError) } + guard modifiedItem.metadata.trashbinOriginalLocation != "" else { - logger.error( - """ - Could not scan restored item \(modifiedItem.filename). - The trashed file's original location is invalid. - """ - ) + logger.error("Could not scan restored item. The trashed file's original location is invalid.", [.name: modifiedItem.filename]) + if #available(macOS 11.3, *) { return (modifiedItem, NSFileProviderError(.unsyncedEdits)) } + return (modifiedItem, NSFileProviderError(.cannotSynchronize)) } - let originalLocation = - account.davFilesUrl + "/" + modifiedItem.metadata.trashbinOriginalLocation + + let originalLocation = account.davFilesUrl + "/" + modifiedItem.metadata.trashbinOriginalLocation let (_, files, _, enumerateError) = await modifiedItem.remoteInterface.enumerate( remotePath: originalLocation, @@ -228,41 +221,27 @@ extension Item { options: .init(), taskHandler: { _ in } ) + guard enumerateError == .success, !files.isEmpty, let target = files.first else { - logger.error( - """ - Could not scan restored state of file \(originalLocation) - Received error: \(enumerateError.errorDescription) - Files: \(files.count) - """ - ) + logger.error("Could not scan restored state of file.", [.error: enumerateError, .url: originalLocation]) + if #available(macOS 11.3, *) { return (modifiedItem, NSFileProviderError(.unsyncedEdits)) } + return (modifiedItem, enumerateError.fileProviderError) } guard target.ocId == modifiedItem.itemIdentifier.rawValue else { - logger.info( - """ - Restored item \(originalLocation) - does not match \(modifiedItem.filename) - (it is likely that when restoring from the trash, there was another identical item). - """ - ) + logger.info("Restored item at location does not match name (it is likely that when restoring from the trash, there was another identical item).", [.name: modifiedItem.filename, .url: originalLocation]) guard let finalSlashIndex = originalLocation.lastIndex(of: "/") else { return (modifiedItem, NSFileProviderError(.cannotSynchronize)) } + var parentDirectoryRemotePath = originalLocation parentDirectoryRemotePath.removeSubrange(finalSlashIndex ..< originalLocation.endIndex) - - logger.info( - """ - Scanning parent folder at \(parentDirectoryRemotePath) for current - state of item restored from trash. - """ - ) + logger.info("Scanning parent folder at \(parentDirectoryRemotePath) for current state of item restored from trash.") let (_, files, _, folderScanError) = await modifiedItem.remoteInterface.enumerate( remotePath: parentDirectoryRemotePath, @@ -294,6 +273,7 @@ extension Item { finished successfully but the target item restored from trash not found. """ ) + return (modifiedItem, NSFileProviderError(.cannotSynchronize)) } diff --git a/Sources/NextcloudFileProviderKit/Item/Item.swift b/Sources/NextcloudFileProviderKit/Item/Item.swift index c96a0e88..01398746 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item.swift @@ -267,7 +267,8 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { uploaded: true, urlBase: "", // Placeholder as not set in original code user: "", // Placeholder as not set in original code - userId: "" // Placeholder as not set in original code + userId: "", // Placeholder as not set in original code + wasTrashedLocally: false ) return Item( metadata: metadata, @@ -310,7 +311,8 @@ public final class Item: NSObject, NSFileProviderItem, Sendable { uploaded: true, urlBase: "", // Placeholder as not set in original code user: "", // Placeholder as not set in original code - userId: "" // Placeholder as not set in original code + userId: "", // Placeholder as not set in original code + wasTrashedLocally: false ) return Item( metadata: metadata, diff --git a/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift index bf702103..0d129d34 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift @@ -96,6 +96,7 @@ public protocol ItemMetadata: Equatable { var user: String { get set } // The user who owns the file (Nextcloud username) var userId: String { get set } // The user who owns the file (backend user id) // (relevant for alt. backends like LDAP) + var wasTrashedLocally: Bool { get set } } public extension ItemMetadata { diff --git a/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift index dd63bb52..84879e78 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift @@ -103,6 +103,7 @@ class RealmItemMetadata: Object, ItemMetadata { @Persisted var user = "" // The user who owns the file (Nextcloud username) @Persisted var userId = "" // The user who owns the file (backend user id) // (relevant for alt. backends like LDAP) + @Persisted var wasTrashedLocally: Bool = false override func isEqual(_ object: Any?) -> Bool { if let object = object as? RealmItemMetadata { diff --git a/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift index de81f4d4..c5117665 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift @@ -78,6 +78,14 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { public var user: String public var userId: String + /// + /// Whether the item was moved to the trash container on the local device or not. + /// + /// This is `false` for items which were placed in the trash on the server or on another client and left out in the trash container content enumeration. + /// We only want locally trashed items to show up in the trash. + /// + public var wasTrashedLocally: Bool + public init( ocId: String, account: String, @@ -144,7 +152,8 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { uploadDate: Date = Date(), urlBase: String, user: String, - userId: String + userId: String, + wasTrashedLocally: Bool ) { self.ocId = ocId self.account = account @@ -212,6 +221,7 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { self.urlBase = urlBase self.user = user self.userId = userId + self.wasTrashedLocally = wasTrashedLocally } init(value: any ItemMetadata) { @@ -281,5 +291,6 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { urlBase = value.urlBase user = value.user userId = value.userId + wasTrashedLocally = value.wasTrashedLocally } } diff --git a/Tests/Interface/ItemMetadata+Init.swift b/Tests/Interface/ItemMetadata+Init.swift index f12ca385..616f495c 100644 --- a/Tests/Interface/ItemMetadata+Init.swift +++ b/Tests/Interface/ItemMetadata+Init.swift @@ -29,7 +29,8 @@ public extension SendableItemMetadata { size: 0, urlBase: account.serverUrl, user: account.username, - userId: account.id + userId: account.id, + wasTrashedLocally: false ) } } diff --git a/Tests/Interface/MockRemoteItem.swift b/Tests/Interface/MockRemoteItem.swift index 98ff24f1..e1ea2b97 100644 --- a/Tests/Interface/MockRemoteItem.swift +++ b/Tests/Interface/MockRemoteItem.swift @@ -201,7 +201,8 @@ public class MockRemoteItem: Equatable { trashbinOriginalLocation: trashbinOriginalLocation ?? "", urlBase: account.serverUrl, user: account.username, - userId: account.id + userId: account.id, + wasTrashedLocally: false ) } } diff --git a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift index 48b79da0..6d4e85b5 100644 --- a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift +++ b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift @@ -1032,49 +1032,7 @@ final class EnumeratorTests: NextcloudFileProviderKitTestCase { ) let observer = MockEnumerationObserver(enumerator: enumerator) try await observer.enumerateItems() - XCTAssertEqual(observer.items.count, 3) - - let storedItemAMaybe = await Item.storedItem( - identifier: .init(remoteTrashItemA.identifier), - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - log: FileProviderLogMock() - ) - let storedItemA = try XCTUnwrap(storedItemAMaybe) - XCTAssertEqual(storedItemA.itemIdentifier.rawValue, remoteTrashItemA.identifier) - XCTAssertEqual(storedItemA.filename, remoteTrashItemA.name) - XCTAssertEqual(storedItemA.documentSize?.int64Value, remoteTrashItemA.size) - XCTAssertEqual(storedItemA.isDownloaded, false) - XCTAssertEqual(storedItemA.isUploaded, true) - - let storedItemBMaybe = await Item.storedItem( - identifier: .init(remoteTrashItemB.identifier), - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - log: FileProviderLogMock() - ) - let storedItemB = try XCTUnwrap(storedItemBMaybe) - XCTAssertEqual(storedItemB.itemIdentifier.rawValue, remoteTrashItemB.identifier) - XCTAssertEqual(storedItemB.filename, remoteTrashItemB.name) - XCTAssertEqual(storedItemB.documentSize?.int64Value, remoteTrashItemB.size) - XCTAssertEqual(storedItemB.isDownloaded, false) - XCTAssertEqual(storedItemB.isUploaded, true) - - let storedItemCMaybe = await Item.storedItem( - identifier: .init(remoteTrashItemC.identifier), - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - log: FileProviderLogMock() - ) - let storedItemC = try XCTUnwrap(storedItemCMaybe) - XCTAssertEqual(storedItemC.itemIdentifier.rawValue, remoteTrashItemC.identifier) - XCTAssertEqual(storedItemC.filename, remoteTrashItemC.name) - XCTAssertEqual(storedItemC.documentSize?.int64Value, remoteTrashItemC.size) - XCTAssertEqual(storedItemC.isDownloaded, false) - XCTAssertEqual(storedItemC.isUploaded, true) + XCTAssertEqual(observer.items.count, 0) } func testTrashChangeEnumeration() async throws { @@ -1106,43 +1064,16 @@ final class EnumeratorTests: NextcloudFileProviderKitTestCase { rootTrashItem.children = [remoteTrashItemA, remoteTrashItemB] remoteTrashItemB.parent = rootTrashItem try await observer.enumerateChanges() - XCTAssertEqual(observer.changedItems.count, 1) - XCTAssertNotNil(Self.dbManager.itemMetadata(ocId: remoteTrashItemB.identifier)) + XCTAssertEqual(observer.changedItems.count, 0) observer.reset() rootTrashItem.children = [remoteTrashItemB, remoteTrashItemC] remoteTrashItemA.parent = nil remoteTrashItemC.parent = rootTrashItem try await observer.enumerateChanges() - XCTAssertEqual(observer.changedItems.count, 1) + XCTAssertEqual(observer.changedItems.count, 0) XCTAssertEqual(observer.deletedItemIdentifiers.count, 1) XCTAssertEqual(Self.dbManager.itemMetadata(ocId: remoteTrashItemA.identifier)?.deleted, true) - XCTAssertNotNil(Self.dbManager.itemMetadata(ocId: remoteTrashItemB.identifier)) - XCTAssertNotNil(Self.dbManager.itemMetadata(ocId: remoteTrashItemC.identifier)) - } - - func testTrashItemEnumerationFailWhenNoTrashInCapabilities() async { - let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem, rootTrashItem: rootTrashItem) - XCTAssert(remoteInterface.capabilities.contains(##""undelete": true,"##)) - remoteInterface.capabilities = - remoteInterface.capabilities.replacingOccurrences(of: ##""undelete": true,"##, with: "") - - let db = Self.dbManager.ncDatabase() // Strong ref for in memory test db - debugPrint(db) // Avoid build-time warning about unused variable, ensure compiler won't free - let enumerator = Enumerator( - enumeratedItemIdentifier: .trashContainer, - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - log: FileProviderLogMock() - ) - let observer = MockEnumerationObserver(enumerator: enumerator) - do { - try await observer.enumerateItems() - XCTFail("Item enumeration should have failed!") - } catch { - XCTAssertEqual((error as NSError?)?.code, NSFeatureUnsupportedError) - } } func testKeepDownloadedRetainedDuringEnumeration() async throws { diff --git a/Tests/NextcloudFileProviderKitTests/RemoteChangeObserverTests.swift b/Tests/NextcloudFileProviderKitTests/RemoteChangeObserverTests.swift index af2ddb78..e0e1d16e 100644 --- a/Tests/NextcloudFileProviderKitTests/RemoteChangeObserverTests.swift +++ b/Tests/NextcloudFileProviderKitTests/RemoteChangeObserverTests.swift @@ -41,6 +41,8 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase { Task { try await server.run() } + + try await Task.sleep(nanoseconds: 500_000_000) // Add a small delay to ensure the server is fully ready } override func tearDown() async throws { @@ -238,8 +240,7 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase { ) serverFolderA.children.append(newFileInA) - let authExpectation = - XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName) + let authExpectation = XCTNSNotificationExpectation(name: NotifyPushAuthenticatedNotificationName) let changeNotifiedExpectation = XCTestExpectation(description: "Change Notified") let notificationInterface = MockChangeNotificationInterface { @@ -257,9 +258,7 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase { // 2. Act & Assert await wait(for: authExpectation, description: "authentication") - Self.notifyPushServer.send(message: "notify_file") - await wait(for: changeNotifiedExpectation, description: "change notification") // 3. Assert Database State @@ -279,20 +278,14 @@ final class RemoteChangeObserverTests: NextcloudFileProviderKitTestCase { // Check deleted items let deletedFileInA = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: "fileInA")) - XCTAssertTrue( - deletedFileInA.deleted, "File inside updated folder should be marked as deleted." - ) - XCTAssertTrue(deletedFileInA.syncTime >= testStartDate, - "Deleted file's sync time should be updated to current time") - XCTAssertGreaterThan(deletedFileInA.syncTime, originalFileInASyncTime, - "Deleted file's sync time should be newer than original sync time") + XCTAssertTrue(deletedFileInA.deleted, "File inside updated folder should be marked as deleted.") + XCTAssertTrue(deletedFileInA.syncTime >= testStartDate, "Deleted file's sync time should be updated to current time") + XCTAssertGreaterThan(deletedFileInA.syncTime, originalFileInASyncTime, "Deleted file's sync time should be newer than original sync time") let deletedFolderB = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: "folderB")) XCTAssertTrue(deletedFolderB.deleted, "The entire folder should be marked as deleted.") - XCTAssertTrue(deletedFolderB.syncTime >= testStartDate, - "Deleted folder's sync time should be updated to current time") - XCTAssertGreaterThan(deletedFolderB.syncTime, originalFolderBSyncTime, - "Deleted folder's sync time should be newer than original sync time") + XCTAssertTrue(deletedFolderB.syncTime >= testStartDate, "Deleted folder's sync time should be updated to current time") + XCTAssertGreaterThan(deletedFolderB.syncTime, originalFolderBSyncTime, "Deleted folder's sync time should be newer than original sync time") } func testIgnoreNonFileNotifications() async throws { From 6340c62b8d7c7c5099c8e6122f5c90594bb60cdc Mon Sep 17 00:00:00 2001 From: Iva Horn Date: Thu, 15 Jan 2026 14:02:17 +0100 Subject: [PATCH 2/5] fix: Lowered Swift package version to 6.1 To stay compatible with the existing tool chain in our CI. The requirement of 6.2 is not necessary as of now. Signed-off-by: Iva Horn --- .swift-version | 2 +- Package.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.swift-version b/.swift-version index 913671cd..e8f1734a 100644 --- a/.swift-version +++ b/.swift-version @@ -1 +1 @@ -6.2 \ No newline at end of file +6.1 \ No newline at end of file diff --git a/Package.swift b/Package.swift index b953100b..b7277acd 100644 --- a/Package.swift +++ b/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version: 6.2 +// swift-tools-version: 6.1 // The swift-tools-version declares the minimum version of Swift required to build this package. import PackageDescription From c3d6e6d0127e97a8b342a6aef7ed0a0a18665512 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Mon, 6 Jul 2026 13:41:50 +0200 Subject: [PATCH 3/5] fix: prevent data loss when parent folder moves during sync. Items with pending uploads and local origin lock files are protected from deletion in the recursive directory delete path, so a moved or renamed parent no longer wipes unsynced children. Pending upload items are also skipped in the enumerator deletion pass, and reported changes are sorted so parents come before children to avoid duplicate folders on rename. Backport of nextcloud/desktop#10134, adapted to the enumerator architecture of the 3.2.x line. The 404 child deferral and move reconciliation logic from master do not apply here. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Camila Ayres --- .../FilesDatabaseManager+Directories.swift | 13 + .../Enumeration/Enumerator.swift | 11 + .../Item/Item+Create.swift | 6 + .../MoveSafeDeletionTests.swift | 264 ++++++++++++++++++ 4 files changed, 294 insertions(+) create mode 100644 Tests/NextcloudFileProviderKitTests/MoveSafeDeletionTests.swift diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Directories.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Directories.swift index 1e18e44a..f4ba3cc8 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Directories.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Directories.swift @@ -74,7 +74,20 @@ public extension FilesDatabaseManager { $0.account == directoryAccount && $0.serverUrl.starts(with: directoryUrlPath) } + // Protect items whose local data is not yet on the server from deletion when their + // parent is removed. Otherwise a moved or renamed parent would wipe unsynced children. + // TODO: the parent directory itself is still deleted here, so a protected child is + // orphaned once its upload completes. Follow up by deferring parent deletion or + // reparenting the child after the upload finishes. for result in results { + if result.status >= Status.inUpload.rawValue { + logger.info("Skipping deletion of child with pending upload.", [.item: result.ocId]) + continue + } + if result.isLockFileOfLocalOrigin { + logger.info("Skipping deletion of local origin lock file during directory delete.", [.item: result.ocId, .name: result.fileName]) + continue + } let inactiveItemMetadata = SendableItemMetadata(value: result) do { try database.write { result.deleted = true } diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index e8820c97..e93a0c7a 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -492,6 +492,17 @@ public final class Enumerator: NSObject, NSFileProviderEnumerator, Sendable { allDeletedMetadatas = deletedMetadatas } + // Protect items with pending uploads: their local data is not yet on the server. + allDeletedMetadatas.removeAll { deletedMetadata in + guard deletedMetadata.status >= Status.inUpload.rawValue else { return false } + logger.info("Skipping deletion of item with pending upload.", [.item: deletedMetadata.ocId]) + return true + } + + // Report parents before children so macOS creates a moved directory before placing its + // contents, preventing both the old and new folder name from appearing during a rename. + allUpdatedMetadatas.sort { $0.remotePath().count < $1.remotePath().count } + let allFpItemDeletionsIdentifiers = Array( allDeletedMetadatas.map { NSFileProviderItemIdentifier($0.ocId) }) if !allFpItemDeletionsIdentifiers.isEmpty { diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Create.swift b/Sources/NextcloudFileProviderKit/Item/Item+Create.swift index 6c457ee9..1ac59b15 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Create.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Create.swift @@ -23,6 +23,12 @@ public extension Item { ) async -> (Item?, Error?) { let logger = FileProviderLogger(category: "Item", log: log) + // Note: when a parent folder is renamed on another client and an editor has a file open + // inside it, the editor may recreate the old folder here, producing a server side duplicate. + // NSFilePresenter callbacks are only delivered when the writer uses NSFileCoordinator. + // It is not confirmed whether the File Provider daemon already does this internally when + // processing didUpdate renames. If it does not, wrapping rename propagation in an + // NSFileCoordinator coordinated write would notify registered presenters. let (_, _, _, createError) = await remoteInterface.createFolder( remotePath: remotePath, account: account, options: .init(), taskHandler: { task in if let domain, let itemTemplate { diff --git a/Tests/NextcloudFileProviderKitTests/MoveSafeDeletionTests.swift b/Tests/NextcloudFileProviderKitTests/MoveSafeDeletionTests.swift new file mode 100644 index 00000000..265eef7a --- /dev/null +++ b/Tests/NextcloudFileProviderKitTests/MoveSafeDeletionTests.swift @@ -0,0 +1,264 @@ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: LGPL-3.0-or-later + +@preconcurrency import FileProvider +import Foundation +@testable import NextcloudFileProviderKit +import NextcloudFileProviderKitMocks +import RealmSwift +import TestInterface +import XCTest + +// MARK: - Move-safe deletion + +final class MoveSafeDeletionTests: NextcloudFileProviderKitTestCase { + static let account = Account( + user: "testUser", id: "testUserId", serverUrl: "https://mock.nc.com", password: "abcd" + ) + + static let dbManager = FilesDatabaseManager( + account: account, + databaseDirectory: makeDatabaseDirectory(), + fileProviderDomainIdentifier: NSFileProviderDomainIdentifier("test"), + log: FileProviderLogMock() + ) + + override func setUp() { + super.setUp() + Realm.Configuration.defaultConfiguration.inMemoryIdentifier = name + } + + func testDeleteDirectorySkipsChildrenWithPendingUpload() throws { + let dir = RealmItemMetadata() + dir.ocId = "upload-dir" + dir.account = "TestAccount" + dir.serverUrl = "https://cloud.example.com/files" + dir.fileName = "uploads" + dir.directory = true + + let normalChild = RealmItemMetadata() + normalChild.ocId = "normal-child" + normalChild.account = "TestAccount" + normalChild.serverUrl = "https://cloud.example.com/files/uploads" + normalChild.fileName = "synced.txt" + normalChild.status = Status.normal.rawValue + + let uploadingChild = RealmItemMetadata() + uploadingChild.ocId = "uploading-child" + uploadingChild.account = "TestAccount" + uploadingChild.serverUrl = "https://cloud.example.com/files/uploads" + uploadingChild.fileName = "uploading.txt" + uploadingChild.status = Status.uploading.rawValue + + let inUploadChild = RealmItemMetadata() + inUploadChild.ocId = "inupload-child" + inUploadChild.account = "TestAccount" + inUploadChild.serverUrl = "https://cloud.example.com/files/uploads" + inUploadChild.fileName = "queued.txt" + inUploadChild.status = Status.inUpload.rawValue + + let realm = Self.dbManager.ncDatabase() + try realm.write { + realm.add(dir) + realm.add(normalChild) + realm.add(uploadingChild) + realm.add(inUploadChild) + } + + let deleted = Self.dbManager.deleteDirectoryAndSubdirectoriesMetadata( + ocId: "upload-dir" + ) + + XCTAssertNotNil(deleted) + let deletedOcIds = deleted?.map(\.ocId) ?? [] + XCTAssertTrue(deletedOcIds.contains("upload-dir"), "Directory itself should be deleted") + XCTAssertTrue(deletedOcIds.contains("normal-child"), "Normal child should be deleted") + XCTAssertFalse( + deletedOcIds.contains("uploading-child"), + "Child with uploading status should be skipped" + ) + XCTAssertFalse( + deletedOcIds.contains("inupload-child"), + "Child with inUpload status should be skipped" + ) + + let uploadingItem = Self.dbManager.itemMetadata(ocId: "uploading-child") + XCTAssertNotNil(uploadingItem) + XCTAssertFalse( + uploadingItem?.deleted ?? true, + "Uploading child should not be marked as deleted" + ) + } + + /// `uploadError (4)` satisfies `status >= inUpload (2)`, so items that failed + /// to upload are preserved just like items that are actively uploading. + /// This keeps the upload error state visible to the user rather than losing it silently. + func testDeleteDirectorySkipsChildrenWithUploadError() throws { + let dir = RealmItemMetadata() + dir.ocId = "uperr-dir" + dir.account = "TestAccount" + dir.serverUrl = "https://cloud.example.com/files" + dir.fileName = "work" + dir.directory = true + + let uploadErrorChild = RealmItemMetadata() + uploadErrorChild.ocId = "uperr-child" + uploadErrorChild.account = "TestAccount" + uploadErrorChild.serverUrl = "https://cloud.example.com/files/work" + uploadErrorChild.fileName = "failed.txt" + uploadErrorChild.status = Status.uploadError.rawValue + + let realm = Self.dbManager.ncDatabase() + try realm.write { + realm.add(dir) + realm.add(uploadErrorChild) + } + + let deleted = Self.dbManager.deleteDirectoryAndSubdirectoriesMetadata(ocId: "uperr-dir") + + XCTAssertNotNil(deleted) + let deletedOcIds = deleted?.map(\.ocId) ?? [] + XCTAssertFalse( + deletedOcIds.contains("uperr-child"), + "Child with uploadError status must be skipped since status >= inUpload protects it." + ) + + let survivingChild = Self.dbManager.itemMetadata(ocId: "uperr-child") + XCTAssertNotNil(survivingChild) + XCTAssertFalse(survivingChild?.deleted ?? true) + } + + func testDeleteDirectoryDeletesChildrenWithDownloadError() throws { + let dir = RealmItemMetadata() + dir.ocId = "dl-err-dir" + dir.account = "TestAccount" + dir.serverUrl = "https://cloud.example.com/files" + dir.fileName = "errors" + dir.directory = true + + let dlErrorChild = RealmItemMetadata() + dlErrorChild.ocId = "dl-err-child" + dlErrorChild.account = "TestAccount" + dlErrorChild.serverUrl = "https://cloud.example.com/files/errors" + dlErrorChild.fileName = "broken.pdf" + dlErrorChild.status = Status.downloadError.rawValue + + let realm = Self.dbManager.ncDatabase() + try realm.write { + realm.add(dir) + realm.add(dlErrorChild) + } + + let deleted = Self.dbManager.deleteDirectoryAndSubdirectoriesMetadata( + ocId: "dl-err-dir" + ) + + XCTAssertNotNil(deleted) + let deletedOcIds = deleted?.map(\.ocId) ?? [] + XCTAssertTrue( + deletedOcIds.contains("dl-err-child"), + "Download error items should still be deleted; only upload status items are protected." + ) + } + + func testDeleteDirectorySkipsLocalOriginLockFile() throws { + let dir = RealmItemMetadata() + dir.ocId = "lock-del-dir" + dir.account = "TestAccount" + dir.serverUrl = "https://cloud.example.com/files" + dir.fileName = "work" + dir.directory = true + + let normalChild = RealmItemMetadata() + normalChild.ocId = "lock-del-normal" + normalChild.account = "TestAccount" + normalChild.serverUrl = "https://cloud.example.com/files/work" + normalChild.fileName = "report.docx" + normalChild.status = Status.normal.rawValue + + let lockFile = RealmItemMetadata() + lockFile.ocId = "lock-del-lockfile" + lockFile.account = "TestAccount" + lockFile.serverUrl = "https://cloud.example.com/files/work" + lockFile.fileName = ".~lock.report.docx#" + lockFile.isLockFileOfLocalOrigin = true + + let realm = Self.dbManager.ncDatabase() + try realm.write { + realm.add(dir) + realm.add(normalChild) + realm.add(lockFile) + } + + let deleted = Self.dbManager.deleteDirectoryAndSubdirectoriesMetadata(ocId: "lock-del-dir") + + XCTAssertNotNil(deleted) + let deletedOcIds = deleted?.map(\.ocId) ?? [] + XCTAssertTrue(deletedOcIds.contains("lock-del-dir"), "Directory itself must be deleted.") + XCTAssertTrue(deletedOcIds.contains("lock-del-normal"), "Normal child must be deleted.") + XCTAssertFalse( + deletedOcIds.contains("lock-del-lockfile"), + "Local origin lock file must be preserved since the editor still has it open." + ) + + let survivingLock = Self.dbManager.itemMetadata(ocId: "lock-del-lockfile") + XCTAssertNotNil(survivingLock) + XCTAssertFalse(survivingLock?.deleted ?? true) + } + + /// When a folder rename and its children are both pending in the working set change + /// list, macOS processes items in the order the extension reports them. If a child + /// arrives before its parent's rename, macOS creates the destination folder to house + /// the child, then cannot complete the parent rename because the destination already + /// exists, leaving both the old and new folder name visible on disk simultaneously. + /// + /// Regression test: verify that pendingWorkingSetChanges output, when sorted by the + /// fix applied in completeChangesObserver, places parent directories before children. + func testPendingChangesAreSortedParentBeforeChild() throws { + let now = Date() + let recentSync = now.addingTimeInterval(-60) + let anchorDate = now.addingTimeInterval(-300) + + // Parent directory and child file both have syncTime newer than the anchor so + // they appear in pendingWorkingSetChanges. + var dirMeta = SendableItemMetadata( + ocId: "sort-dir", fileName: "2026-renamed", account: Self.account + ) + dirMeta.serverUrl = Self.account.davFilesUrl + "/container" + dirMeta.directory = true + dirMeta.visitedDirectory = true + dirMeta.etag = "V2" + dirMeta.uploaded = true + dirMeta.syncTime = recentSync + Self.dbManager.addItemMetadata(dirMeta) + + var fileMeta = SendableItemMetadata( + ocId: "sort-file", fileName: "Spreadsheet.xlsx", account: Self.account + ) + fileMeta.serverUrl = Self.account.davFilesUrl + "/container/2026-renamed" + fileMeta.downloaded = true + fileMeta.uploaded = true + fileMeta.etag = "V2" + fileMeta.syncTime = recentSync + Self.dbManager.addItemMetadata(fileMeta) + + let pending = Self.dbManager.pendingWorkingSetChanges( + account: Self.account, since: anchorDate + ) + + // Both items must be in the pending list. + XCTAssertTrue(pending.updated.contains(where: { $0.ocId == "sort-dir" })) + XCTAssertTrue(pending.updated.contains(where: { $0.ocId == "sort-file" })) + + // Apply the same sort that completeChangesObserver uses before reporting to macOS. + let sorted = pending.updated.sorted { $0.remotePath().count < $1.remotePath().count } + + let dirIndex = try XCTUnwrap(sorted.firstIndex(where: { $0.ocId == "sort-dir" })) + let fileIndex = try XCTUnwrap(sorted.firstIndex(where: { $0.ocId == "sort-file" })) + + XCTAssertLessThan( + dirIndex, fileIndex, + "Parent directory must sort before its children to prevent duplicate folders on rename." + ) + } +} From 6e93c1f8c841f3b7021d29d0138633d5dc52ff69 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 7 Jul 2026 22:31:02 +0200 Subject: [PATCH 4/5] fix(interface): adapt NextcloudKit extension to 7.3.x API. NextcloudKit 7.3.x changed several signatures: upload and download now report a raw response, uploadChunk was replaced by uploadChunkAsync, and NKFile.tags became [NKTag]. Parse the metadata callers expect from the response headers, provide a conforming downloadAsync wrapper, and switch chunked upload to uploadChunkAsync, keeping the RemoteInterface protocol and all call sites unchanged. This makes the 3.2.x line build against NextcloudKit 7.3.x, which CI resolves. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Camila Ayres --- .../Extensions/NKFile+Extensions.swift | 2 +- .../NextcloudKit+RemoteInterface.swift | 97 ++++++++++++++++--- 2 files changed, 83 insertions(+), 16 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index cf0fc4fc..e1b46edc 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -80,7 +80,7 @@ extension NKFile { sharePermissionsCloudMesh: sharePermissionsCloudMesh, shareType: shareType, size: size, - tags: tags, + tags: tags.map(\.name), uploaded: uploaded, trashbinFileName: trashbinFileName, trashbinOriginalLocation: trashbinOriginalLocation, diff --git a/Sources/NextcloudFileProviderKit/Interface/NextcloudKit+RemoteInterface.swift b/Sources/NextcloudFileProviderKit/Interface/NextcloudKit+RemoteInterface.swift index 6e89fa53..0477e625 100644 --- a/Sources/NextcloudFileProviderKit/Interface/NextcloudKit+RemoteInterface.swift +++ b/Sources/NextcloudFileProviderKit/Interface/NextcloudKit+RemoteInterface.swift @@ -60,7 +60,17 @@ extension NextcloudKit: RemoteInterface { requestHandler: requestHandler, taskHandler: taskHandler, progressHandler: progressHandler - ) { account, ocId, etag, date, size, response, nkError in + ) { account, response, nkError in + let allHeaderFields = response?.response?.allHeaderFields + let ocId = self.nkCommonInstance.findHeader("oc-fileid", allHeaderFields: allHeaderFields) + let etag = self.nkCommonInstance.normalizedETag(self.nkCommonInstance.findHeader("oc-etag", allHeaderFields: allHeaderFields)) + let date = self.nkCommonInstance.findHeader("date", allHeaderFields: allHeaderFields)?.parsedDate(using: "EEE, dd MMM y HH:mm:ss zzz") + var size: Int64 = 0 + + if let value = allHeaderFields?["Content-Length"] as? String { + size = Int64(value) ?? 0 + } + continuation.resume(returning: ( account, ocId, @@ -88,7 +98,7 @@ extension NextcloudKit: RemoteInterface { chunkCounter: @escaping (_ counter: Int) -> Void = { _ in }, log: any FileProviderLogging, chunkUploadStartHandler: @escaping (_ filesChunk: [RemoteFileChunk]) -> Void = { _ in }, - requestHandler: @escaping (_ request: UploadRequest) -> Void = { _ in }, + requestHandler _: @escaping (_ request: UploadRequest) -> Void = { _ in }, taskHandler: @escaping (_ task: URLSessionTask) -> Void = { _ in }, progressHandler: @escaping (Progress) -> Void = { _ in }, chunkUploadCompleteHandler: @escaping (_ fileChunk: RemoteFileChunk) -> Void = { _ in } @@ -154,8 +164,10 @@ extension NextcloudKit: RemoteInterface { """ ) - return await withCheckedContinuation { continuation in - uploadChunk( + var startedChunks: [RemoteFileChunk] = [] + + do { + let (uploadAccount, file) = try await uploadChunkAsync( directory: directory, fileChunksOutputDirectory: fileChunksOutputDirectory, fileName: fileName, @@ -168,17 +180,19 @@ extension NextcloudKit: RemoteInterface { chunkSize: chunkSize, account: account.ncKitAccount, options: options, - numChunks: currentNumChunksUpdateHandler, - counterChunk: chunkCounter, - start: { processedChunks in + chunkProgressHandler: { total, counter in + currentNumChunksUpdateHandler(total) + chunkCounter(counter) + }, + uploadStart: { processedChunks in let chunks = RemoteFileChunk.fromNcKitChunks( processedChunks, remoteChunkStoreFolderName: remoteChunkStoreFolderName ) + startedChunks = chunks chunkUploadStartHandler(chunks) }, - requestHandler: requestHandler, - taskHandler: taskHandler, - progressHandler: { totalBytesExpected, totalBytes, _ in + uploadTaskHandler: taskHandler, + uploadProgressHandler: { totalBytesExpected, totalBytes, _ in let currentProgress = Progress(totalUnitCount: totalBytesExpected) currentProgress.completedUnitCount = totalBytes progressHandler(currentProgress) @@ -190,11 +204,64 @@ extension NextcloudKit: RemoteInterface { ) chunkUploadCompleteHandler(chunk) } - ) { account, receivedChunks, file, error in - let chunks = RemoteFileChunk.fromNcKitChunks( - receivedChunks ?? [], remoteChunkStoreFolderName: remoteChunkStoreFolderName - ) - continuation.resume(returning: (account, chunks, file, error)) + ) + + return (uploadAccount, startedChunks, file, .success) + } catch let nkError as NKError { + return (account.ncKitAccount, nil, nil, nkError) + } catch { + return (account.ncKitAccount, nil, nil, .urlError) + } + } + + /// NextcloudKit 7.3.x changed its own downloadAsync to return the raw AFDownloadResponse, + /// which no longer matches RemoteInterface. Provide a conforming wrapper that parses the + /// metadata the callers expect from the response headers. + public func downloadAsync( + serverUrlFileName: Any, + fileNameLocalPath: String, + account: String, + options: NKRequestOptions = .init(), + requestHandler: @escaping (_ request: DownloadRequest) -> Void = { _ in }, + taskHandler: @Sendable @escaping (_ task: URLSessionTask) -> Void = { _ in }, + progressHandler: @escaping (_ progress: Progress) -> Void = { _ in } + ) async -> ( + account: String, + etag: String?, + date: Date?, + length: Int64, + headers: [AnyHashable: any Sendable]?, + afError: AFError?, + nkError: NKError + ) { + await withCheckedContinuation { continuation in + download( + serverUrlFileName: serverUrlFileName, + fileNameLocalPath: fileNameLocalPath, + account: account, + options: options, + requestHandler: requestHandler, + taskHandler: taskHandler, + progressHandler: progressHandler + ) { account, response, nkError in + let allHeaderFields = response?.response?.allHeaderFields + let etag = self.nkCommonInstance.normalizedETag(self.nkCommonInstance.findHeader("oc-etag", allHeaderFields: allHeaderFields)) + let date = self.nkCommonInstance.findHeader("date", allHeaderFields: allHeaderFields)?.parsedDate(using: "EEE, dd MMM y HH:mm:ss zzz") + var length: Int64 = 0 + + if let value = allHeaderFields?["Content-Length"] as? String { + length = Int64(value) ?? 0 + } + + continuation.resume(returning: ( + account, + etag, + date, + length, + allHeaderFields as? [AnyHashable: any Sendable], + response?.error, + nkError + )) } } } From ac1f5717169f6c2ddfecbc0f49945951b4e935cc Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 7 Jul 2026 22:51:36 +0200 Subject: [PATCH 5/5] fix(test): add version to capability mocks NextcloudCapabilitiesKit 2.5.x requires ocs.data.version, so the inline capability JSONs without it made Capabilities(data:) return nil and the force unwrap in capabilitiesFromMockJSON crashed the test run. Add a minimal version block to each mock. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Camila Ayres --- .../RemoteInterfaceTests.swift | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Tests/NextcloudFileProviderKitTests/RemoteInterfaceTests.swift b/Tests/NextcloudFileProviderKitTests/RemoteInterfaceTests.swift index d73ddef5..82676c5d 100644 --- a/Tests/NextcloudFileProviderKitTests/RemoteInterfaceTests.swift +++ b/Tests/NextcloudFileProviderKitTests/RemoteInterfaceTests.swift @@ -81,6 +81,11 @@ struct RemoteInterfaceExtensionTests { "message": "OK" }, "data": { + "version": { + "major": 28, + "minor": 0, + "micro": 4 + }, "capabilities": { "files": { "undelete": false @@ -187,6 +192,11 @@ struct RemoteInterfaceExtensionTests { "message": "OK" }, "data": { + "version": { + "major": 28, + "minor": 0, + "micro": 4 + }, "capabilities": { "files": { "undelete": false @@ -242,6 +252,11 @@ struct RemoteInterfaceExtensionTests { "message": "OK" }, "data": { + "version": { + "major": 28, + "minor": 0, + "micro": 4 + }, "capabilities": { "core": { "pollinterval": 60