diff --git a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt index 75ac49f357..1f436d07a7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt @@ -17,6 +17,7 @@ import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryDataSource import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.perf.runBlockingIncrement +import org.mozilla.fenix.utils.Settings.Companion.SEARCH_GROUP_MINIMUM_SITES import kotlin.math.abs private const val BUFFER_TIME = 15000 /* 15 seconds in ms */ @@ -89,6 +90,7 @@ interface PagedHistoryProvider { class DefaultPagedHistoryProvider( private val historyStorage: PlacesHistoryStorage, private val showHistorySearchGroups: Boolean = FeatureFlags.showHistorySearchGroups, + private val historyImprovementFeatures: Boolean = FeatureFlags.historyImprovementFeatures, ) : PagedHistoryProvider { val urlSet = Array>(HistoryItemTimeGroup.values().size) { mutableSetOf() } @@ -136,6 +138,7 @@ class DefaultPagedHistoryProvider( // in the case of a pull to refresh. if (historyGroups == null || offset == 0) { historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE) + .asSequence() .sortedByDescending { it.createdAt } .filter { it.key.searchTerm != null } .groupBy { it.key.searchTerm!! } @@ -146,6 +149,14 @@ class DefaultPagedHistoryProvider( items = items.map { it.toHistoryDBMetadata() } ) } + .filter { + if (historyImprovementFeatures) { + it.items.size >= SEARCH_GROUP_MINIMUM_SITES + } else { + true + } + } + .toList() } history = getHistoryAndSearchGroups(offset, numberOfItems) diff --git a/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt b/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt index d87837b2d3..d8eaf98dd9 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt @@ -17,6 +17,8 @@ import mozilla.components.concept.storage.HistoryHighlightWeights import mozilla.components.concept.storage.HistoryMetadata import mozilla.components.concept.storage.HistoryMetadataStorage import mozilla.components.support.base.feature.LifecycleAwareFeature +import org.mozilla.fenix.FeatureFlags +import org.mozilla.fenix.FeatureFlags.historyImprovementFeatures import org.mozilla.fenix.home.HomeFragment import org.mozilla.fenix.home.HomeFragmentAction import org.mozilla.fenix.home.HomeFragmentStore @@ -24,6 +26,7 @@ import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryGrou import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryHighlight import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItemInternal.HistoryGroupInternal import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItemInternal.HistoryHighlightInternal +import org.mozilla.fenix.utils.Settings.Companion.SEARCH_GROUP_MINIMUM_SITES import kotlin.math.max @VisibleForTesting internal const val MAX_RESULTS_TOTAL = 9 @@ -48,6 +51,7 @@ class RecentVisitsFeature( private val historyHighlightsStorage: Lazy, private val scope: CoroutineScope, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, + private val historyImprovementFeatures: Boolean = FeatureFlags.historyImprovementFeatures, ) : LifecycleAwareFeature { private var job: Job? = null @@ -208,6 +212,13 @@ class RecentVisitsFeature( groupItems = it.value ) } + .filter { + if (historyImprovementFeatures) { + it.groupItems.size >= SEARCH_GROUP_MINIMUM_SITES + } else { + true + } + } } /** diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 6077b6b417..96a7155d72 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -27,6 +27,7 @@ import mozilla.components.support.ktx.android.content.stringPreference import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.FeatureFlags +import org.mozilla.fenix.FeatureFlags.historyImprovementFeatures import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.metrics.MozillaProductDetector @@ -76,6 +77,12 @@ class Settings(private val appContext: Context) : PreferencesHolder { const val ONE_WEEK_MS = 60 * 60 * 24 * 7 * 1000L const val ONE_MONTH_MS = (60 * 60 * 24 * 365 * 1000L) / 12 + /** + * The minimum number a search groups should contain. + * Filtering is applied depending on the [historyImprovementFeatures] flag value. + */ + const val SEARCH_GROUP_MINIMUM_SITES: Int = 2 + private fun Action.toInt() = when (this) { Action.BLOCKED -> BLOCKED_INT Action.ASK_TO_ALLOW -> ASK_TO_ALLOW_INT diff --git a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt index 3907ef57e7..9513d8c599 100644 --- a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt @@ -30,7 +30,8 @@ class PagedHistoryProviderTest { fun `getHistory uses getVisitsPaginated`() { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - showHistorySearchGroups = true + showHistorySearchGroups = true, + false ) val visitInfo1 = VisitInfo( @@ -147,7 +148,8 @@ class PagedHistoryProviderTest { fun `history metadata matching lower bound`() { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - showHistorySearchGroups = true + showHistorySearchGroups = true, + false ) // Oldest history visit on the page is 15 seconds (buffer time) newer than matching // metadata record. @@ -219,7 +221,8 @@ class PagedHistoryProviderTest { fun `history metadata matching upper bound`() { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - showHistorySearchGroups = true + showHistorySearchGroups = true, + false ) // Newest history visit on the page is 15 seconds (buffer time) older than matching // metadata record. @@ -291,7 +294,8 @@ class PagedHistoryProviderTest { fun `redirects are filtered out from history metadata groups`() { val provider = DefaultPagedHistoryProvider( historyStorage = storage, - showHistorySearchGroups = true + showHistorySearchGroups = true, + false ) val visitInfo1 = VisitInfo( diff --git a/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt index 94140e969f..82a1108723 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt @@ -384,7 +384,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN a list of history highlights and groups WHEN updateState is called THEN emit RecentHistoryChange`() { - val feature = spyk(RecentVisitsFeature(homeStore, mockk(), mockk(), mockk(), mockk())) + val feature = spyk(RecentVisitsFeature(homeStore, mockk(), mockk(), mockk(), mockk(), false)) val expected = List(1) { mockk() } every { feature.getCombinedHistory(any(), any()) } returns expected @@ -398,7 +398,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN highlights visits exist in search groups WHEN getCombined is called THEN remove the highlights already in groups`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(4) val directVisits = getDirectVisitsHistoryMetadataItems(4) val directDupeVisits = getSearchFromHistoryMetadataItems(2).map { @@ -422,7 +422,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN fewer than needed highlights and search groups WHEN getCombined is called THEN the result is sorted by date`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(4) val directVisits = getDirectVisitsHistoryMetadataItems(4) val expected = directVisits.reversed().toRecentHistoryHighlights() @@ -441,7 +441,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN more highlights are newer than search groups WHEN getCombined is called THEN then return an even split then sorted by date`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(5) val directVisits = getDirectVisitsHistoryMetadataItems(14) val expected = directVisits.takeLast(5).reversed().toRecentHistoryHighlights() + @@ -457,7 +457,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN more search groups are newer than highlights WHEN getCombined is called THEN then return an even split then sorted by date`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(14) val directVisits = getDirectVisitsHistoryMetadataItems(5) val expected = visitsFromSearch.takeLast(4).toIndividualRecentHistoryGroups() + @@ -473,7 +473,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN all highlights have metadata WHEN getHistoryHighlights is called THEN return a list of highlights with an inferred last access time`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) @@ -490,7 +490,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN not all highlights have metadata WHEN getHistoryHighlights is called THEN set 0 for the highlights with not found last access time`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) val highlightsWithUnknownAccessTime = directVisits.toHistoryHighlightsInternal().take(5).map { @@ -511,7 +511,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata records for the same highlight WHEN getHistoryHighlights is called THEN set the latest access time from multiple available`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) val newerDirectVisits = directVisits.mapIndexed { index, item -> @@ -533,7 +533,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata entries only for direct accessed pages WHEN getHistorySearchGroups is called THEN return an empty list`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val directVisits = getDirectVisitsHistoryMetadataItems(10) val result = feature.getHistorySearchGroups(directVisits) @@ -543,7 +543,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata entries WHEN getHistorySearchGroups is called THEN group all entries by their search term`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) @@ -556,7 +556,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN multiple metadata entries for the same url WHEN getHistorySearchGroups is called THEN entries are deduped`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val newerVisitsFromSearch = visitsFromSearch.map { it.copy(updatedAt = it.updatedAt * 2) } val directVisits = getDirectVisitsHistoryMetadataItems(10) @@ -575,7 +575,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN highlights and search groups WHEN getSortedHistory is called THEN sort descending all items based on the last access time`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10) val expected = directVisits.reversed().toRecentHistoryHighlights() @@ -594,7 +594,7 @@ class RecentVisitsFeatureTest { @Test fun `GIVEN highlights don't have a valid title WHEN getSortedHistory is called THEN the url is set as title`() { - val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk()) + val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false) val visitsFromSearch = getSearchFromHistoryMetadataItems(10) val directVisits = getDirectVisitsHistoryMetadataItems(10).mapIndexed { index, item -> when (index % 3) { @@ -646,6 +646,7 @@ class RecentVisitsFeatureTest { lazy { historyHightlightsStorage }, CoroutineScope(testDispatcher), testDispatcher, + false ) assertEquals(emptyList(), homeStore.state.recentHistory)