From 4f425c9c866e07d4102ef287a0c747b73ab8f283 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Sun, 29 Mar 2020 17:12:34 -0500 Subject: [PATCH] Implement web feed sync between devices. --- .../CloudKit/CKRecord+Extensions.swift | 8 ++ .../CloudKit/CloudKitAccountDelegate.swift | 29 ++++-- .../CloudKit/CloudKitAccountZone.swift | 13 ++- .../CloudKitAccountZoneDelegate.swift | 50 +++++++++- .../Account/CloudKit/CloudKitZone.swift | 92 ++++++++++--------- .../AccountsAddCloudKitWindowController.swift | 3 +- .../CloudKitAccountViewController.swift | 3 +- iOS/Settings/AddAccountViewController.swift | 2 +- 8 files changed, 138 insertions(+), 62 deletions(-) diff --git a/Frameworks/Account/CloudKit/CKRecord+Extensions.swift b/Frameworks/Account/CloudKit/CKRecord+Extensions.swift index bb1696a89..fc97d2dd7 100644 --- a/Frameworks/Account/CloudKit/CKRecord+Extensions.swift +++ b/Frameworks/Account/CloudKit/CKRecord+Extensions.swift @@ -9,6 +9,14 @@ import Foundation import CloudKit +extension CKRecord { + + var externalID: String { + return recordID.externalID + } + +} + extension CKRecord.ID { var externalID: String { diff --git a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift index 815b14c89..1029c2539 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift @@ -49,12 +49,20 @@ final class CloudKitAccountDelegate: AccountDelegate { accountZone = CloudKitAccountZone(container: container) let databaseFilePath = (dataFolder as NSString).appendingPathComponent("Sync.sqlite3") database = SyncDatabase(databaseFilePath: databaseFilePath) + accountZone.refreshProgress = refreshProgress } func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { - refresher.refreshFeeds(account.flattenedWebFeeds()) { - account.metadata.lastArticleFetchEndTime = Date() - completion(.success(())) + accountZone.fetchChangesInZone() { result in + switch result { + case .success: + self.refresher.refreshFeeds(account.flattenedWebFeeds()) { + account.metadata.lastArticleFetchEndTime = Date() + completion(.success(())) + } + case .failure(let error): + completion(.failure(error)) + } } } @@ -119,11 +127,10 @@ final class CloudKitAccountDelegate: AccountDelegate { switch result { case .success(let feedSpecifiers): - guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers), - let url = URL(string: bestFeedSpecifier.urlString) else { - self.refreshProgress.completeTask() - completion(.failure(AccountError.createErrorNotFound)) - return + guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers), let url = URL(string: bestFeedSpecifier.urlString) else { + self.refreshProgress.completeTask() + completion(.failure(AccountError.createErrorNotFound)) + return } if account.hasWebFeed(withURL: bestFeedSpecifier.urlString) { @@ -132,7 +139,7 @@ final class CloudKitAccountDelegate: AccountDelegate { return } - self.accountZone.createWebFeed(url: urlString, editedName: name) { result in + self.accountZone.createWebFeed(url: bestFeedSpecifier.urlString, editedName: name) { result in switch result { case .success(let externalID): @@ -231,10 +238,12 @@ final class CloudKitAccountDelegate: AccountDelegate { } func accountDidInitialize(_ account: Account) { - accountZone.delegate = CloudKitAcountZoneDelegate(account: account) + accountZone.delegate = CloudKitAcountZoneDelegate(account: account, refreshProgress: refreshProgress) + accountZone.resumeLongLivedOperationIfPossible() } func accountWillBeDeleted(_ account: Account) { + accountZone.resetChangeToken() } static func validateCredentials(transport: Transport, credentials: Credentials, endpoint: URL? = nil, completion: (Result) -> Void) { diff --git a/Frameworks/Account/CloudKit/CloudKitAccountZone.swift b/Frameworks/Account/CloudKit/CloudKitAccountZone.swift index 670b2c556..6fbff3139 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountZone.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountZone.swift @@ -7,6 +7,8 @@ // import Foundation +import os.log +import RSWeb import CloudKit final class CloudKitAccountZone: CloudKitZone { @@ -15,8 +17,11 @@ final class CloudKitAccountZone: CloudKitZone { return CKRecordZone.ID(zoneName: "Account", ownerName: CKCurrentUserDefaultName) } - let container: CKContainer - let database: CKDatabase + var log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "CloudKit") + + weak var container: CKContainer? + weak var database: CKDatabase? + weak var refreshProgress: DownloadProgress? var delegate: CloudKitZoneDelegate? = nil struct CloudKitWebFeed { @@ -27,7 +32,7 @@ final class CloudKitAccountZone: CloudKitZone { } } - init(container: CKContainer) { + init(container: CKContainer) { self.container = container self.database = container.privateCloudDatabase } @@ -43,7 +48,7 @@ final class CloudKitAccountZone: CloudKitZone { save(record: record) { result in switch result { case .success: - completion(.success(record.recordID.externalID)) + completion(.success(record.externalID)) case .failure(let error): completion(.failure(error)) } diff --git a/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift index f2194820c..7ba75fa8b 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift @@ -7,20 +7,26 @@ // import Foundation +import os.log +import RSWeb import CloudKit class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { + private var log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "CloudKit") + weak var account: Account? + weak var refreshProgress: DownloadProgress? - init(account: Account) { + init(account: Account, refreshProgress: DownloadProgress) { self.account = account + self.refreshProgress = refreshProgress } func cloudKitDidChange(record: CKRecord) { switch record.recordType { case CloudKitAccountZone.CloudKitWebFeed.recordType: - addWebFeed(record) + addOrUpdateWebFeed(record) default: assertionFailure("Unknown record type: \(record.recordType)") } @@ -35,12 +41,48 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { } } - func addWebFeed(_ record: CKRecord) { + func addOrUpdateWebFeed(_ record: CKRecord) { + guard let account = account else { return } + let editedName = record[CloudKitAccountZone.CloudKitWebFeed.Fields.editedName] as? String + + if let webFeed = account.existingWebFeed(withExternalID: record.externalID) { + webFeed.editedName = editedName + } else { + if let urlString = record[CloudKitAccountZone.CloudKitWebFeed.Fields.url] as? String, let url = URL(string: urlString) { + downloadAndAddWebFeed(url: url, editedName: editedName, externalID: record.externalID) + } else { + os_log(.error, log: self.log, "Failed to add or update web feed.") + } + } } func removeWebFeed(_ externalID: String) { - + if let webFeed = account?.existingWebFeed(withExternalID: externalID) { + account?.removeWebFeed(webFeed) + } + } + +} + +private extension CloudKitAcountZoneDelegate { + + func downloadAndAddWebFeed(url: URL, editedName: String?, externalID: String) { + guard let account = account else { return } + + let webFeed = account.createWebFeed(with: editedName, url: url.absoluteString, webFeedID: url.absoluteString, homePageURL: nil) + webFeed.editedName = editedName + webFeed.externalID = externalID + account.addWebFeed(webFeed) + + refreshProgress?.addToNumberOfTasksAndRemaining(1) + InitialFeedDownloader.download(url) { parsedFeed in + self.refreshProgress?.completeTask() + if let parsedFeed = parsedFeed { + account.update(webFeed, with: parsedFeed, {_ in }) + } + } + } } diff --git a/Frameworks/Account/CloudKit/CloudKitZone.swift b/Frameworks/Account/CloudKit/CloudKitZone.swift index 75be66fa1..83417d527 100644 --- a/Frameworks/Account/CloudKit/CloudKitZone.swift +++ b/Frameworks/Account/CloudKit/CloudKitZone.swift @@ -7,6 +7,8 @@ // import CloudKit +import os.log +import RSWeb enum CloudKitZoneError: Error { case userDeletedZone @@ -23,39 +25,33 @@ protocol CloudKitZone: class { static var zoneID: CKRecordZone.ID { get } - var container: CKContainer { get } - var database: CKDatabase { get } + var log: OSLog { get } + + var container: CKContainer? { get } + var database: CKDatabase? { get } + var refreshProgress: DownloadProgress? { get set } var delegate: CloudKitZoneDelegate? { get set } - - // func prepare() - - // func fetchChangesInDatabase(_ callback: ((Error?) -> Void)?) - - /// The CloudKit Best Practice is out of date, now use this: - /// https://developer.apple.com/documentation/cloudkit/ckoperation - /// Which problem does this func solve? E.g.: - /// 1.(Offline) You make a local change, involve a operation - /// 2. App exits or ejected by user - /// 3. Back to app again - /// The operation resumes! All works like a magic! - func resumeLongLivedOperationIfPossible() - + } extension CloudKitZone { + func resetChangeToken() { + changeToken = nil + } + func generateRecordID() -> CKRecord.ID { return CKRecord.ID(recordName: UUID().uuidString, zoneID: Self.zoneID) } func resumeLongLivedOperationIfPossible() { - container.fetchAllLongLivedOperationIDs { [weak self]( opeIDs, error) in - guard let self = self, error == nil, let ids = opeIDs else { return } - for id in ids { - self.container.fetchLongLivedOperation(withID: id, completionHandler: { [weak self](ope, error) in - guard let self = self, error == nil else { return } + guard let container = container else { return } + container.fetchAllLongLivedOperationIDs { (opIDs, error) in + guard let opIDs = opIDs else { return } + for opID in opIDs { + container.fetchLongLivedOperation(withID: opID, completionHandler: { (ope, error) in if let modifyOp = ope as? CKModifyRecordsOperation { - self.container.add(modifyOp) + container.add(modifyOp) } }) } @@ -134,10 +130,13 @@ extension CloudKitZone { } } - database.add(op) + database?.add(op) } - func fetchChangesInZones(completion: @escaping (Result) -> Void) { + func fetchChangesInZone(completion: @escaping (Result) -> Void) { + + refreshProgress?.addToNumberOfTasksAndRemaining(1) + let zoneConfig = CKFetchRecordZoneChangesOperation.ZoneConfiguration() zoneConfig.previousServerChangeToken = changeToken let op = CKFetchRecordZoneChangesOperation(recordZoneIDs: [Self.zoneID], configurationsByRecordZoneID: [Self.zoneID: zoneConfig]) @@ -145,43 +144,54 @@ extension CloudKitZone { op.recordZoneChangeTokensUpdatedBlock = { [weak self] zoneId, token, _ in guard let self = self else { return } - self.changeToken = token + DispatchQueue.main.async { + self.changeToken = token + } } op.recordChangedBlock = { [weak self] record in guard let self = self else { return } - self.delegate?.cloudKitDidChange(record: record) + DispatchQueue.main.async { + self.delegate?.cloudKitDidChange(record: record) + } } op.recordWithIDWasDeletedBlock = { [weak self] recordId, recordType in guard let self = self else { return } - self.delegate?.cloudKitDidDelete(recordType: recordType, recordID: recordId) + DispatchQueue.main.async { + self.delegate?.cloudKitDidDelete(recordType: recordType, recordID: recordId) + } } - op.recordZoneFetchCompletionBlock = { [weak self](zoneId ,token, _, _, error) in + op.recordZoneFetchCompletionBlock = { [weak self] zoneId ,token, _, _, error in guard let self = self else { return } switch CloudKitZoneResult.resolve(error) { case .success: - self.changeToken = token + DispatchQueue.main.async { + self.changeToken = token + } case .retry(let timeToWait): self.retryOperationIfPossible(retryAfter: timeToWait) { - self.fetchChangesInZones(completion: completion) + self.fetchChangesInZone(completion: completion) } default: - return - } - } - - op.fetchRecordZoneChangesCompletionBlock = { error in - if let error = error { - completion(.failure(error)) - } else { - completion(.success(())) + os_log(.error, log: self.log, "%@ zone fetch changes error: %@.", zoneId.zoneName, error?.localizedDescription ?? "Unknown") } } - database.add(op) + op.fetchRecordZoneChangesCompletionBlock = { [weak self] error in + DispatchQueue.main.async { + self?.refreshProgress?.completeTask() + if let error = error { + completion(.failure(error)) + } else { + completion(.success(())) + } + } + } + + database?.add(op) } } @@ -213,7 +223,7 @@ private extension CloudKitZone { } func createZoneRecord(completion: @escaping (Result) -> Void) { - database.save(CKRecordZone(zoneID: Self.zoneID)) { (recordZone, error) in + database?.save(CKRecordZone(zoneID: Self.zoneID)) { (recordZone, error) in if let error = error { DispatchQueue.main.async { completion(.failure(error)) diff --git a/Mac/Preferences/Accounts/AccountsAddCloudKitWindowController.swift b/Mac/Preferences/Accounts/AccountsAddCloudKitWindowController.swift index 5bd0e8916..3ce698537 100644 --- a/Mac/Preferences/Accounts/AccountsAddCloudKitWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsAddCloudKitWindowController.swift @@ -35,7 +35,8 @@ class AccountsAddCloudKitWindowController: NSWindowController { } @IBAction func create(_ sender: Any) { - _ = AccountManager.shared.createAccount(type: .cloudKit) + let account = AccountManager.shared.createAccount(type: .cloudKit) + account.refreshAll(completion: { _ in }) hostWindow!.endSheet(window!, returnCode: NSApplication.ModalResponse.OK) } diff --git a/iOS/Account/CloudKitAccountViewController.swift b/iOS/Account/CloudKitAccountViewController.swift index e26135ffd..9c026143a 100644 --- a/iOS/Account/CloudKitAccountViewController.swift +++ b/iOS/Account/CloudKitAccountViewController.swift @@ -25,7 +25,8 @@ class CloudKitAccountViewController: UITableViewController { } @IBAction func add(_ sender: Any) { - _ = AccountManager.shared.createAccount(type: .cloudKit) + let account = AccountManager.shared.createAccount(type: .cloudKit) + account.refreshAll(completion: { _ in }) dismiss(animated: true, completion: nil) delegate?.dismiss() } diff --git a/iOS/Settings/AddAccountViewController.swift b/iOS/Settings/AddAccountViewController.swift index e3d55dc1f..20a59d6c6 100644 --- a/iOS/Settings/AddAccountViewController.swift +++ b/iOS/Settings/AddAccountViewController.swift @@ -47,7 +47,7 @@ class AddAccountViewController: UITableViewController, AddAccountDismissDelegate cell.accountNameLabel?.text = Account.defaultLocalAccountName cell.accountImage?.image = AppAssets.image(for: .onMyMac) case .cloudKit: - cell.accountNameLabel?.text = NSLocalizedString("CloudKit", comment: "CloudKit") + cell.accountNameLabel?.text = NSLocalizedString("iCloud", comment: "iCloud") cell.accountImage?.image = AppAssets.accountCloudKitImage case .feedbin: cell.accountNameLabel?.text = NSLocalizedString("Feedbin", comment: "Feedbin")