Improve Test on Fuel.request, HttpUrlFetcher and fillURLWithParameters

kittinunf · May 21, 2024 · 7a3a7bb · 7a3a7bb
1 parent 28be807
commit 7a3a7bb
Show file tree

Hide file tree

Showing 10 changed files with 61 additions and 17 deletions.
diff --git a/fuel/src/appleMain/kotlin/fuel/HttpUrlFetcher.kt b/fuel/src/appleMain/kotlin/fuel/HttpUrlFetcher.kt
@@ -43,7 +43,7 @@ internal class HttpUrlFetcher(private val sessionConfiguration: NSURLSessionConf
             request.body?.let {
                 setHTTPBody(it.encode())
             }
-            request.headers?.forEach {
+            request.headers.forEach {
                 setValue(it.value, it.key)
             }
             setCachePolicy(NSURLRequestReloadIgnoringCacheData)

diff --git a/fuel/src/commonMain/kotlin/fuel/Fuels.kt b/fuel/src/commonMain/kotlin/fuel/Fuels.kt
@@ -87,6 +87,6 @@ public suspend fun Fuel.request(convertible: RequestConvertible): HttpResponse {
         parameters = request.parameters
         method = request.method
         body = request.body
-        headers = request.headers ?: emptyMap()
+        headers = request.headers
     }
 }
diff --git a/fuel/src/commonMain/kotlin/fuel/Request.kt b/fuel/src/commonMain/kotlin/fuel/Request.kt
@@ -5,7 +5,7 @@ public typealias Parameters = List<Pair<String, String>>
 public class Request(
     public val url: String,
     public val parameters: Parameters?,
-    public val headers: Map<String, String>?,
+    public val headers: Map<String, String>,
     public val body: String?,
     public val method: String?
 ) {

diff --git a/fuel/src/commonTest/kotlin/fuel/ParametersTest.kt b/fuel/src/commonTest/kotlin/fuel/ParametersTest.kt
@@ -0,0 +1,30 @@
+package fuel
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class ParametersTest {
+    @Test
+    fun parametersWithQuestionMarkOnURL() {
+        val test = "http://example.com?".fillURLWithParameters(listOf("test" to "url"))
+        assertEquals("http://example.com?&test=url", test)
+    }
+
+    @Test
+    fun parametersWithQuestionMarkOnParameter() {
+        val test = "http://example.com".fillURLWithParameters(listOf("?test" to "url"))
+        assertEquals("http://example.com?%3Ftest=url", test)
+    }
+
+    @Test
+    fun parametersWithEmptyParameter() {
+        val test = "http://example.com".fillURLWithParameters(listOf())
+        assertEquals("http://example.com?", test)
+    }
+
+    @Test
+    fun parametersWithQuestionMarkAndEmptyParameter() {
+        val test = "http://example.com?".fillURLWithParameters(listOf())
+        assertEquals("http://example.com?", test)
+    }
+}
diff --git a/fuel/src/commonTest/kotlin/fuel/RequestTest.kt b/fuel/src/commonTest/kotlin/fuel/RequestTest.kt
@@ -2,14 +2,15 @@ package fuel
 
 import kotlin.test.Test
 import kotlin.test.assertEquals
-internal class RequestTest {
+
+class RequestTest {
     @Test
     fun testAdd_headers() {
         val request = Request.Builder().apply {
             url = "http://example.com"
             headers = mapOf("X-Test" to "true")
         }.build()
-        assertEquals("true", request.headers?.get("X-Test"))
+        assertEquals("true", request.headers["X-Test"])
     }
 
     @Test

diff --git a/fuel/src/jvmMain/kotlin/fuel/HttpUrlFetcher.kt b/fuel/src/jvmMain/kotlin/fuel/HttpUrlFetcher.kt
@@ -7,10 +7,11 @@ import okhttp3.Request
 
 internal class HttpUrlFetcher(private val callFactory: Lazy<Call.Factory>) {
     fun fetch(request: fuel.Request, builder: Request.Builder): Call {
-        val urlString = request.parameters?.let {
-            request.url.fillURLWithParameters(it)
-        } ?: request.url
-        builder.url(urlString)
+        if (request.parameters != null) {
+            builder.url(request.url.fillURLWithParameters(request.parameters))
+        } else {
+            builder.url(request.url)
+        }
         return callFactory.value.newCall(builder.build())
     }
 }
diff --git a/fuel/src/jvmMain/kotlin/fuel/JVMHttpLoader.kt b/fuel/src/jvmMain/kotlin/fuel/JVMHttpLoader.kt
@@ -73,7 +73,7 @@ public class JVMHttpLoader(callFactoryLazy: Lazy<Call.Factory>) : HttpLoader {
     private fun createRequestBuilder(request: Request, method: String): Builder {
         val builder = Builder()
         with(builder) {
-            request.headers?.forEach {
+            request.headers.forEach {
                 addHeader(it.key, it.value)
             }
 

diff --git a/fuel/src/jvmTest/kotlin/fuel/HttpLoaderBuilderTest.kt b/fuel/src/jvmTest/kotlin/fuel/HttpLoaderBuilderTest.kt
@@ -13,7 +13,7 @@ import java.net.SocketTimeoutException
 import java.util.concurrent.TimeUnit
 import kotlin.test.assertNotNull
 
-internal class HttpLoaderBuilderTest {
+class HttpLoaderBuilderTest {
 
     private lateinit var mockWebServer: MockWebServer
 
@@ -38,9 +38,21 @@ internal class HttpLoaderBuilderTest {
         mockWebServer.shutdown()
     }
 
+    @Test
+    fun `default okhttp settings with parameter`() = runBlocking {
+        mockWebServer.enqueue(MockResponse().setBody("Hello World 3"))
+        val response = JVMHttpLoader().get {
+            url = mockWebServer.url("hello").toString()
+            parameters = listOf("foo" to "bar")
+        }.body.string()
+        assertEquals("Hello World 3", response)
+
+        mockWebServer.shutdown()
+    }
+
     @Test
     fun `setting connect timeouts`() = runBlocking {
-        mockWebServer.enqueue(MockResponse().setBody("Hello World"))
+        mockWebServer.enqueue(MockResponse().setBody("Hello World 4"))
 
         val httpLoader = FuelBuilder()
             .config {
@@ -50,19 +62,19 @@ internal class HttpLoaderBuilderTest {
         val response = httpLoader.get {
             url = mockWebServer.url("hello").toString()
         }.body.string()
-        assertEquals("Hello World", response)
+        assertEquals("Hello World 4", response)
     }
 
     @Test
     fun `setting call timeouts`() = runBlocking {
-        mockWebServer.enqueue(MockResponse().setBody("Hello World 2"))
+        mockWebServer.enqueue(MockResponse().setBody("Hello World 5"))
 
         val okhttp = OkHttpClient.Builder().callTimeout(30L, TimeUnit.MILLISECONDS).build()
         val httpLoader = FuelBuilder().config(okhttp).build()
         val response = httpLoader.get {
             url = mockWebServer.url("hello2").toString()
         }.body.string()
-        assertEquals("Hello World 2", response)
+        assertEquals("Hello World 5", response)
     }
 
     @Test

diff --git a/fuel/src/jvmTest/kotlin/fuel/HttpLoaderTest.kt b/fuel/src/jvmTest/kotlin/fuel/HttpLoaderTest.kt
@@ -9,7 +9,7 @@ import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
 
-internal class HttpLoaderTest {
+class HttpLoaderTest {
     private lateinit var httpLoader: HttpLoader
     private lateinit var mockWebServer: MockWebServer
 

diff --git a/fuel/src/jvmTest/kotlin/fuel/RoutingTest.kt b/fuel/src/jvmTest/kotlin/fuel/RoutingTest.kt
@@ -8,7 +8,7 @@ import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
 
-internal class RoutingTest {
+class RoutingTest {
 
     sealed class TestApi(private val host: String) : FuelRouting {
         override val basePath = this.host