From cfd76c032625d5e9217cfe98412f738744c9745f Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 9 Jul 2020 15:25:17 +0700 Subject: [PATCH] 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" + } + } +}