From 8a03a26a103fcfb9ee932ec7f123ba64a89b57e0 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 28 May 2020 16:24:10 -0500 Subject: [PATCH] Make sync status inserts async when done during the scope of a iCloud sync --- .../CloudKit/CloudKitAccountDelegate.swift | 71 +++++++++++++------ .../LocalAccount/LocalAccountDelegate.swift | 7 +- .../LocalAccount/LocalAccountRefresher.swift | 31 ++++---- Frameworks/SyncDatabase/SyncDatabase.swift | 4 ++ Frameworks/SyncDatabase/SyncStatusTable.swift | 19 +++++ 5 files changed, 95 insertions(+), 37 deletions(-) diff --git a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift index a9daaa188..25a130633 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift @@ -562,9 +562,10 @@ private extension CloudKitAccountDelegate { account.update(webFeed.webFeedID, with: parsedItems) { result in switch result { case .success(let articleChanges): - self.storeArticleChanges(new: articleChanges.newArticles, updated: articleChanges.updatedArticles, deleted: articleChanges.deletedArticles) - self.refreshProgress.completeTask() - group.leave() + self.storeArticleChanges(new: articleChanges.newArticles, updated: articleChanges.updatedArticles, deleted: articleChanges.deletedArticles) { + self.refreshProgress.completeTask() + group.leave() + } case .failure(let error): os_log(.error, log: self.log, "CloudKit Feed refresh update error: %@.", error.localizedDescription) self.refreshProgress.completeTask() @@ -735,17 +736,18 @@ private extension CloudKitAccountDelegate { account.fetchArticlesAsync(.webFeed(feed)) { result in switch result { case .success(let articles): - self.storeArticleChanges(new: articles, updated: Set
(), deleted: Set
()) - self.sendArticleStatus(for: account, showProgress: true) { result in - switch result { - case .success: - self.articlesZone.fetchChangesInZone() { _ in + self.storeArticleChanges(new: articles, updated: Set
(), deleted: Set
()) { + self.sendArticleStatus(for: account, showProgress: true) { result in + switch result { + case .success: + self.articlesZone.fetchChangesInZone() { _ in + self.refreshProgress.clear() + completion(.success(feed)) + } + case .failure(let error): self.refreshProgress.clear() - completion(.success(feed)) + completion(.failure(error)) } - case .failure(let error): - self.refreshProgress.clear() - completion(.failure(error)) } } case .failure(let error): @@ -764,24 +766,45 @@ private extension CloudKitAccountDelegate { } } - func storeArticleChanges(new: Set
?, updated: Set
?, deleted: Set
?) { + func storeArticleChanges(new: Set
?, updated: Set
?, deleted: Set
?, completion: @escaping () -> Void) { // New records with a read status aren't really new, they just didn't have the read article stored + let group = DispatchGroup() if let new = new { let filteredNew = new.filter { $0.status.read == false } - insertSyncStatuses(articles: filteredNew, statusKey: .new, flag: true) + group.enter() + insertSyncStatuses(articles: filteredNew, statusKey: .new, flag: true) { + group.leave() + } + } + + group.enter() + insertSyncStatuses(articles: updated, statusKey: .new, flag: false) { + group.leave() + } + + group.enter() + insertSyncStatuses(articles: deleted, statusKey: .deleted, flag: true) { + group.leave() + } + + group.notify(queue: DispatchQueue.global(qos: .userInitiated)) { + DispatchQueue.main.async { + completion() + } } - insertSyncStatuses(articles: updated, statusKey: .new, flag: false) - insertSyncStatuses(articles: deleted, statusKey: .deleted, flag: true) } - func insertSyncStatuses(articles: Set
?, statusKey: SyncStatus.Key, flag: Bool) { + func insertSyncStatuses(articles: Set
?, statusKey: SyncStatus.Key, flag: Bool, completion: @escaping () -> Void) { guard let articles = articles, !articles.isEmpty else { + completion() return } let syncStatuses = articles.map { article in return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) } - try? database.insertStatuses(syncStatuses) + database.insertStatuses(syncStatuses) { _ in + completion() + } } func sendArticleStatus(for account: Account, showProgress: Bool, completion: @escaping ((Result) -> Void)) { @@ -827,11 +850,15 @@ private extension CloudKitAccountDelegate { extension CloudKitAccountDelegate: LocalAccountRefresherDelegate { - func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed, articleChanges: ArticleChanges?) { + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed) { refreshProgress.completeTask() - if let articleChanges = articleChanges { - self.storeArticleChanges(new: articleChanges.newArticles, updated: articleChanges.updatedArticles, deleted: articleChanges.deletedArticles) - } + } + + func localAccountRefresher(_ refresher: LocalAccountRefresher, articleChanges: ArticleChanges, completion: @escaping () -> Void) { + self.storeArticleChanges(new: articleChanges.newArticles, + updated: articleChanges.updatedArticles, + deleted: articleChanges.deletedArticles, + completion: completion) } } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 14d0d402a..8b69cfe46 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -235,10 +235,15 @@ final class LocalAccountDelegate: AccountDelegate { extension LocalAccountDelegate: LocalAccountRefresherDelegate { - func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed, articleChanges: ArticleChanges?) { + + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed) { refreshProgress.completeTask() } + func localAccountRefresher(_ refresher: LocalAccountRefresher, articleChanges: ArticleChanges, completion: @escaping () -> Void) { + completion() + } + } private extension LocalAccountDelegate { diff --git a/Frameworks/Account/LocalAccount/LocalAccountRefresher.swift b/Frameworks/Account/LocalAccount/LocalAccountRefresher.swift index 0e39b3e45..cc39bf82e 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountRefresher.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountRefresher.swift @@ -14,7 +14,8 @@ import Articles import ArticlesDatabase protocol LocalAccountRefresherDelegate { - func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed, articleChanges: ArticleChanges?) + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed) + func localAccountRefresher(_ refresher: LocalAccountRefresher, articleChanges: ArticleChanges, completion: @escaping () -> Void) } final class LocalAccountRefresher { @@ -72,21 +73,21 @@ extension LocalAccountRefresher: DownloadSessionDelegate { guard !data.isEmpty, !isSuspended else { completion() - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) return } if let error = error { print("Error downloading \(feed.url) - \(error)") completion() - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) return } let dataHash = data.md5String if dataHash == feed.contentHash { completion() - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) return } @@ -95,7 +96,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate { guard let account = feed.account, let parsedFeed = parsedFeed, error == nil else { completion() - self.delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + self.delegate?.localAccountRefresher(self, requestCompletedFor: feed) return } @@ -105,11 +106,13 @@ extension LocalAccountRefresher: DownloadSessionDelegate { feed.conditionalGetInfo = HTTPConditionalGetInfo(urlResponse: httpResponse) } feed.contentHash = dataHash - completion() - self.delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: articleChanges) + self.delegate?.localAccountRefresher(self, requestCompletedFor: feed) + self.delegate?.localAccountRefresher(self, articleChanges: articleChanges) { + completion() + } } else { completion() - self.delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + self.delegate?.localAccountRefresher(self, requestCompletedFor: feed) } } @@ -119,7 +122,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate { func downloadSession(_ downloadSession: DownloadSession, shouldContinueAfterReceivingData data: Data, representedObject: AnyObject) -> Bool { let feed = representedObject as! WebFeed guard !isSuspended else { - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) return false } @@ -128,7 +131,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate { } if data.isDefinitelyNotFeed() { - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) return false } @@ -137,7 +140,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate { if FeedParser.mightBeAbleToParseBasedOnPartialData(parserData) { return true } else { - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) return false } } @@ -147,17 +150,17 @@ extension LocalAccountRefresher: DownloadSessionDelegate { func downloadSession(_ downloadSession: DownloadSession, didReceiveUnexpectedResponse response: URLResponse, representedObject: AnyObject) { let feed = representedObject as! WebFeed - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) } func downloadSession(_ downloadSession: DownloadSession, didReceiveNotModifiedResponse: URLResponse, representedObject: AnyObject) { let feed = representedObject as! WebFeed - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) } func downloadSession(_ downloadSession: DownloadSession, didDiscardDuplicateRepresentedObject representedObject: AnyObject) { let feed = representedObject as! WebFeed - delegate?.localAccountRefresher(self, requestCompletedFor: feed, articleChanges: nil) + delegate?.localAccountRefresher(self, requestCompletedFor: feed) } func downloadSessionDidCompleteDownloadObjects(_ downloadSession: DownloadSession) { diff --git a/Frameworks/SyncDatabase/SyncDatabase.swift b/Frameworks/SyncDatabase/SyncDatabase.swift index a7c754038..9f5fd0827 100644 --- a/Frameworks/SyncDatabase/SyncDatabase.swift +++ b/Frameworks/SyncDatabase/SyncDatabase.swift @@ -36,6 +36,10 @@ public struct SyncDatabase { try syncStatusTable.insertStatuses(statuses) } + public func insertStatuses(_ statuses: [SyncStatus], completion: @escaping DatabaseCompletionBlock) { + syncStatusTable.insertStatuses(statuses, completion: completion) + } + public func selectForProcessing(limit: Int? = nil, completion: @escaping SyncStatusesCompletionBlock) { return syncStatusTable.selectForProcessing(limit: limit, completion: completion) } diff --git a/Frameworks/SyncDatabase/SyncStatusTable.swift b/Frameworks/SyncDatabase/SyncStatusTable.swift index 4b55a9bb0..9661becf7 100644 --- a/Frameworks/SyncDatabase/SyncStatusTable.swift +++ b/Frameworks/SyncDatabase/SyncStatusTable.swift @@ -168,6 +168,25 @@ struct SyncStatusTable: DatabaseTable { throw error } } + + func insertStatuses(_ statuses: [SyncStatus], completion: @escaping DatabaseCompletionBlock) { + queue.runInTransaction { databaseResult in + + func makeDatabaseCall(_ database: FMDatabase) { + let statusArray = statuses.map { $0.databaseDictionary() } + self.insertRows(statusArray, insertType: .orReplace, in: database) + } + + switch databaseResult { + case .success(let database): + makeDatabaseCall(database) + completion(nil) + case .failure(let databaseError): + completion(databaseError) + } + } + } + } private extension SyncStatusTable {