Commit ef631b6d authored by MozLando's avatar MozLando
Browse files

Merge #4693

4693: Fennec data migration scaffolding r=pocmo a=grigoryk

This is going to be a rolling PR for a little bit, until this is ready to land.
Companion Fenix PR: https://github.com/mozilla-mobile/fenix/pull/5878

Workflow for testing it:
- fetch an ESR68 build from TC: https://tools.taskcluster.net/index/gecko.v2.mozilla-esr68.signed-nightly.nightly.latest.mobile
- re-sign it using a local debug keystore:
  - `zip -d target.apk "META-INF/*"`
  - `~/Code/mozilla-central/mobile/android/debug_sign_tool.py target.apk`
- enable auto-publication workflow for a-c in fenix
- assemble fennecProduction version of Fenix (built against this branch, see auto-publication)
  - `./gradlew assembleFennecProduction`
- sign it using keys used to sign fennec esr apk (otherwise we won't be able to overwrite one with another):
  - `~/Code/mozilla-central/mobile/android/debug_sign_tool.py app/build/outputs/apk/geckoBeta/fennecProduction/app-geckoBeta-x86-fennecProduction-unsigned.apk`
- install fennec esr apk build
- generate some profile data for migration (sign-into fxa, browse around, etc..)
- install fenix build: `adb install -r app/build/outputs/apk/geckoBeta/fennecProduction/app-geckoBeta-x86-fennecProduction-unsigned.apk`

TODO:
- currently needs https://github.com/mozilla/application-services/pull/1942

 to work
- add separate migration versioning for history and bookmarks; currently the whole thing is versioned
- add migration tests
  - in fennec, we have a bunch of "schema migration" tests that operate over older versions of browser.db
  - this is a good approach here as well - we can run migration code in tests against a few test db files, that exercise various history/bookmarks configurations and try multiple schema versions
- improve the workflow around this, it's quite tedious currently
Co-authored-by: default avatarGrisha Kruglov <gkruglov@mozilla.com>
parents 77afd92b df01bdf6
...@@ -27,7 +27,7 @@ object Versions { ...@@ -27,7 +27,7 @@ object Versions {
const val disklrucache = "2.0.2" const val disklrucache = "2.0.2"
const val leakcanary = "1.6.3" const val leakcanary = "1.6.3"
const val mozilla_appservices = "0.41.0" const val mozilla_appservices = "0.42.0"
const val material = "1.0.0" const val material = "1.0.0"
......
...@@ -39,6 +39,9 @@ internal interface Connection : Closeable { ...@@ -39,6 +39,9 @@ internal interface Connection : Closeable {
// strange split that doesn't quite map all that well to our internal storage model. // strange split that doesn't quite map all that well to our internal storage model.
fun syncHistory(syncInfo: SyncAuthInfo) fun syncHistory(syncInfo: SyncAuthInfo)
fun syncBookmarks(syncInfo: SyncAuthInfo) fun syncBookmarks(syncInfo: SyncAuthInfo)
fun importVisitsFromFennec(dbPath: String)
fun importBookmarksFromFennec(dbPath: String)
} }
/** /**
...@@ -91,6 +94,16 @@ internal object RustPlacesConnection : Connection { ...@@ -91,6 +94,16 @@ internal object RustPlacesConnection : Connection {
SyncTelemetry.processBookmarksPing(ping) SyncTelemetry.processBookmarksPing(ping)
} }
override fun importVisitsFromFennec(dbPath: String) {
check(api != null) { "must call init first" }
api!!.importVisitsFromFennec(dbPath)
}
override fun importBookmarksFromFennec(dbPath: String) {
check(api != null) { "must call init first" }
api!!.importBookmarksFromFennec(dbPath)
}
override fun close() = synchronized(this) { override fun close() = synchronized(this) {
check(api != null) { "must call init first" } check(api != null) { "must call init first" }
api!!.close() api!!.close()
......
...@@ -12,6 +12,7 @@ import mozilla.appservices.places.BookmarkSeparator ...@@ -12,6 +12,7 @@ import mozilla.appservices.places.BookmarkSeparator
import mozilla.appservices.places.BookmarkTreeNode import mozilla.appservices.places.BookmarkTreeNode
import mozilla.appservices.places.BookmarkUpdateInfo import mozilla.appservices.places.BookmarkUpdateInfo
import mozilla.appservices.places.PlacesApi import mozilla.appservices.places.PlacesApi
import mozilla.appservices.places.PlacesException
import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkInfo
import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarkNodeType
...@@ -185,6 +186,17 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo ...@@ -185,6 +186,17 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo
} }
} }
/**
* Import bookmarks data from Fennec's browser.db file.
* Before running this, first run [PlacesHistoryStorage.importFromFennec] to import history and visits data.
*
* @param dbPath Absolute path to Fennec's browser.db file.
*/
@Throws(PlacesException::class)
fun importFromFennec(dbPath: String) {
places.importBookmarksFromFennec(dbPath)
}
/** /**
* This should be removed. See: https://github.com/mozilla/application-services/issues/1877 * This should be removed. See: https://github.com/mozilla/application-services/issues/1877
* *
......
...@@ -7,6 +7,7 @@ package mozilla.components.browser.storage.sync ...@@ -7,6 +7,7 @@ package mozilla.components.browser.storage.sync
import android.content.Context import android.content.Context
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import mozilla.appservices.places.PlacesApi import mozilla.appservices.places.PlacesApi
import mozilla.appservices.places.PlacesException
import mozilla.appservices.places.VisitObservation import mozilla.appservices.places.VisitObservation
import mozilla.components.concept.storage.HistoryAutocompleteResult import mozilla.components.concept.storage.HistoryAutocompleteResult
import mozilla.components.concept.storage.HistoryStorage import mozilla.components.concept.storage.HistoryStorage
...@@ -191,6 +192,16 @@ open class PlacesHistoryStorage(context: Context) : PlacesStorage(context), Hist ...@@ -191,6 +192,16 @@ open class PlacesHistoryStorage(context: Context) : PlacesStorage(context), Hist
} }
} }
/**
* Import history and visits data from Fennec's browser.db file.
*
* @param dbPath Absolute path to Fennec's browser.db file.
*/
@Throws(PlacesException::class)
fun importFromFennec(dbPath: String) {
places.importVisitsFromFennec(dbPath)
}
/** /**
* This should be removed. See: https://github.com/mozilla/application-services/issues/1877 * This should be removed. See: https://github.com/mozilla/application-services/issues/1877
* *
......
...@@ -7,264 +7,238 @@ package mozilla.components.browser.storage.sync ...@@ -7,264 +7,238 @@ package mozilla.components.browser.storage.sync
import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.ext.junit.runners.AndroidJUnit4
import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runBlocking
import mozilla.appservices.places.BookmarkRoot import mozilla.appservices.places.BookmarkRoot
import mozilla.appservices.places.BookmarkUpdateInfo import mozilla.appservices.places.PlacesException
import mozilla.appservices.places.PlacesReaderConnection
import mozilla.appservices.places.PlacesWriterConnection
import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkInfo
import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.robolectric.testContext
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Assert.fail
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mockito.Mockito.`when`
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
@RunWith(AndroidJUnit4::class) @RunWith(AndroidJUnit4::class)
class PlacesBookmarksStorageTest { class PlacesBookmarksStorageTest {
private var conn: Connection? = null private lateinit var bookmarks: PlacesBookmarksStorage
private var reader: PlacesReaderConnection? = null
private var writer: PlacesWriterConnection? = null
private var storage: PlacesBookmarksStorage? = null
private val newItem = BookmarkNode(BookmarkNodeType.ITEM, "123", "456", null,
"Mozilla", "http://www.mozilla.org", null)
private val newFolder = BookmarkNode(BookmarkNodeType.FOLDER, "789", "321", null,
"Cool Sites", null, listOf())
private val newSeparator = BookmarkNode(BookmarkNodeType.SEPARATOR, "654", "987",
null, null, null, null)
internal class TestablePlacesBookmarksStorage(override val places: Connection) : PlacesBookmarksStorage(testContext)
@Before @Before
fun setup() { fun setup() = runBlocking {
conn = mock() bookmarks = PlacesBookmarksStorage(testContext)
reader = mock() // There's a database on disk which needs to be cleaned up between tests.
writer = mock() bookmarks.writer.deleteEverything()
`when`(conn!!.reader()).thenReturn(reader)
`when`(conn!!.writer()).thenReturn(writer)
storage = TestablePlacesBookmarksStorage(conn!!)
} }
@Test @After
fun `get bookmarks tree by root, recursive or not`() { fun cleanup() = runBlocking {
val reader = reader!! bookmarks.cleanup()
val storage = storage!!
runBlocking {
storage.getTree(BookmarkRoot.Root.id)
}
verify(reader, times(1)).getBookmarksTree(BookmarkRoot.Root.id, false)
runBlocking {
storage.getTree(BookmarkRoot.Root.id, true)
}
verify(reader, times(1)).getBookmarksTree(BookmarkRoot.Root.id, true)
} }
@Test @Test
fun `get bookmarks by URL`() { fun `get bookmarks tree by root, recursive or not`() = runBlocking {
val reader = reader!! val tree = bookmarks.getTree(BookmarkRoot.Root.id)!!
val storage = storage!! assertEquals(BookmarkRoot.Root.id, tree.guid)
assertNotNull(tree.children)
val url = "http://www.mozilla.org" assertEquals(4, tree.children!!.size)
runBlocking { var children = tree.children!!.map { it.guid }
storage.getBookmarksWithUrl(url) assertTrue(BookmarkRoot.Mobile.id in children)
assertTrue(BookmarkRoot.Unfiled.id in children)
assertTrue(BookmarkRoot.Toolbar.id in children)
assertTrue(BookmarkRoot.Menu.id in children)
// Non-recursive means children of children aren't fetched.
for (child in tree.children!!) {
assertNull(child.children)
assertEquals(BookmarkRoot.Root.id, child.parentGuid)
assertEquals(BookmarkNodeType.FOLDER, child.type)
}
val deepTree = bookmarks.getTree(BookmarkRoot.Root.id, true)!!
assertEquals(BookmarkRoot.Root.id, deepTree.guid)
assertNotNull(deepTree.children)
assertEquals(4, deepTree.children!!.size)
children = deepTree.children!!.map { it.guid }
assertTrue(BookmarkRoot.Mobile.id in children)
assertTrue(BookmarkRoot.Unfiled.id in children)
assertTrue(BookmarkRoot.Toolbar.id in children)
assertTrue(BookmarkRoot.Menu.id in children)
// Recursive means children of children are fetched.
for (child in deepTree.children!!) {
// For an empty tree, we expect to see empty lists.
assertEquals(emptyList<BookmarkNode>(), child.children)
assertEquals(BookmarkRoot.Root.id, child.parentGuid)
assertEquals(BookmarkNodeType.FOLDER, child.type)
} }
verify(reader, times(1)).getBookmarksWithURL(url)
} }
@Test @Test
fun `get bookmark by guid`() { fun `bookmarks APIs smoke testing - basic operations`() = runBlocking {
val reader = reader!! val url = "http://www.mozilla.org"
val storage = storage!!
val guid = "123"
runBlocking {
storage.getBookmark(guid)
}
verify(reader, times(1)).getBookmark(guid)
}
@Test
fun `search bookmarks by keyword`() {
val reader = reader!!
val storage = storage!!
runBlocking {
storage.searchBookmarks("mozilla")
}
verify(reader, times(1)).searchBookmarks("mozilla", 10)
runBlocking {
storage.searchBookmarks("mozilla", 30)
}
verify(reader, times(1)).searchBookmarks("mozilla", 30)
}
@Test
fun `add a bookmark item`() {
val writer = writer!!
val storage = storage!!
runBlocking {
storage.addItem(BookmarkRoot.Mobile.id, newItem.url!!, newItem.title!!, null)
}
verify(writer, times(1)).createBookmarkItem(
BookmarkRoot.Mobile.id, "http://www.mozilla.org", "Mozilla", null)
runBlocking {
storage.addItem(BookmarkRoot.Mobile.id, newItem.url!!, newItem.title!!, 3)
}
verify(writer, times(1)).createBookmarkItem(
BookmarkRoot.Mobile.id, "http://www.mozilla.org", "Mozilla", 3)
}
@Test assertEquals(emptyList<BookmarkNode>(), bookmarks.getBookmarksWithUrl(url))
fun `add a bookmark folder`() { assertEquals(emptyList<BookmarkNode>(), bookmarks.searchBookmarks("mozilla"))
val writer = writer!!
val storage = storage!!
runBlocking { val insertedItem = bookmarks.addItem(BookmarkRoot.Mobile.id, url, "Mozilla", 5)
storage.addFolder(BookmarkRoot.Mobile.id, newFolder.title!!, null)
}
verify(writer, times(1)).createFolder(
BookmarkRoot.Mobile.id, "Cool Sites", null)
runBlocking { with(bookmarks.getBookmarksWithUrl(url)) {
storage.addFolder(BookmarkRoot.Mobile.id, newFolder.title!!, 4) assertEquals(1, this.size)
with(this[0]) {
assertEquals(insertedItem, this.guid)
assertEquals(BookmarkNodeType.ITEM, this.type)
assertEquals("Mozilla", this.title)
assertEquals(BookmarkRoot.Mobile.id, this.parentGuid)
// Clamped to actual range. 'Mobile' was empty, so we get 0 back.
assertEquals(0, this.position)
assertEquals("http://www.mozilla.org/", this.url)
}
} }
verify(writer, times(1)).createFolder(
BookmarkRoot.Mobile.id, "Cool Sites", 4)
}
@Test
fun `add a bookmark separator`() {
val writer = writer!!
val storage = storage!!
runBlocking { val folderGuid = bookmarks.addFolder(BookmarkRoot.Mobile.id, "Test Folder", null)
storage.addSeparator(BookmarkRoot.Mobile.id, null) bookmarks.updateNode(insertedItem, BookmarkInfo(
parentGuid = folderGuid, title = null, position = -3, url = null
))
with(bookmarks.getBookmarksWithUrl(url)) {
assertEquals(1, this.size)
with(this[0]) {
assertEquals(insertedItem, this.guid)
assertEquals(BookmarkNodeType.ITEM, this.type)
assertEquals("Mozilla", this.title)
assertEquals(folderGuid, this.parentGuid)
assertEquals(0, this.position)
assertEquals("http://www.mozilla.org/", this.url)
}
} }
verify(writer, times(1)).createSeparator(
BookmarkRoot.Mobile.id, null)
runBlocking { val separatorGuid = bookmarks.addSeparator(folderGuid, 1)
storage.addSeparator(BookmarkRoot.Mobile.id, 4) with(bookmarks.getTree(folderGuid)!!) {
assertEquals(2, this.children!!.size)
assertEquals(BookmarkNodeType.SEPARATOR, this.children!![1].type)
} }
verify(writer, times(1)).createSeparator(
BookmarkRoot.Mobile.id, 4)
}
@Test
fun `move a bookmark item`() {
val writer = writer!!
val storage = storage!!
val info = BookmarkInfo(newItem.parentGuid, newItem.position, newItem.title, newItem.url)
runBlocking { assertTrue(bookmarks.deleteNode(separatorGuid))
storage.updateNode(BookmarkRoot.Mobile.id, info) with(bookmarks.getTree(folderGuid)!!) {
assertEquals(1, this.children!!.size)
assertEquals(BookmarkNodeType.ITEM, this.children!![0].type)
} }
verify(writer, times(1)).updateBookmark(
BookmarkRoot.Mobile.id, info.asBookmarkUpdateInfo())
runBlocking { with(bookmarks.searchBookmarks("mozilla")) {
storage.updateNode(BookmarkRoot.Mobile.id, info.copy(position = 4)) assertEquals(1, this.size)
assertEquals("http://www.mozilla.org/", this[0].url)
} }
verify(writer, times(1)).updateBookmark(
BookmarkRoot.Mobile.id, info.copy(position = 4).asBookmarkUpdateInfo())
}
@Test
fun `move a bookmark folder and its contents`() {
val writer = writer!!
val storage = storage!!
val info = BookmarkInfo(newFolder.parentGuid, newFolder.position, newFolder.title, newFolder.url)
runBlocking { with(bookmarks.getBookmark(folderGuid)!!) {
storage.updateNode(BookmarkRoot.Mobile.id, info) assertEquals(folderGuid, this.guid)
assertEquals("Test Folder", this.title)
assertEquals(BookmarkRoot.Mobile.id, this.parentGuid)
} }
verify(writer, times(1)).updateBookmark(
BookmarkRoot.Mobile.id, info.asBookmarkUpdateInfo()
)
runBlocking {
storage.updateNode(BookmarkRoot.Mobile.id, info.copy(position = 5))
}
verify(writer, times(1)).updateBookmark(
BookmarkRoot.Mobile.id, info.copy(position = 5).asBookmarkUpdateInfo()
)
}
@Test assertTrue(bookmarks.deleteNode(folderGuid))
fun `move a bookmark separator`() {
val writer = writer!!
val storage = storage!!
val info = BookmarkInfo(newSeparator.parentGuid, newSeparator.position, newSeparator.title, newSeparator.url)
runBlocking { for (root in listOf(
storage.updateNode(BookmarkRoot.Mobile.id, info) BookmarkRoot.Mobile, BookmarkRoot.Root, BookmarkRoot.Menu, BookmarkRoot.Toolbar, BookmarkRoot.Unfiled)
} ) {
verify(writer, times(1)).updateBookmark( try {
BookmarkRoot.Mobile.id, info.asBookmarkUpdateInfo() bookmarks.deleteNode(root.id)
) fail("Expected root deletion for ${root.id} to fail")
runBlocking { } catch (e: PlacesException) {}
storage.updateNode(BookmarkRoot.Mobile.id, info.copy(position = 6))
} }
verify(writer, times(1)).updateBookmark(BookmarkRoot.Mobile.id, info.copy(position = 6).asBookmarkUpdateInfo())
}
@Test
fun `update a bookmark item`() {
val writer = writer!!
val storage = storage!!
val info = BookmarkInfo("121", 1, "Firefox", "https://www.mozilla.org/en-US/firefox/") with(bookmarks.searchBookmarks("mozilla")) {
assertTrue(this.isEmpty())
runBlocking {
storage.updateNode(newItem.guid, info)
} }
verify(writer, times(1)).updateBookmark("123", info.asBookmarkUpdateInfo())
} }
@Test @Test
fun `update a bookmark folder`() { fun `bookmarks import v0 empty`() {
val writer = writer!! // Doesn't have a schema or a set user_version pragma.
val storage = storage!! val path = getTestPath("databases/empty-v0.db").absolutePath
try {
val info = BookmarkInfo("131", 2, "Firefox", null) bookmarks.importFromFennec(path)
fail("Expected v0 database to be unsupported")
runBlocking { } catch (e: PlacesException) {
storage.updateNode(newFolder.guid, info) // This is a little brittle, but the places library doesn't have a proper error type for this.
assertEquals("Database version 0 is not supported", e.message)
} }
verify(writer, times(1)).updateBookmark(newFolder.guid, info.asBookmarkUpdateInfo())
} }
@Test @Test
fun `delete a bookmark item`() { fun `bookmarks import v38 populated`() {
val writer = writer!! // Fennec v38 schema populated with data.
val storage = storage!! val path = getTestPath("databases/populated-v38.db").absolutePath
try {
runBlocking { bookmarks.importFromFennec(path)
storage.deleteNode(newItem.guid) fail("Expected v38 database to be unsupported")
} catch (e: PlacesException) {
// This is a little brittle, but the places library doesn't have a proper error type for this.
assertEquals("Database version 38 is not supported", e.message)
} }
verify(writer, times(1)).deleteBookmarkNode(newItem.guid)
} }
@Test @Test
fun `delete a bookmark separator`() { fun `bookmarks import v39 populated`() = runBlocking {
val writer = writer!! val path = getTestPath("databases/populated-v39.db").absolutePath
val storage = storage!!
// Need to import history first before we import bookmarks.
runBlocking { PlacesHistoryStorage(testContext).importFromFennec(path)