diff --git a/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngine.kt b/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngine.kt index 054fec69a0d..ebec210c421 100644 --- a/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngine.kt +++ b/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngine.kt @@ -44,15 +44,15 @@ class SearchEngine internal constructor( var result = template val locale = Locale.getDefault().toString() - result = result.replace(MOZ_PARAM_LOCALE.toRegex(), locale) - result = result.replace(MOZ_PARAM_DIST_ID.toRegex(), "") - result = result.replace(MOZ_PARAM_OFFICIAL.toRegex(), "unofficial") + result = result.replace(MOZ_PARAM_LOCALE, locale) + result = result.replace(MOZ_PARAM_DIST_ID, "") + result = result.replace(MOZ_PARAM_OFFICIAL, "unofficial") - result = result.replace(OS_PARAM_USER_DEFINED.toRegex(), query) - result = result.replace(OS_PARAM_INPUT_ENCODING.toRegex(), "UTF-8") + result = result.replace(OS_PARAM_USER_DEFINED, query) + result = result.replace(OS_PARAM_INPUT_ENCODING, "UTF-8") - result = result.replace(OS_PARAM_LANGUAGE.toRegex(), locale) - result = result.replace(OS_PARAM_OUTPUT_ENCODING.toRegex(), "UTF-8") + result = result.replace(OS_PARAM_LANGUAGE, locale) + result = result.replace(OS_PARAM_OUTPUT_ENCODING, "UTF-8") // Replace any optional parameters result = result.replace(OS_PARAM_OPTIONAL.toRegex(), "") @@ -61,18 +61,23 @@ class SearchEngine internal constructor( } companion object { + // We are using string concatenation here to avoid the Kotlin compiler interpreting this + // as string templates. It is possible to escape the string accordingly. But this seems to + // be inconsistent between Kotlin versions. So to be safe we avoid this completely by + // constructing the strings manually. + // Parameters copied from nsSearchService.js - private const val MOZ_PARAM_LOCALE = "\\{moz:locale}" - private const val MOZ_PARAM_DIST_ID = "\\{moz:distributionID}" - private const val MOZ_PARAM_OFFICIAL = "\\{moz:official}" + private const val MOZ_PARAM_LOCALE = "{" + "moz:locale" + "}" + private const val MOZ_PARAM_DIST_ID = "{" + "moz:distributionID" + "}" + private const val MOZ_PARAM_OFFICIAL = "{" + "moz:official" + "}" // Supported OpenSearch parameters // See http://opensearch.a9.com/spec/1.1/querysyntax/#core - private const val OS_PARAM_USER_DEFINED = "\\{searchTerms\\??}" - private const val OS_PARAM_INPUT_ENCODING = "\\{inputEncoding\\??}" - private const val OS_PARAM_LANGUAGE = "\\{language\\??}" - private const val OS_PARAM_OUTPUT_ENCODING = "\\{outputEncoding\\??}" - private const val OS_PARAM_OPTIONAL = "\\{(?:\\w+:)?\\w+?}" + private const val OS_PARAM_USER_DEFINED = "{" + "searchTerms" + "}" + private const val OS_PARAM_INPUT_ENCODING = "{" + "inputEncoding" + "}" + private const val OS_PARAM_LANGUAGE = "{" + "language" + "}" + private const val OS_PARAM_OUTPUT_ENCODING = "{" + "outputEncoding" + "}" + private const val OS_PARAM_OPTIONAL = "\\{" + "(?:\\w+:)?\\w+?" + "\\}" private fun normalize(input: String): String { val trimmedInput = input.trim { it <= ' ' } diff --git a/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineManager.kt b/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineManager.kt index 34f88f50956..7f2c8472141 100644 --- a/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineManager.kt +++ b/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineManager.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.experimental.CommonPool import kotlinx.coroutines.experimental.Deferred import kotlinx.coroutines.experimental.async import kotlinx.coroutines.experimental.launch +import kotlinx.coroutines.experimental.runBlocking import mozilla.components.browser.search.provider.AssetsSearchEngineProvider import mozilla.components.browser.search.provider.SearchEngineProvider import mozilla.components.browser.search.provider.localization.LocaleSearchLocalizationProvider @@ -40,10 +41,10 @@ class SearchEngineManager( * will perform a load. */ @Synchronized - suspend fun getSearchEngines(context: Context): List { - deferredSearchEngines?.let { return it.await() } + fun getSearchEngines(context: Context): List { + deferredSearchEngines?.let { return runBlocking { it.await() } } - return load(context).await() + return runBlocking { load(context).await() } } /** @@ -52,16 +53,16 @@ class SearchEngineManager( * The default engine is the first engine loaded by the first provider passed to the * constructor of SearchEngineManager. * - * Optionally an identifier can be passed to this method (e.g. from the user's preferences). If + * Optionally a name can be passed to this method (e.g. from the user's preferences). If * a matching search engine was loaded then this search engine will be returned instead. */ @Synchronized - suspend fun getDefaultSearchEngine(context: Context, identifier: String = EMPTY): SearchEngine { + fun getDefaultSearchEngine(context: Context, name: String = EMPTY): SearchEngine { val searchEngines = getSearchEngines(context) - return when (identifier) { + return when (name) { EMPTY -> searchEngines[0] - else -> searchEngines.find { it.identifier == identifier } ?: searchEngines[0] + else -> searchEngines.find { it.name == name } ?: searchEngines[0] } } diff --git a/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineParser.kt b/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineParser.kt index bb190da9026..e84bd2cd18d 100644 --- a/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineParser.kt +++ b/components/browser/search/src/main/java/mozilla/components/browser/search/SearchEngineParser.kt @@ -22,7 +22,7 @@ import java.nio.charset.StandardCharsets /** * A very simple parser for search plugins. */ -internal class SearchEngineParser { +class SearchEngineParser { private class SearchEngineBuilder( private val identifier: String @@ -41,6 +41,10 @@ internal class SearchEngineParser { ) } + /** + * Loads a SearchEngine from the given path in assets and assigns + * it the given identifier. + */ @Throws(IOException::class) fun load(assetManager: AssetManager, identifier: String, path: String): SearchEngine { try { @@ -50,8 +54,12 @@ internal class SearchEngineParser { } } + /** + * Loads a SearchEngine from the given stream and assigns it the given + * identifier. + */ @Throws(IOException::class, XmlPullParserException::class) - internal fun load(identifier: String, stream: InputStream): SearchEngine { + fun load(identifier: String, stream: InputStream): SearchEngine { val builder = SearchEngineBuilder(identifier) val parser = XmlPullParserFactory.newInstance().newPullParser() diff --git a/components/browser/search/src/main/java/mozilla/components/browser/search/provider/AssetsSearchEngineProvider.kt b/components/browser/search/src/main/java/mozilla/components/browser/search/provider/AssetsSearchEngineProvider.kt index 11975a3ba5a..b03e85eea75 100644 --- a/components/browser/search/src/main/java/mozilla/components/browser/search/provider/AssetsSearchEngineProvider.kt +++ b/components/browser/search/src/main/java/mozilla/components/browser/search/provider/AssetsSearchEngineProvider.kt @@ -18,18 +18,32 @@ import org.json.JSONObject /** * SearchEngineProvider implementation to load the included search engines from assets. + * + * A SearchLocalizationProvider implementation is used to customize the returned search engines for + * the language and country of the user/device. + * + * Optionally SearchEngineFilter instances can be provided to remove unwanted search engines from + * the loaded list. + * + * Optionally additionalIdentifiers to be loaded can be specified. A search engine + * identifier corresponds to the search plugin XML file name (e.g. duckduckgo -> duckduckgo.xml). */ class AssetsSearchEngineProvider( private val localizationProvider: SearchLocalizationProvider, - private val filters: List = emptyList() + private val filters: List = emptyList(), + private val additionalIdentifiers: List = emptyList() ) : SearchEngineProvider { /** * Load search engines from this provider. */ override suspend fun loadSearchEngines(context: Context): List { - val searchEngineIdentifiers = loadAndFilterConfiguration(context) - return loadSearchEnginesFromList(context, searchEngineIdentifiers) + val searchEngineIdentifiers = mutableListOf().apply { + addAll(loadAndFilterConfiguration(context)) + addAll(additionalIdentifiers) + } + + return loadSearchEnginesFromList(context, searchEngineIdentifiers.distinct()) } private suspend fun loadSearchEnginesFromList( @@ -51,7 +65,7 @@ class AssetsSearchEngineProvider( deferredSearchEngines.forEach { val searchEngine = it.await() - if (shouldBeFiltered(searchEngine)) { + if (shouldBeFiltered(context, searchEngine)) { searchEngines.add(searchEngine) } } @@ -59,9 +73,9 @@ class AssetsSearchEngineProvider( return searchEngines } - private fun shouldBeFiltered(searchEngine: SearchEngine): Boolean { + private fun shouldBeFiltered(context: Context, searchEngine: SearchEngine): Boolean { filters.forEach { - if (!it.filter(searchEngine)) { + if (!it.filter(context, searchEngine)) { return false } } @@ -73,9 +87,7 @@ class AssetsSearchEngineProvider( assets: AssetManager, parser: SearchEngineParser, identifier: String - ): SearchEngine { - return parser.load(assets, identifier, "searchplugins/$identifier.xml") - } + ): SearchEngine = parser.load(assets, identifier, "searchplugins/$identifier.xml") private fun loadAndFilterConfiguration(context: Context): List { val config = context.assets.readJSONObject("search/list.json") diff --git a/components/browser/search/src/main/java/mozilla/components/browser/search/provider/filter/SearchEngineFilter.kt b/components/browser/search/src/main/java/mozilla/components/browser/search/provider/filter/SearchEngineFilter.kt index 70dd7783d5e..196b9d3331d 100644 --- a/components/browser/search/src/main/java/mozilla/components/browser/search/provider/filter/SearchEngineFilter.kt +++ b/components/browser/search/src/main/java/mozilla/components/browser/search/provider/filter/SearchEngineFilter.kt @@ -4,6 +4,7 @@ package mozilla.components.browser.search.provider.filter +import android.content.Context import mozilla.components.browser.search.SearchEngine /** @@ -15,5 +16,5 @@ interface SearchEngineFilter { * Returns true if the given search engine should be returned by the provider or false if this * search engine should be ignored. */ - fun filter(searchEngine: SearchEngine): Boolean + fun filter(context: Context, searchEngine: SearchEngine): Boolean } diff --git a/components/browser/search/src/test/java/mozilla/components/browser/search/SearchEngineManagerTest.kt b/components/browser/search/src/test/java/mozilla/components/browser/search/SearchEngineManagerTest.kt index 2d7a48512c5..f3dee6f5851 100644 --- a/components/browser/search/src/test/java/mozilla/components/browser/search/SearchEngineManagerTest.kt +++ b/components/browser/search/src/test/java/mozilla/components/browser/search/SearchEngineManagerTest.kt @@ -81,13 +81,16 @@ class SearchEngineManagerTest { fun `manager returns default engine with identifier if it exists`() { runBlocking { val provider = mockProvider(listOf( - mockSearchEngine("mozsearch"), - mockSearchEngine("google"), - mockSearchEngine("bing"))) + mockSearchEngine("mozsearch", "Mozilla Search"), + mockSearchEngine("google", "Google Search"), + mockSearchEngine("bing", "Bing Search"))) val manager = SearchEngineManager(listOf(provider)) - val default = manager.getDefaultSearchEngine(RuntimeEnvironment.application, "bing") + val default = manager.getDefaultSearchEngine( + RuntimeEnvironment.application, + "Bing Search") + assertEquals("bing", default.identifier) } } @@ -150,12 +153,15 @@ class SearchEngineManagerTest { } } - private fun mockSearchEngine(identifier: String): SearchEngine { + private fun mockSearchEngine( + identifier: String, + name: String = UUID.randomUUID().toString() + ): SearchEngine { val uri = Uri.parse("https://${UUID.randomUUID()}.example.org") return SearchEngine( identifier, - UUID.randomUUID().toString(), + name, mock(Bitmap::class.java), listOf(uri)) } diff --git a/components/browser/search/src/test/java/mozilla/components/browser/search/provider/AssetsSearchEngineProviderTest.kt b/components/browser/search/src/test/java/mozilla/components/browser/search/provider/AssetsSearchEngineProviderTest.kt index 25c79c477c2..126ead87c45 100644 --- a/components/browser/search/src/test/java/mozilla/components/browser/search/provider/AssetsSearchEngineProviderTest.kt +++ b/components/browser/search/src/test/java/mozilla/components/browser/search/provider/AssetsSearchEngineProviderTest.kt @@ -4,6 +4,7 @@ package mozilla.components.browser.search.provider +import android.content.Context import kotlinx.coroutines.experimental.runBlocking import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.search.provider.filter.SearchEngineFilter @@ -42,7 +43,7 @@ class AssetsSearchEngineProviderTest { val filter = object : SearchEngineFilter { private val exclude = listOf("yahoo", "bing", "ddg") - override fun filter(searchEngine: SearchEngine): Boolean { + override fun filter(cotext: Context, searchEngine: SearchEngine): Boolean { return !exclude.contains(searchEngine.identifier) } } @@ -134,6 +135,35 @@ class AssetsSearchEngineProviderTest { assertEquals(6, searchEngines.size) } + @Test + fun `provider loads additional identifiers`() { + val usProvider = object : SearchLocalizationProvider() { + override val country: String = "US" + override val language = "en" + override val region: String? = null + } + + // Loading "en-US" without additional identifiers + runBlocking { + val provider = AssetsSearchEngineProvider(usProvider) + val searchEngines = provider.loadSearchEngines(RuntimeEnvironment.application) + + assertEquals(6, searchEngines.size) + assertContainsNotSearchEngine("duckduckgo", searchEngines) + } + + // Load "en-US" with "duckduckgo" added + runBlocking { + val provider = AssetsSearchEngineProvider( + usProvider, + additionalIdentifiers = listOf("duckduckgo")) + val searchEngines = provider.loadSearchEngines(RuntimeEnvironment.application) + + assertEquals(7, searchEngines.size) + assertContainsSearchEngine("duckduckgo", searchEngines) + } + } + private fun assertContainsSearchEngine(identifier: String, searchEngines: List) { searchEngines.forEach { if (identifier == it.identifier) {