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

Commit

Permalink
For #12384 - Add optional filter for top frecent sites.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mergify
Alexandru2909 authored and mergify[bot] committed Jun 29, 2022
1 parent 19fa535 commit 6e52770
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 29 deletions.
13 changes: 8 additions & 5 deletions ...re/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ 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.ext.hasUrl
import mozilla.components.feature.top.sites.ext.toTopSite
import mozilla.components.feature.top.sites.facts.emitTopSitesCountFact
Expand Down Expand Up @@ -86,7 +85,7 @@ class DefaultTopSitesStorage(
@Suppress("ComplexCondition", "TooGenericExceptionCaught")
override suspend fun getTopSites(
totalSites: Int,
frecencyConfig: FrecencyThresholdOption?,
frecencyConfig: TopSitesFrecencyConfig?,
providerConfig: TopSitesProviderConfig?
): List<TopSite> {
val topSites = ArrayList<TopSite>()
Expand Down Expand Up @@ -114,13 +113,17 @@ class DefaultTopSitesStorage(

topSites.addAll(pinnedSites)

if (frecencyConfig != null && numSitesRequired > 0) {
if (frecencyConfig?.frecencyTresholdOption != null && numSitesRequired > 0) {
// Get 'totalSites' sites for duplicate entries with
// existing pinned sites
val frecentSites = historyStorage
.getTopFrecentSites(totalSites, frecencyConfig)
.getTopFrecentSites(totalSites, frecencyConfig.frecencyTresholdOption)
.map { it.toTopSite() }
.filter { !pinnedSites.hasUrl(it.url) && !providerTopSites.hasUrl(it.url) }
.filter {
!pinnedSites.hasUrl(it.url) &&
!providerTopSites.hasUrl(it.url) &&
frecencyConfig.frecencyFilter?.invoke(it) ?: true
}
.take(numSitesRequired)

topSites.addAll(frecentSites)
Expand Down
20 changes: 16 additions & 4 deletions ...ts/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesConfig.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ import mozilla.components.concept.storage.FrecencyThresholdOption
* 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 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.
* @property frecencyConfig An instance of [TopSitesFrecencyConfig] that specifies which top
* frecent sites should be included.
* @property providerConfig An instance of [TopSitesProviderConfig] that specifies whether or
* not to fetch top sites from the [TopSitesProvider].
*/
data class TopSitesConfig(
val totalSites: Int,
val frecencyConfig: FrecencyThresholdOption? = null,
val frecencyConfig: TopSitesFrecencyConfig? = null,
val providerConfig: TopSitesProviderConfig? = null
)

Expand All @@ -36,3 +35,16 @@ data class TopSitesProviderConfig(
val maxThreshold: Int = Int.MAX_VALUE,
val providerFilter: ((TopSite) -> Boolean)? = null
)

/**
* Top sites frecency configuration used to specify which top frecent sites should be included.
*
* @property frecencyTresholdOption If [frecencyTresholdOption] is specified, only visited sites with a frecency
* score above the given threshold will be returned. Otherwise, frecent top site results are
* not included.
* @property frecencyFilter Optional function used to filter the top frecent sites.
*/
data class TopSitesFrecencyConfig(
val frecencyTresholdOption: FrecencyThresholdOption? = null,
val frecencyFilter: ((TopSite) -> Boolean)? = null
)
8 changes: 3 additions & 5 deletions ...s/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package mozilla.components.feature.top.sites

import mozilla.components.browser.storage.sync.PlacesHistoryStorage
import mozilla.components.concept.storage.FrecencyThresholdOption
import mozilla.components.support.base.observer.Observable

/**
Expand Down Expand Up @@ -43,15 +42,14 @@ interface TopSitesStorage : Observable<TopSitesStorage.Observer> {
* 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 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.
* @param frecencyConfig An instance of [TopSitesFrecencyConfig] that specifies which top
* frecent sites to be included.
* @param providerConfig An instance of [TopSitesProviderConfig] that specifies whether or
* not to fetch top sites from the [TopSitesProvider].
*/
suspend fun getTopSites(
totalSites: Int,
frecencyConfig: FrecencyThresholdOption? = null,
frecencyConfig: TopSitesFrecencyConfig? = null,
providerConfig: TopSitesProviderConfig? = null
): List<TopSite>

Expand Down
160 changes: 145 additions & 15 deletions ...op-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package mozilla.components.feature.top.sites

import android.net.Uri
import androidx.test.ext.junit.runners.AndroidJUnit4
import kotlinx.coroutines.ExperimentalCoroutinesApi
import mozilla.components.browser.storage.sync.PlacesHistoryStorage
Expand Down Expand Up @@ -522,7 +523,9 @@ class DefaultTopSitesStorageTest {

var topSites = defaultTopSitesStorage.getTopSites(
totalSites = 0,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = TopSitesProviderConfig(
showProviderTopSites = true
)
Expand All @@ -532,7 +535,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 1,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = TopSitesProviderConfig(
showProviderTopSites = true
)
Expand All @@ -544,7 +549,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 2,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = TopSitesProviderConfig(
showProviderTopSites = true
)
Expand All @@ -557,7 +564,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 3,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = TopSitesProviderConfig(
showProviderTopSites = true
)
Expand All @@ -571,7 +580,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 5,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = TopSitesProviderConfig(
showProviderTopSites = true
)
Expand Down Expand Up @@ -620,14 +631,18 @@ class DefaultTopSitesStorageTest {

var topSites = defaultTopSitesStorage.getTopSites(
totalSites = 0,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

assertTrue(topSites.isEmpty())

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 1,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

assertEquals(1, topSites.size)
Expand All @@ -636,7 +651,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 2,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

assertEquals(2, topSites.size)
Expand All @@ -646,7 +663,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 5,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

assertEquals(3, topSites.size)
Expand All @@ -667,7 +686,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 5,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

assertEquals(5, topSites.size)
Expand All @@ -690,7 +711,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 5,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

assertEquals(5, topSites.size)
Expand Down Expand Up @@ -755,7 +778,9 @@ class DefaultTopSitesStorageTest {

val topSites = defaultTopSitesStorage.getTopSites(
totalSites = 5,
frecencyConfig = FrecencyThresholdOption.NONE
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
)
)

verify(historyStorage).getTopFrecentSites(5, frecencyThreshold = FrecencyThresholdOption.NONE)
Expand Down Expand Up @@ -830,7 +855,9 @@ class DefaultTopSitesStorageTest {

var topSites = defaultTopSitesStorage.getTopSites(
totalSites = 3,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = providerConfig
)

Expand All @@ -842,7 +869,9 @@ class DefaultTopSitesStorageTest {

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 4,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = providerConfig
)

Expand Down Expand Up @@ -920,7 +949,9 @@ class DefaultTopSitesStorageTest {

val topSites = defaultTopSitesStorage.getTopSites(
totalSites = 10,
frecencyConfig = FrecencyThresholdOption.NONE,
frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE
),
providerConfig = TopSitesProviderConfig(
showProviderTopSites = true
)
Expand All @@ -938,4 +969,103 @@ class DefaultTopSitesStorageTest {
assertEquals("mozilla.com", frecentSiteWithNoTitle.toTopSite().title)
assertEquals(defaultTopSitesStorage.cachedTopSites, topSites)
}

@Test
fun `GIVEN frecencyFilter is set WHEN getTopSites is called THEN the frecent top sites are filtered`() = runTestOnMain {
val defaultTopSitesStorage = DefaultTopSitesStorage(
pinnedSitesStorage = pinnedSitesStorage,
historyStorage = historyStorage,
topSitesProvider = topSitesProvider,
coroutineContext = coroutineContext
)

val filterMethod: ((TopSite) -> Boolean) = { topSite ->
val uri = Uri.parse(topSite.url)
if (!uri.queryParameterNames.contains("key")) {
true
} else {
uri.getQueryParameter("key") != "value"
}
}

val filteredUrl = "https://test.com/?key=value"

val frecencyConfig = TopSitesFrecencyConfig(
frecencyTresholdOption = FrecencyThresholdOption.NONE,
frecencyFilter = filterMethod
)

val defaultSite = TopSite.Default(
id = 1,
title = "Firefox",
url = "https://firefox.com",
createdAt = 1
)
val pinnedSite = TopSite.Pinned(
id = 2,
title = "Test",
url = "https://test.com",
createdAt = 2
)

whenever(pinnedSitesStorage.getPinnedSites()).thenReturn(
listOf(
defaultSite,
pinnedSite
)
)

val providedFilteredSite = TopSite.Provided(
id = 3,
title = "Filtered",
url = "https://test.com",
clickUrl = "https://test.com/click",
imageUrl = "https://test.com/image2.jpg",
impressionUrl = "https://example.com",
createdAt = 3
)

whenever(topSitesProvider.getTopSites()).thenReturn(
listOf(
providedFilteredSite
)
)

val frecentSite = TopFrecentSiteInfo("https://getpocket.com", "Pocket")

val frecentFilteredSite = TopFrecentSiteInfo(filteredUrl, "testSearch")

whenever(historyStorage.getTopFrecentSites(anyInt(), any())).thenReturn(
listOf(
frecentSite,
frecentFilteredSite
)
)

var topSites = defaultTopSitesStorage.getTopSites(
totalSites = 4,
frecencyConfig = frecencyConfig,
providerConfig = TopSitesProviderConfig(showProviderTopSites = true)
)

assertEquals(4, topSites.size)
assertTrue(topSites.contains(frecentSite.toTopSite()))
assertTrue(topSites.contains(providedFilteredSite))
assertTrue(topSites.contains(defaultSite))
assertTrue(topSites.contains(pinnedSite))
assertEquals(defaultTopSitesStorage.cachedTopSites, topSites)

topSites = defaultTopSitesStorage.getTopSites(
totalSites = 5,
frecencyConfig = frecencyConfig,
providerConfig = TopSitesProviderConfig(showProviderTopSites = true)
)

assertEquals(4, topSites.size)
assertTrue(topSites.contains(frecentSite.toTopSite()))
assertTrue(topSites.contains(providedFilteredSite))
assertTrue(topSites.contains(defaultSite))
assertTrue(topSites.contains(pinnedSite))
assertEquals(defaultTopSitesStorage.cachedTopSites, topSites)
}
}
3 changes: 3 additions & 0 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ 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**
* Replaced `frecencyConfig` from `TopSitesConfig` with `TopSitesFrecencyConfig`, which specifies the `FrecencyTresholdOption` and the frecency filter, an optional function used to filter the top frecent sites. [#12384] (https://github.com/mozilla-mobile/android-components/issues/12384)

# 103.0.0
* [Commits](https://github.com/mozilla-mobile/android-components/compare/v102.0.0...v103.0.0)
* [Milestone](https://github.com/mozilla-mobile/android-components/milestone/150?closed=1)
Expand Down

1 comment on commit 6e52770

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

Failed to fetch task artifact public/github/customCheckRunText.md for GitHub integration.
Make sure the artifact exists on the worker or other location.

Please sign in to comment.