From 72f7a7cadf873f9ba349fd398e66c73e4ad6821e Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Wed, 27 Nov 2024 22:26:38 -0800 Subject: [PATCH] Fix downloadProgress for local account. --- .../LocalAccount/LocalAccountDelegate.swift | 35 ++++++--------- .../LocalAccount/LocalAccountRefresher.swift | 8 +++- RSWeb/Sources/RSWeb/DownloadProgress.swift | 3 -- RSWeb/Sources/RSWeb/DownloadSession.swift | 43 ++++++++++++++----- 4 files changed, 51 insertions(+), 38 deletions(-) diff --git a/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift b/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift index 28a2a41d2..ddb83a4e2 100644 --- a/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Account/Sources/Account/LocalAccount/LocalAccountDelegate.swift @@ -25,12 +25,10 @@ final class LocalAccountDelegate: AccountDelegate { weak var account: Account? - private lazy var refresher: LocalAccountRefresher? = { - let refresher = LocalAccountRefresher() - refresher.delegate = self - return refresher + lazy var refreshProgress: DownloadProgress = { + refresher!.downloadProgress }() - + let behaviors: AccountBehaviors = [] let isOPMLImportInProgress = false @@ -38,8 +36,12 @@ final class LocalAccountDelegate: AccountDelegate { var credentials: Credentials? var accountMetadata: AccountMetadata? - let refreshProgress = DownloadProgress(numberOfTasks: 0) - + private lazy var refresher: LocalAccountRefresher? = { + let refresher = LocalAccountRefresher() + refresher.delegate = self + return refresher + }() + func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any], completion: @escaping () -> Void) { completion() } @@ -51,7 +53,6 @@ final class LocalAccountDelegate: AccountDelegate { } let webFeeds = account.flattenedWebFeeds() - refreshProgress.addToNumberOfTasksAndRemaining(webFeeds.count) let group = DispatchGroup() @@ -61,11 +62,9 @@ final class LocalAccountDelegate: AccountDelegate { } group.notify(queue: DispatchQueue.main) { - self.refreshProgress.clear() account.metadata.lastArticleFetchEndTime = Date() completion(.success(())) } - } func syncArticleStatus(for account: Account, completion: ((Result) -> Void)? = nil) { @@ -79,17 +78,17 @@ final class LocalAccountDelegate: AccountDelegate { func refreshArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { completion(.success(())) } - + func importOPML(for account:Account, opmlFile: URL, completion: @escaping (Result) -> Void) { var fileData: Data? - + do { fileData = try Data(contentsOf: opmlFile) } catch { completion(.failure(error)) return } - + guard let opmlData = fileData else { completion(.success(())) return @@ -119,7 +118,6 @@ final class LocalAccountDelegate: AccountDelegate { } completion(.success(())) - } func createWebFeed(for account: Account, url urlString: String, name: String?, container: Container, validateFeed: Bool, completion: @escaping (Result) -> Void) { @@ -230,28 +228,24 @@ private extension LocalAccountDelegate { // container before the name has been downloaded. This will put it in the sidebar // with an Untitled name if we don't delay it being added to the sidebar. BatchUpdate.shared.start() - refreshProgress.addToNumberOfTasksAndRemaining(1) FeedFinder.find(url: url) { result in switch result { case .success(let feedSpecifiers): guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers), let url = URL(string: bestFeedSpecifier.urlString) else { - self.refreshProgress.completeTask() BatchUpdate.shared.end() completion(.failure(AccountError.createErrorNotFound)) return } if account.hasWebFeed(withURL: bestFeedSpecifier.urlString) { - self.refreshProgress.completeTask() BatchUpdate.shared.end() completion(.failure(AccountError.createErrorAlreadySubscribed)) return } InitialFeedDownloader.download(url) { parsedFeed in - self.refreshProgress.completeTask() if let parsedFeed = parsedFeed { let feed = account.createWebFeed(with: nil, url: url.absoluteString, webFeedID: url.absoluteString, homePageURL: nil) @@ -266,17 +260,12 @@ private extension LocalAccountDelegate { BatchUpdate.shared.end() completion(.failure(AccountError.createErrorNotFound)) } - } case .failure: BatchUpdate.shared.end() - self.refreshProgress.completeTask() completion(.failure(AccountError.createErrorNotFound)) } - } - } - } diff --git a/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift b/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift index 98223fa81..b769d0df5 100644 --- a/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift +++ b/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift @@ -19,10 +19,14 @@ protocol LocalAccountRefresherDelegate { final class LocalAccountRefresher { + var delegate: LocalAccountRefresherDelegate? + lazy var downloadProgress: DownloadProgress = { + downloadSession.downloadProgress + }() + private var completion: (() -> Void)? = nil private var isSuspended = false - var delegate: LocalAccountRefresherDelegate? - + private lazy var downloadSession: DownloadSession = { return DownloadSession(delegate: self) }() diff --git a/RSWeb/Sources/RSWeb/DownloadProgress.swift b/RSWeb/Sources/RSWeb/DownloadProgress.swift index 84bfffedf..527d5c3c6 100755 --- a/RSWeb/Sources/RSWeb/DownloadProgress.swift +++ b/RSWeb/Sources/RSWeb/DownloadProgress.swift @@ -30,9 +30,6 @@ public final class DownloadProgress { public var numberRemaining = 0 { didSet { - if numberRemaining == 0 && numberOfTasks != 0 { - numberOfTasks = 0 - } if numberRemaining != oldValue { postDidChangeNotification() } diff --git a/RSWeb/Sources/RSWeb/DownloadSession.swift b/RSWeb/Sources/RSWeb/DownloadSession.swift index aa4cc4b25..74dd873e3 100755 --- a/RSWeb/Sources/RSWeb/DownloadSession.swift +++ b/RSWeb/Sources/RSWeb/DownloadSession.swift @@ -19,7 +19,9 @@ public protocol DownloadSessionDelegate { } @objc public final class DownloadSession: NSObject { - + + public let downloadProgress = DownloadProgress(numberOfTasks: 0) + private var urlSession: URLSession! private var tasksInProgress = Set() private var tasksPending = Set() @@ -71,6 +73,7 @@ public protocol DownloadSessionDelegate { addDataTask(url) } urlsInSession.formUnion(urls) + updateDownloadProgress() } } @@ -79,10 +82,12 @@ public protocol DownloadSessionDelegate { extension DownloadSession: URLSessionTaskDelegate { public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { - tasksInProgress.remove(task) + + defer { + removeTask(task) + } guard let info = infoForTask(task) else { - removeTask(task) return } @@ -106,7 +111,11 @@ extension DownloadSession: URLSessionTaskDelegate { extension DownloadSession: URLSessionDataDelegate { public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) { - + + defer { + updateDownloadProgress() + } + tasksInProgress.insert(dataTask) tasksPending.remove(dataTask) @@ -178,8 +187,8 @@ private extension DownloadSession { } func addDataTaskFromQueueIfNecessary() { - guard tasksPending.count < 500, let representedObject = queue.popLast() else { return } - addDataTask(representedObject) + guard tasksPending.count < 500, let url = queue.popLast() else { return } + addDataTask(url) } func infoForTask(_ task: URLSessionTask) -> DownloadInfo? { @@ -193,10 +202,7 @@ private extension DownloadSession { addDataTaskFromQueueIfNecessary() - if tasksInProgress.count + tasksPending.count < 1 { - urlsInSession.removeAll() - delegate.downloadSessionDidComplete(self) - } + updateDownloadProgress() } func urlStringIsBlackListedRedirect(_ urlString: String) -> Bool { @@ -251,6 +257,23 @@ private extension DownloadSession { return currentString == urlString ? nil : currentString } + // MARK: - Download Progress + + func updateDownloadProgress() { + + downloadProgress.numberOfTasks = urlsInSession.count + + let numberRemaining = tasksPending.count + tasksInProgress.count + queue.count + downloadProgress.numberRemaining = min(numberRemaining, downloadProgress.numberOfTasks) + + // Complete? + if downloadProgress.numberOfTasks > 0 && downloadProgress.numberRemaining < 1 { + delegate.downloadSessionDidComplete(self) + urlsInSession.removeAll() + downloadProgress.numberOfTasks = 0 + } + } + // MARK: - 429 Too Many Requests func handle429Response(_ dataTask: URLSessionDataTask, _ response: URLResponse) {