From 36ed959f498052649df9ca7a2b868ba9cd3ce5e0 Mon Sep 17 00:00:00 2001 From: Lina Butler Date: Fri, 27 Oct 2023 18:05:01 -0700 Subject: [PATCH] Bug 1861415 - Refactor `results{Uri, Url}Filter` to be a predicate. This commit: * Changes the bookmarks, history, session, and synced tabs suggestion providers to take an optional predicate that filters matching suggestions by `Uri` (bookmarks, history, session) or URL string (synced tabs), instead of comparing the suggestions' URLs directly. * Hoists the host matching checks out of the providers. --- .../fenix/search/awesomebar/AwesomeBarView.kt | 18 ++++++++++++----- .../search/awesomebar/AwesomeBarViewTest.kt | 20 +++++++++---------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt index f637671d0c..cd72a5d773 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt @@ -32,6 +32,8 @@ import mozilla.components.feature.syncedtabs.DeviceIndicators import mozilla.components.feature.syncedtabs.SyncedTabsStorageSuggestionProvider 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 org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode @@ -363,7 +365,7 @@ class AwesomeBarView( maxNumberOfSuggestions = METADATA_SUGGESTION_LIMIT, showEditSuggestion = false, suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = searchEngineUriFilter, + resultsUriFilter = { it.sameHostWithoutMobileSubdomainAs(searchEngineUriFilter) }, ) } else { HistoryStorageSuggestionProvider( @@ -374,7 +376,7 @@ class AwesomeBarView( maxNumberOfSuggestions = METADATA_SUGGESTION_LIMIT, showEditSuggestion = false, suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = searchEngineUriFilter, + resultsUriFilter = { it.sameHostWithoutMobileSubdomainAs(searchEngineUriFilter) }, ) } } @@ -483,7 +485,9 @@ class AwesomeBarView( getDrawable(activity, R.drawable.ic_search_results_device_tablet), ), suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsHostFilter = searchEngineHostFilter, + resultsUrlFilter = searchEngineHostFilter?.let { + { url -> url.tryGetHostFromUrl() == searchEngineHostFilter } + }, ) } @@ -514,7 +518,9 @@ class AwesomeBarView( getDrawable(activity, R.drawable.ic_search_results_tab), excludeSelectedSession = !fromHomeFragment, suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = searchEngineUriFilter, + resultsUriFilter = searchEngineUriFilter?.let { + { uri -> uri.sameHostWithoutMobileSubdomainAs(it) } + }, ) } @@ -545,7 +551,9 @@ class AwesomeBarView( engine = engineForSpeculativeConnects, showEditSuggestion = false, suggestionsHeader = activity.getString(R.string.firefox_suggest_header), - resultsUriFilter = searchEngineHostFilter, + resultsUriFilter = searchEngineHostFilter?.let { + { uri -> uri.sameHostWithoutMobileSubdomainAs(it) } + }, ) } diff --git a/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt b/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt index ef6f99a332..c75f6ef48b 100644 --- a/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/awesomebar/AwesomeBarViewTest.kt @@ -186,7 +186,7 @@ class AwesomeBarViewTest { val historyProvider = result.firstOrNull { it is CombinedHistorySuggestionProvider } assertNotNull(historyProvider) - assertEquals(url, (historyProvider as CombinedHistorySuggestionProvider).resultsUriFilter) + assertNotNull((historyProvider as CombinedHistorySuggestionProvider).resultsUriFilter) assertEquals(AwesomeBarView.METADATA_SUGGESTION_LIMIT, historyProvider.getMaxNumberOfSuggestions()) } @@ -562,7 +562,7 @@ class AwesomeBarViewTest { val historyProvider = result.firstOrNull { it is HistoryStorageSuggestionProvider } assertNotNull(historyProvider) - assertEquals(url, (historyProvider as HistoryStorageSuggestionProvider).resultsUriFilter) + assertNotNull((historyProvider as HistoryStorageSuggestionProvider).resultsUriFilter) assertEquals(AwesomeBarView.METADATA_SUGGESTION_LIMIT, historyProvider.getMaxNumberOfSuggestions()) } @@ -662,7 +662,7 @@ class AwesomeBarViewTest { val localSessionsProviders = result.filterIsInstance() assertEquals(1, localSessionsProviders.size) - assertEquals(url, localSessionsProviders[0].resultsUriFilter) + assertNotNull(localSessionsProviders[0].resultsUriFilter) } @Test @@ -697,7 +697,7 @@ class AwesomeBarViewTest { val localSessionsProviders = result.filterIsInstance() assertEquals(1, localSessionsProviders.size) - assertNull(localSessionsProviders[0].resultsHostFilter) + assertNull(localSessionsProviders[0].resultsUrlFilter) } @Test @@ -718,7 +718,7 @@ class AwesomeBarViewTest { val localSessionsProviders = result.filterIsInstance() assertEquals(1, localSessionsProviders.size) - assertEquals("test", localSessionsProviders[0].resultsHostFilter) + assertNotNull(localSessionsProviders[0].resultsUrlFilter) } @Test @@ -761,7 +761,7 @@ class AwesomeBarViewTest { val localSessionsProviders = result.filterIsInstance() assertEquals(1, localSessionsProviders.size) - assertEquals(url, localSessionsProviders[0].resultsUriFilter) + assertNotNull(localSessionsProviders[0].resultsUriFilter) } @Test @@ -800,17 +800,17 @@ class AwesomeBarViewTest { val bookmarksProviders: List = result.filterIsInstance() assertEquals(2, bookmarksProviders.size) assertNull(bookmarksProviders[0].resultsUriFilter) // the general bookmarks provider - assertEquals(url, bookmarksProviders[1].resultsUriFilter) // the filtered 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) - assertNull(syncedTabsProviders[0].resultsHostFilter) // the general synced tabs provider - assertEquals("www.test.com", syncedTabsProviders[1].resultsHostFilter) // the filtered synced tabs provider + assertNull(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 - assertEquals(url, localTabsProviders[1].resultsUriFilter) // the filtered tabs provider + assertNotNull(localTabsProviders[1].resultsUriFilter) // the filtered tabs provider assertEquals(1, result.filterIsInstance().size) }