Skip to content

Commit

Permalink
No Bug: Remove old CoreData migration logic. (brave/brave-ios#7185)
Browse files Browse the repository at this point in the history
  • Loading branch information
iccub authored Apr 3, 2023
1 parent e8b1b23 commit dfd814f
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 169 deletions.
3 changes: 0 additions & 3 deletions App/iOS/Delegates/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
22 changes: 0 additions & 22 deletions Sources/Brave/Migration/Migration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
16 changes: 0 additions & 16 deletions Sources/Data/DataPreferences.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<Bool>(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<String?>(key: "database.document-to-support-directory-migration.previous-attempted-version", default: nil)
}

public static let bookmark_v1_12_1RestorationCompleted = Option<Bool>(key: "database.1_12_1-bookmark-restoration-completed", default: false)
}

final class BraveVPNAlertTotals {
public static let consolidatedTrackerCount = Option<Int>(key: "vpn-alert-consolidated-tracker", default: 0)
public static let consolidatedLocationPingCount = Option<Int>(key: "vpn-alert-consolidated-location", default: 0)
Expand Down
129 changes: 2 additions & 127 deletions Sources/Data/models/DataController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ public class DataController {
public func initializeOnce() {
if initializationCompleted { return }

configureContainer(container, store: storeURL)
createOldDocumentStoreIfNeeded()
configureContainer(container, store: supportStoreURL)
initializationCompleted = true
}

Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion Sources/DataTestsUtils/CoreDataTestCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ open class CoreDataTestCase: XCTestCase {
name: NSNotification.Name.NSManagedObjectContextDidSave,
object: nil)

Preferences.Database.DocumentToSupportDirectoryMigration.completed.value = true
DataController.shared = InMemoryDataController()
}

Expand Down

0 comments on commit dfd814f

Please sign in to comment.