Skip to content

Commit

Permalink
Issue mozilla-mobile#11483: Extend DefaultTopSitesStorage to accept a…
Browse files Browse the repository at this point in the history
… top sites provider
  • Loading branch information
gabrielluong committed Jan 11, 2022
1 parent 16584c4 commit 2221220
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pair<String, String>> = listOf(),
coroutineContext: CoroutineContext = Dispatchers.IO
) : TopSitesStorage, Observable<TopSitesStorage.Observer> by ObserverRegistry() {
Expand Down Expand Up @@ -83,9 +86,16 @@ class DefaultTopSitesStorage(
): List<TopSite> {
val topSites = ArrayList<TopSite>()
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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<TopSite>
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 2221220

Please sign in to comment.