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 99cbaa76..f6b6c453 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -537,6 +537,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..a05ae6b7 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 + )) } } } diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Create.swift b/Sources/NextcloudFileProviderKit/Item/Item+Create.swift index 05f6ae3e..286c3f45 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." + ) + } +} 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