Various ThemeDownloader Updates

- `try` added where applicable to ArticleTheme inits
- `ArticleThemePlist` has fixed spelling of theme identifier and conforms to Equatable
- `ArticleTheme` now uses `ArticleThemePlist`
-  `ArticleThemeDownloader` is now a class
- `ArticleThemeDownloader` will now download themes to Application Support/NetNewsWire/Downloads on macOS and iOS.
- `ArticleThemeDownloader` will remove downloaded themes from the Download folder when the application is closed.
- macOS app delegate now observes for theme download fails
- Error display code moved from SceneDelegate to SceneCoordinator so that it can use existing presentError on rootVC.

Mac/AppDelegate.swift

@@ -126,6 +126,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations,
 		NotificationCenter.default.addObserver(self, selector: #selector(unreadCountDidChange(_:)), name: .UnreadCountDidChange, object: nil)
 		NotificationCenter.default.addObserver(self, selector: #selector(inspectableObjectsDidChange(_:)), name: .InspectableObjectsDidChange, object: nil)
 		NotificationCenter.default.addObserver(self, selector: #selector(importDownloadedTheme(_:)), name: .didEndDownloadingTheme, object: nil)
+		NotificationCenter.default.addObserver(self, selector: #selector(themeImportError(_:)), name: .didEndDownloadingThemeWithError, object: nil)
 		NSWorkspace.shared.notificationCenter.addObserver(self, selector: #selector(didWakeNotification(_:)), name: NSWorkspace.didWakeNotification, object: nil)
 
 		appDelegate = self
@@ -322,7 +323,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations,
 	
 	func application(_ sender: NSApplication, openFile filename: String) -> Bool {
 		guard filename.hasSuffix(ArticleTheme.nnwThemeSuffix) else { return false }
-		importTheme(filename: filename)
+		try? importTheme(filename: filename)
 		return true
 	}
 	
@@ -330,6 +331,8 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations,
 		shuttingDown = true
 		saveState()
 		
+		ArticleThemeDownloader.shared.cleanUp()
+		
 		AccountManager.shared.sendArticleStatusAll() {
 			self.isShutDownSyncDone = true
 		}
@@ -383,7 +386,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations,
 			return
 		}
 		DispatchQueue.main.async {
-			self.importTheme(filename: url.path)
+			try? self.importTheme(filename: url.path)
 		}
 	}
 
@@ -808,10 +811,10 @@ internal extension AppDelegate {
 		groupArticlesByFeedMenuItem.state = groupByFeedEnabled ? .on : .off
 	}
 	
-	func importTheme(filename: String) {
+	func importTheme(filename: String) throws {
 		guard let window = mainWindowController?.window else { return }
 		
-		let theme = ArticleTheme(path: filename)
+		let theme = try ArticleTheme(path: filename)
 		
 		let alert = NSAlert()
 		alert.alertStyle = .informational
@@ -906,6 +909,22 @@ internal extension AppDelegate {
 		alert.beginSheetModal(for: window)
 	}
 	
+	@objc func themeImportError(_ note: Notification) {
+		guard let userInfo = note.userInfo,
+			  let error = userInfo["error"] as? Error,
+			  let window = mainWindowController?.window else {
+				  return
+			  }
+		DispatchQueue.main.async {
+			let alert = NSAlert()
+			alert.alertStyle = .warning
+			alert.messageText = NSLocalizedString("Theme Download Error", comment: "Theme download error")
+			alert.informativeText = NSLocalizedString("This theme cannot be downloaded due to the following error: \(error.localizedDescription)", comment: "Theme download error information")
+			alert.addButton(withTitle: NSLocalizedString("OK", comment: "OK"))
+			alert.beginSheetModal(for: window)
+		}
+	}
+	
 }
 
 /*

Mac/Scriptability/AppDelegate+Scriptability.swift

@@ -61,9 +61,9 @@ extension AppDelegate : AppDelegateAppleEvents {
 					}
 					
 					do {
-						try ArticleThemeDownloader.handleFile(at: location)
+						try ArticleThemeDownloader.shared.handleFile(at: location)
 					} catch {
-						print(error)
+						NotificationCenter.default.post(name: .didEndDownloadingThemeWithError, object: nil, userInfo: ["error": error])
 					}
 				}
 				task.resume()

Shared/ArticleStyles/ArticleTheme.swift

@@ -26,22 +26,22 @@ struct ArticleTheme: Equatable {
 	}
 	
 	var creatorHomePage: String {
-		return info?["CreatorHomePage"] as? String ?? Self.unknownValue
+		return info?.creatorHomePage ?? Self.unknownValue
 	}
 	
 	var creatorName: String {
-		return info?["CreatorName"] as? String ?? Self.unknownValue
+		return info?.creatorName ?? Self.unknownValue
 	}
 	
 	var version: String {
-		return info?["Version"] as? String ?? "0.0"
+		return String(describing: info?.version ?? 0)
 	}
 	
-	private let info: NSDictionary?
+	private let info: ArticleThemePlist?
 
 	init() {
 		self.path = nil;
-		self.info = ["CreatorHomePage": "https://netnewswire.com/", "CreatorName": "Ranchero Software", "Version": "1.0"]
+		self.info = ArticleThemePlist(name: "Article Theme", themeIdentifier: "com.ranchero.netnewswire.theme.article", creatorHomePage: "https://netnewswire.com/", creatorName: "Ranchero Software", version: 1)
 
 		let corePath = Bundle.main.path(forResource: "core", ofType: "css")!
 		let stylesheetPath = Bundle.main.path(forResource: "stylesheet", ofType: "css")!
@@ -51,12 +51,13 @@ struct ArticleTheme: Equatable {
 		template = Self.stringAtPath(templatePath)!
 	}
 
-	init(path: String) {
+	init(path: String) throws {
 		self.path = path
 
 		let infoPath = (path as NSString).appendingPathComponent("Info.plist")
-		self.info = NSDictionary(contentsOfFile: infoPath)
-
+		let data = try Data(contentsOf: URL(fileURLWithPath: infoPath))
+		self.info = try PropertyListDecoder().decode(ArticleThemePlist.self, from: data)
+		
 		let corePath = Bundle.main.path(forResource: "core", ofType: "css")!
 		let stylesheetPath = (path as NSString).appendingPathComponent("stylesheet.css")
 		if let stylesheetCSS = Self.stringAtPath(stylesheetPath) {

Shared/ArticleStyles/ArticleThemeDownloader.swift

@@ -9,62 +9,85 @@
 import Foundation
 import Zip
 
-public struct ArticleThemeDownloader {
+public class ArticleThemeDownloader {
 	
-	static func handleFile(at location: URL) throws {
-		#if os(iOS)
+	public enum ArticleThemeDownloaderError: LocalizedError {
+		case noThemeFile
+		
+		public var errorDescription: String? {
+			switch self {
+			case .noThemeFile:
+				return "There is no NetNewsWire theme available."
+			}
+		}
+	}
+	
+	
+	public static let shared = ArticleThemeDownloader()
+	private init() {}
+	
+	public func handleFile(at location: URL) throws {
 		createDownloadDirectoryIfRequired()
-		#endif
 		let movedFileLocation = try moveTheme(from: location)
 		let unzippedFileLocation = try unzipFile(at: movedFileLocation)
-		let renamedFile = try renameFileToThemeName(at: unzippedFileLocation)
-		NotificationCenter.default.post(name: .didEndDownloadingTheme, object: nil, userInfo: ["url" : renamedFile])
+		NotificationCenter.default.post(name: .didEndDownloadingTheme, object: nil, userInfo: ["url" : unzippedFileLocation])
 	}
 	
-	private static func createDownloadDirectoryIfRequired() {
-		let downloadDirectory = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask).first!
-		try? FileManager.default.createDirectory(at: downloadDirectory, withIntermediateDirectories: true, attributes: nil)
+	
+	/// Creates `Application Support/NetNewsWire/Downloads` if needed.
+	private func createDownloadDirectoryIfRequired() {
+		try? FileManager.default.createDirectory(at: downloadDirectory(), withIntermediateDirectories: true, attributes: nil)
 	}
 	
-	private static func moveTheme(from location: URL) throws -> URL {
-		#if os(iOS)
-		var downloadDirectory = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask).first!
-		#else
-		var downloadDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask).first!
-		#endif
-		let tmpFileName = UUID().uuidString + ".zip"
-		downloadDirectory.appendPathComponent("\(tmpFileName)")
-		try FileManager.default.moveItem(at: location, to: downloadDirectory)
-		return downloadDirectory
+	/// Moves the downloaded `.tmp` file to the `downloadDirectory` and renames it a `.zip`
+	/// - Parameter location: The temporary file location.
+	/// - Returns: Destination `URL`.
+	private func moveTheme(from location: URL) throws -> URL {
+		var tmpFileName = location.lastPathComponent
+		tmpFileName = tmpFileName.replacingOccurrences(of: ".tmp", with: ".zip")
+		let fileUrl = downloadDirectory().appendingPathComponent("\(tmpFileName)")
+		try FileManager.default.moveItem(at: location, to: fileUrl)
+		return fileUrl
 	}
 	
-	private static func unzipFile(at location: URL) throws -> URL {
-		var unzippedDir = location.deletingLastPathComponent()
-		unzippedDir.appendPathComponent("newtheme.nnwtheme")
+	/// Unzips the zip file
+	/// - Parameter location: Location of the zip archive.
+	/// - Returns: Enclosed `.nnwtheme` file.
+	private func unzipFile(at location: URL) throws -> URL {
 		do {
-			try Zip.unzipFile(location, destination: unzippedDir, overwrite: true, password: nil, progress: nil, fileOutputHandler: nil)
-			try FileManager.default.removeItem(at: location)
-			return unzippedDir
+			let unzipDirectory = URL(fileURLWithPath: location.path.replacingOccurrences(of: ".zip", with: ""))
+			try Zip.unzipFile(location, destination: unzipDirectory, overwrite: true, password: nil, progress: nil, fileOutputHandler: nil) // Unzips to folder in Application Support/NetNewsWire/Downloads
+			try FileManager.default.removeItem(at: location) // Delete zip in Cache
+			let themeFilePath = FileManager.default.filenames(inFolder: unzipDirectory.path)?.first(where: { $0.contains(".nnwtheme") })
+			if themeFilePath == nil {
+				throw ArticleThemeDownloaderError.noThemeFile
+			}
+			return URL(fileURLWithPath: unzipDirectory.appendingPathComponent(themeFilePath!).path)
 		} catch {
 			try? FileManager.default.removeItem(at: location)
 			throw error
 		}
 	}
 	
-	private static func renameFileToThemeName(at location: URL) throws -> URL {
-		let decoder = PropertyListDecoder()
-		let plistURL = URL(fileURLWithPath: location.appendingPathComponent("Info.plist").path)
-		let data = try Data(contentsOf: plistURL)
-		let plist = try decoder.decode(ArticleThemePlist.self, from: data)
-		var renamedUnzippedDir = location.deletingLastPathComponent()
-		renamedUnzippedDir.appendPathComponent(plist.name + ".nnwtheme")
-		if FileManager.default.fileExists(atPath: renamedUnzippedDir.path) {
-			try FileManager.default.removeItem(at: renamedUnzippedDir)
-		}
-		try FileManager.default.moveItem(at: location, to: renamedUnzippedDir)
-		return renamedUnzippedDir
+	/// The download directory used by the theme downloader: `Application Suppport/NetNewsWire/Downloads`
+	/// - Returns: `URL`
+	private func downloadDirectory() -> URL {
+		FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first!.appendingPathComponent("NetNewsWire/Downloads", isDirectory: true)
 	}
 	
-	
-	
+	/// Removes downloaded themes, where themes == folders, from `Application Suppport/NetNewsWire/Downloads`.
+	public func cleanUp() {
+		guard let filenames = try? FileManager.default.contentsOfDirectory(atPath: downloadDirectory().path) else {
+			return
+		}
+		for path in filenames {
+			do {
+				if FileManager.default.isFolder(atPath: downloadDirectory().appendingPathComponent(path).path) {
+					try FileManager.default.removeItem(atPath: downloadDirectory().appendingPathComponent(path).path)
+				}
+			} catch {
+				print(error)
+			}
+		}
+	}
 }

Shared/ArticleStyles/ArticleThemePlist.swift

@@ -8,10 +8,9 @@
 
 import Foundation
 
-public struct ArticleThemePlist: Codable {
+public struct ArticleThemePlist: Codable, Equatable {
 	public var name: String
 	public var themeIdentifier: String
-	public var themeDescription: String?
 	public var creatorHomePage: String
 	public var creatorName: String
 	public var version: Int
@@ -19,7 +18,6 @@ public struct ArticleThemePlist: Codable {
 	enum CodingKeys: String, CodingKey {
 		case name = "Name"
 		case themeIdentifier = "ThemeIdentifier"
-		case themeDescription = "ThemeDescription"
 		case creatorHomePage = "CreatorHomePage"
 		case creatorName = "CreatorName"
 		case version = "Version"

Shared/ArticleStyles/ArticleThemesManager.swift

@@ -134,7 +134,7 @@ private extension ArticleThemesManager {
 			return nil
 		}
 
-		return ArticleTheme(path: path)
+		return try? ArticleTheme(path: path)
 	}
 
 	func defaultArticleTheme() -> ArticleTheme {

iOS/AppDelegate.swift

@@ -338,6 +338,7 @@ private extension AppDelegate {
 		
 		AccountManager.shared.suspendNetworkAll()
 		AccountManager.shared.suspendDatabaseAll()
+		ArticleThemeDownloader.shared.cleanUp()
 
 		CoalescingQueue.standard.performCallsImmediately()
 		for scene in UIApplication.shared.connectedScenes {

iOS/SceneCoordinator.swift

@@ -323,7 +323,8 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
 		NotificationCenter.default.addObserver(self, selector: #selector(userDefaultsDidChange(_:)), name: UserDefaults.didChangeNotification, object: nil)
 		NotificationCenter.default.addObserver(self, selector: #selector(accountDidDownloadArticles(_:)), name: .AccountDidDownloadArticles, object: nil)
 		NotificationCenter.default.addObserver(self, selector: #selector(willEnterForeground(_:)), name: UIApplication.willEnterForegroundNotification, object: nil)
-
+		NotificationCenter.default.addObserver(self, selector: #selector(importDownloadedTheme(_:)), name: .didEndDownloadingTheme, object: nil)
+		NotificationCenter.default.addObserver(self, selector: #selector(themeDownloadDidFail(_:)), name: .didEndDownloadingThemeWithError, object: nil)
 	}
 	
 	func start(for size: CGSize) -> UIViewController {
@@ -587,6 +588,27 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
 			queueFetchAndMergeArticles()
 		}
 	}
+	
+	@objc func importDownloadedTheme(_ note: Notification) {
+		guard let userInfo = note.userInfo,
+			let url = userInfo["url"] as? URL else {
+			return
+		}
+		
+		DispatchQueue.main.async {
+			self.importTheme(filename: url.path)
+		}
+	}
+	
+	@objc func themeDownloadDidFail(_ note: Notification) {
+		guard let userInfo = note.userInfo,
+			  let error = userInfo["error"] as? Error else {
+				  return
+			  }
+		DispatchQueue.main.async {
+			self.rootSplitViewController.presentError(error, dismiss: nil)
+		}
+	}
 
 	// MARK: API
 	
@@ -1295,7 +1317,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
 	}
 	
 	func importTheme(filename: String) {
-		ArticleThemeImporter.importTheme(controller: rootSplitViewController, filename: filename);
+		try? ArticleThemeImporter.importTheme(controller: rootSplitViewController, filename: filename);
 	}
 	
 }

iOS/SceneDelegate.swift

@@ -29,8 +29,6 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
 		
 		NotificationCenter.default.addObserver(self, selector: #selector(userDefaultsDidChange), name: UserDefaults.didChangeNotification, object: nil)
 		
-		NotificationCenter.default.addObserver(self, selector: #selector(importDownloadedTheme(_:)), name: .didEndDownloadingTheme, object: nil)
-		
 		if let _ = connectionOptions.urlContexts.first?.url  {
 			window?.makeKeyAndVisible()
 			self.scene(scene, openURLContexts: connectionOptions.urlContexts)
@@ -190,11 +188,9 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
 							  let location = location else { return }
 						
 						do {
-							try ArticleThemeDownloader.handleFile(at: location)
+							try ArticleThemeDownloader.shared.handleFile(at: location)
 						} catch {
-							DispatchQueue.main.async {
-								self.showAlert(error)
-							}
+							NotificationCenter.default.post(name: .didEndDownloadingThemeWithError, object: nil, userInfo: ["error": error])
 						}
 					}
 					task.resume()
@@ -208,16 +204,6 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
 			
 		}
 	}
-	
-	
-	
-	private func showAlert(_ error: Error) {
-		let alert = UIAlertController(title: NSLocalizedString("Error", comment: "Error"),
-									  message: error.localizedDescription,
-									  preferredStyle: .alert)
-		alert.addAction(UIAlertAction(title: NSLocalizedString("Dismiss", comment: "Dismiss"), style: .cancel, handler: nil))
-		self.window?.rootViewController?.present(alert, animated: true, completion: nil)
-	}
 }
 
 private extension SceneDelegate {
@@ -252,15 +238,6 @@ private extension SceneDelegate {
 		}
 	}
 	
-	@objc func importDownloadedTheme(_ note: Notification) {
-		guard let userInfo = note.userInfo,
-			let url = userInfo["url"] as? URL else {
-			return
-		}
-		
-		DispatchQueue.main.async {
-			self.coordinator.importTheme(filename: url.path)
-		}
-	}
+	
 	
 }

iOS/Settings/ArticleThemeImporter.swift

@@ -10,8 +10,8 @@ import UIKit
 
 struct ArticleThemeImporter {
 	
-	static func importTheme(controller: UIViewController, filename: String) {
-		let theme = ArticleTheme(path: filename)
+	static func importTheme(controller: UIViewController, filename: String) throws {
+		let theme = try ArticleTheme(path: filename)
 		
 		let localizedTitleText = NSLocalizedString("Install theme “%@” by %@?", comment: "Theme message text")
 		let title = NSString.localizedStringWithFormat(localizedTitleText as NSString, theme.name, theme.creatorName) as String
@@ -28,7 +28,7 @@ struct ArticleThemeImporter {
 			let visitSiteTitle = NSLocalizedString("Show Website", comment: "Show Website")
 			let visitSiteAction = UIAlertAction(title: visitSiteTitle, style: .default) { action in
 				UIApplication.shared.open(url)
-				Self.importTheme(controller: controller, filename: filename)
+				try? Self.importTheme(controller: controller, filename: filename)
 			}
 			alertController.addAction(visitSiteAction)
 		}

iOS/Settings/ArticleThemesTableViewController.swift

@@ -112,7 +112,7 @@ extension ArticleThemesTableViewController: UIDocumentPickerDelegate {
 	
 	func documentPicker(_ controller: UIDocumentPickerViewController, didPickDocumentsAt urls: [URL]) {
 		guard let url = urls.first else { return }
-		ArticleThemeImporter.importTheme(controller: self, filename: url.standardizedFileURL.path)
+		try? ArticleThemeImporter.importTheme(controller: self, filename: url.standardizedFileURL.path)
 	}
 	
 }