Skip to content

Commit

Permalink
Fix mozilla-mobile#6390 and mozilla-mobile#6376: restore bookmark mig…
Browse files Browse the repository at this point in the history
…ration code to support tests

We have tests using browser.db for their bookmarks, which need to be
migrated to places.db for them to run. This means we have a test case
for the migration code, which I didn't realize, and not having a test case
for the migration code was a rationale I used to remove it.
  • Loading branch information
garvankeeley committed Apr 8, 2020
1 parent 324fc94 commit 73e3216
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 4 deletions.
11 changes: 7 additions & 4 deletions Providers/Profile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,10 @@ open class BrowserProfile: Profile {
log.debug("Reopening profile.")
isShutdown = false

if !places.isOpen && !RustFirefoxAccounts.shared.accountManager.hasAccount() {
places.migrateBookmarksIfNeeded(fromBrowserDB: db)
}

db.reopenIfClosed()
_ = logins.reopenIfClosed()
_ = places.reopenIfClosed()
Expand Down Expand Up @@ -427,10 +431,9 @@ open class BrowserProfile: Profile {
return self.legacyPlaces
}

lazy var places: RustPlaces = {
let databasePath = URL(fileURLWithPath: (try! files.getAndEnsureDirectory()), isDirectory: true).appendingPathComponent("places.db").path
return RustPlaces(databasePath: databasePath)
}()
lazy var placesDbPath = URL(fileURLWithPath: (try! files.getAndEnsureDirectory()), isDirectory: true).appendingPathComponent("places.db").path

lazy var places = RustPlaces(databasePath: placesDbPath)

lazy var searchEngines: SearchEngines = {
return SearchEngines(prefs: self.prefs, files: self.files)
Expand Down
29 changes: 29 additions & 0 deletions Storage/Rust/RustPlaces.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,35 @@ public class RustPlaces {
return deferred
}

public func migrateBookmarksIfNeeded(fromBrowserDB browserDB: BrowserDB) {
// Since we use the existence of places.db as an indication that we've
// already migrated bookmarks, assert that places.db is not open here.
assert(!isOpen, "Shouldn't attempt to migrate bookmarks after opening Rust places.db")

// We only need to migrate bookmarks here if the old browser.db file
// already exists AND the new Rust places.db file does NOT exist yet.
// This is to ensure that we only ever run this migration ONCE. In
// addition, it is the caller's (Profile.swift) responsibility to NOT
// use this migration API for users signed into a Firefox Account.
// Those users will automatically get all their bookmarks on next Sync.
guard FileManager.default.fileExists(atPath: browserDB.databasePath),
!FileManager.default.fileExists(atPath: databasePath) else {
return
}

// Ensure that the old BrowserDB schema is up-to-date before migrating.
_ = browserDB.touch().value

// Open the Rust places.db now for the first time.
_ = reopenIfClosed()

do {
try api?.migrateBookmarksFromBrowserDb(path: browserDB.databasePath)
} catch let err as NSError {
Sentry.shared.sendWithStacktrace(message: "Error encountered while migrating bookmarks from BrowserDB", tag: SentryTag.rustPlaces, severity: .error, description: err.localizedDescription)
}
}

public func getBookmarksTree(rootGUID: GUID, recursive: Bool) -> Deferred<Maybe<BookmarkNode?>> {
return withReader { connection in
return try connection.getBookmarksTree(rootGUID: rootGUID, recursive: recursive)
Expand Down

0 comments on commit 73e3216

Please sign in to comment.