From 311f5b2e81571eee0fc34c09ee07cc1378254cee Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 18 Dec 2019 09:19:00 +1100 Subject: [PATCH 1/6] Check the account update error when update a Feedly account and finish with an error, indicating to its delegate that remaining operations should cancel. --- .../FeedlyUpdateAccountFeedsWithItemsOperation.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift index a9465d349..b9820981e 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift @@ -31,7 +31,12 @@ final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { let webFeedIDsAndItems = organisedItemsProvider.parsedItemsKeyedByFeedId - account.update(webFeedIDsAndItems: webFeedIDsAndItems, defaultRead: true) { _ in + account.update(webFeedIDsAndItems: webFeedIDsAndItems, defaultRead: true) { databaseError in + if let error = databaseError { + self.didFinish(error) + return + } + os_log(.debug, log: self.log, "Updated %i feeds for \"%@\"", webFeedIDsAndItems.count, self.organisedItemsProvider.providerName) self.didFinish() } From bd307cbb6c6f250d20858f0fd0346ac9b671df15 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 18 Dec 2019 09:26:57 +1100 Subject: [PATCH 2/6] Give the FeedlySyncStarredArticlesOperation a delegate so that if the database becomes suspended, the remainder of the sync operation cancels. --- .../Account/Feedly/Operations/FeedlySyncAllOperation.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift index 9b58de533..595ad0555 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift @@ -70,6 +70,7 @@ final class FeedlySyncAllOperation: FeedlyOperation { // Get each and every starred article. let syncStarred = FeedlySyncStarredArticlesOperation(account: account, credentials: credentials, service: getStarredArticlesService, log: log) + syncStarred.delegate = self syncStarred.downloadProgress = downloadProgress syncStarred.addDependency(createFeedsOperation) self.operationQueue.addOperation(syncStarred) From 6fb0e2e0d099bace1c22ba267ba81dff7d5aee70 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 18 Dec 2019 09:32:58 +1100 Subject: [PATCH 3/6] Honour the error case when ingesting read and star statuses --- .../FeedlySetStarredArticlesOperation.swift | 11 ++++++----- .../Operations/FeedlySetUnreadArticlesOperation.swift | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift index c7713d6ff..1a9a8384d 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift @@ -32,12 +32,13 @@ final class FeedlySetStarredArticlesOperation: FeedlyOperation { return } - account.fetchStarredArticleIDs { (articleIDsResult) in - if let localStarredArticleIDs = try? articleIDsResult.get() { + account.fetchStarredArticleIDs { result in + switch result { + case .success(let localStarredArticleIDs): self.processStarredArticleIDs(localStarredArticleIDs) - } - else { - self.didFinish() + + case .failure(let error): + self.didFinish(error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift index 6ec12bf4c..425557faa 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift @@ -32,12 +32,13 @@ final class FeedlySetUnreadArticlesOperation: FeedlyOperation { return } - account.fetchUnreadArticleIDs { articleIDsResult in - if let localUnreadArticleIDs = try? articleIDsResult.get() { + account.fetchUnreadArticleIDs { result in + switch result { + case .success(let localUnreadArticleIDs): self.processUnreadArticleIDs(localUnreadArticleIDs) - } - else { - self.didFinish() + + case .failure(let error): + self.didFinish(error) } } } From 7ddcb2fc8e9937b647016412f81b3ede63f4ca9d Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 18 Dec 2019 09:41:45 +1100 Subject: [PATCH 4/6] Add optional completions to avoid race conditions involving these marked statuses. --- Frameworks/Account/Account.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 63f19149d..d3b38278b 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -808,23 +808,23 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } /// Mark articleIDs as read. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. - func markAsRead(_ articleIDs: Set) { - mark(articleIDs: articleIDs, statusKey: .read, flag: true) + func markAsRead(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { + mark(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) { - mark(articleIDs: articleIDs, statusKey: .read, flag: false) + func markAsUnread(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { + mark(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) { - mark(articleIDs: articleIDs, statusKey: .starred, flag: true) + func markAsStarred(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { + mark(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) { - mark(articleIDs: articleIDs, statusKey: .starred, flag: false) + func markAsUnstarred(_ articleIDs: Set, completion: DatabaseCompletionBlock? = nil) { + mark(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. From 5a9b138a9d1aa812a66c988de5519c9f61e33dae Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 18 Dec 2019 09:42:08 +1100 Subject: [PATCH 5/6] Update set unraed articles operation to honour database errors. --- .../FeedlySetUnreadArticlesOperation.swift | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift index 425557faa..f98e93e81 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySetUnreadArticlesOperation.swift @@ -53,11 +53,40 @@ private extension FeedlySetUnreadArticlesOperation { } let remoteUnreadArticleIDs = allUnreadIdsProvider.entryIds - account.markAsUnread(remoteUnreadArticleIDs) + guard !remoteUnreadArticleIDs.isEmpty else { + didFinish() + return + } + + let group = DispatchGroup() + + final class ReadStatusResults { + var markAsUnreadError: Error? + var markAsReadError: Error? + } + + let results = ReadStatusResults() + + group.enter() + account.markAsUnread(remoteUnreadArticleIDs) { error in + results.markAsUnreadError = error + group.leave() + } let articleIDsToMarkRead = localUnreadArticleIDs.subtracting(remoteUnreadArticleIDs) - account.markAsRead(articleIDsToMarkRead) + group.enter() + account.markAsRead(articleIDsToMarkRead) { error in + results.markAsReadError = error + group.leave() + } - didFinish() + group.notify(queue: .main) { + let markingError = results.markAsReadError ?? results.markAsUnreadError + guard let error = markingError else { + self.didFinish() + return + } + self.didFinish(error) + } } } From 40dacd65226449dd7b14471c68cf8362aa480312 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 18 Dec 2019 09:45:30 +1100 Subject: [PATCH 6/6] Update set starred articles operation to honour database errors. --- .../FeedlySetStarredArticlesOperation.swift | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift index 1a9a8384d..e3b0623be 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySetStarredArticlesOperation.swift @@ -51,15 +51,42 @@ private extension FeedlySetStarredArticlesOperation { didFinish() return } - - // Mark as starred + let remoteStarredArticleIDs = allStarredEntryIdsProvider.entryIds - account.mark(articleIDs: remoteStarredArticleIDs, statusKey: .starred, flag: true) + guard !remoteStarredArticleIDs.isEmpty else { + didFinish() + return + } + + let group = DispatchGroup() + + final class StarredStatusResults { + var markAsStarredError: Error? + var markAsUnstarredError: Error? + } + + let results = StarredStatusResults() + + group.enter() + account.markAsStarred(remoteStarredArticleIDs) { error in + results.markAsStarredError = error + group.leave() + } - // Mark as unstarred let deltaUnstarredArticleIDs = localStarredArticleIDs.subtracting(remoteStarredArticleIDs) - account.mark(articleIDs: deltaUnstarredArticleIDs, statusKey: .starred, flag: false) + group.enter() + account.markAsUnstarred(deltaUnstarredArticleIDs) { error in + results.markAsUnstarredError = error + group.leave() + } - didFinish() + group.notify(queue: .main) { + let markingError = results.markAsStarredError ?? results.markAsUnstarredError + guard let error = markingError else { + self.didFinish() + return + } + self.didFinish(error) + } } }