diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift index 5d8a1a3e5b..0e08866fb4 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift @@ -14,7 +14,7 @@ public class KeyboardKeymanPackage : TypedKeymanPackage { override internal init(metadata: KMPMetadata, folder: URL) { super.init(metadata: metadata, folder: folder) self.keyboards = [] - + if let packagedKeyboards = metadata.keyboards { for keyboard in packagedKeyboards { keyboard.packageId = self.id @@ -22,7 +22,7 @@ public class KeyboardKeymanPackage : TypedKeymanPackage { if(keyboard.isValid && FileManager.default.fileExists(atPath: self.sourceFolder.appendingPathComponent("\(keyboard.keyboardId).js").path)) { keyboards.append(keyboard) } else { - log.debug("\(keyboard.name) not valid / corresponding file not found") + SentryManager.breadcrumbAndLog("\(keyboard.name) not valid / corresponding file not found") } } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift index 98bc300fb9..a88337799d 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift @@ -16,6 +16,8 @@ public enum KMPError : String, Error { case invalidPackage = "kmp-error-invalid" case fileSystem = "kmp-error-file-system" case copyFiles = "kmp-error-file-copying" + case unsupportedPackage = "kmp-error-unsupported" + case doesNotExist = "kmp-error-missing" // TODO: Consider changing the parent type so that these may take arguments that may be used // to provide better clarity. case wrongPackageType = "kmp-error-wrong-type" @@ -380,35 +382,37 @@ public class KeymanPackage { * Typecast the return value to either KeyboardKeymanPackage or LexicalModelKeymanPackage for richly-typed * information about the package's contents. */ - static public func parse(_ folder: URL) -> KeymanPackage? { - do { - var path = folder - path.appendPathComponent(kmpFile) - if FileManager.default.fileExists(atPath: path.path) { - let data = try Data(contentsOf: path, options: .mappedIfSafe) - let decoder = JSONDecoder() + static public func parse(_ folder: URL) throws -> KeymanPackage? { + var path = folder + path.appendPathComponent(kmpFile) + if FileManager.default.fileExists(atPath: path.path) { + let data = try Data(contentsOf: path, options: .mappedIfSafe) + let decoder = JSONDecoder() - var metadata: KMPMetadata + var metadata: KMPMetadata - do { - metadata = try decoder.decode(KMPMetadata.self, from: data) - } catch { - throw KMPError.noMetadata - } - - switch metadata.packageType { - case .Keyboard: - return KeyboardKeymanPackage(metadata: metadata, folder: folder) - case .LexicalModel: - return LexicalModelKeymanPackage(metadata: metadata, folder: folder) - default: - throw KMPError.invalidPackage - } + do { + metadata = try decoder.decode(KMPMetadata.self, from: data) + } catch { + throw KMPError.noMetadata + } + + var package: KeymanPackage + + switch metadata.packageType { + case .Keyboard: + package = KeyboardKeymanPackage(metadata: metadata, folder: folder) + case .LexicalModel: + package = LexicalModelKeymanPackage(metadata: metadata, folder: folder) + default: + throw KMPError.invalidPackage + } + + if package.resources.count == 0 { + throw KMPError.unsupportedPackage + } else { + return package } - } catch { - // It's not an app or engine error when the package itself is invalid. - // Definitely worth noting, though. - SentryManager.breadcrumbAndLog("error parsing keyman package: \(error)", sentryLevel: .error) } return nil @@ -417,13 +421,19 @@ public class KeymanPackage { @available(*, deprecated, message: "Use of the completion block is unnecessary; this method now returns synchronously.") static public func extract(fileUrl: URL, destination: URL, complete: @escaping (KeymanPackage?) -> Void) throws { try unzipFile(fileUrl: fileUrl, destination: destination) { - complete(KeymanPackage.parse(destination)) + do { + let package = try KeymanPackage.parse(destination) + complete(package) + } catch { + SentryManager.captureAndLog(error, sentryLevel: .info) + complete(nil) + } } } static public func extract(fileUrl: URL, destination: URL) throws -> KeymanPackage? { try unzipFile(fileUrl: fileUrl, destination: destination) - return KeymanPackage.parse(destination) + return try KeymanPackage.parse(destination) } static public func unzipFile(fileUrl: URL, destination: URL, complete: @escaping () -> Void = {}) throws { diff --git a/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift index 6161e1e6ac..fe9cd930e6 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift @@ -24,7 +24,7 @@ public class LexicalModelKeymanPackage : TypedKeymanPackage Void { - guard let kmp = KeymanPackage.parse(folder) as? KeyboardKeymanPackage else { + guard let kmp = try? KeymanPackage.parse(folder) as? KeyboardKeymanPackage else { throw KMPError.wrongPackageType } @@ -640,7 +640,7 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { // MARK: - Adhoc lexical models static public func parseLMKMP(_ folder: URL, isCustom: Bool) throws -> Void { - guard let kmp = KeymanPackage.parse(folder) as? LexicalModelKeymanPackage else { + guard let kmp = try? KeymanPackage.parse(folder) as? LexicalModelKeymanPackage else { throw KMPError.wrongPackageType } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index df16381c4a..11740ce42b 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -54,7 +54,7 @@ public class ResourceFileManager { var backingPackages: [KeymanPackage] = [] for resource in userResources { - if let package = KeymanPackage.parse(Storage.active.resourceDir(for: resource)!) { + if let package = try? KeymanPackage.parse(Storage.active.resourceDir(for: resource)!) { // On successful parse, just ensure that we haven't already listed the package. if !backingPackages.contains(where: { $0.id == package.id }) { backingPackages.append(package) @@ -86,14 +86,14 @@ public class ResourceFileManager { public func getInstalledPackage(for resource: Resource) -> Resource.Package? { if let packageDir = Storage.active.resourceDir(for: resource) { - return KeymanPackage.parse(packageDir) as? Resource.Package + return try? KeymanPackage.parse(packageDir) as? Resource.Package } else { return nil } } public func getInstalledPackage(withKey key: KeymanPackage.Key) -> KeymanPackage? { - return KeymanPackage.parse(Storage.active.packageDir(forKey: key)) + return try? KeymanPackage.parse(Storage.active.packageDir(forKey: key)) } internal func packageDownloadTempPath(forKey key: KeymanPackage.Key) -> URL { @@ -183,10 +183,14 @@ public class ResourceFileManager { var extractionFolder = cacheDirectory extractionFolder.appendPathComponent("temp/\(archiveUrl.lastPathComponent)") - if let kmp = try KeymanPackage.extract(fileUrl: archiveUrl, destination: extractionFolder) { - return kmp - } else { - throw KMPError.invalidPackage + do { + if let package = try KeymanPackage.extract(fileUrl: archiveUrl, destination: extractionFolder) { + return package + } else { + throw KMPError.doesNotExist + } + } catch { + throw error } } diff --git a/ios/engine/KMEI/KeymanEngine/en.lproj/Localizable.strings b/ios/engine/KMEI/KeymanEngine/en.lproj/Localizable.strings index fec25123c9..dad7514cbe 100644 --- a/ios/engine/KMEI/KeymanEngine/en.lproj/Localizable.strings +++ b/ios/engine/KMEI/KeymanEngine/en.lproj/Localizable.strings @@ -118,12 +118,18 @@ /* Error opening a Keyman package - package is not valid */ "kmp-error-invalid" = "The package's file is corrupted."; +/* Error opening a Keyman package - it does not exist / the specified location is wrong */ +"kmp-error-missing" = "The specified package does not exist."; + /* Error installing a Keyman package - expected resource (keyboard or dictionary) is missing */ "kmp-error-missing-resource" = "This package does not contain the requested keyboard or dictionary."; /* Error opening a Keyman package - cannot parse contents */ "kmp-error-no-metadata" = "This package was not properly built - contents unknown."; +/* Error opening a Keyman package - package's contents are for desktop platforms only */ +"kmp-error-unsupported" = "This package does not include support for your device."; + /* Error opening a Keyman package - package contains unexpected resource (keyboard or dictionary) type */ "kmp-error-wrong-type" = "This package does not contain the expected resource type."; diff --git a/ios/keyman/Keyman/Keyman/PackageBrowserViewController.swift b/ios/keyman/Keyman/Keyman/PackageBrowserViewController.swift index 7388147997..c7d3904387 100644 --- a/ios/keyman/Keyman/Keyman/PackageBrowserViewController.swift +++ b/ios/keyman/Keyman/Keyman/PackageBrowserViewController.swift @@ -65,7 +65,15 @@ class PackageBrowserViewController: UIDocumentPickerViewController, UIDocumentPi return } - if let package = rfm.prepareKMPInstall(from: destinationUrl, alertHost: self) { + // The package browser view has usually self-dismissed at this point and cannot + // present error message alerts. We need to find something in the view + // hierarchy that can present the error. + var alertVC: UIViewController = self + if self.view.superview == nil && self.navVC != nil { + alertVC = self.navVC! + } + + if let package = rfm.prepareKMPInstall(from: destinationUrl, alertHost: alertVC) { // These type checks are necessary due to generic constraints. if let kbdPackage = package as? KeyboardKeymanPackage { doPrompt(for: kbdPackage, withAssociators: [.lexicalModels])