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 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/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..e93a0c7a 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 ) } @@ -537,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/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index 73ff8e4b..e1b46edc 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 @@ -80,7 +80,7 @@ extension NKFile { sharePermissionsCloudMesh: sharePermissionsCloudMesh, shareType: shareType, size: size, - tags: tags, + tags: tags.map(\.name), uploaded: uploaded, trashbinFileName: trashbinFileName, trashbinOriginalLocation: trashbinOriginalLocation, @@ -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/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 + )) } } } diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Create.swift b/Sources/NextcloudFileProviderKit/Item/Item+Create.swift index 05f6ae3e..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 { @@ -213,7 +219,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/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." + ) + } +} 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 { 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