From df2a31f0759f88e9f1d12dab46d26c286386cde2 Mon Sep 17 00:00:00 2001 From: jahorton Date: Wed, 3 Jun 2020 15:58:26 +0700 Subject: [PATCH 1/5] feat(ios/engine): KeymanPackage.contains(resource) helper func --- .../KeymanEngine/Classes/KeymanPackage.swift | 21 +++++++++++++++++++ .../KeymanPackageTests.swift | 12 +++++++++++ .../TestUtils/Keyboards.swift | 4 ++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift index 4d8b8b6b88..11ece2f597 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift @@ -15,6 +15,7 @@ public enum KMPError : String, Error { case fileSystem = "Unable to create directory structure in file system." case copyFiles = "Unable to copy keyboard files to file system." case wrongPackageType = "Package provided does not match the type expected by this method" + case resourceNotInPackage = "Resource cannot be found in specified package" } public class KeymanPackage { @@ -118,4 +119,24 @@ public class KeymanPackage { } }) } + + public func contains(_ resource: LanguageResource) -> Bool { + let matchesFound: [[LanguageResource]] = self.installableResourceSets.compactMap { set in + let setMatches: [LanguageResource] = set.compactMap { other in + if resource.id == other.id && resource.languageID == other.languageID { + return other + } else { + return nil + } + } + + if setMatches.count > 0 { + return setMatches + } else { + return nil + } + } + + return matchesFound.count > 0 + } } diff --git a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift index 8a5046bcc4..2ef17d534e 100644 --- a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift @@ -74,4 +74,16 @@ class KeymanPackageTests: XCTestCase { XCTFail("KeymanPackage.extract failed with error \(error)") } } + + func testPackageContainsResource() { + ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, _ in + XCTAssertTrue(kmp!.contains(TestUtils.Keyboards.khmer_angkor)) + XCTAssertFalse(kmp!.contains(TestUtils.LexicalModels.mtnt)) + } + + ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.LexicalModels.mtntKMP) { kmp, _ in + XCTAssertTrue(kmp!.contains(TestUtils.LexicalModels.mtnt)) + XCTAssertFalse(kmp!.contains(TestUtils.Keyboards.khmer_angkor)) + } + } } diff --git a/ios/engine/KMEI/KeymanEngineTests/TestUtils/Keyboards.swift b/ios/engine/KMEI/KeymanEngineTests/TestUtils/Keyboards.swift index 05226fee0f..cba090233f 100644 --- a/ios/engine/KMEI/KeymanEngineTests/TestUtils/Keyboards.swift +++ b/ios/engine/KMEI/KeymanEngineTests/TestUtils/Keyboards.swift @@ -14,7 +14,7 @@ extension TestUtils { static let khmerAngkorKMP = TestUtils.keyboardsBundle.url(forResource: "khmer_angkor", withExtension: "kmp")! static let khmer_angkor = InstallableKeyboard(id: "khmer_angkor", name: "Khmer Angkor", - languageID: "kh", + languageID: "km", languageName: "Khmer (Central)", version: "1.0.6", isRTL: false, @@ -23,7 +23,7 @@ extension TestUtils { isCustom: false) static let khmer10 = InstallableKeyboard(id: "khmer10", name: "Khmer (NiDA)", - languageID: "kh", + languageID: "km", languageName: "Khmer (Central)", version: "1.3", isRTL: false, From 6535fac385e357df4d45f696ad5ef3ddc41e5acf Mon Sep 17 00:00:00 2001 From: jahorton Date: Wed, 3 Jun 2020 16:28:09 +0700 Subject: [PATCH 2/5] refactor(ios/engine): target abstraction - ResourceFileManager.install --- .../KMEI/KeymanEngine/Classes/Manager.swift | 104 ++---------------- .../ResourceFileManager.swift | 47 ++++++++ .../FileManagementTests.swift | 58 ++++++++++ 3 files changed, 113 insertions(+), 96 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift index d6c8ddd19b..571598e911 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift @@ -617,60 +617,11 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { throw KMPError.wrongPackageType } - for r in kmp.resources { - let installableFiles: [(LanguageResource, [(String, URL)])] = r.installableResources.map { resource in - // (source file, destination file) - var set: [(String, URL)] = [(resource.sourceFilename, Storage.active.resourceURL(for: resource)!)] - let installableFonts: [(String, URL)] = resource.fonts.map { font in - // KMPs only list a single font file for each entry whenever one is included. - // A pre-existing assumption. - let fontFile = font.source[0] - return (fontFile, Storage.active.fontURL(forResource: resource, filename: fontFile)!) - } - - set.append(contentsOf: installableFonts) - - // (resource, source-destination file mapping) - return (resource, set) - } - - var haveInstalledOne = false - for set in installableFiles { - let resource = set.0 - let files = set.1 - - do { - try FileManager.default.createDirectory(at: Storage.active.resourceDir(for: resource)!, - withIntermediateDirectories: true) - } catch { - log.error("Could not create dir for download: \(error)") - throw KMPError.fileSystem - } - - do { - for item in files { - var filePath = folder - filePath.appendPathComponent(item.0) - - // TODO: The rest of this block may be replaced with ResourceFileManager.copyWithOverwrite - // once this method is fully abstracted and its core is placed within said class. - if(FileManager.default.fileExists(atPath: (item.1).path)) { - try FileManager.default.removeItem(at: item.1) - } - try FileManager.default.copyItem(at: filePath, to: item.1) - - } - } catch { - log.error("Error saving the download: \(error)") - throw KMPError.copyFiles - } - - // Only one keyboard is installed by default from a KMP. - // Also, only the first language-pairing it contains - if !haveInstalledOne { - Manager.shared.addKeyboard(resource as! InstallableKeyboard) - haveInstalledOne = true - } + for resourceSet in kmp.installableResourceSets { + for resource in resourceSet { + try ResourceFileManager.shared.install(resource, from: kmp) + // Install the keyboard for only the first language pairing defined in the package. + break } } } @@ -681,48 +632,9 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { throw KMPError.wrongPackageType } - for r in kmp.resources { - let installableFiles: [(LanguageResource, [(String, URL)])] = r.installableResources.map { resource in - // (source file, destination file) - let set: [(String, URL)] = [(resource.sourceFilename, Storage.active.resourceURL(for: resource)!)] - // no fonts for lexical models - - // (resource, source-destination file mapping) - return (resource, set) - } - - for set in installableFiles { - let resource = set.0 - let files = set.1 - - do { - try FileManager.default.createDirectory(at: Storage.active.resourceDir(for: resource)!, - withIntermediateDirectories: true) - } catch { - log.error("Could not create dir for download: \(error)") - throw KMPError.fileSystem - } - - do { - for item in files { - var filePath = folder - filePath.appendPathComponent(item.0) - - // TODO: The rest of this block may be replaced with ResourceFileManager.copyWithOverwrite - // once this method is fully abstracted and its core is placed within said class. - if(FileManager.default.fileExists(atPath: (item.1).path)) { - try FileManager.default.removeItem(at: item.1) - } - try FileManager.default.copyItem(at: filePath, to: item.1) - } - } catch { - log.error("Error saving the lexical model download: \(error)") - throw KMPError.copyFiles - } - - // All pairings are installed for lexical models. - // Also, all models within the KMP - Manager.addLexicalModel(resource as! InstallableLexicalModel) + try kmp.installableResourceSets.forEach { resourceSet in + try resourceSet.forEach { resource in + try ResourceFileManager.shared.install(resource, from: kmp) } } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index b9f1098568..991603d842 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -178,4 +178,51 @@ public class ResourceFileManager { completionHandler(error) } } + + public func install(_ resource: LanguageResource, from package: KeymanPackage) throws { + // Ideally, we wouldn't need the guard... but it's best to check just in case. + guard package.contains(resource) else { + throw KMPError.resourceNotInPackage + } + + // (source, destination) + var installableFiles: [(String, URL)] = [(resource.sourceFilename, Storage.active.resourceURL(for: resource)!)] + let installableFonts: [(String, URL)] = resource.fonts.map { font in + // KMPs only list a single font file for each entry whenever one is included. + // A pre-existing assumption. + let fontFile = font.source[0] + return (fontFile, Storage.active.fontURL(forResource: resource, filename: fontFile)!) + } + + installableFiles.append(contentsOf: installableFonts) + + do { + try FileManager.default.createDirectory(at: Storage.active.resourceDir(for: resource)!, + withIntermediateDirectories: true) + } catch { + log.error("Could not create installation directory: \(error)") + throw KMPError.fileSystem + } + + do { + for item in installableFiles { + var filePath = package.sourceFolder + filePath.appendPathComponent(item.0) + try copyWithOverwrite(from: filePath, to: item.1) + } + } catch { + log.error("Error installing the resource: \(error)") + throw KMPError.copyFiles + } + + // There's no generalized method for this quite yet. Manager doesn't need even + // more of these. + if let keyboard = resource as? InstallableKeyboard { + Manager.shared.addKeyboard(keyboard) + } else if let lexicalModel = resource as? InstallableLexicalModel { + Manager.addLexicalModel(lexicalModel) + } else { + fatalError("Cannot install instance of unexpected LanguageResource subclass") + } + } } diff --git a/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift b/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift index 50d50123f6..826b16871e 100644 --- a/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift @@ -69,4 +69,62 @@ class FileManagementTests: XCTestCase { } } } + + func testInstallKeyboardFromPackage() throws { + ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, error in + XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") + XCTAssertNil(error, "Error occurred while preparing KMP for installation") + XCTAssertNotNil(kmp as? KeyboardKeymanPackage, "KMP resource type improperly recognized - expected a keyboard package!") + + do { + try ResourceFileManager.shared.install(TestUtils.Keyboards.khmer_angkor, from: kmp!) + } catch { + XCTFail("Unexpected error during KeyboardPackage install") + } + + let installURL = Storage.active.keyboardURL(forID: "khmer_angkor", version: "1.0.6") + + XCTAssertTrue(FileManager.default.fileExists(atPath: installURL.path), + "Could not find installed keyboard file") + + let keyboards = Storage.active.userDefaults.userKeyboards! + + XCTAssertEqual(keyboards.count, 1, "Unexpected number of keyboards were installed") + XCTAssertEqual(keyboards[0].id, "khmer_angkor", "Installed keyboard ID mismatch") + + // While the KMP's version of the specified InstallableKeyboard does specify Fonts, + // the literal-based testing version does NOT. Since we're installing from the predefined, + // test-copy instance, we expect NOT to see the font from this test's install! + // + // Yes, the KeymanPackage.contains check isn't exhaustive - it just does a pair of + // id checks. This version of events couldn't happen if we thorough enough there. + let fontURL = Storage.active.fontURL(forKeyboardID: "khmer_angkor", filename: "Mondulkiri-R.ttf") + XCTAssertFalse(FileManager.default.fileExists(atPath: fontURL.path)) + } + } + + func testInstallLexicalModelFromPackage() { + ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.LexicalModels.mtntKMP) { kmp, error in + XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") + XCTAssertNil(error, "Error occurred while preparing KMP for installation") + XCTAssertNotNil(kmp as? LexicalModelKeymanPackage, "KMP resource type improperly recognized - expected a lexical model package!") + + do { + try ResourceFileManager.shared.install(TestUtils.LexicalModels.mtnt, from: kmp!) + } catch { + XCTFail("Unexpected error during LexicalModelPackage install") + } + + let installURL = Storage.active.lexicalModelURL(forID: "nrc.en.mtnt", version: "0.1.4") + + XCTAssertTrue(FileManager.default.fileExists(atPath: installURL.path), + "Could not find installed lexical model file") + + let models = Storage.active.userDefaults.userLexicalModels! + + // This variant is selective - only a single pairing should be installed for the model. + XCTAssertEqual(models.count, 1, "Unexpected number of models were installed") + XCTAssertEqual(models[0].id, "nrc.en.mtnt", "Installed lexical model ID mismatch") + } + } } From 15d0f61541e2a2f087deb3ec8d286dfda3dcd3cb Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 5 Jun 2020 13:51:38 +0700 Subject: [PATCH 3/5] refactor(ios/engine): contains -> findMatch, install(usePackageDefinition) param --- .../KeymanEngine/Classes/KeymanPackage.swift | 12 ++---- .../ResourceFileManager.swift | 17 +++++++-- .../FileManagementTests.swift | 38 ++++++++++++++++--- .../KeymanPackageTests.swift | 10 ++--- 4 files changed, 56 insertions(+), 21 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift index 11ece2f597..2145937d29 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift @@ -120,8 +120,8 @@ public class KeymanPackage { }) } - public func contains(_ resource: LanguageResource) -> Bool { - let matchesFound: [[LanguageResource]] = self.installableResourceSets.compactMap { set in + public func findMatch(_ resource: LanguageResource) -> LanguageResource? { + let matchesFound: [LanguageResource] = self.installableResourceSets.compactMap { set in let setMatches: [LanguageResource] = set.compactMap { other in if resource.id == other.id && resource.languageID == other.languageID { return other @@ -130,13 +130,9 @@ public class KeymanPackage { } } - if setMatches.count > 0 { - return setMatches - } else { - return nil - } + return setMatches.count > 0 ? setMatches[0] : nil } - return matchesFound.count > 0 + return matchesFound.count > 0 ? matchesFound[0] : nil } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index 991603d842..7231edf521 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -179,12 +179,23 @@ public class ResourceFileManager { } } - public func install(_ resource: LanguageResource, from package: KeymanPackage) throws { - // Ideally, we wouldn't need the guard... but it's best to check just in case. - guard package.contains(resource) else { + public func install(_ resourceToMatch: LanguageResource, from package: KeymanPackage, usePackageDefinition: Bool = true) throws { + + var res: LanguageResource + guard let r = package.findMatch(resourceToMatch) else { throw KMPError.resourceNotInPackage } + if(usePackageDefinition) { + res = r + } else { + // In case a KeymanEngine user wants a different definition than is in the source KMP. + res = resourceToMatch + } + + // Now to lock down the reference, using 'let' to make it read-only. + let resource = res + // (source, destination) var installableFiles: [(String, URL)] = [(resource.sourceFilename, Storage.active.resourceURL(for: resource)!)] let installableFonts: [(String, URL)] = resource.fonts.map { font in diff --git a/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift b/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift index 826b16871e..7d4afe5809 100644 --- a/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift @@ -71,6 +71,7 @@ class FileManagementTests: XCTestCase { } func testInstallKeyboardFromPackage() throws { + // Standard installation ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, error in XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") XCTAssertNil(error, "Error occurred while preparing KMP for installation") @@ -92,12 +93,39 @@ class FileManagementTests: XCTestCase { XCTAssertEqual(keyboards.count, 1, "Unexpected number of keyboards were installed") XCTAssertEqual(keyboards[0].id, "khmer_angkor", "Installed keyboard ID mismatch") + // While the LanguageResource definition we provided lacks font definitions, the + // KMP's definition has that data. By default, the KMP's definition takes precedence. + let fontURL = Storage.active.fontURL(forKeyboardID: "khmer_angkor", filename: "Mondulkiri-R.ttf") + XCTAssertTrue(FileManager.default.fileExists(atPath: fontURL.path)) + } + } + + func testInstallKeyboardFromPackageCustomDefinition() throws { + // Modified installation - KeymanEngine user supplies external/literal definition for LanguageResource. + // Separate test method b/c "tearDown()" cleans out installation artifacts from the other version. + ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, error in + XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") + XCTAssertNil(error, "Error occurred while preparing KMP for installation") + XCTAssertNotNil(kmp as? KeyboardKeymanPackage, "KMP resource type improperly recognized - expected a keyboard package!") + + do { + try ResourceFileManager.shared.install(TestUtils.Keyboards.khmer_angkor, from: kmp!, usePackageDefinition: false) + } catch { + XCTFail("Unexpected error during KeyboardPackage install") + } + + let installURL = Storage.active.keyboardURL(forID: "khmer_angkor", version: "1.0.6") + + XCTAssertTrue(FileManager.default.fileExists(atPath: installURL.path), + "Could not find installed keyboard file") + + let keyboards = Storage.active.userDefaults.userKeyboards! + + XCTAssertEqual(keyboards.count, 1, "Unexpected number of keyboards were installed") + XCTAssertEqual(keyboards[0].id, "khmer_angkor", "Installed keyboard ID mismatch") + // While the KMP's version of the specified InstallableKeyboard does specify Fonts, - // the literal-based testing version does NOT. Since we're installing from the predefined, - // test-copy instance, we expect NOT to see the font from this test's install! - // - // Yes, the KeymanPackage.contains check isn't exhaustive - it just does a pair of - // id checks. This version of events couldn't happen if we thorough enough there. + // the literal-based testing version does NOT. Since in this test, we're installing from the predefined, test-copy instance, we expect NOT to see the font from this test's install! let fontURL = Storage.active.fontURL(forKeyboardID: "khmer_angkor", filename: "Mondulkiri-R.ttf") XCTAssertFalse(FileManager.default.fileExists(atPath: fontURL.path)) } diff --git a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift index 2ef17d534e..3c088038ef 100644 --- a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift @@ -75,15 +75,15 @@ class KeymanPackageTests: XCTestCase { } } - func testPackageContainsResource() { + func testPackageFindResourceMatch() { ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, _ in - XCTAssertTrue(kmp!.contains(TestUtils.Keyboards.khmer_angkor)) - XCTAssertFalse(kmp!.contains(TestUtils.LexicalModels.mtnt)) + XCTAssertNotNil(kmp!.findMatch(TestUtils.Keyboards.khmer_angkor)) + XCTAssertNil(kmp!.findMatch(TestUtils.LexicalModels.mtnt)) } ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.LexicalModels.mtntKMP) { kmp, _ in - XCTAssertTrue(kmp!.contains(TestUtils.LexicalModels.mtnt)) - XCTAssertFalse(kmp!.contains(TestUtils.Keyboards.khmer_angkor)) + XCTAssertNotNil(kmp!.findMatch(TestUtils.LexicalModels.mtnt)) + XCTAssertNil(kmp!.findMatch(TestUtils.Keyboards.khmer_angkor)) } } } From 3bf2d8f688995ec96cc9be66a70a061c3fcbecf9 Mon Sep 17 00:00:00 2001 From: jahorton Date: Wed, 10 Jun 2020 09:33:05 +0700 Subject: [PATCH 4/5] chore(ios/engine): missed parts of the merge --- ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift | 6 +++--- .../Classes/Resource Management/ResourceFileManager.swift | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift index b07fe280e5..d6bd0552bc 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift @@ -118,9 +118,9 @@ public class KeymanPackage { }) } - public func findMatch(_ resource: LanguageResource) -> LanguageResource? { - let matchesFound: [LanguageResource] = self.installableResourceSets.compactMap { set in - let setMatches: [LanguageResource] = set.compactMap { other in + public func findMatch(_ resource: AnyLanguageResource) -> AnyLanguageResource? { + let matchesFound: [AnyLanguageResource] = self.installableResourceSets.compactMap { set in + let setMatches: [AnyLanguageResource] = set.compactMap { other in if resource.id == other.id && resource.languageID == other.languageID { return other } else { diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index 7231edf521..7b9356409c 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -179,9 +179,9 @@ public class ResourceFileManager { } } - public func install(_ resourceToMatch: LanguageResource, from package: KeymanPackage, usePackageDefinition: Bool = true) throws { + public func install(_ resourceToMatch: AnyLanguageResource, from package: KeymanPackage, usePackageDefinition: Bool = true) throws { - var res: LanguageResource + var res: AnyLanguageResource guard let r = package.findMatch(resourceToMatch) else { throw KMPError.resourceNotInPackage } From 315b2de2a0be2150d5cb2d1d8e555c5b4d53313a Mon Sep 17 00:00:00 2001 From: jahorton Date: Wed, 10 Jun 2020 10:30:19 +0700 Subject: [PATCH 5/5] change(ios/engine): usage of #3220's new typings, beter docs --- .../Classes/KeyboardKeymanPackage.swift | 2 +- .../KeymanEngine/Classes/KeymanPackage.swift | 60 +++++++++++++------ .../Classes/LexicalModelKeymanPackage.swift | 2 +- .../KMEI/KeymanEngine/Classes/Manager.swift | 4 +- .../ResourceFileManager.swift | 18 ++---- .../FileManagementTests.swift | 45 ++++---------- .../KeymanPackageTests.swift | 22 +++++-- 7 files changed, 79 insertions(+), 74 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift index c33e344f40..71993234b7 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift @@ -11,7 +11,7 @@ import Foundation public class KeyboardKeymanPackage : TypedKeymanPackage { internal var keyboards: [KMPKeyboard]! - override init(metadata: KMPMetadata, folder: URL) { + override internal init(metadata: KMPMetadata, folder: URL) { super.init(metadata: metadata, folder: folder) self.keyboards = [] diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift index d6bd0552bc..dce8a0d192 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift @@ -18,12 +18,15 @@ public enum KMPError : String, Error { case resourceNotInPackage = "Resource cannot be found in specified package" } +/** + * Common base class for the different KeymanPackage types. + */ public class KeymanPackage { static private let kmpFile = "kmp.json" public let sourceFolder: URL internal let metadata: KMPMetadata - init(metadata: KMPMetadata, folder: URL) { + internal init(metadata: KMPMetadata, folder: URL) { sourceFolder = folder self.metadata = metadata } @@ -32,6 +35,21 @@ public class KeymanPackage { return metadata.packageType == .Keyboard } + /** + * Returns the type of LanguageResource contained within the parsed Package. + */ + public func resourceType() -> LanguageResourceType { + switch metadata.packageType { + case .Keyboard: + return .keyboard + case .LexicalModel: + return .lexicalModel + default: + // See KeymanPackage.parse below; an error is thrown there for case .Unsupported. + fatalError("KeymanPackage.parse failed to block construction of unsupported KeymanPackage instance") + } + } + // to be overridden by subclasses public func defaultInfoHtml() -> String { return "base class!" @@ -68,7 +86,13 @@ public class KeymanPackage { public var installableResourceSets: [[AnyLanguageResource]] { fatalError("abstract base method went unimplemented by derived class") } - + + /** + * Parses a decompressed KMP's metadata file, producing the typed KeymanPackage instance corresponding to it. + * + * 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 @@ -117,22 +141,6 @@ public class KeymanPackage { } }) } - - public func findMatch(_ resource: AnyLanguageResource) -> AnyLanguageResource? { - let matchesFound: [AnyLanguageResource] = self.installableResourceSets.compactMap { set in - let setMatches: [AnyLanguageResource] = set.compactMap { other in - if resource.id == other.id && resource.languageID == other.languageID { - return other - } else { - return nil - } - } - - return setMatches.count > 0 ? setMatches[0] : nil - } - - return matchesFound.count > 0 ? matchesFound[0] : nil - } } public class TypedKeymanPackage: KeymanPackage { @@ -150,4 +158,20 @@ public class TypedKeymanPackage: Keyman public override var installableResourceSets: [[AnyLanguageResource]] { return installables } + + public func findResource(withID fullID: TypedLanguageResource.FullID) -> TypedLanguageResource? { + let matchesFound: [TypedLanguageResource] = self.installables.compactMap { set in + let setMatches: [TypedLanguageResource] = set.compactMap { other in + if fullID.id == other.id && fullID.languageID == other.languageID { + return other + } else { + return nil + } + } + + return setMatches.count > 0 ? setMatches[0] : nil + } + + return matchesFound.count > 0 ? matchesFound[0] : nil + } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift index 44ea0d87d9..9936b21ad6 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift @@ -11,7 +11,7 @@ import Foundation public class LexicalModelKeymanPackage : TypedKeymanPackage { internal var models : [KMPLexicalModel]! - override init(metadata: KMPMetadata, folder: URL) { + override internal init(metadata: KMPMetadata, folder: URL) { super.init(metadata: metadata, folder: folder) self.models = [] diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift index c26f21c4b1..9d406e80de 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift @@ -619,7 +619,7 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { for resourceSet in kmp.installables { for resource in resourceSet { - try ResourceFileManager.shared.install(resource, from: kmp) + try ResourceFileManager.shared.install(resourceWithID: resource.fullID, from: kmp) // Install the keyboard for only the first language pairing defined in the package. break } @@ -634,7 +634,7 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { try kmp.installables.forEach { resourceSet in try resourceSet.forEach { resource in - try ResourceFileManager.shared.install(resource, from: kmp) + try ResourceFileManager.shared.install(resourceWithID: resource.fullID, from: kmp) } } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index 7b9356409c..d8d76085ee 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -179,23 +179,15 @@ public class ResourceFileManager { } } - public func install(_ resourceToMatch: AnyLanguageResource, from package: KeymanPackage, usePackageDefinition: Bool = true) throws { + public func install> ( + resourceWithID fullID: ResourceType.FullID, + from package: PackageType) throws { - var res: AnyLanguageResource - guard let r = package.findMatch(resourceToMatch) else { + guard let resource = package.findResource(withID: fullID) else { throw KMPError.resourceNotInPackage } - if(usePackageDefinition) { - res = r - } else { - // In case a KeymanEngine user wants a different definition than is in the source KMP. - res = resourceToMatch - } - - // Now to lock down the reference, using 'let' to make it read-only. - let resource = res - // (source, destination) var installableFiles: [(String, URL)] = [(resource.sourceFilename, Storage.active.resourceURL(for: resource)!)] let installableFonts: [(String, URL)] = resource.fonts.map { font in diff --git a/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift b/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift index 7d4afe5809..96bc4f6dfc 100644 --- a/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/FileManagementTests.swift @@ -75,10 +75,13 @@ class FileManagementTests: XCTestCase { ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, error in XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") XCTAssertNil(error, "Error occurred while preparing KMP for installation") - XCTAssertNotNil(kmp as? KeyboardKeymanPackage, "KMP resource type improperly recognized - expected a keyboard package!") + guard let kmp = kmp as? KeyboardKeymanPackage else { + XCTFail("KMP resource type improperly recognized - expected a keyboard package!") + return + } do { - try ResourceFileManager.shared.install(TestUtils.Keyboards.khmer_angkor, from: kmp!) + try ResourceFileManager.shared.install(resourceWithID: TestUtils.Keyboards.khmer_angkor.fullID, from: kmp) } catch { XCTFail("Unexpected error during KeyboardPackage install") } @@ -100,45 +103,17 @@ class FileManagementTests: XCTestCase { } } - func testInstallKeyboardFromPackageCustomDefinition() throws { - // Modified installation - KeymanEngine user supplies external/literal definition for LanguageResource. - // Separate test method b/c "tearDown()" cleans out installation artifacts from the other version. - ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, error in - XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") - XCTAssertNil(error, "Error occurred while preparing KMP for installation") - XCTAssertNotNil(kmp as? KeyboardKeymanPackage, "KMP resource type improperly recognized - expected a keyboard package!") - - do { - try ResourceFileManager.shared.install(TestUtils.Keyboards.khmer_angkor, from: kmp!, usePackageDefinition: false) - } catch { - XCTFail("Unexpected error during KeyboardPackage install") - } - - let installURL = Storage.active.keyboardURL(forID: "khmer_angkor", version: "1.0.6") - - XCTAssertTrue(FileManager.default.fileExists(atPath: installURL.path), - "Could not find installed keyboard file") - - let keyboards = Storage.active.userDefaults.userKeyboards! - - XCTAssertEqual(keyboards.count, 1, "Unexpected number of keyboards were installed") - XCTAssertEqual(keyboards[0].id, "khmer_angkor", "Installed keyboard ID mismatch") - - // While the KMP's version of the specified InstallableKeyboard does specify Fonts, - // the literal-based testing version does NOT. Since in this test, we're installing from the predefined, test-copy instance, we expect NOT to see the font from this test's install! - let fontURL = Storage.active.fontURL(forKeyboardID: "khmer_angkor", filename: "Mondulkiri-R.ttf") - XCTAssertFalse(FileManager.default.fileExists(atPath: fontURL.path)) - } - } - func testInstallLexicalModelFromPackage() { ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.LexicalModels.mtntKMP) { kmp, error in XCTAssertNotNil(kmp, "Failed to prepare KMP for installation") XCTAssertNil(error, "Error occurred while preparing KMP for installation") - XCTAssertNotNil(kmp as? LexicalModelKeymanPackage, "KMP resource type improperly recognized - expected a lexical model package!") + guard let kmp = kmp as? LexicalModelKeymanPackage else { + XCTFail("KMP resource type improperly recognized - expected a lexical model package!") + return + } do { - try ResourceFileManager.shared.install(TestUtils.LexicalModels.mtnt, from: kmp!) + try ResourceFileManager.shared.install(resourceWithID: TestUtils.LexicalModels.mtnt.fullID, from: kmp) } catch { XCTFail("Unexpected error during LexicalModelPackage install") } diff --git a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift index 3c088038ef..3295eb0849 100644 --- a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift @@ -77,13 +77,27 @@ class KeymanPackageTests: XCTestCase { func testPackageFindResourceMatch() { ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.Keyboards.khmerAngkorKMP) { kmp, _ in - XCTAssertNotNil(kmp!.findMatch(TestUtils.Keyboards.khmer_angkor)) - XCTAssertNil(kmp!.findMatch(TestUtils.LexicalModels.mtnt)) + guard let kmp = kmp as? KeyboardKeymanPackage else { + XCTFail("Incorrect package type loaded for test") + return + } + XCTAssertNotNil(kmp.findResource(withID: TestUtils.Keyboards.khmer_angkor.fullID)) + // This keyboard's not in the specified testing package. + XCTAssertNil(kmp.findResource(withID: TestUtils.Keyboards.khmer10.fullID)) + + // Thanks to our package typing hierarchy, it's impossible to even TRY finding + // a FullLexicalModelID within a KeyboardKeymanPackage! } ResourceFileManager.shared.prepareKMPInstall(from: TestUtils.LexicalModels.mtntKMP) { kmp, _ in - XCTAssertNotNil(kmp!.findMatch(TestUtils.LexicalModels.mtnt)) - XCTAssertNil(kmp!.findMatch(TestUtils.Keyboards.khmer_angkor)) + guard let kmp = kmp as? LexicalModelKeymanPackage else { + XCTFail("Incorrect package type loaded for test") + return + } + XCTAssertNotNil(kmp.findResource(withID: TestUtils.LexicalModels.mtnt.fullID)) + + // Thanks to our package typing hierarchy, it's impossible to even TRY finding + // a FullKeyboardID within a LexicalModelKeymanPackage! } } }