Handle successful responses with empty body

src/main/kotlin/com/haroldadmin/cnradapter/DeferredNetworkResponseAdapter.kt

@@ -1,9 +1,9 @@
 package com.haroldadmin.cnradapter
+
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.Deferred
 import okhttp3.ResponseBody
 import retrofit2.*
-import java.io.IOException
 import java.lang.reflect.Type
 
 /**
@@ -17,8 +17,8 @@ import java.lang.reflect.Type
  */
 
 internal class DeferredNetworkResponseAdapter<T : Any, U : Any>(
-        private val successBodyType: Type,
-        private val errorConverter: Converter<ResponseBody, U>
+    private val successBodyType: Type,
+    private val errorConverter: Converter<ResponseBody, U>
 ) : CallAdapter<T, Deferred<NetworkResponse<T, U>>> {
 
     /**
@@ -54,20 +54,7 @@ internal class DeferredNetworkResponseAdapter<T : Any, U : Any>(
             }
 
             override fun onResponse(call: Call<T>, response: Response<T>) {
-                val headers = response.headers()
-                val responseCode = response.code()
-                val body = response.body()
-
-                val networkResponse = if (body != null) {
-                    NetworkResponse.Success(body, headers)
-                } else {
-                    try {
-                        val convertedErrorBody = errorConverter.convert(response.errorBody())
-                        NetworkResponse.ServerError(convertedErrorBody, responseCode, headers)
-                    } catch (ex: Exception) {
-                        NetworkResponse.UnknownError(ex)
-                    }
-                }
+                val networkResponse = ResponseHandler.handle(response, successBodyType, errorConverter)
                 deferred.complete(networkResponse)
             }
         })

src/main/kotlin/com/haroldadmin/cnradapter/ErrorExtraction.kt

@@ -8,7 +8,7 @@ import java.io.IOException
 internal const val UNKNOWN_ERROR_RESPONSE_CODE = 520
 
 internal fun <E : Any> HttpException.extractFromHttpException(
-        errorConverter: Converter<ResponseBody, E>
+    errorConverter: Converter<ResponseBody, E>
 ): NetworkResponse.ServerError<E> {
     val error = response()?.errorBody()
     val responseCode = response()?.code() ?: UNKNOWN_ERROR_RESPONSE_CODE
@@ -28,7 +28,7 @@ internal fun <E : Any> HttpException.extractFromHttpException(
 }
 
 internal fun <S : Any, E : Any> Throwable.extractNetworkResponse(
-        errorConverter: Converter<ResponseBody, E>
+    errorConverter: Converter<ResponseBody, E>
 ): NetworkResponse<S, E> {
     return when (this) {
         is IOException -> NetworkResponse.NetworkError(this)

src/main/kotlin/com/haroldadmin/cnradapter/Extensions.kt

@@ -15,11 +15,11 @@ import kotlinx.coroutines.delay
  * @return The NetworkResponse value whether it be successful or failed after retrying
  */
 suspend inline fun <T : Any, U : Any> executeWithRetry(
-        times: Int = 10,
-        initialDelay: Long = 100, // 0.1 second
-        maxDelay: Long = 1000, // 1 second
-        factor: Double = 2.0,
-        block: suspend () -> NetworkResponse<T, U>
+    times: Int = 10,
+    initialDelay: Long = 100, // 0.1 second
+    maxDelay: Long = 1000, // 1 second
+    factor: Double = 2.0,
+    block: suspend () -> NetworkResponse<T, U>
 ): NetworkResponse<T, U> {
     var currentDelay = initialDelay
     repeat(times - 1) {

src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponse.kt

@@ -12,7 +12,11 @@ sealed class NetworkResponse<out T : Any, out U : Any> {
     /**
      * A request that resulted in a response with a non-2xx status code.
      */
-    data class ServerError<U : Any>(val body: U?, val code: Int, val headers: Headers? = null) : NetworkResponse<Nothing, U>()
+    data class ServerError<U : Any>(
+        val body: U?,
+        val code: Int,
+        val headers: Headers? = null
+    ) : NetworkResponse<Nothing, U>()
 
     /**
      * A request that didn't result in a response.
@@ -24,5 +28,5 @@ sealed class NetworkResponse<out T : Any, out U : Any> {
      *
      * An example of such an error is JSON parsing exception thrown by a serialization library.
      */
-    data class UnknownError(val error: Throwable): NetworkResponse<Nothing, Nothing>()
+    data class UnknownError(val error: Throwable) : NetworkResponse<Nothing, Nothing>()
 }

src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapter.kt

@@ -7,14 +7,14 @@ import retrofit2.Converter
 import java.lang.reflect.Type
 
 class NetworkResponseAdapter<S : Any, E : Any>(
-        private val successType: Type,
-        private val errorBodyConverter: Converter<ResponseBody, E>
+    private val successType: Type,
+    private val errorBodyConverter: Converter<ResponseBody, E>
 ) : CallAdapter<S, Call<NetworkResponse<S, E>>> {
 
     override fun responseType(): Type = successType
 
     override fun adapt(call: Call<S>): Call<NetworkResponse<S, E>> {
-        return NetworkResponseCall(call, errorBodyConverter)
+        return NetworkResponseCall(call, errorBodyConverter, successType)
     }
 }
 

src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponseCall.kt

@@ -2,41 +2,23 @@ package com.haroldadmin.cnradapter
 
 import okhttp3.Request
 import okhttp3.ResponseBody
-import okio.Timeout
 import retrofit2.Call
 import retrofit2.Callback
 import retrofit2.Converter
 import retrofit2.Response
+import java.lang.reflect.Type
 
 internal class NetworkResponseCall<S : Any, E : Any>(
-        private val backingCall: Call<S>,
-        private val errorConverter: Converter<ResponseBody, E>
+    private val backingCall: Call<S>,
+    private val errorConverter: Converter<ResponseBody, E>,
+    private val successBodyType: Type
 ) : Call<NetworkResponse<S, E>> {
 
     override fun enqueue(callback: Callback<NetworkResponse<S, E>>) = synchronized(this) {
         backingCall.enqueue(object : Callback<S> {
             override fun onResponse(call: Call<S>, response: Response<S>) {
-                val body = response.body()
-                val headers = response.headers()
-                val code = response.code()
-                val errorBody = response.errorBody()
-
-                if (response.isSuccessful) {
-                    if (body != null) {
-                        callback.onResponse(this@NetworkResponseCall, Response.success(NetworkResponse.Success(body, headers)))
-                    } else {
-                        // Response is successful but the body is null, so there's probably a server error here
-                        callback.onResponse(this@NetworkResponseCall, Response.success(NetworkResponse.ServerError(null, code, headers)))
-                    }
-                } else {
-                    val networkResponse = try {
-                        val convertedBody = errorConverter.convert(errorBody)
-                        NetworkResponse.ServerError(convertedBody, code, headers)
-                    } catch(ex: Exception) {
-                        NetworkResponse.UnknownError(ex)
-                    }
-                    callback.onResponse(this@NetworkResponseCall, Response.success(networkResponse))
-                }
+                val networkResponse = ResponseHandler.handle(response, successBodyType, errorConverter)
+                callback.onResponse(this@NetworkResponseCall, Response.success(networkResponse))
             }
 
             override fun onFailure(call: Call<S>, throwable: Throwable) {
@@ -50,7 +32,11 @@ internal class NetworkResponseCall<S : Any, E : Any>(
         backingCall.isExecuted
     }
 
-    override fun clone(): Call<NetworkResponse<S, E>> = NetworkResponseCall(backingCall.clone(), errorConverter)
+    override fun clone(): Call<NetworkResponse<S, E>> = NetworkResponseCall(
+        backingCall.clone(),
+        errorConverter,
+        successBodyType
+    )
 
     override fun isCanceled(): Boolean = synchronized(this) {
         backingCall.isCanceled
@@ -66,5 +52,5 @@ internal class NetworkResponseCall<S : Any, E : Any>(
 
     override fun request(): Request = backingCall.request()
 
-    override fun timeout() =  backingCall.timeout()
+    override fun timeout() = backingCall.timeout()
 }

src/main/kotlin/com/haroldadmin/cnradapter/ResponseHandler.kt

@@ -0,0 +1,61 @@
+package com.haroldadmin.cnradapter
+
+import okhttp3.ResponseBody
+import retrofit2.Converter
+import retrofit2.Response
+import java.lang.reflect.Type
+
+/**
+ * A utility object to centralize the logic of converting a [Response] to a [NetworkResponse]
+ */
+internal object ResponseHandler {
+
+    /**
+     * Converts the given [response] to a subclass of [NetworkResponse] based on different conditions.
+     *
+     * If the server response is successful with:
+     * => a non-empty body -> NetworkResponse.Success<S, E>
+     * => an empty body (and [successBodyType] is Unit) -> NetworkResponse.Success<Unit, E>
+     * => an empty body (and [successBodyType] is not Unit) -> NetworkResponse.ServerError<E>
+     *
+     * @param response Retrofit's response object supplied to the call adapter
+     * @param successBodyType A [Type] representing the success body
+     * @param errorConverter A retrofit converter to convert the error body into the error response type (E)
+     * @param S The success body type generic parameter
+     * @param E The error body type generic parameter
+     */
+    fun <S : Any, E : Any> handle(
+        response: Response<S>,
+        successBodyType: Type,
+        errorConverter: Converter<ResponseBody, E>
+    ): NetworkResponse<S, E> {
+        val body = response.body()
+        val headers = response.headers()
+        val code = response.code()
+        val errorBody = response.errorBody()
+
+        return if (response.isSuccessful) {
+            if (body != null) {
+                NetworkResponse.Success(body, headers)
+            } else {
+                // Special case: If the response is successful and the body is null, return a successful response
+                // if the service method declares the success body type as Unit. Otherwise, return a server error
+                if (successBodyType == Unit::class.java) {
+                    @Suppress("UNCHECKED_CAST")
+                    NetworkResponse.Success(Unit, headers) as NetworkResponse<S, E>
+                } else {
+                    NetworkResponse.ServerError(null, code, headers)
+                }
+            }
+        } else {
+            val networkResponse: NetworkResponse<S, E> = try {
+                val convertedBody = errorConverter.convert(errorBody)
+                NetworkResponse.ServerError(convertedBody, code, headers)
+            } catch (ex: Exception) {
+                NetworkResponse.UnknownError(ex)
+            }
+            networkResponse
+        }
+    }
+
+}

src/test/kotlin/com/haroldadmin/cnradapter/MoshiApplicationTest.kt

@@ -6,6 +6,7 @@ import com.squareup.moshi.Moshi
 import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
 import io.kotlintest.matchers.string.shouldContain
 import io.kotlintest.matchers.types.shouldBeInstanceOf
+import io.kotlintest.shouldBe
 import io.kotlintest.specs.AnnotationSpec
 import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.runBlocking
@@ -44,6 +45,12 @@ internal interface LaunchesService {
     fun launchForFlightNumberAsyncInvalid(
         @Path("flightNumber") flightNumber: Long
     ): Deferred<NetworkResponse<LaunchInvalid, GenericErrorResponseInvalid>>
+
+    @GET("/health")
+    suspend fun healthCheck(): NetworkResponse<Unit, GenericErrorResponse>
+
+    @GET("/health")
+    fun deferredHealthCheck(): Deferred<NetworkResponse<Unit, GenericErrorResponse>>
 }
 
 internal class TestApplication(
@@ -72,6 +79,13 @@ internal class TestApplication(
         launchesService.launchForFlightNumberAsyncInvalid(flightNumber).await()
     }
 
+    fun healthCheck(): NetworkResponse<Unit, GenericErrorResponse> = runBlocking {
+        launchesService.healthCheck()
+    }
+
+    fun deferredHealthCheck(): NetworkResponse<Unit, GenericErrorResponse> = runBlocking {
+        launchesService.deferredHealthCheck().await()
+    }
 }
 
 internal class MoshiApplicationTest: AnnotationSpec() {
@@ -203,4 +217,30 @@ internal class MoshiApplicationTest: AnnotationSpec() {
         response as NetworkResponse.UnknownError
         response.error.shouldBeInstanceOf<JsonDataException>()
     }
+
+    @Test
+    fun `should parse empty body as Unit`() {
+        val app = TestApplication(service)
+        server.enqueue(MockResponse().apply {
+            setResponseCode(204)
+        })
+        val response = app.healthCheck()
+
+        response.shouldBeInstanceOf<NetworkResponse.Success<Unit>>()
+        response as NetworkResponse.Success
+        response.body shouldBe Unit
+    }
+
+    @Test
+    fun `should parse empty body as Unit for deferred methods too`() {
+        val app = TestApplication(service)
+        server.enqueue(MockResponse().apply {
+            setResponseCode(204)
+        })
+        val response = app.deferredHealthCheck()
+
+        response.shouldBeInstanceOf<NetworkResponse.Success<Unit>>()
+        response as NetworkResponse.Success
+        response.body shouldBe Unit
+    }
 }

src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseCallTest.kt

@@ -21,7 +21,7 @@ internal class NetworkResponseCallTest: AnnotationSpec() {
     @Before
     fun setup() {
         backingCall = CompletableCall<String>()
-        networkResponseCall = NetworkResponseCall<String, String>(backingCall, errorConverter)
+        networkResponseCall = NetworkResponseCall<String, String>(backingCall, errorConverter, String::class.java)
     }
 
     @Test(expected = UnsupportedOperationException::class)

src/test/kotlin/com/haroldadmin/cnradapter/Service.kt

@@ -9,4 +9,7 @@ interface Service {
 
     @GET("/suspend")
     suspend fun getTextSuspend(): NetworkResponse<String, String>
+
+    @GET("/suspend-empty-body")
+    suspend fun getEmptyBodySuspend(): NetworkResponse<Unit, String>
 }

src/test/kotlin/com/haroldadmin/cnradapter/SuspendTest.kt

@@ -81,21 +81,35 @@ internal class SuspendTest: AnnotationSpec() {
         response.shouldBeTypeOf<NetworkResponse.NetworkError>()
     }
 
-    // Ignore this test because someone else wants to work on solving this issue:
-    // https://github.com/haroldadmin/NetworkResponseAdapter/issues/9
     @Test
-    @Ignore
     fun `successful response with empty body`() {
         val successResponseCode = 204
         server.enqueue(MockResponse().apply {
             setResponseCode(successResponseCode)
         })
-        val response = runBlocking { service.getTextSuspend() }
+
+        val response = runBlocking { service.getEmptyBodySuspend() }
 
         with(response) {
-            shouldBeTypeOf<NetworkResponse.Success<String>>()
+            shouldBeTypeOf<NetworkResponse.Success<Unit>>()
             this as NetworkResponse.Success
-            body shouldBe null
+            body shouldBe Unit
+        }
+    }
+
+    @Test
+    fun `unsuccessful response with empty body`() {
+        val successResponseCode = 400
+        server.enqueue(MockResponse().apply {
+            setResponseCode(successResponseCode)
+        })
+
+        val response = runBlocking { service.getEmptyBodySuspend() }
+
+        with(response) {
+            shouldBeTypeOf<NetworkResponse.ServerError<String>>()
+            this as NetworkResponse.ServerError
+            body shouldBe ""
         }
     }