🐛 [Bug] [Stetho] Fix bug in stetho module (kittinunf#602)

buildSrc/src/main/kotlin/Constants.kt

@@ -116,7 +116,7 @@ object RxJava {
 
 // Lint
 object Ktlint {
-    const val version = "1.20.1"
+    const val version = "1.21.0"
     const val plugin = "org.jmailen.kotlinter"
 }
 

fuel-stetho/src/main/java/com/github/kittinunf/fuel/stetho/StethoHook.kt

@@ -7,23 +7,36 @@ import com.github.kittinunf.fuel.core.Request
 import java.io.IOException
 import java.io.InputStream
 import java.net.HttpURLConnection
+import java.util.UUID
+import java.util.concurrent.ConcurrentHashMap
 
 class StethoHook(val friendlyName: String = "StethoFuelConnectionManager") : Client.Hook {
 
-    val stetho = StethoURLConnectionManager(friendlyName)
+    val stethoCache = ConcurrentHashMap<UUID, StethoURLConnectionManager>()
 
     override fun preConnect(connection: HttpURLConnection, request: Request) {
+        // attach UUID tag for the request
+        request.tag(UUID.randomUUID())
+
+        val stetho = stethoCache.getOrPut(request.getTag(UUID::class)!!) {
+            StethoURLConnectionManager(friendlyName)
+        }
         stetho.preConnect(connection, ByteArrayRequestEntity(request.body.toByteArray()))
     }
 
-    override fun interpretResponseStream(inputStream: InputStream): InputStream =
-        stetho.interpretResponseStream(inputStream)
+    override fun postConnect(request: Request) {
+        stethoCache[request.getTag(UUID::class)]?.postConnect()
+    }
 
-    override fun postConnect() {
-        stetho.postConnect()
+    // means the connection ended with success, allow stetho to intercept response, remove it from the cache
+    override fun interpretResponseStream(request: Request, inputStream: InputStream): InputStream {
+        val stetho = stethoCache.remove(request.getTag(UUID::class))
+        return stetho?.interpretResponseStream(inputStream) ?: inputStream
     }
 
-    override fun httpExchangeFailed(exception: IOException) {
-        stetho.httpExchangeFailed(exception)
+    // means the connection ended with failure, allow stetho to intercept failure response, remove it from the cache
+    override fun httpExchangeFailed(request: Request, exception: IOException) {
+        val stetho = stethoCache.remove(request.getTag(UUID::class))
+        stetho?.httpExchangeFailed(exception)
     }
 }

fuel-stetho/src/test/java/com/github/kittinunf/fuel/StethoHookTest.kt

@@ -1,12 +1,15 @@
 package com.github.kittinunf.fuel
 
 import com.github.kittinunf.fuel.stetho.StethoHook
+import com.github.kittinunf.fuel.test.MockHttpTestCase
 import org.hamcrest.CoreMatchers.equalTo
-import org.hamcrest.CoreMatchers.notNullValue
 import org.hamcrest.MatcherAssert.assertThat
 import org.junit.Test
+import java.io.ByteArrayInputStream
+import java.net.HttpURLConnection
+import java.net.URL
 
-class StethoHookTest {
+class StethoHookTest : MockHttpTestCase() {
 
     private val hook = StethoHook()
 
@@ -16,7 +19,36 @@ class StethoHookTest {
     }
 
     @Test
-    fun underlyObjectNotNull() {
-        assertThat(hook.stetho, notNullValue())
+    fun stethoConnectionIsCreated() {
+        val cache = hook.stethoCache
+
+        assertThat(cache.size, equalTo(0))
+
+        hook.preConnect(URL("http://foo.bar").openConnection() as HttpURLConnection, mock.path("").httpGet())
+
+        assertThat(cache.size, equalTo(1))
+    }
+
+    @Test
+    fun stethoLifecycleIsCallingCorrectly() {
+        val cache = hook.stethoCache
+
+        val r1 = mock.path("").httpGet()
+        val r2 = mock.path("").httpPut()
+        val r3 = mock.path("").httpDelete()
+
+        hook.preConnect(URL("http://foo.bar").openConnection() as HttpURLConnection, r1)
+        hook.preConnect(URL("http://foo.bar").openConnection() as HttpURLConnection, r2)
+        hook.preConnect(URL("http://foo.bar").openConnection() as HttpURLConnection, r3)
+
+        hook.postConnect(r2)
+        hook.postConnect(r3)
+        hook.postConnect(r1)
+
+        hook.interpretResponseStream(r3, ByteArrayInputStream(byteArrayOf()))
+        hook.interpretResponseStream(r2, ByteArrayInputStream(byteArrayOf()))
+        hook.interpretResponseStream(r1, ByteArrayInputStream(byteArrayOf()))
+
+        assertThat(cache.size, equalTo(0))
     }
 }

fuel/src/main/kotlin/com/github/kittinunf/fuel/core/Client.kt

@@ -10,8 +10,8 @@ interface Client {
 
     interface Hook {
         fun preConnect(connection: HttpURLConnection, request: Request)
-        fun interpretResponseStream(inputStream: InputStream): InputStream
-        fun postConnect()
-        fun httpExchangeFailed(exception: IOException)
+        fun postConnect(request: Request)
+        fun interpretResponseStream(request: Request, inputStream: InputStream): InputStream
+        fun httpExchangeFailed(request: Request, exception: IOException)
     }
-}
+}

fuel/src/main/kotlin/com/github/kittinunf/fuel/core/DefaultHook.kt

@@ -9,13 +9,13 @@ internal class DefaultHook : Client.Hook {
         // no-op
     }
 
-    override fun interpretResponseStream(inputStream: InputStream): InputStream = inputStream
+    override fun interpretResponseStream(request: Request, inputStream: InputStream): InputStream = inputStream
 
-    override fun postConnect() {
+    override fun postConnect(request: Request) {
         // no-op
     }
 
-    override fun httpExchangeFailed(exception: IOException) {
+    override fun httpExchangeFailed(request: Request, exception: IOException) {
         // no-op
     }
-}
+}

fuel/src/main/kotlin/com/github/kittinunf/fuel/core/Request.kt

@@ -5,9 +5,11 @@ import java.io.File
 import java.io.InputStream
 import java.net.URL
 import java.nio.charset.Charset
+import kotlin.reflect.KClass
 
 typealias Parameters = List<Pair<String, Any?>>
 typealias RequestFeatures = MutableMap<String, Request>
+typealias Tags = MutableMap<KClass<*>, Any>
 
 interface Request : RequestFactory.RequestConvertible {
     val method: Method
@@ -470,4 +472,26 @@ interface Request : RequestFactory.RequestConvertible {
      * @return self
      */
     fun validate(validator: ResponseValidator): Request
+
+    /**
+     * Attach tag to the request
+     *
+     * @note tag is a generic purpose tagging for Request. This can be used to attach arbitrarily object to the Request instance.
+     * @note Tags internally is represented as hashMap that uses class as a key.
+     *
+     * @param t [Any]
+     * @return [Request] the modified request
+     */
+    fun tag(t: Any): Request
+
+    /**
+     * Return corresponding tag from the request
+     *
+     * @note tag is a generic purpose tagging for Request. This can be used to attach arbitrarily object to the Request instance.
+     * @note Tags internally is represented as hashMap that uses class as a key.
+     *
+     * @param clazz [KClass]
+     * @return [Any] previously attached tag if any, null otherwise
+     */
+    fun <T : Any> getTag(clazz: KClass<T>): T?
 }

fuel/src/main/kotlin/com/github/kittinunf/fuel/core/requests/DefaultRequest.kt

@@ -18,6 +18,7 @@ import com.github.kittinunf.fuel.core.ResponseHandler
 import com.github.kittinunf.fuel.core.ResponseResultHandler
 import com.github.kittinunf.fuel.core.ResponseValidator
 import com.github.kittinunf.fuel.core.ResultHandler
+import com.github.kittinunf.fuel.core.Tags
 import com.github.kittinunf.fuel.core.deserializers.ByteArrayDeserializer
 import com.github.kittinunf.fuel.core.deserializers.StringDeserializer
 import com.github.kittinunf.fuel.core.response
@@ -28,14 +29,16 @@ import java.io.InputStream
 import java.net.URL
 import java.net.URLConnection
 import java.nio.charset.Charset
+import kotlin.reflect.KClass
 
 data class DefaultRequest(
     override val method: Method,
     override var url: URL,
     override val headers: Headers = Headers(),
     override var parameters: Parameters = listOf(),
     internal var _body: Body = DefaultBody(),
-    override val enabledFeatures: RequestFeatures = mutableMapOf()
+    override val enabledFeatures: RequestFeatures = mutableMapOf(),
+    private val tags: Tags = mutableMapOf()
 ) : Request {
     override lateinit var executionOptions: RequestExecutionOptions
     override val body: Body get() = _body
@@ -241,7 +244,7 @@ data class DefaultRequest(
         body(bytes = body.toByteArray(charset), charset = charset)
             .let {
                 if (header(Headers.CONTENT_TYPE).lastOrNull().isNullOrBlank())
-                    header(Headers.CONTENT_TYPE, "text/plain; charset=${charset.name()}")
+                header(Headers.CONTENT_TYPE, "text/plain; charset=${charset.name()}")
                 else it
             }
 
@@ -375,7 +378,7 @@ data class DefaultRequest(
      * Overwrite [RequestExecutionOptions] response validator block
      *
      * @note The default responseValidator is to throw [com.github.kittinunf.fuel.core.HttpException]
-     * if the response http status code is not in the range of (100 - 399) which should consider as failure response
+     * @note if the response http status code is not in the range of (100 - 399) which should consider as failure response
      *
      * @param validator [ResponseValidator]
      * @return [Request] the modified request
@@ -384,6 +387,30 @@ data class DefaultRequest(
         it.executionOptions.responseValidator = validator
     }
 
+    /**
+     * Attach tag to the request
+     *
+     * @note tag is a generic purpose tagging for Request. This can be used to attach arbitrarily object to the Request instance.
+     * @note Tags internally is represented as hashMap that uses class as a key.
+     *
+     * @param t [Any]
+     * @return [Request] the modified request
+     */
+    override fun tag(t: Any) = request.also {
+        tags[t::class] = t
+    }
+
+    /**
+     * Return corresponding tag from the request
+     *
+     * @note tag is a generic purpose tagging for Request. This can be used to attach arbitrarily object to the Request instance.
+     * @note Tags internally is represented as hashMap that uses class as a key.
+     *
+     * @param clazz [KClass]
+     * @return [Any] previously attached tag if any, null otherwise
+     */
+    override fun <T : Any> getTag(clazz: KClass<T>) = tags[clazz] as? T
+
     override val request: Request get() = this
 
     /**

fuel/src/main/kotlin/com/github/kittinunf/fuel/toolbox/HttpClient.kt

@@ -1,14 +1,14 @@
 package com.github.kittinunf.fuel.toolbox
 
 import com.github.kittinunf.fuel.core.Client
-import com.github.kittinunf.fuel.core.requests.DefaultBody
 import com.github.kittinunf.fuel.core.FuelError
 import com.github.kittinunf.fuel.core.FuelManager
 import com.github.kittinunf.fuel.core.HeaderName
 import com.github.kittinunf.fuel.core.Headers
 import com.github.kittinunf.fuel.core.Method
 import com.github.kittinunf.fuel.core.Request
 import com.github.kittinunf.fuel.core.Response
+import com.github.kittinunf.fuel.core.requests.DefaultBody
 import com.github.kittinunf.fuel.core.requests.isCancelled
 import com.github.kittinunf.fuel.util.ProgressInputStream
 import com.github.kittinunf.fuel.util.ProgressOutputStream
@@ -126,7 +126,7 @@ class HttpClient(
     private fun retrieveResponse(request: Request, connection: HttpURLConnection): Response {
         ensureRequestActive(request, connection)
 
-        hook.postConnect()
+        hook.postConnect(request)
 
         val headers = Headers.from(connection.headerFields)
         val transferEncoding = headers[Headers.TRANSFER_ENCODING].flatMap { it.split(',') }.map { it.trim() }
@@ -172,7 +172,7 @@ class HttpClient(
             contentLength = -1
         }
 
-        val contentStream = dataStream(connection)?.decode(transferEncoding) ?: ByteArrayInputStream(ByteArray(0))
+        val contentStream = dataStream(request, connection)?.decode(transferEncoding) ?: ByteArrayInputStream(ByteArray(0))
         val inputStream = if (shouldDecode && contentEncoding != null) contentStream.decode(contentEncoding) else contentStream
         val cancellationConnection = WeakReference<HttpURLConnection>(connection)
         val progressStream = ProgressInputStream(
@@ -197,18 +197,18 @@ class HttpClient(
         )
     }
 
-    private fun dataStream(connection: HttpURLConnection): InputStream? {
+    private fun dataStream(request: Request, connection: HttpURLConnection): InputStream? {
         return try {
             try {
-                val inputStream = hook.interpretResponseStream(connection.inputStream)
+                val inputStream = hook.interpretResponseStream(request, connection.inputStream)
                 BufferedInputStream(inputStream)
             } catch (_: IOException) {
                 // The InputStream SHOULD be closed, but just in case the backing implementation is faulty, this ensures
                 // the InputStream ís actually always closed.
                 try { connection.inputStream?.close() } catch (_: IOException) {}
 
                 connection.errorStream?.let {
-                    BufferedInputStream(hook.interpretResponseStream(it))
+                    BufferedInputStream(hook.interpretResponseStream(request, it))
                 }
             } finally {
                 // We want the stream to live. Closing the stream is handled by Deserialize
@@ -218,7 +218,7 @@ class HttpClient(
             // ErrorStream ís actually always closed.
             try { connection.errorStream?.close() } catch (_: IOException) {}
 
-            hook.httpExchangeFailed(exception)
+            hook.httpExchangeFailed(request, exception)
 
             ByteArrayInputStream(exception.message?.toByteArray() ?: ByteArray(0))
         } finally {
@@ -279,4 +279,4 @@ class HttpClient(
         private val SUPPORTED_DECODING = listOf("gzip", "deflate; q=0.5")
         private fun coerceMethod(method: Method) = if (method == Method.PATCH) Method.POST else method
     }
-}
+}

fuel/src/test/kotlin/com/github/kittinunf/fuel/RequestTest.kt

@@ -22,8 +22,10 @@ import org.junit.Test
 import org.mockserver.model.BinaryBody
 import java.net.HttpURLConnection
 import java.util.Random
+import java.util.UUID
 
 class RequestTest : MockHttpTestCase() {
+
     private val manager: FuelManager by lazy { FuelManager() }
 
     class PathStringConvertibleImpl(url: String) : RequestFactory.PathStringConvertible {
@@ -220,8 +222,8 @@ class RequestTest : MockHttpTestCase() {
         )
 
         val (request, response, result) = manager.request(Method.POST, mock.path("post"))
-            .jsonBody(body)
-            .responseString()
+                .jsonBody(body)
+                .responseString()
         val (data, error) = result
 
         val string = data as String
@@ -505,4 +507,21 @@ class RequestTest : MockHttpTestCase() {
         assertEquals(JSONArray("[\"$lionel\"]").toString(), query.getJSONArray("bar").toString())
         assertEquals(list.toList(), query.getJSONArray("foo[]").map { it.toString() })
     }
+
+    @Test
+    fun tagRequest() {
+        val t1 = "tag"
+        val t2 = 5
+        val t3 = UUID.randomUUID()
+
+        val (req, _) = mock.path("get").httpGet().tag(t1).tag(t2).tag(t3).response()
+
+        assertThat(req.getTag(String::class), equalTo(t1))
+        assertThat(req.getTag(Int::class), equalTo(t2))
+        assertThat(req.getTag(UUID::class), equalTo(t3))
+
+        val (anotherReq, _) = mock.path("get").httpGet().response()
+
+        assertThat(anotherReq.getTag(String::class), nullValue())
+    }
 }

fuel/src/test/kotlin/com/github/kittinunf/fuel/toolbox/HttpClientTest.kt

@@ -27,13 +27,13 @@ class HttpClientTest : MockHttpTestCase() {
             // no-op
         }
 
-        override fun interpretResponseStream(inputStream: InputStream): InputStream = inputStream
+        override fun interpretResponseStream(request: Request, inputStream: InputStream): InputStream = inputStream
 
-        override fun postConnect() {
+        override fun postConnect(request: Request) {
             // no-op
         }
 
-        override fun httpExchangeFailed(exception: IOException) {
+        override fun httpExchangeFailed(request: Request, exception: IOException) {
             // no-op
         }
     }
@@ -184,4 +184,4 @@ class HttpClientTest : MockHttpTestCase() {
         val client = request.executionOptions.client as HttpClient
         assertThat(client.hook, instanceOf(TestHook::class.java))
     }
-}
+}