From 292a0039de1b6fb5a019f9efad77b8ccaf133ba1 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 23 Jul 2021 09:02:34 +0700 Subject: [PATCH 1/2] fix(ios/engine): engine migration must precede installs --- .../Resource Management/ResourceFileManager.swift | 12 ++++++++++++ ios/samples/KMSample2/KMSample2/AppDelegate.swift | 2 ++ 2 files changed, 14 insertions(+) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index 572afce019..190a3c8180 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -328,6 +328,18 @@ public class ResourceFileManager { throw KMPError.resourceNotInPackage } + /** + * A surprisingly critical line. The Manager.shared instance must be initialized at some + * point before the resources are _actually_ stored and reigstered. Otherwise, the initial + * Migrations pass (called early in Manager.init) will interpret the installation as from a + * different engine version and will break anything installed before it's run. + * + * Fortunately... all 14.0's installation methods pass through this single method in order to + * do the actual "storing" and "registering". So, it's a decent-enough place to force + * Manager.shared's init. + */ + _ = Manager.shared + do { try copyWithOverwrite(from: package.sourceFolder, to: Storage.active.packageDir(for: package)!) diff --git a/ios/samples/KMSample2/KMSample2/AppDelegate.swift b/ios/samples/KMSample2/KMSample2/AppDelegate.swift index f27725c98c..bff8a7e0a4 100644 --- a/ios/samples/KMSample2/KMSample2/AppDelegate.swift +++ b/ios/samples/KMSample2/KMSample2/AppDelegate.swift @@ -20,6 +20,8 @@ class AppDelegate: UIResponder, UIApplicationDelegate { KeymanEngine.log.logAppDetails() // Replace with your application group id + // Ensure this happens before installing any keyboards or models within the engine + // whenever using App Group Identifiers. Manager.applicationGroupIdentifier = "group.KMSample" return true } From 0b6915918ea9f8ce270fbef1bb469e6ad5264a76 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 23 Jul 2021 09:42:25 +0700 Subject: [PATCH 2/2] fix(ios/engine): recursive init reqts are bad --- .../KMEI/KeymanEngine/Classes/Manager.swift | 19 ++------ .../ResourceFileManager.swift | 47 ++++++++++++++----- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift index 5719e5ed99..564d75706d 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Manager.swift @@ -201,20 +201,11 @@ public class Manager: NSObject, UIGestureRecognizerDelegate { URLProtocol.registerClass(KeymanURLProtocol.self) - Migrations.migrate(storage: Storage.active) - Migrations.updateResources(storage: Storage.active) - - if Storage.active.userDefaults.userKeyboards?.isEmpty ?? true { - Storage.active.userDefaults.userKeyboards = [Defaults.keyboard] - - // Ensure the default keyboard is installed in this case. - do { - try Storage.active.installDefaultKeyboard(from: Resources.bundle) - } catch { - log.error("Failed to copy default keyboard from bundle: \(error)") - } - } - Migrations.engineVersion = Version.latestFeature + /** + * As of 14.0, ResourceFileManager is responsible for handlng resources... including + * any necessary "migration" of their internal storage structure & tracked metadata. + */ + ResourceFileManager.shared.runMigrationsIfNeeded() if Util.isSystemKeyboard || Storage.active.userDefaults.bool(forKey: Key.keyboardPickerDisplayed) { isKeymanHelpOn = false diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift index 190a3c8180..b922fc766e 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift @@ -17,9 +17,38 @@ import UIKit public class ResourceFileManager { public static let shared = ResourceFileManager() + private var haveRunMigrations: Bool = false + fileprivate init() { } + func runMigrationsIfNeeded() { + if !haveRunMigrations { + // Set here in order to prevent recursively calling itself in case + // Migrations itself needs to use installation methods. + haveRunMigrations = true + + // We must make sure that all resources are properly migrated before + // allowing any new resources to be installed. + Migrations.migrate(storage: Storage.active) + Migrations.updateResources(storage: Storage.active) + + if Storage.active.userDefaults.userKeyboards?.isEmpty ?? true { + Storage.active.userDefaults.userKeyboards = [Defaults.keyboard] + + // Ensure the default keyboard is installed in this case. + do { + try Storage.active.installDefaultKeyboard(from: Resources.bundle) + } catch { + log.error("Failed to copy default keyboard from bundle: \(error)") + } + } + Migrations.engineVersion = Version.latestFeature + } + + haveRunMigrations = true + } + public var installedPackages: [KeymanPackage] { let userResources = Storage.active.userDefaults.userResources ?? [] @@ -328,17 +357,13 @@ public class ResourceFileManager { throw KMPError.resourceNotInPackage } - /** - * A surprisingly critical line. The Manager.shared instance must be initialized at some - * point before the resources are _actually_ stored and reigstered. Otherwise, the initial - * Migrations pass (called early in Manager.init) will interpret the installation as from a - * different engine version and will break anything installed before it's run. - * - * Fortunately... all 14.0's installation methods pass through this single method in order to - * do the actual "storing" and "registering". So, it's a decent-enough place to force - * Manager.shared's init. - */ - _ = Manager.shared + // Resources should only be installed after Migrations have been run. + // Otherwise, the Migrations engine may misinterpret the installed format + // on an app's first install. + // + // It is possible for a KeymanEngine framework consumer to reach this point + // without `Manager.init` having been run. + runMigrationsIfNeeded() do { try copyWithOverwrite(from: package.sourceFolder,