From 70892aea03f48d9aa2ba11acc427df98329dab7e Mon Sep 17 00:00:00 2001 From: Sammy Khamis Date: Thu, 20 Jan 2022 18:36:54 -1000 Subject: [PATCH] Uniffi Places (#11487) * fixup the VisitObservation, HistoryVisitInfo APIs based on changes in a-s * Uniffi top frecent site info (#3) * uses VisitTransition in HistoryVisitInfo (#4) * fixup the VisitObservation, HistoryVisitInfo APIs based on changes in a-s * updated based on bookmarks being uniffied * fix issues with how we are rethrowing places exceptions * bump appservices version * fix ktlint lexiographic and ununsed import errors * updated fennec migration code Co-authored-by: Tarik Eshaq Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- buildSrc/src/main/java/Dependencies.kt | 2 +- .../storage/sync/PlacesBookmarksStorage.kt | 14 ++-- .../storage/sync/PlacesHistoryStorage.kt | 10 +-- .../browser/storage/sync/PlacesStorage.kt | 11 ++- .../components/browser/storage/sync/Types.kt | 82 ++++++++++++------- .../sync/PlacesBookmarksStorageTest.kt | 16 ++-- .../storage/sync/PlacesHistoryStorageTest.kt | 23 +++--- .../concept/storage/BookmarksStorage.kt | 10 +-- .../BookmarksStorageSuggestionProviderTest.kt | 6 +- .../components/service/nimbus/Nimbus.kt | 7 +- .../components/service/nimbus/NimbusTest.kt | 4 +- .../support/migration/FennecMigratorTest.kt | 10 +-- 12 files changed, 108 insertions(+), 87 deletions(-) diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 1c2030f311a..db4f8b7c0e4 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -29,7 +29,7 @@ object Versions { const val disklrucache = "2.0.2" const val leakcanary = "2.8.1" - const val mozilla_appservices = "87.2.0" + const val mozilla_appservices = "90.0.0" const val mozilla_glean = "43.0.2" diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt index 1a243048e4d..f33312e696d 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt @@ -7,9 +7,8 @@ package mozilla.components.browser.storage.sync import android.content.Context import androidx.annotation.VisibleForTesting import kotlinx.coroutines.withContext -import mozilla.appservices.places.BookmarkUpdateInfo import mozilla.appservices.places.PlacesApi -import mozilla.appservices.places.PlacesException +import mozilla.appservices.places.uniffi.PlacesException import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarksStorage @@ -95,8 +94,9 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo } else { 0 } - reader.getRecentBookmarks(limit).filter { it.dateAdded >= threshold } + reader.getRecentBookmarks(limit) .map { it.asBookmarkNode() } + .filter { it.dateAdded >= threshold } } } @@ -111,7 +111,7 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo * @param position The optional position to add the new node or null to append. * @return The guid of the newly inserted bookmark item. */ - override suspend fun addItem(parentGuid: String, url: String, title: String, position: Int?): String { + override suspend fun addItem(parentGuid: String, url: String, title: String, position: UInt?): String { return withContext(writeScope.coroutineContext) { writer.createBookmarkItem(parentGuid, url, title, position) } @@ -127,7 +127,7 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo * @param position The optional position to add the new node or null to append. * @return The guid of the newly inserted bookmark item. */ - override suspend fun addFolder(parentGuid: String, title: String, position: Int?): String { + override suspend fun addFolder(parentGuid: String, title: String, position: UInt?): String { return withContext(writeScope.coroutineContext) { writer.createFolder(parentGuid, title, position) } @@ -142,7 +142,7 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo * @param position The optional position to add the new node or null to append. * @return The guid of the newly inserted bookmark item. */ - override suspend fun addSeparator(parentGuid: String, position: Int?): String { + override suspend fun addSeparator(parentGuid: String, position: UInt?): String { return withContext(writeScope.coroutineContext) { writer.createSeparator(parentGuid, position) } @@ -158,7 +158,7 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo */ override suspend fun updateNode(guid: String, info: BookmarkInfo) { return withContext(writeScope.coroutineContext) { - writer.updateBookmark(guid, BookmarkUpdateInfo(info.parentGuid, info.position, info.title, info.url)) + writer.updateBookmark(guid, info.parentGuid, info.position, info.title, info.url) } } diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt index bd1e56cf011..54594f30178 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt @@ -8,8 +8,8 @@ import android.content.Context import android.net.Uri import kotlinx.coroutines.withContext import mozilla.appservices.places.PlacesApi -import mozilla.appservices.places.PlacesException -import mozilla.appservices.places.VisitObservation +import mozilla.appservices.places.uniffi.PlacesException +import mozilla.appservices.places.uniffi.VisitObservation import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.concept.storage.HistoryAutocompleteResult @@ -57,7 +57,7 @@ open class PlacesHistoryStorage( places.writer().noteObservation( VisitObservation( uri, - visitType = visit.visitType.into(), + visitType = visit.visitType.intoTransitionType(), isRedirectSource = visit.redirectSource != null, isPermanentRedirectSource = visit.redirectSource == RedirectSource.PERMANENT ) @@ -71,7 +71,7 @@ open class PlacesHistoryStorage( logger.debug("Not recording observation (canAddUri=false) for: $uri") return } - // NB: visitType 'UPDATE_PLACE' means "record meta information about this URL". + // NB: visitType being null means "record meta information about this URL". withContext(writeScope.coroutineContext) { // Ignore exceptions related to uris. This means we may drop some of the data on the floor // if the underlying storage layer refuses it. @@ -79,7 +79,7 @@ open class PlacesHistoryStorage( places.writer().noteObservation( VisitObservation( url = uri, - visitType = mozilla.appservices.places.VisitType.UPDATE_PLACE, + visitType = null, title = observation.title, previewImageUrl = observation.previewImageUrl ) diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt index 5a3f2408d8d..150c003e027 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt @@ -11,10 +11,9 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.withContext -import mozilla.appservices.places.InternalPanic -import mozilla.appservices.places.PlacesException import mozilla.appservices.places.PlacesReaderConnection import mozilla.appservices.places.PlacesWriterConnection +import mozilla.appservices.places.uniffi.PlacesException import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.concept.storage.Storage import mozilla.components.concept.sync.SyncStatus @@ -86,7 +85,7 @@ abstract class PlacesStorage( protected inline fun handlePlacesExceptions(operation: String, block: () -> Unit) { try { block() - } catch (e: InternalPanic) { + } catch (e: PlacesException.UnexpectedPlacesException) { throw e } catch (e: PlacesException) { crashReporter?.submitCaughtException(e) @@ -110,7 +109,7 @@ abstract class PlacesStorage( ): T { return try { block() - } catch (e: InternalPanic) { + } catch (e: PlacesException.UnexpectedPlacesException) { throw e } catch (e: PlacesException) { crashReporter?.submitCaughtException(e) @@ -130,8 +129,8 @@ abstract class PlacesStorage( logger.debug("Successfully synced.") SyncStatus.Ok - // Order of these catches matters: InternalPanic extends PlacesException. - } catch (e: InternalPanic) { + // Order of these catches matters: UnexpectedPlacesException extends PlacesException + } catch (e: PlacesException.UnexpectedPlacesException) { logger.error("Places panic while syncing", e) throw e } catch (e: PlacesException) { diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt index cb0f6492816..d940bf410f0 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt @@ -6,11 +6,8 @@ package mozilla.components.browser.storage.sync -import mozilla.appservices.places.BookmarkFolder -import mozilla.appservices.places.BookmarkItem -import mozilla.appservices.places.BookmarkSeparator -import mozilla.appservices.places.BookmarkTreeNode import mozilla.appservices.places.SyncAuthInfo +import mozilla.appservices.places.uniffi.BookmarkItem import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.DocumentType @@ -43,9 +40,9 @@ internal fun mozilla.components.concept.sync.SyncAuthInfo.into(): SyncAuthInfo { * Conversion from a generic [FrecencyThresholdOption] into its richer comrade within the 'places' lib. */ internal fun FrecencyThresholdOption.into() = when (this) { - FrecencyThresholdOption.NONE -> mozilla.appservices.places.FrecencyThresholdOption.NONE + FrecencyThresholdOption.NONE -> mozilla.appservices.places.uniffi.FrecencyThresholdOption.NONE FrecencyThresholdOption.SKIP_ONE_TIME_PAGES -> - mozilla.appservices.places.FrecencyThresholdOption.SKIP_ONE_TIME_PAGES + mozilla.appservices.places.uniffi.FrecencyThresholdOption.SKIP_ONE_TIME_PAGES } /** @@ -77,58 +74,85 @@ internal fun mozilla.appservices.places.VisitType.into() = when (this) { mozilla.appservices.places.VisitType.FRAMED_LINK -> VisitType.FRAMED_LINK } -internal fun mozilla.appservices.places.VisitInfo.into(): VisitInfo { +internal fun VisitType.intoTransitionType() = when (this) { + VisitType.NOT_A_VISIT -> null + VisitType.LINK -> mozilla.appservices.places.uniffi.VisitTransition.LINK + VisitType.RELOAD -> mozilla.appservices.places.uniffi.VisitTransition.RELOAD + VisitType.TYPED -> mozilla.appservices.places.uniffi.VisitTransition.TYPED + VisitType.BOOKMARK -> mozilla.appservices.places.uniffi.VisitTransition.BOOKMARK + VisitType.EMBED -> mozilla.appservices.places.uniffi.VisitTransition.EMBED + VisitType.REDIRECT_PERMANENT -> mozilla.appservices.places.uniffi.VisitTransition.REDIRECT_PERMANENT + VisitType.REDIRECT_TEMPORARY -> mozilla.appservices.places.uniffi.VisitTransition.REDIRECT_TEMPORARY + VisitType.DOWNLOAD -> mozilla.appservices.places.uniffi.VisitTransition.DOWNLOAD + VisitType.FRAMED_LINK -> mozilla.appservices.places.uniffi.VisitTransition.FRAMED_LINK +} + +internal fun mozilla.appservices.places.uniffi.VisitTransition.into() = when (this) { + mozilla.appservices.places.uniffi.VisitTransition.LINK -> VisitType.LINK + mozilla.appservices.places.uniffi.VisitTransition.RELOAD -> VisitType.RELOAD + mozilla.appservices.places.uniffi.VisitTransition.TYPED -> VisitType.TYPED + mozilla.appservices.places.uniffi.VisitTransition.BOOKMARK -> VisitType.BOOKMARK + mozilla.appservices.places.uniffi.VisitTransition.EMBED -> VisitType.EMBED + mozilla.appservices.places.uniffi.VisitTransition.REDIRECT_PERMANENT -> VisitType.REDIRECT_PERMANENT + mozilla.appservices.places.uniffi.VisitTransition.REDIRECT_TEMPORARY -> VisitType.REDIRECT_TEMPORARY + mozilla.appservices.places.uniffi.VisitTransition.DOWNLOAD -> VisitType.DOWNLOAD + mozilla.appservices.places.uniffi.VisitTransition.FRAMED_LINK -> VisitType.FRAMED_LINK +} + +private val intToVisitType: Map = VisitType.values().associateBy(VisitType::type) + +internal fun mozilla.appservices.places.uniffi.HistoryVisitInfo.into(): VisitInfo { return VisitInfo( url = this.url, title = this.title, - visitTime = this.visitTime, + visitTime = this.timestamp, visitType = this.visitType.into(), previewImageUrl = this.previewImageUrl ) } -internal fun mozilla.appservices.places.TopFrecentSiteInfo.into(): TopFrecentSiteInfo { +internal fun mozilla.appservices.places.uniffi.TopFrecentSiteInfo.into(): TopFrecentSiteInfo { return TopFrecentSiteInfo( url = this.url, title = this.title ) } -internal fun BookmarkTreeNode.asBookmarkNode(): BookmarkNode { +internal fun BookmarkItem.asBookmarkNode(): BookmarkNode { return when (this) { - is BookmarkItem -> { + is BookmarkItem.Bookmark -> { BookmarkNode( BookmarkNodeType.ITEM, - this.guid, - this.parentGUID, - this.position, - this.title, - this.url, - this.dateAdded, + this.b.guid, + this.b.parentGuid, + this.b.position, + this.b.title, + this.b.url, + this.b.dateAdded, null ) } - is BookmarkFolder -> { + is BookmarkItem.Folder -> { BookmarkNode( BookmarkNodeType.FOLDER, - this.guid, - this.parentGUID, - this.position, - this.title, + this.f.guid, + this.f.parentGuid, + this.f.position, + this.f.title, null, - this.dateAdded, - this.children?.map(BookmarkTreeNode::asBookmarkNode) + this.f.dateAdded, + this.f.childNodes?.map(BookmarkItem::asBookmarkNode) ) } - is BookmarkSeparator -> { + is BookmarkItem.Separator -> { BookmarkNode( BookmarkNodeType.SEPARATOR, - this.guid, - this.parentGUID, - this.position, + this.s.guid, + this.s.parentGuid, + this.s.position, null, null, - this.dateAdded, + this.s.dateAdded, null ) } diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt index 58b31ad826e..4e18bb4a535 100644 --- a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt @@ -7,7 +7,7 @@ package mozilla.components.browser.storage.sync import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.runBlocking import mozilla.appservices.places.BookmarkRoot -import mozilla.appservices.places.PlacesException +import mozilla.appservices.places.uniffi.PlacesException import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType @@ -86,7 +86,7 @@ class PlacesBookmarksStorageTest { assertEquals(emptyList(), bookmarks.getBookmarksWithUrl(url)) assertEquals(emptyList(), bookmarks.searchBookmarks("mozilla")) - val insertedItem = bookmarks.addItem(BookmarkRoot.Mobile.id, url, "Mozilla", 5) + val insertedItem = bookmarks.addItem(BookmarkRoot.Mobile.id, url, "Mozilla", 5u) with(bookmarks.getBookmarksWithUrl(url)) { assertEquals(1, this.size) @@ -96,7 +96,7 @@ class PlacesBookmarksStorageTest { 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(0u, this.position) assertEquals("http://www.mozilla.org/", this.url) } } @@ -105,7 +105,7 @@ class PlacesBookmarksStorageTest { bookmarks.updateNode( insertedItem, BookmarkInfo( - parentGuid = folderGuid, title = null, position = -3, url = null + parentGuid = folderGuid, title = null, position = 9999u, url = null ) ) with(bookmarks.getBookmarksWithUrl(url)) { @@ -115,12 +115,12 @@ class PlacesBookmarksStorageTest { assertEquals(BookmarkNodeType.ITEM, this.type) assertEquals("Mozilla", this.title) assertEquals(folderGuid, this.parentGuid) - assertEquals(0, this.position) + assertEquals(0u, this.position) assertEquals("http://www.mozilla.org/", this.url) } } - val separatorGuid = bookmarks.addSeparator(folderGuid, 1) + val separatorGuid = bookmarks.addSeparator(folderGuid, 1u) with(bookmarks.getTree(folderGuid)!!) { assertEquals(2, this.children!!.size) assertEquals(BookmarkNodeType.SEPARATOR, this.children!![1].type) @@ -155,7 +155,7 @@ class PlacesBookmarksStorageTest { assertTrue(this.isEmpty()) } - val secondInsertedItem = bookmarks.addItem(BookmarkRoot.Unfiled.id, url, "Mozilla", 6) + val secondInsertedItem = bookmarks.addItem(BookmarkRoot.Unfiled.id, url, "Mozilla", 6u) with(bookmarks.getRecentBookmarks(2)) { assertEquals(secondInsertedItem, this[0].guid) @@ -199,7 +199,7 @@ class PlacesBookmarksStorageTest { fail("Expected v0 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("Can not import from database version 0", e.message) + assertEquals("Unexpected error: Can not import from database version 0", e.message) } } diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt index bc06ebd7e26..4080f06e0bc 100644 --- a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt @@ -6,11 +6,10 @@ package mozilla.components.browser.storage.sync import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.runBlocking -import mozilla.appservices.places.InternalPanic -import mozilla.appservices.places.PlacesException import mozilla.appservices.places.PlacesReaderConnection import mozilla.appservices.places.PlacesWriterConnection -import mozilla.appservices.places.VisitObservation +import mozilla.appservices.places.uniffi.PlacesException +import mozilla.appservices.places.uniffi.VisitObservation import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.DocumentType import mozilla.components.concept.storage.FrecencyThresholdOption @@ -247,7 +246,7 @@ class PlacesHistoryStorageTest { assertEquals(VisitType.RELOAD, visits[1].visitType) assertEquals("http://www.firefox.com/", visits[2].url) - assertEquals("", visits[2].title) + assertEquals(null, visits[2].title) assertEquals(VisitType.LINK, visits[2].visitType) val visitsAll = history.getDetailedVisits(0) @@ -707,7 +706,8 @@ class PlacesHistoryStorageTest { @Test fun `storage passes through sync exceptions`() = runBlocking { - val exception = PlacesException("test error") + // Can be any PlacesException + val exception = PlacesException.UrlParseFailed("test error") val conn = object : Connection { override fun reader(): PlacesReaderConnection { fail() @@ -760,9 +760,9 @@ class PlacesHistoryStorageTest { assertEquals("test error", error.exception.message) } - @Test(expected = InternalPanic::class) + @Test(expected = PlacesException::class) fun `storage re-throws sync panics`() = runBlocking { - val exception = InternalPanic("test panic") + val exception = PlacesException.UnexpectedPlacesException("test panic") val conn = object : Connection { override fun reader(): PlacesReaderConnection { fail() @@ -819,7 +819,7 @@ class PlacesHistoryStorageTest { fail("Expected v0 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("Can not import from database version 0", e.message) + assertEquals("Unexpected error: Can not import from database version 0", e.message) } } @@ -832,7 +832,7 @@ class PlacesHistoryStorageTest { fail("Expected v23 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("Can not import from database version 23", e.message) + assertEquals("Unexpected error: Can not import from database version 23", e.message) } } @@ -1032,7 +1032,7 @@ class PlacesHistoryStorageTest { VisitObservation( url = "https://www.youtube.com/watch?v=F7PQdCDiE44", title = "DW next crisis", - visitType = mozilla.appservices.places.VisitType.LINK + visitType = mozilla.appservices.places.uniffi.VisitTransition.LINK ) ) @@ -1314,7 +1314,8 @@ class PlacesHistoryStorageTest { @Test fun `safe read from places`() = runBlocking { val result = history.handlePlacesExceptions("test", default = emptyList()) { - throw PlacesException("test") + // Can be any PlacesException error + throw PlacesException.PlacesConnectionBusy("test") } assertEquals(emptyList(), result) } diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/BookmarksStorage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/BookmarksStorage.kt index 5335da20013..f1570d38e78 100644 --- a/components/concept/storage/src/main/java/mozilla/components/concept/storage/BookmarksStorage.kt +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/BookmarksStorage.kt @@ -68,7 +68,7 @@ interface BookmarksStorage : Storage { * @param position The optional position to add the new node or null to append. * @return The guid of the newly inserted bookmark item. */ - suspend fun addItem(parentGuid: String, url: String, title: String, position: Int?): String + suspend fun addItem(parentGuid: String, url: String, title: String, position: UInt?): String /** * Adds a new bookmark folder to a given node. @@ -80,7 +80,7 @@ interface BookmarksStorage : Storage { * @param position The optional position to add the new node or null to append. * @return The guid of the newly inserted bookmark item. */ - suspend fun addFolder(parentGuid: String, title: String, position: Int? = null): String + suspend fun addFolder(parentGuid: String, title: String, position: UInt? = null): String /** * Adds a new bookmark separator to a given node. @@ -91,7 +91,7 @@ interface BookmarksStorage : Storage { * @param position The optional position to add the new node or null to append. * @return The guid of the newly inserted bookmark item. */ - suspend fun addSeparator(parentGuid: String, position: Int?): String + suspend fun addSeparator(parentGuid: String, position: UInt?): String /** * Edits the properties of an existing bookmark item and/or moves an existing one underneath a new parent guid. @@ -133,7 +133,7 @@ data class BookmarkNode( val type: BookmarkNodeType, val guid: String, val parentGuid: String?, - val position: Int?, + val position: UInt?, val title: String?, val url: String?, val dateAdded: Long, @@ -145,7 +145,7 @@ data class BookmarkNode( */ data class BookmarkInfo( val parentGuid: String?, - val position: Int?, + val position: UInt?, val title: String?, val url: String? ) diff --git a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt index ed359ef6239..67238a73951 100644 --- a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt +++ b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt @@ -166,7 +166,7 @@ class BookmarksStorageSuggestionProviderTest { parentGuid: String, url: String, title: String, - position: Int? + position: UInt? ): String { val id = UUID.randomUUID().toString() bookmarkMap[id] = @@ -174,12 +174,12 @@ class BookmarksStorageSuggestionProviderTest { return id } - override suspend fun addFolder(parentGuid: String, title: String, position: Int?): String { + override suspend fun addFolder(parentGuid: String, title: String, position: UInt?): String { // "Not needed for the test" throw NotImplementedError() } - override suspend fun addSeparator(parentGuid: String, position: Int?): String { + override suspend fun addSeparator(parentGuid: String, position: UInt?): String { // "Not needed for the test" throw NotImplementedError() } diff --git a/components/service/nimbus/src/main/java/mozilla/components/service/nimbus/Nimbus.kt b/components/service/nimbus/src/main/java/mozilla/components/service/nimbus/Nimbus.kt index b36fcd44297..e1217f49e07 100644 --- a/components/service/nimbus/src/main/java/mozilla/components/service/nimbus/Nimbus.kt +++ b/components/service/nimbus/src/main/java/mozilla/components/service/nimbus/Nimbus.kt @@ -98,9 +98,6 @@ class Nimbus( * experiment. */ class NimbusDisabled( + override val context: Context, private val delegate: Observable = ObserverRegistry() -) : NimbusApi, Observable by delegate { - companion object { - val instance: NimbusApi by lazy { NimbusDisabled() } - } -} +) : NimbusApi, Observable by delegate diff --git a/components/service/nimbus/src/test/java/mozilla/components/service/nimbus/NimbusTest.kt b/components/service/nimbus/src/test/java/mozilla/components/service/nimbus/NimbusTest.kt index 883e5b64335..182e0bb214e 100644 --- a/components/service/nimbus/src/test/java/mozilla/components/service/nimbus/NimbusTest.kt +++ b/components/service/nimbus/src/test/java/mozilla/components/service/nimbus/NimbusTest.kt @@ -27,7 +27,7 @@ class NimbusTest { @Test fun `Nimbus disabled and enabled can have observers registered on it`() { val enabled: NimbusApi = Nimbus(context, appInfo, null) - val disabled: NimbusApi = NimbusDisabled.instance + val disabled: NimbusApi = NimbusDisabled(context) val observer = object : NimbusInterface.Observer {} @@ -37,7 +37,7 @@ class NimbusTest { @Test fun `NimbusDisabled is empty`() { - val nimbus: NimbusApi = NimbusDisabled() + val nimbus: NimbusApi = NimbusDisabled(context) nimbus.fetchExperiments() nimbus.applyPendingExperiments() assertTrue("getActiveExperiments should be empty", nimbus.getActiveExperiments().isEmpty()) diff --git a/components/support/migration/src/test/java/mozilla/components/support/migration/FennecMigratorTest.kt b/components/support/migration/src/test/java/mozilla/components/support/migration/FennecMigratorTest.kt index 267a1dabeba..f8ba0fce898 100644 --- a/components/support/migration/src/test/java/mozilla/components/support/migration/FennecMigratorTest.kt +++ b/components/support/migration/src/test/java/mozilla/components/support/migration/FennecMigratorTest.kt @@ -8,7 +8,7 @@ import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.runBlocking import mozilla.appservices.places.BookmarkRoot -import mozilla.appservices.places.PlacesException +import mozilla.appservices.places.uniffi.PlacesException import mozilla.components.browser.state.action.BrowserAction import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.state.BrowserState @@ -365,7 +365,7 @@ class FennecMigratorTest { val historyStorage: PlacesHistoryStorage = mock() // Fail during history migration. - `when`(historyStorage.importFromFennec(any())).thenThrow(PlacesException("test exception")) + `when`(historyStorage.importFromFennec(any())).thenThrow(PlacesException.InvalidParent("test exception")) val migrator = FennecMigrator.Builder(testContext, mock()) .migrateHistory(lazy { historyStorage }) @@ -401,7 +401,7 @@ class FennecMigratorTest { val historyStorage: PlacesHistoryStorage = mock() // Fail during history migration. - `when`(historyStorage.importFromFennec(any())).thenThrow(PlacesException("test exception")) + `when`(historyStorage.importFromFennec(any())).thenThrow(PlacesException.InvalidParent("test exception")) `when`(bookmarkStorage.importFromFennec(any())).thenReturn( JSONObject().also { @@ -449,8 +449,8 @@ class FennecMigratorTest { val historyStorage: PlacesHistoryStorage = mock() // Both migrations failed. - `when`(historyStorage.importFromFennec(any())).thenThrow(PlacesException("test exception")) - `when`(bookmarkStorage.importFromFennec(any())).thenThrow(PlacesException("test exception")) + `when`(historyStorage.importFromFennec(any())).thenThrow(PlacesException.InvalidParent("test exception")) + `when`(bookmarkStorage.importFromFennec(any())).thenThrow(PlacesException.BookmarksCorruption("test exception")) val migrator = FennecMigrator.Builder(testContext, mock()) .migrateHistory(lazy { historyStorage })