From 98c8135d045b74d9b16ea38ed619f793d9a333f3 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Fri, 27 Oct 2023 21:49:23 -0700 Subject: [PATCH] Convert AccountDelegate.refreshAll to async/await. --- Account/Sources/Account/Account.swift | 29 +++++++----- Account/Sources/Account/AccountDelegate.swift | 2 +- .../FeedbinAccountDelegate.swift | 46 +++++++++---------- .../LocalAccountDelegate.swift | 35 +++++++------- .../NewsBlurAccountDelegate.swift | 15 +++++- .../ReaderAPIAccountDelegate.swift | 15 +++++- Account/Sources/Account/AccountManager.swift | 31 ++++++------- .../CloudKit/CloudKitAccountDelegate.swift | 17 +++++-- .../Feedly/FeedlyAccountDelegate.swift | 34 ++++++++------ Mac/AppDelegate.swift | 4 +- .../AccountsFeedbinWindowController.swift | 11 ++--- .../AccountsNewsBlurWindowController.swift | 11 ++--- .../AccountsPreferencesViewController.swift | 11 ++--- .../AccountsReaderAPIWindowController.swift | 11 ++--- Shared/Timer/AccountRefreshTimer.swift | 4 +- 15 files changed, 154 insertions(+), 122 deletions(-) diff --git a/Account/Sources/Account/Account.swift b/Account/Sources/Account/Account.swift index 82de92c0f..086ae5f1d 100644 --- a/Account/Sources/Account/Account.swift +++ b/Account/Sources/Account/Account.swift @@ -428,8 +428,8 @@ public enum FetchType { await delegate.receiveRemoteNotification(for: self, userInfo: userInfo) } - public func refreshAll(completion: @escaping (Result) -> Void) { - delegate.refreshAll(for: self, completion: completion) + public func refreshAll() async throws { + try await delegate.refreshAll(for: self) } public func sendArticleStatus(completion: ((Result) -> Void)? = nil) { @@ -453,18 +453,23 @@ public enum FetchType { return } - delegate.importOPML(for: self, opmlFile: opmlFile) { [weak self] result in - switch result { - case .success: - guard let self = self else { return } - // Reset the last fetch date to get the article history for the added feeds. - self.metadata.lastArticleFetchStartTime = nil - self.delegate.refreshAll(for: self, completion: completion) - case .failure(let error): - completion(.failure(error)) + delegate.importOPML(for: self, opmlFile: opmlFile) { result in + Task { @MainActor in + switch result { + case .success: + // Reset the last fetch date to get the article history for the added feeds. + self.metadata.lastArticleFetchStartTime = nil + do { + try await self.delegate.refreshAll(for: self) + completion(result) + } catch { + completion(.failure(error)) + } + case .failure: + completion(result) + } } } - } public func suspendNetwork() { diff --git a/Account/Sources/Account/AccountDelegate.swift b/Account/Sources/Account/AccountDelegate.swift index 227c0dd5c..44aad2272 100644 --- a/Account/Sources/Account/AccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegate.swift @@ -25,7 +25,7 @@ import Secrets func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any]) async - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) + func refreshAll(for account: Account) async throws func syncArticleStatus(for account: Account, completion: ((Result) -> Void)?) func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) func refreshArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) diff --git a/Account/Sources/Account/AccountDelegates/FeedbinAccountDelegate.swift b/Account/Sources/Account/AccountDelegates/FeedbinAccountDelegate.swift index 73d33070b..b88cdfe34 100644 --- a/Account/Sources/Account/AccountDelegates/FeedbinAccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegates/FeedbinAccountDelegate.swift @@ -75,37 +75,37 @@ public enum FeedbinAccountDelegateError: String, Error { return } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { - + func refreshAll(for account: Account) async throws { + refreshProgress.addToNumberOfTasksAndRemaining(5) - refreshAccount(account) { result in - switch result { - case .success(): + try await withCheckedThrowingContinuation { continuation in + refreshAccount(account) { result in + switch result { + case .success(): - self.refreshArticlesAndStatuses(account) { result in - switch result { - case .success(): - completion(.success(())) - case .failure(let error): - DispatchQueue.main.async { - self.refreshProgress.clear() - let wrappedError = WrappedAccountError(accountID: account.accountID, accountNameForDisplay: account.nameForDisplay, underlyingError: error) - completion(.failure(wrappedError)) + self.refreshArticlesAndStatuses(account) { result in + switch result { + case .success(): + continuation.resume() + case .failure(let error): + Task { @MainActor in + self.refreshProgress.clear() + let wrappedError = WrappedAccountError(accountID: account.accountID, accountNameForDisplay: account.nameForDisplay, underlyingError: error) + continuation.resume(throwing: wrappedError) + } } } - } - - case .failure(let error): - DispatchQueue.main.async { - self.refreshProgress.clear() - let wrappedError = WrappedAccountError(accountID: account.accountID, accountNameForDisplay: account.nameForDisplay, underlyingError: error) - completion(.failure(wrappedError)) + + case .failure(let error): + Task { @MainActor in + self.refreshProgress.clear() + let wrappedError = WrappedAccountError(accountID: account.accountID, accountNameForDisplay: account.nameForDisplay, underlyingError: error) + continuation.resume(throwing: wrappedError) + } } } - } - } func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { diff --git a/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift b/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift index a6759bd15..73cb92dd7 100644 --- a/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift @@ -24,7 +24,7 @@ public enum LocalAccountDelegateError: String, Error { final class LocalAccountDelegate: AccountDelegate, Logging { weak var account: Account? - + private lazy var refresher: LocalAccountRefresher? = { let refresher = LocalAccountRefresher() refresher.delegate = self @@ -44,28 +44,31 @@ final class LocalAccountDelegate: AccountDelegate, Logging { return } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { + func refreshAll(for account: Account) async throws { guard refreshProgress.isComplete else { - completion(.success(())) return } - let feeds = account.flattenedFeeds() - let feedURLs = Set(feeds.map{ $0.url }) - refreshProgress.addToNumberOfTasksAndRemaining(feedURLs.count) + try await withCheckedThrowingContinuation { continuation in + Task { @MainActor in + let feeds = account.flattenedFeeds() + let feedURLs = Set(feeds.map{ $0.url }) + refreshProgress.addToNumberOfTasksAndRemaining(feedURLs.count) - let group = DispatchGroup() + let group = DispatchGroup() - group.enter() - refresher?.refreshFeedURLs(feedURLs) { - group.leave() - } + group.enter() + refresher?.refreshFeedURLs(feedURLs) { + group.leave() + } - group.notify(queue: DispatchQueue.main) { - self.refreshProgress.clear() - account.metadata.lastArticleFetchEndTime = Date() - completion(.success(())) - } + group.notify(queue: DispatchQueue.main) { + self.refreshProgress.clear() + account.metadata.lastArticleFetchEndTime = Date() + continuation.resume() + } + } + } } diff --git a/Account/Sources/Account/AccountDelegates/NewsBlurAccountDelegate.swift b/Account/Sources/Account/AccountDelegates/NewsBlurAccountDelegate.swift index 2b7829ed6..b0fa288f1 100644 --- a/Account/Sources/Account/AccountDelegates/NewsBlurAccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegates/NewsBlurAccountDelegate.swift @@ -66,7 +66,7 @@ final class NewsBlurAccountDelegate: AccountDelegate, Logging { return } - func refreshAll(for account: Account, completion: @escaping (Result) -> ()) { + private func refreshAll(for account: Account, completion: @escaping (Result) -> ()) { self.refreshProgress.addToNumberOfTasksAndRemaining(4) refreshFeeds(for: account) { result in @@ -118,6 +118,19 @@ final class NewsBlurAccountDelegate: AccountDelegate, Logging { } } + func refreshAll(for account: Account) async throws { + try await withCheckedThrowingContinuation { continuation in + self.refreshAll(for: account) { result in + switch result { + case .success(): + continuation.resume() + case .failure(let error): + continuation.resume(throwing: error) + } + } + } + } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { sendArticleStatus(for: account) { result in switch result { diff --git a/Account/Sources/Account/AccountDelegates/ReaderAPIAccountDelegate.swift b/Account/Sources/Account/AccountDelegates/ReaderAPIAccountDelegate.swift index 06a37d45a..5c37d36c6 100644 --- a/Account/Sources/Account/AccountDelegates/ReaderAPIAccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegates/ReaderAPIAccountDelegate.swift @@ -166,11 +166,22 @@ public enum ReaderAPIAccountDelegateError: LocalizedError { } } } - } - } + func refreshAll(for account: Account) async throws { + try await withCheckedThrowingContinuation { continuation in + self.refreshAll(for: account) { result in + switch result { + case .success(): + continuation.resume() + case .failure(let error): + continuation.resume(throwing: error) + } + } + } + } + func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { guard variant != .inoreader else { completion?(.success(())) diff --git a/Account/Sources/Account/AccountManager.swift b/Account/Sources/Account/AccountManager.swift index ab9750e30..c195e7568 100644 --- a/Account/Sources/Account/AccountManager.swift +++ b/Account/Sources/Account/AccountManager.swift @@ -245,28 +245,23 @@ import RSDatabase } } - public func refreshAll(errorHandler: @escaping @MainActor (Error) -> Void, completion: (() -> Void)? = nil) { + public func refreshAll(errorHandler: @escaping @MainActor (Error) -> Void) async { + guard let reachability = try? Reachability(hostname: "apple.com"), reachability.connection != .unavailable else { return } - let group = DispatchGroup() - - for account in activeAccounts { - group.enter() - account.refreshAll() { result in - group.leave() - switch result { - case .success: - break - case .failure(let error): - Task { @MainActor in - errorHandler(error) - } + await withTaskGroup(of: Void.self) { group in + for account in activeAccounts { + group.addTask { + do { + try await account.refreshAll() + } catch { + Task { @MainActor in + errorHandler(error) + } + } } } - } - - group.notify(queue: DispatchQueue.main) { - completion?() + await group.waitForAll() } } diff --git a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift index 063778e55..889e11024 100644 --- a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift @@ -78,20 +78,27 @@ final class CloudKitAccountDelegate: AccountDelegate, Logging { } } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { + func refreshAll(for account: Account) async throws { guard refreshProgress.isComplete else { - completion(.success(())) return } let reachability = SCNetworkReachabilityCreateWithName(nil, "apple.com") var flags = SCNetworkReachabilityFlags() guard SCNetworkReachabilityGetFlags(reachability!, &flags), flags.contains(.reachable) else { - completion(.success(())) return } - - standardRefreshAll(for: account, completion: completion) + + try await withCheckedThrowingContinuation { continuation in + standardRefreshAll(for: account) { result in + switch result { + case .success(): + continuation.resume() + case .failure(let error): + continuation.resume(throwing: error) + } + } + } } func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { diff --git a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift index 73ed72576..50485d2b2 100644 --- a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift @@ -107,19 +107,17 @@ final class FeedlyAccountDelegate: AccountDelegate, Logging { return } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { + func refreshAll(for account: Account) async throws { assert(Thread.isMainThread) guard currentSyncAllOperation == nil else { self.logger.debug("Ignoring refreshAll: Feedly sync already in progress.") - completion(.success(())) return } guard let credentials = credentials else { self.logger.debug("Ignoring refreshAll: Feedly account has no credentials.") - completion(.failure(FeedlyAccountDelegateError.notLoggedIn)) - return + throw FeedlyAccountDelegateError.notLoggedIn } let syncAllOperation = FeedlySyncAllOperation(account: account, feedlyUserID: credentials.username, caller: caller, database: database, lastSuccessfulFetchStartDate: accountMetadata?.lastArticleFetchStartTime, downloadProgress: refreshProgress) @@ -127,19 +125,25 @@ final class FeedlyAccountDelegate: AccountDelegate, Logging { syncAllOperation.downloadProgress = refreshProgress let date = Date() - syncAllOperation.syncCompletionHandler = { [weak self] result in - if case .success = result { - self?.accountMetadata?.lastArticleFetchStartTime = date - self?.accountMetadata?.lastArticleFetchEndTime = Date() + + try await withCheckedThrowingContinuation { continuation in + syncAllOperation.syncCompletionHandler = { result in + self.logger.debug("Sync took \(-date.timeIntervalSinceNow, privacy: .public) seconds.") + + switch result { + case .success(): + self.accountMetadata?.lastArticleFetchStartTime = date + self.accountMetadata?.lastArticleFetchEndTime = Date() + continuation.resume() + case .failure(let error): + continuation.resume(throwing: error) + } } - - self?.logger.debug("Sync took \(-date.timeIntervalSinceNow, privacy: .public) seconds.") - completion(result) + + currentSyncAllOperation = syncAllOperation + + operationQueue.add(syncAllOperation) } - - currentSyncAllOperation = syncAllOperation - - operationQueue.add(syncAllOperation) } func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { diff --git a/Mac/AppDelegate.swift b/Mac/AppDelegate.swift index eaa2181e2..237849b52 100644 --- a/Mac/AppDelegate.swift +++ b/Mac/AppDelegate.swift @@ -577,7 +577,9 @@ var appDelegate: AppDelegate! } @IBAction func refreshAll(_ sender: Any?) { - AccountManager.shared.refreshAll(errorHandler: ErrorHandler.present) + Task { + await AccountManager.shared.refreshAll(errorHandler: ErrorHandler.present) + } } @IBAction func showAddFeedWindow(_ sender: Any?) { diff --git a/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift b/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift index 61328690d..d2fdf7c5f 100644 --- a/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsFeedbinWindowController.swift @@ -94,13 +94,10 @@ import Secrets try self.account?.removeCredentials(type: .basic) try self.account?.storeCredentials(validatedCredentials) - self.account?.refreshAll() { result in - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) - } + do { + try await self.account?.refreshAll() + } catch { + NSApplication.shared.presentError(error) } self.hostWindow?.endSheet(self.window!, returnCode: NSApplication.ModalResponse.OK) diff --git a/Mac/Preferences/Accounts/AccountsNewsBlurWindowController.swift b/Mac/Preferences/Accounts/AccountsNewsBlurWindowController.swift index fe9602069..ac9b86646 100644 --- a/Mac/Preferences/Accounts/AccountsNewsBlurWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsNewsBlurWindowController.swift @@ -92,13 +92,10 @@ import Secrets try self.account?.storeCredentials(credentials) try self.account?.storeCredentials(validatedCredentials) - self.account?.refreshAll() { result in - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) - } + do { + try await self.account?.refreshAll() + } catch { + NSApplication.shared.presentError(error) } self.hostWindow?.endSheet(self.window!, returnCode: NSApplication.ModalResponse.OK) diff --git a/Mac/Preferences/Accounts/AccountsPreferencesViewController.swift b/Mac/Preferences/Accounts/AccountsPreferencesViewController.swift index 05aaa2dff..b2f77f2ee 100644 --- a/Mac/Preferences/Accounts/AccountsPreferencesViewController.swift +++ b/Mac/Preferences/Accounts/AccountsPreferencesViewController.swift @@ -272,12 +272,11 @@ extension AccountsPreferencesViewController: OAuthAccountAuthorizationOperationD // because the user probably wants to see the result of authorizing NetNewsWire to act on their behalf. NSApp.activate(ignoringOtherApps: true) - account.refreshAll { [weak self] result in - switch result { - case .success: - break - case .failure(let error): - self?.presentError(error) + Task { + do { + try await account.refreshAll() + } catch { + self.presentError(error) } } } diff --git a/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift b/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift index 7f5b52031..4e431d12f 100644 --- a/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsReaderAPIWindowController.swift @@ -151,13 +151,10 @@ import ReaderAPI try self.account?.storeCredentials(credentials) try self.account?.storeCredentials(validatedCredentials) - self.account?.refreshAll() { result in - switch result { - case .success: - break - case .failure(let error): - NSApplication.shared.presentError(error) - } + do { + try await self.account?.refreshAll() + } catch { + NSApplication.shared.presentError(error) } self.hostWindow?.endSheet(self.window!, returnCode: NSApplication.ModalResponse.OK) diff --git a/Shared/Timer/AccountRefreshTimer.swift b/Shared/Timer/AccountRefreshTimer.swift index 9b5310fc1..4ec6e0da8 100644 --- a/Shared/Timer/AccountRefreshTimer.swift +++ b/Shared/Timer/AccountRefreshTimer.swift @@ -73,6 +73,8 @@ import Account lastTimedRefresh = Date() update() - AccountManager.shared.refreshAll(errorHandler: ErrorHandler.log, completion: nil) + Task { + await AccountManager.shared.refreshAll(errorHandler: ErrorHandler.log) + } } }