For #191 - refactors SearchSuggestionClient to re-throw exceptions

mozilla-mobile · Jul 20, 2018 · e383431 · e383431
1 parent 3f79dde
commit e383431
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 20 deletions.
diff --git a/...rch/src/main/java/mozilla/components/browser/search/suggestions/SearchSuggestionClient.kt b/...rch/src/main/java/mozilla/components/browser/search/suggestions/SearchSuggestionClient.kt
@@ -5,6 +5,8 @@
 package mozilla.components.browser.search.suggestions
 
 import mozilla.components.browser.search.SearchEngine
+import org.json.JSONException
+import java.io.IOException
 
 /**
  * Async function responsible for taking a URL and returning the results
@@ -14,25 +16,41 @@ typealias SearchSuggestionFetcher = suspend (url: String) -> String?
 /**
  *  Provides an interface to get search suggestions from a given SearchEngine.
  */
-class SearchSuggestionClient private constructor(
+class SearchSuggestionClient(
     private val searchEngine: SearchEngine,
     private val fetcher: SearchSuggestionFetcher
 ) {
-    companion object {
-        fun create(searchEngine: SearchEngine, fetcher: SearchSuggestionFetcher): SearchSuggestionClient? {
-            if (!searchEngine.canProvideSearchSuggestions) { return null }
 
-            return SearchSuggestionClient(searchEngine, fetcher)
+    /**
+     * Exception types for errors caught while getting a list of suggestions
+     */
+    class FetchException : Exception("There was a problem fetching suggestions")
+    class ResponseParserException : Exception("There was a problem parsing the suggestion response")
+
+    init {
+        if (!searchEngine.canProvideSearchSuggestions) {
+            throw IllegalArgumentException("SearchEngine does not support search suggestions!")
         }
     }
 
     /**
      * Gets search suggestions for a given query
      */
     suspend fun getSuggestions(query: String): List<String>? {
-        val suggestionsURL = searchEngine.buildSuggestionsURL(query) ?: return null
+        val suggestionsURL = searchEngine.buildSuggestionsURL(query)
+
         val parser = selectResponseParser(searchEngine)
 
-        return fetcher(suggestionsURL)?.let { parser(it) }
+        val suggestionResults = try {
+            suggestionsURL?.let { fetcher(it) }
+        } catch (_: IOException) {
+            throw FetchException()
+        }
+
+        return try {
+            suggestionResults?.let(parser)
+        } catch (_: JSONException) {
+            throw ResponseParserException()
+        }
     }
 }
diff --git a/...onents/browser/search/src/test/java/mozilla/components/browser/search/SearchEngineTest.kt b/...onents/browser/search/src/test/java/mozilla/components/browser/search/SearchEngineTest.kt
@@ -6,7 +6,9 @@ package mozilla.components.browser.search
 
 import android.graphics.Bitmap
 import android.net.Uri
-import org.junit.Assert.*
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Assert.assertFalse
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.mock

diff --git a/...src/test/java/mozilla/components/browser/search/suggestions/SearchSuggestionClientTest.kt b/...src/test/java/mozilla/components/browser/search/suggestions/SearchSuggestionClientTest.kt
@@ -6,13 +6,12 @@ package mozilla.components.browser.search.suggestions
 
 import kotlinx.coroutines.experimental.runBlocking
 import mozilla.components.browser.search.SearchEngineParser
-import org.json.JSONException
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.RuntimeEnvironment
+import java.io.IOException
 
 @RunWith(RobolectricTestRunner::class)
 class SearchSuggestionClientTest {
@@ -28,7 +27,7 @@ class SearchSuggestionClientTest {
                 RuntimeEnvironment.application.assets,
                 "google", "searchplugins/google-nocodes.xml")
 
-        val client = SearchSuggestionClient.create(searchEngine, GOOGLE_MOCK_RESPONSE) ?: return
+        val client = SearchSuggestionClient(searchEngine, GOOGLE_MOCK_RESPONSE)
 
         runBlocking {
             val results = client.getSuggestions("firefox")
@@ -44,7 +43,7 @@ class SearchSuggestionClientTest {
                 RuntimeEnvironment.application.assets,
                 "google", "searchplugins/qwant.xml")
 
-        val client = SearchSuggestionClient.create(searchEngine, QWANT_MOCK_RESPONSE) ?: return
+        val client = SearchSuggestionClient(searchEngine, QWANT_MOCK_RESPONSE)
 
         runBlocking {
             val results = client.getSuggestions("firefox")
@@ -54,26 +53,38 @@ class SearchSuggestionClientTest {
         }
     }
 
-    @Test(expected = JSONException::class)
-    fun `Check that a bad response will throw an exception`() {
+    @Test(expected = SearchSuggestionClient.ResponseParserException::class)
+    fun `Check that a bad response will throw a parser exception`() {
         val searchEngine = SearchEngineParser().load(
                 RuntimeEnvironment.application.assets,
                 "google", "searchplugins/google-nocodes.xml")
 
-        val client = SearchSuggestionClient.create(searchEngine, SERVER_ERROR_RESPONSE) ?: return
+        val client = SearchSuggestionClient(searchEngine, SERVER_ERROR_RESPONSE)
 
         runBlocking {
             client.getSuggestions("firefox")
         }
     }
 
-    @Test
-    fun `Check that a search engine without a suggestURI cant create a SearchSuggestionClient`() {
+    @Test(expected = SearchSuggestionClient.FetchException::class)
+    fun `Check that an exception in the suggestionFetcher will re-throw an IOException`() {
+        val searchEngine = SearchEngineParser().load(
+                RuntimeEnvironment.application.assets,
+                "google", "searchplugins/google-nocodes.xml")
+
+        val client = SearchSuggestionClient(searchEngine, { throw IOException() })
+
+        runBlocking {
+            client.getSuggestions("firefox")
+        }
+    }
+
+    @Test(expected = IllegalArgumentException::class)
+    fun `Check that a search engine without a suggestURI will throw an exception`() {
         val searchEngine = SearchEngineParser().load(
                 RuntimeEnvironment.application.assets,
-                "google", "searchplugins/drae.xml")
+                "drae", "searchplugins/drae.xml")
 
-        val client = SearchSuggestionClient.create(searchEngine, { "no-op" })
-        assertNull(client)
+        val client = SearchSuggestionClient(searchEngine, { "no-op" })
     }
 }