From 109370b1ee6d51b93d90358cfc904ff7b5f160cd Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 10:40:53 +0800 Subject: [PATCH 01/38] Add materialisation-relevant property to item metadatas for folders Signed-off-by: Claudio Cambra --- .../Metadata/ItemMetadata.swift | 1 + .../Metadata/RealmItemMetadata.swift | 1 + .../Metadata/SendableItemMetadata.swift | 10 +++++++--- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift index dc27e8ea..d7e3d58d 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift @@ -93,6 +93,7 @@ public protocol ItemMetadata: Equatable { var downloaded: Bool { get set } var uploaded: Bool { get set } var keepDownloaded: Bool { get set } + var visitedDirectory: Bool { get set } var trashbinFileName: String { get set } var trashbinOriginalLocation: String { get set } var trashbinDeletionTime: Date { get set } diff --git a/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift index 322fb6c9..7945f129 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift @@ -85,6 +85,7 @@ internal class RealmItemMetadata: Object, ItemMetadata { @Persisted public var downloaded = false @Persisted public var uploaded = false @Persisted public var keepDownloaded = false + @Persisted public var visitedDirectory = false @Persisted public var trashbinFileName = "" @Persisted public var trashbinOriginalLocation = "" @Persisted public var trashbinDeletionTime = Date() diff --git a/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift index 532ebfbe..adf27f11 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/SendableItemMetadata.swift @@ -64,6 +64,7 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { public var downloaded: Bool public var uploaded: Bool public var keepDownloaded: Bool + public var visitedDirectory: Bool public var trashbinFileName: String public var trashbinOriginalLocation: String public var trashbinDeletionTime: Date @@ -91,12 +92,12 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { fileId: String, fileName: String, fileNameView: String, - hasPreview: Bool, + hasPreview: Bool = false, hidden: Bool = false, - iconName: String, + iconName: String = "", iconUrl: String = "", livePhotoFile: String? = nil, - mountType: String, + mountType: String = "", name: String = "", note: String = "", ownerId: String, @@ -127,6 +128,7 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { downloaded: Bool = false, uploaded: Bool = false, keepDownloaded: Bool = false, + visitedDirectory: Bool = false, trashbinFileName: String = "", trashbinOriginalLocation: String = "", trashbinDeletionTime: Date = Date(), @@ -189,6 +191,7 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { self.downloaded = downloaded self.uploaded = uploaded self.keepDownloaded = keepDownloaded + self.visitedDirectory = visitedDirectory self.trashbinFileName = trashbinFileName self.trashbinOriginalLocation = trashbinOriginalLocation self.trashbinDeletionTime = trashbinDeletionTime @@ -252,6 +255,7 @@ public struct SendableItemMetadata: ItemMetadata, Sendable { self.downloaded = value.downloaded self.uploaded = value.uploaded self.keepDownloaded = value.keepDownloaded + self.visitedDirectory = value.visitedDirectory self.tags = value.tags self.trashbinFileName = value.trashbinFileName self.trashbinOriginalLocation = value.trashbinOriginalLocation From 30f849a8e9c352b37372dc3050e6e1760596ca8d Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 10:41:28 +0800 Subject: [PATCH 02/38] Add method to retrieve all materialised items from database Signed-off-by: Claudio Cambra --- .../Database/FilesDatabaseManager.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index 90e63b83..d06e7be3 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -601,4 +601,13 @@ public final class FilesDatabaseManager: Sendable { } return NSFileProviderItemIdentifier(parentMetadata.ocId) } + + public func materialisedItemMetadatas(account: String) -> [SendableItemMetadata] { + itemMetadatas + .where { + $0.account == account && + (($0.directory && $0.visitedDirectory) || (!$0.directory && $0.downloaded)) + } + .toUnmanagedResults() + } } From 2023a23133a43fb376766118175c9df8bc3555f3 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 10:41:40 +0800 Subject: [PATCH 03/38] Add method to test retrieval of all materialised files from database Signed-off-by: Claudio Cambra --- .../FilesDatabaseManagerTests.swift | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift b/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift index b04d73ba..78eb597f 100644 --- a/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift +++ b/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift @@ -1089,4 +1089,64 @@ final class FilesDatabaseManagerTests: XCTestCase { XCTAssertNotNil(retrievedParentIdentifier) XCTAssertEqual(retrievedParentIdentifier?.rawValue, remoteFolder.identifier) } + + func testMaterialisedFiles() async throws { + let itemA = RealmItemMetadata() + let itemB = RealmItemMetadata() + let itemC = RealmItemMetadata() + let folderA = RealmItemMetadata() + let folderB = RealmItemMetadata() + let folderC = RealmItemMetadata() + let notFolderA = RealmItemMetadata() + let notFolderB = RealmItemMetadata() + + folderA.directory = true + folderB.directory = true + folderC.directory = true + + itemA.ocId = "itemA" + itemB.ocId = "itemB" + itemC.ocId = "itemC" + folderA.ocId = "folderA" + folderB.ocId = "folderB" + folderC.ocId = "folderC" + notFolderA.ocId = "notFolderA" + notFolderB.ocId = "notFolderB" + + itemA.account = Self.account.ncKitAccount + itemB.account = Self.account.ncKitAccount + itemC.account = "another account" + folderA.account = Self.account.ncKitAccount + folderB.account = Self.account.ncKitAccount + folderC.account = "another account" + notFolderA.account = Self.account.ncKitAccount + notFolderB.account = "another account" + + itemA.downloaded = true + itemB.downloaded = false + itemC.downloaded = true + folderA.visitedDirectory = true + folderB.visitedDirectory = false + folderC.visitedDirectory = true + notFolderA.visitedDirectory = true + notFolderB.visitedDirectory = true + + let realm = Self.dbManager.ncDatabase() + try realm.write { + realm.add(itemA) + realm.add(itemB) + realm.add(itemC) + realm.add(folderA) + realm.add(folderB) + realm.add(folderC) + } + + let materialised = + Self.dbManager.materialisedItemMetadatas(account: Self.account.ncKitAccount) + XCTAssertEqual(materialised.count, 2) + + let materialisedOcIds = materialised.map(\.ocId) + XCTAssertTrue(materialisedOcIds.contains(itemA.ocId)) + XCTAssertTrue(materialisedOcIds.contains(folderA.ocId)) + } } From 3cd9b6c8966a8152a349495ed36170503db4fc2e Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 11:03:37 +0800 Subject: [PATCH 04/38] Observe both newly unmaterialised and newly materialised items Signed-off-by: Claudio Cambra --- .../MaterialisedEnumerationObserver.swift | 72 +++++++++++-------- ...MaterialisedEnumerationObserverTests.swift | 4 +- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift b/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift index cd16aeae..d61be780 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift @@ -21,13 +21,17 @@ public class MaterialisedEnumerationObserver: NSObject, NSFileProviderEnumeratio static let logger = Logger(subsystem: Logger.subsystem, category: "materialisedobservation") public let ncKitAccount: String let dbManager: FilesDatabaseManager - private let completionHandler: (_ deletedOcIds: Set) -> Void - private var allEnumeratedItemIds: Set = .init() + private let completionHandler: ( + _ materialisedIds: Set, _ unmaterialisedIds: Set + ) -> Void + private var allEnumeratedItemIds = Set() public required init( ncKitAccount: String, dbManager: FilesDatabaseManager, - completionHandler: @escaping (_ deletedOcIds: Set) -> Void + completionHandler: @escaping ( + _ materialisedIds: Set, _ unmaterialisedIds: Set + ) -> Void ) { self.ncKitAccount = ncKitAccount self.dbManager = dbManager @@ -36,11 +40,7 @@ public class MaterialisedEnumerationObserver: NSObject, NSFileProviderEnumeratio } public func didEnumerate(_ updatedItems: [NSFileProviderItemProtocol]) { - let updatedItemsIds = Array(updatedItems.map(\.itemIdentifier.rawValue)) - - for updatedItemsId in updatedItemsIds { - allEnumeratedItemIds.insert(updatedItemsId) - } + updatedItems.map(\.itemIdentifier.rawValue).forEach { allEnumeratedItemIds.insert($0) } } public func finishEnumerating(upTo _: NSFileProviderPage?) { @@ -69,34 +69,50 @@ public class MaterialisedEnumerationObserver: NSObject, NSFileProviderEnumeratio _ itemIds: Set, account: String, dbManager: FilesDatabaseManager, - completionHandler: @escaping (_ deletedOcIds: Set) -> Void + completionHandler: @escaping ( + _ materialisedIds: Set, _ unmaterialisedIds: Set + ) -> Void ) { - let databaseLocalFileMetadatas = dbManager - .itemMetadatas - .where({ $0.account == account && $0.downloaded }) - .toUnmanagedResults() - var noLongerMaterialisedIds = Set() + let materialisedMetadatas = dbManager.materialisedItemMetadatas(account: account) + var materialisedMetadatasMap = [String: SendableItemMetadata]() + var unmaterialisedIds = Set() + var newMaterialisedIds = Set() - DispatchQueue.global(qos: .background).async { - for localFile in databaseLocalFileMetadatas { - let localFileOcId = localFile.ocId + materialisedMetadatas.forEach { + materialisedMetadatasMap[$0.ocId] = $0 + unmaterialisedIds.insert($0.ocId) + } - guard itemIds.contains(localFileOcId) else { - noLongerMaterialisedIds.insert(localFileOcId) + for enumeratedId in itemIds { + if unmaterialisedIds.contains(enumeratedId) { + unmaterialisedIds.remove(enumeratedId) + } else { + newMaterialisedIds.insert(enumeratedId) + guard var metadata = dbManager.itemMetadata(ocId: enumeratedId) else { + Self.logger.error("No metadata for \(enumeratedId, privacy: .public) found") continue } - } - - DispatchQueue.main.async { - Self.logger.info("Cleaning up local file metadatas for unmaterialised items") - for itemId in noLongerMaterialisedIds { - guard var itemMetadata = dbManager.itemMetadata(ocId: itemId) else { continue } - itemMetadata.downloaded = false - dbManager.addItemMetadata(itemMetadata) + if metadata.directory { + metadata.visitedDirectory = true + } else { + metadata.downloaded = true } + dbManager.addItemMetadata(metadata) + } + } - completionHandler(noLongerMaterialisedIds) + for unmaterialisedId in unmaterialisedIds { + guard var metadata = materialisedMetadatasMap[unmaterialisedId] else { + Self.logger.error("No materialised for \(unmaterialisedId, privacy: .public) found") + continue } + metadata.downloaded = false + metadata.visitedDirectory = false + dbManager.addItemMetadata(metadata) } + + // TODO: Do we need to signal the working set now? Unclear + + completionHandler(newMaterialisedIds, unmaterialisedIds) } } diff --git a/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift b/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift index d6aed563..fee1947b 100644 --- a/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift +++ b/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift @@ -31,7 +31,7 @@ final class MaterialisedEnumerationObserverTests: XCTestCase { let expect = XCTestExpectation(description: "Enumerator") let observer = MaterialisedEnumerationObserver( ncKitAccount: Self.account.ncKitAccount, dbManager: dbManager - ) { deletedOcIds in + ) { _, deletedOcIds in XCTAssertTrue(deletedOcIds.isEmpty) expect.fulfill() } @@ -63,7 +63,7 @@ final class MaterialisedEnumerationObserverTests: XCTestCase { let expect = XCTestExpectation(description: "Enumerator") let observer = MaterialisedEnumerationObserver( ncKitAccount: Self.account.ncKitAccount, dbManager: dbManager - ) { deletedOcIds in + ) { _, deletedOcIds in // itemA is downloaded, itemB is not, itemC is (and is new). Only the local copy of // itemA should come up as deleted XCTAssertEqual(deletedOcIds.count, 1) // Item B deleted From 1eef8982ca4b62c0c470af728daac1bcf863e5d3 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 11:27:56 +0800 Subject: [PATCH 05/38] Fix visitedDirectory state in conversions to RealmItemMetadata Signed-off-by: Claudio Cambra --- .../NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift b/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift index 7945f129..748d0334 100644 --- a/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift +++ b/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift @@ -160,6 +160,7 @@ internal class RealmItemMetadata: Object, ItemMetadata { self.downloaded = value.downloaded self.uploaded = value.uploaded self.keepDownloaded = value.keepDownloaded + self.visitedDirectory = value.visitedDirectory self.trashbinFileName = value.trashbinFileName self.trashbinOriginalLocation = value.trashbinOriginalLocation self.trashbinDeletionTime = value.trashbinDeletionTime From 4af5e9eb12df8bbd9e01ee669bf6ce5e77540157 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 11:28:09 +0800 Subject: [PATCH 06/38] Check corrected state in FilesDatabaseManager Signed-off-by: Claudio Cambra --- .../FilesDatabaseManagerTests.swift | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift b/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift index 78eb597f..8f13d343 100644 --- a/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift +++ b/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift @@ -1141,12 +1141,34 @@ final class FilesDatabaseManagerTests: XCTestCase { realm.add(folderC) } + // Test with addItemMetadata too + var sItemA = SendableItemMetadata(ocId: "sItemA", fileName: "sItemA", account: Self.account) + sItemA.downloaded = true + + var sItemB = SendableItemMetadata(ocId: "sItemB", fileName: "sItemB", account: Self.account) + sItemB.downloaded = false + + var sItemC = SendableItemMetadata(ocId: "sItemC", fileName: "sItemC", account: Self.account) + sItemC.downloaded = true + + var sDirD = SendableItemMetadata(ocId: "sDirD", fileName: "sDirD", account: Self.account) + sDirD.directory = true + sDirD.visitedDirectory = true + + Self.dbManager.addItemMetadata(sItemA) + Self.dbManager.addItemMetadata(sItemB) + Self.dbManager.addItemMetadata(sItemC) + Self.dbManager.addItemMetadata(sDirD) + let materialised = Self.dbManager.materialisedItemMetadatas(account: Self.account.ncKitAccount) - XCTAssertEqual(materialised.count, 2) + XCTAssertEqual(materialised.count, 5) let materialisedOcIds = materialised.map(\.ocId) XCTAssertTrue(materialisedOcIds.contains(itemA.ocId)) XCTAssertTrue(materialisedOcIds.contains(folderA.ocId)) + XCTAssertTrue(materialisedOcIds.contains(sItemA.ocId)) + XCTAssertTrue(materialisedOcIds.contains(sItemC.ocId)) + XCTAssertTrue(materialisedOcIds.contains(sDirD.ocId)) } } From 0c2b5c52d7edc917c468a1e22625ecff1e0c6153 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 11:28:27 +0800 Subject: [PATCH 07/38] Add logging to MaterialisedEnumerationObserver Signed-off-by: Claudio Cambra --- .../MaterialisedEnumerationObserver.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift b/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift index d61be780..4d8ad03e 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/MaterialisedEnumerationObserver.swift @@ -97,6 +97,13 @@ public class MaterialisedEnumerationObserver: NSObject, NSFileProviderEnumeratio } else { metadata.downloaded = true } + Self.logger.info( + """ + Updating materialisation state for item to MATERIALISED + with id \(enumeratedId, privacy: .public) + with filename \(metadata.fileName, privacy: .public) + """ + ) dbManager.addItemMetadata(metadata) } } @@ -106,6 +113,13 @@ public class MaterialisedEnumerationObserver: NSObject, NSFileProviderEnumeratio Self.logger.error("No materialised for \(unmaterialisedId, privacy: .public) found") continue } + Self.logger.info( + """ + Updating materialisation state for item to UNMATERIALISED + with id \(unmaterialisedId, privacy: .public) + with filename \(metadata.fileName, privacy: .public) + """ + ) metadata.downloaded = false metadata.visitedDirectory = false dbManager.addItemMetadata(metadata) From 1399ff94dc7624eb903e0e0cd0f0b7056f31162c Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 11:32:09 +0800 Subject: [PATCH 08/38] Improve tests for MaterialisedEnumerationObserver Signed-off-by: Claudio Cambra --- ...MaterialisedEnumerationObserverTests.swift | 109 +++++++++++++++--- 1 file changed, 94 insertions(+), 15 deletions(-) diff --git a/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift b/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift index fee1947b..0bc1008d 100644 --- a/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift +++ b/Tests/NextcloudFileProviderKitTests/MaterialisedEnumerationObserverTests.swift @@ -23,34 +23,80 @@ final class MaterialisedEnumerationObserverTests: XCTestCase { Realm.Configuration.defaultConfiguration.inMemoryIdentifier = name } - func testMaterialisedObserver() async { + func testMaterialisedObserverWithNoPreexistingState() async { let dbManager = FilesDatabaseManager( realmConfig: .defaultConfiguration, account: Self.account ) + // The database is intentionally left empty. + let remoteInterface = MockRemoteInterface() - let expect = XCTestExpectation(description: "Enumerator") + + let enumeratedFile = + SendableItemMetadata(ocId: "file1", fileName: "file1.txt", account: Self.account) + var enumeratedDir = + SendableItemMetadata(ocId: "dir1", fileName: "dir1", account: Self.account) + enumeratedDir.directory = true + + let expect = XCTestExpectation(description: "Enumerator completion handler called") + + // The observer's logic requires metadata to exist in the DB to update it. let observer = MaterialisedEnumerationObserver( ncKitAccount: Self.account.ncKitAccount, dbManager: dbManager - ) { _, deletedOcIds in - XCTAssertTrue(deletedOcIds.isEmpty) + ) { newlyMaterialisedIds, unmaterialisedIds in + XCTAssertTrue( + unmaterialisedIds.isEmpty, + "Unmaterialised set should be empty when DB starts empty." + ) + + // The items are correctly identified as newly materialised because they weren't in the + // DB's materialised list (which was empty). + XCTAssertEqual( + newlyMaterialisedIds.count, + 2, + "Both enumerated items should be identified as newly materialised." + ) + XCTAssertTrue(newlyMaterialisedIds.contains("file1")) + XCTAssertTrue(newlyMaterialisedIds.contains("dir1")) + + // Verify that the database state is NOT updated + let fileMetadata = dbManager.itemMetadata(ocId: "file1") + XCTAssertNil( + fileMetadata, + "Metadata should NOT be in the DB, as the observer does not add missing items." + ) + + let dirMetadata = dbManager.itemMetadata(ocId: "dir1") + XCTAssertNil( + dirMetadata, + "Metadata should NOT be in the DB, as the observer does not add missing items." + ) + expect.fulfill() } + let enumerator = MockEnumerator( account: Self.account, dbManager: dbManager, remoteInterface: remoteInterface ) + enumerator.enumeratorItems = [enumeratedFile, enumeratedDir] enumerator.enumerateItems(for: observer, startingAt: NSFileProviderPage(Data(count: 1))) + await fulfillment(of: [expect], timeout: 1) } - func testMaterialisedFiles() async { + func testMaterialisedObserverWithMixedState() async { + // Setup a DB with a mix of materialised and non-materialised items. var itemA = SendableItemMetadata(ocId: "itemA", fileName: "itemA", account: Self.account) - itemA.downloaded = true + itemA.downloaded = true // Was materialised var itemB = SendableItemMetadata(ocId: "itemB", fileName: "itemB", account: Self.account) - itemB.downloaded = false + itemB.downloaded = false // Was NOT materialised var itemC = SendableItemMetadata(ocId: "itemC", fileName: "itemC", account: Self.account) - itemC.downloaded = true + itemC.downloaded = true // Was materialised + + var dirD = SendableItemMetadata(ocId: "dirD", fileName: "dirD", account: Self.account) + dirD.directory = true + dirD.visitedDirectory = true // Was materialised let dbManager = FilesDatabaseManager( realmConfig: .defaultConfiguration, account: Self.account @@ -58,23 +104,56 @@ final class MaterialisedEnumerationObserverTests: XCTestCase { dbManager.addItemMetadata(itemA) dbManager.addItemMetadata(itemB) dbManager.addItemMetadata(itemC) + dbManager.addItemMetadata(dirD) let remoteInterface = MockRemoteInterface() - let expect = XCTestExpectation(description: "Enumerator") + let expect = XCTestExpectation(description: "Enumerator completion handler called") + let enumeratorItemsToReturn = [itemB, itemC] + let observer = MaterialisedEnumerationObserver( ncKitAccount: Self.account.ncKitAccount, dbManager: dbManager - ) { _, deletedOcIds in - // itemA is downloaded, itemB is not, itemC is (and is new). Only the local copy of - // itemA should come up as deleted - XCTAssertEqual(deletedOcIds.count, 1) // Item B deleted - XCTAssertEqual(deletedOcIds.first, itemA.ocId) + ) { newlyMaterialisedIds, unmaterialisedIds in + // Unmaterialised: itemA and dirD were materialised but not in the latest enumeration. + XCTAssertEqual( + unmaterialisedIds.count, 2, "itemA and dirD should be reported as unmaterialised." + ) + XCTAssertTrue(unmaterialisedIds.contains("itemA")) + XCTAssertTrue(unmaterialisedIds.contains("dirD")) + + // Newly Materialised: itemB was NOT materialised but WAS in the latest enumeration. + XCTAssertEqual( + newlyMaterialisedIds.count, 1, "itemB should be reported as newly materialised." + ) + XCTAssertEqual(newlyMaterialisedIds.first, "itemB") + + // Check final database state + let finalItemA = dbManager.itemMetadata(ocId: "itemA") + XCTAssertFalse( + finalItemA?.downloaded ?? true, "itemA should now be marked as not downloaded." + ) + + let finalItemB = dbManager.itemMetadata(ocId: "itemB") + XCTAssertTrue( + finalItemB?.downloaded ?? false, "itemB should now be marked as downloaded." + ) + + let finalItemC = dbManager.itemMetadata(ocId: "itemC") + XCTAssertTrue(finalItemC?.downloaded ?? false, "itemC should remain downloaded.") + + let finalDirD = dbManager.itemMetadata(ocId: "dirD") + XCTAssertFalse( + finalDirD?.visitedDirectory ?? true, "dirD should now be marked as not visited." + ) + expect.fulfill() } + let enumerator = MockEnumerator( account: Self.account, dbManager: dbManager, remoteInterface: remoteInterface ) - enumerator.enumeratorItems = [itemB, itemC] + enumerator.enumeratorItems = enumeratorItemsToReturn enumerator.enumerateItems(for: observer, startingAt: NSFileProviderPage(Data(count: 1))) + await fulfillment(of: [expect], timeout: 1) } } From f00b98806db12b692966a7f506ad21dd3682e548 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 11:37:26 +0800 Subject: [PATCH 09/38] Log visitedDirectory state Signed-off-by: Claudio Cambra --- .../NextcloudFileProviderKit/Database/FilesDatabaseManager.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index d06e7be3..fb4a4563 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -463,6 +463,7 @@ public final class FilesDatabaseManager: Sendable { trashbinFileName: \(metadata.trashbinFileName, privacy: .public) downloaded: \(metadata.downloaded, privacy: .public) uploaded: \(metadata.uploaded, privacy: .public) + visitedDirectory: \(metadata.visitedDirectory, privacy: .public) """ ) } From 4ce0b0da35d471b1d1df8dadd93fa9751e165417 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:34:21 +0800 Subject: [PATCH 10/38] Modify readServerUrl to mark directories as visited where relevant AND TO RETURN TARGET ITEM Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator+SyncEngine.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift index 38d23afc..eb4bd3ad 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift @@ -279,10 +279,11 @@ extension Enumerator { !(firstFile.fileName == "." && firstFile.serverUrl == "..") { var metadata = firstFile.toItemMetadata() - if metadata.directory, - let existingMetadata = dbManager.itemMetadata(ocId: metadata.ocId) - { - metadata.downloaded = existingMetadata.downloaded + if metadata.directory { + metadata.visitedDirectory = true + if let existingMetadata = dbManager.itemMetadata(ocId: metadata.ocId) { + metadata.downloaded = existingMetadata.downloaded + } } dbManager.addItemMetadata(metadata) } @@ -333,9 +334,11 @@ extension Enumerator { if let existingMetadata = dbManager.itemMetadata(ocId: directoryMetadata.ocId) { directoryMetadata.downloaded = existingMetadata.downloaded } - dbManager.addItemMetadata(directoryMetadata) + directoryMetadata.visitedDirectory = true } + metadatas.insert(directoryMetadata, at: 0) + let changedMetadatas = dbManager.depth1ReadUpdateItemMetadatas( account: account.ncKitAccount, serverUrl: serverUrl, @@ -358,7 +361,7 @@ extension Enumerator { // Paginated reads is used by enumerateItems, non-paginated reads is used by enumerateChanges. // // Paginated reads WILL NOT HANDLE REMOVAL OF REMOTELY DELETED ITEMS FROM THE LOCAL DATABASE. - // Paginated reads WILL ONLY REPORT THE FILES DISCOVERED LOCALLY. + // Paginated reads WILL ONLY REPORT THE FILES DISCOVERED REMOTELY. // This means that if you decide to use this method to implement change enumeration, you will // have to collect the full results of all the pages before proceeding with discovering what // has changed relative to the state of the local database -- manually! From 6220410cc56752446d06f999a7bb08c3bc39b27e Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:34:44 +0800 Subject: [PATCH 11/38] Amend Item fetch behaviour to account for new readServerUrl behaviour Signed-off-by: Claudio Cambra --- Sources/NextcloudFileProviderKit/Item/Item+Fetch.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Fetch.swift b/Sources/NextcloudFileProviderKit/Item/Item+Fetch.swift index 46131901..9ec1b6e5 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Fetch.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Fetch.swift @@ -50,17 +50,20 @@ public extension Item { ) ?? NSFileProviderError(.cannotSynchronize) } - guard let metadatas else { + guard var metadatas else { Self.logger.error( """ Could not fetch directory contents for - \(self.metadata.fileName, privacy: .public) - at \(remoteDirectoryPath, privacy: .public), received nil metadatas + \(self.metadata.fileName, privacy: .public) + at \(remoteDirectoryPath, privacy: .public), received nil metadatas """ ) throw NSFileProviderError(.cannotSynchronize) } + if !metadatas.isEmpty { + metadatas.removeFirst() // Remove the dir itself + } progress.totalUnitCount += Int64(metadatas.count) for var metadata in metadatas { From 996d8f08e6029d5b92239eef3784461ef96ee717 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:35:19 +0800 Subject: [PATCH 12/38] Amend Item pakcage/bundle modify to account for new readServerUrl behaviour Signed-off-by: Claudio Cambra --- .../NextcloudFileProviderKit/Item/Item+Modify.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift b/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift index f8309848..6a7dd90f 100644 --- a/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift +++ b/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift @@ -243,8 +243,8 @@ public extension Item { Self.logger.error( """ Could not modify bundle or package contents as was provided nil contents url - for item with ocID \(self.itemIdentifier.rawValue, privacy: .public) - (\(self.filename, privacy: .public)) + for item with ocID \(self.itemIdentifier.rawValue, privacy: .public) + (\(self.filename, privacy: .public)) """ ) throw NSFileProviderError(.cannotSynchronize) @@ -253,7 +253,7 @@ public extension Item { Self.logger.debug( """ Handling modified bundle/package/internal directory at: - \(contents.path, privacy: .public) + \(contents.path, privacy: .public) """ ) @@ -291,7 +291,7 @@ public extension Item { ) throw remoteErrorToThrow(readError) } - guard let metadatas else { + guard var metadatas else { Self.logger.error( """ Could not read server url for item with ocID @@ -303,6 +303,9 @@ public extension Item { throw NSFileProviderError(.serverUnreachable) } + if !metadatas.isEmpty { + metadatas.removeFirst() // Remove bundle itself + } allMetadatas.append(contentsOf: metadatas) var childDirPaths = [String]() From 46549ce0af9821cd4b8428b34e4cff8e5999abc2 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:38:40 +0800 Subject: [PATCH 13/38] Modify database manager handling of depth 1 read and account for readServerUrl changes Signed-off-by: Claudio Cambra --- .../Database/FilesDatabaseManager.swift | 46 +++++++++++++-- .../FilesDatabaseManagerTests.swift | 58 ++++++++++++++----- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index fb4a4563..76a0106e 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -336,24 +336,42 @@ public final class FilesDatabaseManager: Sendable { // // - the ones that do exist remotely still are either the same or have been updated // - the ones that don't have been deleted + var cleanServerUrl = serverUrl + if cleanServerUrl.last == "/" { + cleanServerUrl.removeLast() + } let existingMetadatas = database .objects(RealmItemMetadata.self) - .where { $0.account == account && $0.serverUrl == serverUrl && $0.uploaded } + .where { $0.account == account && $0.serverUrl == cleanServerUrl && $0.uploaded } + + var updatedChildMetadatas = updatedMetadatas + + let readTargetMetadata: SendableItemMetadata? + if let targetMetadata = updatedMetadatas.first { + if targetMetadata.directory { + readTargetMetadata = updatedChildMetadatas.removeFirst() + } else { + readTargetMetadata = targetMetadata + } + } else { + readTargetMetadata = nil + } // NOTE: These metadatas are managed -- be careful! let metadatasToDelete = processItemMetadatasToDelete( existingMetadatas: existingMetadatas, - updatedMetadatas: updatedMetadatas) + updatedMetadatas: updatedChildMetadatas + ) let metadatasToDeleteCopy = metadatasToDelete.map { SendableItemMetadata(value: $0) } let metadatasToChange = processItemMetadatasToUpdate( existingMetadatas: existingMetadatas, - updatedMetadatas: updatedMetadatas, + updatedMetadatas: updatedChildMetadatas, keepExistingDownloadState: keepExistingDownloadState ) var metadatasToUpdate = metadatasToChange.updatedMetadatas - let metadatasToCreate = metadatasToChange.newMetadatas + var metadatasToCreate = metadatasToChange.newMetadatas let directoriesNeedingRename = metadatasToChange.directoriesNeedingRename for metadata in directoriesNeedingRename { @@ -366,6 +384,26 @@ public final class FilesDatabaseManager: Sendable { } } + if var readTargetMetadata { + if let existing = itemMetadata(ocId: readTargetMetadata.ocId) { + if existing.status == Status.normal.rawValue, + !existing.isInSameDatabaseStoreableRemoteState(readTargetMetadata) + { + Self.logger.info("Depth 1 read target changed: \(readTargetMetadata.ocId)") + if keepExistingDownloadState { + readTargetMetadata.downloaded = existing.downloaded + } + if readTargetMetadata.directory { + readTargetMetadata.visitedDirectory = true + } + metadatasToUpdate.insert(readTargetMetadata, at: 0) + } + } else { + Self.logger.info("Depth 1 read target is new: \(readTargetMetadata.ocId)") + metadatasToCreate.insert(readTargetMetadata, at: 0) + } + } + try database.write { database.delete(metadatasToDelete) database.add(metadatasToUpdate.map { RealmItemMetadata(value: $0) }, update: .modified) diff --git a/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift b/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift index 8f13d343..012ef09b 100644 --- a/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift +++ b/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift @@ -118,6 +118,10 @@ final class FilesDatabaseManagerTests: XCTestCase { func testUpdateRenamesDirectoryAndPropagatesToChildren() throws { let account = Account(user: "test", id: "t", serverUrl: "https://example.com", password: "") + var rootMetadata = SendableItemMetadata(ocId: "root", fileName: "", account: Self.account) + rootMetadata.directory = true + Self.dbManager.addItemMetadata(rootMetadata) + // Insert original parent directory var parent = SendableItemMetadata(ocId: "dir1", fileName: "oldDir", account: account) parent.directory = true @@ -148,7 +152,7 @@ final class FilesDatabaseManagerTests: XCTestCase { let result = Self.dbManager.depth1ReadUpdateItemMetadatas( account: account.ncKitAccount, serverUrl: account.davFilesUrl, - updatedMetadatas: [renamedParent], + updatedMetadatas: [rootMetadata, renamedParent], keepExistingDownloadState: true ) @@ -353,50 +357,64 @@ final class FilesDatabaseManagerTests: XCTestCase { user: "TestAccount", id: "taid", serverUrl: "https://example.com", password: "pass" ) + let parent = RealmItemMetadata() + parent.ocId = "parent" + parent.fileName = "Parent" + parent.account = "TestAccount" + parent.serverUrl = "https://example.com" + parent.directory = true + parent.downloaded = true + parent.uploaded = true + // Simulate existing metadata in the database let existingMetadata = RealmItemMetadata() existingMetadata.ocId = "id-1" existingMetadata.fileName = "File.pdf" existingMetadata.account = "TestAccount" - existingMetadata.serverUrl = "https://example.com" + existingMetadata.serverUrl = "https://example.com/Parent" existingMetadata.downloaded = true existingMetadata.uploaded = true // Simulate updated metadata that includes changes and a new entry - let updatedMetadata = + var updatedParent = SendableItemMetadata(ocId: "parent", fileName: "Parent", account: account) + updatedParent.directory = true + + var updatedMetadata = SendableItemMetadata(ocId: "id-1", fileName: "UpdatedFile.pdf", account: account) + updatedMetadata.serverUrl = "https://example.com/Parent" - let newMetadata = + var newMetadata = SendableItemMetadata(ocId: "id-2", fileName: "NewFile.pdf", account: account) + newMetadata.serverUrl = "https://example.com/Parent" let realm = Self.dbManager.ncDatabase() try realm.write { + realm.add(parent) realm.add(existingMetadata) } let results = Self.dbManager.depth1ReadUpdateItemMetadatas( account: "TestAccount", - serverUrl: "https://example.com", - updatedMetadatas: [updatedMetadata, newMetadata], + serverUrl: "https://example.com/Parent", + updatedMetadatas: [updatedParent, updatedMetadata, newMetadata], keepExistingDownloadState: true ) XCTAssertEqual(results.newMetadatas?.count, 1, "Should create one new metadata") - XCTAssertEqual(results.updatedMetadatas?.count, 1, "Should update one existing metadata") + XCTAssertEqual(results.updatedMetadatas?.count, 2, "Should update two existing metadata") XCTAssertEqual( results.newMetadatas?.first?.ocId, "id-2", "New metadata ocId should be 'id-2'" ) XCTAssertEqual( - results.updatedMetadatas?.first?.fileName, + results.updatedMetadatas?.last?.fileName, "UpdatedFile.pdf", "Updated metadata should have the new file name" ) } func testUnuploadedItemsAreNotDeletedDuringUpdate() throws { - let testAccount = "TestAccount" - let testServerUrl = "https://example.com/files/" - + let testAccount = Self.account.ncKitAccount + let testServerUrl = Self.account.davFilesUrl // 1. Item that exists locally and is marked as uploaded let uploadedItem = RealmItemMetadata() uploadedItem.ocId = "ocid-uploaded-123" @@ -416,19 +434,25 @@ final class FilesDatabaseManagerTests: XCTestCase { unuploadedItem.uploaded = false // IMPORTANT: Not marked as uploaded unuploadedItem.status = Status.normal.rawValue // Ensure it's not in a transient state if relevant + var rootMetadata = SendableItemMetadata(ocId: "root", fileName: "", account: Self.account) + rootMetadata.directory = true + + Self.dbManager.addItemMetadata(rootMetadata) + let realm = Self.dbManager.ncDatabase() try realm.write { realm.add(uploadedItem) realm.add(unuploadedItem) } - // Verify initial state (optional but good practice) - XCTAssertEqual(realm.objects(RealmItemMetadata.self).where { $0.account == testAccount && $0.serverUrl == testServerUrl }.count, 2) + XCTAssertEqual(realm.objects(RealmItemMetadata.self).where { + $0.account == testAccount && $0.serverUrl == testServerUrl + }.count, 3) // Simulate an update from the server that contains NEITHER of these items. // This means the server thinks 'SyncedFile.txt' was deleted, // and it doesn't know about 'NewLocalFile.txt' yet. - let updatedMetadatasFromServer: [SendableItemMetadata] = [] + let updatedMetadatasFromServer = [rootMetadata] let results = Self.dbManager.depth1ReadUpdateItemMetadatas( account: testAccount, @@ -448,9 +472,11 @@ final class FilesDatabaseManagerTests: XCTestCase { // Check the actual database state after the write transaction - XCTAssertEqual(remainingMetadatas.count, 1, "Only one item should remain in the database.") + XCTAssertEqual( // The root item and the remaining item + remainingMetadatas.count, 2, "Only two items should remain in the database." + ) - let survivingItem = remainingMetadatas.first + let survivingItem = remainingMetadatas.last XCTAssertNotNil(survivingItem, "An item should survive.") XCTAssertEqual(survivingItem?.ocId, "ocid-local-456", "The surviving item should be the unuploaded one.") XCTAssertEqual(survivingItem?.fileName, "NewLocalFile.txt", "Filename should match the unuploaded item.") From 6ad99abbf500ef83d03ba4386d50ec6b2596b16d Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:39:10 +0800 Subject: [PATCH 14/38] Make MockRemoteItem versionIdentifier modifiable Signed-off-by: Claudio Cambra --- Tests/Interface/MockRemoteItem.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Interface/MockRemoteItem.swift b/Tests/Interface/MockRemoteItem.swift index 4f033d0f..7ae5aadc 100644 --- a/Tests/Interface/MockRemoteItem.swift +++ b/Tests/Interface/MockRemoteItem.swift @@ -18,7 +18,7 @@ public class MockRemoteItem: Equatable { public var children: [MockRemoteItem] = [] public var identifier: String - public let versionIdentifier: String + public var versionIdentifier: String public var name: String public var remotePath: String public let directory: Bool From d7a98c7f7430a08142ae0300513f02d63a492ceb Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:39:37 +0800 Subject: [PATCH 15/38] Return 404 when enumerating non-existent item URL in MockRemoteInterface Signed-off-by: Claudio Cambra --- Tests/Interface/MockRemoteInterface.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Tests/Interface/MockRemoteInterface.swift b/Tests/Interface/MockRemoteInterface.swift index f7a8a6c4..388c84ef 100644 --- a/Tests/Interface/MockRemoteInterface.swift +++ b/Tests/Interface/MockRemoteInterface.swift @@ -1036,7 +1036,12 @@ public class MockRemoteInterface: RemoteInterface { print("Enumerating \(remotePath)") guard let item = item(remotePath: remotePath, account: account) else { print("Item at \(remotePath) not found.") - return (account.ncKitAccount, [], nil, .urlError) + return ( + account.ncKitAccount, + [], + nil, + NKError(statusCode: 404, fallbackDescription: "File not found") + ) } func generateResponse(itemCount: Int, finalPage: Bool) -> AFDataResponse? { From 37cdea9b1a91ade286909e20c39a96753ee5904b Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:43:02 +0800 Subject: [PATCH 16/38] Modify working set enumeration to only return materialised items Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator.swift | 57 +-- .../EnumeratorTests.swift | 476 +++--------------- 2 files changed, 70 insertions(+), 463 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index 30901dad..5d71ac4b 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -193,6 +193,11 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { if enumeratedItemIdentifier == .workingSet { Self.logger.info("Upcoming enumeration is of working set.") + let ncKitAccount = account.ncKitAccount + // Visited folders and downloaded files + let materialisedItems = dbManager.materialisedItemMetadatas(account: ncKitAccount) + completeEnumerationObserver(observer, nextPage: nil, itemMetadatas: materialisedItems) + return } guard serverUrl != "" else { @@ -221,7 +226,6 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { var providedPage: NSFileProviderPage? = nil // Used for pagination token sent to server // Do not pass in the NSFileProviderPage default pages, these are not valid Nextcloud // pagination tokens - var nextServerUrls: [String]? = nil var pageIndex = 0 var pageTotal: Int? = nil if page != NSFileProviderPage.initialPageSortedByName as NSFileProviderPage && @@ -230,12 +234,6 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { if let enumPageResponse = try? JSONDecoder().decode(EnumeratorPageResponse.self, from: page.rawValue) { - if let actualServerUrl = enumPageResponse.nextServerUrl { - Self.logger.info( - "Setting enumerator server URL to \(actualServerUrl, privacy: .public)" - ) - serverUrl = actualServerUrl - } if let token = enumPageResponse.token?.data(using: .utf8) { providedPage = NSFileProviderPage(token) } @@ -243,7 +241,6 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { pageTotal = total } pageIndex = enumPageResponse.index - nextServerUrls = enumPageResponse.serverUrlQueue } else { Self.logger.error("Could not parse page") } @@ -293,45 +290,11 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { } pageTotal = nextPage?.total ?? pageTotal - - if enumeratedItemIdentifier == .workingSet { - var serverUrlQueue = nextServerUrls ?? [] - for metadata in metadatas where metadata.directory { - let metadataRemoteUrl = metadata.serverUrl + "/" + metadata.fileName - serverUrlQueue.append(metadataRemoteUrl) - } - let nextPageValid = nextPage != nil - let nextPageToken = nextPage?.token ?? "" // For logging - Self.logger.debug( - """ - Current folders awaiting paged enumeration: - \(serverUrlQueue, privacy: .public) - next page is valid: \(nextPageValid, privacy: .public) - next page token: \(nextPageToken, privacy: .public) - page total: \(pageTotal ?? -1, privacy: .public) - """ - ) - - if let rPage = nextPage, let pageTotal, rPage.index * pageItemCount >= pageTotal { - // Server will sometimes provide a valid next page data even though there are no - // items to enumerate anymore - Self.logger.debug("No more items to enumerate, stopping paged enumeration") - nextPage = nil - } - - if nextPage != nil { - Self.logger.debug("Applying actual server url onto server page response") - nextPage!.total = pageTotal - nextPage!.serverUrlQueue = serverUrlQueue - nextPage!.nextServerUrl = serverUrl - } else if !serverUrlQueue.isEmpty { - // If we have finished paged enumeration of the current serverUrl, move to next - // child to scan - let nextServerUrl = serverUrlQueue.removeFirst() - nextPage = EnumeratorPageResponse( - nextServerUrl: nextServerUrl, serverUrlQueue: serverUrlQueue - ) - } + if let rPage = nextPage, let pageTotal, rPage.index * pageItemCount >= pageTotal { + // Server will sometimes provide a valid next page data even though there are no + // items to enumerate anymore + Self.logger.debug("No more items to enumerate, stopping paged enumeration") + nextPage = nil } Self.logger.info( diff --git a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift index dbf6fd61..a9cb2d48 100644 --- a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift +++ b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift @@ -8,8 +8,8 @@ import FileProvider import NextcloudKit import RealmSwift -import TestInterface import XCTest +@testable import TestInterface @testable import NextcloudFileProviderKit final class EnumeratorTests: XCTestCase { @@ -183,454 +183,98 @@ final class EnumeratorTests: XCTestCase { } func testWorkingSetEnumeration() async throws { - let db = Self.dbManager.ncDatabase() // Strong ref for in memory test db + // This test verifies that the working set enumerator correctly returns ONLY the + // items that are "materialised" (downloaded files or visited directories) from the database + let db = Self.dbManager.ncDatabase() // Init DB debugPrint(db) - let remoteInterface = MockRemoteInterface(rootItem: rootItem) - let enumerator = Enumerator( - enumeratedItemIdentifier: .workingSet, - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager - ) - let observer = MockEnumerationObserver(enumerator: enumerator) - try await observer.enumerateItems() - XCTAssertEqual(observer.items.count, 3) + // Item 1: A downloaded file (should be in working set) + var downloadedFile = remoteItemA.toItemMetadata(account: Self.account) + downloadedFile.downloaded = true + Self.dbManager.addItemMetadata(downloadedFile) - let retrievedFolderItem = try XCTUnwrap(observer.items.first) - XCTAssertEqual(retrievedFolderItem.itemIdentifier.rawValue, remoteFolder.identifier) - XCTAssertEqual(retrievedFolderItem.filename, remoteFolder.name) - XCTAssertEqual(retrievedFolderItem.parentItemIdentifier.rawValue, rootItem.identifier) - XCTAssertEqual(retrievedFolderItem.creationDate, remoteFolder.creationDate) - XCTAssertEqual( - Int(retrievedFolderItem.contentModificationDate??.timeIntervalSince1970 ?? 0), - Int(remoteFolder.modificationDate.timeIntervalSince1970) - ) - XCTAssertEqual(retrievedFolderItem.isUploaded, true) + // Item 2: A visited directory (should be in working set) + var visitedDir = remoteFolder.toItemMetadata(account: Self.account) + visitedDir.visitedDirectory = true + Self.dbManager.addItemMetadata(visitedDir) - // Ensure the newly discovered folder has no etag - let dbFolder = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: remoteFolder.identifier)) - XCTAssertEqual(dbFolder.etag, remoteFolder.versionIdentifier) - } + // Item 3: A file that is in the DB but not downloaded (should NOT be in working set) + var notDownloadedFile = remoteItemB.toItemMetadata(account: Self.account) + notDownloadedFile.downloaded = false + Self.dbManager.addItemMetadata(notDownloadedFile) - func testWorkingSetPaginatedEnumeration() async throws { - // 1. Setup a flat file structure with more items than the page size. - rootItem.children = [] // Clear existing children - for i in 0..<15 { - let childItem = MockRemoteItem( - identifier: "item\(i)", - name: "item\(i).txt", - remotePath: Self.account.davFilesUrl + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = rootItem - rootItem.children.append(childItem) - } + // Item 4: A directory that is in the DB but not visited (should NOT be in working set) + var notVisitedDir = remoteItemC.toItemMetadata(account: Self.account) + notVisitedDir.directory = true + notVisitedDir.visitedDirectory = false + Self.dbManager.addItemMetadata(notVisitedDir) - let db = Self.dbManager.ncDatabase() // Strong ref for in-memory test db - debugPrint(db) - let remoteInterface = MockRemoteInterface(rootItem: rootItem, pagination: true) - - // 2. Create the enumerator with a specific page size. let enumerator = Enumerator( enumeratedItemIdentifier: .workingSet, account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - pageSize: 5 + remoteInterface: MockRemoteInterface(), + dbManager: Self.dbManager ) let observer = MockEnumerationObserver(enumerator: enumerator) - // 3. Enumerate the items. + // 2. Act try await observer.enumerateItems() - // 4. Assert the results. - XCTAssertEqual(observer.items.count, 15) - - for item in observer.items { - XCTAssertNotNil(Self.dbManager.itemMetadata(ocId: item.itemIdentifier.rawValue)) - } + // 3. Assert + XCTAssertNil(observer.error, "Enumeration should complete without error.") + XCTAssertEqual(observer.items.count, 2, "Should only enumerate the 2 materialised items.") - // The enumeration flow: - // - Initial call enumerates the root. - // - The enumerator then gets the first page of children. - // - The observer requests the next pages until all items are fetched. - // With 15 children and a page size of 5, we expect 3 pages of children. - // Total pages observed: 1 (initial) + 3 (for children) = 4 - XCTAssertEqual(observer.observedPages.count, 4) - XCTAssertEqual( - observer.observedPages.first, - NSFileProviderPage.initialPageSortedByName as NSFileProviderPage + let enumeratedIds = Set(observer.items.map(\.itemIdentifier.rawValue)) + XCTAssertTrue( + enumeratedIds.contains(downloadedFile.ocId), + "The downloaded file should be in the working set." ) - } - - func testWorkingSetPaginatedEnumerationWithMultipleDirectories() async throws { - // 1. Setup a more complex, nested directory structure. - // root - // |- folder1 (7 items) - // |- folder2 - // |- subfolder (3 items) - // |- (8 items) - - rootItem.children = [] // Clear existing children - - let folder1 = MockRemoteItem( - identifier: "folder1", - name: "folder1", - remotePath: Self.account.davFilesUrl + "/folder1", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl + XCTAssertTrue( + enumeratedIds.contains(visitedDir.ocId), + "The visited directory should be in the working set." ) - folder1.parent = rootItem - rootItem.children.append(folder1) - for i in 0..<7 { - let childItem = MockRemoteItem( - identifier: "folder1-item\(i)", - name: "item\(i).txt", - remotePath: folder1.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = folder1 - folder1.children.append(childItem) - } - - let folder2 = MockRemoteItem( - identifier: "folder2", - name: "folder2", - remotePath: Self.account.davFilesUrl + "/folder2", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl + XCTAssertFalse( + enumeratedIds.contains(notDownloadedFile.ocId), + "The non-downloaded file should NOT be in the working set." ) - folder2.parent = rootItem - rootItem.children.append(folder2) - - for i in 0..<8 { - let childItem = MockRemoteItem( - identifier: "folder2-item\(i)", - name: "item\(i).txt", - remotePath: folder2.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = folder2 - folder2.children.append(childItem) - } - - let subfolder = MockRemoteItem( - identifier: "subfolder", - name: "subfolder", - remotePath: folder2.remotePath + "/subfolder", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl + XCTAssertFalse( + enumeratedIds.contains(notVisitedDir.ocId), + "The non-visited directory should NOT be in the working set." ) - subfolder.parent = folder2 - folder2.children.append(subfolder) + } - for i in 0..<3 { - let childItem = MockRemoteItem( - identifier: "subfolder-item\(i)", - name: "item\(i).txt", - remotePath: subfolder.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = subfolder - subfolder.children.append(childItem) - } + func testWorkingSetEnumerationWhenNoMaterialisedItems() async throws { + // This test verifies that the enumerator behaves correctly when there are + // no materialised items in the database. + // 1. Arrange let db = Self.dbManager.ncDatabase() debugPrint(db) - let remoteInterface = MockRemoteInterface(rootItem: rootItem, pagination: true) - // 2. Create the enumerator. + // Add items to the DB, but none are materialised. + var notDownloadedFile = remoteItemA.toItemMetadata(account: Self.account) + notDownloadedFile.downloaded = false + Self.dbManager.addItemMetadata(notDownloadedFile) + + var notVisitedDir = remoteFolder.toItemMetadata(account: Self.account) + notVisitedDir.visitedDirectory = false + Self.dbManager.addItemMetadata(notVisitedDir) + let enumerator = Enumerator( enumeratedItemIdentifier: .workingSet, account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - pageSize: 5 + remoteInterface: MockRemoteInterface(), + dbManager: Self.dbManager ) let observer = MockEnumerationObserver(enumerator: enumerator) - // 3. Enumerate the items. + // 2. Act try await observer.enumerateItems() - // 4. Assert the results. - // Total items = - // 0 (root) + - // 1 (folder1) + - // 7 (items in folder1) + - // 1 (folder2) + - // 8 (items in folder2) + - // 1 (subfolder) + - // 3 (items in subfolder) - // = 22 - let expectedTotalItems = 0 + 1 + 7 + 1 + 8 + 1 + 3 - print(observer.items.map(\.itemIdentifier.rawValue)) - XCTAssertEqual(observer.items.count, expectedTotalItems) - - for item in observer.items { - XCTAssertNotNil(Self.dbManager.itemMetadata(ocId: item.itemIdentifier.rawValue)) - } - - // The working set enumeration is recursive. The enumerator will first list the items - // in the root directory, then it will be called again with the URLs of the subdirectories - // as pages. This process continues until all directories have been enumerated. - // We can verify that all items are discovered and stored in the database. - let allItemIds = rootItem.children.map(\.identifier) - + folder1.children.map(\.identifier) - + folder2.children.map(\.identifier) - + subfolder.children.map(\.identifier) - - for itemId in allItemIds { - XCTAssertNotNil( - Self.dbManager.itemMetadata(ocId: itemId), - "Item with id \(itemId) should be in the database" - ) - } - // Root should not be stored in database - XCTAssertNil(Self.dbManager.itemMetadata(ocId: rootItem.identifier)) - } - - func testWorkingSetPaginatedEnumerationWithEnumeratorRecreation() async throws { - // This test simulates the system creating a new enumerator for each subsequent page of a - // paginated request - - // 1. Setup a more complex, nested directory structure. - rootItem.children = [] // Clear existing children - - let folder1 = MockRemoteItem( - identifier: "folder1", - name: "folder1", - remotePath: Self.account.davFilesUrl + "/folder1", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - folder1.parent = rootItem - rootItem.children.append(folder1) - for i in 0..<7 { - let childItem = MockRemoteItem( - identifier: "folder1-item\(i)", - name: "item\(i).txt", - remotePath: folder1.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = folder1 - folder1.children.append(childItem) - } - - let folder2 = MockRemoteItem( - identifier: "folder2", - name: "folder2", - remotePath: Self.account.davFilesUrl + "/folder2", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - folder2.parent = rootItem - rootItem.children.append(folder2) - - let subfolder = MockRemoteItem( - identifier: "subfolder", - name: "subfolder", - remotePath: folder2.remotePath + "/subfolder", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - subfolder.parent = folder2 - folder2.children.append(subfolder) - for i in 0..<3 { - let childItem = MockRemoteItem( - identifier: "subfolder-item\(i)", - name: "item\(i).txt", - remotePath: subfolder.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = subfolder - subfolder.children.append(childItem) - } - for i in 0..<8 { // Add loose items to folder2 - let childItem = MockRemoteItem( - identifier: "folder2-item\(i)", - name: "item\(i).txt", - remotePath: folder2.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = folder2 - folder2.children.append(childItem) - } - - let db = Self.dbManager.ncDatabase() - debugPrint(db) - let remoteInterface = MockRemoteInterface(rootItem: rootItem, pagination: true) - let pageSize = 5 - - // 2. Manually drive the pagination loop, creating a new enumerator each time. - var allEnumeratedItems: [NSFileProviderItem] = [] - var currentPage: NSFileProviderPage? = - NSFileProviderPage.initialPageSortedByName as NSFileProviderPage - - while let pageToEnumerate = currentPage { - currentPage = nil // Reset for the next iteration - - // Create a NEW enumerator and observer for this single page request - let enumerator = Enumerator( - enumeratedItemIdentifier: .workingSet, - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - pageSize: pageSize - ) - let observer = MockEnumerationObserver(enumerator: enumerator) - try await observer.enumerateItemsPage(page: pageToEnumerate) - XCTAssertNil(observer.error) - allEnumeratedItems += observer.items - currentPage = observer.page - } - - let expectedTotalItems = 1 + 7 + 1 + 8 + 1 + 3 // 21 items total - XCTAssertEqual( - allEnumeratedItems.count, - expectedTotalItems, - "The test failed, likely because the enumerator's state was lost and it did not recursively scan subdirectories." - ) - - let allItemIds = rootItem.children.map(\.identifier) - + folder1.children.map(\.identifier) - + folder2.children.map(\.identifier) - + subfolder.children.map(\.identifier) - - // Check if the database contains all items, which it likely won't if the test fails. - let itemsInDB = allItemIds.compactMap { Self.dbManager.itemMetadata(ocId: $0) } - XCTAssertEqual( - itemsInDB.count, - expectedTotalItems, - "The database should contain metadata for all discovered items." - ) - XCTAssertNil(Self.dbManager.itemMetadata(ocId: rootItem.identifier)) - } - - func testWorkingSetEnumerationStopsOnExhaustedTotal() async throws { - // This test verifies that enumeration correctly terminates when a faulty server - // provides a `nextPage` token but all items (according to `pageTotal`) have been received. - - // 1. Setup: A folder with 9 items which, when including itself in the propfind, is an exact - // multiple of the page size. - rootItem.children = [] - let folder = MockRemoteItem( - identifier: "folder10", - name: "folder10", - remotePath: Self.account.davFilesUrl + "/folder10", - directory: true, - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - folder.parent = rootItem - rootItem.children.append(folder) - - for i in 0..<9 { - let childItem = MockRemoteItem( - identifier: "item\(i)", - name: "item\(i).txt", - remotePath: folder.remotePath + "/item\(i).txt", - account: Self.account.ncKitAccount, - username: Self.account.username, - userId: Self.account.id, - serverUrl: Self.account.serverUrl - ) - childItem.parent = folder - folder.children.append(childItem) - } - - let db = Self.dbManager.ncDatabase() - debugPrint(db) - - let remoteInterface = MockRemoteInterface(rootItem: rootItem, pagination: true) - let pageSize = 5 - - // This test requires a modification to `MockRemoteInterface` to simulate a faulty server - remoteInterface.forceNextPageOnLastContentPage = true - - // 2. Manually drive the pagination loop. - var allEnumeratedItems: [NSFileProviderItem] = [] - var currentPage: NSFileProviderPage? = - NSFileProviderPage.initialPageSortedByName as NSFileProviderPage - var loopCount = 0 - - while let pageToEnumerate = currentPage { - loopCount += 1 - currentPage = nil // Reset for the next iteration - - // Create a NEW enumerator and observer for this single page request - let enumerator = Enumerator( - enumeratedItemIdentifier: .workingSet, - account: Self.account, - remoteInterface: remoteInterface, - dbManager: Self.dbManager, - pageSize: pageSize - ) - let observer = MockEnumerationObserver(enumerator: enumerator) - try await observer.enumerateItemsPage(page: pageToEnumerate) - XCTAssertNil(observer.error) - allEnumeratedItems += observer.items - currentPage = observer.page - } - - // 3. Assert the results. - // The loop should have run 3 times: - // 1. For root - // 2. For `folder10` page 1 (items 0-4). - // 3. For `folder10` page 2 (items 5-9). The enumerator logic should see that all items - // have been delivered and should not return the phantom page token, terminating the loop. - XCTAssertNil(currentPage, "Loop should have terminated with a nil next page.") - XCTAssertEqual(loopCount, 3, "The enumeration loop should have terminated correctly.") - - // Total items expected: 1 folder + 9 children = 10. - let expectedTotalItems = 1 + 9 - XCTAssertEqual( - allEnumeratedItems.count, - expectedTotalItems, - "The correct number of total items should be enumerated." - ) + // 3. Assert + XCTAssertNil(observer.error, "Enumeration should complete without error.") + XCTAssertTrue(observer.items.isEmpty, "Result should be empty when no items materialised.") } func testReadServerUrlFollowUpPagination() async throws { From 9d71867b51beabf1e85b8246b91fdffd7febd1f2 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:48:55 +0800 Subject: [PATCH 17/38] Modify change enumeration for working set to only check for materialised items Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator.swift | 118 ++++++++++----- .../EnumeratorTests.swift | 138 ++++++++++++++++-- 2 files changed, 206 insertions(+), 50 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index 5d71ac4b..0bde3e51 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -336,49 +336,95 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { "Enumerating working set changes for \(self.account.ncKitAccount, privacy: .public)" ) + let fakeRootMetadata = SendableItemMetadata( + ocId: NSFileProviderItemIdentifier.rootContainer.rawValue, + account: account.ncKitAccount, + classFile: "", + contentType: "", + creationDate: .init(), + directory: true, + e2eEncrypted: false, + etag: "", + fileId: NSFileProviderItemIdentifier.rootContainer.rawValue, + fileName: "", + fileNameView: "", + ownerId: account.id, + ownerDisplayName: "", + path: "/", + serverUrl: account.davFilesUrl, + size: 0, + urlBase: account.serverUrl, + user: account.username, + userId: account.id + ) + // Unlike when enumerating items we can't progressively enumerate items as we need to // wait to see which items are truly deleted and which have just been moved elsewhere. Task { - let ( - _, newMetadatas, updatedMetadatas, deletedMetadatas, error - ) = await fullRecursiveScan( - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - scanChangesOnly: true - ) + let ncKitAccount = account.ncKitAccount + // Visited folders and downloaded files. Sort in terms of their remote URLs. + // This way we ensure we visit parent folders before their children. + let materialisedItems = [fakeRootMetadata] + dbManager + .materialisedItemMetadatas(account: ncKitAccount) + .sorted { + ($0.serverUrl + "/" + $0.fileName).count < + ($1.serverUrl + "/" + $0.fileName).count + } - if self.isInvalidated { - Self.logger.info( - """ - Enumerator invalidated during working set change scan. - For user: \(self.account.ncKitAccount, privacy: .public) - """ - ) - observer.finishEnumeratingWithError(NSFileProviderError(.cannotSynchronize)) - return - } - guard error == nil else { - Self.logger.info( - """ - Finished recursive change enumeration of working set for user: - \(self.account.ncKitAccount, privacy: .public) - with error: \(error!.errorDescription, privacy: .public) - """ + var allNewMetadatas = [SendableItemMetadata]() + var allUpdatedMetadatas = [SendableItemMetadata]() + var allDeletedMetadatas = [SendableItemMetadata]() + var examinedItemIds = Set() + for item in materialisedItems where !examinedItemIds.contains(item.ocId) { + let ( + metadatas, newMetadatas, updatedMetadatas, deletedMetadatas, _, readError + ) = await Self.readServerUrl( + item.serverUrl + "/" + item.fileName, + account: account, + remoteInterface: remoteInterface, + dbManager: dbManager, + depth: item.directory ? .targetAndDirectChildren : .target ) - // TODO: Refactor for conciseness - let fpError = error?.fileProviderError( - handlingNoSuchItemErrorUsingItemIdentifier: self.enumeratedItemIdentifier - ) ?? NSFileProviderError(.cannotSynchronize) - observer.finishEnumeratingWithError(fpError) - return + if readError?.errorCode == 404 { + allDeletedMetadatas.append(item) + examinedItemIds.insert(item.ocId) + } else if let readError, readError != .success { + Self.logger.info( + """ + Finished change enumeration of working set for user: + \(self.account.ncKitAccount, privacy: .public) + with error: \(readError.errorDescription, privacy: .public) + """ + ) + let fpError = readError.fileProviderError( + handlingNoSuchItemErrorUsingItemIdentifier: self.enumeratedItemIdentifier + ) ?? NSFileProviderError(.cannotSynchronize) + observer.finishEnumeratingWithError(fpError) + } else { + allDeletedMetadatas += deletedMetadatas ?? [] + allUpdatedMetadatas += updatedMetadatas ?? [] + allNewMetadatas += newMetadatas ?? [] + if let target = metadatas?.first { + examinedItemIds.insert(target.ocId) + } + // Just because we have read child directories metadata doesn't mean we need + // to in turn scan their children. This is not the case for files + let examinedChildFilesAndDeletedItems = + ((metadatas?.count ?? 0) > 1 + ? (metadatas?[1...].filter{ !$0.directory }.map(\.ocId) ?? []) + : []) + + (deletedMetadatas?.map(\.ocId) ?? []) + + examinedItemIds.formUnion(examinedChildFilesAndDeletedItems) + } } Self.logger.info( """ - Finished recursive change enumeration of working set for user: - \(self.account.ncKitAccount, privacy: .public). Enumerating items. + Finished change enumeration of working set for user: + \(self.account.ncKitAccount, privacy: .public). + Enumerating items. """ ) @@ -389,9 +435,9 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { account: account, remoteInterface: remoteInterface, dbManager: dbManager, - newMetadatas: newMetadatas, - updatedMetadatas: updatedMetadatas, - deletedMetadatas: deletedMetadatas + newMetadatas: allNewMetadatas, + updatedMetadatas: allUpdatedMetadatas, + deletedMetadatas: allDeletedMetadatas ) } return diff --git a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift index a9cb2d48..673f57ca 100644 --- a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift +++ b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift @@ -475,11 +475,95 @@ final class EnumeratorTests: XCTestCase { ) } - func testWorkingSetChangeEnumeration() async throws { - let db = Self.dbManager.ncDatabase() // Strong ref for in memory test db + func testWorkingSetEnumerateChanges() async throws { + // This test verifies that `enumerateChanges` for the working set correctly identifies + // new, updated, and deleted items by iterating through materialised items and fetching + // their state. + + // 1. Arrange + let db = Self.dbManager.ncDatabase() debugPrint(db) + + // --- Database State (The "local truth" before enumeration) --- + // A materialised folder that we will check for changes. + var materialisedFolder = remoteFolder.toItemMetadata(account: Self.account) + materialisedFolder.visitedDirectory = true + materialisedFolder.etag = "ETAG_OLD" + Self.dbManager.addItemMetadata(materialisedFolder) + + let remoteUnmaterialisedFolder = MockRemoteItem( + identifier: "unmaterialised-folder", + versionIdentifier: "NEW", + name: "unmaterialised folder", + remotePath: Self.account.davFilesUrl + "/" + "unmaterialised folder", + directory: true, + account: Self.account.ncKitAccount, + username: Self.account.username, + userId: Self.account.id, + serverUrl: Self.account.serverUrl + ) + remoteUnmaterialisedFolder.parent = rootItem + rootItem.children.append(remoteUnmaterialisedFolder) + + var unmaterialisedFolder = remoteUnmaterialisedFolder.toItemMetadata(account: Self.account) + unmaterialisedFolder.visitedDirectory = false + unmaterialisedFolder.etag = "ETAG_OLD" + Self.dbManager.addItemMetadata(unmaterialisedFolder) + + let remoteUnmaterialisedFolderChild = MockRemoteItem( + identifier: "unmaterialised-folder-child", + versionIdentifier: "NEW", + name: "unmaterialised folder child", + remotePath: remoteUnmaterialisedFolder.remotePath + "/" + "unmaterialised folder child", + account: Self.account.ncKitAccount, + username: Self.account.username, + userId: Self.account.id, + serverUrl: Self.account.serverUrl + ) + remoteUnmaterialisedFolderChild.parent = remoteUnmaterialisedFolder + remoteUnmaterialisedFolder.children.append(remoteUnmaterialisedFolderChild) + + // A materialised file inside the folder that will be updated on the server. + var fileToUpdate = remoteItemA.toItemMetadata(account: Self.account) + fileToUpdate.downloaded = true + fileToUpdate.etag = "ETAG_OLD" + Self.dbManager.addItemMetadata(fileToUpdate) + + // A materialised file inside the folder that will be deleted from the server. + var fileToBeDeleted = remoteItemB.toItemMetadata(account: Self.account) + fileToBeDeleted.downloaded = true + Self.dbManager.addItemMetadata(fileToBeDeleted) + + // A top-level materialised file that does not exist remotely (i.e. deleted), will cause 404 + var topLevelFileToBeDeleted = SendableItemMetadata( + ocId: "toplevel-deleted", fileName: "toplevel-deleted.txt", account: Self.account + ) + topLevelFileToBeDeleted.downloaded = true + topLevelFileToBeDeleted.uploaded = true + Self.dbManager.addItemMetadata(topLevelFileToBeDeleted) + + // --- Server State (The "remote truth" for the test) --- let remoteInterface = MockRemoteInterface(rootItem: rootItem) + // Update server state to reflect the changes + remoteItemA.versionIdentifier = "ETAG_NEW" + + // Add a new file on the server inside the materialised folder + let newChildFile = MockRemoteItem( + identifier: "new-child", + name: "new-child.txt", + remotePath: remoteFolder.remotePath + "/new-child.txt", + account: Self.account.ncKitAccount, + username: Self.account.username, + userId: Self.account.id, + serverUrl: Self.account.serverUrl + ) + newChildFile.parent = remoteFolder + remoteFolder.children.append(newChildFile) + + // Remove the "deleted" file from the server + remoteFolder.children.removeAll(where: { $0.identifier == fileToBeDeleted.ocId }) + let enumerator = Enumerator( enumeratedItemIdentifier: .workingSet, account: Self.account, @@ -487,23 +571,49 @@ final class EnumeratorTests: XCTestCase { dbManager: Self.dbManager ) let observer = MockChangeObserver(enumerator: enumerator) + + // 2. Act try await observer.enumerateChanges() - XCTAssertEqual(observer.changedItems.count, 3) // Should get all items - let retrievedFolderItem = try XCTUnwrap(observer.changedItems.first) - XCTAssertEqual(retrievedFolderItem.itemIdentifier.rawValue, remoteFolder.identifier) - XCTAssertEqual(retrievedFolderItem.filename, remoteFolder.name) - XCTAssertEqual(retrievedFolderItem.parentItemIdentifier.rawValue, rootItem.identifier) - XCTAssertEqual(retrievedFolderItem.creationDate, remoteFolder.creationDate) + // 3. Assert + XCTAssertNil(observer.error, "Enumeration should complete without error.") + + // Verify Deletions + let deletedIds = observer.deletedItemIdentifiers.map(\.rawValue) + XCTAssertEqual(deletedIds.count, 2, "There should be two deleted items.") + XCTAssertTrue( + deletedIds.contains(fileToBeDeleted.ocId), + "The file deleted from the server folder should be reported as deleted." + ) + XCTAssertTrue( + deletedIds.contains(topLevelFileToBeDeleted.ocId), + "The top-level file that resulted in a 404 should be reported as deleted." + ) + + // Verify Updates and Additions + let changedIds = observer.changedItems.map(\.itemIdentifier.rawValue) + print(changedIds) XCTAssertEqual( - Int(retrievedFolderItem.contentModificationDate??.timeIntervalSince1970 ?? 0), - Int(remoteFolder.modificationDate.timeIntervalSince1970) + changedIds.count, + 5, + "There should be one updated file, three updated folders, and one new file." + ) + XCTAssertTrue(changedIds.contains(NSFileProviderItemIdentifier.rootContainer.rawValue)) + XCTAssertTrue(changedIds.contains(materialisedFolder.ocId)) + XCTAssertTrue(changedIds.contains(unmaterialisedFolder.ocId)) + XCTAssertTrue(changedIds.contains(fileToUpdate.ocId)) + XCTAssertTrue(changedIds.contains(newChildFile.identifier)) + + // Verify the database state was updated + let finalUpdatedFile = Self.dbManager.itemMetadata(ocId: fileToUpdate.ocId) + XCTAssertEqual( + finalUpdatedFile?.etag, + "ETAG_NEW", + "The ETag for the updated file should be changed in the database." ) - XCTAssertEqual(retrievedFolderItem.isUploaded, true) - // Ensure the newly discovered folder has an etag - let dbFolder = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: remoteFolder.identifier)) - XCTAssertEqual(dbFolder.etag, remoteFolder.versionIdentifier) + let finalNewFile = Self.dbManager.itemMetadata(ocId: newChildFile.identifier) + XCTAssertNotNil(finalNewFile, "The new file should now exist in the database.") } func testFolderEnumeration() async throws { From 10e9a4f07791ce0652936957089cdb98000608a0 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:49:31 +0800 Subject: [PATCH 18/38] Remove unused recursive scan components from sync engine Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator+SyncEngine.swift | 247 ------------------ 1 file changed, 247 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift index eb4bd3ad..aee2c99d 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift @@ -17,253 +17,6 @@ import NextcloudKit import OSLog extension Enumerator { - func fullRecursiveScan( - account: Account, - remoteInterface: RemoteInterface, - dbManager: FilesDatabaseManager, - scanChangesOnly: Bool - ) async -> ( - metadatas: [SendableItemMetadata], - newMetadatas: [SendableItemMetadata], - updatedMetadatas: [SendableItemMetadata], - deletedMetadatas: [SendableItemMetadata], - error: NKError? - ) { - let results = await self.scanRecursively( - Item.rootContainer( - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - remoteSupportsTrash: await remoteInterface.supportsTrash(account: account) - ).metadata, - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - scanChangesOnly: scanChangesOnly - ) - - // Run a check to ensure files deleted in one location are not updated in another - // (e.g. when moved) - // The recursive scan provides us with updated/deleted metadatas only on a folder by - // folder basis; so we need to check we are not simultaneously marking a moved file as - // deleted and updated - var checkedDeletedMetadatas = results.deletedMetadatas - - for updatedMetadata in results.updatedMetadatas { - guard let matchingDeletedMetadataIdx = checkedDeletedMetadatas.firstIndex( - where: { $0.ocId == updatedMetadata.ocId } - ) else { continue } - - checkedDeletedMetadatas.remove(at: matchingDeletedMetadataIdx) - } - - return ( - results.metadatas, - results.newMetadatas, - results.updatedMetadatas, - checkedDeletedMetadatas, - results.error - ) - } - - private func scanRecursively( - _ directoryMetadata: SendableItemMetadata, - account: Account, - remoteInterface: RemoteInterface, - dbManager: FilesDatabaseManager, - scanChangesOnly: Bool - ) async -> ( - metadatas: [SendableItemMetadata], - newMetadatas: [SendableItemMetadata], - updatedMetadatas: [SendableItemMetadata], - deletedMetadatas: [SendableItemMetadata], - error: NKError? - ) { - if isInvalidated { - return ([], [], [], [], nil) - } - - assert(directoryMetadata.directory, "Can only recursively scan a directory.") - - // Will include results of recursive calls - var allMetadatas: [SendableItemMetadata] = [] - var allNewMetadatas: [SendableItemMetadata] = [] - var allUpdatedMetadatas: [SendableItemMetadata] = [] - var allDeletedMetadatas: [SendableItemMetadata] = [] - - let itemServerUrl = - directoryMetadata.ocId == NSFileProviderItemIdentifier.rootContainer.rawValue - ? account.davFilesUrl - : directoryMetadata.serverUrl + "/" + directoryMetadata.fileName - - Self.logger.debug("About to read: \(itemServerUrl, privacy: .public)") - - let ( - metadatas, newMetadatas, updatedMetadatas, deletedMetadatas, _, readError - ) = await Self.readServerUrl( - itemServerUrl, - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - domain: domain, - enumeratedItemIdentifier: enumeratedItemIdentifier - ) - - if let readError, readError != .success { - // Is the error is that we have found matching etags on this item, then ignore it - // if we are doing a full rescan - if readError.isNoChangesError, scanChangesOnly { - Self.logger.info("No changes in \(self.serverUrl) and only scanning changes.") - } else { - Self.logger.error( - """ - Finishing enumeration of changes at \(itemServerUrl, privacy: .public) - with \(readError.errorDescription, privacy: .public) - """ - ) - - if readError.isNotFoundError { - Self.logger.info( - """ - 404 error means item no longer exists. - Deleting metadata and reporting as deletion without error. - """ - ) - - if let deletedMetadatas = dbManager.deleteDirectoryAndSubdirectoriesMetadata( - ocId: directoryMetadata.ocId - ) { - allDeletedMetadatas += deletedMetadatas - } else { - Self.logger.error( - """ - An error occurred while trying to delete directory in database, - children not found in recursive scan - """ - ) - } - - } else if readError.isNoChangesError { // All is well, just no changed etags - Self.logger.info( - "Error was to say no changed files, not bad error. Won't check children." - ) - - } else if readError.isUnauthenticatedError || readError.isCouldntConnectError { - Self.logger.error( - "Error will affect next enumerated items, so stopping enumeration." - ) - return ([], [] , [], [], readError) - } - } - } - - Self.logger.info( - """ - Finished reading serverUrl: \(itemServerUrl, privacy: .public) - for user: \(account.ncKitAccount, privacy: .public) - """ - ) - - if let metadatas { - allMetadatas += metadatas - } else { - Self.logger.warning( - """ - Nil metadatas received in change read at \(itemServerUrl, privacy: .public) - for user: \(account.ncKitAccount, privacy: .public) - """ - ) - } - - if let newMetadatas { - allNewMetadatas += newMetadatas - } else { - Self.logger.warning( - """ - Nil new metadatas received in change read at \(itemServerUrl, privacy: .public) - for user: \(account.ncKitAccount, privacy: .public) - """ - ) - } - - if let updatedMetadatas { - allUpdatedMetadatas += updatedMetadatas - } else { - Self.logger.warning( - """ - Nil updated metadatas received in change read at \(itemServerUrl, privacy: .public) - for user: \(account.ncKitAccount, privacy: .public) - """ - ) - } - - if let deletedMetadatas { - allDeletedMetadatas += deletedMetadatas - } else { - Self.logger.warning( - """ - Nil deleted metadatas received in change read at \(itemServerUrl, privacy: .public) - for user: \(account.ncKitAccount, privacy: .public) - """ - ) - } - - var childDirectoriesToScan: [SendableItemMetadata] = [] - var candidateMetadatas: [SendableItemMetadata] - - if scanChangesOnly { - candidateMetadatas = allUpdatedMetadatas + allNewMetadatas - } else { - candidateMetadatas = allMetadatas - } - - for candidateMetadata in candidateMetadatas where candidateMetadata.directory { - childDirectoriesToScan.append(candidateMetadata) - } - - Self.logger.debug( - "Candidate metadatas for further scan: \(childDirectoriesToScan, privacy: .public)" - ) - - if childDirectoriesToScan.isEmpty { - return ( - metadatas: allMetadatas, - newMetadatas: allNewMetadatas, - updatedMetadatas: allUpdatedMetadatas, - deletedMetadatas: allDeletedMetadatas, - nil - ) - } - - for childDirectory in childDirectoriesToScan { - let childDirectoryUrl = childDirectory.serverUrl + "/" + childDirectory.fileName - Self.logger.debug( - """ - About to recursively scan: \(childDirectoryUrl, privacy: .public) - with etag: \(childDirectory.etag, privacy: .public) - """ - ) - let childScanResult = await scanRecursively( - childDirectory, - account: account, - remoteInterface: remoteInterface, - dbManager: dbManager, - scanChangesOnly: scanChangesOnly - ) - - allMetadatas += childScanResult.metadatas - allNewMetadatas += childScanResult.newMetadatas - allUpdatedMetadatas += childScanResult.updatedMetadatas - allDeletedMetadatas += childScanResult.deletedMetadatas - } - - return ( - metadatas: allMetadatas, newMetadatas: allNewMetadatas, - updatedMetadatas: allUpdatedMetadatas, - deletedMetadatas: allDeletedMetadatas, nil - ) - } - static func handlePagedReadResults( files: [NKFile], pageIndex: Int, dbManager: FilesDatabaseManager ) -> (metadatas: [SendableItemMetadata]?, error: NKError?) { From 83fd3746d375289f6aa041e474ef45f7225b6244 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:49:48 +0800 Subject: [PATCH 19/38] Remove unused components of EnumeratorPageResponse Signed-off-by: Claudio Cambra --- .../Enumeration/EnumeratorPageResponse.swift | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/EnumeratorPageResponse.swift b/Sources/NextcloudFileProviderKit/Enumeration/EnumeratorPageResponse.swift index 8a687885..81071d25 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/EnumeratorPageResponse.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/EnumeratorPageResponse.swift @@ -15,8 +15,6 @@ struct EnumeratorPageResponse: Sendable, Codable { let token: String? // Required by server to serve the next page of items let index: Int // Needed to calculate the offset for the next paginated request var total: Int? // Total item count, provided in the first non-offset paginated response - var serverUrlQueue: [String]? - var nextServerUrl: String? = nil init?(nkResponseData: AFDataResponse?, index: Int) { guard let headers = nkResponseData?.response?.allHeaderFields as? [String: String] else { @@ -42,8 +40,6 @@ struct EnumeratorPageResponse: Sendable, Codable { } else { total = nil } - nextServerUrl = nil - serverUrlQueue = nil let totalString = total != nil ? String(total ?? -1) : "nil" logger.debug( @@ -55,13 +51,4 @@ struct EnumeratorPageResponse: Sendable, Codable { """ ) } - - init(nextServerUrl: String, serverUrlQueue: [String]? = nil) { - logger.debug("Creating artificial page response with \(nextServerUrl, privacy: .public)") - self.token = nil - self.index = 0 - self.total = nil - self.nextServerUrl = nextServerUrl - self.serverUrlQueue = serverUrlQueue - } } From f149ab2b19878692640f6a68caf35ac0b735411a Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 17:50:16 +0800 Subject: [PATCH 20/38] Address readServerUrl related changes in enumerator tests Signed-off-by: Claudio Cambra --- .../EnumeratorTests.swift | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift index 673f57ca..0ec6e0d9 100644 --- a/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift +++ b/Tests/NextcloudFileProviderKitTests/EnumeratorTests.swift @@ -769,8 +769,8 @@ final class EnumeratorTests: XCTestCase { ) let observer = MockChangeObserver(enumerator: enumerator) try await observer.enumerateChanges() - // There are three changes: changed Item A, removed Item B, added Item C - XCTAssertEqual(observer.changedItems.count, 2) + // There are four changes: changed folder, changed Item A, removed Item B, added Item C + XCTAssertEqual(observer.changedItems.count, 3) XCTAssertTrue(observer.changedItems.contains( where: { $0.itemIdentifier.rawValue == remoteItemA.identifier } )) @@ -794,7 +794,6 @@ final class EnumeratorTests: XCTestCase { ) XCTAssertNil(Self.dbManager.itemMetadata(ocId: remoteItemB.identifier)) XCTAssertEqual(dbFolderMetadata.etag, remoteFolder.versionIdentifier) - XCTAssertNotEqual(dbFolderMetadata.etag, oldFolderEtag) XCTAssertTrue(dbFolderMetadata.downloaded) XCTAssertEqual(dbItemAMetadata.etag, remoteItemA.versionIdentifier) XCTAssertNotEqual(dbItemAMetadata.etag, oldItemAEtag) @@ -884,7 +883,7 @@ final class EnumeratorTests: XCTestCase { let observer = MockChangeObserver(enumerator: enumerator) try await observer.enumerateChanges() // rootContainer has changed, folder has changed, itemA has changed - XCTAssertEqual(observer.changedItems.count, 2) // Not including target (TODO) + XCTAssertEqual(observer.changedItems.count, 3) XCTAssertTrue(observer.deletedItemIdentifiers.isEmpty) let retrievedItemA = try XCTUnwrap(observer.changedItems.first( @@ -922,7 +921,7 @@ final class EnumeratorTests: XCTestCase { remoteSupportsTrash: await remoteInterface.supportsTrash(account: Self.account) ) print(storedRootItem.metadata.serverUrl) - XCTAssertEqual(storedRootItem.childItemCount?.intValue, 3) // All items + XCTAssertEqual(storedRootItem.childItemCount?.intValue, 4) // All items let storedFolderMaybe = await Item.storedItem( identifier: .init(remoteFolder.identifier), @@ -968,7 +967,7 @@ final class EnumeratorTests: XCTestCase { ) let observer = MockChangeObserver(enumerator: enumerator) try await observer.enumerateChanges() - XCTAssertEqual(observer.changedItems.count, 3) + XCTAssertEqual(observer.changedItems.count, 4) let dbItemAMetadata = try XCTUnwrap( Self.dbManager.itemMetadata(ocId: remoteItemA.identifier) @@ -1043,7 +1042,7 @@ final class EnumeratorTests: XCTestCase { let observer = MockChangeObserver(enumerator: enumerator) try await observer.enumerateChanges() // rootContainer has changed, itemA has changed - XCTAssertEqual(observer.changedItems.count, 1) + XCTAssertEqual(observer.changedItems.count, 2) let dbItemAMetadata = try XCTUnwrap( Self.dbManager.itemMetadata(ocId: remoteItemA.identifier) From 50350b9d8b94d555858a7182408ac11d8c4f349b Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 13 Jun 2025 18:22:19 +0800 Subject: [PATCH 21/38] Avoid scanning materialised children of materialised parents we have confirmed have no updates Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator.swift | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index 0bde3e51..945f2c10 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -377,10 +377,11 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { var allDeletedMetadatas = [SendableItemMetadata]() var examinedItemIds = Set() for item in materialisedItems where !examinedItemIds.contains(item.ocId) { + let itemRemoteUrl = item.serverUrl + "/" + item.fileName let ( metadatas, newMetadatas, updatedMetadatas, deletedMetadatas, _, readError ) = await Self.readServerUrl( - item.serverUrl + "/" + item.fileName, + itemRemoteUrl, account: account, remoteInterface: remoteInterface, dbManager: dbManager, @@ -405,18 +406,33 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { allDeletedMetadatas += deletedMetadatas ?? [] allUpdatedMetadatas += updatedMetadatas ?? [] allNewMetadatas += newMetadatas ?? [] - if let target = metadatas?.first { - examinedItemIds.insert(target.ocId) - } + // Just because we have read child directories metadata doesn't mean we need // to in turn scan their children. This is not the case for files - let examinedChildFilesAndDeletedItems = - ((metadatas?.count ?? 0) > 1 - ? (metadatas?[1...].filter{ !$0.directory }.map(\.ocId) ?? []) - : []) - + (deletedMetadatas?.map(\.ocId) ?? []) + var examinedChildFilesAndDeletedItems = Set() + if let metadatas, let target = metadatas.first { + examinedItemIds.insert(target.ocId) + + if metadatas.count > 1 { + examinedChildFilesAndDeletedItems.formUnion( + metadatas[1...].filter { !$0.directory }.map(\.ocId) + ) + } + + if !allUpdatedMetadatas.contains(where: { $0.ocId == target.ocId }) { + let materialisedChildren = materialisedItems.filter { + $0.serverUrl.hasPrefix(itemRemoteUrl) + }.map(\.ocId) + examinedChildFilesAndDeletedItems.formUnion(materialisedChildren) + } + + if let deletedMetadataOcIds = deletedMetadatas?.map(\.ocId) { + examinedChildFilesAndDeletedItems.formUnion(deletedMetadataOcIds) + } + } examinedItemIds.formUnion(examinedChildFilesAndDeletedItems) + // TODO: Observer changes here!! } } From 051bb9aac04951e03b22cd9f6df69532aa60eb05 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 10:34:10 +0800 Subject: [PATCH 22/38] Imitate NextcloudKit behaviour in MockRemoteItem conversion to NKFile Signed-off-by: Claudio Cambra --- Tests/Interface/MockRemoteItem.swift | 13 +++++++++---- Tests/InterfaceTests/MockRemoteInterfaceTests.swift | 4 +++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Tests/Interface/MockRemoteItem.swift b/Tests/Interface/MockRemoteItem.swift index 7ae5aadc..1a7a2a29 100644 --- a/Tests/Interface/MockRemoteItem.swift +++ b/Tests/Interface/MockRemoteItem.swift @@ -60,7 +60,7 @@ public class MockRemoteItem: Equatable { MockRemoteItem( identifier: NSFileProviderItemIdentifier.rootContainer.rawValue, versionIdentifier: "root", - name: ".", + name: "", remotePath: account.davFilesUrl, directory: true, account: account.ncKitAccount, @@ -121,15 +121,20 @@ public class MockRemoteItem: Equatable { } public func toNKFile() -> NKFile { + let isRoot = identifier == NSFileProviderItemIdentifier.rootContainer.rawValue var file = NKFile() - file.fileName = trashbinOriginalLocation?.split(separator: "/").last?.toString() ?? name + file.fileName = isRoot + ? "." + : trashbinOriginalLocation?.split(separator: "/").last?.toString() ?? name file.size = size file.date = creationDate - file.directory = directory + file.directory = isRoot ? false : directory file.etag = versionIdentifier file.ocId = identifier file.fileId = identifier.replacingOccurrences(of: trashedItemIdSuffix, with: "") - file.serverUrl = parent?.remotePath ?? remotePath + file.serverUrl = isRoot + ? ".." + : parent?.remotePath ?? remotePath file.account = account file.user = username file.userId = userId diff --git a/Tests/InterfaceTests/MockRemoteInterfaceTests.swift b/Tests/InterfaceTests/MockRemoteInterfaceTests.swift index 725b69fb..1fcba6ac 100644 --- a/Tests/InterfaceTests/MockRemoteInterfaceTests.swift +++ b/Tests/InterfaceTests/MockRemoteInterfaceTests.swift @@ -504,7 +504,9 @@ final class MockRemoteInterfaceTests: XCTestCase { let targetRootFile = result.files.first let expectedRoot = remoteInterface.rootItem XCTAssertEqual(targetRootFile?.ocId, expectedRoot?.identifier) - XCTAssertEqual(targetRootFile?.fileName, expectedRoot?.name) + XCTAssertEqual(targetRootFile?.fileName, ".") // NextcloudKit gives the root dir this name + XCTAssertNotEqual(targetRootFile?.fileName, expectedRoot?.name) + XCTAssertEqual(targetRootFile?.serverUrl, "..") // NextcloudKit gives the root dir this surl XCTAssertEqual(targetRootFile?.date, expectedRoot?.creationDate) XCTAssertEqual(targetRootFile?.etag, expectedRoot?.versionIdentifier) From 65f0b63f6b57f974e4f684e510772f7a8451bbbd Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 10:34:55 +0800 Subject: [PATCH 23/38] Try to clean up received path a bit in MockRemoteInterface Signed-off-by: Claudio Cambra --- Tests/Interface/MockRemoteInterface.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/Interface/MockRemoteInterface.swift b/Tests/Interface/MockRemoteInterface.swift index 388c84ef..15894b71 100644 --- a/Tests/Interface/MockRemoteInterface.swift +++ b/Tests/Interface/MockRemoteInterface.swift @@ -1033,6 +1033,13 @@ public class MockRemoteInterface: RemoteInterface { options: NKRequestOptions = .init(), taskHandler: @escaping (URLSessionTask) -> Void = { _ in } ) async -> (account: String, files: [NKFile], data: AFDataResponse?, error: NKError) { + var remotePath = remotePath + if remotePath.last == "." { + remotePath.removeLast() + } + if remotePath.last == "/" { + remotePath.removeLast() + } print("Enumerating \(remotePath)") guard let item = item(remotePath: remotePath, account: account) else { print("Item at \(remotePath) not found.") From 5f2d1cdf6fb8ee1302e9b8712ce17f45b1f87058 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 10:35:41 +0800 Subject: [PATCH 24/38] Prevent bad trimming of serverUrl for root item in MockRemoteItem conversion to SendableItemMetadata Signed-off-by: Claudio Cambra --- Tests/Interface/MockRemoteItem.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Tests/Interface/MockRemoteItem.swift b/Tests/Interface/MockRemoteItem.swift index 1a7a2a29..01555f7c 100644 --- a/Tests/Interface/MockRemoteItem.swift +++ b/Tests/Interface/MockRemoteItem.swift @@ -166,11 +166,13 @@ public class MockRemoteItem: Equatable { let serverUrlTrimCount = name.count var trimmedServerUrl = remotePath - trimmedServerUrl.removeSubrange( - remotePath.index( - remotePath.endIndex, offsetBy: -(serverUrlTrimCount + 1) // Remove trailing slash - ).. Date: Mon, 16 Jun 2025 10:39:20 +0800 Subject: [PATCH 25/38] Fix up mangling of root container metadata performed by NextcloudKit Signed-off-by: Claudio Cambra --- .../Extensions/NKFile+Extensions.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index 1a8d25ea..7795793d 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -5,6 +5,7 @@ // Created by Claudio Cambra on 2024-12-02. // +import FileProvider import Foundation import NextcloudKit import RealmSwift @@ -27,6 +28,12 @@ extension NKFile { : classFile // Support for finding the correct filename for e2ee files should go here + let rootRequiresFixup = serverUrl == ".." && fileName == "." + let serverUrl = rootRequiresFixup + ? urlBase + Account.webDavFilesUrlSuffix + userId + : self.serverUrl + let fileName = rootRequiresFixup ? "" : self.fileName + return SendableItemMetadata( ocId: ocId, account: account, @@ -116,9 +123,15 @@ extension Array { childDirectoriesMetadatas: [SendableItemMetadata], metadatas: [SendableItemMetadata] )? { - guard let targetDirectoryMetadata = first?.toItemMetadata() else { + guard var targetDirectoryMetadata = first?.toItemMetadata() else { return nil } + // Don't ask me why, NextcloudKit renames and moves the root folder details + // Also don't ask me why, but, NextcloudKit marks the NKFile for this as not a directory + if first?.serverUrl == ".." && first?.fileName == "." { + targetDirectoryMetadata.ocId = NSFileProviderItemIdentifier.rootContainer.rawValue + targetDirectoryMetadata.directory = true + } let conversionActor = DirectoryReadConversionActor(target: targetDirectoryMetadata) await concurrentChunkedForEach { file in guard file.ocId != targetDirectoryMetadata.ocId else { return } From 77a2e17d8cd0746b975024edb3587fe58e6a0a55 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 13:00:16 +0800 Subject: [PATCH 26/38] Ingest metadata for the root container Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator+SyncEngine.swift | 47 +++++++------------ .../EnumeratorTests.swift | 11 +++-- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift index aee2c99d..56c6921d 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift @@ -26,20 +26,14 @@ extension Enumerator { let startIndex = pageIndex > 0 ? 0 : 1 if pageIndex == 0 { guard let firstFile = files.first else { return (nil, .invalidResponseError) } - // Do not ingest metadata for the root container - if !firstFile.fullUrlMatches(dbManager.account.davFilesUrl), - !firstFile.fullUrlMatches(dbManager.account.davFilesUrl + "/."), - !(firstFile.fileName == "." && firstFile.serverUrl == "..") - { - var metadata = firstFile.toItemMetadata() - if metadata.directory { - metadata.visitedDirectory = true - if let existingMetadata = dbManager.itemMetadata(ocId: metadata.ocId) { - metadata.downloaded = existingMetadata.downloaded - } + var metadata = firstFile.toItemMetadata() + if metadata.directory { + metadata.visitedDirectory = true + if let existingMetadata = dbManager.itemMetadata(ocId: metadata.ocId) { + metadata.downloaded = existingMetadata.downloaded } - dbManager.addItemMetadata(metadata) } + dbManager.addItemMetadata(metadata) } let metadatas = files[startIndex.. Date: Mon, 16 Jun 2025 13:04:33 +0800 Subject: [PATCH 27/38] Do not create fake root metadata for working set enumeration Now that we ingest the root metadata, rely on this Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator.swift | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index 945f2c10..bdfc6001 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -336,35 +336,13 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { "Enumerating working set changes for \(self.account.ncKitAccount, privacy: .public)" ) - let fakeRootMetadata = SendableItemMetadata( - ocId: NSFileProviderItemIdentifier.rootContainer.rawValue, - account: account.ncKitAccount, - classFile: "", - contentType: "", - creationDate: .init(), - directory: true, - e2eEncrypted: false, - etag: "", - fileId: NSFileProviderItemIdentifier.rootContainer.rawValue, - fileName: "", - fileNameView: "", - ownerId: account.id, - ownerDisplayName: "", - path: "/", - serverUrl: account.davFilesUrl, - size: 0, - urlBase: account.serverUrl, - user: account.username, - userId: account.id - ) - // Unlike when enumerating items we can't progressively enumerate items as we need to // wait to see which items are truly deleted and which have just been moved elsewhere. Task { let ncKitAccount = account.ncKitAccount // Visited folders and downloaded files. Sort in terms of their remote URLs. // This way we ensure we visit parent folders before their children. - let materialisedItems = [fakeRootMetadata] + dbManager + let materialisedItems = dbManager .materialisedItemMetadatas(account: ncKitAccount) .sorted { ($0.serverUrl + "/" + $0.fileName).count < From a0b83bd6d63f7e1cff7a4c4b8d74010f678af79f Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 13:11:12 +0800 Subject: [PATCH 28/38] Exclude root container from child metadata change calculation Signed-off-by: Claudio Cambra --- .../Database/FilesDatabaseManager.swift | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index 76a0106e..9a46e7f3 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -342,7 +342,13 @@ public final class FilesDatabaseManager: Sendable { } let existingMetadatas = database .objects(RealmItemMetadata.self) - .where { $0.account == account && $0.serverUrl == cleanServerUrl && $0.uploaded } + .where { + // Don't worry — root will be updated at the end of this method if is the target + $0.ocId != NSFileProviderItemIdentifier.rootContainer.rawValue && + $0.account == account && + $0.serverUrl == cleanServerUrl && + $0.uploaded + } var updatedChildMetadatas = updatedMetadatas From 8c0e0b5a107dd0d97db5fab4bc838312ee5ade22 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 14:17:17 +0800 Subject: [PATCH 29/38] Fix sorting of filenames Signed-off-by: Claudio Cambra --- Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index bdfc6001..bd1267cb 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -346,7 +346,7 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { .materialisedItemMetadatas(account: ncKitAccount) .sorted { ($0.serverUrl + "/" + $0.fileName).count < - ($1.serverUrl + "/" + $0.fileName).count + ($1.serverUrl + "/" + $1.fileName).count } From 8f9c974970ec5936bcdc171f151c4cbaa3a3e037 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 14:34:35 +0800 Subject: [PATCH 30/38] Improve deleted metadata log formatting Signed-off-by: Claudio Cambra --- .../Database/FilesDatabaseManager.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift index 9a46e7f3..a5932357 100644 --- a/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift +++ b/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift @@ -240,7 +240,12 @@ public final class FilesDatabaseManager: Sendable { deletedMetadatas.append(metadataToDelete) Self.logger.debug( - "Deleting item metadata during update. ocID: \(existingMetadata.ocId, privacy: .public), etag: \(existingMetadata.etag, privacy: .public), fileName: \(existingMetadata.fileName, privacy: .public)" + """ + Deleting item metadata during update. + ocID: \(existingMetadata.ocId, privacy: .public) + etag: \(existingMetadata.etag, privacy: .public) + fileName: \(existingMetadata.fileName, privacy: .public)" + """ ) } From 10d04abfcba67a594da517e6601d1315206221b2 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 14:35:23 +0800 Subject: [PATCH 31/38] Mark moved metadatas as updated and not deleted in working set change enumeration Signed-off-by: Claudio Cambra --- .../Enumeration/Enumerator.swift | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index bd1267cb..275d81f1 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -410,10 +410,25 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { } examinedItemIds.formUnion(examinedChildFilesAndDeletedItems) - // TODO: Observer changes here!! } } + // Run a check to ensure files deleted in one location are not updated in another + // (e.g. when moved) + // The recursive scan provides us with updated/deleted metadatas only on a folder by + // folder basis; so we need to check we are not simultaneously marking a moved file as + // deleted and updated + var checkedDeletedMetadatas = allDeletedMetadatas + + for updatedMetadata in allUpdatedMetadatas { + guard let matchingDeletedMetadataIdx = checkedDeletedMetadatas.firstIndex( + where: { $0.ocId == updatedMetadata.ocId } + ) else { continue } + checkedDeletedMetadatas.remove(at: matchingDeletedMetadataIdx) + } + + allDeletedMetadatas = checkedDeletedMetadatas + Self.logger.info( """ Finished change enumeration of working set for user: From 97235f905d50f85e641928866ebc24fd4bdcb340 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 15:23:09 +0800 Subject: [PATCH 32/38] Explain optimisation with examined urls Signed-off-by: Claudio Cambra --- Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift index 275d81f1..13ebafe5 100644 --- a/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift +++ b/Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift @@ -397,6 +397,8 @@ public class Enumerator: NSObject, NSFileProviderEnumerator { ) } + // If the target is not in the updated metadatas then neither it, nor + // any of its kids have changed. So skip examining all of them if !allUpdatedMetadatas.contains(where: { $0.ocId == target.ocId }) { let materialisedChildren = materialisedItems.filter { $0.serverUrl.hasPrefix(itemRemoteUrl) From 1c6ae15e48ad06144ccd424f5e785526884e14f9 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 18:38:02 +0800 Subject: [PATCH 33/38] Also catch non-bungled root metadata metadatas Signed-off-by: Claudio Cambra --- .../Extensions/NKFile+Extensions.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index 7795793d..5cec57d6 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -128,7 +128,9 @@ extension Array { } // Don't ask me why, NextcloudKit renames and moves the root folder details // Also don't ask me why, but, NextcloudKit marks the NKFile for this as not a directory - if first?.serverUrl == ".." && first?.fileName == "." { + if (targetDirectoryMetadata.serverUrl == ".." && targetDirectoryMetadata.fileName == ".") || + (targetDirectoryMetadata.serverUrl + "/" + targetDirectoryMetadata.fileName == account.davFilesUrl + "/") + { targetDirectoryMetadata.ocId = NSFileProviderItemIdentifier.rootContainer.rawValue targetDirectoryMetadata.directory = true } From 5d206aa0ef090425bfb3c0336d26004f8eab865d Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 18:38:18 +0800 Subject: [PATCH 34/38] Make sure to check for root container metadata after changing target ocId Signed-off-by: Claudio Cambra --- .../Extensions/NKFile+Extensions.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index 5cec57d6..7cdcb804 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -136,7 +136,9 @@ extension Array { } let conversionActor = DirectoryReadConversionActor(target: targetDirectoryMetadata) await concurrentChunkedForEach { file in - guard file.ocId != targetDirectoryMetadata.ocId else { return } + guard file.ocId != targetDirectoryMetadata.ocId || + file.ocId != NSFileProviderItemIdentifier.rootContainer.rawValue + else { return } await conversionActor.add(metadata: file.toItemMetadata()) } return await conversionActor.convertedMetadatas() From 2ad72903f54460601b02dbd759bbebe862c35c75 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 19:38:16 +0800 Subject: [PATCH 35/38] Make chunkedConcurrentMap compatible with all random access collections Signed-off-by: Claudio Cambra --- ...> RandomAccessCollection+Extensions.swift} | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) rename Sources/NextcloudFileProviderKit/Extensions/{Array+Extensions.swift => RandomAccessCollection+Extensions.swift} (54%) diff --git a/Sources/NextcloudFileProviderKit/Extensions/Array+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/RandomAccessCollection+Extensions.swift similarity index 54% rename from Sources/NextcloudFileProviderKit/Extensions/Array+Extensions.swift rename to Sources/NextcloudFileProviderKit/Extensions/RandomAccessCollection+Extensions.swift index 338ce139..8e4afaac 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/Array+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/RandomAccessCollection+Extensions.swift @@ -1,26 +1,40 @@ // -// Array+Extensions.swift +// RandomAccessCollection+Extensions.swift // NextcloudFileProviderKit // // Created by Claudio Cambra on 2024-12-18. // +import Foundation + fileprivate let defaultChunkSize = 64 -extension Array { - func chunked(into size: Int = defaultChunkSize) -> [ArraySlice] { +extension RandomAccessCollection { + /// Chunks a collection into an array of its subsequences. + /// - Parameter size: The size of each chunk. + /// - Returns: An array of subsequences (e.g., ArraySlice). + func chunked(into size: Int = defaultChunkSize) -> [SubSequence] { guard size > 0 else { return [] } // Avoid invalid chunk sizes - return stride(from: 0, to: count, by: size).map { startIndex in - self[startIndex..(into size: Int = defaultChunkSize, transform: (Element) -> T) -> [[T]] { return self.chunked(into: size).map { chunk in chunk.map(transform) } } + /// Performs an asynchronous `forEach` operation on the collection in concurrent chunks. func concurrentChunkedForEach( into size: Int = defaultChunkSize, operation: @escaping (Element) async -> Void ) async { @@ -35,12 +49,14 @@ extension Array { } } + /// Performs an asynchronous `compactMap` operation on the collection in concurrent chunks. func concurrentChunkedCompactMap( into size: Int = defaultChunkSize, transform: @escaping (Element) throws -> T? ) async throws -> [T] { try await withThrowingTaskGroup(of: [T].self) { group in var results = [T]() - results.reserveCapacity(self.count) + // Reserving capacity is still a good optimization, though we can't know the exact final count. + results.reserveCapacity(Int(self.count)) for chunk in chunked(into: size) { group.addTask { From 355a62552d412a22c2d8a3b316db8ebea6c7bc2a Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 19:39:01 +0800 Subject: [PATCH 36/38] Ensure we do not repeat the target metadata in the child metadatas during nkfile conversions Signed-off-by: Claudio Cambra --- .../Extensions/NKFile+Extensions.swift | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index 7cdcb804..ba2ba5e1 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -135,11 +135,10 @@ extension Array { targetDirectoryMetadata.directory = true } let conversionActor = DirectoryReadConversionActor(target: targetDirectoryMetadata) - await concurrentChunkedForEach { file in - guard file.ocId != targetDirectoryMetadata.ocId || - file.ocId != NSFileProviderItemIdentifier.rootContainer.rawValue - else { return } - await conversionActor.add(metadata: file.toItemMetadata()) + if self.count > 1 { + await self[1...].concurrentChunkedForEach { file in + await conversionActor.add(metadata: file.toItemMetadata()) + } } return await conversionActor.convertedMetadatas() } From aca2c44e22be29e6a029fade8bc5945136c55dc6 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 19:39:11 +0800 Subject: [PATCH 37/38] Add tests for nkfile conversions Signed-off-by: Claudio Cambra --- .../NKFileExtensionTests.swift | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 Tests/NextcloudFileProviderKitTests/NKFileExtensionTests.swift diff --git a/Tests/NextcloudFileProviderKitTests/NKFileExtensionTests.swift b/Tests/NextcloudFileProviderKitTests/NKFileExtensionTests.swift new file mode 100644 index 00000000..515e1003 --- /dev/null +++ b/Tests/NextcloudFileProviderKitTests/NKFileExtensionTests.swift @@ -0,0 +1,182 @@ +// +// NKFileExtensionTests.swift +// NextcloudFileProviderKit +// +// Created by Claudio Cambra on 16/6/25. +// + +import XCTest +import FileProvider +import NextcloudKit +@testable import NextcloudFileProviderKit + +final class NKFileExtensionsTests: XCTestCase { + + static let account = Account( + user: "testUser", + id: "testUserId", + serverUrl: "https://mock.nc.com", + password: "abcd" + ) + + /// Creates a mock NKFile. + private func createNKFile( + ocId: String = "id1", + serverUrl: String? = nil, + fileName: String = "file.txt", + directory: Bool = false, + userId: String? = nil, + urlBase: String? = nil + ) -> NKFile { + var file = NKFile() + file.ocId = ocId + file.serverUrl = serverUrl ?? Self.account.davFilesUrl + file.fileName = fileName + file.directory = directory + file.userId = userId ?? Self.account.id + file.urlBase = urlBase ?? Self.account.serverUrl + // Add other necessary properties with default values + file.account = Self.account.ncKitAccount + file.date = Date() + file.creationDate = Date() + file.etag = "etag" + return file + } + + // MARK: - toItemMetadata() Tests + + func testToItemMetadataHandlesRootFixupCorrectly() { + // 1. Arrange: Create an NKFile representing the root container, + // which has a special serverUrl and fileName. + let rootNKFile = createNKFile( + ocId: "rootId", + serverUrl: "..", // Special root value + fileName: ".", // Special root value + directory: false // NextcloudKit sometimes marks the root as not a directory + ) + + // 2. Act + let metadata = rootNKFile.toItemMetadata() + + // 3. Assert + // The `rootRequiresFixup` logic should trigger. + XCTAssertEqual( + metadata.serverUrl, + Self.account.davFilesUrl, + "The serverUrl for the root should be corrected to the full WebDAV path." + ) + XCTAssertEqual( + metadata.fileName, + "", + "The fileName for the root should be corrected to an empty string." + ) + } + + func testToItemMetadataHandlesStandardFileCorrectly() { + // 1. Arrange: Create a standard NKFile for a regular file. + let standardNKFile = createNKFile( + ocId: "file123", + serverUrl: Self.account.davFilesUrl + "/photos", + fileName: "image.jpg" + ) + + // 2. Act + let metadata = standardNKFile.toItemMetadata() + + // 3. Assert + // The `rootRequiresFixup` logic should NOT trigger. + XCTAssertEqual( + metadata.serverUrl, + Self.account.davFilesUrl + "/photos", + "The serverUrl for a standard file should remain unchanged." + ) + XCTAssertEqual( + metadata.fileName, + "image.jpg", + "The fileName for a standard file should remain unchanged." + ) + } + + // MARK: - toDirectoryReadMetadatas(account:) Tests + + func testToDirectoryReadMetadatasHandlesRootAsTargetCorrectly() async { + // 1. Arrange: Create an array of NKFiles where the first item is the special root. + let rootNKFile = createNKFile( + ocId: "rootId", // This will be overridden by the logic + serverUrl: "..", + fileName: ".", + directory: false // Mimic NextcloudKit behavior + ) + let childNKFile = createNKFile( + ocId: "child1", + serverUrl: Self.account.davFilesUrl, + fileName: "document.txt" + ) + + let files = [rootNKFile, childNKFile] + + // 2. Act + let result = await files.toDirectoryReadMetadatas(account: Self.account) + + // 3. Assert + XCTAssertNotNil(result, "The conversion should succeed.") + guard let result else { return } + + // The logic should identify the first item as the root and fix its properties. + XCTAssertEqual( + result.directoryMetadata.ocId, + NSFileProviderItemIdentifier.rootContainer.rawValue, + "The target directory's ocId should be corrected to the root container identifier." + ) + XCTAssertTrue( + result.directoryMetadata.directory, + "The target directory should be correctly marked as a directory, even if the NKFile was not." + ) + + // Ensure the child item is processed correctly. + XCTAssertEqual(result.metadatas.count, 1, "There should be one child metadata object.") + XCTAssertEqual(result.metadatas.first?.ocId, "child1") + XCTAssertTrue(result.childDirectoriesMetadatas.isEmpty, "The child is a file, so child directories should be empty.") + } + + func testToDirectoryReadMetadatasHandlesNormalFolderAsTarget() async { + // 1. Arrange: Create an array for a normal folder and its children. + let parentFolderNKFile = createNKFile( + ocId: "folder1", + serverUrl: Self.account.davFilesUrl, + fileName: "MyFolder", + directory: true + ) + let childFileNKFile = createNKFile( + ocId: "file1", + serverUrl: Self.account.davFilesUrl + "/MyFolder", + fileName: "report.docx" + ) + let childDirNKFile = createNKFile( + ocId: "dir2", + serverUrl: Self.account.davFilesUrl + "/MyFolder", + fileName: "Subfolder", + directory: true + ) + + let files = [parentFolderNKFile, childFileNKFile, childDirNKFile] + + // 2. Act + let result = await files.toDirectoryReadMetadatas(account: Self.account) + + // 3. Assert + XCTAssertNotNil(result) + guard let result else { return } + + // The root fixup logic should NOT trigger for a normal folder. + XCTAssertEqual(result.directoryMetadata.ocId, "folder1") + XCTAssertEqual(result.directoryMetadata.fileName, "MyFolder") + XCTAssertTrue(result.directoryMetadata.directory) + + // Check children processing + XCTAssertEqual(result.metadatas.count, 2, "Should have two child metadata objects.") + XCTAssertEqual(result.childDirectoriesMetadatas.count, 1, "Should identify one child directory.") + XCTAssertEqual(result.childDirectoriesMetadatas.first?.ocId, "dir2") + } +} + From d66181728bba8d346ad642f95a88d9884f5d2e3f Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 16 Jun 2025 20:11:27 +0800 Subject: [PATCH 38/38] Move root correction to nkfile conversion Signed-off-by: Claudio Cambra --- .../Extensions/NKFile+Extensions.swift | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift index ba2ba5e1..9bae2503 100644 --- a/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift +++ b/Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift @@ -28,7 +28,13 @@ extension NKFile { : classFile // Support for finding the correct filename for e2ee files should go here + // Don't ask me why, NextcloudKit renames and moves the root folder details + // Also don't ask me why, but, NextcloudKit marks the NKFile for this as not a directory let rootRequiresFixup = serverUrl == ".." && fileName == "." + let ocId = rootRequiresFixup + ? NSFileProviderItemIdentifier.rootContainer.rawValue + : self.ocId + let directory = rootRequiresFixup ? true : self.directory let serverUrl = rootRequiresFixup ? urlBase + Account.webDavFilesUrlSuffix + userId : self.serverUrl @@ -126,14 +132,6 @@ extension Array { guard var targetDirectoryMetadata = first?.toItemMetadata() else { return nil } - // Don't ask me why, NextcloudKit renames and moves the root folder details - // Also don't ask me why, but, NextcloudKit marks the NKFile for this as not a directory - if (targetDirectoryMetadata.serverUrl == ".." && targetDirectoryMetadata.fileName == ".") || - (targetDirectoryMetadata.serverUrl + "/" + targetDirectoryMetadata.fileName == account.davFilesUrl + "/") - { - targetDirectoryMetadata.ocId = NSFileProviderItemIdentifier.rootContainer.rawValue - targetDirectoryMetadata.directory = true - } let conversionActor = DirectoryReadConversionActor(target: targetDirectoryMetadata) if self.count > 1 { await self[1...].concurrentChunkedForEach { file in