From 72dec6091c2a39d8515b32fb206afece58e81553 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Mon, 4 Sep 2023 14:17:39 -0700 Subject: [PATCH] Move LocalAccountRefresher from the Account module to the LocalAccount module. Make it use feedURL (String) rather than Feed objects, since it no longer knows about Feed objects. Update LocalAccountRefresherDelegate to match. --- .../LocalAccountDelegate.swift | 83 +++++++-- .../CloudKit/CloudKitAccountDelegate.swift | 91 +++++++-- .../LocalAccount/LocalAccountRefresher.swift | 172 ------------------ SyncClients/LocalAccount/Package.swift | 11 +- .../LocalAccount/LocalAccountRefresher.swift | 122 +++++++++++++ 5 files changed, 279 insertions(+), 200 deletions(-) delete mode 100644 Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift create mode 100644 SyncClients/LocalAccount/Sources/LocalAccount/LocalAccountRefresher.swift diff --git a/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift b/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift index 152c1c28c..eb08e774b 100644 --- a/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift +++ b/Account/Sources/Account/AccountDelegates/LocalAccountDelegate.swift @@ -51,12 +51,13 @@ final class LocalAccountDelegate: AccountDelegate, Logging { } let feeds = account.flattenedFeeds() - refreshProgress.addToNumberOfTasksAndRemaining(feeds.count) + let feedURLs = Set(feeds.map{ $0.url }) + refreshProgress.addToNumberOfTasksAndRemaining(feedURLs.count) let group = DispatchGroup() group.enter() - refresher?.refreshFeeds(feeds) { + refresher?.refreshFeedURLs(feedURLs) { group.leave() } @@ -224,16 +225,45 @@ final class LocalAccountDelegate: AccountDelegate, Logging { } extension LocalAccountDelegate: LocalAccountRefresherDelegate { - - - func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: Feed) { - refreshProgress.completeTask() - } - - func localAccountRefresher(_ refresher: LocalAccountRefresher, articleChanges: ArticleChanges, completion: @escaping () -> Void) { - completion() + + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestForFeedURL feedURL: String) -> URLRequest? { + + guard let url = URL(string: feedURL) else { + return nil + } + + var request = URLRequest(url: url) + if let feed = account?.existingFeed(withURL: feedURL) { + feed.conditionalGetInfo?.addRequestHeadersToURLRequest(&request) + } + + return request } + func localAccountRefresher(_ refresher: LocalAccountRefresher, feedURL: String, response: URLResponse?, data: Data, error: Error?, completion: @escaping () -> Void) { + + guard !data.isEmpty else { + completion() + return + } + + if let error = error { + print("Error downloading \(feedURL) - \(error)") + completion() + return + } + + guard let feed = account?.existingFeed(withURL: feedURL) else { + completion() + return + } + + processFeed(feed, response, data, completion) + } + + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedForFeedURL: String) { + refreshProgress.completeTask() + } } private extension LocalAccountDelegate { @@ -288,9 +318,36 @@ private extension LocalAccountDelegate { self.refreshProgress.completeTask() completion(.failure(AccountError.createErrorNotFound)) } - } - } - + + func processFeed(_ feed: Feed, _ response: URLResponse?, _ data: Data, _ completion: @escaping () -> Void) { + + let dataHash = data.md5String + if dataHash == feed.contentHash { + completion() + return + } + + let parserData = ParserData(url: feed.url, data: data) + FeedParser.parse(parserData) { (parsedFeed, error) in + + Task { @MainActor in + guard let account = self.account, let parsedFeed = parsedFeed, error == nil else { + completion() + return + } + + account.update(feed, with: parsedFeed) { result in + if case .success(_) = result { + if let httpResponse = response as? HTTPURLResponse { + feed.conditionalGetInfo = HTTPConditionalGetInfo(urlResponse: httpResponse) + } + feed.contentHash = dataHash + } + completion() + } + } + } + } } diff --git a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift index fef44df5a..8ac652c11 100644 --- a/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Account/Sources/Account/CloudKit/CloudKitAccountDelegate.swift @@ -591,10 +591,11 @@ private extension CloudKitAccountDelegate { func combinedRefresh(_ account: Account, _ feeds: Set, completion: @escaping (Result) -> Void) { + let feedURLs = Set(feeds.map{ $0.url }) let group = DispatchGroup() group.enter() - refresher.refreshFeeds(feeds) { + refresher.refreshFeedURLs(feedURLs) { group.leave() } @@ -825,17 +826,85 @@ private extension CloudKitAccountDelegate { } extension CloudKitAccountDelegate: LocalAccountRefresherDelegate { - - func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: Feed) { + + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestForFeedURL feedURL: String) -> URLRequest? { + + guard let url = URL(string: feedURL) else { + return nil + } + + var request = URLRequest(url: url) + if let feed = account?.existingFeed(withURL: feedURL) { + feed.conditionalGetInfo?.addRequestHeadersToURLRequest(&request) + } + + return request + } + + func localAccountRefresher(_ refresher: LocalAccountRefresher, feedURL: String, response: URLResponse?, data: Data, error: Error?, completion: @escaping () -> Void) { + + guard !data.isEmpty else { + completion() + return + } + + if let error { + print("Error downloading \(feedURL) - \(error)") + completion() + return + } + + guard let feed = account?.existingFeed(withURL: feedURL) else { + completion() + return + } + + processFeed(feed, response, data, completion) + } + + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedForFeedURL: String) { refreshProgress.completeTask() } - - func localAccountRefresher(_ refresher: LocalAccountRefresher, articleChanges: ArticleChanges, completion: @escaping () -> Void) { - self.storeArticleChanges(new: articleChanges.newArticles, - updated: articleChanges.updatedArticles, - deleted: articleChanges.deletedArticles, - completion: completion) - } - } +private extension CloudKitAccountDelegate { + + func processFeed(_ feed: Feed, _ response: URLResponse?, _ data: Data, _ completion: @escaping () -> Void) { + + let dataHash = data.md5String + if dataHash == feed.contentHash { + completion() + return + } + + let parserData = ParserData(url: feed.url, data: data) + FeedParser.parse(parserData) { (parsedFeed, error) in + + Task { @MainActor in + guard let account = self.account, let parsedFeed = parsedFeed, error == nil else { + completion() + return + } + + account.update(feed, with: parsedFeed) { result in + switch result { + + case .success(let articleChanges): + if let httpResponse = response as? HTTPURLResponse { + feed.conditionalGetInfo = HTTPConditionalGetInfo(urlResponse: httpResponse) + } + feed.contentHash = dataHash + + self.storeArticleChanges(new: articleChanges.newArticles, + updated: articleChanges.updatedArticles, + deleted: articleChanges.deletedArticles, + completion: completion) + + case .failure: + completion() + } + } + } + } + } +} diff --git a/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift b/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift deleted file mode 100644 index 73400d2d4..000000000 --- a/Account/Sources/Account/LocalAccount/LocalAccountRefresher.swift +++ /dev/null @@ -1,172 +0,0 @@ -// -// LocalAccountRefresher.swift -// NetNewsWire -// -// Created by Brent Simmons on 9/6/16. -// Copyright © 2016 Ranchero Software, LLC. All rights reserved. -// - -import Foundation -import RSCore -import RSParser -import RSWeb -import Articles -import ArticlesDatabase - -protocol LocalAccountRefresherDelegate { - func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: Feed) - func localAccountRefresher(_ refresher: LocalAccountRefresher, articleChanges: ArticleChanges, completion: @escaping () -> Void) -} - -final class LocalAccountRefresher { - - private var completion: (() -> Void)? = nil - private var isSuspended = false - var delegate: LocalAccountRefresherDelegate? - - private lazy var downloadSession: DownloadSession = { - return DownloadSession(delegate: self) - }() - - public func refreshFeeds(_ feeds: Set, completion: (() -> Void)? = nil) { - guard !feeds.isEmpty else { - completion?() - return - } - self.completion = completion - downloadSession.downloadObjects(feeds as NSSet) - } - - public func suspend() { - downloadSession.cancelAll() - isSuspended = true - } - - public func resume() { - isSuspended = false - } - -} - -// MARK: - DownloadSessionDelegate - -extension LocalAccountRefresher: DownloadSessionDelegate { - - func downloadSession(_ downloadSession: DownloadSession, requestForRepresentedObject representedObject: AnyObject) -> URLRequest? { - guard let feed = representedObject as? Feed else { - return nil - } - guard let url = URL(string: feed.url) else { - return nil - } - - var request = URLRequest(url: url) - if let conditionalGetInfo = feed.conditionalGetInfo { - conditionalGetInfo.addRequestHeadersToURLRequest(&request) - } - - return request - } - - func downloadSession(_ downloadSession: DownloadSession, downloadDidCompleteForRepresentedObject representedObject: AnyObject, response: URLResponse?, data: Data, error: NSError?, completion: @escaping () -> Void) { - let feed = representedObject as! Feed - - guard !data.isEmpty, !isSuspended else { - completion() - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - return - } - - if let error = error { - print("Error downloading \(feed.url) - \(error)") - completion() - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - return - } - - let dataHash = data.md5String - if dataHash == feed.contentHash { - completion() - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - return - } - - let parserData = ParserData(url: feed.url, data: data) - FeedParser.parse(parserData) { (parsedFeed, error) in - - Task { @MainActor in - guard let account = feed.account, let parsedFeed = parsedFeed, error == nil else { - completion() - self.delegate?.localAccountRefresher(self, requestCompletedFor: feed) - return - } - - account.update(feed, with: parsedFeed) { result in - if case .success(let articleChanges) = result { - if let httpResponse = response as? HTTPURLResponse { - feed.conditionalGetInfo = HTTPConditionalGetInfo(urlResponse: httpResponse) - } - feed.contentHash = dataHash - self.delegate?.localAccountRefresher(self, requestCompletedFor: feed) - self.delegate?.localAccountRefresher(self, articleChanges: articleChanges) { - completion() - } - } else { - completion() - self.delegate?.localAccountRefresher(self, requestCompletedFor: feed) - } - } - } - } - } - - func downloadSession(_ downloadSession: DownloadSession, shouldContinueAfterReceivingData data: Data, representedObject: AnyObject) -> Bool { - let feed = representedObject as! Feed - guard !isSuspended else { - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - return false - } - - if data.isEmpty { - return true - } - - if data.isDefinitelyNotFeed() { - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - return false - } - - return true - } - - func downloadSession(_ downloadSession: DownloadSession, didReceiveUnexpectedResponse response: URLResponse, representedObject: AnyObject) { - let feed = representedObject as! Feed - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - } - - func downloadSession(_ downloadSession: DownloadSession, didReceiveNotModifiedResponse: URLResponse, representedObject: AnyObject) { - let feed = representedObject as! Feed - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - } - - func downloadSession(_ downloadSession: DownloadSession, didDiscardDuplicateRepresentedObject representedObject: AnyObject) { - let feed = representedObject as! Feed - delegate?.localAccountRefresher(self, requestCompletedFor: feed) - } - - func downloadSessionDidCompleteDownloadObjects(_ downloadSession: DownloadSession) { - completion?() - completion = nil - } - -} - -// MARK: - Utility - -private extension Data { - - func isDefinitelyNotFeed() -> Bool { - // We only detect a few image types for now. This should get fleshed-out at some later date. - return self.isImage - } -} diff --git a/SyncClients/LocalAccount/Package.swift b/SyncClients/LocalAccount/Package.swift index 6aed0b1cf..be98a5944 100644 --- a/SyncClients/LocalAccount/Package.swift +++ b/SyncClients/LocalAccount/Package.swift @@ -13,17 +13,20 @@ let package = Package( targets: ["LocalAccount"]), ], dependencies: [ - .package(url: "https://github.com/Ranchero-Software/RSParser.git", .upToNextMajor(from: "2.0.2")), + .package(url: "https://github.com/Ranchero-Software/RSCore.git", .upToNextMajor(from: "2.0.1")), .package(url: "https://github.com/Ranchero-Software/RSWeb.git", .upToNextMajor(from: "1.0.0")), + .package(url: "https://github.com/Ranchero-Software/RSParser.git", .upToNextMajor(from: "2.0.2")), ], - targets: [ + targets: [ // Targets are the basic building blocks of a package. A target can define a module or a test suite. // Targets can depend on other targets in this package, and on products in packages this package depends on. .target( name: "LocalAccount", dependencies: [ - "RSParser", - "RSWeb"] + "RSCore", + "RSWeb", + "RSParser" + ] ), .testTarget( name: "LocalAccountTests", diff --git a/SyncClients/LocalAccount/Sources/LocalAccount/LocalAccountRefresher.swift b/SyncClients/LocalAccount/Sources/LocalAccount/LocalAccountRefresher.swift new file mode 100644 index 000000000..3bccd5d43 --- /dev/null +++ b/SyncClients/LocalAccount/Sources/LocalAccount/LocalAccountRefresher.swift @@ -0,0 +1,122 @@ +// +// LocalAccountRefresher.swift +// NetNewsWire +// +// Created by Brent Simmons on 9/6/16. +// Copyright © 2016 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import RSCore +import RSWeb + +public protocol LocalAccountRefresherDelegate { + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestForFeedURL: String) -> URLRequest? + func localAccountRefresher(_ refresher: LocalAccountRefresher, feedURL: String, response: URLResponse?, data: Data, error: Error?, completion: @escaping () -> Void) + func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedForFeedURL: String) +} + +public final class LocalAccountRefresher { + + private var completion: (() -> Void)? = nil + private var isSuspended = false + public var delegate: LocalAccountRefresherDelegate? + + private lazy var downloadSession: DownloadSession = { + return DownloadSession(delegate: self) + }() + + public init() {} + + public func refreshFeedURLs(_ feedURLs: Set, completion: (() -> Void)? = nil) { + guard !feedURLs.isEmpty else { + completion?() + return + } + self.completion = completion + downloadSession.downloadObjects(feedURLs as NSSet) + } + + public func suspend() { + downloadSession.cancelAll() + isSuspended = true + } + + public func resume() { + isSuspended = false + } +} + +// MARK: - DownloadSessionDelegate + +extension LocalAccountRefresher: DownloadSessionDelegate { + + public func downloadSession(_ downloadSession: DownloadSession, requestForRepresentedObject representedObject: AnyObject) -> URLRequest? { + let feedURL = representedObject as! String + return delegate?.localAccountRefresher(self, requestForFeedURL: feedURL) + } + + public func downloadSession(_ downloadSession: DownloadSession, downloadDidCompleteForRepresentedObject representedObject: AnyObject, response: URLResponse?, data: Data, error: NSError?, completion: @escaping () -> Void) { + + guard !isSuspended else { + completion() + return + } + + let feedURL = representedObject as! String + + delegate?.localAccountRefresher(self, feedURL: feedURL, response: response, data: data, error: error) { + completion() + self.delegate?.localAccountRefresher(self, requestCompletedForFeedURL: feedURL) + } + } + + public func downloadSession(_ downloadSession: DownloadSession, shouldContinueAfterReceivingData data: Data, representedObject: AnyObject) -> Bool { + let feedURL = representedObject as! String + guard !isSuspended else { + delegate?.localAccountRefresher(self, requestCompletedForFeedURL: feedURL) + return false + } + + if data.isEmpty { + return true + } + + if data.isDefinitelyNotFeed() { + delegate?.localAccountRefresher(self, requestCompletedForFeedURL: feedURL) + return false + } + + return true + } + + public func downloadSession(_ downloadSession: DownloadSession, didReceiveUnexpectedResponse response: URLResponse, representedObject: AnyObject) { + let feedURL = representedObject as! String + delegate?.localAccountRefresher(self, requestCompletedForFeedURL: feedURL) + } + + public func downloadSession(_ downloadSession: DownloadSession, didReceiveNotModifiedResponse: URLResponse, representedObject: AnyObject) { + let feedURL = representedObject as! String + delegate?.localAccountRefresher(self, requestCompletedForFeedURL: feedURL) + } + + public func downloadSession(_ downloadSession: DownloadSession, didDiscardDuplicateRepresentedObject representedObject: AnyObject) { + let feedURL = representedObject as! String + delegate?.localAccountRefresher(self, requestCompletedForFeedURL: feedURL) + } + + public func downloadSessionDidCompleteDownloadObjects(_ downloadSession: DownloadSession) { + completion?() + completion = nil + } +} + +// MARK: - Utility + +private extension Data { + + func isDefinitelyNotFeed() -> Bool { + // We only detect a few image types for now. This should get fleshed-out at some later date. + return self.isImage + } +}