From 4418a4bb0207bbe7fc4b83ff7fafed205b9109ff Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Fri, 10 Apr 2020 15:19:33 -0500 Subject: [PATCH] Add completion block that returns new status records when we are marking statuses asynchronously. --- Frameworks/Account/Account.swift | 38 +++++++++++-------- ...edlyIngestStarredArticleIdsOperation.swift | 12 ++++-- ...eedlyIngestUnreadArticleIdsOperation.swift | 12 ++++-- .../ArticlesDatabase/ArticlesDatabase.swift | 4 +- .../ArticlesDatabase/ArticlesTable.swift | 12 +++--- .../ArticlesDatabase/StatusesTable.swift | 11 +++--- 6 files changed, 52 insertions(+), 37 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index cc4fc37f4..e3b2e4890 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -800,39 +800,45 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, /// Mark articleIDs statuses based on statusKey and flag. /// Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. - func mark(articleIDs: Set, statusKey: ArticleStatus.Key, flag: Bool, completion: DatabaseCompletionBlock? = nil) { + /// Returns a set of new article statuses. + func markAndFetchNew(articleIDs: Set, statusKey: ArticleStatus.Key, flag: Bool, completion: ArticleStatusesResultBlock? = nil) { guard !articleIDs.isEmpty else { - completion?(nil) + completion?(.success(Set())) return } - database.mark(articleIDs: articleIDs, statusKey: statusKey, flag: flag) { error in - if let error = error { - completion?(error) - return + database.markAndFetchNew(articleIDs: articleIDs, statusKey: statusKey, flag: flag) { result in + switch result { + case .success(let newArticleStatuses): + self.noteStatusesForArticleIDsDidChange(articleIDs) + completion?(.success(newArticleStatuses)) + case .failure(let databaseError): + completion?(.failure(databaseError)) } - self.noteStatusesForArticleIDsDidChange(articleIDs) - completion?(nil) } } /// Mark articleIDs as read. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. - func markAsRead(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { - mark(articleIDs: articleIDs, statusKey: .read, flag: true, completion: completion) + /// Returns a set of new article statuses. + func markAsRead(_ articleIDs: Set, completion: ArticleStatusesResultBlock? = nil) { + markAndFetchNew(articleIDs: articleIDs, statusKey: .read, flag: true, completion: completion) } /// Mark articleIDs as unread. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. - func markAsUnread(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { - mark(articleIDs: articleIDs, statusKey: .read, flag: false, completion: completion) + /// Returns a set of new article statuses. + func markAsUnread(_ articleIDs: Set, completion: ArticleStatusesResultBlock? = nil) { + markAndFetchNew(articleIDs: articleIDs, statusKey: .read, flag: false, completion: completion) } /// Mark articleIDs as starred. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. - func markAsStarred(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { - mark(articleIDs: articleIDs, statusKey: .starred, flag: true, completion: completion) + /// Returns a set of new article statuses. + func markAsStarred(_ articleIDs: Set, completion: ArticleStatusesResultBlock? = nil) { + markAndFetchNew(articleIDs: articleIDs, statusKey: .starred, flag: true, completion: completion) } /// Mark articleIDs as unstarred. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. - func markAsUnstarred(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { - mark(articleIDs: articleIDs, statusKey: .starred, flag: false, completion: completion) + /// Returns a set of new article statuses. + func markAsUnstarred(_ articleIDs: Set, completion: ArticleStatusesResultBlock? = nil) { + markAndFetchNew(articleIDs: articleIDs, statusKey: .starred, flag: false, completion: completion) } /// Empty caches that can reasonably be emptied. Call when the app goes in the background, for instance. diff --git a/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift index 02dc42043..0be82fa95 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift @@ -124,15 +124,19 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { let results = StarredStatusResults() group.enter() - account.markAsStarred(remoteStarredArticleIDs) { error in - results.markAsStarredError = error + account.markAsStarred(remoteStarredArticleIDs) { result in + if case .failure(let error) = result { + results.markAsStarredError = error + } group.leave() } let deltaUnstarredArticleIDs = localStarredArticleIDs.subtracting(remoteStarredArticleIDs) group.enter() - account.markAsUnstarred(deltaUnstarredArticleIDs) { error in - results.markAsUnstarredError = error + account.markAsUnstarred(deltaUnstarredArticleIDs) { result in + if case .failure(let error) = result { + results.markAsUnstarredError = error + } group.leave() } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift index 669a9672d..985b51006 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift @@ -124,15 +124,19 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { let results = ReadStatusResults() group.enter() - account.markAsUnread(remoteUnreadArticleIDs) { error in - results.markAsUnreadError = error + account.markAsUnread(remoteUnreadArticleIDs) { result in + if case .failure(let error) = result { + results.markAsUnreadError = error + } group.leave() } let articleIDsToMarkRead = localUnreadArticleIDs.subtracting(remoteUnreadArticleIDs) group.enter() - account.markAsRead(articleIDsToMarkRead) { error in - results.markAsReadError = error + account.markAsRead(articleIDsToMarkRead) { result in + if case .failure(let error) = result { + results.markAsReadError = error + } group.leave() } diff --git a/Frameworks/ArticlesDatabase/ArticlesDatabase.swift b/Frameworks/ArticlesDatabase/ArticlesDatabase.swift index 4ca73e177..179b3928f 100644 --- a/Frameworks/ArticlesDatabase/ArticlesDatabase.swift +++ b/Frameworks/ArticlesDatabase/ArticlesDatabase.swift @@ -222,8 +222,8 @@ public final class ArticlesDatabase { return try articlesTable.mark(articles, statusKey, flag) } - public func mark(articleIDs: Set, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping DatabaseCompletionBlock) { - articlesTable.mark(articleIDs, statusKey, flag, completion) + public func markAndFetchNew(articleIDs: Set, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping ArticleStatusesResultBlock) { + articlesTable.markAndFetchNew(articleIDs, statusKey, flag, completion) } /// Create statuses for specified articleIDs. For existing statuses, don’t do anything. diff --git a/Frameworks/ArticlesDatabase/ArticlesTable.swift b/Frameworks/ArticlesDatabase/ArticlesTable.swift index 9aa7bc06b..a6c72b790 100644 --- a/Frameworks/ArticlesDatabase/ArticlesTable.swift +++ b/Frameworks/ArticlesDatabase/ArticlesTable.swift @@ -194,7 +194,7 @@ final class ArticlesTable: DatabaseTable { func makeDatabaseCalls(_ database: FMDatabase) { let articleIDs = parsedItems.articleIDs() - let statusesDictionary = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, false, database) //1 + let (statusesDictionary, _) = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, false, database) //1 assert(statusesDictionary.count == articleIDs.count) let allIncomingArticles = Article.articlesWithParsedItems(parsedItems, webFeedID, self.accountID, statusesDictionary) //2 @@ -266,7 +266,7 @@ final class ArticlesTable: DatabaseTable { articleIDs.formUnion(parsedItems.articleIDs()) } - let statusesDictionary = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, read, database) //1 + let (statusesDictionary, _) = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, read, database) //1 assert(statusesDictionary.count == articleIDs.count) let allIncomingArticles = Article.articlesWithWebFeedIDsAndItems(webFeedIDsAndItems, self.accountID, statusesDictionary) //2 @@ -418,17 +418,17 @@ final class ArticlesTable: DatabaseTable { return statuses } - func mark(_ articleIDs: Set, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ completion: @escaping DatabaseCompletionBlock) { + func markAndFetchNew(_ articleIDs: Set, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ completion: @escaping ArticleStatusesResultBlock) { queue.runInTransaction { databaseResult in switch databaseResult { case .success(let database): - self.statusesTable.mark(articleIDs, statusKey, flag, database) + let newStatusIDs = self.statusesTable.markAndFetchNew(articleIDs, statusKey, flag, database) DispatchQueue.main.async { - completion(nil) + completion(.success(newStatusIDs)) } case .failure(let databaseError): DispatchQueue.main.async { - completion(databaseError) + completion(.failure(databaseError)) } } } diff --git a/Frameworks/ArticlesDatabase/StatusesTable.swift b/Frameworks/ArticlesDatabase/StatusesTable.swift index 04916d3de..aa8d3b711 100644 --- a/Frameworks/ArticlesDatabase/StatusesTable.swift +++ b/Frameworks/ArticlesDatabase/StatusesTable.swift @@ -27,11 +27,11 @@ final class StatusesTable: DatabaseTable { // MARK: - Creating/Updating - func ensureStatusesForArticleIDs(_ articleIDs: Set, _ read: Bool, _ database: FMDatabase) -> [String: ArticleStatus] { + func ensureStatusesForArticleIDs(_ articleIDs: Set, _ read: Bool, _ database: FMDatabase) -> ([String: ArticleStatus], Set) { // Check cache. let articleIDsMissingCachedStatus = articleIDsWithNoCachedStatus(articleIDs) if articleIDsMissingCachedStatus.isEmpty { - return statusesDictionary(articleIDs) + return (statusesDictionary(articleIDs), Set()) } // Check database. @@ -43,7 +43,7 @@ final class StatusesTable: DatabaseTable { self.createAndSaveStatusesForArticleIDs(articleIDsNeedingStatus, read, database) } - return statusesDictionary(articleIDs) + return (statusesDictionary(articleIDs), articleIDsNeedingStatus) } func existingStatusesForArticleIDs(_ articleIDs: Set, _ database: FMDatabase) -> [String: ArticleStatus] { @@ -85,10 +85,11 @@ final class StatusesTable: DatabaseTable { return updatedStatuses } - func mark(_ articleIDs: Set, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ database: FMDatabase) { - let statusesDictionary = ensureStatusesForArticleIDs(articleIDs, flag, database) + func markAndFetchNew(_ articleIDs: Set, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ database: FMDatabase) -> Set { + let (statusesDictionary, newStatusIDs) = ensureStatusesForArticleIDs(articleIDs, flag, database) let statuses = Set(statusesDictionary.values) mark(statuses, statusKey, flag, database) + return Set(newStatusIDs.compactMap({ statusesDictionary[$0] })) } // MARK: - Fetching