From 6870133d607aae1222e1de67a7dbf36c3f8efa6b Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Mon, 27 Apr 2020 16:41:45 -0500 Subject: [PATCH] Enhance SyncStatus so that it can communicate new, updated, and deleted --- .../CloudKit/CloudKitAccountDelegate.swift | 175 ++++++++---------- .../CloudKit/CloudKitArticlesZone.swift | 65 ++++--- .../FeedWrangler/FeedWranglerAPICaller.swift | 5 +- .../FeedWranglerAccountDelegate.swift | 2 +- .../Feedbin/FeedbinAccountDelegate.swift | 10 +- .../Feedly/FeedlyAccountDelegate.swift | 2 +- .../FeedlySendArticleStatusesOperation.swift | 2 +- .../NewsBlur/NewsBlurAccountDelegate.swift | 10 +- .../ReaderAPI/ReaderAPIAccountDelegate.swift | 10 +- Frameworks/SyncDatabase/SyncStatus.swift | 21 ++- Frameworks/SyncDatabase/SyncStatusTable.swift | 2 +- 11 files changed, 158 insertions(+), 146 deletions(-) diff --git a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift index 633efb4a7..7d35ff1ac 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift @@ -112,7 +112,12 @@ final class CloudKitAccountDelegate: AccountDelegate { func processWithArticles(_ articles: Set
) { - self.articlesZone.modifyArticles(articles) { result in + let articlesDict = articles.reduce(into: [String: Article]()) { result, article in + result[article.articleID] = article + } + let statusedArticles = syncStatuses.map { ($0, articlesDict[$0.articleID]) } + + self.articlesZone.modifyArticles(statusedArticles) { result in switch result { case .success: self.database.deleteSelectedForProcessing(syncStatuses.map({ $0.articleID }) ) @@ -428,7 +433,7 @@ final class CloudKitAccountDelegate: AccountDelegate { func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { let syncStatuses = articles.map { article in - return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + return SyncStatus(articleID: article.articleID, key: SyncStatus.Key(statusKey), flag: flag) } database.insertStatuses(syncStatuses) @@ -586,10 +591,6 @@ private extension CloudKitAccountDelegate { func combinedRefresh(_ account: Account, _ webFeeds: Set, completion: @escaping () -> Void) { - var newArticles = Set
() - var updatedArticles = Set
() - var deletedArticles = Set
() - var refresherWebFeeds = Set() let group = DispatchGroup() @@ -605,14 +606,10 @@ private extension CloudKitAccountDelegate { account.update(webFeed.webFeedID, with: parsedItems) { result in switch result { case .success(let articleChanges): - - newArticles.formUnion(articleChanges.newArticles ?? Set
()) - updatedArticles.formUnion(articleChanges.updatedArticles ?? Set
()) - deletedArticles.formUnion(articleChanges.deletedArticles ?? Set
()) - - 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() @@ -634,14 +631,13 @@ private extension CloudKitAccountDelegate { group.enter() refresher.refreshFeeds(refresherWebFeeds) { refresherNewArticles, refresherUpdatedArticles, refresherDeletedArticles in - newArticles.formUnion(refresherNewArticles) - updatedArticles.formUnion(refresherUpdatedArticles) - deletedArticles.formUnion(refresherDeletedArticles) - group.leave() + self.storeArticleChanges(new: refresherNewArticles, updated: refresherUpdatedArticles, deleted: refresherDeletedArticles) { + group.leave() + } } group.notify(queue: DispatchQueue.main) { - self.processRecords(new: newArticles, updated: updatedArticles, deleted: deletedArticles) { + self.refreshArticleStatus(for: account) { _ in self.articlesZone.fetchChangesInZone() { _ in self.refreshProgress.completeTask() completion() @@ -650,53 +646,6 @@ private extension CloudKitAccountDelegate { } } - - func processRecords(new: Set
, updated: Set
, deleted: Set
, completion: @escaping () -> Void) { - - self.articlesZone.deleteArticles(deleted) { result in - self.refreshProgress.completeTask() - switch result { - case .success: - self.articlesZone.modifyArticles(updated) { result in - self.refreshProgress.completeTask() - switch result { - case .success: - self.saveNewArticles(new) { - completion() - } - case .failure(let error): - os_log(.error, log: self.log, "CloudKit modify articles error: %@.", error.localizedDescription) - completion() - } - } - case .failure(let error): - os_log(.error, log: self.log, "CloudKit delete articles error: %@.", error.localizedDescription) - completion() - } - } - } - - func saveNewArticles(_ articles: Set
, completion: @escaping () -> Void) { - let group = DispatchGroup() - - let articleGroups = Array(articles).chunked(into: 300).map { Set($0) } - refreshProgress.addToNumberOfTasksAndRemaining(articleGroups.count) - - for articleGroup in articleGroups { - group.enter() - self.articlesZone.saveNewArticles(articleGroup) { result in - self.refreshProgress.completeTask() - group.leave() - if case .failure(let error) = result { - os_log(.error, log: self.log, "CloudKit new articles error: %@.", error.localizedDescription) - } - } - } - - group.notify(queue: DispatchQueue.main) { - completion() - } - } func createProviderWebFeed(for account: Account, urlComponents: URLComponents, editedName: String?, container: Container, feedProvider: FeedProvider, completion: @escaping (Result) -> Void) { refreshProgress.addToNumberOfTasksAndRemaining(6) @@ -731,21 +680,7 @@ private extension CloudKitAccountDelegate { account.update(urlString, with: parsedItems) { result in switch result { case .success: - - account.fetchArticlesAsync(.webFeed(feed)) { result in - switch result { - case .success(let articles): - self.processRecords(new: articles, updated: Set
(), deleted: Set
()) { - self.articlesZone.fetchChangesInZone() { _ in - self.refreshProgress.clear() - completion(.success(feed)) - } - } - case .failure(let error): - completion(.failure(error)) - } - } - + self.sendNewArticlesToTheCloud(account, feed, completion: completion) case .failure(let error): self.refreshProgress.clear() completion(.failure(error)) @@ -812,23 +747,8 @@ private extension CloudKitAccountDelegate { self.refreshProgress.completeTask() switch result { case .success(let externalID): - feed.externalID = externalID - - account.fetchArticlesAsync(.webFeed(feed)) { result in - switch result { - case .success(let articles): - self.processRecords(new: articles, updated: Set
(), deleted: Set
()) { - self.articlesZone.fetchChangesInZone() { _ in - self.refreshProgress.clear() - completion(.success(feed)) - } - } - case .failure(let error): - completion(.failure(error)) - } - } - + self.sendNewArticlesToTheCloud(account, feed, completion: completion) case .failure(let error): self.refreshProgress.clear() completion(.failure(error)) @@ -859,6 +779,31 @@ private extension CloudKitAccountDelegate { } } + func sendNewArticlesToTheCloud(_ account: Account, _ feed: WebFeed, completion: @escaping (Result) -> Void) { + account.fetchArticlesAsync(.webFeed(feed)) { result in + switch result { + case .success(let articles): + self.storeArticleChanges(new: articles, updated: Set
(), deleted: Set
()) { + self.refreshArticleStatus(for: account) { result in + switch result { + case .success: + self.articlesZone.fetchChangesInZone() { _ in + self.refreshProgress.clear() + completion(.success(feed)) + } + case .failure(let error): + self.refreshProgress.clear() + completion(.failure(error)) + } + } + } + case .failure(let error): + self.refreshProgress.clear() + completion(.failure(error)) + } + } + } + func processAccountError(_ account: Account, _ error: Error) { if case CloudKitZoneError.userDeletedZone = error { account.removeFeeds(account.topLevelWebFeeds) @@ -868,6 +813,42 @@ private extension CloudKitAccountDelegate { } } + func storeArticleChanges(new: Set
?, updated: Set
?, deleted: Set
?, completion: @escaping () -> Void) { + let group = DispatchGroup() + + group.enter() + insertSyncStatuses(articles: new, 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.main) { + completion() + } + } + + func insertSyncStatuses(articles: Set
?, statusKey: SyncStatus.Key, flag: Bool, completion: @escaping () -> Void) { + guard let articles = articles else { + completion() + return + } + let syncStatuses = articles.map { article in + return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + } + database.insertStatuses(syncStatuses) { _ in + completion() + } + } + } extension CloudKitAccountDelegate: LocalAccountRefresherDelegate { diff --git a/Frameworks/Account/CloudKit/CloudKitArticlesZone.swift b/Frameworks/Account/CloudKit/CloudKitArticlesZone.swift index 7131efb1c..d883a9740 100644 --- a/Frameworks/Account/CloudKit/CloudKitArticlesZone.swift +++ b/Frameworks/Account/CloudKit/CloudKitArticlesZone.swift @@ -12,6 +12,7 @@ import RSParser import RSWeb import CloudKit import Articles +import SyncDatabase final class CloudKitArticlesZone: CloudKitZone { @@ -95,40 +96,50 @@ final class CloudKitArticlesZone: CloudKitZone { delete(ckQuery: ckQuery, completion: completion) } - func deleteArticles(_ articles: Set
, completion: @escaping ((Result) -> Void)) { - guard !articles.isEmpty else { + func modifyArticles(_ statusArticles: [(status: SyncStatus, article: Article?)], completion: @escaping ((Result) -> Void)) { + guard !statusArticles.isEmpty else { completion(.success(())) return } - let recordIDs = articles.map { CKRecord.ID(recordName: $0.articleID, zoneID: Self.zoneID) } - delete(recordIDs: recordIDs, completion: completion) - } - - func modifyArticles(_ articles: Set
, completion: @escaping ((Result) -> Void)) { - guard !articles.isEmpty else { - completion(.success(())) - return + var newRecords = [CKRecord]() + var modifyRecords = [CKRecord]() + var deleteRecordIDs = [CKRecord.ID]() + + for statusArticle in statusArticles { + switch (statusArticle.status.key, statusArticle.status.flag) { + case (.new, true): + // create status + if let article = statusArticle.article { + newRecords.append(contentsOf: makeArticleRecords(article)) + } + case (.starred, true), (.read, false): + // create status + if let article = statusArticle.article { + modifyRecords.append(contentsOf: makeArticleRecords(article)) + } + case (.deleted, true): + deleteRecordIDs.append(CKRecord.ID(recordName: statusArticle.status.articleID, zoneID: Self.zoneID)) + default: + print() + // create status + // delete article record + } } - var records = [CKRecord]() - - let saveArticles = articles.filter { $0.status.read == false || $0.status.starred == true } - for saveArticle in saveArticles { - records.append(contentsOf: makeArticleRecords(saveArticle)) - } - - let hollowArticles = articles.subtracting(saveArticles) - for hollowArticle in hollowArticles { - records.append(contentsOf: makeHollowArticleRecords(hollowArticle)) - } - - self.modify(recordsToSave: records, recordIDsToDelete: []) { result in + saveIfNew(newRecords) { result in switch result { case .success: - completion(.success(())) + self.modify(recordsToSave: modifyRecords, recordIDsToDelete: deleteRecordIDs) { result in + switch result { + case .success: + completion(.success(())) + case .failure(let error): + self.handleSendArticleStatusError(error, statusArticles: statusArticles, completion: completion) + } + } case .failure(let error): - self.handleSendArticleStatusError(error, articles: articles, completion: completion) + self.handleSendArticleStatusError(error, statusArticles: statusArticles, completion: completion) } } } @@ -137,12 +148,12 @@ final class CloudKitArticlesZone: CloudKitZone { private extension CloudKitArticlesZone { - func handleSendArticleStatusError(_ error: Error, articles: Set
, completion: @escaping ((Result) -> Void)) { + func handleSendArticleStatusError(_ error: Error, statusArticles: [(status: SyncStatus, article: Article?)], completion: @escaping ((Result) -> Void)) { if case CloudKitZoneError.userDeletedZone = error { self.createZoneRecord() { result in switch result { case .success: - self.modifyArticles(articles, completion: completion) + self.modifyArticles(statusArticles, completion: completion) case .failure(let error): completion(.failure(error)) } diff --git a/Frameworks/Account/FeedWrangler/FeedWranglerAPICaller.swift b/Frameworks/Account/FeedWrangler/FeedWranglerAPICaller.swift index 17c1ae089..1c525e63c 100644 --- a/Frameworks/Account/FeedWrangler/FeedWranglerAPICaller.swift +++ b/Frameworks/Account/FeedWrangler/FeedWranglerAPICaller.swift @@ -261,9 +261,12 @@ final class FeedWranglerAPICaller: NSObject { switch status.key { case .read: return URLQueryItem(name: "read", value: status.flag.description) - case .starred: return URLQueryItem(name: "starred", value: status.flag.description) + case .deleted: + return nil + case .new: + return nil } } queryItems.append(URLQueryItem(name: "feed_item_id", value: articleID)) diff --git a/Frameworks/Account/FeedWrangler/FeedWranglerAccountDelegate.swift b/Frameworks/Account/FeedWrangler/FeedWranglerAccountDelegate.swift index dffff49af..9ae70a87f 100644 --- a/Frameworks/Account/FeedWrangler/FeedWranglerAccountDelegate.swift +++ b/Frameworks/Account/FeedWrangler/FeedWranglerAccountDelegate.swift @@ -436,7 +436,7 @@ final class FeedWranglerAccountDelegate: AccountDelegate { } func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { - let syncStatuses = articles.map { SyncStatus(articleID: $0.articleID, key: statusKey, flag: flag)} + let syncStatuses = articles.map { SyncStatus(articleID: $0.articleID, key: SyncStatus.Key(statusKey), flag: flag)} database.insertStatuses(syncStatuses) database.selectPendingCount { result in diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 8159c4838..575848806 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -119,10 +119,10 @@ final class FeedbinAccountDelegate: AccountDelegate { database.selectForProcessing { result in func processStatuses(_ syncStatuses: [SyncStatus]) { - let createUnreadStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.read && $0.flag == false } - let deleteUnreadStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.read && $0.flag == true } - let createStarredStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.starred && $0.flag == true } - let deleteStarredStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.starred && $0.flag == false } + let createUnreadStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.read && $0.flag == false } + let deleteUnreadStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.read && $0.flag == true } + let createStarredStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.starred && $0.flag == true } + let deleteStarredStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.starred && $0.flag == false } let group = DispatchGroup() var errorOccurred = false @@ -537,7 +537,7 @@ final class FeedbinAccountDelegate: AccountDelegate { func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { let syncStatuses = articles.map { article in - return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + return SyncStatus(articleID: article.articleID, key: SyncStatus.Key(statusKey), flag: flag) } database.insertStatuses(syncStatuses) diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 61dba4f39..9390f493d 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -489,7 +489,7 @@ final class FeedlyAccountDelegate: AccountDelegate { func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { let syncStatuses = articles.map { article in - return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + return SyncStatus(articleID: article.articleID, key: SyncStatus.Key(statusKey), flag: flag) } database.insertStatuses(syncStatuses) diff --git a/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift index e01e6d4c7..e5d71060d 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift @@ -47,7 +47,7 @@ final class FeedlySendArticleStatusesOperation: FeedlyOperation { private extension FeedlySendArticleStatusesOperation { func processStatuses(_ pending: [SyncStatus]) { - let statuses: [(status: ArticleStatus.Key, flag: Bool, action: FeedlyMarkAction)] = [ + let statuses: [(status: SyncStatus.Key, flag: Bool, action: FeedlyMarkAction)] = [ (.read, false, .unread), (.read, true, .read), (.starred, true, .saved), diff --git a/Frameworks/Account/NewsBlur/NewsBlurAccountDelegate.swift b/Frameworks/Account/NewsBlur/NewsBlurAccountDelegate.swift index b95ae7dab..638d3559a 100644 --- a/Frameworks/Account/NewsBlur/NewsBlurAccountDelegate.swift +++ b/Frameworks/Account/NewsBlur/NewsBlurAccountDelegate.swift @@ -131,16 +131,16 @@ final class NewsBlurAccountDelegate: AccountDelegate { func processStatuses(_ syncStatuses: [SyncStatus]) { let createUnreadStatuses = syncStatuses.filter { - $0.key == ArticleStatus.Key.read && $0.flag == false + $0.key == SyncStatus.Key.read && $0.flag == false } let deleteUnreadStatuses = syncStatuses.filter { - $0.key == ArticleStatus.Key.read && $0.flag == true + $0.key == SyncStatus.Key.read && $0.flag == true } let createStarredStatuses = syncStatuses.filter { - $0.key == ArticleStatus.Key.starred && $0.flag == true + $0.key == SyncStatus.Key.starred && $0.flag == true } let deleteStarredStatuses = syncStatuses.filter { - $0.key == ArticleStatus.Key.starred && $0.flag == false + $0.key == SyncStatus.Key.starred && $0.flag == false } let group = DispatchGroup() @@ -575,7 +575,7 @@ final class NewsBlurAccountDelegate: AccountDelegate { func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { let syncStatuses = articles.map { article in - return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + return SyncStatus(articleID: article.articleID, key: SyncStatus.Key(statusKey), flag: flag) } database.insertStatuses(syncStatuses) diff --git a/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index f36c69ac8..a4bc9f062 100644 --- a/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -123,10 +123,10 @@ final class ReaderAPIAccountDelegate: AccountDelegate { database.selectForProcessing { result in func processStatuses(_ syncStatuses: [SyncStatus]) { - let createUnreadStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.read && $0.flag == false } - let deleteUnreadStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.read && $0.flag == true } - let createStarredStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.starred && $0.flag == true } - let deleteStarredStatuses = syncStatuses.filter { $0.key == ArticleStatus.Key.starred && $0.flag == false } + let createUnreadStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.read && $0.flag == false } + let deleteUnreadStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.read && $0.flag == true } + let createStarredStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.starred && $0.flag == true } + let deleteStarredStatuses = syncStatuses.filter { $0.key == SyncStatus.Key.starred && $0.flag == false } let group = DispatchGroup() @@ -412,7 +412,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { let syncStatuses = articles.map { article in - return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + return SyncStatus(articleID: article.articleID, key: SyncStatus.Key(statusKey), flag: flag) } database.insertStatuses(syncStatuses) diff --git a/Frameworks/SyncDatabase/SyncStatus.swift b/Frameworks/SyncDatabase/SyncStatus.swift index 3b9db3cd9..cb6c71a45 100644 --- a/Frameworks/SyncDatabase/SyncStatus.swift +++ b/Frameworks/SyncDatabase/SyncStatus.swift @@ -12,12 +12,29 @@ import RSDatabase public struct SyncStatus: Hashable, Equatable { + public enum Key: String { + case read = "read" + case starred = "starred" + case deleted = "deleted" + case new = "new" + + public init(_ articleStatusKey: ArticleStatus.Key) { + switch articleStatusKey { + case .read: + self = Self.read + case .starred: + self = Self.starred + } + } + + } + public let articleID: String - public let key: ArticleStatus.Key + public let key: SyncStatus.Key public let flag: Bool public let selected: Bool - public init(articleID: String, key: ArticleStatus.Key, flag: Bool, selected: Bool = false) { + public init(articleID: String, key: SyncStatus.Key, flag: Bool, selected: Bool = false) { self.articleID = articleID self.key = key self.flag = flag diff --git a/Frameworks/SyncDatabase/SyncStatusTable.swift b/Frameworks/SyncDatabase/SyncStatusTable.swift index 3d93ffab1..a0be0e8d2 100644 --- a/Frameworks/SyncDatabase/SyncStatusTable.swift +++ b/Frameworks/SyncDatabase/SyncStatusTable.swift @@ -155,7 +155,7 @@ private extension SyncStatusTable { func statusWithRow(_ row: FMResultSet) -> SyncStatus? { guard let articleID = row.string(forColumn: DatabaseKey.articleID), let rawKey = row.string(forColumn: DatabaseKey.key), - let key = ArticleStatus.Key(rawValue: rawKey) else { + let key = SyncStatus.Key(rawValue: rawKey) else { return nil }