Add custom result type for HttpFetcher

luebke-b1 · Jun 1, 2023 · 9ae4c89 · 9ae4c89
1 parent 87a1fd6
commit 9ae4c89
Show file tree

Hide file tree

Showing 10 changed files with 81 additions and 41 deletions.
diff --git a/...ery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigDiscovery.kt b/...ery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigDiscovery.kt
@@ -3,6 +3,8 @@ package app.k9mail.autodiscovery.autoconfig
 import app.k9mail.autodiscovery.api.AutoDiscovery
 import app.k9mail.autodiscovery.api.AutoDiscoveryResult
 import app.k9mail.autodiscovery.api.AutoDiscoveryRunnable
+import app.k9mail.autodiscovery.autoconfig.HttpFetchResult.ErrorResponse
+import app.k9mail.autodiscovery.autoconfig.HttpFetchResult.SuccessResponse
 import app.k9mail.core.common.mail.EmailAddress
 import app.k9mail.core.common.mail.toDomain
 import com.fsck.k9.logging.Timber
@@ -30,8 +32,13 @@ class AutoconfigDiscovery internal constructor(
 
     private suspend fun getAutoconfig(email: EmailAddress, autoconfigUrl: HttpUrl): AutoDiscoveryResult? {
         return try {
-            fetcher.fetch(autoconfigUrl)?.use { inputStream ->
-                parser.parseSettings(inputStream, email)
+            when (val fetchResult = fetcher.fetch(autoconfigUrl)) {
+                is SuccessResponse -> {
+                    fetchResult.inputStream.use { inputStream ->
+                        parser.parseSettings(inputStream, email)
+                    }
+                }
+                is ErrorResponse -> null
             }
         } catch (e: AutoconfigParserException) {
             Timber.d(e, "Failed to parse config from URL: %s", autoconfigUrl)

diff --git a/...scovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/HttpFetchResult.kt b/...scovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/HttpFetchResult.kt
@@ -0,0 +1,24 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import java.io.InputStream
+
+/**
+ * Result type for [HttpFetcher].
+ */
+internal sealed interface HttpFetchResult {
+    /**
+     * The HTTP request returned a success response.
+     *
+     * @param inputStream Contains the response body.
+     */
+    data class SuccessResponse(
+        val inputStream: InputStream,
+    ) : HttpFetchResult
+
+    /**
+     * The server returned an error response.
+     *
+     * @param code The HTTP status code.
+     */
+    data class ErrorResponse(val code: Int) : HttpFetchResult
+}
diff --git a/...todiscovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/HttpFetcher.kt b/...todiscovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/HttpFetcher.kt
@@ -1,8 +1,7 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import java.io.InputStream
 import okhttp3.HttpUrl
 
 internal interface HttpFetcher {
-    suspend fun fetch(url: HttpUrl): InputStream?
+    suspend fun fetch(url: HttpUrl): HttpFetchResult
 }
diff --git a/...config/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/MxLookupAutoconfigDiscovery.kt b/...config/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/MxLookupAutoconfigDiscovery.kt
@@ -3,6 +3,8 @@ package app.k9mail.autodiscovery.autoconfig
 import app.k9mail.autodiscovery.api.AutoDiscovery
 import app.k9mail.autodiscovery.api.AutoDiscoveryResult
 import app.k9mail.autodiscovery.api.AutoDiscoveryRunnable
+import app.k9mail.autodiscovery.autoconfig.HttpFetchResult.ErrorResponse
+import app.k9mail.autodiscovery.autoconfig.HttpFetchResult.SuccessResponse
 import app.k9mail.core.common.mail.EmailAddress
 import app.k9mail.core.common.mail.toDomain
 import app.k9mail.core.common.net.Domain
@@ -76,8 +78,13 @@ class MxLookupAutoconfigDiscovery internal constructor(
 
     private suspend fun getAutoconfig(email: EmailAddress, autoconfigUrl: HttpUrl): AutoDiscoveryResult? {
         return try {
-            fetcher.fetch(autoconfigUrl)?.use { inputStream ->
-                parser.parseSettings(inputStream, email)
+            when (val fetchResult = fetcher.fetch(autoconfigUrl)) {
+                is SuccessResponse -> {
+                    fetchResult.inputStream.use { inputStream ->
+                        parser.parseSettings(inputStream, email)
+                    }
+                }
+                is ErrorResponse -> null
             }
         } catch (e: AutoconfigParserException) {
             Timber.d(e, "Failed to parse config from URL: %s", autoconfigUrl)

diff --git a/...discovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/OkHttpFetcher.kt b/...discovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/OkHttpFetcher.kt
@@ -1,8 +1,6 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import com.fsck.k9.logging.Timber
 import java.io.IOException
-import java.io.InputStream
 import kotlin.coroutines.resume
 import kotlinx.coroutines.suspendCancellableCoroutine
 import okhttp3.Call
@@ -16,16 +14,7 @@ internal class OkHttpFetcher(
     private val okHttpClient: OkHttpClient,
 ) : HttpFetcher {
 
-    override suspend fun fetch(url: HttpUrl): InputStream? {
-        return try {
-            performHttpRequest(url)
-        } catch (e: IOException) {
-            Timber.d(e, "Error fetching URL: %s", url)
-            null
-        }
-    }
-
-    private suspend fun performHttpRequest(url: HttpUrl): InputStream? {
+    override suspend fun fetch(url: HttpUrl): HttpFetchResult {
         return suspendCancellableCoroutine { cancellableContinuation ->
             val request = Builder()
                 .url(url)
@@ -44,12 +33,16 @@ internal class OkHttpFetcher(
 
                 override fun onResponse(call: Call, response: Response) {
                     if (response.isSuccessful) {
-                        val inputStream = response.body?.byteStream()
-                        cancellableContinuation.resume(inputStream)
+                        val result = HttpFetchResult.SuccessResponse(
+                            inputStream = response.body!!.byteStream(),
+                        )
+                        cancellableContinuation.resume(result)
                     } else {
                         // We don't care about the body of error responses.
                         response.close()
-                        cancellableContinuation.resume(null)
+
+                        val result = HttpFetchResult.ErrorResponse(response.code)
+                        cancellableContinuation.resume(result)
                     }
                 }
             }

diff --git a/...autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigDiscoveryTest.kt b/...autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigDiscoveryTest.kt
@@ -24,7 +24,7 @@ class AutoconfigDiscoveryTest {
         val emailAddress = "user@domain.example".toEmailAddress()
         val autoconfigUrl = "https://autoconfig.domain.invalid/mail/config-v1.1.xml".toHttpUrl()
         urlProvider.addResult(listOf(autoconfigUrl))
-        fetcher.addResult("data")
+        fetcher.addSuccessResult("data")
         parser.addResult(MockAutoconfigParser.RESULT_ONE)
 
         val autoDiscoveryRunnables = discovery.initDiscovery(emailAddress)
@@ -50,8 +50,8 @@ class AutoconfigDiscoveryTest {
             ),
         )
         fetcher.apply {
-            addResult("data1")
-            addResult("data2")
+            addSuccessResult("data1")
+            addSuccessResult("data2")
         }
         parser.apply {
             addResult(MockAutoconfigParser.RESULT_ONE)

diff --git a/...scovery/autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/MockHttpFetcher.kt b/...scovery/autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/MockHttpFetcher.kt
@@ -1,6 +1,5 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import java.io.InputStream
 import okhttp3.HttpUrl
 
 internal class MockHttpFetcher : HttpFetcher {
@@ -9,13 +8,21 @@ internal class MockHttpFetcher : HttpFetcher {
     val callCount: Int
         get() = callArguments.size
 
-    private val results = mutableListOf<InputStream?>()
+    private val results = mutableListOf<HttpFetchResult>()
 
-    fun addResult(data: String?) {
-        results.add(data?.byteInputStream())
+    fun addSuccessResult(data: String) {
+        val result = HttpFetchResult.SuccessResponse(
+            inputStream = data.byteInputStream(),
+        )
+
+        results.add(result)
+    }
+
+    fun addErrorResult(code: Int) {
+        results.add(HttpFetchResult.ErrorResponse(code))
     }
 
-    override suspend fun fetch(url: HttpUrl): InputStream? {
+    override suspend fun fetch(url: HttpUrl): HttpFetchResult {
         callArguments.add(url)
 
         check(results.isNotEmpty()) { "MockHttpFetcher.fetch($url) called but no result provided" }

diff --git a/...ig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/MxLookupAutoconfigDiscoveryTest.kt b/...ig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/MxLookupAutoconfigDiscoveryTest.kt
@@ -32,7 +32,7 @@ class MxLookupAutoconfigDiscoveryTest {
         val emailAddress = "user@company.example".toEmailAddress()
         mxResolver.addResult("mx.emailprovider.example".toDomain())
         urlProvider.addResult(listOf("https://ispdb.invalid/emailprovider.example".toHttpUrl()))
-        fetcher.addResult("data")
+        fetcher.addSuccessResult("data")
         parser.addResult(MockAutoconfigParser.RESULT_ONE)
 
         val autoDiscoveryRunnables = discovery.initDiscovery(emailAddress)
@@ -61,8 +61,8 @@ class MxLookupAutoconfigDiscoveryTest {
             addResult(listOf("https://ispdb.invalid/emailprovider.example".toHttpUrl()))
         }
         fetcher.apply {
-            addResult(null)
-            addResult(null)
+            addErrorResult(404)
+            addErrorResult(404)
         }
 
         val autoDiscoveryRunnables = discovery.initDiscovery(emailAddress)

diff --git a/...overy/autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/OkHttpFetcherTest.kt b/...overy/autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/OkHttpFetcherTest.kt
@@ -1,9 +1,12 @@
 package app.k9mail.autodiscovery.autoconfig
 
+import assertk.all
+import assertk.assertFailure
 import assertk.assertThat
 import assertk.assertions.isEqualTo
-import assertk.assertions.isNull
-import kotlin.test.assertNotNull
+import assertk.assertions.isInstanceOf
+import assertk.assertions.prop
+import java.net.UnknownHostException
 import kotlinx.coroutines.test.runTest
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import okhttp3.OkHttpClient
@@ -19,9 +22,9 @@ class OkHttpFetcherTest {
         val nonExistentUrl =
             "https://autoconfig.domain.invalid/mail/config-v1.1.xml?emailaddress=test%40domain.example".toHttpUrl()
 
-        val inputStream = fetcher.fetch(nonExistentUrl)
-
-        assertThat(inputStream).isNull()
+        assertFailure {
+            fetcher.fetch(nonExistentUrl)
+        }.isInstanceOf<UnknownHostException>()
     }
 
     @Test
@@ -36,10 +39,10 @@ class OkHttpFetcherTest {
         }
         val url = server.url("/empty/")
 
-        val inputStream = fetcher.fetch(url)
+        val result = fetcher.fetch(url)
 
-        assertNotNull(inputStream) { inputStream ->
-            assertThat(inputStream.available()).isEqualTo(0)
+        assertThat(result).isInstanceOf<HttpFetchResult.SuccessResponse>().all {
+            prop(HttpFetchResult.SuccessResponse::inputStream).transform { it.available() }.isEqualTo(0)
         }
     }
 }
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -145,7 +145,7 @@ mockito-kotlin = "org.mockito.kotlin:mockito-kotlin:4.1.0"
 turbine = "app.cash.turbine:turbine:0.12.3"
 jdom2 = "org.jdom:jdom2:2.0.6.1"
 icu4j-charset = "com.ibm.icu:icu4j-charset:72.1"
-assertk = "com.willowtreeapps.assertk:assertk-jvm:0.26"
+assertk = "com.willowtreeapps.assertk:assertk-jvm:0.26.1"
 
 leakcanary-android = "com.squareup.leakcanary:leakcanary-android:2.9.1"