From be116f01c5a4ac313f9e450aad31501ffef44a76 Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 9 Jul 2020 14:05:19 +0700 Subject: [PATCH 1/5] feat(ios/engine): runtime package-version query result caching --- .../Classes/Model/FullKeyboardID.swift | 2 +- .../Classes/Model/FullLexicalModelID.swift | 2 +- .../Classes/Model/LanguageResource.swift | 12 +++++- .../Queries/Queries+PackageVersion.swift | 25 ++++++++++- .../QueryPackageVersionTests.swift | 41 +++++++++++++++++++ 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Model/FullKeyboardID.swift b/ios/engine/KMEI/KeymanEngine/Classes/Model/FullKeyboardID.swift index e97f53e9a0..852459ba4a 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Model/FullKeyboardID.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Model/FullKeyboardID.swift @@ -9,7 +9,7 @@ import Foundation /// A complete identifier for an `InstallableKeyboard`. Keyboards must have unique `FullKeyboardID`s. -public struct FullKeyboardID: Codable, LanguageResourceFullID, Equatable { +public struct FullKeyboardID: Codable, LanguageResourceFullID, Hashable { public typealias Resource = InstallableKeyboard public var keyboardID: String diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Model/FullLexicalModelID.swift b/ios/engine/KMEI/KeymanEngine/Classes/Model/FullLexicalModelID.swift index 060b119978..8f0e8e9a3d 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Model/FullLexicalModelID.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Model/FullLexicalModelID.swift @@ -9,7 +9,7 @@ import Foundation /// A complete identifier for an `InstallableLexicalModel`. LexicalModels must have unique `FullLexicalModelID`s. -public struct FullLexicalModelID: Codable, LanguageResourceFullID, Equatable { +public struct FullLexicalModelID: Codable, LanguageResourceFullID, Hashable { public typealias Resource = InstallableLexicalModel public var lexicalModelID: String diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Model/LanguageResource.swift b/ios/engine/KMEI/KeymanEngine/Classes/Model/LanguageResource.swift index 2b429ea328..c012505185 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Model/LanguageResource.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Model/LanguageResource.swift @@ -39,13 +39,21 @@ extension AnyLanguageResourceFullID where Self: Equatable { } } +extension AnyLanguageResourceFullID where Self: Hashable { + public func hash(into hasher: inout Hasher) { + hasher.combine(self.id) + hasher.combine(self.languageID) + hasher.combine(self.type) + } +} + extension AnyLanguageResourceFullID { var description: String { return "{\(type): {id = \(id), languageID=\(languageID)}}" } } -public protocol LanguageResourceFullID: AnyLanguageResourceFullID { +public protocol LanguageResourceFullID: AnyLanguageResourceFullID, Hashable { associatedtype Resource: LanguageResource where Resource.FullID == Self } @@ -79,7 +87,7 @@ extension AnyLanguageResource { // Necessary due to Swift details 'documented' at // https://stackoverflow.com/questions/42561685/why-cant-a-get-only-property-requirement-in-a-protocol-be-satisfied-by-a-proper public protocol LanguageResource: AnyLanguageResource { - associatedtype FullID: LanguageResourceFullID where FullID: Equatable, FullID.Resource == Self + associatedtype FullID: LanguageResourceFullID where FullID.Resource == Self associatedtype Package: KeymanPackage var typedFullID: FullID { get } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift b/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift index 528194d2cd..1ba122391c 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift @@ -10,6 +10,8 @@ import Foundation extension Queries { class PackageVersion { + internal static var cachedResults: [AnyHashable : ResultEntry] = [:] + class ResultComponent {} class ResultEntry: ResultComponent, Decodable { @@ -118,7 +120,28 @@ extension Queries { log.info("Querying package versions through API endpoint: \(urlComponents.url!)") // Step 2: configure the completion closure. - let completionClosure = Queries.jsonDataTaskCompletionAdapter(resultType: Result.self, completionBlock: fetchCompletion) + let completionClosure = Queries.jsonDataTaskCompletionAdapter(resultType: Result.self) { result, error in + // Cache the results for future lookup. + if let result = result { + if let keyboards = result.keyboards { + fullIDs.compactMap { $0 as? FullKeyboardID }.forEach { kbd in + if let entry = keyboards[kbd.id] as? ResultEntry { + self.cachedResults[kbd] = entry + } + } + } + + if let models = result.models { + fullIDs.compactMap { $0 as? FullLexicalModelID }.forEach { lm in + if let entry = models[lm.id] as? ResultEntry { + self.cachedResults[lm] = entry + } + } + } + } + + fetchCompletion(result, error) + } // Step 3: run the actual query, letting the prepared completion closure take care of the rest. let task = session.dataTask(with: urlComponents.url!, completionHandler: completionClosure) diff --git a/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift b/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift index a40a7f4364..0f758963a0 100644 --- a/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift @@ -18,6 +18,7 @@ class QueryPackageVersionTests: XCTestCase { override func tearDownWithError() throws { let queueWasCleared = mockedURLSession!.queueIsEmpty + Queries.PackageVersion.cachedResults = [:] mockedURLSession = nil if !queueWasCleared { @@ -107,6 +108,46 @@ class QueryPackageVersionTests: XCTestCase { wait(for: [expectation], timeout: 5) } + /** + * Tests the caching behavior of fetch calls.. + */ + func testFetchCaching() throws { + let mockedResult = TestUtils.Downloading.MockResult(location: TestUtils.Queries.package_version_case_1, error: nil) + mockedURLSession?.queueMockResult(.data(mockedResult)) + + let badKbdFullID = FullKeyboardID(keyboardID: "foo", languageID: "en") + let badLexFullID = FullLexicalModelID(lexicalModelID: "bar", languageID: "km") + let fullIDs = [TestUtils.Keyboards.khmer_angkor.fullID, + TestUtils.Keyboards.sil_euro_latin.fullID, + badKbdFullID, + TestUtils.LexicalModels.mtnt.fullID, + badLexFullID] + + let expectation = XCTestExpectation(description: "Query complete and results analyzed") + + Queries.PackageVersion.fetch(for: fullIDs, withSession: mockedURLSession!) { results, error in + if let _ = error { + XCTFail(String(describing: error)) + expectation.fulfill() + return + } + XCTAssertNotNil(results) + + // Check to see that the results were appropriately cached. + XCTAssertNotNil(Queries.PackageVersion.cachedResults[TestUtils.Keyboards.khmer_angkor.fullID]) + XCTAssertNotNil(Queries.PackageVersion.cachedResults[TestUtils.Keyboards.sil_euro_latin.fullID]) + XCTAssertNotNil(Queries.PackageVersion.cachedResults[TestUtils.LexicalModels.mtnt.fullID]) + + // Error results are not cached. + XCTAssertNil(Queries.PackageVersion.cachedResults[badKbdFullID]) + XCTAssertNil(Queries.PackageVersion.cachedResults[badLexFullID]) + + expectation.fulfill() + } + + wait(for: [expectation], timeout: 5) + } + // Tests a fetch against a single resource. func testLexicalModelFetch() throws { let mockedResult = TestUtils.Downloading.MockResult(location: TestUtils.Queries.package_version_case_mtnt, error: nil) From ed33638f13a73817d01247183530ea3ce6a85eca Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 9 Jul 2020 15:25:17 +0700 Subject: [PATCH 2/5] refactor(ios/engine): better caching, stateForKeyboard rework/test --- .../Queries/Queries+PackageVersion.swift | 32 +++++++---- .../ResourceDownloadManager.swift | 6 +- .../QueryPackageVersionTests.swift | 12 ++-- .../ResourceDownloadManagerTests.swift | 55 ++++++++++++++++++- .../KeymanEngineTests/TestUtils/Queries.swift | 2 + .../package-version-km-updated.json | 8 +++ 6 files changed, 91 insertions(+), 24 deletions(-) create mode 100644 ios/engine/KMEI/KeymanEngineTests/resources/Queries.bundle/package-version-km-updated.json diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift b/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift index 1ba122391c..6708a4164d 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift @@ -10,7 +10,7 @@ import Foundation extension Queries { class PackageVersion { - internal static var cachedResults: [AnyHashable : ResultEntry] = [:] + private static var cachedResults: [LanguageResourceType : [String:ResultEntry]] = [.keyboard: [:], .lexicalModel: [:]] class ResultComponent {} @@ -121,31 +121,39 @@ extension Queries { // Step 2: configure the completion closure. let completionClosure = Queries.jsonDataTaskCompletionAdapter(resultType: Result.self) { result, error in - // Cache the results for future lookup. - if let result = result { - if let keyboards = result.keyboards { - fullIDs.compactMap { $0 as? FullKeyboardID }.forEach { kbd in - if let entry = keyboards[kbd.id] as? ResultEntry { - self.cachedResults[kbd] = entry + // Cache the results for future lookup. + if let result = result { + if let keyboards = result.keyboards { + keyboards.keys.forEach { id in + if let entry = keyboards[id] as? ResultEntry { + self.cachedResults[.keyboard]![id] = entry } } } if let models = result.models { - fullIDs.compactMap { $0 as? FullLexicalModelID }.forEach { lm in - if let entry = models[lm.id] as? ResultEntry { - self.cachedResults[lm] = entry + models.keys.forEach { id in + if let entry = models[id] as? ResultEntry { + self.cachedResults[.lexicalModel]![id] = entry } } } - } - fetchCompletion(result, error) + fetchCompletion(result, error) + } } // Step 3: run the actual query, letting the prepared completion closure take care of the rest. let task = session.dataTask(with: urlComponents.url!, completionHandler: completionClosure) task.resume() } + + public static func resetCache() { + self.cachedResults = [.keyboard: [:], .lexicalModel: [:]] + } + + static func cachedResult(for fullID: FullID) -> ResultEntry? { + return cachedResults[fullID.type]![fullID.id] + } } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift index 987716b237..a52db0e6a3 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift @@ -210,9 +210,8 @@ public class ResourceDownloadManager { return .needsDownload } - // TODO: convert to use of package-version API. // Check version - if let repositoryVersionString = Manager.shared.apiKeyboardRepository.keyboards?[keyboardID]?.version { + if let repositoryVersionString = Queries.PackageVersion.cachedResult(for: fullKeyboardID)?.version { let downloadedVersion = Version(userKeyboard.version) ?? Version.fallback let repositoryVersion = Version(repositoryVersionString) ?? Version.fallback if downloadedVersion < repositoryVersion { @@ -333,9 +332,8 @@ public class ResourceDownloadManager { return .needsDownload } - // TODO: Convert to use of package-version API. // Check version - if let repositoryVersionString = Manager.shared.apiLexicalModelRepository.lexicalModels?[lexicalModelID]?.version { + if let repositoryVersionString = Queries.PackageVersion.cachedResult(for: fullLexicalModelID)?.version { let downloadedVersion = Version(userLexicalModel.version) ?? Version.fallback let repositoryVersion = Version(repositoryVersionString) ?? Version.fallback if downloadedVersion < repositoryVersion { diff --git a/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift b/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift index 0f758963a0..b552b3e517 100644 --- a/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift @@ -18,7 +18,7 @@ class QueryPackageVersionTests: XCTestCase { override func tearDownWithError() throws { let queueWasCleared = mockedURLSession!.queueIsEmpty - Queries.PackageVersion.cachedResults = [:] + Queries.PackageVersion.resetCache() mockedURLSession = nil if !queueWasCleared { @@ -134,13 +134,13 @@ class QueryPackageVersionTests: XCTestCase { XCTAssertNotNil(results) // Check to see that the results were appropriately cached. - XCTAssertNotNil(Queries.PackageVersion.cachedResults[TestUtils.Keyboards.khmer_angkor.fullID]) - XCTAssertNotNil(Queries.PackageVersion.cachedResults[TestUtils.Keyboards.sil_euro_latin.fullID]) - XCTAssertNotNil(Queries.PackageVersion.cachedResults[TestUtils.LexicalModels.mtnt.fullID]) + XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: TestUtils.Keyboards.khmer_angkor.fullID)) + XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: TestUtils.Keyboards.sil_euro_latin.fullID)) + XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: TestUtils.LexicalModels.mtnt.fullID)) // Error results are not cached. - XCTAssertNil(Queries.PackageVersion.cachedResults[badKbdFullID]) - XCTAssertNil(Queries.PackageVersion.cachedResults[badLexFullID]) + XCTAssertNil(Queries.PackageVersion.cachedResult(for: badKbdFullID)) + XCTAssertNil(Queries.PackageVersion.cachedResult(for: badLexFullID)) expectation.fulfill() } diff --git a/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift b/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift index 1c0defd7bc..c2e1134210 100644 --- a/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift @@ -34,12 +34,12 @@ class ResourceDownloadManagerTests: XCTestCase { func testDownloadPackageForKeyboard() throws { let expectation = XCTestExpectation(description: "Mocked \"download\" should complete successfully.") - let mtnt_id = TestUtils.Keyboards.khmer_angkor.fullID + let khmer_angkor_id = TestUtils.Keyboards.khmer_angkor.fullID let mockedResult = TestUtils.Downloading.MockResult(location: TestUtils.Keyboards.khmerAngkorKMP, error: nil) mockedURLSession?.queueMockResult(.download(mockedResult)) - downloadManager?.downloadPackage(forFullID: mtnt_id, from: TestUtils.Keyboards.khmerAngkorKMP, withNotifications: false) { package, error in + downloadManager?.downloadPackage(forFullID: khmer_angkor_id, from: TestUtils.Keyboards.khmerAngkorKMP, withNotifications: false) { package, error in let tempDownloadKMP = ResourceFileManager.shared.packageDownloadTempPath(forID: TestUtils.Keyboards.khmer_angkor.fullID) @@ -162,4 +162,55 @@ class ResourceDownloadManagerTests: XCTestCase { wait(for: [expectation], timeout: 5) } + + func testStateForKeyboard() { + let baseInstallation = XCTestExpectation(description: "Mocked \"download\" should complete successfully.") + let khmer_angkor_id = TestUtils.Keyboards.khmer_angkor.fullID + + let mockedResult = TestUtils.Downloading.MockResult(location: TestUtils.Keyboards.khmerAngkorKMP, error: nil) + mockedURLSession?.queueMockResult(.download(mockedResult)) + + XCTAssertEqual(downloadManager!.stateForKeyboard(withID: khmer_angkor_id.id), .needsDownload) + + downloadManager!.downloadPackage(forFullID: khmer_angkor_id, + from: TestUtils.Keyboards.khmerAngkorKMP, + withNotifications: false) { package, error in + if let _ = error { + XCTFail() + baseInstallation.fulfill() + return + } else if let package = package { + do { + try ResourceFileManager.shared.install(resourceWithID: khmer_angkor_id, from: package) + baseInstallation.fulfill() + } catch { + XCTFail() + baseInstallation.fulfill() + } + } + } + + XCTAssertEqual(downloadManager!.stateForKeyboard(withID: khmer_angkor_id.id), .downloading) + + downloadManager!.downloader.step() + + wait(for: [baseInstallation], timeout: 5) + + XCTAssertEqual(downloadManager!.stateForKeyboard(withID: khmer_angkor_id.id), .upToDate) + + // Now, to test the update-check part of the function... + // This fixture was hand-altered. At the time of this test's creation, this version did not exist. + let mockedQuery = TestUtils.Downloading.MockResult(location: TestUtils.Queries.package_version_km_updated, error: nil) + mockedURLSession?.queueMockResult(.data(mockedQuery)) + + let versionQuery = XCTestExpectation() + + // Now to do a package-version check. + Queries.PackageVersion.fetch(for: [khmer_angkor_id], withSession: downloadManager!.session) { _, _ in + XCTAssertEqual(self.downloadManager!.stateForKeyboard(withID: khmer_angkor_id.id), .needsUpdate) + versionQuery.fulfill() + } + + wait(for: [versionQuery], timeout: 5) + } } diff --git a/ios/engine/KMEI/KeymanEngineTests/TestUtils/Queries.swift b/ios/engine/KMEI/KeymanEngineTests/TestUtils/Queries.swift index b06b747785..4d78d63c60 100644 --- a/ios/engine/KMEI/KeymanEngineTests/TestUtils/Queries.swift +++ b/ios/engine/KMEI/KeymanEngineTests/TestUtils/Queries.swift @@ -16,6 +16,8 @@ extension TestUtils { static let package_version_case_mtnt = query_bundle.url(forResource: "package-version-case-mtnt", withExtension: "json")! + static let package_version_km_updated = query_bundle.url(forResource: "package-version-km-updated", withExtension: "json")! + static let model_case_en = query_bundle.url(forResource: "model-case-en", withExtension: "json")! } } diff --git a/ios/engine/KMEI/KeymanEngineTests/resources/Queries.bundle/package-version-km-updated.json b/ios/engine/KMEI/KeymanEngineTests/resources/Queries.bundle/package-version-km-updated.json new file mode 100644 index 0000000000..98071de56f --- /dev/null +++ b/ios/engine/KMEI/KeymanEngineTests/resources/Queries.bundle/package-version-km-updated.json @@ -0,0 +1,8 @@ +{ + "keyboards": { + "khmer_angkor": { + "version": "1.1.0", + "kmp": "https://downloads.keyman.com/keyboards/khmer_angkor/1.0.6/khmer_angkor.kmp" + } + } +} From c51a9ce3562ccb63bb77f4a48792dbbb3f6ca4a7 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 10 Jul 2020 09:02:09 +0700 Subject: [PATCH 3/5] feat(ios/engine): starts package-version prefetch --- .../ResourceDownloadManager.swift | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift index a52db0e6a3..c62193b4fa 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift @@ -385,12 +385,44 @@ public class ResourceDownloadManager { } // MARK: Update checks + management + /** + * Given that an update-check query has already been run, returns whether or not any updates are available. + */ public var updatesAvailable: Bool { get { return getAvailableUpdates() != nil } } - + + /** + * Runs the package-version query against all installed resources to determine if any updates are available. + */ + public func fetchAvailableUpdates(completionBlock: (([AnyLanguageResourceFullID]?, Error?) -> Void)? = nil) { + let userDefaults = Storage.active.userDefaults + let kbdIDs = userDefaults.userKeyboards?.map { $0.fullID } + let modelIDs = userDefaults.userLexicalModels?.map { $0.fullID } + + let allIDs: [AnyLanguageResourceFullID] = (kbdIDs ?? []) + (modelIDs ?? []) + + Queries.PackageVersion.fetch(for: allIDs) { results, error in + guard error == nil else { + completionBlock?(nil, error) + return + } + + // If no completionBlock was specified, the caller simply wanted a prefetch. + // Any further processing we might try to do would go to waste, so stop here. + guard let completionBlock = completionBlock else { + return + } + + // Check for updates among the returned versions IF a completion block is specified. + // This facilitates a more proactive update notification. + + // TODO: flesh out! + } + } + public func getAvailableUpdates() -> [AnyLanguageResource]? { // Relies upon KMManager's preload; this was the case before the rework. if Manager.shared.apiKeyboardRepository.languages == nil && Manager.shared.apiLexicalModelRepository.languages == nil { From 85a93489b5fc063e8aacb4c1c9289f7530f445a3 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 10 Jul 2020 10:08:47 +0700 Subject: [PATCH 4/5] chore(ios/engine): adjustment to new base branch --- .../KMEI/KeymanEngine/Classes/Manager.swift | 2 ++ .../Queries/Queries+PackageVersion.swift | 4 ++-- .../ResourceDownloadManager.swift | 21 +++++++++++------- .../QueryPackageVersionTests.swift | 22 +++++++++---------- .../ResourceDownloadManagerTests.swift | 2 +- 5 files changed, 29 insertions(+), 22 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift index 6514440e13..e4a2037fa4 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift @@ -858,11 +858,13 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { completionBlock: completionBlock) } + @available(*, deprecated, message: "") // TODO: Write method on KeymanPackage for this. public func stateForKeyboard(withID keyboardID: String) -> KeyboardState { return ResourceDownloadManager.shared.stateForKeyboard(withID: keyboardID) } // Technically new, but it does closely parallel an old API point. + @available(*, deprecated, message: "") // TODO: Write method on KeymanPackage for this. public func stateForLexicalModel(withID modelID: String) -> KeyboardState { return ResourceDownloadManager.shared.stateForLexicalModel(withID: modelID) } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift b/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift index 6708a4164d..632f5f1d10 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Queries/Queries+PackageVersion.swift @@ -152,8 +152,8 @@ extension Queries { self.cachedResults = [.keyboard: [:], .lexicalModel: [:]] } - static func cachedResult(for fullID: FullID) -> ResultEntry? { - return cachedResults[fullID.type]![fullID.id] + static func cachedResult(for packageKey: KeymanPackage.Key) -> ResultEntry? { + return cachedResults[packageKey.type]![packageKey.id] } } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift index c62193b4fa..eb74e3be2b 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift @@ -196,8 +196,10 @@ public class ResourceDownloadManager { return font.source.filter({ $0.hasFontExtension }) .map({ options.fontBaseURL.appendingPathComponent($0) }) } - + + // Deprecating due to tricky assumption - a keyboard _can_ be installed from two separate packages. /// - Returns: The current state for a keyboard + @available(*, deprecated, message: "") // TODO: Write method on KeymanPackage for this. public func stateForKeyboard(withID keyboardID: String) -> KeyboardState { // For this call, we don't actually need the language ID to be correct. let fullKeyboardID = FullKeyboardID(keyboardID: keyboardID, languageID: "") @@ -211,7 +213,8 @@ public class ResourceDownloadManager { } // Check version - if let repositoryVersionString = Queries.PackageVersion.cachedResult(for: fullKeyboardID)?.version { + let packageKey = KeymanPackage.Key(forResource: userKeyboard) + if let repositoryVersionString = Queries.PackageVersion.cachedResult(for: packageKey)?.version { let downloadedVersion = Version(userKeyboard.version) ?? Version.fallback let repositoryVersion = Version(repositoryVersionString) ?? Version.fallback if downloadedVersion < repositoryVersion { @@ -320,6 +323,7 @@ public class ResourceDownloadManager { /// - Returns: The current state for a lexical model //TODO: rename KeyboardState to ResourceState? so it can be used with both keybaoards and lexical models without confusion + @available(*, deprecated, message: "") // TODO: Write method on KeymanPackage for this. public func stateForLexicalModel(withID lexicalModelID: String) -> KeyboardState { // For this call, we don't actually need the language ID to be correct. let fullLexicalModelID = FullLexicalModelID(lexicalModelID: lexicalModelID, languageID: "") @@ -333,7 +337,8 @@ public class ResourceDownloadManager { } // Check version - if let repositoryVersionString = Queries.PackageVersion.cachedResult(for: fullLexicalModelID)?.version { + let packageKey = KeymanPackage.Key(forResource: userLexicalModel) + if let repositoryVersionString = Queries.PackageVersion.cachedResult(for: packageKey)?.version { let downloadedVersion = Version(userLexicalModel.version) ?? Version.fallback let repositoryVersion = Version(repositoryVersionString) ?? Version.fallback if downloadedVersion < repositoryVersion { @@ -397,14 +402,14 @@ public class ResourceDownloadManager { /** * Runs the package-version query against all installed resources to determine if any updates are available. */ - public func fetchAvailableUpdates(completionBlock: (([AnyLanguageResourceFullID]?, Error?) -> Void)? = nil) { + public func fetchAvailableUpdates(completionBlock: (([KeymanPackage.Key]?, Error?) -> Void)? = nil) { let userDefaults = Storage.active.userDefaults - let kbdIDs = userDefaults.userKeyboards?.map { $0.fullID } - let modelIDs = userDefaults.userLexicalModels?.map { $0.fullID } + let keyboardPackages = userDefaults.userKeyboards?.map { $0.packageKey } + let lexicalModelPackages = userDefaults.userLexicalModels?.map { $0.packageKey } - let allIDs: [AnyLanguageResourceFullID] = (kbdIDs ?? []) + (modelIDs ?? []) + let packageKeys = (keyboardPackages ?? []) + (lexicalModelPackages ?? []) - Queries.PackageVersion.fetch(for: allIDs) { results, error in + Queries.PackageVersion.fetch(for: packageKeys) { results, error in guard error == nil else { completionBlock?(nil, error) return diff --git a/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift b/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift index b552b3e517..3d63b90f08 100644 --- a/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/QueryPackageVersionTests.swift @@ -117,15 +117,15 @@ class QueryPackageVersionTests: XCTestCase { let badKbdFullID = FullKeyboardID(keyboardID: "foo", languageID: "en") let badLexFullID = FullLexicalModelID(lexicalModelID: "bar", languageID: "km") - let fullIDs = [TestUtils.Keyboards.khmer_angkor.fullID, - TestUtils.Keyboards.sil_euro_latin.fullID, - badKbdFullID, - TestUtils.LexicalModels.mtnt.fullID, - badLexFullID] + let packageKeys = [TestUtils.Keyboards.khmer_angkor.fullID, + TestUtils.Keyboards.sil_euro_latin.fullID, + badKbdFullID, + TestUtils.LexicalModels.mtnt.fullID, + badLexFullID].map { packageKey(for: $0) } let expectation = XCTestExpectation(description: "Query complete and results analyzed") - Queries.PackageVersion.fetch(for: fullIDs, withSession: mockedURLSession!) { results, error in + Queries.PackageVersion.fetch(for: packageKeys, withSession: mockedURLSession!) { results, error in if let _ = error { XCTFail(String(describing: error)) expectation.fulfill() @@ -134,13 +134,13 @@ class QueryPackageVersionTests: XCTestCase { XCTAssertNotNil(results) // Check to see that the results were appropriately cached. - XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: TestUtils.Keyboards.khmer_angkor.fullID)) - XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: TestUtils.Keyboards.sil_euro_latin.fullID)) - XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: TestUtils.LexicalModels.mtnt.fullID)) + XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: self.packageKey(for: TestUtils.Keyboards.khmer_angkor.fullID))) + XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: self.packageKey(for: TestUtils.Keyboards.sil_euro_latin.fullID))) + XCTAssertNotNil(Queries.PackageVersion.cachedResult(for: self.packageKey(for: TestUtils.LexicalModels.mtnt.fullID))) // Error results are not cached. - XCTAssertNil(Queries.PackageVersion.cachedResult(for: badKbdFullID)) - XCTAssertNil(Queries.PackageVersion.cachedResult(for: badLexFullID)) + XCTAssertNil(Queries.PackageVersion.cachedResult(for: self.packageKey(for: badKbdFullID))) + XCTAssertNil(Queries.PackageVersion.cachedResult(for: self.packageKey(for: badLexFullID))) expectation.fulfill() } diff --git a/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift b/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift index c2e1134210..b583489f37 100644 --- a/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/ResourceDownloadManagerTests.swift @@ -206,7 +206,7 @@ class ResourceDownloadManagerTests: XCTestCase { let versionQuery = XCTestExpectation() // Now to do a package-version check. - Queries.PackageVersion.fetch(for: [khmer_angkor_id], withSession: downloadManager!.session) { _, _ in + Queries.PackageVersion.fetch(for: [KeymanPackage.Key(id: khmer_angkor_id.id, type: .keyboard)], withSession: downloadManager!.session) { _, _ in XCTAssertEqual(self.downloadManager!.stateForKeyboard(withID: khmer_angkor_id.id), .needsUpdate) versionQuery.fulfill() } From cbf43dcadb865e533b0593026a9c81617460c3f4 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 10 Jul 2020 13:35:45 +0700 Subject: [PATCH 5/5] refactor(ios/engine): package support-state processing --- .../KeymanEngine/Classes/KeymanPackage.swift | 114 ++++++++++++++++++ .../ResourceDownloadManager.swift | 29 ----- .../KeymanPackageTests.swift | 51 ++++++++ 3 files changed, 165 insertions(+), 29 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift index f19b3ef3a9..cfe52a6d81 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift @@ -54,6 +54,63 @@ public class KeymanPackage { } } + /** + * Indicates the distribution level and state of the package. + */ + public enum SupportState: String, Codable { + /** + * Indicates that the support level for the package is unknown. Generally occurs when a + * package is installed via file-sharing before KeymanEngine is able to perform a package-version check. + */ + case unknown + + /** + * Indicates that the package is publicly distributed but no longer maintained. Indicates that other packages targetting + * the same targets are more favored. + */ + case deprecated + + /** + * Indicates that this package receives full support and maintenance from the resource development community at large. + */ + case publiclyReleased = "publicly released" + + /** + * Indicates that this package is known to not be publicly distributed. + */ + case custom + } + + /** + * Cloud/query related metadata not tracked (or even trackable) within kmp.json regarding the support state of a keyboard. + */ + public struct SupportStateMetadata: Codable { + var latestVersion: String? + var timestampForLastQuery: TimeInterval? + + var supportState: SupportState + + init(from queryResult: Queries.PackageVersion.ResultComponent) { + if let entry = queryResult as? Queries.PackageVersion.ResultEntry { + self.latestVersion = entry.version + self.supportState = .publiclyReleased + } else /* if queryResult is Queries.PackageVersion.ResultError */ { + self.latestVersion = nil + // The package-version query knows nothing about it - must be custom. + self.supportState = .custom + } + + self.timestampForLastQuery = NSDate().timeIntervalSince1970 + } + + init(fromQuery: Bool = true) { + latestVersion = nil + timestampForLastQuery = nil + + supportState = fromQuery ? .publiclyReleased : .unknown + } + } + static private let kmpFile = "kmp.json" public let sourceFolder: URL public let id: String @@ -225,6 +282,63 @@ public class KeymanPackage { return nil } } + + /** + * Runs the package-version query for the specified packages to determine their current support-state. + */ + public static func querySupportStates(for keys: [Key], withSession session: URLSession = URLSession.shared, completionBlock: (([Key : SupportStateMetadata]?, Error?)-> Void)? = nil) { + Queries.PackageVersion.fetch(for: keys, withSession: session) { results, error in + guard error == nil, let results = results else { + completionBlock?(nil, error) + return + } + + var keyboardStates: [Key : SupportStateMetadata] = [:] + keyboardStates.reserveCapacity(results.keyboards?.count ?? 0) + + // Sadly, Dictionaries do not support mapping to other dictionaries when considering the keys. + results.keyboards?.forEach { key, value in + keyboardStates[KeymanPackage.Key(id: key, type: .keyboard)] = KeymanPackage.SupportStateMetadata(from: value) + } + + var lexicalModelStates: [Key : SupportStateMetadata] = [:] + lexicalModelStates.reserveCapacity(results.models?.count ?? 0) + + results.models?.forEach { key, value in + lexicalModelStates[KeymanPackage.Key(id: key, type: .lexicalModel)] = KeymanPackage.SupportStateMetadata(from: value) + } + + // TODO: Save these states to UserDefaults! + + // There will be no 'merge' conflicts, so we ignore them by simply selecting the original. + if let completionBlock = completionBlock { + let stateSet = keyboardStates.merging(lexicalModelStates, uniquingKeysWith: { lhs, _ in return lhs }) + completionBlock(stateSet, nil) + } + } + } + + /** + * Runs the package-version query for the specified packages to determine if any updates are available. + */ + public static func queryCurrentVersions(for keys: [Key], withSession session: URLSession = URLSession.shared, completionBlock: (([Key : Version]?, Error?) -> Void)? = nil) { + querySupportStates(for: keys, withSession: session) { stateSet, error in + guard error == nil, let stateSet = stateSet else { + completionBlock?(nil, error) + return + } + + let versionSet: [Key: Version] = stateSet.compactMapValues { value in + if let versionString = value.latestVersion, let version = Version(versionString) { + return version + } else { + return nil + } + } + + completionBlock?(versionSet, nil) + } + } } /** diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift index eb74e3be2b..78dc063e22 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceDownloadManager.swift @@ -399,35 +399,6 @@ public class ResourceDownloadManager { } } - /** - * Runs the package-version query against all installed resources to determine if any updates are available. - */ - public func fetchAvailableUpdates(completionBlock: (([KeymanPackage.Key]?, Error?) -> Void)? = nil) { - let userDefaults = Storage.active.userDefaults - let keyboardPackages = userDefaults.userKeyboards?.map { $0.packageKey } - let lexicalModelPackages = userDefaults.userLexicalModels?.map { $0.packageKey } - - let packageKeys = (keyboardPackages ?? []) + (lexicalModelPackages ?? []) - - Queries.PackageVersion.fetch(for: packageKeys) { results, error in - guard error == nil else { - completionBlock?(nil, error) - return - } - - // If no completionBlock was specified, the caller simply wanted a prefetch. - // Any further processing we might try to do would go to waste, so stop here. - guard let completionBlock = completionBlock else { - return - } - - // Check for updates among the returned versions IF a completion block is specified. - // This facilitates a more proactive update notification. - - // TODO: flesh out! - } - } - public func getAvailableUpdates() -> [AnyLanguageResource]? { // Relies upon KMManager's preload; this was the case before the rework. if Manager.shared.apiKeyboardRepository.languages == nil && Manager.shared.apiLexicalModelRepository.languages == nil { diff --git a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift index 4af869e89c..7771632cf0 100644 --- a/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift +++ b/ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift @@ -137,4 +137,55 @@ class KeymanPackageTests: XCTestCase { // Deinit should have triggered - were the files automatically cleaned up? XCTAssertFalse(FileManager.default.fileExists(atPath: tempDir.path)) } + + // Analogous to QueryPackageVersionTests.testMockedBatchFetchParse, but with more analysis applied + // and more integration. + func testQueryCurrentVersions() throws { + let mockedURLSession = TestUtils.Downloading.URLSessionMock() + + let expectation = XCTestExpectation(description: "The query completes as expected.") + + // Test setup + + let mockedResult = TestUtils.Downloading.MockResult(location: TestUtils.Queries.package_version_case_1, error: nil) + mockedURLSession.queueMockResult(.data(mockedResult)) + + let badKbdKey = KeymanPackage.Key(id: "foo", type: .keyboard) + let badLexKey = KeymanPackage.Key(id: "bar", type: .lexicalModel) + let packageKeys = [KeymanPackage.Key(forResource: TestUtils.Keyboards.khmer_angkor), + KeymanPackage.Key(forResource: TestUtils.Keyboards.sil_euro_latin), + KeymanPackage.Key(id: "foo", type: .keyboard), + KeymanPackage.Key(forResource: TestUtils.LexicalModels.mtnt), + KeymanPackage.Key(id: "bar", type: .lexicalModel)] + + KeymanPackage.queryCurrentVersions(for: packageKeys, withSession: mockedURLSession) { results, error in + guard error == nil, let results = results else { + XCTFail() + expectation.fulfill() + return + } + + let khmer_angkor = KeymanPackage.Key(forResource: TestUtils.Keyboards.khmer_angkor) + let sil_euro_latin = KeymanPackage.Key(forResource: TestUtils.Keyboards.sil_euro_latin) + let mtnt = KeymanPackage.Key(forResource: TestUtils.LexicalModels.mtnt) + XCTAssertEqual(results[khmer_angkor], Version("1.0.6")) + XCTAssertEqual(results[sil_euro_latin], Version("1.9.1")) + XCTAssertEqual(results[mtnt], Version("0.1.4")) + XCTAssertNil(results[badKbdKey]) + XCTAssertNil(results[badLexKey]) + expectation.fulfill() + } + + wait(for: [expectation], timeout: 5) + + // Post-execution cleanup + let queueWasCleared = mockedURLSession.queueIsEmpty + Queries.PackageVersion.resetCache() + + if !queueWasCleared { + throw NSError(domain: "Keyman", + code: 4, + userInfo: [NSLocalizedDescriptionKey: "A test did not fully utilize its queued mock results!"]) + } + } }