From 836814b0f3e712fa7fc9946d1325683b20cb15cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Buczek?= Date: Mon, 3 Apr 2023 17:02:29 +0200 Subject: [PATCH] No Bug: Remove old CoreData migration logic. (#7185) --- App/iOS/Delegates/AppDelegate.swift | 3 - Sources/Brave/Migration/Migration.swift | 22 --- Sources/Data/DataPreferences.swift | 16 --- Sources/Data/models/DataController.swift | 129 +----------------- Sources/DataTestsUtils/CoreDataTestCase.swift | 1 - 5 files changed, 2 insertions(+), 169 deletions(-) diff --git a/App/iOS/Delegates/AppDelegate.swift b/App/iOS/Delegates/AppDelegate.swift index 90a05082f62..4c7f29f1937 100644 --- a/App/iOS/Delegates/AppDelegate.swift +++ b/App/iOS/Delegates/AppDelegate.swift @@ -145,9 +145,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate { migration = Migration(braveCore: braveCore) - // Must happen before passcode check, otherwise may unnecessarily reset keychain - migration?.moveDatabaseToApplicationDirectory() - // Passcode checking, must happen on immediate launch if !DataController.shared.storeExists() { // Reset password authentication prior to `WindowProtection` diff --git a/Sources/Brave/Migration/Migration.swift b/Sources/Brave/Migration/Migration.swift index 17aa4da399b..7d9f36fc0f2 100644 --- a/Sources/Brave/Migration/Migration.swift +++ b/Sources/Brave/Migration/Migration.swift @@ -49,28 +49,6 @@ public class Migration { ) } - public func moveDatabaseToApplicationDirectory() { - if Preferences.Database.DocumentToSupportDirectoryMigration.completed.value { - // Migration has been done in some regard, so drop out. - return - } - - if Preferences.Database.DocumentToSupportDirectoryMigration.previousAttemptedVersion.value == AppInfo.appVersion { - // Migration has already been attempted for this version. - return - } - - // Moves Coredata sqlite file from documents dir to application support dir. - do { - try DataController.shared.migrateToNewPathIfNeeded() - } catch { - Logger.module.error("\(error.localizedDescription)") - } - - // Regardless of what happened, we attemtped a migration and document it: - Preferences.Database.DocumentToSupportDirectoryMigration.previousAttemptedVersion.value = AppInfo.appVersion - } - @objc private func enableUserSelectedTypesForSync() { guard braveCore.syncAPI.isInSyncGroup else { Logger.module.info("Sync is not active") diff --git a/Sources/Data/DataPreferences.swift b/Sources/Data/DataPreferences.swift index 226da2a4608..fdc486203b0 100644 --- a/Sources/Data/DataPreferences.swift +++ b/Sources/Data/DataPreferences.swift @@ -6,22 +6,6 @@ import Foundation import BraveShared public extension Preferences { - - final class Database { - - public final class DocumentToSupportDirectoryMigration { - /// This indicates whether the associated Document -> Support directory migration was / is considered - /// successful or not. Once it is `true`, it should never be set to `false` again. - public static let completed = Option(key: "database.document-to-support-directory-migration.completed", default: false) - - /// Since the migration may need to be re-attempted on each Brave version update this is used to store - /// the past version attempt, so it can be determined if another migration attempt is due. - public static let previousAttemptedVersion = Option(key: "database.document-to-support-directory-migration.previous-attempted-version", default: nil) - } - - public static let bookmark_v1_12_1RestorationCompleted = Option(key: "database.1_12_1-bookmark-restoration-completed", default: false) - } - final class BraveVPNAlertTotals { public static let consolidatedTrackerCount = Option(key: "vpn-alert-consolidated-tracker", default: 0) public static let consolidatedLocationPingCount = Option(key: "vpn-alert-consolidated-location", default: 0) diff --git a/Sources/Data/models/DataController.swift b/Sources/Data/models/DataController.swift index 020aef61f72..4d39e3e18c5 100644 --- a/Sources/Data/models/DataController.swift +++ b/Sources/Data/models/DataController.swift @@ -55,8 +55,7 @@ public class DataController { public func initializeOnce() { if initializationCompleted { return } - configureContainer(container, store: storeURL) - createOldDocumentStoreIfNeeded() + configureContainer(container, store: supportStoreURL) initializationCompleted = true } @@ -66,129 +65,13 @@ public class DataController { public static var sharedInMemory: DataController = InMemoryDataController() public func storeExists() -> Bool { - return FileManager.default.fileExists(atPath: storeURL.path) + return FileManager.default.fileExists(atPath: supportStoreURL.path) } private let container = NSPersistentContainer( name: DataController.modelName, managedObjectModel: DataController.model) - // MARK: - Old Database migration methods - - /// Returns old pre 1.12 persistent container or nil if it doesn't exist on the device. - public var oldDocumentStore: NSPersistentContainer? - private var migrationContainer: NSPersistentContainer? - - private func createOldDocumentStoreIfNeeded() { - let fm = FileManager.default - guard - let urls = fm.urls( - for: FileManager.SearchPathDirectory.documentDirectory, - in: .userDomainMask - ).last - else { return } - - let name = DataController.databaseName - let path = urls.appendingPathComponent(name).path - - if fm.fileExists(atPath: path) { - migrationContainer = NSPersistentContainer(name: DataController.modelName, managedObjectModel: DataController.model) - if let migrationContainer = migrationContainer { - configureContainer(migrationContainer, store: oldDocumentStoreURL) - } - } - } - - public var newStoreExists: Bool { - let fm = FileManager.default - guard - let urls = fm.urls( - for: FileManager.SearchPathDirectory.applicationSupportDirectory, - in: .userDomainMask - ).last - else { return false } - - let name = DataController.databaseName - let path = urls.appendingPathComponent(name).path - - return fm.fileExists(atPath: path) - } - - public func migrateToNewPathIfNeeded() throws { - enum MigrationError: Error { - case OldStoreMissing(String) - case MigrationFailed(String) - case CleanupFailed(String) - } - - // This logic must account for 4 different situations: - // 1. New Users (no migration, use new location - // 2. Upgraded users with successful migration (use new database) - // 3. Upgraded users with unsuccessful migrations (use new database, ignore old files) - // 4. Upgrading users (attempt migration, if fail, use old store, if successful delete old files) - // - re-attempt migration on every new app version, until they are in #2 - - if oldDocumentStore == nil || newStoreExists { - // Old store absent, no data to migrate (#1 | #3) - // or - // New store already exists, do not attempt to overwrite (#2) - - // Update flag to avoid re-running this logic - Preferences.Database.DocumentToSupportDirectoryMigration.completed.value = true - return - } - - // Going to attempt migration (#4 in some level) - guard let migrationContainer = migrationContainer else { - throw MigrationError.OldStoreMissing("Migration container missing") - } - - let coordinator = migrationContainer.persistentStoreCoordinator - - guard let oldStore = coordinator.persistentStore(for: oldDocumentStoreURL) else { - throw MigrationError.OldStoreMissing("Old store unavailable") - } - - // Attempting actual database migration Document -> Support ๐Ÿคž - do { - let migrationOptions = [ - NSPersistentStoreFileProtectionKey: true - ] - try coordinator.migratePersistentStore(oldStore, to: supportStoreURL, options: migrationOptions, withType: NSSQLiteStoreType) - } catch { - throw MigrationError.MigrationFailed("Document -> Support database migration failed: \(error.localizedDescription)") - // Migration failed somehow, and old store is present. Flag not being updated ๐Ÿ˜ญ - } - - // Regardless of cleanup logic, the actual migration was successful, so we're just going for it ๐Ÿ™€๐Ÿ˜Ž - Preferences.Database.DocumentToSupportDirectoryMigration.completed.value = true - - // Cleanup time ๐Ÿงน - do { - try coordinator.destroyPersistentStore(at: oldDocumentStoreURL, ofType: NSSQLiteStoreType, options: nil) - - let documentFiles = try FileManager.default.contentsOfDirectory( - at: oldDocumentStoreURL.deletingLastPathComponent(), - includingPropertiesForKeys: nil, - options: []) - - // Delete all Brave.X files - try documentFiles - .filter { $0.lastPathComponent.hasPrefix(DataController.databaseName) } - .forEach(FileManager.default.removeItem) - } catch { - throw MigrationError.CleanupFailed("Document -> Support database cleanup failed: \(error.localizedDescription)") - // Do not re-point store, as the migration was successful, just the clean up failed - } - - // At this point, everything was a pure success ๐Ÿ‘ - } - - /// Warning! Please use `storeURL`. This is for migration purpose only. - private var oldDocumentStoreURL: URL { - return storeURL(for: FileManager.SearchPathDirectory.documentDirectory) - } - /// Warning! Please use `storeURL`. This is for migration purposes only. private var supportStoreURL: URL { return storeURL(for: FileManager.SearchPathDirectory.applicationSupportDirectory) @@ -272,14 +155,6 @@ public class DataController { container.persistentStoreDescriptions = [storeDescription] } - var storeURL: URL { - let supportDirectory = Preferences.Database.DocumentToSupportDirectoryMigration.completed.value - if !supportDirectory { - assertionFailure("App wants to use very old document store, this can't be right.") - } - return supportDirectory ? supportStoreURL : oldDocumentStoreURL - } - private func configureContainer(_ container: NSPersistentContainer, store: URL) { addPersistentStore(for: container, store: store) diff --git a/Sources/DataTestsUtils/CoreDataTestCase.swift b/Sources/DataTestsUtils/CoreDataTestCase.swift index 33c2bd8dcc4..747ae48ee77 100644 --- a/Sources/DataTestsUtils/CoreDataTestCase.swift +++ b/Sources/DataTestsUtils/CoreDataTestCase.swift @@ -17,7 +17,6 @@ open class CoreDataTestCase: XCTestCase { name: NSNotification.Name.NSManagedObjectContextDidSave, object: nil) - Preferences.Database.DocumentToSupportDirectoryMigration.completed.value = true DataController.shared = InMemoryDataController() }