From d1ca2cac7940341dae61ac4d66940eb9f72a020e Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Fri, 24 Apr 2020 13:33:43 -0500 Subject: [PATCH] Remove username from WebFeed and put it in the URL --- .../CloudKit/CloudKitAccountDelegate.swift | 32 +++++++++---------- .../CloudKit/CloudKitAccountZone.swift | 6 +--- .../CloudKitAccountZoneDelegate.swift | 25 +++++++-------- .../Account/FeedProvider/FeedProvider.swift | 2 +- .../FeedProvider/FeedProviderManager.swift | 10 +++--- .../Twitter/TwitterFeedProvider.swift | 8 ++--- .../LocalAccount/LocalAccountDelegate.swift | 14 +++----- Frameworks/Account/WebFeed.swift | 9 ------ Frameworks/Account/WebFeedMetadata.swift | 9 ------ .../ExtensionPointManager.swift | 4 +-- Shared/Images/WebFeedIconDownloader.swift | 2 +- 11 files changed, 44 insertions(+), 77 deletions(-) diff --git a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift index 42393d4bb..3b3358447 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountDelegate.swift @@ -220,7 +220,7 @@ final class CloudKitAccountDelegate: AccountDelegate { let editedName = name == nil || name!.isEmpty ? nil : name // Username should be part of the URL on new feed adds - if let feedProvider = FeedProviderManager.shared.best(for: urlComponents, with: nil) { + if let feedProvider = FeedProviderManager.shared.best(for: urlComponents) { createProviderWebFeed(for: account, urlComponents: urlComponents, editedName: editedName, container: container, feedProvider: feedProvider, completion: completion) } else { createRSSWebFeed(for: account, url: url, editedName: editedName, container: container, completion: completion) @@ -292,7 +292,7 @@ final class CloudKitAccountDelegate: AccountDelegate { func restoreWebFeed(for account: Account, feed: WebFeed, container: Container, completion: @escaping (Result) -> Void) { refreshProgress.addToNumberOfTasksAndRemaining(1) - accountZone.createWebFeed(url: feed.url, editedName: feed.editedName, username: feed.username, container: container) { result in + accountZone.createWebFeed(url: feed.url, editedName: feed.editedName, container: container) { result in self.refreshProgress.completeTask() switch result { case .success(let externalID): @@ -494,7 +494,7 @@ private extension CloudKitAccountDelegate { self.sendArticleStatus(for: account) { result in switch result { case .success: - self.refreshProgress.completeTask() + self.refreshProgress.clear() completion(.success(())) case .failure(let error): fail(error) @@ -543,6 +543,7 @@ private extension CloudKitAccountDelegate { self.refreshProgress.completeTask() self.refreshWebFeeds(account, webFeeds) { + self.refreshProgress.clear() account.metadata.lastArticleFetchEndTime = Date() } @@ -574,7 +575,7 @@ private extension CloudKitAccountDelegate { refreshProgress.addToNumberOfTasksAndRemaining(2) for webFeed in webFeeds { - if let components = URLComponents(string: webFeed.url), let feedProvider = FeedProviderManager.shared.best(for: components, with: webFeed.username) { + if let components = URLComponents(string: webFeed.url), let feedProvider = FeedProviderManager.shared.best(for: components) { group.enter() feedProvider.refresh(webFeed) { result in switch result { @@ -637,23 +638,19 @@ private extension CloudKitAccountDelegate { case .success(let name): - // Move the user to the WebFeed and out of the URL - var newURLComponents = urlComponents - newURLComponents.user = nil - guard let newURLString = newURLComponents.url?.absoluteString else { + guard let urlString = urlComponents.url?.absoluteString else { completion(.failure(AccountError.createErrorNotFound)) return } - self.accountZone.createWebFeed(url: newURLString, editedName: editedName, username: urlComponents.user, container: container) { result in + self.accountZone.createWebFeed(url: urlString, editedName: editedName, container: container) { result in self.refreshProgress.completeTask() switch result { case .success(let externalID): - let feed = account.createWebFeed(with: name, url: newURLString, webFeedID: newURLString, homePageURL: nil) + let feed = account.createWebFeed(with: name, url: urlString, webFeedID: urlString, homePageURL: nil) feed.editedName = editedName - feed.username = urlComponents.user feed.externalID = externalID container.addWebFeed(feed) @@ -662,7 +659,7 @@ private extension CloudKitAccountDelegate { switch result { case .success(let parsedItems): - account.update(newURLString, with: parsedItems) { result in + account.update(urlString, with: parsedItems) { result in switch result { case .success(let articleChanges): @@ -672,18 +669,20 @@ private extension CloudKitAccountDelegate { self.articlesZone.deleteArticles(deletedArticles) { _ in self.refreshProgress.completeTask() self.articlesZone.sendNewArticles(newArticles) { _ in - self.refreshProgress.completeTask() + self.refreshProgress.clear() completion(.success(feed)) } } case .failure(let error): + self.refreshProgress.clear() completion(.failure(error)) } } case .failure: + self.refreshProgress.clear() completion(.failure(AccountError.createErrorNotFound)) } } @@ -723,7 +722,7 @@ private extension CloudKitAccountDelegate { return } - self.accountZone.createWebFeed(url: bestFeedSpecifier.urlString, editedName: editedName, username: nil, container: container) { result in + self.accountZone.createWebFeed(url: bestFeedSpecifier.urlString, editedName: editedName, container: container) { result in self.refreshProgress.completeTask() switch result { @@ -749,17 +748,19 @@ private extension CloudKitAccountDelegate { self.articlesZone.deleteArticles(deletedArticles) { _ in self.refreshProgress.completeTask() self.articlesZone.sendNewArticles(newArticles) { _ in - self.refreshProgress.completeTask() + self.refreshProgress.clear() completion(.success(feed)) } } case .failure(let error): + self.refreshProgress.clear() completion(.failure(error)) } } } else { + self.refreshProgress.clear() completion(.success(feed)) } @@ -810,7 +811,6 @@ extension CloudKitAccountDelegate: LocalAccountRefresherDelegate { } func localAccountRefresherDidFinish(_ refresher: LocalAccountRefresher) { - refreshProgress.clear() } } diff --git a/Frameworks/Account/CloudKit/CloudKitAccountZone.swift b/Frameworks/Account/CloudKit/CloudKitAccountZone.swift index b812a2eec..69ff6bef2 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountZone.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountZone.swift @@ -29,7 +29,6 @@ final class CloudKitAccountZone: CloudKitZone { struct Fields { static let url = "url" static let editedName = "editedName" - static let username = "username" static let containerExternalIDs = "containerExternalIDs" } } @@ -82,16 +81,13 @@ final class CloudKitAccountZone: CloudKitZone { } /// Persist a web feed record to iCloud and return the external key - func createWebFeed(url: String, editedName: String?, username: String?, container: Container, completion: @escaping (Result) -> Void) { + func createWebFeed(url: String, editedName: String?, container: Container, completion: @escaping (Result) -> Void) { let recordID = CKRecord.ID(recordName: url.md5String, zoneID: Self.zoneID) let record = CKRecord(recordType: CloudKitWebFeed.recordType, recordID: recordID) record[CloudKitWebFeed.Fields.url] = url if let editedName = editedName { record[CloudKitWebFeed.Fields.editedName] = editedName } - if let username = username { - record[CloudKitWebFeed.Fields.username] = username - } guard let containerExternalID = container.externalID else { completion(.failure(CloudKitZoneError.invalidParameter)) diff --git a/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift b/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift index 869971284..deed6f597 100644 --- a/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift +++ b/Frameworks/Account/CloudKit/CloudKitAccountZoneDelegate.swift @@ -13,7 +13,7 @@ import CloudKit class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { - private typealias UnclaimedWebFeed = (url: URL, editedName: String?, username: String?, webFeedExternalID: String) + private typealias UnclaimedWebFeed = (url: URL, editedName: String?, webFeedExternalID: String) private var unclaimedWebFeeds = [String: [UnclaimedWebFeed]]() private var log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "CloudKit") @@ -72,11 +72,10 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { } let editedName = record[CloudKitAccountZone.CloudKitWebFeed.Fields.editedName] as? String - let username = record[CloudKitAccountZone.CloudKitWebFeed.Fields.username] as? String if let webFeed = account.existingWebFeed(withExternalID: record.externalID) { - updateWebFeed(webFeed, editedName: editedName, username: username, containerExternalIDs: containerExternalIDs) + updateWebFeed(webFeed, editedName: editedName, containerExternalIDs: containerExternalIDs) completion() } else { @@ -85,11 +84,11 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { for containerExternalID in containerExternalIDs { group.enter() if let container = account.existingContainer(withExternalID: containerExternalID) { - createWebFeedIfNecessary(url: url, editedName: editedName, username: username, webFeedExternalID: record.externalID, container: container) { webFeed in + createWebFeedIfNecessary(url: url, editedName: editedName, webFeedExternalID: record.externalID, container: container) { webFeed in group.leave() } } else { - addUnclaimedWebFeed(url: url, editedName: editedName, username: username, webFeedExternalID: record.externalID, containerExternalID: containerExternalID) + addUnclaimedWebFeed(url: url, editedName: editedName, webFeedExternalID: record.externalID, containerExternalID: containerExternalID) group.leave() } } @@ -130,7 +129,7 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { for unclaimedWebFeed in unclaimedWebFeeds { group.enter() - createWebFeedIfNecessary(url: unclaimedWebFeed.url, editedName: unclaimedWebFeed.editedName, username: unclaimedWebFeed.username, webFeedExternalID: unclaimedWebFeed.webFeedExternalID, container: folder) { webFeed in + createWebFeedIfNecessary(url: unclaimedWebFeed.url, editedName: unclaimedWebFeed.editedName, webFeedExternalID: unclaimedWebFeed.webFeedExternalID, container: folder) { webFeed in group.leave() } } @@ -157,10 +156,9 @@ class CloudKitAcountZoneDelegate: CloudKitZoneDelegate { private extension CloudKitAcountZoneDelegate { - func updateWebFeed(_ webFeed: WebFeed, editedName: String?, username: String?, containerExternalIDs: [String]) { + func updateWebFeed(_ webFeed: WebFeed, editedName: String?, containerExternalIDs: [String]) { guard let account = account else { return } webFeed.editedName = editedName - webFeed.username = username let existingContainers = account.existingContainers(withWebFeed: webFeed) let existingContainerExternalIds = existingContainers.compactMap { $0.externalID } @@ -181,7 +179,7 @@ private extension CloudKitAcountZoneDelegate { } } - func createWebFeedIfNecessary(url: URL, editedName: String?, username: String?, webFeedExternalID: String, container: Container, completion: @escaping (WebFeed) -> Void) { + func createWebFeedIfNecessary(url: URL, editedName: String?, webFeedExternalID: String, container: Container, completion: @escaping (WebFeed) -> Void) { guard let account = account, let urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) else { return } if let webFeed = account.existingWebFeed(withExternalID: webFeedExternalID) { @@ -191,10 +189,9 @@ private extension CloudKitAcountZoneDelegate { let webFeed = account.createWebFeed(with: nil, url: url.absoluteString, webFeedID: url.absoluteString, homePageURL: nil) webFeed.editedName = editedName - webFeed.username = username webFeed.externalID = webFeedExternalID - if let feedProvider = FeedProviderManager.shared.best(for: urlComponents, with: username) { + if let feedProvider = FeedProviderManager.shared.best(for: urlComponents) { refreshProgress?.addToNumberOfTasksAndRemaining(2) feedProvider.assignName(urlComponents) { result in @@ -241,13 +238,13 @@ private extension CloudKitAcountZoneDelegate { } - func addUnclaimedWebFeed(url: URL, editedName: String?, username: String?, webFeedExternalID: String, containerExternalID: String) { + func addUnclaimedWebFeed(url: URL, editedName: String?, webFeedExternalID: String, containerExternalID: String) { if var unclaimedWebFeeds = self.unclaimedWebFeeds[containerExternalID] { - unclaimedWebFeeds.append(UnclaimedWebFeed(url: url, editedName: editedName, username: username, webFeedExternalID: webFeedExternalID)) + unclaimedWebFeeds.append(UnclaimedWebFeed(url: url, editedName: editedName, webFeedExternalID: webFeedExternalID)) self.unclaimedWebFeeds[containerExternalID] = unclaimedWebFeeds } else { var unclaimedWebFeeds = [UnclaimedWebFeed]() - unclaimedWebFeeds.append(UnclaimedWebFeed(url: url, editedName: editedName, username: username, webFeedExternalID: webFeedExternalID)) + unclaimedWebFeeds.append(UnclaimedWebFeed(url: url, editedName: editedName, webFeedExternalID: webFeedExternalID)) self.unclaimedWebFeeds[containerExternalID] = unclaimedWebFeeds } } diff --git a/Frameworks/Account/FeedProvider/FeedProvider.swift b/Frameworks/Account/FeedProvider/FeedProvider.swift index a11858d39..eecb9d915 100644 --- a/Frameworks/Account/FeedProvider/FeedProvider.swift +++ b/Frameworks/Account/FeedProvider/FeedProvider.swift @@ -19,7 +19,7 @@ public enum FeedProviderAbility { public protocol FeedProvider { /// Informs the caller of the ability for this feed provider to service the given URL - func ability(_ urlComponents: URLComponents, forUsername: String?) -> FeedProviderAbility + func ability(_ urlComponents: URLComponents) -> FeedProviderAbility /// Provide the iconURL of the given URL func iconURL(_ urlComponents: URLComponents, completion: @escaping (Result) -> Void) diff --git a/Frameworks/Account/FeedProvider/FeedProviderManager.swift b/Frameworks/Account/FeedProvider/FeedProviderManager.swift index 486c232c1..4177f550c 100644 --- a/Frameworks/Account/FeedProvider/FeedProviderManager.swift +++ b/Frameworks/Account/FeedProvider/FeedProviderManager.swift @@ -17,21 +17,21 @@ public final class FeedProviderManager { public static let shared = FeedProviderManager() public weak var delegate: FeedProviderManagerDelegate? - public func best(for offered: URLComponents, with username: String?) -> FeedProvider? { - if let owner = feedProviderMatching(offered, forUsername: username, ability: .owner) { + public func best(for offered: URLComponents) -> FeedProvider? { + if let owner = feedProviderMatching(offered, ability: .owner) { return owner } - return feedProviderMatching(offered, forUsername: username, ability: .available) + return feedProviderMatching(offered, ability: .available) } } private extension FeedProviderManager { - func feedProviderMatching(_ offered: URLComponents, forUsername username: String?, ability: FeedProviderAbility) -> FeedProvider? { + func feedProviderMatching(_ offered: URLComponents, ability: FeedProviderAbility) -> FeedProvider? { if let delegate = delegate { for feedProvider in delegate.activeFeedProviders { - if feedProvider.ability(offered, forUsername: username) == ability { + if feedProvider.ability(offered) == ability { return feedProvider } } diff --git a/Frameworks/Account/FeedProvider/Twitter/TwitterFeedProvider.swift b/Frameworks/Account/FeedProvider/Twitter/TwitterFeedProvider.swift index 7c3cf4cd0..6ce028472 100644 --- a/Frameworks/Account/FeedProvider/Twitter/TwitterFeedProvider.swift +++ b/Frameworks/Account/FeedProvider/Twitter/TwitterFeedProvider.swift @@ -88,12 +88,12 @@ public struct TwitterFeedProvider: FeedProvider { version: .oauth1) } - public func ability(_ urlComponents: URLComponents, forUsername username: String?) -> FeedProviderAbility { + public func ability(_ urlComponents: URLComponents) -> FeedProviderAbility { guard urlComponents.host?.hasSuffix("twitter.com") ?? false else { return .none } - if let username = username { + if let username = urlComponents.user { if username == screenName { return .owner } else { @@ -101,10 +101,6 @@ public struct TwitterFeedProvider: FeedProvider { } } - if let user = urlComponents.user, user == screenName { - return .owner - } - return .available } diff --git a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift index 318f72a52..8be696fd0 100644 --- a/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift +++ b/Frameworks/Account/LocalAccount/LocalAccountDelegate.swift @@ -56,7 +56,7 @@ final class LocalAccountDelegate: AccountDelegate { let group = DispatchGroup() for webFeed in webFeeds { - if let components = URLComponents(string: webFeed.url), let feedProvider = FeedProviderManager.shared.best(for: components, with: webFeed.username) { + if let components = URLComponents(string: webFeed.url), let feedProvider = FeedProviderManager.shared.best(for: components) { refreshProgress.addToNumberOfTasksAndRemaining(1) group.enter() feedProvider.refresh(webFeed) { result in @@ -147,7 +147,7 @@ final class LocalAccountDelegate: AccountDelegate { } // Username should be part of the URL on new feed adds - if let feedProvider = FeedProviderManager.shared.best(for: urlComponents, with: nil) { + if let feedProvider = FeedProviderManager.shared.best(for: urlComponents) { createProviderWebFeed(for: account, urlComponents: urlComponents, editedName: name, container: container, feedProvider: feedProvider, completion: completion) } else { createRSSWebFeed(for: account, url: url, editedName: name, container: container, completion: completion) @@ -260,24 +260,20 @@ private extension LocalAccountDelegate { case .success(let name): - // Move the user to the WebFeed and out of the URL - var newURLComponents = urlComponents - newURLComponents.user = nil - guard let newURLString = newURLComponents.url?.absoluteString else { + guard let urlString = urlComponents.url?.absoluteString else { completion(.failure(AccountError.createErrorNotFound)) return } - let feed = account.createWebFeed(with: name, url: newURLString, webFeedID: newURLString, homePageURL: nil) + let feed = account.createWebFeed(with: name, url: urlString, webFeedID: urlString, homePageURL: nil) feed.editedName = editedName - feed.username = urlComponents.user container.addWebFeed(feed) feedProvider.refresh(feed) { result in self.refreshProgress.completeTask() switch result { case .success(let parsedItems): - account.update(newURLString, with: parsedItems) { _ in + account.update(urlString, with: parsedItems) { _ in completion(.success(feed)) } case .failure: diff --git a/Frameworks/Account/WebFeed.swift b/Frameworks/Account/WebFeed.swift index 1d3dec88d..a964bc70b 100644 --- a/Frameworks/Account/WebFeed.swift +++ b/Frameworks/Account/WebFeed.swift @@ -77,15 +77,6 @@ public final class WebFeed: Feed, Renamable, Hashable { } } - public var username: String? { - get { - return metadata.username - } - set { - metadata.username = newValue - } - } - public var name: String? public var authors: Set? { diff --git a/Frameworks/Account/WebFeedMetadata.swift b/Frameworks/Account/WebFeedMetadata.swift index 30323d8a7..424cdc638 100644 --- a/Frameworks/Account/WebFeedMetadata.swift +++ b/Frameworks/Account/WebFeedMetadata.swift @@ -21,7 +21,6 @@ final class WebFeedMetadata: Codable { case homePageURL case iconURL case faviconURL - case username case editedName case authors case contentHash @@ -64,14 +63,6 @@ final class WebFeedMetadata: Codable { } } - var username: String? { - didSet { - if username != oldValue { - valueDidChange(.username) - } - } - } - var editedName: String? { didSet { if editedName != oldValue { diff --git a/Shared/ExtensionPoints/ExtensionPointManager.swift b/Shared/ExtensionPoints/ExtensionPointManager.swift index aecb20ba3..a1810ce95 100644 --- a/Shared/ExtensionPoints/ExtensionPointManager.swift +++ b/Shared/ExtensionPoints/ExtensionPointManager.swift @@ -130,9 +130,9 @@ private extension ExtensionPointManager { } } - func feedProviderMatching(_ offered: URLComponents, forUsername username: String?, ability: FeedProviderAbility) -> FeedProvider? { + func feedProviderMatching(_ offered: URLComponents, ability: FeedProviderAbility) -> FeedProvider? { for extensionPoint in activeExtensionPoints.values { - if let feedProvider = extensionPoint as? FeedProvider, feedProvider.ability(offered, forUsername: username) == ability { + if let feedProvider = extensionPoint as? FeedProvider, feedProvider.ability(offered) == ability { return feedProvider } } diff --git a/Shared/Images/WebFeedIconDownloader.swift b/Shared/Images/WebFeedIconDownloader.swift index f3148570d..25fae6d27 100644 --- a/Shared/Images/WebFeedIconDownloader.swift +++ b/Shared/Images/WebFeedIconDownloader.swift @@ -118,7 +118,7 @@ public final class WebFeedIconDownloader { return nil } - if let components = URLComponents(string: feed.url), let feedProvider = FeedProviderManager.shared.best(for: components, with: nil) { + if let components = URLComponents(string: feed.url), let feedProvider = FeedProviderManager.shared.best(for: components) { feedProvider.iconURL(components) { result in switch result { case .success(let feedProviderURL):