diff --git a/Account/Sources/Account/AccountManager.swift b/Account/Sources/Account/AccountManager.swift index 8982ea690..6731e2835 100644 --- a/Account/Sources/Account/AccountManager.swift +++ b/Account/Sources/Account/AccountManager.swift @@ -88,12 +88,9 @@ public final class AccountManager: UnreadCountProvider { } return false } - - public var combinedRefreshProgress: CombinedRefreshProgress { - let downloadProgressArray = activeAccounts.map { $0.refreshProgress } - return CombinedRefreshProgress(downloadProgressArray: downloadProgressArray) - } - + + public let combinedRefreshProgress = CombinedRefreshProgress() + public init(accountsFolder: String) { self.accountsFolder = accountsFolder @@ -243,12 +240,17 @@ public final class AccountManager: UnreadCountProvider { } } - public func refreshAll(errorHandler: @escaping (Error) -> Void, completion: (() -> Void)? = nil) { - guard let reachability = try? Reachability(hostname: "apple.com"), reachability.connection != .unavailable else { return } + public func refreshAll(errorHandler: ((Error) -> Void)? = nil, completion: (() -> Void)? = nil) { + + guard let reachability = try? Reachability(hostname: "apple.com"), reachability.connection != .unavailable else { + return + } + + combinedRefreshProgress.reset() let group = DispatchGroup() - activeAccounts.forEach { account in + for account in activeAccounts { group.enter() account.refreshAll() { result in group.leave() @@ -256,43 +258,16 @@ public final class AccountManager: UnreadCountProvider { case .success: break case .failure(let error): - errorHandler(error) + errorHandler?(error) } } } group.notify(queue: DispatchQueue.main) { + self.combinedRefreshProgress.reset() completion?() } } - - public func refreshAll(completion: (() -> Void)? = nil) { - guard let reachability = try? Reachability(hostname: "apple.com"), reachability.connection != .unavailable else { return } - - var syncErrors = [AccountSyncError]() - let group = DispatchGroup() - - activeAccounts.forEach { account in - group.enter() - account.refreshAll() { result in - group.leave() - switch result { - case .success: - break - case .failure(let error): - syncErrors.append(AccountSyncError(account: account, error: error)) - } - } - } - - group.notify(queue: DispatchQueue.main) { - if syncErrors.count > 0 { - NotificationCenter.default.post(Notification(name: .AccountsDidFailToSyncWithErrors, object: self, userInfo: [Account.UserInfoKey.syncErrors: syncErrors])) - } - completion?() - } - - } public func sendArticleStatusAll(completion: (() -> Void)? = nil) { let group = DispatchGroup() diff --git a/Account/Sources/Account/AccountSyncError.swift b/Account/Sources/Account/AccountSyncError.swift deleted file mode 100644 index efc88da1e..000000000 --- a/Account/Sources/Account/AccountSyncError.swift +++ /dev/null @@ -1,29 +0,0 @@ -// -// AccountSyncError.swift -// Account -// -// Created by Stuart Breckenridge on 24/7/20. -// Copyright © 2020 Ranchero Software, LLC. All rights reserved. -// - -import Foundation -import os.log - -public extension Notification.Name { - static let AccountsDidFailToSyncWithErrors = Notification.Name("AccountsDidFailToSyncWithErrors") -} - -public struct AccountSyncError { - - private static let log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "Application") - public let account: Account - public let error: Error - - init(account: Account, error: Error) { - self.account = account - self.error = error - os_log(.error, log: AccountSyncError.log, "%@", error.localizedDescription) - } - -} - diff --git a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift index dd752c721..7e6bc1b65 100644 --- a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift @@ -87,6 +87,8 @@ final class CloudKitAccountDelegate: AccountDelegate { return } + syncProgress.reset() + let reachability = SCNetworkReachabilityCreateWithName(nil, "apple.com") var flags = SCNetworkReachabilityFlags() guard SCNetworkReachabilityGetFlags(reachability!, &flags), flags.contains(.reachable) else { @@ -511,7 +513,7 @@ private extension CloudKitAccountDelegate { func fail(_ error: Error) { self.processAccountError(account, error) - self.syncProgress.clear() + self.syncProgress.reset() completion(.failure(error)) } @@ -529,7 +531,7 @@ private extension CloudKitAccountDelegate { case .success: self.combinedRefresh(account, webFeeds) { - self.syncProgress.clear() + self.syncProgress.reset() account.metadata.lastArticleFetchEndTime = Date() } @@ -549,7 +551,7 @@ private extension CloudKitAccountDelegate { func fail(_ error: Error) { self.processAccountError(account, error) - self.syncProgress.clear() + self.syncProgress.reset() completion(.failure(error)) } @@ -566,7 +568,7 @@ private extension CloudKitAccountDelegate { self.syncProgress.completeTask() self.combinedRefresh(account, webFeeds) { self.sendArticleStatus(for: account, showProgress: true) { _ in - self.syncProgress.clear() + self.syncProgress.reset() account.metadata.lastArticleFetchEndTime = Date() completion(.success(())) } diff --git a/Account/Sources/Account/CombinedRefreshProgress.swift b/Account/Sources/Account/CombinedRefreshProgress.swift index 803fb9e1c..db2d298e2 100644 --- a/Account/Sources/Account/CombinedRefreshProgress.swift +++ b/Account/Sources/Account/CombinedRefreshProgress.swift @@ -9,34 +9,86 @@ import Foundation import RSWeb -// Combines the refresh progress of multiple accounts into one struct, -// for use by refresh status view and so on. +extension Notification.Name { + public static let combinedRefreshProgressDidChange = Notification.Name("combinedRefreshProgressDidChange") +} -public struct CombinedRefreshProgress { +/// Combine the refresh progress of multiple accounts into one place, +/// for use by refresh status view and so on. +public final class CombinedRefreshProgress { - public let numberOfTasks: Int - public let numberRemaining: Int - public let numberCompleted: Int - public let isComplete: Bool + public private(set) var numberOfTasks = 0 + public private(set) var numberRemaining = 0 + public private(set) var numberCompleted = 0 - init(numberOfTasks: Int, numberRemaining: Int, numberCompleted: Int) { - self.numberOfTasks = max(numberOfTasks, 0) - self.numberRemaining = max(numberRemaining, 0) - self.numberCompleted = max(numberCompleted, 0) - self.isComplete = numberRemaining < 1 + public var isComplete: Bool { + numberRemaining < 1 } - public init(downloadProgressArray: [DownloadProgress]) { - var numberOfTasks = 0 - var numberRemaining = 0 - var numberCompleted = 0 + init() { - for downloadProgress in downloadProgressArray { - numberOfTasks += downloadProgress.numberOfTasks - numberRemaining += downloadProgress.numberRemaining - numberCompleted += downloadProgress.numberCompleted + NotificationCenter.default.addObserver(self, selector: #selector(refreshProgressDidChange(_:)), name: .DownloadProgressDidChange, object: nil) + } + + func reset() { + + let didMakeChange = numberOfTasks != 0 || numberRemaining != 0 || numberCompleted != 0 + + numberOfTasks = 0 + numberRemaining = 0 + numberCompleted = 0 + + if didMakeChange { + postDidChangeNotification() + } + } + + @objc func refreshProgressDidChange(_ notification: Notification) { + + var updatedNumberOfTasks = 0 + var updatedNumberRemaining = 0 + var updatedNumberCompleted = 0 + + var didMakeChange = false + + let downloadProgresses = AccountManager.shared.activeAccounts.map { $0.refreshProgress } + for downloadProgress in downloadProgresses { + updatedNumberOfTasks += downloadProgress.numberOfTasks + updatedNumberRemaining += downloadProgress.numberRemaining + updatedNumberCompleted += downloadProgress.numberCompleted } - self.init(numberOfTasks: numberOfTasks, numberRemaining: numberRemaining, numberCompleted: numberCompleted) + if updatedNumberOfTasks > numberOfTasks { + numberOfTasks = updatedNumberOfTasks + didMakeChange = true + } + + assert(updatedNumberRemaining <= numberOfTasks) + updatedNumberRemaining = max(updatedNumberRemaining, numberRemaining) + updatedNumberRemaining = min(updatedNumberRemaining, numberOfTasks) + if updatedNumberRemaining != numberRemaining { + numberRemaining = updatedNumberRemaining + didMakeChange = true + } + + assert(updatedNumberCompleted <= numberOfTasks) + updatedNumberCompleted = max(updatedNumberCompleted, numberCompleted) + updatedNumberCompleted = min(updatedNumberCompleted, numberOfTasks) + if updatedNumberCompleted != numberCompleted { + numberCompleted = updatedNumberCompleted + didMakeChange = true + } + + if didMakeChange { + postDidChangeNotification() + } + } +} + +private extension CombinedRefreshProgress { + + func postDidChangeNotification() { + + NotificationCenter.default.post(name: .combinedRefreshProgressDidChange, object: self) } } diff --git a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift index 418c1c81a..1d7dcfe49 100644 --- a/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Account/Sources/Account/Feedbin/FeedbinAccountDelegate.swift @@ -80,7 +80,8 @@ final class FeedbinAccountDelegate: AccountDelegate { } func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { - + + refreshProgress.reset() refreshProgress.addToNumberOfTasksAndRemaining(5) refreshAccount(account) { result in @@ -93,7 +94,7 @@ final class FeedbinAccountDelegate: AccountDelegate { completion(.success(())) case .failure(let error): DispatchQueue.main.async { - self.refreshProgress.clear() + self.refreshProgress.reset() let wrappedError = AccountError.wrappedError(error: error, account: account) completion(.failure(wrappedError)) } @@ -102,7 +103,7 @@ final class FeedbinAccountDelegate: AccountDelegate { case .failure(let error): DispatchQueue.main.async { - self.refreshProgress.clear() + self.refreshProgress.reset() let wrappedError = AccountError.wrappedError(error: error, account: account) completion(.failure(wrappedError)) } @@ -719,7 +720,7 @@ private extension FeedbinAccountDelegate { case .success: DispatchQueue.main.async { - self.refreshProgress.clear() + self.refreshProgress.reset() completion(.success(())) } diff --git a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift index a5504742b..2d0bcddb2 100644 --- a/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Account/Sources/Account/Feedly/FeedlyAccountDelegate.swift @@ -123,10 +123,12 @@ final class FeedlyAccountDelegate: AccountDelegate { return } + refreshProgress.reset() + let log = self.log - + let syncAllOperation = FeedlySyncAllOperation(account: account, feedlyUserId: credentials.username, caller: caller, database: database, lastSuccessfulFetchStartDate: accountMetadata?.lastArticleFetchStartTime, downloadProgress: refreshProgress, log: log) - + syncAllOperation.downloadProgress = refreshProgress let date = Date() @@ -138,7 +140,7 @@ final class FeedlyAccountDelegate: AccountDelegate { os_log(.debug, log: log, "Sync took %{public}.3f seconds", -date.timeIntervalSinceNow) completion(result) - self?.refreshProgress.clear() + self?.refreshProgress.reset() } currentSyncAllOperation = syncAllOperation diff --git a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift index b415d03d6..fa3948231 100644 --- a/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift +++ b/Account/Sources/Account/NewsBlur/NewsBlurAccountDelegate.swift @@ -63,6 +63,9 @@ final class NewsBlurAccountDelegate: AccountDelegate { } func refreshAll(for account: Account, completion: @escaping (Result) -> ()) { + + refreshProgress.reset() + self.refreshProgress.addToNumberOfTasksAndRemaining(4) refreshFeeds(for: account) { result in @@ -91,7 +94,7 @@ final class NewsBlurAccountDelegate: AccountDelegate { case .failure(let error): DispatchQueue.main.async { - self.refreshProgress.clear() + self.refreshProgress.reset() let wrappedError = AccountError.wrappedError(error: error, account: account) completion(.failure(wrappedError)) } diff --git a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index e93ffb282..a605a4709 100644 --- a/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Account/Sources/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -105,6 +105,8 @@ final class ReaderAPIAccountDelegate: AccountDelegate { } func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { + + refreshProgress.reset() refreshProgress.addToNumberOfTasksAndRemaining(6) refreshAccount(account) { result in @@ -120,7 +122,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { self.refreshArticleStatus(for: account) { _ in self.refreshProgress.completeTask() self.refreshMissingArticles(account) { - self.refreshProgress.clear() + self.refreshProgress.reset() DispatchQueue.main.async { completion(.success(())) } @@ -135,7 +137,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { case .failure(let error): DispatchQueue.main.async { - self.refreshProgress.clear() + self.refreshProgress.reset() let wrappedError = AccountError.wrappedError(error: error, account: account) if wrappedError.isCredentialsError, let basicCredentials = try? account.retrieveCredentials(type: .readerBasic), let endpoint = account.endpointURL { @@ -405,7 +407,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { case .success(let feedSpecifiers): let feedSpecifiers = feedSpecifiers.filter { !$0.urlString.contains("json") } guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers) else { - self.refreshProgress.clear() + self.refreshProgress.reset() completion(.failure(AccountError.createErrorNotFound)) return } @@ -431,7 +433,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { } case .failure: - self.refreshProgress.clear() + self.refreshProgress.reset() completion(.failure(AccountError.createErrorNotFound)) } @@ -961,7 +963,7 @@ private extension ReaderAPIAccountDelegate { self.refreshArticleStatus(for: account) { _ in self.refreshProgress.completeTask() self.refreshMissingArticles(account) { - self.refreshProgress.clear() + self.refreshProgress.reset() DispatchQueue.main.async { completion(.success(feed)) } diff --git a/Mac/MainWindow/Sidebar/SidebarStatusBarView.swift b/Mac/MainWindow/Sidebar/SidebarStatusBarView.swift index d65733d51..d1e9de7dc 100644 --- a/Mac/MainWindow/Sidebar/SidebarStatusBarView.swift +++ b/Mac/MainWindow/Sidebar/SidebarStatusBarView.swift @@ -21,11 +21,6 @@ final class SidebarStatusBarView: NSView { private var isAnimatingProgress = false - private var progress: CombinedRefreshProgress? = nil { - didSet { - CoalescingQueue.standard.add(self, #selector(updateUI)) - } - } override var isFlipped: Bool { return true } @@ -39,25 +34,20 @@ final class SidebarStatusBarView: NSView { progressLabel.font = NSFont.monospacedDigitSystemFont(ofSize: progressLabelFontSize, weight: NSFont.Weight.regular) progressLabel.stringValue = "" - NotificationCenter.default.addObserver(self, selector: #selector(progressDidChange(_:)), name: .AccountRefreshProgressDidChange, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(progressDidChange(_:)), name: .combinedRefreshProgressDidChange, object: nil) } @objc func updateUI() { - guard let progress = progress else { - stopProgressIfNeeded() - return - } - - updateProgressIndicator(progress) - updateProgressLabel(progress) + updateProgressIndicator() + updateProgressLabel() } // MARK: Notifications @objc dynamic func progressDidChange(_ notification: Notification) { - progress = AccountManager.shared.combinedRefreshProgress + CoalescingQueue.standard.add(self, #selector(updateUI)) } } @@ -73,13 +63,13 @@ private extension SidebarStatusBarView { return } isAnimatingProgress = false - self.progressIndicator.stopAnimation(self) + progressIndicator.stopAnimation(self) progressIndicator.isHidden = true progressLabel.isHidden = true superview?.layoutSubtreeIfNeeded() - NSAnimationContext.runAnimationGroup{ (context) in + NSAnimationContext.runAnimationGroup { context in context.duration = SidebarStatusBarView.animationDuration context.allowsImplicitAnimation = true bottomConstraint.constant = -(heightConstraint.constant) @@ -99,7 +89,7 @@ private extension SidebarStatusBarView { superview?.layoutSubtreeIfNeeded() - NSAnimationContext.runAnimationGroup{ (context) in + NSAnimationContext.runAnimationGroup { context in context.duration = SidebarStatusBarView.animationDuration context.allowsImplicitAnimation = true bottomConstraint.constant = 0 @@ -107,7 +97,9 @@ private extension SidebarStatusBarView { } } - func updateProgressIndicator(_ progress: CombinedRefreshProgress) { + func updateProgressIndicator() { + + let progress = AccountManager.shared.combinedRefreshProgress if progress.isComplete { stopProgressIfNeeded() @@ -127,7 +119,9 @@ private extension SidebarStatusBarView { } } - func updateProgressLabel(_ progress: CombinedRefreshProgress) { + func updateProgressLabel() { + + let progress = AccountManager.shared.combinedRefreshProgress if progress.isComplete { progressLabel.stringValue = "" @@ -135,8 +129,8 @@ private extension SidebarStatusBarView { } let formatString = NSLocalizedString("%@ of %@", comment: "Status bar progress") - let s = NSString(format: formatString as NSString, NSNumber(value: progress.numberCompleted), NSNumber(value: progress.numberOfTasks)) + let s = String(format: formatString, NSNumber(value: progress.numberCompleted), NSNumber(value: progress.numberOfTasks)) - progressLabel.stringValue = s as String + progressLabel.stringValue = s } } diff --git a/RSWeb/Sources/RSWeb/DownloadProgress.swift b/RSWeb/Sources/RSWeb/DownloadProgress.swift index 527d5c3c6..1a3444e76 100755 --- a/RSWeb/Sources/RSWeb/DownloadProgress.swift +++ b/RSWeb/Sources/RSWeb/DownloadProgress.swift @@ -82,8 +82,9 @@ public final class DownloadProgress { } } - public func clear() { + public func reset() { assert(Thread.isMainThread) + numberRemaining = 0 numberOfTasks = 0 } } diff --git a/Shared/Timer/AccountRefreshTimer.swift b/Shared/Timer/AccountRefreshTimer.swift index 04c944a5a..bcf102ac0 100644 --- a/Shared/Timer/AccountRefreshTimer.swift +++ b/Shared/Timer/AccountRefreshTimer.swift @@ -9,7 +9,7 @@ import Foundation import Account -class AccountRefreshTimer { +final class AccountRefreshTimer { var shuttingDown = false @@ -73,8 +73,6 @@ class AccountRefreshTimer { lastTimedRefresh = Date() update() - //AccountManager.shared.refreshAll(errorHandler: ErrorHandler.log) - AccountManager.shared.refreshAll(completion: nil) + AccountManager.shared.refreshAll() } - }