diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 64be74c3165..1f088a0982c 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -30,7 +30,7 @@ object Versions { const val disklrucache = "2.0.2" const val leakcanary = "2.4" - const val mozilla_appservices = "63.0.0" + const val mozilla_appservices = "66.0.0" const val mozilla_glean = "33.0.4" diff --git a/components/browser/storage-memory/src/main/java/mozilla/components/browser/storage/memory/InMemoryHistoryStorage.kt b/components/browser/storage-memory/src/main/java/mozilla/components/browser/storage/memory/InMemoryHistoryStorage.kt index e34fd88e6b1..40842276efb 100644 --- a/components/browser/storage-memory/src/main/java/mozilla/components/browser/storage/memory/InMemoryHistoryStorage.kt +++ b/components/browser/storage-memory/src/main/java/mozilla/components/browser/storage/memory/InMemoryHistoryStorage.kt @@ -5,12 +5,13 @@ package mozilla.components.browser.storage.memory import androidx.annotation.VisibleForTesting +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.concept.storage.HistoryAutocompleteResult import mozilla.components.concept.storage.HistoryStorage import mozilla.components.concept.storage.PageObservation -import mozilla.components.concept.storage.SearchResult import mozilla.components.concept.storage.PageVisit import mozilla.components.concept.storage.RedirectSource +import mozilla.components.concept.storage.SearchResult import mozilla.components.concept.storage.TopFrecentSiteInfo import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType @@ -28,6 +29,7 @@ const val AUTOCOMPLETE_SOURCE_NAME = "memoryHistory" class InMemoryHistoryStorage : HistoryStorage { @VisibleForTesting internal var pages: HashMap> = linkedMapOf() + @VisibleForTesting internal val pageMeta: HashMap = hashMapOf() @@ -50,9 +52,10 @@ class InMemoryHistoryStorage : HistoryStorage { } } - override suspend fun recordObservation(uri: String, observation: PageObservation) = synchronized(pageMeta) { - pageMeta[uri] = observation - } + override suspend fun recordObservation(uri: String, observation: PageObservation) = + synchronized(pageMeta) { + pageMeta[uri] = observation + } override suspend fun getVisited(uris: List): List = synchronized(pages) { return uris.map { @@ -67,7 +70,11 @@ class InMemoryHistoryStorage : HistoryStorage { return pages.keys.toList() } - override suspend fun getVisitsPaginated(offset: Long, count: Long, excludeTypes: List): List { + override suspend fun getVisitsPaginated( + offset: Long, + count: Long, + excludeTypes: List + ): List { throw UnsupportedOperationException("Pagination is not yet supported by the in-memory history storage") } @@ -80,13 +87,18 @@ class InMemoryHistoryStorage : HistoryStorage { pages.forEach { it.value.forEach { visit -> - if (visit.timestamp >= start && visit.timestamp <= end && !excludeTypes.contains(visit.type)) { - visits.add(VisitInfo( - url = it.key, - title = pageMeta[it.key]?.title, - visitTime = visit.timestamp, - visitType = visit.type - )) + if (visit.timestamp >= start && visit.timestamp <= end && !excludeTypes.contains( + visit.type + ) + ) { + visits.add( + VisitInfo( + url = it.key, + title = pageMeta[it.key]?.title, + visitTime = visit.timestamp, + visitType = visit.type + ) + ) } } } @@ -94,43 +106,59 @@ class InMemoryHistoryStorage : HistoryStorage { return visits } - override suspend fun getTopFrecentSites(numItems: Int): List { + override suspend fun getTopFrecentSites( + numItems: Int, + frecencyThreshold: FrecencyThresholdOption + ): List { throw UnsupportedOperationException("getTopFrecentSites is not yet supported by the in-memory history storage") } - override fun getSuggestions(query: String, limit: Int): List = synchronized(pages + pageMeta) { - data class Hit(val url: String, val score: Int) + override fun getSuggestions(query: String, limit: Int): List = + synchronized(pages + pageMeta) { + data class Hit(val url: String, val score: Int) - val urlMatches = pages.asSequence().map { - Hit(it.key, levenshteinDistance(it.key, query)) - } - val titleMatches = pageMeta.asSequence().map { - Hit(it.key, levenshteinDistance(it.value.title ?: "", query)) - } - val matchedUrls = mutableMapOf() - urlMatches.plus(titleMatches).forEach { - if (matchedUrls.containsKey(it.url) && matchedUrls[it.url]!! < it.score) { - matchedUrls[it.url] = it.score - } else { - matchedUrls[it.url] = it.score + val urlMatches = pages.asSequence().map { + Hit(it.key, levenshteinDistance(it.key, query)) + } + val titleMatches = pageMeta.asSequence().map { + Hit(it.key, levenshteinDistance(it.value.title ?: "", query)) + } + val matchedUrls = mutableMapOf() + urlMatches.plus(titleMatches).forEach { + if (matchedUrls.containsKey(it.url) && matchedUrls[it.url]!! < it.score) { + matchedUrls[it.url] = it.score + } else { + matchedUrls[it.url] = it.score + } } + // Calculate maxScore so that we can invert our scoring. + // Lower Levenshtein distance should produce a higher score. + val maxScore = + urlMatches.maxByOrNull { it.score }?.score ?: return@synchronized listOf() + + // TODO exclude non-matching results entirely? Score that implies complete mismatch. + matchedUrls.asSequence().sortedBy { it.value }.map { + SearchResult( + id = it.key, + score = maxScore - it.value, + url = it.key, + title = pageMeta[it.key]?.title + ) + }.take(limit).toList() } - // Calculate maxScore so that we can invert our scoring. - // Lower Levenshtein distance should produce a higher score. - val maxScore = urlMatches.maxByOrNull { it.score }?.score ?: return@synchronized listOf() - - // TODO exclude non-matching results entirely? Score that implies complete mismatch. - matchedUrls.asSequence().sortedBy { it.value }.map { - SearchResult(id = it.key, score = maxScore - it.value, url = it.key, title = pageMeta[it.key]?.title) - }.take(limit).toList() - } - override fun getAutocompleteSuggestion(query: String): HistoryAutocompleteResult? = synchronized(pages) { - return segmentAwareDomainMatch(query, pages.keys)?.let { urlMatch -> - HistoryAutocompleteResult( - query, urlMatch.matchedSegment, urlMatch.url, AUTOCOMPLETE_SOURCE_NAME, pages.size) + override fun getAutocompleteSuggestion(query: String): HistoryAutocompleteResult? = + synchronized(pages) { + return segmentAwareDomainMatch(query, pages.keys)?.let { urlMatch -> + HistoryAutocompleteResult( + query, + urlMatch.matchedSegment, + urlMatch.url, + AUTOCOMPLETE_SOURCE_NAME, + pages.size + ) + } } - } override suspend fun deleteEverything() = synchronized(pages + pageMeta) { pages.clear() 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 ba35d3054ed..0c0dba7cffa 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 @@ -9,19 +9,20 @@ import kotlinx.coroutines.withContext import mozilla.appservices.places.PlacesApi import mozilla.appservices.places.PlacesException import mozilla.appservices.places.VisitObservation +import mozilla.components.concept.base.crash.CrashReporting +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.concept.storage.HistoryAutocompleteResult import mozilla.components.concept.storage.HistoryStorage import mozilla.components.concept.storage.PageObservation import mozilla.components.concept.storage.PageVisit -import mozilla.components.concept.storage.SearchResult import mozilla.components.concept.storage.RedirectSource +import mozilla.components.concept.storage.SearchResult import mozilla.components.concept.storage.TopFrecentSiteInfo import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.components.concept.sync.SyncableStore -import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.utils.segmentAwareDomainMatch import org.json.JSONObject @@ -100,9 +101,13 @@ open class PlacesHistoryStorage( } } - override suspend fun getTopFrecentSites(numItems: Int): List { + override suspend fun getTopFrecentSites( + numItems: Int, + frecencyThreshold: FrecencyThresholdOption + ): List { return withContext(readScope.coroutineContext) { - places.reader().getTopFrecentSiteInfos(numItems).map { it.into() } + places.reader().getTopFrecentSiteInfos(numItems, frecencyThreshold.into()) + .map { it.into() } } } 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 921291308d8..8f0079883b1 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 @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ @file:Suppress("MatchingDeclarationName") + package mozilla.components.browser.storage.sync import mozilla.appservices.places.BookmarkFolder @@ -12,6 +13,7 @@ import mozilla.appservices.places.BookmarkTreeNode import mozilla.appservices.places.SyncAuthInfo import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.concept.storage.TopFrecentSiteInfo import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType @@ -31,6 +33,15 @@ 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.SKIP_ONE_TIME_PAGES -> + mozilla.appservices.places.FrecencyThresholdOption.SKIP_ONE_TIME_PAGES +} + /** * Conversion from a generic [VisitType] into its richer comrade within the 'places' lib. */ @@ -79,14 +90,37 @@ internal fun mozilla.appservices.places.TopFrecentSiteInfo.into(): TopFrecentSit internal fun BookmarkTreeNode.asBookmarkNode(): BookmarkNode { return when (this) { is BookmarkItem -> { - BookmarkNode(BookmarkNodeType.ITEM, this.guid, this.parentGUID, this.position, this.title, this.url, null) + BookmarkNode( + BookmarkNodeType.ITEM, + this.guid, + this.parentGUID, + this.position, + this.title, + this.url, + null + ) } is BookmarkFolder -> { - BookmarkNode(BookmarkNodeType.FOLDER, this.guid, this.parentGUID, this.position, this.title, null, - this.children?.map(BookmarkTreeNode::asBookmarkNode)) + BookmarkNode( + BookmarkNodeType.FOLDER, + this.guid, + this.parentGUID, + this.position, + this.title, + null, + this.children?.map(BookmarkTreeNode::asBookmarkNode) + ) } is BookmarkSeparator -> { - BookmarkNode(BookmarkNodeType.SEPARATOR, this.guid, this.parentGUID, this.position, null, null, null) + BookmarkNode( + BookmarkNodeType.SEPARATOR, + this.guid, + this.parentGUID, + this.position, + null, + null, + null + ) } } } 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 aad049e845c..e42627e664d 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 @@ -11,6 +11,7 @@ import mozilla.appservices.places.PlacesException import mozilla.appservices.places.PlacesReaderConnection import mozilla.appservices.places.PlacesWriterConnection import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.concept.storage.PageObservation import mozilla.components.concept.storage.PageVisit import mozilla.components.concept.storage.RedirectSource @@ -156,15 +157,24 @@ class PlacesHistoryStorageTest { history.recordVisit(url, PageVisit(VisitType.LINK, RedirectSource.NOT_A_SOURCE)) } - var infos = history.getTopFrecentSites(0) + var infos = history.getTopFrecentSites(0, frecencyThreshold = FrecencyThresholdOption.NONE) assertEquals(0, infos.size) - infos = history.getTopFrecentSites(3) + infos = history.getTopFrecentSites(0, frecencyThreshold = FrecencyThresholdOption.SKIP_ONE_TIME_PAGES) + assertEquals(0, infos.size) + + infos = history.getTopFrecentSites(3, frecencyThreshold = FrecencyThresholdOption.NONE) assertEquals(3, infos.size) assertEquals("https://www.mozilla.com/foo/bar/baz", infos[0].url) assertEquals("https://www.example.com/123", infos[1].url) + assertEquals("https://news.ycombinator.com/", infos[2].url) + + infos = history.getTopFrecentSites(3, frecencyThreshold = FrecencyThresholdOption.SKIP_ONE_TIME_PAGES) + assertEquals(2, infos.size) + assertEquals("https://www.mozilla.com/foo/bar/baz", infos[0].url) + assertEquals("https://www.example.com/123", infos[1].url) - infos = history.getTopFrecentSites(5) + infos = history.getTopFrecentSites(5, frecencyThreshold = FrecencyThresholdOption.NONE) assertEquals(5, infos.size) assertEquals("https://www.mozilla.com/foo/bar/baz", infos[0].url) assertEquals("https://www.example.com/123", infos[1].url) @@ -172,13 +182,23 @@ class PlacesHistoryStorageTest { assertEquals("https://mozilla.com/a1/b2/c3", infos[3].url) assertEquals("https://www.example.com/12345", infos[4].url) - infos = history.getTopFrecentSites(100) + infos = history.getTopFrecentSites(5, frecencyThreshold = FrecencyThresholdOption.SKIP_ONE_TIME_PAGES) + assertEquals(2, infos.size) + assertEquals("https://www.mozilla.com/foo/bar/baz", infos[0].url) + assertEquals("https://www.example.com/123", infos[1].url) + + infos = history.getTopFrecentSites(100, frecencyThreshold = FrecencyThresholdOption.NONE) assertEquals(5, infos.size) assertEquals("https://www.mozilla.com/foo/bar/baz", infos[0].url) assertEquals("https://www.example.com/123", infos[1].url) assertEquals("https://news.ycombinator.com/", infos[2].url) assertEquals("https://mozilla.com/a1/b2/c3", infos[3].url) assertEquals("https://www.example.com/12345", infos[4].url) + + infos = history.getTopFrecentSites(100, frecencyThreshold = FrecencyThresholdOption.SKIP_ONE_TIME_PAGES) + assertEquals(2, infos.size) + assertEquals("https://www.mozilla.com/foo/bar/baz", infos[0].url) + assertEquals("https://www.example.com/123", infos[1].url) } @Test diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryStorage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryStorage.kt index ba8240f9f4b..37d95df54bf 100644 --- a/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryStorage.kt +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryStorage.kt @@ -69,13 +69,18 @@ interface HistoryStorage : Storage { ): List /** - * Returns a list of the top frecent site infos limited by the given number of items - * sorted by most to least frecent. + * Returns a list of the top frecent site infos limited by the given number of items and + * frecency threshold sorted by most to least frecent. * * @param numItems the number of top frecent sites to return in the list. + * @param frecencyThreshold frecency threshold option for filtering visited sites based on + * their frecency score. * @return a list of the [TopFrecentSiteInfo], most frecent first. */ - suspend fun getTopFrecentSites(numItems: Int): List + suspend fun getTopFrecentSites( + numItems: Int, + frecencyThreshold: FrecencyThresholdOption + ): List /** * Retrieves suggestions matching the [query]. @@ -146,8 +151,10 @@ data class PageVisit( enum class RedirectSource { // The page didn't redirect to another page. NOT_A_SOURCE, + // The page temporarily redirected to another page. TEMPORARY, + // The page permanently redirected to another page. PERMANENT, } @@ -165,6 +172,17 @@ data class TopFrecentSiteInfo( val title: String? ) +/** + * Frecency threshold options for fetching top frecent sites. + */ +enum class FrecencyThresholdOption { + /** Returns all visited pages. */ + NONE, + + /** Skip visited pages that were only visited once. */ + SKIP_ONE_TIME_PAGES +} + /** * Information about a history visit. * @@ -187,8 +205,10 @@ data class VisitInfo( enum class VisitType(val type: Int) { // Internal visit type used for meta data updates. Doesn't represent an actual page visit. NOT_A_VISIT(-1), + // User followed a link. LINK(1), + // User typed a URL or selected it from the UI (autocomplete results, etc). TYPED(2), BOOKMARK(3), diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt index 3d841faf47e..0ab569ab2c8 100644 --- a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt @@ -8,6 +8,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.browser.storage.sync.PlacesHistoryStorage +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.feature.top.sites.TopSite.Type.FRECENT import mozilla.components.feature.top.sites.ext.hasUrl import mozilla.components.feature.top.sites.ext.toTopSite @@ -77,18 +78,18 @@ class DefaultTopSitesStorage( override suspend fun getTopSites( totalSites: Int, - includeFrecent: Boolean + frecencyConfig: FrecencyThresholdOption? ): List { val topSites = ArrayList() val pinnedSites = pinnedSitesStorage.getPinnedSites().take(totalSites) val numSitesRequired = totalSites - pinnedSites.size topSites.addAll(pinnedSites) - if (includeFrecent && numSitesRequired > 0) { + if (frecencyConfig != null && numSitesRequired > 0) { // Get 'totalSites' sites for duplicate entries with // existing pinned sites val frecentSites = historyStorage - .getTopFrecentSites(totalSites) + .getTopFrecentSites(totalSites, frecencyConfig) .map { it.toTopSite() } .filter { !pinnedSites.hasUrl(it.url) } .take(numSitesRequired) diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesConfig.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesConfig.kt index 85bdca62eea..13af6d7949a 100644 --- a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesConfig.kt +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesConfig.kt @@ -4,14 +4,18 @@ package mozilla.components.feature.top.sites +import mozilla.components.concept.storage.FrecencyThresholdOption + /** * Top sites configuration to specify the number of top sites to display and * whether or not to include top frecent sites in the top sites feature. * * @property totalSites A total number of sites that will be displayed. - * @property includeFrecent If true, includes frecent top site results. + * @property frecencyConfig If [frecencyConfig] is specified, only visited sites with a frecency + * score above the given threshold will be returned. Otherwise, frecent top site results are + * not included. */ data class TopSitesConfig( val totalSites: Int, - val includeFrecent: Boolean + val frecencyConfig: FrecencyThresholdOption? ) diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt index 538dbbc92a8..ea6dd151ea8 100644 --- a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.top.sites +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.support.base.observer.Observable /** @@ -37,12 +38,17 @@ interface TopSitesStorage : Observable { /** * Return a unified list of top sites based on the given number of sites desired. - * If `includeFrecent` is true, fill in any missing top sites with frecent top site results. + * If `frecencyConfig` is specified, fill in any missing top sites with frecent top site results. * * @param totalSites A total number of sites that will be retrieve if possible. - * @param includeFrecent If true, includes frecent top site results. + * @param frecencyConfig If [frecencyConfig] is specified, only visited sites with a frecency + * score above the given threshold will be returned. Otherwise, frecent top site results are + * not included. */ - suspend fun getTopSites(totalSites: Int, includeFrecent: Boolean): List + suspend fun getTopSites( + totalSites: Int, + frecencyConfig: FrecencyThresholdOption? + ): List /** * Interface to be implemented by classes that want to observe the top site storage. diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt index 92ae4f71286..0e94184b2bb 100644 --- a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt @@ -44,7 +44,10 @@ internal class DefaultTopSitesPresenter( val innerConfig = config.invoke() scope.launch { - val topSites = storage.getTopSites(innerConfig.totalSites, innerConfig.includeFrecent) + val topSites = storage.getTopSites( + innerConfig.totalSites, + innerConfig.frecencyConfig + ) scope.launch(Dispatchers.Main) { view.displayTopSites(topSites) diff --git a/components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt b/components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt index f0df75f229e..75bb3729d16 100644 --- a/components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt +++ b/components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt @@ -8,8 +8,10 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest import mozilla.components.browser.storage.sync.PlacesHistoryStorage +import mozilla.components.concept.storage.FrecencyThresholdOption import mozilla.components.concept.storage.TopFrecentSiteInfo import mozilla.components.feature.top.sites.ext.toTopSite +import mozilla.components.support.test.any import mozilla.components.support.test.mock import mozilla.components.support.test.whenever import org.junit.Assert.assertEquals @@ -106,7 +108,7 @@ class DefaultTopSitesStorageTest { } @Test - fun `getTopSites returns only default and pinned sites when includeFrecent is false`() = runBlockingTest { + fun `getTopSites returns only default and pinned sites when frecencyConfig is null`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( pinnedSitesStorage, historyStorage, @@ -135,22 +137,22 @@ class DefaultTopSitesStorageTest { ) ) - var topSites = defaultTopSitesStorage.getTopSites(0, false) + var topSites = defaultTopSitesStorage.getTopSites(0, frecencyConfig = null) assertTrue(topSites.isEmpty()) assertEquals(defaultTopSitesStorage.cachedTopSites, topSites) - topSites = defaultTopSitesStorage.getTopSites(1, false) + topSites = defaultTopSitesStorage.getTopSites(1, frecencyConfig = null) assertEquals(1, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(defaultTopSitesStorage.cachedTopSites, topSites) - topSites = defaultTopSitesStorage.getTopSites(2, false) + topSites = defaultTopSitesStorage.getTopSites(2, frecencyConfig = null) assertEquals(2, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(pinnedSite, topSites[1]) assertEquals(defaultTopSitesStorage.cachedTopSites, topSites) - topSites = defaultTopSitesStorage.getTopSites(5, false) + topSites = defaultTopSitesStorage.getTopSites(5, frecencyConfig = null) assertEquals(2, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(pinnedSite, topSites[1]) @@ -158,7 +160,7 @@ class DefaultTopSitesStorageTest { } @Test - fun `getTopSites returns pinned and frecent sites when includeFrecent is true`() = runBlockingTest { + fun `getTopSites returns pinned and frecent sites when frecencyConfig is specified`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( pinnedSitesStorage, historyStorage, @@ -188,23 +190,23 @@ class DefaultTopSitesStorageTest { ) val frecentSite1 = TopFrecentSiteInfo("https://mozilla.com", "Mozilla") - whenever(historyStorage.getTopFrecentSites(anyInt())).thenReturn(listOf(frecentSite1)) + whenever(historyStorage.getTopFrecentSites(anyInt(), any())).thenReturn(listOf(frecentSite1)) - var topSites = defaultTopSitesStorage.getTopSites(0, true) + var topSites = defaultTopSitesStorage.getTopSites(0, frecencyConfig = FrecencyThresholdOption.NONE) assertTrue(topSites.isEmpty()) - topSites = defaultTopSitesStorage.getTopSites(1, true) + topSites = defaultTopSitesStorage.getTopSites(1, frecencyConfig = FrecencyThresholdOption.NONE) assertEquals(1, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(defaultTopSitesStorage.cachedTopSites, topSites) - topSites = defaultTopSitesStorage.getTopSites(2, true) + topSites = defaultTopSitesStorage.getTopSites(2, frecencyConfig = FrecencyThresholdOption.NONE) assertEquals(2, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(pinnedSite, topSites[1]) assertEquals(defaultTopSitesStorage.cachedTopSites, topSites) - topSites = defaultTopSitesStorage.getTopSites(5, true) + topSites = defaultTopSitesStorage.getTopSites(5, frecencyConfig = FrecencyThresholdOption.NONE) assertEquals(3, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(pinnedSite, topSites[1]) @@ -213,7 +215,7 @@ class DefaultTopSitesStorageTest { val frecentSite2 = TopFrecentSiteInfo("https://example.com", "Example") val frecentSite3 = TopFrecentSiteInfo("https://getpocket.com", "Pocket") - whenever(historyStorage.getTopFrecentSites(anyInt())).thenReturn( + whenever(historyStorage.getTopFrecentSites(anyInt(), any())).thenReturn( listOf( frecentSite1, frecentSite2, @@ -221,7 +223,7 @@ class DefaultTopSitesStorageTest { ) ) - topSites = defaultTopSitesStorage.getTopSites(5, true) + topSites = defaultTopSitesStorage.getTopSites(5, frecencyConfig = FrecencyThresholdOption.NONE) assertEquals(5, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(pinnedSite, topSites[1]) @@ -231,7 +233,7 @@ class DefaultTopSitesStorageTest { assertEquals(defaultTopSitesStorage.cachedTopSites, topSites) val frecentSite4 = TopFrecentSiteInfo("https://example2.com", "Example2") - whenever(historyStorage.getTopFrecentSites(anyInt())).thenReturn( + whenever(historyStorage.getTopFrecentSites(anyInt(), any())).thenReturn( listOf( frecentSite1, frecentSite2, @@ -240,7 +242,7 @@ class DefaultTopSitesStorageTest { ) ) - topSites = defaultTopSitesStorage.getTopSites(5, true) + topSites = defaultTopSitesStorage.getTopSites(5, frecencyConfig = FrecencyThresholdOption.NONE) assertEquals(5, topSites.size) assertEquals(defaultSite, topSites[0]) assertEquals(pinnedSite, topSites[1]) @@ -292,7 +294,7 @@ class DefaultTopSitesStorageTest { val frecentSiteFirefox = TopFrecentSiteInfo("https://firefox.com", "Firefox") val frecentSite1 = TopFrecentSiteInfo("https://getpocket.com", "Pocket") val frecentSite2 = TopFrecentSiteInfo("https://www.example.com", "Example") - whenever(historyStorage.getTopFrecentSites(anyInt())).thenReturn( + whenever(historyStorage.getTopFrecentSites(anyInt(), any())).thenReturn( listOf( frecentSiteWithNoTitle, frecentSiteFirefox, @@ -301,7 +303,7 @@ class DefaultTopSitesStorageTest { ) ) - val topSites = defaultTopSitesStorage.getTopSites(5, true) + val topSites = defaultTopSitesStorage.getTopSites(5, frecencyConfig = FrecencyThresholdOption.NONE) assertEquals(5, topSites.size) assertEquals(defaultSiteFirefox, topSites[0]) assertEquals(pinnedSite1, topSites[1]) diff --git a/docs/changelog.md b/docs/changelog.md index a0ce9294e99..f607d8e6027 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -31,6 +31,9 @@ permalink: /changelog/ * **feature-accounts-push** * Made `FxaPushSupportFeature` aware of the `PushConfig.disableRateLimit` flag. +* **feature-top-sites** + * ⚠️ **This is a breaking change**: Replaces `includeFrecent` with an optional `frecencyConfig` in `TopSitesConfig` and `TopSitesStorage.getTopSites` to specify the frecency threshold for the returned list of top frecent sites see [#8690](https://github.com/mozilla-mobile/android-components/issues/8690). + # 64.0.0 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v63.0.0...v64.0.0)