From 2be69a65f91316a0772cb29c833663f3ea2478c6 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Tue, 7 Dec 2021 18:21:31 -0500 Subject: [PATCH] For #11483 - Part 1: Extend DefaultTopSitesStorage to accept a top sites provider --- .../top/sites/DefaultTopSitesStorage.kt | 12 +++- .../feature/top/sites/TopSitesProvider.kt | 16 ++++++ .../top/sites/DefaultTopSitesStorageTest.kt | 56 +++++++++---------- docs/changelog.md | 2 + 4 files changed, 57 insertions(+), 29 deletions(-) create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesProvider.kt 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 41e8168df15..a86e9c6dd93 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 @@ -23,12 +23,15 @@ import kotlin.coroutines.CoroutineContext * @param pinnedSitesStorage An instance of [PinnedSiteStorage], used for storing pinned sites. * @param historyStorage An instance of [PlacesHistoryStorage], used for retrieving top frecent * sites from history. + * @param topSitesProvider An optional instance of [TopSitesProvider], used for retrieving top + * sites from a provider. * @param defaultTopSites A list containing a title to url pair of default top sites to be added * to the [PinnedSiteStorage]. */ class DefaultTopSitesStorage( private val pinnedSitesStorage: PinnedSiteStorage, private val historyStorage: PlacesHistoryStorage, + private val topSitesProvider: TopSitesProvider? = null, private val defaultTopSites: List> = listOf(), coroutineContext: CoroutineContext = Dispatchers.IO ) : TopSitesStorage, Observable by ObserverRegistry() { @@ -83,9 +86,16 @@ class DefaultTopSitesStorage( ): List { val topSites = ArrayList() val pinnedSites = pinnedSitesStorage.getPinnedSites().take(totalSites) - val numSitesRequired = totalSites - pinnedSites.size + var numSitesRequired = totalSites - pinnedSites.size + topSites.addAll(pinnedSites) + topSitesProvider?.let { provider -> + val providerTopSites = provider.getTopSites() + topSites.addAll(providerTopSites.take(numSitesRequired)) + numSitesRequired -= providerTopSites.size + } + if (frecencyConfig != null && numSitesRequired > 0) { // Get 'totalSites' sites for duplicate entries with // existing pinned sites diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesProvider.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesProvider.kt new file mode 100644 index 00000000000..751f2b62883 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesProvider.kt @@ -0,0 +1,16 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites + +/** + * A contract that indicates how a top sites provider must behave. + */ +interface TopSitesProvider { + + /** + * Provides a list of top sites. + */ + suspend fun getTopSites(): List +} 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 f6ddd6182f0..9383e2d9231 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 @@ -37,10 +37,10 @@ class DefaultTopSitesStorageTest { ) DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - defaultTopSites, - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = defaultTopSites, + coroutineContext = coroutineContext ) verify(pinnedSitesStorage).addAllPinnedSites(defaultTopSites, isDefault = true) @@ -49,10 +49,10 @@ class DefaultTopSitesStorageTest { @Test fun `addPinnedSite`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - listOf(), - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext ) defaultTopSitesStorage.addTopSite("Mozilla", "https://mozilla.com", isDefault = false) @@ -66,10 +66,10 @@ class DefaultTopSitesStorageTest { @Test fun `removeTopSite`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - listOf(), - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext ) val frecentSite = TopSite( @@ -111,10 +111,10 @@ class DefaultTopSitesStorageTest { @Test fun `updateTopSite`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - listOf(), - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext ) val defaultSite = TopSite( @@ -154,10 +154,10 @@ class DefaultTopSitesStorageTest { @Test fun `getTopSites returns only default and pinned sites when frecencyConfig is null`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - listOf(), - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext ) val defaultSite = TopSite( @@ -207,10 +207,10 @@ class DefaultTopSitesStorageTest { @Test fun `getTopSites returns pinned and frecent sites when frecencyConfig is specified`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - listOf(), - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext ) val defaultSite = TopSite( @@ -301,10 +301,10 @@ class DefaultTopSitesStorageTest { @Test fun `getTopSites filters out frecent sites that already exist in pinned sites`() = runBlockingTest { val defaultTopSitesStorage = DefaultTopSitesStorage( - pinnedSitesStorage, - historyStorage, - listOf(), - coroutineContext + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext ) val defaultSiteFirefox = TopSite( diff --git a/docs/changelog.md b/docs/changelog.md index 0ad00cacacf..ae6a317bdd7 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,8 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml) +* **feature-top-sites** + * Extend `DefaultTopSitesStorage` to accept a `TopSitesProvider` for fetching top sites. [#11483](https://github.com/mozilla-mobile/android-components/issues/11483) # 97.0.0 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v96.0.0...v97.0.0)