Skip to content

Commit

Permalink
Issue mozilla-mobile#12149: Add support for application search engine…
Browse files Browse the repository at this point in the history
… type
  • Loading branch information
rocketsroger authored and mergify[bot] committed May 16, 2022
1 parent 2f5e16e commit 77a57ac
Show file tree
Hide file tree
Showing 11 changed files with 242 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ data class SearchEngine(
* A custom search engine added by the user.
*/
CUSTOM,

/**
* A search engine add by the application.
*/
APPLICATION,
}

// Cache these parameters to avoid repeated parsing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import mozilla.components.browser.state.search.SearchEngine
* @property region The region of the user.
* @property regionSearchEngines The list of bundled [SearchEngine]s for the "home" region of the user.
* @property customSearchEngines The list of custom [SearchEngine]s, added by the user.
* @property applicationSearchEngines The list of optional [SearchEngine]s, added by application.
* @property additionalSearchEngines Additional [SearchEngine]s that the application decided to load
* and that the user explicitly added to their list of search engines.
* @property additionalAvailableSearchEngines Additional [SearchEngine]s that the application decided
Expand All @@ -33,6 +34,7 @@ data class SearchState(
val region: RegionState? = null,
val regionSearchEngines: List<SearchEngine> = emptyList(),
val customSearchEngines: List<SearchEngine> = emptyList(),
val applicationSearchEngines: List<SearchEngine> = emptyList(),
val additionalSearchEngines: List<SearchEngine> = emptyList(),
val additionalAvailableSearchEngines: List<SearchEngine> = emptyList(),
val hiddenSearchEngines: List<SearchEngine> = emptyList(),
Expand All @@ -47,7 +49,7 @@ data class SearchState(
* The list of search engines to be used for searches (bundled and custom search engines).
*/
val SearchState.searchEngines: List<SearchEngine>
get() = (regionSearchEngines + additionalSearchEngines + customSearchEngines)
get() = (regionSearchEngines + additionalSearchEngines + customSearchEngines + applicationSearchEngines)

/**
* The list of search engines that are available for the user to be added to their list of search
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class SearchStateTest {
SearchEngine("engine-d", "Engine D", mock(), type = SearchEngine.Type.CUSTOM),
SearchEngine("engine-e", "Engine E", mock(), type = SearchEngine.Type.CUSTOM)
),
applicationSearchEngines = listOf(
SearchEngine("engine-j", "Engine J", mock(), type = SearchEngine.Type.APPLICATION),
),
additionalSearchEngines = listOf(
SearchEngine("engine-f", "Engine F", mock(), type = SearchEngine.Type.BUNDLED_ADDITIONAL)
),
Expand Down Expand Up @@ -60,6 +63,9 @@ class SearchStateTest {
SearchEngine("engine-d", "Engine D", mock(), type = SearchEngine.Type.CUSTOM),
SearchEngine("engine-e", "Engine E", mock(), type = SearchEngine.Type.CUSTOM)
),
applicationSearchEngines = listOf(
SearchEngine("engine-j", "Engine J", mock(), type = SearchEngine.Type.APPLICATION),
),
additionalSearchEngines = listOf(
SearchEngine("engine-f", "Engine F", mock(), type = SearchEngine.Type.BUNDLED_ADDITIONAL)
),
Expand Down Expand Up @@ -94,6 +100,9 @@ class SearchStateTest {
SearchEngine("engine-d", "Engine D", mock(), type = SearchEngine.Type.CUSTOM),
SearchEngine("engine-e", "Engine E", mock(), type = SearchEngine.Type.CUSTOM)
),
applicationSearchEngines = listOf(
SearchEngine("engine-j", "Engine J", mock(), type = SearchEngine.Type.APPLICATION),
),
additionalSearchEngines = listOf(
SearchEngine("engine-f", "Engine F", mock(), type = SearchEngine.Type.BUNDLED_ADDITIONAL)
),
Expand Down Expand Up @@ -128,6 +137,9 @@ class SearchStateTest {
SearchEngine("engine-d", "Engine D", mock(), type = SearchEngine.Type.CUSTOM),
SearchEngine("engine-e", "Engine E", mock(), type = SearchEngine.Type.CUSTOM)
),
applicationSearchEngines = listOf(
SearchEngine("engine-j", "Engine J", mock(), type = SearchEngine.Type.APPLICATION),
),
additionalSearchEngines = listOf(
SearchEngine("engine-f", "Engine F", mock(), type = SearchEngine.Type.BUNDLED_ADDITIONAL)
),
Expand Down Expand Up @@ -168,6 +180,9 @@ class SearchStateTest {
SearchEngine("engine-d", "Engine D", mock(), type = SearchEngine.Type.CUSTOM),
SearchEngine("engine-e", "Engine E", mock(), type = SearchEngine.Type.CUSTOM)
),
applicationSearchEngines = listOf(
SearchEngine("engine-j", "Engine J", mock(), type = SearchEngine.Type.APPLICATION),
),
additionalSearchEngines = listOf(
SearchEngine("engine-f", "Engine F", mock(), type = SearchEngine.Type.BUNDLED_ADDITIONAL)
),
Expand All @@ -184,7 +199,7 @@ class SearchStateTest {
)

val searchEngines = state.searchEngines
assertEquals(6, searchEngines.size)
assertEquals(7, searchEngines.size)
assertEquals("engine-a", searchEngines[0].id)
assertEquals("engine-b", searchEngines[1].id)
assertEquals("engine-c", searchEngines[2].id)
Expand All @@ -206,6 +221,9 @@ class SearchStateTest {
SearchEngine("engine-d", "Engine D", mock(), type = SearchEngine.Type.CUSTOM),
SearchEngine("engine-e", "Engine E", mock(), type = SearchEngine.Type.CUSTOM)
),
applicationSearchEngines = listOf(
SearchEngine("engine-j", "Engine J", mock(), type = SearchEngine.Type.APPLICATION),
),
additionalSearchEngines = listOf(
SearchEngine("engine-f", "Engine F", mock(), type = SearchEngine.Type.BUNDLED_ADDITIONAL)
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class CombinedHistorySuggestionProvider(
private val loadUrlUseCase: SessionUseCases.LoadUrlUseCase,
private val icons: BrowserIcons? = null,
internal val engine: Engine? = null,
@VisibleForTesting internal val maxNumberOfSuggestions: Int = DEFAULT_COMBINED_SUGGESTION_LIMIT,
@VisibleForTesting internal var maxNumberOfSuggestions: Int = DEFAULT_COMBINED_SUGGESTION_LIMIT,
private val showEditSuggestion: Boolean = true,
) : AwesomeBar.SuggestionProvider {
override val id: String = UUID.randomUUID().toString()
Expand Down Expand Up @@ -95,4 +95,22 @@ class CombinedHistorySuggestionProvider(

return@coroutineScope combinedSuggestions
}

/**
* Set maximum number of suggestions.
*/
fun setMaxNumberOfSuggestions(maxNumber: Int) {
if (maxNumber <= 0) {
return
}

maxNumberOfSuggestions = maxNumber
}

/**
* Reset maximum number of suggestions to default.
*/
fun resetToDefaultMaxSuggestions() {
maxNumberOfSuggestions = DEFAULT_COMBINED_SUGGESTION_LIMIT
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class HistoryStorageSuggestionProvider(
private val loadUrlUseCase: SessionUseCases.LoadUrlUseCase,
private val icons: BrowserIcons? = null,
internal val engine: Engine? = null,
@VisibleForTesting internal val maxNumberOfSuggestions: Int = DEFAULT_HISTORY_SUGGESTION_LIMIT,
@VisibleForTesting internal var maxNumberOfSuggestions: Int = DEFAULT_HISTORY_SUGGESTION_LIMIT,
private val showEditSuggestion: Boolean = true,
) : AwesomeBar.SuggestionProvider {

Expand All @@ -63,6 +63,24 @@ class HistoryStorageSuggestionProvider(

return suggestions.into(this, icons, loadUrlUseCase, showEditSuggestion)
}

/**
* Set maximum number of suggestions.
*/
fun setMaxNumberOfSuggestions(maxNumber: Int) {
if (maxNumber <= 0) {
return
}

maxNumberOfSuggestions = maxNumber
}

/**
* Reset maximum number of suggestions to default.
*/
fun resetToDefaultMaxSuggestions() {
maxNumberOfSuggestions = DEFAULT_HISTORY_SUGGESTION_LIMIT
}
}

internal suspend fun Iterable<SearchResult>.into(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,59 @@ class CombinedHistorySuggestionProviderTest {
assertEquals("http://www.mozilla.com/firefox/", result[0].description)
assertNull(result[0].editSuggestion)
}

@Test
fun `WHEN provider max number of suggestions is changed THEN the number of return suggestions is updated`() = runTest {
val history: HistoryStorage = mock()
val metadata: HistoryMetadataStorage = mock()
doReturn(emptyList<HistoryMetadata>()).`when`(metadata).queryHistoryMetadata(eq("moz"), anyInt())
doReturn(
(1..50).map {
SearchResult("id$it", "http://www.mozilla.com/$it/", 10)
}
).`when`(history).getSuggestions(eq("moz"), anyInt())

val provider = CombinedHistorySuggestionProvider(history, metadata, mock(), showEditSuggestion = false)

provider.setMaxNumberOfSuggestions(2)
var suggestions = provider.onInputChanged("moz")
assertEquals(2, suggestions.size)

provider.setMaxNumberOfSuggestions(22)
suggestions = provider.onInputChanged("moz")
assertEquals(22, suggestions.size)

provider.setMaxNumberOfSuggestions(0)
suggestions = provider.onInputChanged("moz")
assertEquals(22, suggestions.size)

provider.setMaxNumberOfSuggestions(45)
suggestions = provider.onInputChanged("moz")
assertEquals(45, suggestions.size)
}

@Test
fun `WHEN reset provider max number of suggestions THEN the number of return suggestions is reset to default`() = runTest {
val history: HistoryStorage = mock()
val metadata: HistoryMetadataStorage = mock()
doReturn(emptyList<HistoryMetadata>()).`when`(metadata).queryHistoryMetadata(eq("moz"), anyInt())
doReturn(
(1..50).map {
SearchResult("id$it", "http://www.mozilla.com/$it/", 10)
}
).`when`(history).getSuggestions(eq("moz"), anyInt())

val provider = CombinedHistorySuggestionProvider(history, metadata, mock(), showEditSuggestion = false)

var suggestions = provider.onInputChanged("moz")
assertEquals(5, suggestions.size)

provider.setMaxNumberOfSuggestions(45)
suggestions = provider.onInputChanged("moz")
assertEquals(45, suggestions.size)

provider.resetToDefaultMaxSuggestions()
suggestions = provider.onInputChanged("moz")
assertEquals(5, suggestions.size)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,64 @@ class HistoryStorageSuggestionProviderTest {
assertEquals(22, suggestions.size)
}

@Test
fun `WHEN provider max number of suggestions is changed THEN the number of return suggestions is updated`() = runTest {
val history: HistoryStorage = mock()
Mockito.doReturn(
(1..50).map {
SearchResult("id$it", "http://www.mozilla.com/$it/", 10)
}
).`when`(history).getSuggestions(eq("moz"), Mockito.anyInt())

val provider = HistoryStorageSuggestionProvider(
historyStorage = history, loadUrlUseCase = mock()
)

var suggestions = provider.onInputChanged("moz")
assertEquals(20, suggestions.size)

provider.setMaxNumberOfSuggestions(2)
suggestions = provider.onInputChanged("moz")
assertEquals(2, suggestions.size)

provider.setMaxNumberOfSuggestions(22)
suggestions = provider.onInputChanged("moz")
assertEquals(22, suggestions.size)

provider.setMaxNumberOfSuggestions(45)
suggestions = provider.onInputChanged("moz")
assertEquals(45, suggestions.size)

provider.setMaxNumberOfSuggestions(0)
suggestions = provider.onInputChanged("moz")
assertEquals(45, suggestions.size)
}

@Test
fun `WHEN reset provider max number of suggestions THEN the number of return suggestions is reset to default`() = runTest {
val history: HistoryStorage = mock()
Mockito.doReturn(
(1..50).map {
SearchResult("id$it", "http://www.mozilla.com/$it/", 10)
}
).`when`(history).getSuggestions(eq("moz"), Mockito.anyInt())

val provider = HistoryStorageSuggestionProvider(
historyStorage = history, loadUrlUseCase = mock()
)

var suggestions = provider.onInputChanged("moz")
assertEquals(20, suggestions.size)

provider.setMaxNumberOfSuggestions(45)
suggestions = provider.onInputChanged("moz")
assertEquals(45, suggestions.size)

provider.resetToDefaultMaxSuggestions()
suggestions = provider.onInputChanged("moz")
assertEquals(20, suggestions.size)
}

@Test
fun `Provider dedupes suggestions`() = runTest {
val storage: HistoryStorage = mock()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ class SearchUseCases(
SearchEngine.Type.CUSTOM -> store.dispatch(
SearchAction.UpdateCustomSearchEngineAction(searchEngine)
)

SearchEngine.Type.APPLICATION -> { /* Do nothing */ }
}
}
}
Expand Down Expand Up @@ -209,6 +211,8 @@ class SearchUseCases(
SearchEngine.Type.CUSTOM -> store.dispatch(
SearchAction.RemoveCustomSearchEngineAction(searchEngine.id)
)

SearchEngine.Type.APPLICATION -> { /* Do nothing */ }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ fun createSearchEngine(
)
}

/**
* Creates an application [SearchEngine].
*/
fun createApplicationSearchEngine(
id: String? = null,
name: String,
url: String,
icon: Bitmap,
suggestUrl: String? = null,
): SearchEngine {
return SearchEngine(
id = id ?: UUID.randomUUID().toString(),
name = name,
icon = icon,
type = SearchEngine.Type.APPLICATION,
resultUrls = listOf(url),
suggestUrl = suggestUrl,
)
}

/**
* Whether this [SearchEngine] has a [SearchEngine.suggestUrl] set and can provide search
* suggestions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ import kotlin.coroutines.CoroutineContext
/**
* [Middleware] implementation for loading and saving [SearchEngine]s whenever the state changes.
*
* @param context The application context.
* @param additionalBundledSearchEngineIds List of (bundled) search engine IDs that will be loaded
* in addition to the search engines for the user's region and made available through
* [SearchState.additionalSearchEngines] and [SearchState.additionalSearchEngines].
* @param migration Interface for a class that can provide data from a legacy system to be imported into the
* storage used by the middleware.
* @param customStorage A storage for custom search engines of the user.
* @param bundleStorage A storage for loading bundled search engines.
* @param metadataStorage A storage for saving additional metadata related to search.
* @param ioDispatcher The coroutine dispatcher to be used when loading.
*/
@Suppress("LongParameterList")
class SearchMiddleware(
Expand All @@ -41,7 +46,7 @@ class SearchMiddleware(
private val customStorage: CustomStorage = CustomSearchEngineStorage(context),
private val bundleStorage: BundleStorage = BundledSearchEnginesStorage(context),
private val metadataStorage: MetadataStorage = SearchMetadataStorage(context),
private val ioDispatcher: CoroutineContext = Dispatchers.IO
private val ioDispatcher: CoroutineContext = Dispatchers.IO,
) : Middleware<BrowserState, BrowserAction> {
private val logger = Logger("SearchMiddleware")
private val scope = CoroutineScope(ioDispatcher)
Expand Down
Loading

0 comments on commit 77a57ac

Please sign in to comment.