diff --git a/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt b/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt index c9bc63c74ba6..abcd1a741c56 100644 --- a/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt +++ b/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt @@ -234,6 +234,15 @@ fun String.tryGetHostFromUrl(): String = try { this } +/** + * Returns `true` if this string is a valid URL that contains [searchParameters] in its query parameters. + */ +fun String.urlContainsQueryParameters(searchParameters: String): Boolean = try { + URL(this).query?.split("&")?.any { it == searchParameters } ?: false +} catch (e: MalformedURLException) { + false +} + /** * Compares 2 URLs and returns true if they have the same origin, * which means: same protocol, same host, same port. diff --git a/android-components/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt b/android-components/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt index b8f9630517b6..89ade2aace7c 100644 --- a/android-components/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt +++ b/android-components/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt @@ -568,6 +568,25 @@ class StringTest { assertNull(invalidBase64String2.extractBase6RawString()) } + @Test + fun `GIVEN a URL with matching parameters WHEN testing if a URL contains query parameters THEN the result is true`() { + assertTrue("http://example.com?a".urlContainsQueryParameters("a")) + assertTrue("http://example.com?a&b&c".urlContainsQueryParameters("b")) + assertTrue("http://example.com?a=b".urlContainsQueryParameters("a=b")) + assertTrue("http://example.com?a=b&c=d&e=f".urlContainsQueryParameters("c=d")) + assertTrue("http://example.com?a=b&c=d&e=f#g=h".urlContainsQueryParameters("e=f")) + } + + @Test + fun `GIVEN a URL without matching parameters WHEN testing if a URL contains query parameters THEN the result is false`() { + assertFalse("".urlContainsQueryParameters("a")) + assertFalse("!@#$%^&*()-+".urlContainsQueryParameters("a")) + assertFalse("http://example.com".urlContainsQueryParameters("a")) + assertFalse("http://example.com?a&b".urlContainsQueryParameters("c")) + assertFalse("http://example.com?a=b".urlContainsQueryParameters("a")) + assertFalse("http://example.com?a=b&c=d&e=f#g=h".urlContainsQueryParameters("g=h")) + } + private infix fun String.shortenedShouldBecome(expect: String) { assertEquals(expect, this.shortened()) } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt b/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt index cd72a5d773fe..12f002394625 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.search.awesomebar import android.content.Context import android.graphics.drawable.Drawable +import android.net.Uri import androidx.annotation.VisibleForTesting import androidx.appcompat.content.res.AppCompatResources import androidx.core.graphics.BlendModeColorFilterCompat.createBlendModeColorFilterCompat @@ -34,6 +35,7 @@ import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl +import mozilla.components.support.ktx.kotlin.urlContainsQueryParameters import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode @@ -41,6 +43,7 @@ import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.Core.Companion.METADATA_HISTORY_SUGGESTION_LIMIT import org.mozilla.fenix.components.Core.Companion.METADATA_SHORTCUT_SUGGESTION_LIMIT import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.containsQueryParameters import org.mozilla.fenix.ext.settings import org.mozilla.fenix.search.SearchEngineSource import org.mozilla.fenix.search.SearchFragmentState @@ -255,7 +258,7 @@ class AwesomeBarView( } } - @Suppress("ComplexMethod") + @Suppress("ComplexMethod", "LongMethod") @VisibleForTesting internal fun getProvidersToAdd( state: SearchProviderState, @@ -280,25 +283,31 @@ class AwesomeBarView( } if (state.showAllHistorySuggestions) { - if (activity.settings().historyMetadataUIFeature) { - providersToAdd.add(defaultCombinedHistoryProvider) - } else { - providersToAdd.add(defaultHistoryStorageProvider) - } + providersToAdd.add( + getHistoryProvider( + filter = getFilterToExcludeSponsoredResults(state), + ), + ) } if (state.showHistorySuggestionsForCurrentEngine) { - getHistoryProvidersForSearchEngine(state.searchEngineSource)?.let { - providersToAdd.add(it) + getFilterForCurrentEngineResults(state)?.let { + providersToAdd.add(getHistoryProvider(it)) } } if (state.showAllBookmarkSuggestions) { - providersToAdd.add(getBookmarksProvider(state.searchEngineSource)) + providersToAdd.add( + getBookmarksProvider( + filter = getFilterToExcludeSponsoredResults(state), + ), + ) } if (state.showBookmarksSuggestionsForCurrentEngine) { - providersToAdd.add(getBookmarksProvider(state.searchEngineSource, true)) + getFilterForCurrentEngineResults(state)?.let { + providersToAdd.add(getBookmarksProvider(it)) + } } if (state.showSearchSuggestions) { @@ -306,19 +315,28 @@ class AwesomeBarView( } if (state.showAllSyncedTabsSuggestions) { - providersToAdd.add(getSyncedTabsProvider(state.searchEngineSource)) + providersToAdd.add( + getSyncedTabsProvider( + filter = getFilterToExcludeSponsoredResults(state), + ), + ) } if (state.showSyncedTabsSuggestionsForCurrentEngine) { - providersToAdd.add(getSyncedTabsProvider(state.searchEngineSource, true)) + getFilterForCurrentEngineResults(state)?.let { + providersToAdd.add(getSyncedTabsProvider(it)) + } } if (activity.browsingModeManager.mode == BrowsingMode.Normal && state.showAllSessionSuggestions) { - providersToAdd.add(getLocalTabsProvider(state.searchEngineSource)) + // Unlike other providers, we don't exclude sponsored suggestions for open tabs. + providersToAdd.add(getLocalTabsProvider()) } if (activity.browsingModeManager.mode == BrowsingMode.Normal && state.showSessionSuggestionsForCurrentEngine) { - providersToAdd.add(getLocalTabsProvider(state.searchEngineSource, true)) + getFilterForCurrentEngineResults(state)?.let { + providersToAdd.add(getLocalTabsProvider(it)) + } } if (state.showSponsoredSuggestions || state.showNonSponsoredSuggestions) { @@ -340,44 +358,48 @@ class AwesomeBarView( } /** - * Get a new history suggestion provider that will return suggestions only from the current - * search engine's host. - * Used only for when unified search is active. + * Get a history suggestion provider configured for this [AwesomeBarView]. * - * @param searchEngineSource Search engine wrapper also informing about the selection type. + * @param filter Optional filter to limit the returned history suggestions. * * @return A [CombinedHistorySuggestionProvider] or [HistoryStorageSuggestionProvider] depending - * on if the history metadata feature is enabled or `null` if the current engine's host is unknown. + * on if the history metadata feature is enabled. */ @VisibleForTesting - internal fun getHistoryProvidersForSearchEngine( - searchEngineSource: SearchEngineSource, - ): AwesomeBar.SuggestionProvider? { - val searchEngineUriFilter = searchEngineSource.searchEngine?.resultsUrl ?: return null - + internal fun getHistoryProvider( + filter: SearchResultFilter? = null, + ): AwesomeBar.SuggestionProvider { return if (activity.settings().historyMetadataUIFeature) { - CombinedHistorySuggestionProvider( - historyStorage = components.core.historyStorage, - historyMetadataStorage = components.core.historyStorage, - loadUrlUseCase = loadUrlUseCase, - icons = components.core.icons, - engine = engineForSpeculativeConnects, - maxNumberOfSuggestions = METADATA_SUGGESTION_LIMIT, - showEditSuggestion = false, - suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = { it.sameHostWithoutMobileSubdomainAs(searchEngineUriFilter) }, - ) + if (filter != null) { + CombinedHistorySuggestionProvider( + historyStorage = components.core.historyStorage, + historyMetadataStorage = components.core.historyStorage, + loadUrlUseCase = loadUrlUseCase, + icons = components.core.icons, + engine = engineForSpeculativeConnects, + maxNumberOfSuggestions = METADATA_SUGGESTION_LIMIT, + showEditSuggestion = false, + suggestionsHeader = activity.getString(R.string.firefox_suggest_header), + resultsUriFilter = filter::shouldIncludeUri, + ) + } else { + defaultCombinedHistoryProvider + } } else { - HistoryStorageSuggestionProvider( - historyStorage = components.core.historyStorage, - loadUrlUseCase = loadUrlUseCase, - icons = components.core.icons, - engine = engineForSpeculativeConnects, - maxNumberOfSuggestions = METADATA_SUGGESTION_LIMIT, - showEditSuggestion = false, - suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = { it.sameHostWithoutMobileSubdomainAs(searchEngineUriFilter) }, - ) + if (filter != null) { + HistoryStorageSuggestionProvider( + historyStorage = components.core.historyStorage, + loadUrlUseCase = loadUrlUseCase, + icons = components.core.icons, + engine = engineForSpeculativeConnects, + maxNumberOfSuggestions = METADATA_SUGGESTION_LIMIT, + showEditSuggestion = false, + suggestionsHeader = activity.getString(R.string.firefox_suggest_header), + resultsUriFilter = filter::shouldIncludeUri, + ) + } else { + defaultHistoryStorageProvider + } } } @@ -457,24 +479,16 @@ class AwesomeBarView( } /** - * Get a synced tabs provider automatically configured to filter or not results from just the current search engine. + * Get a synced tabs provider configured for this [AwesomeBarView]. * - * @param searchEngineSource Search engine wrapper also informing about the selection type. - * @param filterByCurrentEngine Whether to apply a filter to the constructed provider such that - * it will return bookmarks only for the current search engine. + * @param filter Optional filter to limit the returned synced tab suggestions. * * @return [SyncedTabsStorageSuggestionProvider] providing suggestions for the [AwesomeBar]. */ @VisibleForTesting internal fun getSyncedTabsProvider( - searchEngineSource: SearchEngineSource, - filterByCurrentEngine: Boolean = false, + filter: SearchResultFilter? = null, ): SyncedTabsStorageSuggestionProvider { - val searchEngineHostFilter = when (filterByCurrentEngine) { - true -> searchEngineSource.searchEngine?.resultsUrl?.host - false -> null - } - return SyncedTabsStorageSuggestionProvider( components.backgroundServices.syncedTabsStorage, loadUrlUseCase, @@ -485,31 +499,21 @@ class AwesomeBarView( getDrawable(activity, R.drawable.ic_search_results_device_tablet), ), suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUrlFilter = searchEngineHostFilter?.let { - { url -> url.tryGetHostFromUrl() == searchEngineHostFilter } - }, + resultsUrlFilter = filter?.let { it::shouldIncludeUrl }, ) } /** - * Get a local tabs provider automatically configured to filter or not results from just the current search engine. + * Get a local tabs provider configured for this [AwesomeBarView]. * - * @param searchEngineSource Search engine wrapper also informing about the selection type. - * @param filterByCurrentEngine Whether to apply a filter to the constructed provider such that - * it will return bookmarks only for the current search engine. + * @param filter Optional filter to limit the returned local tab suggestions. * * @return [SessionSuggestionProvider] providing suggestions for the [AwesomeBar]. */ @VisibleForTesting internal fun getLocalTabsProvider( - searchEngineSource: SearchEngineSource, - filterByCurrentEngine: Boolean = false, + filter: SearchResultFilter? = null, ): SessionSuggestionProvider { - val searchEngineUriFilter = when (filterByCurrentEngine) { - true -> searchEngineSource.searchEngine?.resultsUrl - false -> null - } - return SessionSuggestionProvider( activity.resources, components.core.store, @@ -518,31 +522,21 @@ class AwesomeBarView( getDrawable(activity, R.drawable.ic_search_results_tab), excludeSelectedSession = !fromHomeFragment, suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = searchEngineUriFilter?.let { - { uri -> uri.sameHostWithoutMobileSubdomainAs(it) } - }, + resultsUriFilter = filter?.let { it::shouldIncludeUri }, ) } /** - * Get a bookmarks provider automatically configured to filter or not results from just the current search engine. + * Get a bookmarks provider configured for this [AwesomeBarView]. * - * @param searchEngineSource Search engine wrapper also informing about the selection type. - * @param filterByCurrentEngine Whether to apply a filter to the constructed provider such that - * it will return bookmarks only for the current search engine. + * @param filter Optional filter to limit the returned bookmark suggestions. * * @return [BookmarksStorageSuggestionProvider] providing suggestions for the [AwesomeBar]. */ @VisibleForTesting internal fun getBookmarksProvider( - searchEngineSource: SearchEngineSource, - filterByCurrentEngine: Boolean = false, + filter: SearchResultFilter? = null, ): BookmarksStorageSuggestionProvider { - val searchEngineHostFilter = when (filterByCurrentEngine) { - true -> searchEngineSource.searchEngine?.resultsUrl - false -> null - } - return BookmarksStorageSuggestionProvider( bookmarksStorage = components.core.bookmarksStorage, loadUrlUseCase = loadUrlUseCase, @@ -551,12 +545,28 @@ class AwesomeBarView( engine = engineForSpeculativeConnects, showEditSuggestion = false, suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = searchEngineHostFilter?.let { - { uri -> uri.sameHostWithoutMobileSubdomainAs(it) } - }, + resultsUriFilter = filter?.let { it::shouldIncludeUri }, ) } + /** + * Returns a [SearchResultFilter] that only includes results for the current search engine. + */ + internal fun getFilterForCurrentEngineResults(state: SearchProviderState): SearchResultFilter? = + state.searchEngineSource.searchEngine?.resultsUrl?.let { + SearchResultFilter.CurrentEngine(it) + } + + /** + * Returns a [SearchResultFilter] that excludes sponsored results. + */ + internal fun getFilterToExcludeSponsoredResults(state: SearchProviderState): SearchResultFilter? = + if (state.showSponsoredSuggestions) { + SearchResultFilter.ExcludeSponsored(activity.settings().frecencyFilterQuery) + } else { + null + } + data class SearchProviderState( val showSearchShortcuts: Boolean, val showSearchTermHistory: Boolean, @@ -574,6 +584,41 @@ class AwesomeBarView( val searchEngineSource: SearchEngineSource, ) + /** + * Filters to limit the suggestions returned from a suggestion provider. + */ + sealed interface SearchResultFilter { + /** + * A filter for the currently selected search engine. This filter only includes suggestions + * whose URLs have the same host as [resultsUri]. + */ + data class CurrentEngine(val resultsUri: Uri) : SearchResultFilter + + /** + * A filter that excludes sponsored suggestions, whose URLs contain the given + * [queryParameter]. + */ + data class ExcludeSponsored(val queryParameter: String) : SearchResultFilter + + /** + * Returns `true` if the suggestion with the given [uri] should be included in the + * suggestions returned from the provider. + */ + fun shouldIncludeUri(uri: Uri): Boolean = when (this) { + is CurrentEngine -> this.resultsUri.sameHostWithoutMobileSubdomainAs(uri) + is ExcludeSponsored -> !uri.containsQueryParameters(queryParameter) + } + + /** + * Returns `true` if the suggestion with the given [url] string should be included in the + * suggestions returned from the provider. + */ + fun shouldIncludeUrl(url: String): Boolean = when (this) { + is CurrentEngine -> resultsUri.host == url.tryGetHostFromUrl() + is ExcludeSponsored -> !url.urlContainsQueryParameters(queryParameter) + } + } + companion object { // Maximum number of suggestions returned. const val METADATA_SUGGESTION_LIMIT = 3 diff --git a/fenix/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt index c75f6ef48b97..07448da47402 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt @@ -87,13 +87,14 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN a search from history and history metadata enabled WHEN setting the providers THEN set more suggestions to be shown`() { + fun `GIVEN a search from history and history metadata is enabled and sponsored suggestions are enabled WHEN setting the providers THEN set less suggestions to be shown`() { val settings: Settings = mockk(relaxed = true) { every { historyMetadataUIFeature } returns true } every { activity.settings() } returns settings val state = getSearchProviderState( searchEngineSource = SearchEngineSource.History(mockk(relaxed = true)), + showSponsoredSuggestions = true, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -101,19 +102,20 @@ class AwesomeBarViewTest { val historyProvider = result.firstOrNull { it is CombinedHistorySuggestionProvider } assertNotNull(historyProvider) assertEquals( - Companion.METADATA_HISTORY_SUGGESTION_LIMIT, + AwesomeBarView.METADATA_SUGGESTION_LIMIT, (historyProvider as CombinedHistorySuggestionProvider).getMaxNumberOfSuggestions(), ) } @Test - fun `GIVEN a search from history and history metadata disabled WHEN setting the providers THEN set more suggestions to be shown`() { + fun `GIVEN a search from history and history metadata is enabled and sponsored suggestions are disabled WHEN setting the providers THEN set more suggestions to be shown`() { val settings: Settings = mockk(relaxed = true) { every { historyMetadataUIFeature } returns true } every { activity.settings() } returns settings val state = getSearchProviderState( searchEngineSource = SearchEngineSource.History(mockk(relaxed = true)), + showSponsoredSuggestions = false, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -127,13 +129,98 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN a search not from history and history metadata enabled WHEN setting the providers THEN set less suggestions to be shown`() { + fun `GIVEN a search from history and history metadata is disabled and sponsored suggestions are enabled WHEN setting the providers THEN set less suggestions to be shown`() { + val settings: Settings = mockk(relaxed = true) { + every { historyMetadataUIFeature } returns false + } + every { activity.settings() } returns settings + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.History(mockk(relaxed = true)), + showSponsoredSuggestions = true, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProvider = result.firstOrNull { it is HistoryStorageSuggestionProvider } + assertNotNull(historyProvider) + assertEquals( + AwesomeBarView.METADATA_SUGGESTION_LIMIT, + (historyProvider as HistoryStorageSuggestionProvider).getMaxNumberOfSuggestions(), + ) + } + + @Test + fun `GIVEN a search from history and history metadata is disabled and sponsored suggestions are disabled WHEN setting the providers THEN set more suggestions to be shown`() { + val settings: Settings = mockk(relaxed = true) { + every { historyMetadataUIFeature } returns false + } + every { activity.settings() } returns settings + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.History(mockk(relaxed = true)), + showSponsoredSuggestions = false, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProvider = result.firstOrNull { it is HistoryStorageSuggestionProvider } + assertNotNull(historyProvider) + assertEquals( + Companion.METADATA_HISTORY_SUGGESTION_LIMIT, + (historyProvider as HistoryStorageSuggestionProvider).getMaxNumberOfSuggestions(), + ) + } + + @Test + fun `GIVEN a search not from history and history metadata is enabled and sponsored suggestions are enabled WHEN setting the providers THEN set less suggestions to be shown`() { val settings: Settings = mockk(relaxed = true) { every { historyMetadataUIFeature } returns true } every { activity.settings() } returns settings val state = getSearchProviderState( searchEngineSource = SearchEngineSource.Shortcut(mockk(relaxed = true)), + showSponsoredSuggestions = true, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProvider = result.firstOrNull { it is CombinedHistorySuggestionProvider } + assertNotNull(historyProvider) + assertEquals( + AwesomeBarView.METADATA_SUGGESTION_LIMIT, + (historyProvider as CombinedHistorySuggestionProvider).getMaxNumberOfSuggestions(), + ) + } + + @Test + fun `GIVEN a search not from history and history metadata is enabled and sponsored suggestions are disabled WHEN setting the providers THEN set less suggestions to be shown`() { + val settings: Settings = mockk(relaxed = true) { + every { historyMetadataUIFeature } returns true + } + every { activity.settings() } returns settings + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Shortcut(mockk(relaxed = true)), + showSponsoredSuggestions = false, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProvider = result.firstOrNull { it is CombinedHistorySuggestionProvider } + assertNotNull(historyProvider) + assertEquals( + AwesomeBarView.METADATA_SUGGESTION_LIMIT, + (historyProvider as CombinedHistorySuggestionProvider).getMaxNumberOfSuggestions(), + ) + } + + @Test + fun `GIVEN a search not from history and history metadata is disabled and sponsored suggestions are enabled WHEN setting the providers THEN set less suggestions to be shown`() { + val settings: Settings = mockk(relaxed = true) { + every { historyMetadataUIFeature } returns true + } + every { activity.settings() } returns settings + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Bookmarks(mockk(relaxed = true)), + showSponsoredSuggestions = true, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -167,7 +254,7 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN a search that should show filtered history WHEN history metadata is enabled THEN return a history metadata provider with an engine filter`() { + fun `GIVEN a search that should show filtered history WHEN history metadata is enabled and sponsored suggestions are enabled THEN return a history metadata provider with an engine filter`() { val settings: Settings = mockk(relaxed = true) { every { historyMetadataUIFeature } returns true } @@ -180,6 +267,7 @@ class AwesomeBarViewTest { every { resultsUrl } returns url }, ), + showSponsoredSuggestions = true, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -190,6 +278,34 @@ class AwesomeBarViewTest { assertEquals(AwesomeBarView.METADATA_SUGGESTION_LIMIT, historyProvider.getMaxNumberOfSuggestions()) } + @Test + fun `GIVEN a search that should show filtered history WHEN history metadata is enabled and sponsored suggestions are disabled THEN return a history metadata provider with an engine filter`() { + val settings: Settings = mockk(relaxed = true) { + every { historyMetadataUIFeature } returns true + } + val url = Uri.parse("test.com") + every { activity.settings() } returns settings + val state = getSearchProviderState( + showAllHistorySuggestions = false, + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + showSponsoredSuggestions = false, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProvider = result.firstOrNull { it is CombinedHistorySuggestionProvider } + assertNotNull(historyProvider) + assertNotNull((historyProvider as CombinedHistorySuggestionProvider).resultsUriFilter) + assertEquals( + AwesomeBarView.METADATA_SUGGESTION_LIMIT, + historyProvider.getMaxNumberOfSuggestions(), + ) + } + @Test fun `GIVEN the default engine is selected WHEN history metadata is enabled THEN suggestions are disabled in history and bookmark providers`() { val settings: Settings = mockk(relaxed = true) { @@ -543,7 +659,32 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN a search that should show filtered history WHEN history metadata is disabled THEN return a history provider with an engine filter`() { + fun `GIVEN a search that should show filtered history WHEN history metadata is disabled and sponsored suggestions are enabled THEN return a history provider with an engine filter`() { + val settings: Settings = mockk(relaxed = true) { + every { historyMetadataUIFeature } returns false + } + val url = Uri.parse("test.com") + every { activity.settings() } returns settings + val state = getSearchProviderState( + showAllHistorySuggestions = false, + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + showSponsoredSuggestions = true, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProvider = result.firstOrNull { it is HistoryStorageSuggestionProvider } + assertNotNull(historyProvider) + assertNotNull((historyProvider as HistoryStorageSuggestionProvider).resultsUriFilter) + assertEquals(AwesomeBarView.METADATA_SUGGESTION_LIMIT, historyProvider.getMaxNumberOfSuggestions()) + } + + @Test + fun `GIVEN a search that should show filtered history WHEN history metadata is disabled and sponsored suggestions are disabled THEN return a history provider with an engine filter`() { val settings: Settings = mockk(relaxed = true) { every { historyMetadataUIFeature } returns false } @@ -556,6 +697,7 @@ class AwesomeBarViewTest { every { resultsUrl } returns url }, ), + showSponsoredSuggestions = false, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -623,17 +765,42 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN normal browsing mode and needing to show all local tabs suggestions WHEN configuring providers THEN add the tabs provider`() { + fun `GIVEN normal browsing mode and needing to show all local tabs suggestions and sponsored suggestions are enabled WHEN configuring providers THEN add the tabs provider`() { val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") every { activity.settings() } returns settings every { activity.browsingModeManager.mode } returns BrowsingMode.Normal val state = getSearchProviderState( showSessionSuggestionsForCurrentEngine = false, searchEngineSource = SearchEngineSource.Shortcut( mockk(relaxed = true) { - every { resultsUrl.host } returns "test" + every { resultsUrl } returns url }, ), + showSponsoredSuggestions = true, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val localSessionsProviders = result.filterIsInstance() + assertEquals(1, localSessionsProviders.size) + assertNull(localSessionsProviders[0].resultsUriFilter) + } + + @Test + fun `GIVEN normal browsing mode and needing to show all local tabs suggestions and sponsored suggestions are disabled WHEN configuring providers THEN add the tabs provider`() { + val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") + every { activity.settings() } returns settings + every { activity.browsingModeManager.mode } returns BrowsingMode.Normal + val state = getSearchProviderState( + showSessionSuggestionsForCurrentEngine = false, + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + showSponsoredSuggestions = false, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -680,38 +847,42 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN needing to show all synced tabs suggestions WHEN configuring providers THEN add the synced tabs provider`() { + fun `GIVEN needing to show all synced tabs suggestions and sponsored suggestions are enabled WHEN configuring providers THEN add the synced tabs provider with a sponsored filter`() { val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") every { activity.settings() } returns settings every { activity.browsingModeManager.mode } returns BrowsingMode.Normal val state = getSearchProviderState( showSyncedTabsSuggestionsForCurrentEngine = false, searchEngineSource = SearchEngineSource.Shortcut( mockk(relaxed = true) { - every { resultsUrl.host } returns "test" + every { resultsUrl } returns url }, ), + showSponsoredSuggestions = true, ) val result = awesomeBarView.getProvidersToAdd(state) val localSessionsProviders = result.filterIsInstance() assertEquals(1, localSessionsProviders.size) - assertNull(localSessionsProviders[0].resultsUrlFilter) + assertNotNull(localSessionsProviders[0].resultsUrlFilter) } @Test - fun `GIVEN needing to show filtered synced tabs suggestions WHEN configuring providers THEN add the synced tabs provider with an engine filter`() { + fun `GIVEN needing to show filtered synced tabs suggestions and sponsored suggestions are enabled WHEN configuring providers THEN add the synced tabs provider with an engine filter`() { val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") every { activity.settings() } returns settings every { activity.browsingModeManager.mode } returns BrowsingMode.Normal val state = getSearchProviderState( showAllSyncedTabsSuggestions = false, searchEngineSource = SearchEngineSource.Shortcut( mockk(relaxed = true) { - every { resultsUrl.host } returns "test" + every { resultsUrl } returns url }, ), + showSponsoredSuggestions = true, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -722,17 +893,65 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN needing to show all bookmarks suggestions WHEN configuring providers THEN add the bookmarks provider`() { + fun `GIVEN needing to show all synced tabs suggestions and sponsored suggestions are disabled WHEN configuring providers THEN add the synced tabs provider`() { + val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") + every { activity.settings() } returns settings + every { activity.browsingModeManager.mode } returns BrowsingMode.Normal + val state = getSearchProviderState( + showSyncedTabsSuggestionsForCurrentEngine = false, + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + showSponsoredSuggestions = false, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val localSessionsProviders = result.filterIsInstance() + assertEquals(1, localSessionsProviders.size) + assertNull(localSessionsProviders[0].resultsUrlFilter) + } + + @Test + fun `GIVEN needing to show all bookmarks suggestions and sponsored suggestions are enabled WHEN configuring providers THEN add the bookmarks provider with a sponsored filter`() { + val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") + every { activity.settings() } returns settings + every { activity.browsingModeManager.mode } returns BrowsingMode.Normal + val state = getSearchProviderState( + showBookmarksSuggestionsForCurrentEngine = false, + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + showSponsoredSuggestions = true, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val localSessionsProviders = result.filterIsInstance() + assertEquals(1, localSessionsProviders.size) + assertNotNull(localSessionsProviders[0].resultsUriFilter) + } + + @Test + fun `GIVEN needing to show all bookmarks suggestions and sponsored suggestions are disabled WHEN configuring providers THEN add the bookmarks provider`() { val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") every { activity.settings() } returns settings every { activity.browsingModeManager.mode } returns BrowsingMode.Normal val state = getSearchProviderState( showBookmarksSuggestionsForCurrentEngine = false, searchEngineSource = SearchEngineSource.Shortcut( mockk(relaxed = true) { - every { resultsUrl.host } returns "test" + every { resultsUrl } returns url }, ), + showSponsoredSuggestions = false, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -778,7 +997,7 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN a search from the default engine with all suggestions asked WHEN configuring providers THEN add them all`() { + fun `GIVEN a search from the default engine with all suggestions asked and sponsored suggestions are enabled WHEN configuring providers THEN add them all`() { val settings: Settings = mockk(relaxed = true) val url = Uri.parse("https://www.test.com") every { activity.settings() } returns settings @@ -789,6 +1008,45 @@ class AwesomeBarViewTest { every { resultsUrl } returns url }, ), + showSponsoredSuggestions = true, + ) + + val result = awesomeBarView.getProvidersToAdd(state) + + val historyProviders: List = result.filterIsInstance() + assertEquals(2, historyProviders.size) + assertNotNull(historyProviders[0].resultsUriFilter) // the general history provider + assertNotNull(historyProviders[1].resultsUriFilter) // the filtered history provider + val bookmarksProviders: List = result.filterIsInstance() + assertEquals(2, bookmarksProviders.size) + assertNotNull(bookmarksProviders[0].resultsUriFilter) // the general bookmarks provider + assertNotNull(bookmarksProviders[1].resultsUriFilter) // the filtered bookmarks provider + assertEquals(1, result.filterIsInstance().size) + assertEquals(1, result.filterIsInstance().size) + val syncedTabsProviders: List = result.filterIsInstance() + assertEquals(2, syncedTabsProviders.size) + assertNotNull(syncedTabsProviders[0].resultsUrlFilter) // the general synced tabs provider + assertNotNull(syncedTabsProviders[1].resultsUrlFilter) // the filtered synced tabs provider + val localTabsProviders: List = result.filterIsInstance() + assertEquals(2, localTabsProviders.size) + assertNull(localTabsProviders[0].resultsUriFilter) // the general tabs provider + assertNotNull(localTabsProviders[1].resultsUriFilter) // the filtered tabs provider + assertEquals(1, result.filterIsInstance().size) + } + + @Test + fun `GIVEN a search from the default engine with all suggestions asked and sponsored suggestions are disabled WHEN configuring providers THEN add them all`() { + val settings: Settings = mockk(relaxed = true) + val url = Uri.parse("https://www.test.com") + every { activity.settings() } returns settings + every { activity.browsingModeManager.mode } returns BrowsingMode.Normal + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Default( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + showSponsoredSuggestions = false, ) val result = awesomeBarView.getProvidersToAdd(state) @@ -928,15 +1186,6 @@ class AwesomeBarViewTest { assertNull(fxSuggestProvider) } - @Test - fun `GIVEN the current search engine's url is not known WHEN creating a history provider for that engine THEN return null`() { - val engineSource = SearchEngineSource.None - - val result = awesomeBarView.getHistoryProvidersForSearchEngine(engineSource) - - assertNull(result) - } - @Test fun `GIVEN a valid search engine and history metadata enabled WHEN creating a history provider for that engine THEN return a history metadata provider with engine filter`() { val settings: Settings = mockk { @@ -944,8 +1193,11 @@ class AwesomeBarViewTest { } every { activity.settings() } returns settings val searchEngineSource = SearchEngineSource.Shortcut(mockk(relaxed = true)) + val state = getSearchProviderState(searchEngineSource = searchEngineSource) - val result = awesomeBarView.getHistoryProvidersForSearchEngine(searchEngineSource) + val result = awesomeBarView.getHistoryProvider( + filter = awesomeBarView.getFilterForCurrentEngineResults(state), + ) assertNotNull(result) assertTrue(result is CombinedHistorySuggestionProvider) @@ -954,14 +1206,19 @@ class AwesomeBarViewTest { } @Test - fun `GIVEN a valid search engine and history metadata disabled WHEN creating a history provider for that engine THEN return a history metadata provider with engine filter`() { + fun `GIVEN a valid search engine and history metadata disabled WHEN creating a history provider for that engine THEN return a history metadata provider with an engine filter`() { val settings: Settings = mockk { every { historyMetadataUIFeature } returns false } every { activity.settings() } returns settings val searchEngineSource = SearchEngineSource.Shortcut(mockk(relaxed = true)) + val state = getSearchProviderState( + searchEngineSource = searchEngineSource, + ) - val result = awesomeBarView.getHistoryProvidersForSearchEngine(searchEngineSource) + val result = awesomeBarView.getHistoryProvider( + filter = awesomeBarView.getFilterForCurrentEngineResults(state), + ) assertNotNull(result) assertTrue(result is HistoryStorageSuggestionProvider) @@ -969,54 +1226,6 @@ class AwesomeBarViewTest { assertEquals(AwesomeBarView.METADATA_SUGGESTION_LIMIT, result.getMaxNumberOfSuggestions()) } - @Test - fun `GIVEN a filter is required WHEN configuring a bookmarks provider THEN include a url filter`() { - assertNotNull( - awesomeBarView.getBookmarksProvider( - searchEngineSource = mockk(relaxed = true), - ), - ) - - assertNotNull( - awesomeBarView.getBookmarksProvider( - searchEngineSource = mockk(relaxed = true), - filterByCurrentEngine = true, - ), - ) - } - - @Test - fun `GIVEN a filter is required WHEN configuring a synced tabs provider THEN include a url filter`() { - assertNotNull( - awesomeBarView.getSyncedTabsProvider( - searchEngineSource = mockk(relaxed = true), - ), - ) - - assertNotNull( - awesomeBarView.getSyncedTabsProvider( - searchEngineSource = mockk(relaxed = true), - filterByCurrentEngine = true, - ), - ) - } - - @Test - fun `GIVEN a filter is required WHEN configuring a local tabs provider THEN include a url filter`() { - assertNotNull( - awesomeBarView.getLocalTabsProvider( - searchEngineSource = mockk(relaxed = true), - ), - ) - - assertNotNull( - awesomeBarView.getLocalTabsProvider( - searchEngineSource = mockk(relaxed = true), - filterByCurrentEngine = true, - ), - ) - } - @Test fun `GIVEN a search engine is not available WHEN asking for a search term provider THEN return null`() { val searchEngineSource: SearchEngineSource = SearchEngineSource.None @@ -1122,6 +1331,121 @@ class AwesomeBarViewTest { assertEquals(1, result.filterIsInstance().size) } + + @Test + fun `GIVEN sponsored suggestions are enabled WHEN getting a filter to exclude sponsored suggestions THEN return the filter`() { + every { activity.settings() } returns mockk(relaxed = true) { + every { frecencyFilterQuery } returns "query=value" + } + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Default(mockk(relaxed = true)), + showSponsoredSuggestions = true, + ) + val filter = awesomeBarView.getFilterToExcludeSponsoredResults(state) + + assertEquals(AwesomeBarView.SearchResultFilter.ExcludeSponsored("query=value"), filter) + } + + @Test + fun `GIVEN sponsored suggestions are disabled WHEN getting a filter to exclude sponsored suggestions THEN return null`() { + every { activity.settings() } returns mockk(relaxed = true) { + every { frecencyFilterQuery } returns "query=value" + } + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Default(mockk(relaxed = true)), + showSponsoredSuggestions = false, + ) + val filter = awesomeBarView.getFilterToExcludeSponsoredResults(state) + + assertNull(filter) + } + + @Test + fun `GIVEN a sponsored query parameter and a sponsored filter WHEN a URL contains the sponsored query parameter THEN that URL should be excluded`() { + every { activity.settings() } returns mockk(relaxed = true) { + every { frecencyFilterQuery } returns "query=value" + } + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Default(mockk(relaxed = true)), + showSponsoredSuggestions = true, + ) + val filter = requireNotNull(awesomeBarView.getFilterToExcludeSponsoredResults(state)) + + assertFalse(filter.shouldIncludeUri(Uri.parse("http://example.com?query=value"))) + assertFalse(filter.shouldIncludeUri(Uri.parse("http://example.com/a?query=value"))) + assertFalse(filter.shouldIncludeUri(Uri.parse("http://example.com/a?b=c&query=value"))) + assertFalse(filter.shouldIncludeUri(Uri.parse("http://example.com/a?b=c&query=value&d=e"))) + + assertFalse(filter.shouldIncludeUrl("http://example.com?query=value")) + assertFalse(filter.shouldIncludeUrl("http://example.com/a?query=value")) + assertFalse(filter.shouldIncludeUrl("http://example.com/a?b=c&query=value")) + assertFalse(filter.shouldIncludeUrl("http://example.com/a?b=c&query=value&d=e")) + } + + @Test + fun `GIVEN a sponsored query parameter and a sponsored filter WHEN a URL does not contain the sponsored query parameter THEN that URL should be included`() { + every { activity.settings() } returns mockk(relaxed = true) { + every { frecencyFilterQuery } returns "query=value" + } + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Default(mockk(relaxed = true)), + showSponsoredSuggestions = true, + ) + val filter = requireNotNull(awesomeBarView.getFilterToExcludeSponsoredResults(state)) + + assertTrue(filter.shouldIncludeUri(Uri.parse("http://example.com"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://example.com?query"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://example.com/a"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://example.com/a?b"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://example.com/a?b&c=d"))) + + assertTrue(filter.shouldIncludeUrl("http://example.com")) + assertTrue(filter.shouldIncludeUrl("http://example.com?query")) + assertTrue(filter.shouldIncludeUrl("http://example.com/a")) + assertTrue(filter.shouldIncludeUrl("http://example.com/a?b")) + assertTrue(filter.shouldIncludeUrl("http://example.com/a?b&c=d")) + } + + @Test + fun `GIVEN an engine with a results URL and an engine filter WHEN a URL matches the results URL THEN that URL should be included`() { + val url = Uri.parse("http://test.com") + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + ) + + val filter = requireNotNull(awesomeBarView.getFilterForCurrentEngineResults(state)) + + assertTrue(filter.shouldIncludeUri(Uri.parse("http://test.com"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://test.com/a"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://mobile.test.com"))) + assertTrue(filter.shouldIncludeUri(Uri.parse("http://mobile.test.com/a"))) + + assertTrue(filter.shouldIncludeUrl("http://test.com")) + assertTrue(filter.shouldIncludeUrl("http://test.com/a")) + } + + @Test + fun `GIVEN an engine with a results URL and an engine filter WHEN a URL does not match the results URL THEN that URL should be excluded`() { + val url = Uri.parse("http://test.com") + val state = getSearchProviderState( + searchEngineSource = SearchEngineSource.Shortcut( + mockk(relaxed = true) { + every { resultsUrl } returns url + }, + ), + ) + + val filter = requireNotNull(awesomeBarView.getFilterForCurrentEngineResults(state)) + + assertFalse(filter.shouldIncludeUri(Uri.parse("http://other.com"))) + assertFalse(filter.shouldIncludeUri(Uri.parse("http://subdomain.test.com"))) + + assertFalse(filter.shouldIncludeUrl("http://mobile.test.com")) + } } /**