Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
For #11483 - Part 1: Extend DefaultTopSitesStorage to accept a top si…
Browse files Browse the repository at this point in the history
…tes provider
  • Loading branch information
gabrielluong authored and mergify[bot] committed Jan 17, 2022
1 parent 53273cc commit d754645
Show file tree
Hide file tree
Showing 4 changed files with 58 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
* additional 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
3 changes: 3 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ permalink: /changelog/
* **lib/publicsuffixlist**
* ⚠️ **This is a breaking change**: Removed `String.urlToTrimmedHost` extension method.

* **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)
* [Milestone](https://github.com/mozilla-mobile/android-components/milestone/144?closed=1)
Expand Down

0 comments on commit d754645

Please sign in to comment.