From 0d20bccc55fb2e3d1ce2106936bb0ba42687e889 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Tue, 15 Oct 2019 18:31:24 +1100 Subject: [PATCH 1/2] Send and receive starred articles, refactoring status syncing code into operations for better code reuse. --- .../Account/Account.xcodeproj/project.pbxproj | 16 ++- .../Feedly/FeedlyAccountDelegate.swift | 55 ++++---- .../FeedlyArticleStatusCoordinator.swift | 121 ------------------ .../Feedly/FeedlyCompoundOperation.swift | 5 + .../Account/Feedly/FeedlyOperation.swift | 1 - ...lyOrganiseParsedItemsByFeedOperation.swift | 9 +- ...yRefreshStreamEntriesStatusOperation.swift | 25 +++- .../FeedlySendArticleStatusesOperation.swift | 71 ++++++++++ .../FeedlySetStarredArticlesOperation.swift | 54 ++++++++ .../FeedlySyncStarredArticlesOperation.swift | 105 +++++++++++++++ .../Feedly/Refresh/FeedlySyncStrategy.swift | 57 +++------ ...UpdateAccountFeedsWithItemsOperation.swift | 4 +- 12 files changed, 316 insertions(+), 207 deletions(-) delete mode 100644 Frameworks/Account/Feedly/FeedlyArticleStatusCoordinator.swift create mode 100644 Frameworks/Account/Feedly/Refresh/FeedlySendArticleStatusesOperation.swift create mode 100644 Frameworks/Account/Feedly/Refresh/FeedlySetStarredArticlesOperation.swift create mode 100644 Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 83a481911..60b24c587 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -83,6 +83,7 @@ 9E1773D7234575AB0056A5A8 /* FeedlyTag.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1773D6234575AB0056A5A8 /* FeedlyTag.swift */; }; 9E1773D923458D590056A5A8 /* FeedlyResourceId.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1773D823458D590056A5A8 /* FeedlyResourceId.swift */; }; 9E1773DB234593CF0056A5A8 /* FeedlyResourceIdTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1773DA234593CF0056A5A8 /* FeedlyResourceIdTests.swift */; }; + 9E1AF38B2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1AF38A2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift */; }; 9E1D154D233370D800F4944C /* FeedlySyncStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1D154C233370D800F4944C /* FeedlySyncStrategy.swift */; }; 9E1D154F233371DD00F4944C /* FeedlyGetCollectionsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1D154E233371DD00F4944C /* FeedlyGetCollectionsOperation.swift */; }; 9E1D15512334282100F4944C /* FeedlyMirrorCollectionsAsFoldersOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1D15502334282100F4944C /* FeedlyMirrorCollectionsAsFoldersOperation.swift */; }; @@ -113,12 +114,13 @@ 9EAEC626233318400085D7C9 /* FeedlyStream.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EAEC625233318400085D7C9 /* FeedlyStream.swift */; }; 9EAEC62823331C350085D7C9 /* FeedlyCategory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EAEC62723331C350085D7C9 /* FeedlyCategory.swift */; }; 9EAEC62A23331EE70085D7C9 /* FeedlyOrigin.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EAEC62923331EE70085D7C9 /* FeedlyOrigin.swift */; }; - 9EBC31B7233987C1002A567B /* FeedlyArticleStatusCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EBC31B6233987C1002A567B /* FeedlyArticleStatusCoordinator.swift */; }; 9EC688EA232B973C00A8D0A2 /* FeedlyAPICaller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EC688E9232B973C00A8D0A2 /* FeedlyAPICaller.swift */; }; 9EC688EC232C583300A8D0A2 /* FeedlyAccountDelegate+OAuth.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EC688EB232C583300A8D0A2 /* FeedlyAccountDelegate+OAuth.swift */; }; 9EC688EE232C58E800A8D0A2 /* OAuthAuthorizationCodeGranting.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EC688ED232C58E800A8D0A2 /* OAuthAuthorizationCodeGranting.swift */; }; 9ECC9A85234DC16E009B5144 /* FeedlyAccountDelegateError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9ECC9A84234DC16E009B5144 /* FeedlyAccountDelegateError.swift */; }; 9EE4CCFA234F106600FBAE4B /* FeedlyFeedContainerValidator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EE4CCF9234F106600FBAE4B /* FeedlyFeedContainerValidator.swift */; }; + 9EEEF71F23545CB4009E9D80 /* FeedlySendArticleStatusesOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EEEF71E23545CB4009E9D80 /* FeedlySendArticleStatusesOperation.swift */; }; + 9EEEF7212355277F009E9D80 /* FeedlySyncStarredArticlesOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EEEF7202355277F009E9D80 /* FeedlySyncStarredArticlesOperation.swift */; }; 9EF35F7A234E830E003AE2AE /* FeedlyCompoundOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF35F79234E830E003AE2AE /* FeedlyCompoundOperation.swift */; }; /* End PBXBuildFile section */ @@ -261,6 +263,7 @@ 9E1773D6234575AB0056A5A8 /* FeedlyTag.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyTag.swift; sourceTree = ""; }; 9E1773D823458D590056A5A8 /* FeedlyResourceId.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyResourceId.swift; sourceTree = ""; }; 9E1773DA234593CF0056A5A8 /* FeedlyResourceIdTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyResourceIdTests.swift; sourceTree = ""; }; + 9E1AF38A2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlySetStarredArticlesOperation.swift; sourceTree = ""; }; 9E1D154C233370D800F4944C /* FeedlySyncStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlySyncStrategy.swift; sourceTree = ""; }; 9E1D154E233371DD00F4944C /* FeedlyGetCollectionsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetCollectionsOperation.swift; sourceTree = ""; }; 9E1D15502334282100F4944C /* FeedlyMirrorCollectionsAsFoldersOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyMirrorCollectionsAsFoldersOperation.swift; sourceTree = ""; }; @@ -291,12 +294,13 @@ 9EAEC625233318400085D7C9 /* FeedlyStream.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyStream.swift; sourceTree = ""; }; 9EAEC62723331C350085D7C9 /* FeedlyCategory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCategory.swift; sourceTree = ""; }; 9EAEC62923331EE70085D7C9 /* FeedlyOrigin.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyOrigin.swift; sourceTree = ""; }; - 9EBC31B6233987C1002A567B /* FeedlyArticleStatusCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyArticleStatusCoordinator.swift; sourceTree = ""; }; 9EC688E9232B973C00A8D0A2 /* FeedlyAPICaller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAPICaller.swift; sourceTree = ""; }; 9EC688EB232C583300A8D0A2 /* FeedlyAccountDelegate+OAuth.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "FeedlyAccountDelegate+OAuth.swift"; sourceTree = ""; }; 9EC688ED232C58E800A8D0A2 /* OAuthAuthorizationCodeGranting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OAuthAuthorizationCodeGranting.swift; sourceTree = ""; }; 9ECC9A84234DC16E009B5144 /* FeedlyAccountDelegateError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAccountDelegateError.swift; sourceTree = ""; }; 9EE4CCF9234F106600FBAE4B /* FeedlyFeedContainerValidator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyFeedContainerValidator.swift; sourceTree = ""; }; + 9EEEF71E23545CB4009E9D80 /* FeedlySendArticleStatusesOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlySendArticleStatusesOperation.swift; sourceTree = ""; }; + 9EEEF7202355277F009E9D80 /* FeedlySyncStarredArticlesOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlySyncStarredArticlesOperation.swift; sourceTree = ""; }; 9EF35F79234E830E003AE2AE /* FeedlyCompoundOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCompoundOperation.swift; sourceTree = ""; }; D511EEB5202422BB00712EC3 /* Account_project_debug.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_project_debug.xcconfig; sourceTree = ""; }; D511EEB6202422BB00712EC3 /* Account_target.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_target.xcconfig; sourceTree = ""; }; @@ -561,7 +565,6 @@ 9EF35F79234E830E003AE2AE /* FeedlyCompoundOperation.swift */, 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */, 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedOperation.swift */, - 9EBC31B6233987C1002A567B /* FeedlyArticleStatusCoordinator.swift */, 9EBC31B32338AC2E002A567B /* Models */, 9EBC31B22338AC0F002A567B /* Refresh */, ); @@ -580,6 +583,9 @@ 9E1D155A2334423300F4944C /* FeedlyOrganiseParsedItemsByFeedOperation.swift */, 9E1D155C233447F000F4944C /* FeedlyUpdateAccountFeedsWithItemsOperation.swift */, 9E713652233AD63E00765C84 /* FeedlyRefreshStreamEntriesStatusOperation.swift */, + 9EEEF7202355277F009E9D80 /* FeedlySyncStarredArticlesOperation.swift */, + 9E1AF38A2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift */, + 9EEEF71E23545CB4009E9D80 /* FeedlySendArticleStatusesOperation.swift */, ); path = Refresh; sourceTree = ""; @@ -837,6 +843,7 @@ 841974251F6DDCE4006346C4 /* AccountDelegate.swift in Sources */, 510BD113232C3E9D002692E4 /* FeedMetadataFile.swift in Sources */, 5165D73122837F3400D9D53D /* InitialFeedDownloader.swift in Sources */, + 9EEEF71F23545CB4009E9D80 /* FeedlySendArticleStatusesOperation.swift in Sources */, 846E77541F6F00E300A165E2 /* AccountManager.swift in Sources */, 515E4EB72324FF8C0057B0E7 /* Credentials.swift in Sources */, 51E490362288C37100C791F0 /* FeedbinDate.swift in Sources */, @@ -851,6 +858,7 @@ 5154367B228EEB28005E1CDF /* FeedbinImportResult.swift in Sources */, 84B2D4D02238CD8A00498ADA /* FeedMetadata.swift in Sources */, 9EAEC624233315F60085D7C9 /* FeedlyEntry.swift in Sources */, + 9EEEF7212355277F009E9D80 /* FeedlySyncStarredArticlesOperation.swift in Sources */, 5144EA49227B497600D19003 /* FeedbinAPICaller.swift in Sources */, 84B99C9F1FAE8D3200ECDEDB /* ContainerPath.swift in Sources */, 9E510D6E234F16A8002E6F1A /* FeedlyAddFeedRequest.swift in Sources */, @@ -888,8 +896,8 @@ 844B297F210CE37E004020B3 /* UnreadCountProvider.swift in Sources */, 9E1773D5234570E30056A5A8 /* FeedlyEntryParser.swift in Sources */, 9E1D1555233431A600F4944C /* FeedlyOperation.swift in Sources */, + 9E1AF38B2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift in Sources */, 84F1F06E2243524700DA0616 /* AccountMetadata.swift in Sources */, - 9EBC31B7233987C1002A567B /* FeedlyArticleStatusCoordinator.swift in Sources */, 84245C851FDDD8CB0074AFBB /* FeedbinSubscription.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 4f41bd8c0..e399917ae 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -44,7 +44,7 @@ final class FeedlyAccountDelegate: AccountDelegate { private let caller: FeedlyAPICaller private let log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "Feedly") - private let articleStatusCoodinator: FeedlyArticleStatusCoordinator + private let database: SyncDatabase init(dataFolder: String, transport: Transport?, api: FeedlyAPICaller.API = .default) { @@ -70,9 +70,8 @@ final class FeedlyAccountDelegate: AccountDelegate { caller = FeedlyAPICaller(transport: session, api: api) } - articleStatusCoodinator = FeedlyArticleStatusCoordinator(dataFolderPath: dataFolder, - caller: caller, - log: log) + let databaseFilePath = (dataFolder as NSString).appendingPathComponent("Sync.sqlite3") + self.database = SyncDatabase(databaseFilePath: databaseFilePath) } // MARK: Account API @@ -93,7 +92,13 @@ final class FeedlyAccountDelegate: AccountDelegate { func sendArticleStatus(for account: Account, completion: @escaping (() -> Void)) { // Ensure remote articles have the same status as they do locally. - articleStatusCoodinator.sendArticleStatus(for: account, completion: completion) + let send = FeedlySendArticleStatusesOperation(database: database, caller: caller, log: log) + send.completionBlock = { + DispatchQueue.main.async { + completion() + } + } + OperationQueue.main.addOperation(send) } /// Attempts to ensure local articles have the same status as they do remotely. @@ -216,28 +221,6 @@ final class FeedlyAccountDelegate: AccountDelegate { } } - private func isValidContainer(for account: Account, container: Container) throws -> (Folder, String) { - guard let folder = container as? Folder else { - throw FeedlyAccountDelegateError.addFeedChooseFolder - } - - guard let collectionId = folder.externalID else { - throw FeedlyAccountDelegateError.addFeedInvalidFolder(folder) - } - - guard let userId = credentials?.username else { - throw FeedlyAccountDelegateError.notLoggedIn - } - - let uncategorized = FeedlyCategoryResourceId.uncategorized(for: userId) - - guard collectionId != uncategorized.id else { - throw FeedlyAccountDelegateError.addFeedInvalidFolder(folder) - } - - return (folder, collectionId) - } - var createFeedRequest: FeedlyAddFeedRequest? func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { @@ -411,12 +394,18 @@ final class FeedlyAccountDelegate: AccountDelegate { func markArticles(for account: Account, articles: Set
, statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { - let acceptedStatuses = articleStatusCoodinator.articles(articles, - for: account, - didChangeStatus: statusKey, - flag: flag) + let syncStatuses = articles.map { article in + return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) + } - return acceptedStatuses + database.insertStatuses(syncStatuses) + os_log(.debug, log: log, "Marking %@ as %@.", articles.map { $0.title }, syncStatuses) + + if database.selectPendingCount() > 100 { + sendArticleStatus(for: account) { } + } + + return account.update(articles, statusKey: statusKey, flag: flag) } func accountDidInitialize(_ account: Account) { @@ -424,7 +413,7 @@ final class FeedlyAccountDelegate: AccountDelegate { syncStrategy = FeedlySyncStrategy(account: account, caller: caller, - articleStatusCoordinator: articleStatusCoodinator, + database: database, log: log) } diff --git a/Frameworks/Account/Feedly/FeedlyArticleStatusCoordinator.swift b/Frameworks/Account/Feedly/FeedlyArticleStatusCoordinator.swift deleted file mode 100644 index 1450783c9..000000000 --- a/Frameworks/Account/Feedly/FeedlyArticleStatusCoordinator.swift +++ /dev/null @@ -1,121 +0,0 @@ -// -// FeedlyArticleStatusCoordinator.swift -// Account -// -// Created by Kiel Gillard on 24/9/19. -// Copyright © 2019 Ranchero Software, LLC. All rights reserved. -// - -import Foundation -import SyncDatabase -import Articles -import os.log - -final class FeedlyArticleStatusCoordinator { - private let database: SyncDatabase - private let log: OSLog - private let caller: FeedlyAPICaller - - init(dataFolderPath: String, caller: FeedlyAPICaller, log: OSLog) { - let databaseFilePath = (dataFolderPath as NSString).appendingPathComponent("Sync.sqlite3") - self.database = SyncDatabase(databaseFilePath: databaseFilePath) - self.log = log - self.caller = caller - } - - /// Stores a status for a particular article locally. - func articles(_ articles: Set
, for account: Account, didChangeStatus statusKey: ArticleStatus.Key, flag: Bool) -> Set
? { - - let syncStatuses = articles.map { article in - return SyncStatus(articleID: article.articleID, key: statusKey, flag: flag) - } - - database.insertStatuses(syncStatuses) - os_log(.debug, log: log, "Marking %@ as %@.", articles.map { $0.title }, syncStatuses) - - if database.selectPendingCount() > 100 { - sendArticleStatus(for: account) - } - - return account.update(articles, statusKey: statusKey, flag: flag) - } - - /// Ensures local articles have the same status as they do remotely. - func refreshArticleStatus(for account: Account, entries: [FeedlyEntry], completion: @escaping (() -> Void)) { - - let unreadArticleIds = Set(entries.filter { $0.unread }.map { $0.id }) - - // Mark articles as unread - let currentUnreadArticleIDs = account.fetchUnreadArticleIDs() - let deltaUnreadArticleIDs = unreadArticleIds.subtracting(currentUnreadArticleIDs) - let markUnreadArticles = account.fetchArticles(.articleIDs(deltaUnreadArticleIDs)) - account.update(markUnreadArticles, statusKey: .read, flag: false) - - let readAritcleIds = Set(entries.filter { !$0.unread }.map { $0.id }) - - let deltaReadArticleIDs = currentUnreadArticleIDs.intersection(readAritcleIds) - let markReadArticles = account.fetchArticles(.articleIDs(deltaReadArticleIDs)) - account.update(markReadArticles, statusKey: .read, flag: true) - -// os_log(.debug, log: log, "\"%@\" - updated %i UNREAD and %i read article(s).", collection.label, unreadArticleIds.count, markReadArticles.count) - - completion() - - // TODO: starred - -// group.enter() -// caller.retrieveStarredEntries() { result in -// switch result { -// case .success(let articleIDs): -// self.syncArticleStarredState(account: account, articleIDs: articleIDs) -// group.leave() -// case .failure(let error): -// os_log(.info, log: self.log, "Retrieving starred entries failed: %@.", error.localizedDescription) -// group.leave() -// } -// -// } - - } - - /// Ensures remote articles have the same status as they do locally. - func sendArticleStatus(for account: Account, completion: (() -> Void)? = nil) { - os_log(.debug, log: log, "Sending article statuses...") - - let pending = database.selectForProcessing() - - let statuses: [(status: ArticleStatus.Key, flag: Bool, action: FeedlyAPICaller.MarkAction)] = [ - (.read, false, .unread), - (.read, true, .read), - (.starred, true, .saved), - (.starred, false, .unsaved), - ] - - let group = DispatchGroup() - - for pairing in statuses { - let articleIds = pending.filter { $0.key == pairing.status && $0.flag == pairing.flag } - guard !articleIds.isEmpty else { - continue - } - - let ids = Set(articleIds.map { $0.articleID }) - let database = self.database - group.enter() - caller.mark(ids, as: pairing.action) { result in - switch result { - case .success: - database.deleteSelectedForProcessing(Array(ids)) - case .failure: - database.resetSelectedForProcessing(Array(ids)) - } - group.leave() - } - } - - group.notify(queue: DispatchQueue.main) { - os_log(.debug, log: self.log, "Done sending article statuses.") - completion?() - } - } -} diff --git a/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift b/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift index c4dd061f2..06033e8cb 100644 --- a/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift +++ b/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift @@ -37,4 +37,9 @@ final class FeedlyCompoundOperation: FeedlyOperation { operationQueue.addOperations(operationsWithFinish, waitUntilFinished: false) } + + override func cancel() { + operationQueue.cancelAllOperations() + super.cancel() + } } diff --git a/Frameworks/Account/Feedly/FeedlyOperation.swift b/Frameworks/Account/Feedly/FeedlyOperation.swift index 8ede26a1f..abbaba525 100644 --- a/Frameworks/Account/Feedly/FeedlyOperation.swift +++ b/Frameworks/Account/Feedly/FeedlyOperation.swift @@ -24,7 +24,6 @@ class FeedlyOperation: Operation { } func didFinish(_ error: Error) { - assert(delegate != nil) delegate?.feedlyOperation(self, didFailWith: error) didFinish() } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift index ed9ff2ad6..aa2e692c1 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift @@ -11,6 +11,7 @@ import RSParser import os.log protocol FeedlyParsedItemsByFeedProviding { + var providerName: String { get } var allFeeds: Set { get } func parsedItems(for feed: Feed) -> Set? } @@ -22,14 +23,20 @@ final class FeedlyOrganiseParsedItemsByFeedOperation: FeedlyOperation, FeedlyPar private let log: OSLog var allFeeds: Set { + assert(Thread.isMainThread) // Needs to be on main thread because Feed is a main-thread-only model type. let keys = Set(itemsKeyedByFeedId.keys) return account.flattenedFeeds().filter { keys.contains($0.feedID) } } func parsedItems(for feed: Feed) -> Set? { + assert(Thread.isMainThread) // Needs to be on main thread because Feed is a main-thread-only model type. return itemsKeyedByFeedId[feed.feedID] } + var providerName: String { + return entryProvider.resource.id + } + private var itemsKeyedByFeedId = [String: Set]() init(account: Account, entryProvider: FeedlyEntryProviding, log: OSLog) { @@ -61,7 +68,7 @@ final class FeedlyOrganiseParsedItemsByFeedOperation: FeedlyOperation, FeedlyPar guard !isCancelled else { return } } -// os_log(.debug, log: log, "Grouped %i items by %i feeds for %@", items.count, dict.count, parsedItemsProvider.collection.label) + os_log(.debug, log: log, "Grouped %i items by %i feeds for %@", items.count, dict.count, entryProvider.resource.id) itemsKeyedByFeedId = dict } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyRefreshStreamEntriesStatusOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyRefreshStreamEntriesStatusOperation.swift index a4419d7d1..c4a5868da 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyRefreshStreamEntriesStatusOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyRefreshStreamEntriesStatusOperation.swift @@ -14,11 +14,9 @@ final class FeedlyRefreshStreamEntriesStatusOperation: FeedlyOperation { private let account: Account private let entryProvider: FeedlyEntryProviding private let log: OSLog - let articleStatusCoordinator: FeedlyArticleStatusCoordinator - init(account: Account, entryProvider: FeedlyEntryProviding, articleStatusCoordinator: FeedlyArticleStatusCoordinator, log: OSLog) { + init(account: Account, entryProvider: FeedlyEntryProviding, log: OSLog) { self.account = account - self.articleStatusCoordinator = articleStatusCoordinator self.entryProvider = entryProvider self.log = log } @@ -29,8 +27,23 @@ final class FeedlyRefreshStreamEntriesStatusOperation: FeedlyOperation { return } - articleStatusCoordinator.refreshArticleStatus(for: account, entries: entryProvider.entries) { - self.didFinish() - } + let entries = entryProvider.entries + let unreadArticleIds = Set(entries.filter { $0.unread }.map { $0.id }) + + // Mark articles as unread + let currentUnreadArticleIDs = account.fetchUnreadArticleIDs() + let deltaUnreadArticleIDs = unreadArticleIds.subtracting(currentUnreadArticleIDs) + let markUnreadArticles = account.fetchArticles(.articleIDs(deltaUnreadArticleIDs)) + account.update(markUnreadArticles, statusKey: .read, flag: false) + + let readAritcleIds = Set(entries.filter { !$0.unread }.map { $0.id }) + + let deltaReadArticleIDs = currentUnreadArticleIDs.intersection(readAritcleIds) + let markReadArticles = account.fetchArticles(.articleIDs(deltaReadArticleIDs)) + account.update(markReadArticles, statusKey: .read, flag: true) + + // os_log(.debug, log: log, "\"%@\" - updated %i UNREAD and %i read article(s).", collection.label, unreadArticleIds.count, markReadArticles.count) + + didFinish() } } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySendArticleStatusesOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlySendArticleStatusesOperation.swift new file mode 100644 index 000000000..1842ae9e9 --- /dev/null +++ b/Frameworks/Account/Feedly/Refresh/FeedlySendArticleStatusesOperation.swift @@ -0,0 +1,71 @@ +// +// FeedlySendArticleStatusesOperation.swift +// Account +// +// Created by Kiel Gillard on 14/10/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import Articles +import SyncDatabase +import os.log + +/// Single responsibility is to update or ensure articles from the entry provider are the only starred articles. +final class FeedlySendArticleStatusesOperation: FeedlyOperation { + private let database: SyncDatabase + private let log: OSLog + private let caller: FeedlyAPICaller + + init(database: SyncDatabase, caller: FeedlyAPICaller, log: OSLog) { + self.database = database + self.caller = caller + self.log = log + } + + override func main() { + defer { didFinish() } + + guard !isCancelled else { + return + } + + os_log(.debug, log: log, "Sending article statuses...") + + let pending = database.selectForProcessing() + + let statuses: [(status: ArticleStatus.Key, flag: Bool, action: FeedlyAPICaller.MarkAction)] = [ + (.read, false, .unread), + (.read, true, .read), + (.starred, true, .saved), + (.starred, false, .unsaved), + ] + + let group = DispatchGroup() + + for pairing in statuses { + let articleIds = pending.filter { $0.key == pairing.status && $0.flag == pairing.flag } + guard !articleIds.isEmpty else { + continue + } + + let ids = Set(articleIds.map { $0.articleID }) + let database = self.database + group.enter() + caller.mark(ids, as: pairing.action) { result in + switch result { + case .success: + database.deleteSelectedForProcessing(Array(ids)) + case .failure: + database.resetSelectedForProcessing(Array(ids)) + } + group.leave() + } + } + + group.notify(queue: DispatchQueue.main) { + os_log(.debug, log: self.log, "Done sending article statuses.") + self.didFinish() + } + } +} diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySetStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlySetStarredArticlesOperation.swift new file mode 100644 index 000000000..1a1c493cd --- /dev/null +++ b/Frameworks/Account/Feedly/Refresh/FeedlySetStarredArticlesOperation.swift @@ -0,0 +1,54 @@ +// +// FeedlySetStarredArticlesOperation.swift +// Account +// +// Created by Kiel Gillard on 14/10/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import os.log + +/// Single responsibility is to update or ensure articles from the entry provider are the only starred articles. +final class FeedlySetStarredArticlesOperation: FeedlyOperation { + private let account: Account + private let allStarredEntriesProvider: FeedlyEntryProviding + private let log: OSLog + + init(account: Account, allStarredEntriesProvider: FeedlyEntryProviding, log: OSLog) { + self.account = account + self.allStarredEntriesProvider = allStarredEntriesProvider + self.log = log + } + + override func main() { + defer { didFinish() } + + guard !isCancelled else { + return + } + + let remoteStarredArticleIds = Set(allStarredEntriesProvider.entries.map { $0.id }) + let localStarredArticleIDs = account.fetchStarredArticleIDs() + + // Mark articles as starred + let deltaStarredArticleIDs = remoteStarredArticleIds.subtracting(localStarredArticleIDs) + let markStarredArticles = account.fetchArticles(.articleIDs(deltaStarredArticleIDs)) + account.update(markStarredArticles, statusKey: .starred, flag: true) + + // Save any starred statuses for articles we haven't yet received + let markStarredArticleIDs = Set(markStarredArticles.map { $0.articleID }) + let missingStarredArticleIDs = deltaStarredArticleIDs.subtracting(markStarredArticleIDs) + account.ensureStatuses(missingStarredArticleIDs, true, .starred, true) + + // Mark articles as unstarred + let deltaUnstarredArticleIDs = localStarredArticleIDs.subtracting(remoteStarredArticleIds) + let markUnstarredArticles = account.fetchArticles(.articleIDs(deltaUnstarredArticleIDs)) + account.update(markUnstarredArticles, statusKey: .starred, flag: false) + + // Save any unstarred statuses for articles we haven't yet received + let markUnstarredArticleIDs = Set(markUnstarredArticles.map { $0.articleID }) + let missingUnstarredArticleIDs = deltaUnstarredArticleIDs.subtracting(markUnstarredArticleIDs) + account.ensureStatuses(missingUnstarredArticleIDs, true, .starred, false) + } +} diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift new file mode 100644 index 000000000..bc584f046 --- /dev/null +++ b/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift @@ -0,0 +1,105 @@ +// +// FeedlySyncStarredArticlesOperation.swift +// Account +// +// Created by Kiel Gillard on 15/10/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import os.log + +final class FeedlySyncStarredArticlesOperation: FeedlyOperation { + private let account: Account + private let operationQueue: OperationQueue + private let caller: FeedlyAPICaller + private let log: OSLog + + init(account: Account, caller: FeedlyAPICaller, log: OSLog) { + self.account = account + self.caller = caller + self.operationQueue = OperationQueue() + self.log = log + } + + override func cancel() { + operationQueue.cancelAllOperations() + super.cancel() + } + + override func main() { + guard !isCancelled else { + didFinish() + return + } + + guard let user = caller.credentials?.username else { + didFinish(FeedlyAccountDelegateError.notLoggedIn) + return + } + + class Delegate: FeedlyOperationDelegate { + var error: Error? + weak var compoundOperation: FeedlyCompoundOperation? + + func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { + compoundOperation?.cancel() + self.error = error + } + } + + let delegate = Delegate() + + let syncSaved = FeedlyCompoundOperation { + + let saved = FeedlyTagResourceId.saved(for: user) + let getSavedStream = FeedlyGetStreamOperation(account: account, + resource: saved, + caller: caller, + newerThan: nil) + getSavedStream.delegate = delegate + + // set statuses + let setStatuses = FeedlySetStarredArticlesOperation(account: account, + allStarredEntriesProvider: getSavedStream, + log: log) + setStatuses.delegate = delegate + setStatuses.addDependency(getSavedStream) + + // ingest articles + let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, + entryProvider: getSavedStream, + log: log) + organiseByFeed.delegate = delegate + organiseByFeed.addDependency(setStatuses) + + let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, + organisedItemsProvider: organiseByFeed, + log: log) + updateAccount.delegate = delegate + updateAccount.addDependency(organiseByFeed) + + return [getSavedStream, setStatuses, organiseByFeed, updateAccount] + } + + delegate.compoundOperation = syncSaved + + let finalOperation = BlockOperation { [weak self] in + guard let self = self else { + return + } + if let error = delegate.error { + self.didFinish(error) + } else { + self.didFinish() + } + os_log(.debug, log: self.log, "Done syncing starred articles.") + } + + finalOperation.addDependency(syncSaved) + operationQueue.addOperations([syncSaved, finalOperation], waitUntilFinished: false) + + os_log(.debug, log: log, "Syncing starred articles.") + } + +} diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift b/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift index 0ce386cbb..757f20466 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift @@ -8,21 +8,22 @@ import Foundation import os.log +import SyncDatabase final class FeedlySyncStrategy { let account: Account let caller: FeedlyAPICaller let operationQueue: OperationQueue - let articleStatusCoordinator: FeedlyArticleStatusCoordinator + let database: SyncDatabase let log: OSLog - init(account: Account, caller: FeedlyAPICaller, articleStatusCoordinator: FeedlyArticleStatusCoordinator, log: OSLog) { + init(account: Account, caller: FeedlyAPICaller, database: SyncDatabase, log: OSLog) { self.account = account self.caller = caller self.operationQueue = OperationQueue() self.log = log - self.articleStatusCoordinator = articleStatusCoordinator + self.database = database } func cancel() { @@ -48,9 +49,14 @@ final class FeedlySyncStrategy { return } + let sendArticleStatuses = FeedlySendArticleStatusesOperation(database: database, caller: caller, log: log) + sendArticleStatuses.delegate = self + + // Since the truth is in the cloud, everything hinges of what Collections the user has. let getCollections = FeedlyGetCollectionsOperation(caller: caller, log: log) getCollections.delegate = self + getCollections.addDependency(sendArticleStatuses) // Ensure a folder exists for each Collection, removing Folders without a corresponding Collection. let mirrorCollectionsAsFolders = FeedlyMirrorCollectionsAsFoldersOperation(account: account, @@ -79,40 +85,10 @@ final class FeedlySyncStrategy { getCollectionStreams.queueDelegate = self getCollectionStreams.addDependency(getCollections) -// if let user = caller.credentials?.username { -// -// let syncSaved = FeedlyCompoundOperation { -// -// let saved = FeedlyTagResourceId.saved(for: user) -// let getSavedStream = FeedlyGetStreamOperation(account: account, -// resource: saved, -// caller: caller, -// newerThan: newerThan) -// getSavedStream.delegate = self -// -// getSavedStream.addDependency(getCollections) -// getSavedStream.addDependency(mirrorCollectionsAsFolders) -// getSavedStream.addDependency(createFeedsOperation) -// -// let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, -// streamProvider: getSavedStream, -// log: log) -// organiseByFeed.delegate = self -// organiseByFeed.addDependency(getSavedStream) -// -// let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, -// organisedItemsProvider: organiseByFeed, -// log: log) -// updateAccount.delegate = self -// updateAccount.addDependency(organiseByFeed) -// -// // refresh stream entries status -// -// return [getSavedStream, organiseByFeed] -// } -// -// operationQueue.addOperation(syncSaved) -// } + let syncStarred = FeedlySyncStarredArticlesOperation(account: account, caller: caller, log: log) + syncStarred.addDependency(getCollections) + syncStarred.addDependency(mirrorCollectionsAsFolders) + syncStarred.addDependency(createFeedsOperation) // Last operation to perform, which should be dependent on any other operation added to the queue. let syncId = UUID().uuidString @@ -128,18 +104,22 @@ final class FeedlySyncStrategy { } } + completionOperation.addDependency(sendArticleStatuses) completionOperation.addDependency(getCollections) completionOperation.addDependency(mirrorCollectionsAsFolders) completionOperation.addDependency(createFeedsOperation) completionOperation.addDependency(getCollectionStreams) + completionOperation.addDependency(syncStarred) finalOperation = completionOperation startSyncCompletionHandler = completionHandler - let minimumOperations = [getCollections, + let minimumOperations = [sendArticleStatuses, + getCollections, mirrorCollectionsAsFolders, createFeedsOperation, getCollectionStreams, + syncStarred, completionOperation] operationQueue.addOperations(minimumOperations, waitUntilFinished: false) @@ -175,7 +155,6 @@ extension FeedlySyncStrategy: FeedlyRequestStreamsOperationDelegate { // Once the articles are in the account, ensure they have the correct status let ensureUnreadOperation = FeedlyRefreshStreamEntriesStatusOperation(account: account, entryProvider: streamOperation, - articleStatusCoordinator: articleStatusCoordinator, log: log) ensureUnreadOperation.delegate = self diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift index d7fd764d4..c5ce1425c 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift @@ -31,7 +31,7 @@ final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { let allFeeds = organisedItemsProvider.allFeeds -// os_log(.debug, log: log, "Begin updating %i feeds in collection \"%@\"", allFeeds.count, organisedItemsProvider.collection.label) + os_log(.debug, log: log, "Begin updating %i feeds for \"%@\"", allFeeds.count, organisedItemsProvider.providerName) var feedIDsAndItems = [String: Set]() for feed in allFeeds { @@ -41,7 +41,7 @@ final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { feedIDsAndItems[feed.feedID] = items } account.update(feedIDsAndItems: feedIDsAndItems, defaultRead: true) { -// os_log(.debug, log: self.log, "Finished updating feeds in collection \"%@\"", self.organisedItemsProvider.collection.label) + os_log(.debug, log: self.log, "Finished updating feeds in collection \"%@\"", self.organisedItemsProvider.providerName) self.didFinish() } } From 2ecdf92f4007db0b351d3fc3bd33ff421223ebb8 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Tue, 15 Oct 2019 18:39:09 +1100 Subject: [PATCH 2/2] Tweaking the logging to be more useful. --- .../Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift | 4 ++-- .../Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift index bc584f046..550ef5073 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift @@ -53,6 +53,8 @@ final class FeedlySyncStarredArticlesOperation: FeedlyOperation { let syncSaved = FeedlyCompoundOperation { let saved = FeedlyTagResourceId.saved(for: user) + os_log(.debug, log: log, "Getting starred articles from \"%@\".", saved.id) + let getSavedStream = FeedlyGetStreamOperation(account: account, resource: saved, caller: caller, @@ -98,8 +100,6 @@ final class FeedlySyncStarredArticlesOperation: FeedlyOperation { finalOperation.addDependency(syncSaved) operationQueue.addOperations([syncSaved, finalOperation], waitUntilFinished: false) - - os_log(.debug, log: log, "Syncing starred articles.") } } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift index c5ce1425c..f272f84e2 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift @@ -41,7 +41,7 @@ final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { feedIDsAndItems[feed.feedID] = items } account.update(feedIDsAndItems: feedIDsAndItems, defaultRead: true) { - os_log(.debug, log: self.log, "Finished updating feeds in collection \"%@\"", self.organisedItemsProvider.providerName) + os_log(.debug, log: self.log, "Finished updating feeds for \"%@\"", self.organisedItemsProvider.providerName) self.didFinish() } }