Issue #69: Add additional functionality for integration into Focus.

Author: pocmo

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 <= ' ' }

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<SearchEngine> {
-        deferredSearchEngines?.let { return it.await() }
+    fun getSearchEngines(context: Context): List<SearchEngine> {
+        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]
         }
     }
 

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 <code>SearchEngine</code> from the given <code>path</code> in assets and assigns
+     * it the given <code>identifier</code>.
+     */
     @Throws(IOException::class)
     fun load(assetManager: AssetManager, identifier: String, path: String): SearchEngine {
         try {
@@ -50,8 +54,12 @@ internal class SearchEngineParser {
         }
     }
 
+    /**
+     * Loads a <code>SearchEngine</code> from the given <code>stream</code> and assigns it the given
+     * <code>identifier</code>.
+     */
     @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()

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 <code>additionalIdentifiers</code> 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<SearchEngineFilter> = emptyList()
+    private val filters: List<SearchEngineFilter> = emptyList(),
+    private val additionalIdentifiers: List<String> = emptyList()
 ) : SearchEngineProvider {
 
     /**
      * Load search engines from this provider.
      */
     override suspend fun loadSearchEngines(context: Context): List<SearchEngine> {
-        val searchEngineIdentifiers = loadAndFilterConfiguration(context)
-        return loadSearchEnginesFromList(context, searchEngineIdentifiers)
+        val searchEngineIdentifiers = mutableListOf<String>().apply {
+            addAll(loadAndFilterConfiguration(context))
+            addAll(additionalIdentifiers)
+        }
+
+        return loadSearchEnginesFromList(context, searchEngineIdentifiers.distinct())
     }
 
     private suspend fun loadSearchEnginesFromList(
@@ -51,17 +65,17 @@ class AssetsSearchEngineProvider(
 
         deferredSearchEngines.forEach {
             val searchEngine = it.await()
-            if (shouldBeFiltered(searchEngine)) {
+            if (shouldBeFiltered(context, searchEngine)) {
                 searchEngines.add(searchEngine)
             }
         }
 
         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<String> {
         val config = context.assets.readJSONObject("search/list.json")

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
 }

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))
     }

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<SearchEngine>) {
         searchEngines.forEach {
             if (identifier == it.identifier) {