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.
gitstart · Nov 3, 2023 · 8dcfad5 · 8dcfad5
1 parent 8a46f76
commit 8dcfad5
Show file tree

Hide file tree

Showing 12 changed files with 90 additions and 61 deletions.
diff --git a/...java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProvider.kt b/...java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProvider.kt
@@ -17,7 +17,6 @@ import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarksStorage
 import mozilla.components.feature.awesomebar.facts.emitBookmarkSuggestionClickedFact
 import mozilla.components.feature.session.SessionUseCases
-import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import java.util.UUID
 
 /**
@@ -46,7 +45,7 @@ internal const val BOOKMARKS_RESULTS_TO_FILTER_SCALE_FACTOR = 10
  * highest scored suggestion URL.
  * @param showEditSuggestion optional parameter to specify if the suggestion should show the edit button
  * @param suggestionsHeader optional parameter to specify if the suggestion should have a header
- * @param resultsUriFilter Optional filter for the host url of the suggestions to show.
+ * @param resultsUriFilter Optional predicate to filter matching suggestions by URL.
  */
 class BookmarksStorageSuggestionProvider(
     @get:VisibleForTesting internal val bookmarksStorage: BookmarksStorage,
@@ -56,7 +55,7 @@ class BookmarksStorageSuggestionProvider(
     private val engine: Engine? = null,
     @get:VisibleForTesting val showEditSuggestion: Boolean = true,
     private val suggestionsHeader: String? = null,
-    @get:VisibleForTesting val resultsUriFilter: Uri? = null,
+    @get:VisibleForTesting val resultsUriFilter: ((Uri) -> Boolean)? = null,
 ) : AwesomeBar.SuggestionProvider {
     override val id: String = UUID.randomUUID().toString()
 
@@ -73,7 +72,7 @@ class BookmarksStorageSuggestionProvider(
 
         val suggestions = when (resultsUriFilter) {
             null -> getBookmarksSuggestions(text)
-            else -> getBookmarksSuggestionsFromHost(resultsUriFilter, text)
+            else -> getFilteredBookmarksSuggestions(text, resultsUriFilter)
         }
 
         suggestions.firstOrNull()?.url?.let { url -> engine?.speculativeConnect(url) }
@@ -93,15 +92,15 @@ class BookmarksStorageSuggestionProvider(
         .sortedBy { it.guid }
 
     /**
-     * Get up to [BOOKMARKS_SUGGESTION_LIMIT] bookmarks matching [query] from the indicated [url].
+     * Get up to [BOOKMARKS_SUGGESTION_LIMIT] bookmarks matching [query] and [filter].
      *
      * @param query String to filter bookmarks' title or URL by.
-     * @param url URL to filter all bookmarks' URL host by.
+     * @param filter Predicate to filter the URLs of the bookmarks that match the [query].
      */
-    private suspend fun getBookmarksSuggestionsFromHost(url: Uri, query: String) = bookmarksStorage
+    private suspend fun getFilteredBookmarksSuggestions(query: String, filter: (Uri) -> Boolean) = bookmarksStorage
         .searchBookmarks(query, BOOKMARKS_SUGGESTION_LIMIT * BOOKMARKS_RESULTS_TO_FILTER_SCALE_FACTOR)
         .filter {
-            it.url?.toUri()?.sameHostWithoutMobileSubdomainAs(url) ?: true
+            it.url?.toUri()?.let(filter) ?: true
         }
         .distinctBy { it.url }
         .sortedBy { it.guid }

diff --git a/.../java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProvider.kt b/.../java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProvider.kt
@@ -16,7 +16,6 @@ import mozilla.components.concept.storage.HistoryMetadata
 import mozilla.components.concept.storage.HistoryMetadataStorage
 import mozilla.components.concept.storage.HistoryStorage
 import mozilla.components.feature.session.SessionUseCases
-import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import java.util.UUID
 
 /**
@@ -51,7 +50,7 @@ internal const val COMBINED_HISTORY_RESULTS_TO_FILTER_SCALE_FACTOR = 10
  * defaults to [DEFAULT_COMBINED_SUGGESTION_LIMIT].
  * @param showEditSuggestion optional parameter to specify if the suggestion should show the edit button
  * @param suggestionsHeader optional parameter to specify if the suggestion should have a header
- * @param resultsUriFilter Optional filter for the host url of the suggestions to show.
+ * @param resultsUriFilter Optional predicate to filter matching suggestions by URL.
  */
 @Suppress("LongParameterList")
 class CombinedHistorySuggestionProvider(
@@ -63,7 +62,7 @@ class CombinedHistorySuggestionProvider(
     internal var maxNumberOfSuggestions: Int = DEFAULT_COMBINED_SUGGESTION_LIMIT,
     @get:VisibleForTesting val showEditSuggestion: Boolean = true,
     private val suggestionsHeader: String? = null,
-    @get:VisibleForTesting val resultsUriFilter: Uri? = null,
+    @get:VisibleForTesting val resultsUriFilter: ((Uri) -> Boolean)? = null,
 ) : AwesomeBar.SuggestionProvider {
     override val id: String = UUID.randomUUID().toString()
 
@@ -82,14 +81,14 @@ class CombinedHistorySuggestionProvider(
         val metadataSuggestionsAsync = async {
             when (resultsUriFilter) {
                 null -> getMetadataSuggestions(text)
-                else -> getMetadataSuggestionsFromHost(resultsUriFilter, text)
+                else -> getFilteredMetadataSuggestions(text, resultsUriFilter)
             }
         }
 
         val historySuggestionsAsync = async {
             when (resultsUriFilter) {
                 null -> getHistorySuggestions(text)
-                else -> getHistorySuggestionsFromHost(resultsUriFilter, text)
+                else -> getFilteredHistorySuggestions(text, resultsUriFilter)
             }
         }
 
@@ -151,15 +150,15 @@ class CombinedHistorySuggestionProvider(
         .into(this@CombinedHistorySuggestionProvider, icons, loadUrlUseCase, showEditSuggestion)
 
     /**
-     * Get up to [maxNumberOfSuggestions] history metadata suggestions matching [query] from the indicated [url].
+     * Get up to [maxNumberOfSuggestions] history metadata suggestions matching [query] and [filter].
      *
      * @param query String to filter history entry's title or URL by.
-     * @param url URL host to filter all history entry's URL host by.
+     * @param filter Predicate to filter the URLs of the history entries that match the [query].
      */
-    private suspend fun getMetadataSuggestionsFromHost(url: Uri, query: String) = historyMetadataStorage
+    private suspend fun getFilteredMetadataSuggestions(query: String, filter: (Uri) -> Boolean) = historyMetadataStorage
         .queryHistoryMetadata(query, maxNumberOfSuggestions * COMBINED_HISTORY_RESULTS_TO_FILTER_SCALE_FACTOR)
         .filter {
-            it.totalViewTime > 0 && it.key.url.toUri().sameHostWithoutMobileSubdomainAs(url)
+            it.totalViewTime > 0 && filter(it.key.url.toUri())
         }
         .take(maxNumberOfSuggestions)
         .into(this@CombinedHistorySuggestionProvider, icons, loadUrlUseCase, showEditSuggestion)
@@ -176,17 +175,17 @@ class CombinedHistorySuggestionProvider(
         .into(this@CombinedHistorySuggestionProvider, icons, loadUrlUseCase, showEditSuggestion)
 
     /**
-     * Get up to [maxNumberOfSuggestions] history metadata suggestions matching [query] from the indicated [url].
+     * Get up to [maxNumberOfSuggestions] history metadata suggestions matching [query] and [filter].
      *
      * @param query String to filter history entry's title or URL by.
-     * @param url URL host to filter all bookmarks' URL host by.
+     * @param filter Predicate to filter the URLs of the history entries that match the [query].
      */
-    private suspend fun getHistorySuggestionsFromHost(url: Uri, query: String) = historyStorage
+    private suspend fun getFilteredHistorySuggestions(query: String, filter: (Uri) -> Boolean) = historyStorage
         .getSuggestions(query, maxNumberOfSuggestions * COMBINED_HISTORY_RESULTS_TO_FILTER_SCALE_FACTOR)
         .distinctBy { it.id }
         .sortedByDescending { it.score }
         .filter {
-            it.url.toUri().sameHostWithoutMobileSubdomainAs(url)
+            filter(it.url.toUri())
         }
         .take(maxNumberOfSuggestions)
         .into(this@CombinedHistorySuggestionProvider, icons, loadUrlUseCase, showEditSuggestion)

diff --git a/...n/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProvider.kt b/...n/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProvider.kt
@@ -15,7 +15,6 @@ import mozilla.components.concept.storage.HistoryStorage
 import mozilla.components.concept.storage.SearchResult
 import mozilla.components.feature.awesomebar.facts.emitHistorySuggestionClickedFact
 import mozilla.components.feature.session.SessionUseCases
-import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import java.util.UUID
 
 /**
@@ -45,7 +44,7 @@ internal const val HISTORY_RESULTS_TO_FILTER_SCALE_FACTOR = 10
  * defaults to [DEFAULT_HISTORY_SUGGESTION_LIMIT]
  * @param showEditSuggestion optional parameter to specify if the suggestion should show the edit button
  * @param suggestionsHeader optional parameter to specify if the suggestion should have a header
- * @param resultsUriFilter Optional filter for the host url of the suggestions to show.
+ * @param resultsUriFilter Optional predicate to filter matching suggestions by URL.
  */
 class HistoryStorageSuggestionProvider(
     @get:VisibleForTesting internal val historyStorage: HistoryStorage,
@@ -55,7 +54,7 @@ class HistoryStorageSuggestionProvider(
     @get:VisibleForTesting internal var maxNumberOfSuggestions: Int = DEFAULT_HISTORY_SUGGESTION_LIMIT,
     @get:VisibleForTesting val showEditSuggestion: Boolean = true,
     private val suggestionsHeader: String? = null,
-    @get:VisibleForTesting val resultsUriFilter: Uri? = null,
+    @get:VisibleForTesting val resultsUriFilter: ((Uri) -> Boolean)? = null,
 ) : AwesomeBar.SuggestionProvider {
 
     override val id: String = UUID.randomUUID().toString()
@@ -73,7 +72,7 @@ class HistoryStorageSuggestionProvider(
 
         val suggestions = when (resultsUriFilter) {
             null -> getHistorySuggestions(text)
-            else -> getHistorySuggestionsFromHost(resultsUriFilter, text)
+            else -> getFilteredHistorySuggestions(text, resultsUriFilter)
         }
 
         suggestions.firstOrNull()?.url?.let { url -> engine?.speculativeConnect(url) }
@@ -119,17 +118,17 @@ class HistoryStorageSuggestionProvider(
         .take(maxNumberOfSuggestions)
 
     /**
-     * Get up to [maxNumberOfSuggestions] history suggestions matching [query] from the indicated [url].
+     * Get up to [maxNumberOfSuggestions] history suggestions matching [query] and [filter].
      *
      * @param query String to filter history entry's title or URL by.
-     * @param url URL host to filter all history entry's URL host by.
+     * @param filter Predicate to filter the URLs of the history entries that match the [query].
      */
-    private fun getHistorySuggestionsFromHost(url: Uri, query: String) = historyStorage
+    private fun getFilteredHistorySuggestions(query: String, filter: (Uri) -> Boolean) = historyStorage
         .getSuggestions(query, maxNumberOfSuggestions * HISTORY_RESULTS_TO_FILTER_SCALE_FACTOR)
         .sortedByDescending { it.score }
         .distinctBy { it.id }
         .filter {
-            it.url.toUri().sameHostWithoutMobileSubdomainAs(url)
+            filter(it.url.toUri())
         }
         .take(maxNumberOfSuggestions)
 }

diff --git a/...src/main/java/mozilla/components/feature/awesomebar/provider/SessionSuggestionProvider.kt b/...src/main/java/mozilla/components/feature/awesomebar/provider/SessionSuggestionProvider.kt
@@ -20,7 +20,6 @@ import mozilla.components.concept.awesomebar.AwesomeBar
 import mozilla.components.feature.awesomebar.R
 import mozilla.components.feature.awesomebar.facts.emitOpenTabSuggestionClickedFact
 import mozilla.components.feature.tabs.TabsUseCases
-import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import java.util.UUID
 
 /**
@@ -36,7 +35,7 @@ class SessionSuggestionProvider(
     private val indicatorIcon: Drawable? = null,
     private val excludeSelectedSession: Boolean = false,
     private val suggestionsHeader: String? = null,
-    @get:VisibleForTesting val resultsUriFilter: Uri? = null,
+    @get:VisibleForTesting val resultsUriFilter: ((Uri) -> Boolean)? = null,
 ) : AwesomeBar.SuggestionProvider {
     override val id: String = UUID.randomUUID().toString()
 
@@ -62,7 +61,7 @@ class SessionSuggestionProvider(
         val searchWords = searchText.split(" ")
         tabs.zip(iconRequests) { result, icon ->
             if (
-                resultsUriFilter?.sameHostWithoutMobileSubdomainAs(result.content.url.toUri()) != false &&
+                resultsUriFilter?.invoke(result.content.url.toUri()) != false &&
                 searchWords.all { result.contains(it) } &&
                 !result.content.private &&
                 shouldIncludeSelectedTab(state, result)

diff --git a/.../mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt b/.../mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt
@@ -13,6 +13,7 @@ import mozilla.components.concept.storage.BookmarkInfo
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
 import mozilla.components.concept.storage.BookmarksStorage
+import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import mozilla.components.support.test.eq
 import mozilla.components.support.test.mock
 import mozilla.components.support.utils.StorageUtils.levenshteinDistance
@@ -179,7 +180,9 @@ class BookmarksStorageSuggestionProviderTest {
         val provider = BookmarksStorageSuggestionProvider(
             bookmarksStorage = bookmarksSpy,
             loadUrlUseCase = mock(),
-            resultsUriFilter = "https://www.test.com".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://www.test.com".toUri())
+            },
         )
 
         provider.onInputChanged("moz")
@@ -196,7 +199,9 @@ class BookmarksStorageSuggestionProviderTest {
         val provider = BookmarksStorageSuggestionProvider(
             bookmarksStorage = bookmarksSpy,
             loadUrlUseCase = mock(),
-            resultsUriFilter = "https://mozilla.com".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://mozilla.com".toUri())
+            },
         )
 
         bookmarks.addItem("Other", "https://mozilla.com/firefox", newItem.title!!, null)

diff --git a/...a/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProviderTest.kt b/...a/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProviderTest.kt
@@ -14,6 +14,7 @@ import mozilla.components.concept.storage.HistoryMetadataKey
 import mozilla.components.concept.storage.HistoryMetadataStorage
 import mozilla.components.concept.storage.HistoryStorage
 import mozilla.components.concept.storage.SearchResult
+import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import mozilla.components.support.test.eq
 import mozilla.components.support.test.mock
 import org.junit.Assert.assertEquals
@@ -298,7 +299,9 @@ class CombinedHistorySuggestionProviderTest {
             historyMetadataStorage = metadata,
             loadUrlUseCase = mock(),
             showEditSuggestion = false,
-            resultsUriFilter = "test".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("test".toUri())
+            },
         )
 
         provider.onInputChanged("moz")
@@ -327,7 +330,9 @@ class CombinedHistorySuggestionProviderTest {
             historyMetadataStorage = metadata,
             loadUrlUseCase = mock(),
             showEditSuggestion = false,
-            resultsUriFilter = "https://mozilla.com".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://mozilla.com".toUri())
+            },
         )
 
         val suggestions = provider.onInputChanged("moz")
@@ -358,7 +363,9 @@ class CombinedHistorySuggestionProviderTest {
             historyMetadataStorage = metadata,
             loadUrlUseCase = mock(),
             showEditSuggestion = false,
-            resultsUriFilter = "https://mozilla.com".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://mozilla.com".toUri())
+            },
         )
 
         val suggestions = provider.onInputChanged("moz")

diff --git a/...va/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProviderTest.kt b/...va/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProviderTest.kt
@@ -17,6 +17,7 @@ import mozilla.components.support.base.facts.Action
 import mozilla.components.support.base.facts.Fact
 import mozilla.components.support.base.facts.FactProcessor
 import mozilla.components.support.base.facts.Facts
+import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import mozilla.components.support.test.eq
 import mozilla.components.support.test.mock
 import org.junit.Assert.assertEquals
@@ -326,7 +327,9 @@ class HistoryStorageSuggestionProviderTest {
             historyStorage = history,
             loadUrlUseCase = mock(),
             maxNumberOfSuggestions = 13,
-            resultsUriFilter = "test".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("test".toUri())
+            },
         )
         val expectedQueryCount = 13 * HISTORY_RESULTS_TO_FILTER_SCALE_FACTOR
 
@@ -351,7 +354,9 @@ class HistoryStorageSuggestionProviderTest {
         val provider = HistoryStorageSuggestionProvider(
             historyStorage = history,
             loadUrlUseCase = mock(),
-            resultsUriFilter = "https://mozilla.com".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://mozilla.com".toUri())
+            },
         )
 
         val suggestions = provider.onInputChanged("moz")
@@ -376,7 +381,9 @@ class HistoryStorageSuggestionProviderTest {
         val provider = HistoryStorageSuggestionProvider(
             historyStorage = history,
             loadUrlUseCase = mock(),
-            resultsUriFilter = "https://mozilla.com".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://mozilla.com".toUri())
+            },
         )
 
         val suggestions = provider.onInputChanged("moz")

diff --git a/...test/java/mozilla/components/feature/awesomebar/provider/SessionSuggestionProviderTest.kt b/...test/java/mozilla/components/feature/awesomebar/provider/SessionSuggestionProviderTest.kt
@@ -15,6 +15,7 @@ import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.feature.tabs.TabsUseCases
+import mozilla.components.support.ktx.android.net.sameHostWithoutMobileSubdomainAs
 import mozilla.components.support.test.mock
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -366,7 +367,9 @@ class SessionSuggestionProviderTest {
             resources = resources,
             store = store,
             selectTabUseCase = mock(),
-            resultsUriFilter = "https://mozilla.org".toUri(),
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs("https://mozilla.org".toUri())
+            },
         )
 
         val suggestions = provider.onInputChanged("moz")
@@ -399,7 +402,9 @@ class SessionSuggestionProviderTest {
             resources = resources,
             store = store,
             selectTabUseCase = mock(),
-            resultsUriFilter = uriFilter,
+            resultsUriFilter = {
+                it.sameHostWithoutMobileSubdomainAs(uriFilter)
+            },
         )
 
         val suggestions = provider.onInputChanged("moz")

diff --git a/...rc/main/java/mozilla/components/feature/syncedtabs/SyncedTabsStorageSuggestionProvider.kt b/...rc/main/java/mozilla/components/feature/syncedtabs/SyncedTabsStorageSuggestionProvider.kt
@@ -14,7 +14,6 @@ import mozilla.components.feature.session.SessionUseCases
 import mozilla.components.feature.syncedtabs.ext.getActiveDeviceTabs
 import mozilla.components.feature.syncedtabs.facts.emitSyncedTabSuggestionClickedFact
 import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage
-import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl
 import java.util.UUID
 
 /**
@@ -27,7 +26,7 @@ class SyncedTabsStorageSuggestionProvider(
     private val icons: BrowserIcons? = null,
     private val deviceIndicators: DeviceIndicators = DeviceIndicators(),
     private val suggestionsHeader: String? = null,
-    @get:VisibleForTesting val resultsHostFilter: String? = null,
+    @get:VisibleForTesting val resultsUrlFilter: ((String) -> Boolean)? = null,
 ) : AwesomeBar.SuggestionProvider {
     override val id: String = UUID.randomUUID().toString()
 
@@ -43,7 +42,7 @@ class SyncedTabsStorageSuggestionProvider(
         val results = syncedTabs.getActiveDeviceTabs { tab ->
             // This is a fairly naive match implementation, but this is what we do on Desktop 🤷.
             (tab.url.contains(text, ignoreCase = true) || tab.title.contains(text, ignoreCase = true)) &&
-                resultsHostFilter?.equals(tab.url.tryGetHostFromUrl()) != false
+                resultsUrlFilter?.invoke(tab.url) != false
         }
 
         return results.sortedByDescending { it.lastUsed }.into()