From 4c9e98c150d8b828f17bbd4aaa8e4d7660ff3399 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Thu, 7 Nov 2019 20:44:51 +1100 Subject: [PATCH] Ensures refresh tokens occurs before syncing. Implements cancelAll for Feedly. --- .../Feedly/FeedlyAccountDelegate.swift | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 17ffacc7c..37e33ff16 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -15,6 +15,8 @@ import os.log final class FeedlyAccountDelegate: AccountDelegate { + /// Feedly has a sandbox API and a production API. + /// This property is referred to when clients need to know which environment it should be pointing to. static var environment: FeedlyAPICaller.API { #if DEBUG // https://developer.feedly.com/v3/developer/ @@ -61,11 +63,18 @@ final class FeedlyAccountDelegate: AccountDelegate { private let database: SyncDatabase private weak var currentSyncAllOperation: FeedlySyncAllOperation? + private let operationQueue: OperationQueue init(dataFolder: String, transport: Transport?, api: FeedlyAPICaller.API) { + self.operationQueue = OperationQueue() + // Many operations have their own operation queues, such as the sync all operation. + // Making this a serial queue at this higher level of abstraction means we can ensure, + // for example, a `FeedlyRefreshAccessTokenOperation` occurs before a `FeedlySyncAllOperation`, + // improving our ability to debug, reason about and predict the behaviour of the code. + self.operationQueue.maxConcurrentOperationCount = 1 if let transport = transport { - caller = FeedlyAPICaller(transport: transport, api: api) + self.caller = FeedlyAPICaller(transport: transport, api: api) } else { @@ -83,7 +92,7 @@ final class FeedlyAccountDelegate: AccountDelegate { } let session = URLSession(configuration: sessionConfiguration) - caller = FeedlyAPICaller(transport: session, api: api) + self.caller = FeedlyAPICaller(transport: session, api: api) } let databaseFilePath = (dataFolder as NSString).appendingPathComponent("Sync.sqlite3") @@ -93,7 +102,7 @@ final class FeedlyAccountDelegate: AccountDelegate { // MARK: Account API func cancelAll(for account: Account) { - // TODO: Implement me please + operationQueue.cancelAllOperations() } func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { @@ -128,7 +137,7 @@ final class FeedlyAccountDelegate: AccountDelegate { currentSyncAllOperation = operation - OperationQueue.main.addOperation(operation) + operationQueue.addOperation(operation) } func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { @@ -139,7 +148,7 @@ final class FeedlyAccountDelegate: AccountDelegate { completion(.success(())) } } - OperationQueue.main.addOperation(send) + operationQueue.addOperation(send) } /// Attempts to ensure local articles have the same status as they do remotely. @@ -159,18 +168,18 @@ final class FeedlyAccountDelegate: AccountDelegate { let group = DispatchGroup() - let getUnread = FeedlySyncUnreadStatusesOperation(account: account, credentials: credentials, service: caller, newerThan: nil, log: log) + let syncUnread = FeedlySyncUnreadStatusesOperation(account: account, credentials: credentials, service: caller, newerThan: nil, log: log) group.enter() - getUnread.completionBlock = { + syncUnread.completionBlock = { group.leave() } - let getStarred = FeedlySyncStarredArticlesOperation(account: account, credentials: credentials, service: caller, log: log) + let syncStarred = FeedlySyncStarredArticlesOperation(account: account, credentials: credentials, service: caller, log: log) group.enter() - getStarred.completionBlock = { + syncStarred.completionBlock = { group.leave() } @@ -178,7 +187,7 @@ final class FeedlyAccountDelegate: AccountDelegate { completion(.success(())) } - OperationQueue.main.addOperations([getUnread, getStarred], waitUntilFinished: false) + operationQueue.addOperations([syncUnread, syncStarred], waitUntilFinished: false) } func importOPML(for account: Account, opmlFile: URL, completion: @escaping (Result) -> Void) { @@ -477,10 +486,11 @@ final class FeedlyAccountDelegate: AccountDelegate { let client = FeedlyAccountDelegate.oauthAuthorizationClient let refreshAccessToken = FeedlyRefreshAccessTokenOperation(account: account, service: self, oauthClient: client, log: log) - OperationQueue.main.addOperation(refreshAccessToken) + operationQueue.addOperation(refreshAccessToken) } static func validateCredentials(transport: Transport, credentials: Credentials, endpoint: URL?, completion: @escaping (Result) -> Void) { - fatalError() + assertionFailure("An `account` instance should enqueue an \(FeedlyRefreshAccessTokenOperation.self) instead.") + completion(.success(credentials)) } }