Refactor create feed functionality to increase code reuse and encapsulation

Frameworks/Account/Account.swift

@@ -377,8 +377,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
 		delegate.removeFeed(for: self, from: container, with: feed, completion: completion)
 	}
 	
-	public func createFeed(url: String, completion: @escaping (Result<Feed, Error>) -> Void) {
+	public func createFeed(url: String, name: String?, container: Container, completion: @escaping (Result<Feed, Error>) -> Void) {
-		delegate.createFeed(for: self, url: url, completion: completion)
+		delegate.createFeed(for: self, url: url, name: name, container: container, completion: completion)
 	}
 	
 	func createFeed(with name: String?, url: String, feedID: String, homePageURL: String?) -> Feed {
@@ -401,8 +401,8 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
 		delegate.renameFeed(for: self, with: feed, to: name, completion: completion)
 	}
 	
-	public func restoreFeed(_ feed: Feed, folder: Folder?, completion: @escaping (Result<Void, Error>) -> Void) {
+	public func restoreFeed(_ feed: Feed, container: Container, completion: @escaping (Result<Void, Error>) -> Void) {
-		delegate.restoreFeed(for: self, feed: feed, folder: folder, completion: completion)
+		delegate.restoreFeed(for: self, feed: feed, container: container, completion: completion)
 	}
 	
 	public func deleteFolder(_ folder: Folder, completion: @escaping (Result<Void, Error>) -> Void) {

Frameworks/Account/AccountDelegate.swift

@@ -31,14 +31,14 @@ protocol AccountDelegate {
 	func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result<Void, Error>) -> Void)
 	func deleteFolder(for account: Account, with folder: Folder, completion: @escaping (Result<Void, Error>) -> Void)
 
-	func createFeed(for account: Account, url: String, completion: @escaping (Result<Feed, Error>) -> Void)
+	func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result<Feed, Error>) -> Void)
 	func renameFeed(for account: Account, with feed: Feed, to name: String, completion: @escaping (Result<Void, Error>) -> Void)
 	func deleteFeed(for account: Account, with feed: Feed, completion: @escaping (Result<Void, Error>) -> Void)
 	
 	func addFeed(for account: Account, to container: Container, with: Feed, completion: @escaping (Result<Void, Error>) -> Void)
 	func removeFeed(for account: Account, from container: Container, with: Feed, completion: @escaping (Result<Void, Error>) -> Void)
 
-	func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result<Void, Error>) -> Void)
+	func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result<Void, Error>) -> Void)
 	func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result<Void, Error>) -> Void)
 
 	func markArticles(for account: Account, articles: Set<Article>, statusKey: ArticleStatus.Key, flag: Bool) -> Set<Article>?

Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift

@@ -284,16 +284,16 @@ final class FeedbinAccountDelegate: AccountDelegate {
 		
 	}
 	
-	func createFeed(for account: Account, url: String, completion: @escaping (Result<Feed, Error>) -> Void) {
+	func createFeed(for account: Account, url: String, name: String?, container: Container, completion: @escaping (Result<Feed, Error>) -> Void) {
 		
 		caller.createSubscription(url: url) { result in
 			switch result {
 			case .success(let subResult):
 				switch subResult {
 				case .created(let subscription):
-					self.createFeed(account: account, subscription: subscription, completion: completion)
+					self.createFeed(account: account, subscription: subscription, name: name, container: container, completion: completion)
 				case .multipleChoice(let choices):
-					self.decideBestFeedChoice(account: account, url: url, choices: choices, completion: completion)
+					self.decideBestFeedChoice(account: account, url: url, name: name, container: container, choices: choices, completion: completion)
 				case .alreadySubscribed:
 					DispatchQueue.main.async {
 						completion(.failure(AccountError.createErrorAlreadySubscribed))
@@ -424,19 +424,14 @@ final class FeedbinAccountDelegate: AccountDelegate {
 		
 	}
 	
-	func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result<Void, Error>) -> Void) {
+	func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result<Void, Error>) -> Void) {
 		
-		let editedName = feed.editedName
+		createFeed(for: account, url: feed.url, name: feed.editedName, container: container) { result in
-		
-		createFeed(for: account, url: feed.url) { result in
 			switch result {
-			case .success(let feed):
+			case .success:
-				self.processRestoredFeed(for: account, feed: feed, editedName: editedName, folder: folder, completion: completion)
+				completion(.success(()))
 			case .failure(let error):
-				DispatchQueue.main.async {
+				completion(.failure(error))
-					let wrappedError = AccountError.wrappedError(error: error, account: account)
-					completion(.failure(wrappedError))
-				}
 			}
 		}
 		
@@ -824,74 +819,6 @@ private extension FeedbinAccountDelegate {
 		
 	}
 	
-	func processRestoredFeed(for account: Account, feed: Feed, editedName: String?, folder: Folder?, completion: @escaping (Result<Void, Error>) -> Void) {
-		
-		if let folder = folder {
-			
-			addFeed(for: account, to: folder, with: feed) { result in
-				
-				switch result {
-				case .success:
-					
-					if editedName != nil {
-						DispatchQueue.main.async {
-							account.removeFeed(feed)
-							folder.addFeed(feed)
-						}
-						self.processRestoredFeedName(for: account, feed: feed, editedName: editedName!, completion: completion)
-					} else {
-						DispatchQueue.main.async {
-							account.removeFeed(feed)
-							folder.addFeed(feed)
-							completion(.success(()))
-						}
-					}
-					
-				case .failure(let error):
-					DispatchQueue.main.async {
-						completion(.failure(error))
-					}
-				}
-				
-			}
-			
-		} else {
-			
-			DispatchQueue.main.async {
-				account.addFeed(feed)
-			}
-			
-			if editedName != nil {
-				processRestoredFeedName(for: account, feed: feed, editedName: editedName!, completion: completion)
-			} else {
-				DispatchQueue.main.async {
-					completion(.success(()))
-				}
-			}
-			
-		}
-		
-	}
-	
-	func processRestoredFeedName(for account: Account, feed: Feed, editedName: String, completion: @escaping (Result<Void, Error>) -> Void) {
-		
-		renameFeed(for: account, with: feed, to: editedName) { result in
-			switch result {
-			case .success:
-				DispatchQueue.main.async {
-					feed.editedName = editedName
-					completion(.success(()))
-				}
-			case .failure(let error):
-				DispatchQueue.main.async {
-					completion(.failure(error))
-				}
-			}
-			
-		}
-		
-	}
-	
 	func clearFolderRelationship(for feed: Feed, withFolderName folderName: String) {
 		if var folderRelationship = feed.folderRelationship {
 			folderRelationship[folderName] = nil
@@ -908,7 +835,7 @@ private extension FeedbinAccountDelegate {
 		}
 	}
 
-	func decideBestFeedChoice(account: Account, url: String, choices: [FeedbinSubscriptionChoice], completion: @escaping (Result<Feed, Error>) -> Void) {
+	func decideBestFeedChoice(account: Account, url: String, name: String?, container: Container, choices: [FeedbinSubscriptionChoice], completion: @escaping (Result<Feed, Error>) -> Void) {
 		
 		let feedSpecifiers: [FeedSpecifier] = choices.map { choice in
 			let source = url == choice.url ? FeedSpecifier.Source.UserEntered : FeedSpecifier.Source.HTMLLink
@@ -918,7 +845,7 @@ private extension FeedbinAccountDelegate {
 
 		if let bestSpecifier = FeedSpecifier.bestFeed(in: Set(feedSpecifiers)) {
 			if let bestSubscription = choices.filter({ bestSpecifier.urlString == $0.url }).first {
-				createFeed(for: account, url: bestSubscription.url, completion: completion)
+				createFeed(for: account, url: bestSubscription.url, name: name, container: container, completion: completion)
 			} else {
 				DispatchQueue.main.async {
 					completion(.failure(FeedbinAccountDelegateError.invalidParameter))
@@ -932,44 +859,67 @@ private extension FeedbinAccountDelegate {
 		
 	}
 	
-	func createFeed( account: Account, subscription sub: FeedbinSubscription, completion: @escaping (Result<Feed, Error>) -> Void) {
+	func createFeed( account: Account, subscription sub: FeedbinSubscription, name: String?, container: Container, completion: @escaping (Result<Feed, Error>) -> Void) {
 		
+
 		DispatchQueue.main.async {
 			
 			let feed = account.createFeed(with: sub.name, url: sub.url, feedID: String(sub.feedID), homePageURL: sub.homePageURL)
 			feed.subscriptionID = String(sub.subscriptionID)
 		
-			// Download the initial articles
+			container.addFeed(feed) { result in
-			self.caller.retrieveEntries(feedID: feed.feedID) { result in
-				
 				switch result {
-				case .success(let (entries, page)):
+				case .success:
-					
+					if let name = name {
-					self.processEntries(account: account, entries: entries) {
+						account.renameFeed(feed, to: name) { result in
-						self.refreshArticles(account, page: page) {
+							switch result {
-							self.refreshArticleStatus(for: account) {
+							case .success:
-								self.refreshMissingArticles(account) {
+								self.initialFeedDownload(account: account, feed: feed, completion: completion)
-									DispatchQueue.main.async {
+							case .failure(let error):
-										completion(.success(feed))
+								completion(.failure(error))
-									}
-								}
 							}
 						}
+					} else {
+						self.initialFeedDownload(account: account, feed: feed, completion: completion)
 					}
-					
 				case .failure(let error):
-					os_log(.error, log: self.log, "Initial articles download failed: %@.", error.localizedDescription)
+					completion(.failure(error))
-					DispatchQueue.main.async {
-						completion(.success(feed))
-					}
 				}
-				
 			}
-
+			
 		}
 		
 	}
 
+	func initialFeedDownload( account: Account, feed: Feed, completion: @escaping (Result<Feed, Error>) -> Void) {
+		
+		// Download the initial articles
+		self.caller.retrieveEntries(feedID: feed.feedID) { result in
+			
+			switch result {
+			case .success(let (entries, page)):
+				
+				self.processEntries(account: account, entries: entries) {
+					self.refreshArticles(account, page: page) {
+						self.refreshArticleStatus(for: account) {
+							self.refreshMissingArticles(account) {
+								DispatchQueue.main.async {
+									completion(.success(feed))
+								}
+							}
+						}
+					}
+				}
+				
+			case .failure(let error):
+				completion(.failure(error))
+			}
+			
+		}
+
+		
+	}
+	
 	func refreshArticles(_ account: Account, completion: @escaping (() -> Void)) {
 
 		os_log(.debug, log: log, "Refreshing articles...")

Frameworks/Account/LocalAccount/LocalAccountDelegate.swift

@@ -27,6 +27,8 @@ final class LocalAccountDelegate: AccountDelegate {
 
 	private weak var account: Account?
 	private var feedFinder: FeedFinder?
+	private var createFeedName: String?
+	private var createFeedContainer: Container?
 	private var createFeedCompletion: ((Result<Feed, Error>) -> Void)?
 	
 	private let refresher = LocalAccountRefresher()
@@ -99,7 +101,7 @@ final class LocalAccountDelegate: AccountDelegate {
 		completion(.success(()))
 	}
 	
-	func createFeed(for account: Account, url urlString: String, completion: @escaping (Result<Feed, Error>) -> Void) {
+	func createFeed(for account: Account, url urlString: String, name: String?, container: Container, completion: @escaping (Result<Feed, Error>) -> Void) {
 		
 		guard let url = URL(string: urlString) else {
 			completion(.failure(LocalAccountDelegateError.invalidParameter))
@@ -107,8 +109,9 @@ final class LocalAccountDelegate: AccountDelegate {
 		}
 	
 		self.account = account
+		createFeedName = name
+		createFeedContainer =  container
 		createFeedCompletion = completion
-		
 		feedFinder = FeedFinder(url: url, delegate: self)
 		
 	}
@@ -143,10 +146,8 @@ final class LocalAccountDelegate: AccountDelegate {
 	func addFeed(for account: Account, to container: Container, with feed: Feed, completion: @escaping (Result<Void, Error>) -> Void) {
 		if let folder = container as? Folder {
 			folder.addFeed(feed)
-			feed.account = folder.account
 		} else if let account = container as? Account {
 			account.addFeed(feed)
-			feed.account = account
 		}
 		completion(.success(()))
 	}
@@ -161,13 +162,8 @@ final class LocalAccountDelegate: AccountDelegate {
 		completion(.success(()))
 	}
 	
-	func restoreFeed(for account: Account, feed: Feed, folder: Folder?, completion: @escaping (Result<Void, Error>) -> Void) {
+	func restoreFeed(for account: Account, feed: Feed, container: Container, completion: @escaping (Result<Void, Error>) -> Void) {
-		if let folder = folder {
+		container.addFeed(feed, completion: completion)
-			folder.addFeed(feed)
-		} else {
-			account.addFeed(feed)
-		}
-		completion(.success(()))
 	}
 	
 	func restoreFolder(for account: Account, folder: Folder, completion: @escaping (Result<Void, Error>) -> Void) {
@@ -216,11 +212,24 @@ extension LocalAccountDelegate: FeedFinderDelegate {
 		}
 		
 		let feed = account.createFeed(with: nil, url: url.absoluteString, feedID: url.absoluteString, homePageURL: nil)
-		InitialFeedDownloader.download(url) { [weak self] parsedFeed in
+		
+		InitialFeedDownloader.download(url) { parsedFeed in
+			
 			if let parsedFeed = parsedFeed {
 				account.update(feed, with: parsedFeed, {})
 			}
-			self?.createFeedCompletion!(.success(feed))
+			
+			feed.editedName = self.createFeedName
+			
+			self.createFeedContainer?.addFeed(feed) { result in
+				switch result {
+				case .success:
+					self.createFeedCompletion?(.success(feed))
+				case .failure(let error):
+					self.createFeedCompletion?(.failure(error))
+				}
+			}
+			
 		}
 
 	}

Mac/MainWindow/AddFeed/AddFeedController.swift

@@ -52,7 +52,6 @@ class AddFeedController: AddFeedWindowControllerDelegate {
 			return
 		}
 		let account = accountAndFolderSpecifier.account
-		let folder = accountAndFolderSpecifier.folder
 
 		if account.hasFeed(withURL: url.absoluteString) {
 			showAlreadySubscribedError(url.absoluteString)
@@ -61,20 +60,20 @@ class AddFeedController: AddFeedWindowControllerDelegate {
 
 		BatchUpdate.shared.start()
 		
-		account.createFeed(url: url.absoluteString) { [weak self] result in
+		account.createFeed(url: url.absoluteString, name: title, container: container) { result in
-			
-			self?.endShowingProgress()
 			
+			self.endShowingProgress()
+			BatchUpdate.shared.end()
+
 			switch result {
 			case .success(let feed):
-				self?.processFeed(feed, account: account, folder: folder, url: url, title: title)
+				NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
 			case .failure(let error):
-				BatchUpdate.shared.end()
 				switch error {
 				case AccountError.createErrorAlreadySubscribed:
-					self?.showAlreadySubscribedError(url.absoluteString)
+					self.showAlreadySubscribedError(url.absoluteString)
 				case AccountError.createErrorNotFound:
-					self?.showNoFeedsErrorMessage()
+					self.showNoFeedsErrorMessage()
 				default:
 					NSApplication.shared.presentError(error)
 				}
@@ -125,45 +124,6 @@ private extension AddFeedController {
 		}
 	}
 
-	func processFeed(_ feed: Feed, account: Account, folder: Folder?, url: URL, title: String?) {
-		
-		if let title = title {
-			account.renameFeed(feed, to: title) { result in
-				switch result {
-				case .success:
-					break
-				case .failure(let error):
-					NSApplication.shared.presentError(error)
-				}
-			}
-		}
-		
-		if let folder = folder {
-			folder.addFeed(feed) { result in
-				switch result {
-				case .success:
-					BatchUpdate.shared.end()
-					NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
-				case .failure(let error):
-					BatchUpdate.shared.end()
-					NSApplication.shared.presentError(error)
-				}
-			}
-		} else {
-			account.addFeed(feed) { result in
-				switch result {
-				case .success:
-					BatchUpdate.shared.end()
-					NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
-				case .failure(let error):
-					BatchUpdate.shared.end()
-					NSApplication.shared.presentError(error)
-				}
-			}
-		}
-		
-	}
-	
 	// MARK: Errors
 
 	func showAlreadySubscribedError(_ urlString: String) {

Mac/Scriptability/Feed+Scriptability.swift

@@ -91,7 +91,9 @@ class ScriptableFeed: NSObject, UniqueIdScriptingObject, ScriptingObjectContaine
         if let existingFeed = account.existingFeed(withURL:url) {
             return self.scriptableFeed(existingFeed, account:account, folder:folder)
         }
-    
+		
+		let container: Container = folder != nil ? folder! : account
+		
         // at this point, we need to download the feed and parse it.
         // RS Parser does the callback for the download on the main thread (which it probably shouldn't?)
         // because we can't wait here (on the main thread, maybe) for the callback, we have to return from this function
@@ -100,27 +102,12 @@ class ScriptableFeed: NSObject, UniqueIdScriptingObject, ScriptingObjectContaine
         // suspendExecution().  When we get the callback, we can supply the event result and call resumeExecution()
         command.suspendExecution()
         
-		account.createFeed(url: url) { result in
+		account.createFeed(url: url, name: titleFromArgs, container: container) { result in
 			switch result {
 			case .success(let feed):
-
+				NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
-				if let editedName = titleFromArgs {
+				let scriptableFeed = self.scriptableFeed(feed, account:account, folder:folder)
-					account.renameFeed(feed, to: editedName) { result in
+				command.resumeExecution(withResult:scriptableFeed.objectSpecifier)
-					}
-				}
-				
-				// add the feed, putting it in a folder if needed
-				account.addFeed(feed) { result in
-					switch result {
-					case .success:
-						NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
-						let scriptableFeed = self.scriptableFeed(feed, account:account, folder:folder)
-						command.resumeExecution(withResult:scriptableFeed.objectSpecifier)
-					case .failure:
-						command.resumeExecution(withResult:nil)
-					}
-				}
-					
 			case .failure:
 				command.resumeExecution(withResult:nil)
 			}

Shared/Commands/DeleteCommand.swift

@@ -161,12 +161,12 @@ private struct SidebarItemSpecifier {
 
 	private func restoreFeed() {
 
-		guard let account = account, let feed = feed else {
+		guard let account = account, let feed = feed, let container = path.resolveContainer() else {
 			return
 		}
 		
 		BatchUpdate.shared.start()
-		account.restoreFeed(feed, folder: resolvedFolder()) { result in
+		account.restoreFeed(feed, container: container) { result in
 			BatchUpdate.shared.end()
 			self.checkResult(result)
 		}
@@ -187,10 +187,6 @@ private struct SidebarItemSpecifier {
 		
 	}
 
-	private func resolvedFolder() -> Folder? {
-		return path.resolveContainer() as? Folder
-	}
-	
 	private func checkResult(_ result: Result<Void, Error>) {
 		
 		switch result {

iOS/Add/AddFeedViewController.swift

@@ -80,13 +80,10 @@ class AddFeedViewController: UITableViewController, AddContainerViewControllerCh
 		let container = pickerData.containers[folderPickerView.selectedRow(inComponent: 0)]
 		
 		var account: Account?
-		var folder: Folder?
 		if let containerAccount = container as? Account {
 			account = containerAccount
-		}
+		} else if let containerFolder = container as? Folder, let containerAccount = containerFolder.account {
-		if let containerFolder = container as? Folder, let containerAccount = containerFolder.account {
 			account = containerAccount
-			folder = containerFolder
 		}
 		
 		if account!.hasFeed(withURL: url.absoluteString) {
@@ -94,26 +91,28 @@ class AddFeedViewController: UITableViewController, AddContainerViewControllerCh
  			return
 		}
 		
-		let title = nameTextField.text
-		
 		delegate?.processingDidBegin()
+		BatchUpdate.shared.start()
+		
+		account!.createFeed(url: url.absoluteString, name: nameTextField.text, container: container) { result in
 
-		account!.createFeed(url: url.absoluteString) { [weak self] result in
+			BatchUpdate.shared.end()
 			
 			switch result {
 			case .success(let feed):
-				self?.processFeed(feed, account: account!, folder: folder, url: url, title: title)
+				self.delegate?.processingDidEnd()
+				NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
 			case .failure(let error):
 				switch error {
 				case AccountError.createErrorAlreadySubscribed:
-					self?.showAlreadySubscribedError()
+					self.showAlreadySubscribedError()
-					self?.delegate?.processingDidCancel()
+					self.delegate?.processingDidCancel()
 				case AccountError.createErrorNotFound:
-					self?.showNoFeedsErrorMessage()
+					self.showNoFeedsErrorMessage()
-					self?.delegate?.processingDidCancel()
+					self.delegate?.processingDidCancel()
 				default:
-					self?.presentError(error)
+					self.presentError(error)
-					self?.delegate?.processingDidCancel()
+					self.delegate?.processingDidCancel()
 				}
 			}
 
@@ -178,45 +177,6 @@ private extension AddFeedViewController {
 		presentError(title: title, message: message as String)
 	}
 	
-	func processFeed(_ feed: Feed, account: Account, folder: Folder?, url: URL, title: String?) {
-		
-		if let title = title {
-			account.renameFeed(feed, to: title) { [weak self] result in
-				switch result {
-				case .success:
-					break
-				case .failure(let error):
-					self?.presentError(error)
-				}
-			}
-		}
-		
-		if let folder = folder {
-			folder.addFeed(feed) { [weak self] result in
-				switch result {
-				case .success:
-					self?.delegate?.processingDidEnd()
-					NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
-				case .failure(let error):
-					self?.delegate?.processingDidEnd()
-					self?.presentError(error)
-				}
-			}
-		} else {
-			account.addFeed(feed) { [weak self] result in
-				switch result {
-				case .success:
-					self?.delegate?.processingDidEnd()
-					NotificationCenter.default.post(name: .UserDidAddFeed, object: self, userInfo: [UserInfoKey.feed: feed])
-				case .failure(let error):
-					self?.delegate?.processingDidEnd()
-					self?.presentError(error)
-				}
-			}
-		}
-		
-	}
-	
 }
 
 extension AddFeedViewController: UITextFieldDelegate {