From df1faa368f3807c4fc99271d087fdb90df6cfd4b Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 31 Mar 2020 02:20:47 -0500 Subject: [PATCH] Refactored add feed code to be more reliable. --- Frameworks/Account/Account.swift | 13 +++ .../CloudKit/CloudKitAccountDelegate.swift | 5 +- .../CloudKit/CloudKitAccountZone.swift | 36 +++----- .../CloudKitAccountZoneDelegate.swift | 92 +++++++++---------- .../Account/CloudKit/CloudKitZone.swift | 4 +- 5 files changed, 70 insertions(+), 80 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 8e802625c..f05cd1855 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -518,6 +518,19 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return existingFolder(withExternalID: externalID) } + func existingContainers(withWebFeed webFeed: WebFeed) -> [Container] { + var containers = [Container]() + if topLevelWebFeeds.contains(webFeed) { + containers.append(self) + } + folders?.forEach { folder in + if folder.topLevelWebFeeds.contains(webFeed) { + containers.append(folder) + } + } + return containers + } + @discardableResult func ensureFolder(with name: String) -> Folder? { // TODO: support subfolders, maybe, some day diff --git a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift index 4c9959808..4b07d9cbf 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift @@ -157,7 +157,7 @@ final class CloudKitAccountDelegate: AccountDelegate { self.accountZone.createWebFeed(url: bestFeedSpecifier.urlString, editedName: name, container: container) { result in switch result { - case .success(let containerWebFeed): + case .success(let externalID): let feed = account.createWebFeed(with: nil, url: url.absoluteString, webFeedID: url.absoluteString, homePageURL: nil) @@ -168,8 +168,7 @@ final class CloudKitAccountDelegate: AccountDelegate { account.update(feed, with: parsedFeed, {_ in feed.editedName = name - feed.externalID = containerWebFeed.webFeedExternalID - feed.folderRelationship?[containerWebFeed.containerWebFeedExternalID] = containerWebFeed.containerExternalID + feed.externalID = externalID container.addWebFeed(feed) completion(.success(feed)) diff --git a/Frameworks/Account/CloudKit/CloudKitAccountZone.swift b/Frameworks/Account/CloudKit/CloudKitAccountZone.swift index d45ab3833..9f3db714a 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountZone.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountZone.swift @@ -31,6 +31,7 @@ final class CloudKitAccountZone: CloudKitZone { struct Fields { static let url = "url" static let editedName = "editedName" + static let containerExternalIDs = "folderExternalIDs" } } @@ -42,44 +43,29 @@ final class CloudKitAccountZone: CloudKitZone { } } - struct CloudKitContainerWebFeed { - static let recordType = "ContainerWebFeed" - struct Fields { - static let container = "container" - static let webFeed = "webFeed" - } - } - init(container: CKContainer) { self.container = container self.database = container.privateCloudDatabase } /// Persist a web feed record to iCloud and return the external key - func createWebFeed(url: String, editedName: String?, container: Container, completion: @escaping (Result) -> Void) { - let webFeedRecord = CKRecord(recordType: CloudKitWebFeed.recordType, recordID: generateRecordID()) - webFeedRecord[CloudKitWebFeed.Fields.url] = url + func createWebFeed(url: String, editedName: String?, container: Container, completion: @escaping (Result) -> Void) { + let record = CKRecord(recordType: CloudKitWebFeed.recordType, recordID: generateRecordID()) + record[CloudKitWebFeed.Fields.url] = url if let editedName = editedName { - webFeedRecord[CloudKitWebFeed.Fields.editedName] = editedName + record[CloudKitWebFeed.Fields.editedName] = editedName } guard let containerExternalID = container.externalID else { completion(.failure(CloudKitZoneError.invalidParameter)) return } - - let containerRecordID = CKRecord.ID(recordName: containerExternalID, zoneID: Self.zoneID) - let containerWebFeedRecord = CKRecord(recordType: CloudKitContainerWebFeed.recordType, recordID: generateRecordID()) - containerWebFeedRecord[CloudKitContainerWebFeed.Fields.container] = CKRecord.Reference(recordID: containerRecordID, action: .deleteSelf) - containerWebFeedRecord[CloudKitContainerWebFeed.Fields.webFeed] = CKRecord.Reference(record: webFeedRecord, action: .deleteSelf) + record[CloudKitWebFeed.Fields.containerExternalIDs] = [containerExternalID] - save([webFeedRecord, containerWebFeedRecord]) { result in + save(record) { result in switch result { case .success: - let cwf = ContainerWebFeed(webFeedExternalID: webFeedRecord.externalID, - containerWebFeedExternalID: containerWebFeedRecord.externalID, - containerExternalID: containerExternalID) - completion(.success(cwf)) + completion(.success(record.externalID)) case .failure(let error): completion(.failure(error)) } @@ -97,7 +83,7 @@ final class CloudKitAccountZone: CloudKitZone { let record = CKRecord(recordType: CloudKitWebFeed.recordType, recordID: recordID) record[CloudKitWebFeed.Fields.editedName] = editedName - save([record]) { result in + save(record) { result in switch result { case .success: completion(.success(())) @@ -140,7 +126,7 @@ final class CloudKitAccountZone: CloudKitZone { let record = CKRecord(recordType: CloudKitContainer.recordType, recordID: recordID) record[CloudKitContainer.Fields.name] = name - save([record]) { result in + save(record) { result in switch result { case .success: completion(.success(())) @@ -163,7 +149,7 @@ private extension CloudKitAccountZone { record[CloudKitContainer.Fields.name] = name record[CloudKitContainer.Fields.isAccount] = isAccount ? "true" : "false" - save([record]) { result in + save(record) { result in switch result { case .success: completion(.success(record.externalID)) diff --git a/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift index f100f30c6..62dd94f5f 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift @@ -13,9 +13,6 @@ import CloudKit class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { - private typealias UnclaimedWebFeed = (url: String, editedName: String?) - private var unclaimedWebFeeds = [String: UnclaimedWebFeed]() - private var log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "CloudKit") weak var account: Account? @@ -32,8 +29,6 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { addOrUpdateWebFeed(record) case CloudKitAccountZone.CloudKitContainer.recordType: addOrUpdateContainer(record) - case CloudKitAccountZone.CloudKitContainerWebFeed.recordType: - addOrUpdateContainerWebFeed(record) default: assertionFailure("Unknown record type: \(record.recordType)") } @@ -42,27 +37,33 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { func cloudKitDidDelete(recordType: CKRecord.RecordType, recordID: CKRecord.ID) { switch recordType { case CloudKitAccountZone.CloudKitWebFeed.recordType: - break + removeWebFeed(recordID.externalID) case CloudKitAccountZone.CloudKitContainer.recordType: removeContainer(recordID.externalID) - case CloudKitAccountZone.CloudKitContainerWebFeed.recordType: - removeContainerWebFeed(recordID.externalID) default: assertionFailure("Unknown record type: \(recordID.externalID)") } } func addOrUpdateWebFeed(_ record: CKRecord) { - guard let account = account else { return } + guard let account = account, + let urlString = record[CloudKitAccountZone.CloudKitWebFeed.Fields.url] as? String, + let containerExternalIDs = record[CloudKitAccountZone.CloudKitWebFeed.Fields.containerExternalIDs] as? [String], + let defaultContainerExternalID = containerExternalIDs.first, + let url = URL(string: urlString) else { return } let editedName = record[CloudKitAccountZone.CloudKitWebFeed.Fields.editedName] as? String if let webFeed = account.existingWebFeed(withExternalID: record.externalID) { - webFeed.editedName = editedName + updateWebFeed(webFeed, editedName: editedName, containerExternalIDs: containerExternalIDs) } else { - if let urlString = record[CloudKitAccountZone.CloudKitWebFeed.Fields.url] as? String { - unclaimedWebFeeds[record.externalID] = UnclaimedWebFeed(url: urlString, editedName: editedName) - } + addWebFeed(url: url, editedName: editedName, webFeedExternalID: record.externalID, containerExternalID: defaultContainerExternalID) + } + } + + func removeWebFeed(_ externalID: String) { + if let webFeed = account?.existingWebFeed(withExternalID: externalID) { + account?.removeWebFeed(webFeed) } } @@ -86,35 +87,41 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { } } - func addOrUpdateContainerWebFeed(_ record: CKRecord) { - guard let account = account, - let containerReference = record[CloudKitAccountZone.CloudKitContainerWebFeed.Fields.container] as? CKRecord.Reference, - let webFeedReference = record[CloudKitAccountZone.CloudKitContainerWebFeed.Fields.webFeed] as? CKRecord.Reference else { return } +} + +private extension CloudKitAcountZoneDelegate { + + func updateWebFeed(_ webFeed: WebFeed, editedName: String?, containerExternalIDs: [String]) { + guard let account = account else { return } + webFeed.editedName = editedName - let containerWebFeedExternalID = record.externalID - let containerExternalID = containerReference.recordID.externalID - let webFeedExternalID = webFeedReference.recordID.externalID + let existingContainers = account.existingContainers(withWebFeed: webFeed) + let existingContainerExternalIds = existingContainers.compactMap { $0.externalID } + + let diff = containerExternalIDs.difference(from: existingContainerExternalIds) - guard let container = account.existingContainer(withExternalID: containerExternalID) else { return } - - if let webFeed = account.existingWebFeed(withExternalID: webFeedExternalID) { - webFeed.folderRelationship?[containerWebFeedExternalID] = containerExternalID - container.addWebFeed(webFeed) - return + for change in diff { + switch change { + case .remove(_, let externalID, _): + if let container = existingContainers.first(where: { $0.externalID == externalID }) { + container.removeWebFeed(webFeed) + } + case .insert(_, let externalID, _): + if let container = existingContainers.first(where: { $0.externalID == externalID }) { + container.addWebFeed(webFeed) + } + } } + } + + func addWebFeed(url: URL, editedName: String?, webFeedExternalID: String, containerExternalID: String) { + guard let account = account, let container = account.existingContainer(withExternalID: containerExternalID) else { return } - guard let unclaimedWebFeed = unclaimedWebFeeds[webFeedExternalID] else { return } - unclaimedWebFeeds.removeValue(forKey: webFeedExternalID) - - let webFeed = account.createWebFeed(with: nil, url: unclaimedWebFeed.url, webFeedID: unclaimedWebFeed.url, homePageURL: nil) - webFeed.editedName = unclaimedWebFeed.editedName + let webFeed = account.createWebFeed(with: editedName, url: url.absoluteString, webFeedID: url.absoluteString, homePageURL: nil) + webFeed.editedName = editedName webFeed.externalID = webFeedExternalID - webFeed.folderRelationship = [String: String]() - webFeed.folderRelationship![containerWebFeedExternalID] = containerExternalID container.addWebFeed(webFeed) - guard let url = URL(string: unclaimedWebFeed.url) else { return } - refreshProgress?.addToNumberOfTasksAndRemaining(1) InitialFeedDownloader.download(url) { parsedFeed in self.refreshProgress?.completeTask() @@ -122,22 +129,7 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { account.update(webFeed, with: parsedFeed, {_ in }) } } - } - - func removeContainerWebFeed(_ containerWebFeedExternalID: String) { - guard let account = account, - let webFeed = account.flattenedWebFeeds().first(where: { $0.folderRelationship?.keys.contains(containerWebFeedExternalID) ?? false }), - let containerExternalId = webFeed.folderRelationship?[containerWebFeedExternalID] else { return } - - webFeed.folderRelationship?.removeValue(forKey: containerWebFeedExternalID) - guard account.externalID != containerExternalId else { - account.removeWebFeed(webFeed) - return - } - - guard let folder = account.existingFolder(withExternalID: containerExternalId) else { return } - folder.removeWebFeed(webFeed) } } diff --git a/Frameworks/Account/CloudKit/CloudKitZone.swift b/Frameworks/Account/CloudKit/CloudKitZone.swift index 1f0fa7cd9..cf998ad9c 100644 --- a/Frameworks/Account/CloudKit/CloudKitZone.swift +++ b/Frameworks/Account/CloudKit/CloudKitZone.swift @@ -96,8 +96,8 @@ extension CloudKitZone { } } - func save(_ records: [CKRecord], completion: @escaping (Result) -> Void) { - modify(recordsToSave: records, recordIDsToDelete: [], completion: completion) + func save(_ record: CKRecord, completion: @escaping (Result) -> Void) { + modify(recordsToSave: [record], recordIDsToDelete: [], completion: completion) } func delete(externalID: String?, completion: @escaping (Result) -> Void) {