diff --git a/Folders/Extensions/FileManager.swift b/Folders/Extensions/FileManager.swift index 27efa35..9a6cfb8 100644 --- a/Folders/Extensions/FileManager.swift +++ b/Folders/Extensions/FileManager.swift @@ -46,7 +46,8 @@ extension FileManager { print("Failed to determine content type for \(fileURL).") continue } - files.append(Details(ownerURL: ownerURL ?? directoryURL, + files.append(Details(uuid: UUID(), + ownerURL: ownerURL ?? directoryURL, url: fileURL, contentType: contentType, contentModificationDate: contentModificationDate.millisecondsSinceReferenceDate)) @@ -76,7 +77,8 @@ extension FileManager { throw FoldersError.general("Unable to get content modification date for file '\(url.path)'.") } - return Details(ownerURL: owner, + return Details(uuid: UUID(), + ownerURL: owner, url: url, contentType: isDirectory ? .directory : contentType, contentModificationDate: contentModificationDate.millisecondsSinceReferenceDate) diff --git a/Folders/Models/ApplicationModel.swift b/Folders/Models/ApplicationModel.swift index b375f30..5517a7b 100644 --- a/Folders/Models/ApplicationModel.swift +++ b/Folders/Models/ApplicationModel.swift @@ -149,15 +149,18 @@ class ApplicationModel: NSObject, ObservableObject { updater.stop() } + // Remove the sidebar entry. + sidebarItems.removeAll { $0.url == url } + // Remove the entires from the database. - do { - try store.removeBlocking(owner: url) - } catch { - // TODO: Better error handling. - print("Failed to remove files with error \(error).") + DispatchQueue.global(qos: .background).async { + do { + try self.store.removeBlocking(owner: url) + } catch { + // TODO: Better error handling. + print("Failed to remove files with error \(error).") + } } - - sidebarItems.removeAll { $0.url == url } } } @@ -201,16 +204,24 @@ extension ApplicationModel: StoreViewDelegate { return items } - func storeViewDidUpdate(_ storeView: StoreView) { - self.lookup = sidebarItems(for: storeView.files) + func storeView(_ storeView: StoreView, didUpdateFiles files: [Details]) { + assert(Set(files.map({ $0.url })).count == files.count) + self.lookup = sidebarItems(for: files) + } + + func storeView(_ storeView: StoreView, didInsertFile file: Details, atIndex: Int, files: [Details]) { + assert(Set(files.map({ $0.url })).count == files.count) + self.lookup = sidebarItems(for: files) } - func storeView(_ storeView: StoreView, didInsertFile file: Details, atIndex: Int) { - self.lookup = sidebarItems(for: storeView.files) + func storeView(_ storeView: StoreView, didUpdateFile file: Details, atIndex: Int, files: [Details]) { + assert(Set(files.map({ $0.url })).count == files.count) + self.lookup = sidebarItems(for: files) } - func storeView(_ storeView: StoreView, didRemoveFileWithIdentifier identifier: Details.Identifier, atIndex: Int) { - self.lookup = sidebarItems(for: storeView.files) + func storeView(_ storeView: StoreView, didRemoveFileWithIdentifier identifier: Details.Identifier, atIndex: Int, files: [Details]) { + assert(Set(files.map({ $0.url })).count == files.count) + self.lookup = sidebarItems(for: files) } } diff --git a/Folders/Models/Details.swift b/Folders/Models/Details.swift index 4cd8659..344f7b3 100644 --- a/Folders/Models/Details.swift +++ b/Folders/Models/Details.swift @@ -25,6 +25,7 @@ import UniformTypeIdentifiers struct Details: Hashable { + // TODO: This isn't really an identifier anymore is it. struct Identifier: Equatable, Hashable { let ownerURL: URL let url: URL @@ -36,6 +37,7 @@ struct Details: Hashable { let identifier: Identifier let ownerURL: URL + let uuid: UUID let url: URL let contentType: UTType @@ -44,7 +46,8 @@ struct Details: Hashable { // instead store the contentModificationDate as an Int which represents milliseconds since the reference data. let contentModificationDate: Int - init(ownerURL: URL, url: URL, contentType: UTType, contentModificationDate: Int) { + init(uuid: UUID, ownerURL: URL, url: URL, contentType: UTType, contentModificationDate: Int) { + self.uuid = uuid self.identifier = Identifier(ownerURL: ownerURL, url: url) self.ownerURL = ownerURL self.url = url @@ -57,10 +60,28 @@ struct Details: Hashable { } func setting(ownerURL: URL) -> Details { - return Details(ownerURL: ownerURL, + return Details(uuid: uuid, + ownerURL: ownerURL, url: url, contentType: contentType, contentModificationDate: contentModificationDate) } + func equivalent(to details: Details) -> Bool { + // TODO: Does the content type ever change? + // TODO: Function overload? + return (ownerURL == details.ownerURL && + url == details.url && + contentType == details.contentType && + contentModificationDate == details.contentModificationDate) + } + + func applying(details: Details) -> Details { + return Details(uuid: uuid, + ownerURL: ownerURL, + url: url, + contentType: contentType, + contentModificationDate: details.contentModificationDate) + } + } diff --git a/Folders/Models/SceneModel.swift b/Folders/Models/SceneModel.swift index 20aedac..c9dbc81 100644 --- a/Folders/Models/SceneModel.swift +++ b/Folders/Models/SceneModel.swift @@ -41,4 +41,9 @@ class SceneModel: ObservableObject { selection = sidebarItem.id } + func remove(_ url: URL) { + selection = nil + applicationModel.remove(url) + } + } diff --git a/Folders/Utilities/DirectoryScanner.swift b/Folders/Utilities/DirectoryScanner.swift index 5507e5a..c697246 100644 --- a/Folders/Utilities/DirectoryScanner.swift +++ b/Folders/Utilities/DirectoryScanner.swift @@ -37,7 +37,7 @@ class DirectoryScanner { let url: URL let workQueue = DispatchQueue(label: "workQueue") var stream: FSEventStream? = nil - var identifiers: Set = [] // Synchronized on workQueue + var identifiers = [Details.Identifier: Details]() // Synchronized on workQueue weak var delegate: DirectoryScannerDelegate? init(url: URL) { @@ -46,6 +46,7 @@ class DirectoryScanner { func start(load: @escaping () -> Set
, onFileCreation: @escaping (any Collection
) -> Void, + onFileUpdate: @escaping (any Collection
) -> Void, onFileDeletion: @escaping (any Collection) -> Void) { // TODO: Allow this to be run with a blocking startup. dispatchPrecondition(condition: .notOnQueue(workQueue)) @@ -73,14 +74,14 @@ class DirectoryScanner { // Depending on the system load, it seems like we sometimes receive events for file operations that // were already captured in our initial snapshot that we want to ignore. - guard !self.identifiers.contains(details.identifier) else { + guard self.identifiers[details.identifier] == nil else { return } print("File created at path '\(path)'") onFileCreation([details]) - self.identifiers.insert(details.identifier) + self.identifiers[details.identifier] = details case .itemRenamed(path: let path, itemType: let itemType, eventId: _, fromUs: _): @@ -94,9 +95,13 @@ class DirectoryScanner { // If a file exists at the new path and also exists in our runtime cache of files then we infer // that this rename actuall represents a content modification operation; our file has been // atomically replaced by a new file containing new content. - if self.identifiers.contains(details.identifier) { + if let old = self.identifiers[details.identifier] { print("File updated by rename '\(url)'") - onFileDeletion([details.identifier]) +// onFileDeletion([details.identifier]) + let update = old.applying(details: details) + self.identifiers[details.identifier] = update + onFileUpdate([update]) + return // TODO: We should ensure we delete all our children if we're a directory. } else { print("File added by rename '\(url)'") @@ -106,10 +111,12 @@ class DirectoryScanner { if itemType == .dir { let files = try fileManager.files(directoryURL: url, ownerURL: ownerURL) onFileCreation(files) - self.identifiers.formUnion(files.map({ $0.identifier })) + for file in files { + self.identifiers[file.identifier] = file + } } else { onFileCreation([details]) - self.identifiers.insert(details.identifier) + self.identifiers[details.identifier] = details } } else { @@ -118,12 +125,14 @@ class DirectoryScanner { // If it's a directory, then we need to work out what files are being removed. let identifier = Details.Identifier(ownerURL: ownerURL, url: url) if itemType == .dir { - let identifiers = self.identifiers.filter { $0.url.path.hasPrefix(url.path + "/") } + [identifier] + let identifiers = self.identifiers.keys.filter { $0.url.path.hasPrefix(url.path + "/") } + [identifier] onFileDeletion(Array(identifiers)) - self.identifiers.subtract(identifiers) + for identifier in identifiers { + self.identifiers.removeValue(forKey: identifier) + } } else { onFileDeletion([identifier]) - self.identifiers.remove(identifier) + self.identifiers.removeValue(forKey: identifier) } } @@ -133,7 +142,7 @@ class DirectoryScanner { print("File removed '\(url)'") let identifier = Details.Identifier(ownerURL: ownerURL, url: url) onFileDeletion([identifier]) - self.identifiers.remove(identifier) + self.identifiers.removeValue(forKey: identifier) case .itemInodeMetadataModified(path: let path, itemType: let itemType, eventId: _, fromUs: _): @@ -142,18 +151,19 @@ class DirectoryScanner { // TODO: Consider generalising this code. let url = URL(filePath: path, itemType: itemType) let identifier = Details.Identifier(ownerURL: ownerURL, url: url) + let details = try fileManager.details(for: url, owner: ownerURL) // Remove the file if it exists in our set. - if self.identifiers.contains(identifier) { - onFileDeletion([identifier]) + if let old = self.identifiers[identifier] { + let new = old.applying(details: details) + onFileUpdate([new]) + self.identifiers[identifier] = new + return } // Create a new identifier corresponding to the udpated file. - let details = try fileManager.details(for: url, owner: ownerURL) // TODO: details(for identifier: Details.Identifier)? onFileCreation([details]) - - // Ensure there's an entry for the (potentially) new file. - self.identifiers.insert(identifier) + self.identifiers[identifier] = details default: print("Unhandled file event \(event).") @@ -180,34 +190,65 @@ class DirectoryScanner { // TODO: Handle errors. let fileManager = FileManager.default - let files = Set(try! fileManager.files(directoryURL: url)) - let currentState = load() + // TODO: Index by URL not Identifier - // Add just the new files. - let created = files.subtracting(currentState) - if created.count > 0 { - print("Inserting \(created.count) new files...") - onFileCreation(created) + // Load the snapshot. + let snapshot = load() + // TODO: Rename + let snapshotIdentifiers = snapshot.reduce(into: [Details.Identifier: Details]()) { partialResult, details in + partialResult[details.identifier] = details } - // Remove the remaining files. - let deleted = currentState.subtracting(files) - .map { $0.identifier } - if deleted.count > 0 { - print("Removing \(deleted.count) deleted files...") - onFileDeletion(deleted) + // Load the current state from the file system. + let current = Set(try! fileManager.files(directoryURL: url)) + let currentIdentifiers = current.reduce(into: [Details.Identifier: Details]()) { partialResult, details in + partialResult[details.identifier] = details } - // Cache the initial state. - self.identifiers = files - .map { - return $0.identifier - } - .reduce(into: Set()) { partialResult, identifier in - partialResult.insert(identifier) + // Determine the deleted files and apply the changes. + let deletedIdentifiers = Set(snapshotIdentifiers.keys).subtracting(currentIdentifiers.keys) + if deletedIdentifiers.count > 0 { + print("Removing \(deletedIdentifiers.count) deleted files...") + onFileDeletion(deletedIdentifiers) + } + + // Walk the current files, determine the operation required to update the snapshot, and assemble the + // in-memory state. + var additions = [Details]() + var updates = [Details]() + var state = [Details.Identifier: Details]() + + // TODO: Can and should we track deletions here too? + for file in current { + if let snapshot = snapshotIdentifiers[file.identifier] { + if !snapshot.equivalent(to: file) { // Modified. + let update = snapshot.applying(details: file) + updates.append(update) + state[file.identifier] = update + } else { // Unchanged. + state[file.identifier] = snapshot + } + } else { // Created. + additions.append(file) + state[file.identifier] = file } + } + + // Add the new files. + if additions.count > 0 { + print("Inserting \(additions.count) new files...") + onFileCreation(additions) + } + // Apply the updates. + if updates.count > 0 { + print("Updating \(updates.count) modified files...") + onFileUpdate(updates) + } + + // Cache the initial state. + self.identifiers = state self.delegate?.directoryScannerDidStart(self) } diff --git a/Folders/Utilities/Store.swift b/Folders/Utilities/Store.swift index 5c6bcdf..afe8acf 100644 --- a/Folders/Utilities/Store.swift +++ b/Folders/Utilities/Store.swift @@ -28,15 +28,19 @@ import SQLite protocol StoreObserver: NSObject { func store(_ store: Store, didInsertFiles files: [Details]) + func store(_ store: Store, didUpdateFiles files: [Details]) func store(_ store: Store, didRemoveFilesWithIdentifiers identifiers: [Details.Identifier]) } +// TODO: Notifying observers in a transaction seems incredibly inefficient. + class Store { struct Schema { static let files = Table("files") static let id = Expression("id") + static let uuid = Expression("uuid") static let owner = Expression("owner") static let path = Expression("path") static let name = Expression("name") @@ -44,19 +48,21 @@ class Store { static let modificationDate = Expression("modification_date") } - static let majorVersion = 47 + static let majorVersion = 49 var observers: [StoreObserver] = [] let databaseURL: URL let syncQueue = DispatchQueue(label: "Store.syncQueue") let connection: Connection + let observerLock = NSRecursiveLock() static var migrations: [Int32: (Connection) throws -> Void] = [ 1: { connection in print("create the files table...") try connection.run(Schema.files.create(ifNotExists: true) { t in - t.column(Schema.id, primaryKey: true) + t.column(Schema.id, primaryKey: true) // TODO: I could maybe drop this for the uuid? + t.column(Schema.uuid) t.column(Schema.owner) t.column(Schema.path) t.column(Schema.name) @@ -78,39 +84,24 @@ class Store { } func add(observer: StoreObserver) { - dispatchPrecondition(condition: .notOnQueue(syncQueue)) - syncQueue.sync { - observers.append(observer) + observerLock.withLock { + self.observers.append(observer) } } - // TODO: This feels janky. It might be a cleaner API to return an 'observer' instead. func remove(observer: StoreObserver) { - dispatchPrecondition(condition: .notOnQueue(syncQueue)) - syncQueue.sync { - // TODO: This might be insufficient unless we use some kind of thread-safe cancel operation. + // Since we guarantee that we only ever notify our delegates while holding the observer lock, we can guarantee + // that when we exit from this function, `observer` will never receive another callback. + observerLock.withLock { observers.removeAll { $0.isEqual(observer) } } } - private func run(perform: @escaping () throws -> T) async throws -> T { - return try await withCheckedThrowingContinuation { continuation in - syncQueue.async { - let result = Swift.Result { - try Task.checkCancellation() - return try perform() - } - continuation.resume(with: result) - } - } - } - - // TODO: This can't be cancelled? private func runBlocking(perform: @escaping () throws -> T) throws -> T { + dispatchPrecondition(condition: .notOnQueue(.main)) dispatchPrecondition(condition: .notOnQueue(syncQueue)) - // TODO: IS THIS QUEUE REALLY NEEDED? var result: Swift.Result? = nil - syncQueue.sync { + syncQueue.sync { // TODO: Is the syncQueue necessary? result = Swift.Result { return try perform() } @@ -158,6 +149,7 @@ class Store { // If it does not, we insert it. try connection.run(Schema.files.insert(or: .fail, + Schema.uuid <- file.uuid, Schema.owner <- file.ownerURL.path, Schema.path <- file.url.path, Schema.name <- file.url.displayName, @@ -167,12 +159,39 @@ class Store { // Track the inserted files to notify our observers. insertions.append(file) } - for observer in self.observers { - DispatchQueue.global(qos: .default).async { - observer.store(self, didInsertFiles: insertions) + self.observerLock.withLock { + for observer in self.observers { + DispatchQueue.global(qos: .default).async { + observer.store(self, didInsertFiles: insertions) + } } } + } + } + } + func updateBlocking(files: any Collection
) throws { + return try runBlocking { [connection] in + try connection.transaction { + var updates = [Details]() + for file in files { + let row = Schema.files.filter(Schema.uuid == file.uuid) + let count = try connection.run(row.update(Schema.owner <- file.ownerURL.path, + Schema.path <- file.url.path, + Schema.name <- file.url.displayName, + Schema.type <- file.contentType.identifier, + Schema.modificationDate <- file.contentModificationDate)) + if count > 0 { + updates.append(file) + } + } + self.observerLock.withLock { + for observer in self.observers { + DispatchQueue.global(qos: .default).async { + observer.store(self, didUpdateFiles: updates) + } + } + } } } } @@ -193,9 +212,11 @@ class Store { guard removals.count > 0 else { return } - for observer in self.observers { - DispatchQueue.global(qos: .default).async { - observer.store(self, didRemoveFilesWithIdentifiers: removals) + self.observerLock.withLock { + for observer in self.observers { + DispatchQueue.global(qos: .default).async { + observer.store(self, didRemoveFilesWithIdentifiers: removals) + } } } } @@ -204,15 +225,23 @@ class Store { func removeBlocking(owner: URL) throws { return try runBlocking { [connection] in - let count = try connection.run(Schema.files.filter(Schema.owner == owner.path).delete()) - print("Deleted \(count) rows.") - // TODO: Consider notifying our clients (though this is probably unnecessary and noisy). + try connection.transaction { + let files = try self.syncQueue_files(filter: .owner(owner), sort: .displayNameAscending) + try connection.run(Schema.files.filter(Schema.owner == owner.path).delete()) + self.observerLock.withLock { + for observer in self.observers { + DispatchQueue.global(qos: .default).async { + observer.store(self, didRemoveFilesWithIdentifiers: files.map({ $0.identifier })) + } + } + } + } } } func syncQueue_files(filter: Filter, sort: Sort) throws -> [Details] { dispatchPrecondition(condition: .onQueue(syncQueue)) - return try connection.prepareRowIterator(Schema.files.select(Schema.owner, Schema.path, Schema.type, Schema.modificationDate) + return try connection.prepareRowIterator(Schema.files.select(Schema.files[*]) .filter(filter.filter) .order(sort.order)) .map { row in @@ -221,7 +250,12 @@ class Store { let url = URL(filePath: row[Schema.path], directoryHint: type.conforms(to: .directory) ? .isDirectory : .notDirectory) let modificationDate = row[Schema.modificationDate] - return Details(ownerURL: ownerURL, url: url, contentType: type, contentModificationDate: modificationDate) + let uuid = row[Schema.uuid] + return Details(uuid: uuid, + ownerURL: ownerURL, + url: url, + contentType: type, + contentModificationDate: modificationDate) } } @@ -231,10 +265,4 @@ class Store { } } - func files(filter: Filter, sort: Sort) async throws -> [Details] { - return try await run { - return try self.syncQueue_files(filter: filter, sort: sort) - } - } - } diff --git a/Folders/Utilities/StorePublisher.swift b/Folders/Utilities/StorePublisher.swift index a210e97..cda51ac 100644 --- a/Folders/Utilities/StorePublisher.swift +++ b/Folders/Utilities/StorePublisher.swift @@ -51,6 +51,10 @@ struct StorePublisher: Publisher { _ = target?.receive(.add(files)) } + func store(_ store: Store, didUpdateFiles files: [Details]) { + _ = target?.receive(.add(files)) + } + func store(_ store: Store, didRemoveFilesWithIdentifiers identifiers: [Details.Identifier]) { _ = target?.receive(.remove(identifiers)) } diff --git a/Folders/Utilities/StoreUpdater.swift b/Folders/Utilities/StoreUpdater.swift index 234a8a7..69cb583 100644 --- a/Folders/Utilities/StoreUpdater.swift +++ b/Folders/Utilities/StoreUpdater.swift @@ -50,6 +50,12 @@ class StoreUpdater { } catch { print("Failed to perform creation update with error \(error).") } + } onFileUpdate: { files in + do { + try self.store.updateBlocking(files: files) + } catch { + print("Failed to perform update update with error \(error).") + } } onFileDeletion: { identifiers in do { try self.store.removeBlocking(identifiers: identifiers) diff --git a/Folders/Utilities/StoreView.swift b/Folders/Utilities/StoreView.swift index 72fc3db..7082e05 100644 --- a/Folders/Utilities/StoreView.swift +++ b/Folders/Utilities/StoreView.swift @@ -27,22 +27,25 @@ import Algorithms protocol StoreViewDelegate: NSObject { - func storeViewDidUpdate(_ storeView: StoreView) - // TODO: Expose the details directly in the update. - func storeView(_ storeView: StoreView, didInsertFile file: Details, atIndex: Int) - func storeView(_ storeView: StoreView, didRemoveFileWithIdentifier identifier: Details.Identifier, atIndex: Int) + func storeView(_ storeView: StoreView, didUpdateFiles files: [Details]) + func storeView(_ storeView: StoreView, didInsertFile file: Details, atIndex: Int, files: [Details]) // TODO: atIndex index + func storeView(_ storeView: StoreView, didUpdateFile file: Details, atIndex: Int, files: [Details]) + func storeView(_ storeView: StoreView, didRemoveFileWithIdentifier identifier: Details.Identifier, atIndex: Int, files: [Details]) } class StoreView: NSObject, StoreObserver { let store: Store - let workQueue = DispatchQueue(label: "StoreView.workQueue") + let workQueue = DispatchQueue(label: "StoreView.workQueue", qos: .userInteractive) let filter: Filter let sort: Sort let threshold: Int + private var isRunning: Bool = false // Synchronized on workQueue + private var files: [Details] = [] // Synchronized on workQueue - var files: [Details] = [] + // TODO: We _could_ consider using a thread to guard against mutation to `files` instead of the queue. + // TODO: It might be nice to have a targetQueue for the delegate to make the serialisation explicit. weak var delegate: StoreViewDelegate? = nil @@ -55,23 +58,25 @@ class StoreView: NSObject, StoreObserver { } func start() { - Task { + workQueue.async { do { + precondition(self.isRunning == false) + self.isRunning = true // Start observing the database. - store.add(observer: self) + // TODO: Maybe this takes workQueue? + self.store.add(observer: self) // Get them out sorted. let queryStart = Date() let queryDuration = queryStart.distance(to: Date()) - let sortedFiles = try await store.files(filter: filter, sort: sort) - print("Query took \(queryDuration.formatted()) seconds and returned \(sortedFiles.count) files.") + self.files = try self.store.filesBlocking(filter: self.filter, sort: self.sort) + print("Query took \(queryDuration.formatted()) seconds and returned \(self.files.count) files.") + let snapshot = self.files DispatchQueue.main.async { [self] in - self.files = sortedFiles - self.delegate?.storeViewDidUpdate(self) + self.delegate?.storeView(self, didUpdateFiles: snapshot) } - } catch { // TODO: Provide a delegate model that actually returns errors. print("Failed to scan for files with error \(error).") @@ -81,33 +86,94 @@ class StoreView: NSObject, StoreObserver { func stop() { store.remove(observer: self) + // TODO: Clean up? } func store(_ store: Store, didInsertFiles files: [Details]) { dispatchPrecondition(condition: .notOnQueue(.main)) - DispatchQueue.main.async { + workQueue.async { + guard self.isRunning else { + return + } + + // Ignore unrelated updates and updates that are already in our view. + // This can occur because there's a period of time during which we are subscribed but have yet to fetch + // the data in the database. In this scenario it's possible to receive additions that we then get back in + // our database query. + // TODO: Using a flat array to store our files isn't very efficient for this kind of lookup. + // TODO: It's very slightly possible this could be an update? + let files = files.filter { self.filter.matches(details: $0) && !self.files.contains($0) } + guard files.count > 0 else { + return + } + + // These sets should never intersect. + assert(Set(self.files.map { $0.url }).intersection(files.map { $0.url }).count == 0) + if files.count < self.threshold { for file in files { - guard self.filter.matches(details: file) else { - continue - } let index = self.files.partitioningIndex { return self.sort.compare(file, $0) } self.files.insert(file, at: index) - self.delegate?.storeView(self, didInsertFile: file, atIndex: index) + let snapshot = self.files + DispatchQueue.main.async { + self.delegate?.storeView(self, didInsertFile: file, atIndex: index, files: snapshot) + } } } else { + // TODO: There's an optimisation here if the list is empty. for file in files { - guard self.filter.matches(details: file) else { - continue - } let index = self.files.partitioningIndex { return self.sort.compare(file, $0) } self.files.insert(file, at: index) } - self.delegate?.storeViewDidUpdate(self) + // TODO: We might as well cascade these changes down to the table view to allow it decide on performance + // characteristics. + let snapshot = self.files + DispatchQueue.main.async { + self.delegate?.storeView(self, didUpdateFiles: snapshot) + } + } + } + } + + func store(_ store: Store, didUpdateFiles files: [Details]) { + dispatchPrecondition(condition: .notOnQueue(.main)) + workQueue.async { + guard self.isRunning else { + return + } + + // Ignore unrelated updates. + let files = files.filter { self.filter.matches(details: $0) } + guard files.count > 0 else { + return + } + + // TODO: We have a race condition during startup whereby we can sometimes receive updates as we're querying + // and before we've received the full dataset. + var indexes = [(Details, Int)]() + for file in files { + let index = self.files.firstIndex { $0.uuid == file.uuid }! + self.files[index] = file + indexes.append((file, index)) + } + // TODO: Actual update API. + + if indexes.count < self.threshold { + let snapshot = self.files + for (file, index) in indexes { + DispatchQueue.main.async { + self.delegate?.storeView(self, didUpdateFile: file, atIndex: index, files: snapshot) + } + } + } else { + let snapshot = self.files + DispatchQueue.main.async { + self.delegate?.storeView(self, didUpdateFiles: snapshot) + } } } } @@ -115,20 +181,37 @@ class StoreView: NSObject, StoreObserver { func store(_ store: Store, didRemoveFilesWithIdentifiers identifiers: [Details.Identifier]) { dispatchPrecondition(condition: .notOnQueue(.main)) // TODO: Maybe this shouldn't live on main queue - DispatchQueue.main.async { + // TODO: Pre-filter the updates. + workQueue.async { + guard self.isRunning else { + return + } + + // TODO: Pre-filter the identifiers here if we can? Or somehow work out which intersect before deciding + // how to notify our delegate. + if identifiers.count < self.threshold { for identifier in identifiers { guard let index = self.files.firstIndex(where: { $0.identifier == identifier }) else { continue } self.files.remove(at: index) - self.delegate?.storeView(self, didRemoveFileWithIdentifier: identifier, atIndex: index) + let snapshot = self.files + DispatchQueue.main.async { + self.delegate?.storeView(self, didRemoveFileWithIdentifier: identifier, atIndex: index, files: snapshot) + } } } else { for identifier in identifiers { - _ = self.files.firstIndex(where: { $0.identifier == identifier }) + guard let index = self.files.firstIndex(where: { $0.identifier == identifier }) else { + continue + } + self.files.remove(at: index) + } + let snapshot = self.files + DispatchQueue.main.async { + self.delegate?.storeView(self, didUpdateFiles: snapshot) } - self.delegate?.storeViewDidUpdate(self) } } } diff --git a/Folders/Views/GridView.swift b/Folders/Views/GridView.swift index 757875b..f1dc4b5 100644 --- a/Folders/Views/GridView.swift +++ b/Folders/Views/GridView.swift @@ -127,6 +127,10 @@ class InnerGridView: NSView { fatalError("init(coder:) has not been implemented") } + deinit { + storeView.stop() // TODO: This is currently necessary to ensure the observers are removed. + } + override func acceptsPreviewPanelControl(_ panel: QLPreviewPanel!) -> Bool { return true } @@ -173,15 +177,15 @@ extension InnerGridView: QLPreviewPanelDataSource, QLPreviewPanelDelegate { extension InnerGridView: StoreViewDelegate { - func storeViewDidUpdate(_ storeView: StoreView) { + func storeView(_ storeView: StoreView, didUpdateFiles files: [Details]) { var snapshot = Snapshot() snapshot.appendSections([.none]) - snapshot.appendItems(storeView.files.map({ $0.identifier }), toSection: Section.none) + snapshot.appendItems(files.map({ $0.identifier }), toSection: Section.none) dataSource.apply(snapshot, animatingDifferences: false) } // TODO: Insert details. - func storeView(_ storeView: StoreView, didInsertFile file: Details, atIndex index: Int) { + func storeView(_ storeView: StoreView, didInsertFile file: Details, atIndex index: Int, files: [Details]) { // TODO: Insert in the correct place. // TODO: We may need to rate-limit these updates. var snapshot = dataSource.snapshot() @@ -190,17 +194,28 @@ extension InnerGridView: StoreViewDelegate { snapshot.appendSections([.none]) } - if index >= snapshot.itemIdentifiers.count { - snapshot.appendItems([file.identifier]) - } else { + if index < snapshot.itemIdentifiers.count { let beforeItem = snapshot.itemIdentifiers[index] snapshot.insertItems([file.identifier], beforeItem: beforeItem) + } else if index == snapshot.itemIdentifiers.count { + snapshot.appendItems([file.identifier]) + } else { + fatalError("Attempting to insert an item beyond the end of the list.") + } dataSource.apply(snapshot, animatingDifferences: true) } - func storeView(_ storeView: StoreView, didRemoveFileWithIdentifier identifier: Details.Identifier, atIndex: Int) { + func storeView(_ storeView: StoreView, didUpdateFile file: Details, atIndex: Int, files: [Details]) { + guard let indexPath = dataSource.indexPath(for: file.identifier), + let cell = collectionView.item(at: indexPath) as? ShortcutItemView else { + return + } + cell.configure(url: file.url) + } + + func storeView(_ storeView: StoreView, didRemoveFileWithIdentifier identifier: Details.Identifier, atIndex: Int, files: [Details]) { var snapshot = dataSource.snapshot() snapshot.deleteItems([identifier]) dataSource.apply(snapshot, animatingDifferences: true) diff --git a/Folders/Views/Sidebar.swift b/Folders/Views/Sidebar.swift index f1598f4..25960d1 100644 --- a/Folders/Views/Sidebar.swift +++ b/Folders/Views/Sidebar.swift @@ -39,7 +39,7 @@ struct Sidebar: View { if item.kind == .owner { Divider() Button(role: .destructive) { - applicationModel.remove(item.url) + sceneModel.remove(item.url) } label: { Label("Remove", systemImage: "trash") } diff --git a/FoldersTests/DirectoryScannerTests.swift b/FoldersTests/DirectoryScannerTests.swift index 7970ebb..5b86e66 100644 --- a/FoldersTests/DirectoryScannerTests.swift +++ b/FoldersTests/DirectoryScannerTests.swift @@ -35,6 +35,8 @@ final class DirectoryScannerTests: XCTestCase { XCTAssertEqual(files.first?.url, url) XCTAssertEqual(files.first?.contentType, .plainText) expectation.fulfill() + } onFileUpdate: { files in + XCTFail("Unexpected deletion event") } onFileDeletion: { identifiers in XCTFail("Unexpected deletion event") } diff --git a/FoldersTests/Extensions/XCTestCase.swift b/FoldersTests/Extensions/XCTestCase.swift index bb20d1a..9a10892 100644 --- a/FoldersTests/Extensions/XCTestCase.swift +++ b/FoldersTests/Extensions/XCTestCase.swift @@ -63,6 +63,7 @@ extension XCTestCase { func scan(directoryURL: URL, onFileCreation: @escaping (any Collection
) -> Void, + onFileUpdate: @escaping (any Collection
) -> Void, onFileDeletion: @escaping (any Collection) -> Void) throws -> DirectoryScanner { let scanner = DirectoryScanner(url: directoryURL) let delegate = DirectoryScannerTestDelegate() @@ -75,6 +76,9 @@ extension XCTestCase { } onFileCreation: { files in print("DirectoryScanner onFileCreation:\n\(files.map({ " \($0.url.debugRepresentation)" }).joined(separator: "\n"))") onFileCreation(files) + } onFileUpdate: { files in + print("DirectoryScanner onFileUpdate:\n\(files.map({ " \($0.url.debugRepresentation)" }).joined(separator: "\n"))") + onFileUpdate(files) } onFileDeletion: { identifiers in print("DirectoryScanner onFileDeletion:\n\(identifiers.map({ " \($0.url.debugRepresentation)" }).joined(separator: "\n"))") onFileDeletion(identifiers)