From 438338ac9fc50df9032e88b214a57f85fd07b330 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Fri, 11 Oct 2019 20:32:21 +1100 Subject: [PATCH] Refactor add and create feeds since they differ only by refreshing after adding. --- .../Account/Account.xcodeproj/project.pbxproj | 12 +-- .../Feedly/FeedlyAccountDelegate.swift | 56 ++++------- .../Feedly/FeedlyAccountDelegateError.swift | 8 ++ .../Feedly/FeedlyAddFeedOperation.swift | 5 - .../Account/Feedly/FeedlyAddFeedRequest.swift | 57 ++++++++--- .../Feedly/FeedlyCreateFeedRequest.swift | 99 ------------------- 6 files changed, 78 insertions(+), 159 deletions(-) delete mode 100644 Frameworks/Account/Feedly/FeedlyCreateFeedRequest.swift diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index eeefbb373..5dcec2703 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -91,9 +91,8 @@ 9E1D15572334355900F4944C /* FeedlyRequestStreamsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1D15562334355900F4944C /* FeedlyRequestStreamsOperation.swift */; }; 9E1D155B2334423300F4944C /* FeedlyOrganiseParsedItemsByFeedOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1D155A2334423300F4944C /* FeedlyOrganiseParsedItemsByFeedOperation.swift */; }; 9E1D155D233447F000F4944C /* FeedlyUpdateAccountFeedsWithItemsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E1D155C233447F000F4944C /* FeedlyUpdateAccountFeedsWithItemsOperation.swift */; }; - 9E510D6E234F16A8002E6F1A /* FeedlyCreateFeedRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E510D6D234F16A8002E6F1A /* FeedlyCreateFeedRequest.swift */; }; + 9E510D6E234F16A8002E6F1A /* FeedlyAddFeedRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E510D6D234F16A8002E6F1A /* FeedlyAddFeedRequest.swift */; }; 9E713653233AD63E00765C84 /* FeedlyRefreshStreamEntriesStatusOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E713652233AD63E00765C84 /* FeedlyRefreshStreamEntriesStatusOperation.swift */; }; - 9E7299D523505DC700DAEFB7 /* FeedlyAddFeedRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7299D423505DC700DAEFB7 /* FeedlyAddFeedRequest.swift */; }; 9E7299D723505E9600DAEFB7 /* FeedlyAddFeedOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedOperation.swift */; }; 9E7299D9235062A200DAEFB7 /* FeedlyResourceProviding.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */; }; 9E7F15072341E96700F860D1 /* AccountFeedlySyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7F15062341E96700F860D1 /* AccountFeedlySyncTest.swift */; }; @@ -269,9 +268,8 @@ 9E1D15562334355900F4944C /* FeedlyRequestStreamsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyRequestStreamsOperation.swift; sourceTree = ""; }; 9E1D155A2334423300F4944C /* FeedlyOrganiseParsedItemsByFeedOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyOrganiseParsedItemsByFeedOperation.swift; sourceTree = ""; }; 9E1D155C233447F000F4944C /* FeedlyUpdateAccountFeedsWithItemsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyUpdateAccountFeedsWithItemsOperation.swift; sourceTree = ""; }; - 9E510D6D234F16A8002E6F1A /* FeedlyCreateFeedRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCreateFeedRequest.swift; sourceTree = ""; }; + 9E510D6D234F16A8002E6F1A /* FeedlyAddFeedRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAddFeedRequest.swift; sourceTree = ""; }; 9E713652233AD63E00765C84 /* FeedlyRefreshStreamEntriesStatusOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyRefreshStreamEntriesStatusOperation.swift; sourceTree = ""; }; - 9E7299D423505DC700DAEFB7 /* FeedlyAddFeedRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAddFeedRequest.swift; sourceTree = ""; }; 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAddFeedOperation.swift; sourceTree = ""; }; 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyResourceProviding.swift; sourceTree = ""; }; 9E7F15062341E96700F860D1 /* AccountFeedlySyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFeedlySyncTest.swift; sourceTree = ""; }; @@ -557,8 +555,7 @@ 9EC688EB232C583300A8D0A2 /* FeedlyAccountDelegate+OAuth.swift */, 9EC688ED232C58E800A8D0A2 /* OAuthAuthorizationCodeGranting.swift */, 9EC688E9232B973C00A8D0A2 /* FeedlyAPICaller.swift */, - 9E510D6D234F16A8002E6F1A /* FeedlyCreateFeedRequest.swift */, - 9E7299D423505DC700DAEFB7 /* FeedlyAddFeedRequest.swift */, + 9E510D6D234F16A8002E6F1A /* FeedlyAddFeedRequest.swift */, 9E1D1554233431A600F4944C /* FeedlyOperation.swift */, 9EF35F79234E830E003AE2AE /* FeedlyCompoundOperation.swift */, 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */, @@ -854,7 +851,7 @@ 9EAEC624233315F60085D7C9 /* FeedlyEntry.swift in Sources */, 5144EA49227B497600D19003 /* FeedbinAPICaller.swift in Sources */, 84B99C9F1FAE8D3200ECDEDB /* ContainerPath.swift in Sources */, - 9E510D6E234F16A8002E6F1A /* FeedlyCreateFeedRequest.swift in Sources */, + 9E510D6E234F16A8002E6F1A /* FeedlyAddFeedRequest.swift in Sources */, 5133231122810EB200C30F19 /* FeedbinIcon.swift in Sources */, 846E77501F6EF9C400A165E2 /* LocalAccountRefresher.swift in Sources */, 55203300229D5D5A009559E0 /* ReaderAPICaller.swift in Sources */, @@ -891,7 +888,6 @@ 9E1D1555233431A600F4944C /* FeedlyOperation.swift in Sources */, 84F1F06E2243524700DA0616 /* AccountMetadata.swift in Sources */, 9EBC31B7233987C1002A567B /* FeedlyArticleStatusCoordinator.swift in Sources */, - 9E7299D523505DC700DAEFB7 /* FeedlyAddFeedRequest.swift in Sources */, 84245C851FDDD8CB0074AFBB /* FeedbinSubscription.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 359f6b551..4f41bd8c0 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -238,18 +238,18 @@ final class FeedlyAccountDelegate: AccountDelegate { return (folder, collectionId) } - var createFeedRequest: FeedlyCreateFeedRequest? + var createFeedRequest: FeedlyAddFeedRequest? func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result) -> Void) { let progress = refreshProgress progress.addToNumberOfTasksAndRemaining(1) - let createFeedRequest = FeedlyCreateFeedRequest(account: account, caller: caller, container: container, log: log) + let request = FeedlyAddFeedRequest(account: account, caller: caller, container: container, log: log) - self.createFeedRequest = createFeedRequest + self.createFeedRequest = request - createFeedRequest.start(url: url, name: name) { [weak self] result in + request.addNewFeed(at: url, name: name) { [weak self] result in progress.completeTask() self?.createFeedRequest = nil completion(result) @@ -259,7 +259,7 @@ final class FeedlyAccountDelegate: AccountDelegate { func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result) -> Void) { let folderCollectionIds = account.folders?.filter { $0.has(feed) }.compactMap { $0.externalID } guard let collectionIds = folderCollectionIds, let collectionId = collectionIds.first else { - completion(.failure(FeedbinAccountDelegateError.invalidParameter)) + completion(.failure(FeedlyAccountDelegateError.unableToRenameFeed(feed.nameForDisplay, name))) return } @@ -283,41 +283,25 @@ final class FeedlyAccountDelegate: AccountDelegate { feed.editedName = name } - func addFeed(for account: Account, with: Feed, to container: Container, completion: @escaping (Result) -> Void) { - let (folder, collectionId): (Folder, String) - do { - (folder, collectionId) = try isValidContainer(for: account, container: container) - } catch { - return DispatchQueue.main.async { - completion(.failure(error)) - } - } + var addFeedRequest: FeedlyAddFeedRequest? + + func addFeed(for account: Account, with feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { + + let progress = refreshProgress + progress.addToNumberOfTasksAndRemaining(1) - let feedId = FeedlyFeedResourceId(id: with.feedID) + let request = FeedlyAddFeedRequest(account: account, caller: caller, container: container, log: log) - caller.addFeed(with: feedId, toCollectionWith: collectionId) { result in + self.addFeedRequest = request + + request.add(existing: feed) { [weak self] result in + progress.completeTask() + + self?.addFeedRequest = nil + switch result { - case .success(let feedlyFeeds): - for feedlyFeed in feedlyFeeds where !folder.hasFeed(with: feedlyFeed.feedId) { - let feed: Feed = { - if with.url == FeedlyFeedResourceId(id: feedlyFeed.id).url { - with.metadata.feedID = feedlyFeed.id - with.name = feedlyFeed.title - with.homePageURL = feedlyFeed.website - return with - } else { - let resourceId = FeedlyFeedResourceId(id: feedlyFeed.id) - return account.createFeed(with: feedlyFeed.title, - url: resourceId.url, - feedID: feedlyFeed.id, - homePageURL: feedlyFeed.website) - } - }() - folder.addFeed(feed) - } - + case .success: completion(.success(())) - case .failure(let error): completion(.failure(error)) } diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegateError.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegateError.swift index d601f3122..ed23a71cc 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegateError.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegateError.swift @@ -16,6 +16,7 @@ enum FeedlyAccountDelegateError: LocalizedError { case unableToMoveFeedBetweenFolders(Feed, Folder, Folder) case addFeedChooseFolder case addFeedInvalidFolder(Folder) + case unableToRenameFeed(String, String) case unableToRemoveFeed(Feed) var errorDescription: String? { @@ -46,6 +47,10 @@ enum FeedlyAccountDelegateError: LocalizedError { let template = NSLocalizedString("Feeds cannot be added to the \"%@\" folder.", comment: "Feedly - Feed can only be added to folders.") return String(format: template, invalidFolder.nameForDisplay) + case .unableToRenameFeed(let from, let to): + let template = NSLocalizedString("Could not rename \"%@\" to \"%@\".", comment: "Feedly - Could not rename a feed.") + return String(format: template, from, to) + case .unableToRemoveFeed(let feed): let template = NSLocalizedString("Could not remove \"%@\".", comment: "Feedly - Could not remove a feed.") return String(format: template, feed.nameForDisplay) @@ -78,6 +83,9 @@ enum FeedlyAccountDelegateError: LocalizedError { case .unableToRemoveFeed: return nil + + case .unableToRenameFeed: + return nil } } } diff --git a/Frameworks/Account/Feedly/FeedlyAddFeedOperation.swift b/Frameworks/Account/Feedly/FeedlyAddFeedOperation.swift index 60d2fd7b7..71f35f345 100644 --- a/Frameworks/Account/Feedly/FeedlyAddFeedOperation.swift +++ b/Frameworks/Account/Feedly/FeedlyAddFeedOperation.swift @@ -16,11 +16,6 @@ final class FeedlyAddFeedOperation: FeedlyOperation, FeedlyFeedsAndFoldersProvid let folder: Folder let feedResource: FeedlyFeedResourceId - convenience init(account: Account, folder: Folder, url: String, feedName: String? = nil, collectionId: String, caller: FeedlyAPICaller) { - let resource = FeedlyFeedResourceId(url: url) - self.init(account: account, folder: folder, feedResource: resource, feedName: feedName, collectionId: collectionId, caller: caller) - } - init(account: Account, folder: Folder, feedResource: FeedlyFeedResourceId, feedName: String? = nil, collectionId: String, caller: FeedlyAPICaller) { self.account = account self.folder = folder diff --git a/Frameworks/Account/Feedly/FeedlyAddFeedRequest.swift b/Frameworks/Account/Feedly/FeedlyAddFeedRequest.swift index 3bb53b827..ae44f0cd6 100644 --- a/Frameworks/Account/Feedly/FeedlyAddFeedRequest.swift +++ b/Frameworks/Account/Feedly/FeedlyAddFeedRequest.swift @@ -1,8 +1,8 @@ // -// FeedlyAddFeedRequest.swift +// FeedlyCreateFeedRequest.swift // Account // -// Created by Kiel Gillard on 11/10/19. +// Created by Kiel Gillard on 10/10/19. // Copyright © 2019 Ranchero Software, LLC. All rights reserved. // @@ -37,7 +37,17 @@ final class FeedlyAddFeedRequest { } } - func start(adding feed: Feed, to container: Container, completion: @escaping (Result) -> Void) { + func addNewFeed(at url: String, name: String? = nil, completion: @escaping (Result) -> Void) { + let resource = FeedlyFeedResourceId(url: url) + self.start(resource: resource, name: name, refreshes: true, completion: completion) + } + + func add(existing feed: Feed, name: String? = nil, completion: @escaping (Result) -> Void) { + let resource = FeedlyFeedResourceId(id: feed.feedID) + self.start(resource: resource, name: name, refreshes: false, completion: completion) + } + + private func start(resource: FeedlyFeedResourceId, name: String?, refreshes: Bool, completion: @escaping (Result) -> Void) { let (folder, collectionId): (Folder, String) do { @@ -49,21 +59,46 @@ final class FeedlyAddFeedRequest { } } - let resource = FeedlyFeedResourceId(id: feed.feedID) - let delegate = Delegate(resourceProvider: resource) delegate.completionHandler = completion - let addFeed = FeedlyCompoundOperation() { - let addRequest = FeedlyAddFeedOperation(account: account, folder: folder, feedResource: resource, collectionId: collectionId, caller: caller) + let createFeed = FeedlyCompoundOperation() { + let addRequest = FeedlyAddFeedOperation(account: account, folder: folder, feedResource: resource, feedName: name, collectionId: collectionId, caller: caller) let createFeeds = FeedlyCreateFeedsForCollectionFoldersOperation(account: account, feedsAndFoldersProvider: addRequest, log: log) - createFeeds.addDependency(addRequest) - let operations = [addRequest, createFeeds] + let getStream: FeedlyGetStreamOperation? = { + if refreshes { + let op = FeedlyGetStreamOperation(account: account, resourceProvider: addRequest, caller: caller, newerThan: nil) + op.addDependency(createFeeds) + return op + } + return nil + }() + + let organiseByFeed: FeedlyOrganiseParsedItemsByFeedOperation? = { + if let getStream = getStream { + let op = FeedlyOrganiseParsedItemsByFeedOperation(account: account, entryProvider: getStream, log: log) + op.addDependency(getStream) + return op + } + return nil + }() + + let updateAccount: FeedlyUpdateAccountFeedsWithItemsOperation? = { + if let organiseByFeed = organiseByFeed { + let op = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, organisedItemsProvider: organiseByFeed, log: log) + op.addDependency(organiseByFeed) + return op + } + return nil + }() + + let operations = [addRequest, createFeeds, getStream, organiseByFeed, updateAccount].compactMap { $0 } for operation in operations { + assert(operation.isReady == (operation === addRequest), "Only the add request operation should be ready.") operation.delegate = delegate } @@ -88,8 +123,8 @@ final class FeedlyAddFeedRequest { } } - callback.addDependency(addFeed) + callback.addDependency(createFeed) - OperationQueue.main.addOperations([addFeed, callback], waitUntilFinished: false) + OperationQueue.main.addOperations([createFeed, callback], waitUntilFinished: false) } } diff --git a/Frameworks/Account/Feedly/FeedlyCreateFeedRequest.swift b/Frameworks/Account/Feedly/FeedlyCreateFeedRequest.swift deleted file mode 100644 index b22adf945..000000000 --- a/Frameworks/Account/Feedly/FeedlyCreateFeedRequest.swift +++ /dev/null @@ -1,99 +0,0 @@ -// -// FeedlyCreateFeedRequest.swift -// Account -// -// Created by Kiel Gillard on 10/10/19. -// Copyright © 2019 Ranchero Software, LLC. All rights reserved. -// - -import Foundation -import os.log - -final class FeedlyCreateFeedRequest { - let account: Account - let caller: FeedlyAPICaller - let container: Container - let log: OSLog - - init(account: Account, caller: FeedlyAPICaller, container: Container, log: OSLog) { - self.account = account - self.caller = caller - self.container = container - self.log = log - } - - private class Delegate: FeedlyOperationDelegate { - let resourceProvider: FeedlyResourceProviding - - init(resourceProvider: FeedlyResourceProviding) { - self.resourceProvider = resourceProvider - } - - var completionHandler: ((Result) -> ())? - var error: Error? - - func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { - self.error = error - } - } - - func start(url: String, name: String?, completion: @escaping (Result) -> Void) { - - let (folder, collectionId): (Folder, String) - do { - let validator = FeedlyFeedContainerValidator(container: container, userId: caller.credentials?.username) - (folder, collectionId) = try validator.getValidContainer() - } catch { - return DispatchQueue.main.async { - completion(.failure(error)) - } - } - - let subscribeRequest = FeedlyAddFeedOperation(account: account, folder: folder, url: url, feedName: name, collectionId: collectionId, caller: caller) - - let delegate = Delegate(resourceProvider: subscribeRequest) - delegate.completionHandler = completion - - let createFeed = FeedlyCompoundOperation() { - let createFeeds = FeedlyCreateFeedsForCollectionFoldersOperation(account: account, feedsAndFoldersProvider: subscribeRequest, log: log) - let getStream = FeedlyGetStreamOperation(account: account, resourceProvider: subscribeRequest, caller: caller, newerThan: nil) - let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, entryProvider: getStream, log: log) - let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, organisedItemsProvider: organiseByFeed, log: log) - - createFeeds.addDependency(subscribeRequest) - getStream.addDependency(createFeeds) - organiseByFeed.addDependency(getStream) - updateAccount.addDependency(organiseByFeed) - - let operations = [subscribeRequest, createFeeds, getStream, organiseByFeed, updateAccount] - - for operation in operations { - operation.delegate = delegate - } - - return operations - } - - let callback = BlockOperation() { - guard let handler = delegate.completionHandler else { - return - } - - defer { delegate.completionHandler = nil } - - if let error = delegate.error { - handler(.failure(error)) - - } else if let feed = folder.existingFeed(withFeedID: subscribeRequest.resource.id) { - handler(.success(feed)) - - } else { - handler(.failure(AccountError.createErrorNotFound)) - } - } - - callback.addDependency(createFeed) - - OperationQueue.main.addOperations([createFeed, callback], waitUntilFinished: false) - } -}