From adc8f0b48c9dbd40913cf45daea4dfa8efb257df Mon Sep 17 00:00:00 2001 From: Leandro Date: Tue, 8 Feb 2022 14:52:00 +1300 Subject: [PATCH] Simplify generic types; handle no content response --- .../haroldadmin/cnradapter/NetworkResponse.kt | 70 +++++++++++++++---- .../haroldadmin/cnradapter/ResponseParser.kt | 23 +++--- .../DeferredNetworkResponseAdapterTest.kt | 16 ++--- .../haroldadmin/cnradapter/ExtensionsTest.kt | 14 ++-- .../cnradapter/MoshiApplicationTest.kt | 30 ++++---- .../NetworkResponseAdapterFactoryTest.kt | 2 +- .../cnradapter/NetworkResponseAdapterTest.kt | 22 +++--- .../cnradapter/NetworkResponseCallTest.kt | 35 ++++++---- 8 files changed, 124 insertions(+), 88 deletions(-) diff --git a/src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponse.kt b/src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponse.kt index f58ea71..11dc0ed 100644 --- a/src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponse.kt +++ b/src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponse.kt @@ -24,7 +24,13 @@ import java.io.IOException * - Internet connectivity problems, the [NetworkResponse] is [NetworkResponse.NetworkError] * - Any other problems (e.g. Serialization errors), the [NetworkResponse] is [NetworkResponse.UnknownError]. */ -public sealed interface NetworkResponse { +public sealed interface NetworkResponse { + + public sealed interface Success : NetworkResponse { + public val body: S + public val response: Response<*> + } + /** * The result of a successful network request. * @@ -35,10 +41,43 @@ public sealed interface NetworkResponse { * @param body The parsed body of the successful response. * @param response The original [Response] from Retrofit */ - public data class Success( - public val body: S, - public val response: Response<*> - ) : NetworkResponse { + public data class OK( + public override val body: S, + public override val response: Response<*> + ) : Success { + /** + * The status code returned by the server. + * + * Alias for [Response.code] of the original response + */ + public val code: Int + get() = response.code() + + /** + * The headers returned by the server. + * + * Alias for [Response.headers] of the original response + */ + public val headers: Headers + get() = response.headers() + } + + /** + * The result of a successful network request. + * + * If you expect your server response to not contain a body, set the success body type ([S]) to [Unit]. + * If you expect your server response to sometimes not contain a body (e.g. for response code 204), set + * [S] to [Unit] and deserialize the raw [response] manually when needed. + * + * @param response The original [Response] from Retrofit + */ + public data class NoContent( + public override val response: Response<*> + ) : Success { + + override val body: Nothing + get() = throw UnsupportedOperationException("Attempted to obtain body from no content response.") + /** * The status code returned by the server. * @@ -63,7 +102,7 @@ public sealed interface NetworkResponse { * body ([ServerError]), or due to a connectivity error ([NetworkError]), or due to an unknown * error ([UnknownError]). */ - public sealed interface Error : NetworkResponse { + public sealed interface Error : NetworkResponse { /** * The body of the failed network request, if available. */ @@ -81,10 +120,10 @@ public sealed interface NetworkResponse { * This result may or may not contain a [body], depending on the body * supplied by the server. */ - public data class ServerError( + public data class ServerError( public override val body: E?, public val response: Response<*>?, - ) : Error { + ) : Error { /** * The status code returned by the server. * @@ -108,28 +147,29 @@ public sealed interface NetworkResponse { /** * The result of a network connectivity error */ - public data class NetworkError( + public data class NetworkError( public override val error: IOException, - ) : Error { + ) : Error { /** * Always `null` for a [NetworkError] */ - override val body: E? = null + override val body: Nothing? = null } /** * Result of an unknown error during a network request * (e.g. Serialization errors) */ - public data class UnknownError( + public data class UnknownError( public override val error: Throwable, public val response: Response<*>? - ) : Error { + ) : Error { + /** * Always `null` for an [UnknownError] */ - override val body: E? = null + override val body: Nothing? = null /** * The status code returned by the server. @@ -152,4 +192,4 @@ public sealed interface NetworkResponse { * * Useful for specifying return types of API calls that do not return a useful value. */ -public typealias CompletableResponse = NetworkResponse +public typealias CompletableResponse = NetworkResponse diff --git a/src/main/kotlin/com/haroldadmin/cnradapter/ResponseParser.kt b/src/main/kotlin/com/haroldadmin/cnradapter/ResponseParser.kt index fbc834a..a874c21 100644 --- a/src/main/kotlin/com/haroldadmin/cnradapter/ResponseParser.kt +++ b/src/main/kotlin/com/haroldadmin/cnradapter/ResponseParser.kt @@ -21,7 +21,7 @@ internal fun Response.asNetworkResponse( return if (!isSuccessful) { parseUnsuccessfulResponse(this, errorConverter) } else { - parseSuccessfulResponse(this, successType) + parseSuccessfulResponse(this) } } @@ -41,8 +41,9 @@ internal fun Response.asNetworkResponse( private fun parseUnsuccessfulResponse( response: Response, errorConverter: Converter -): NetworkResponse.Error { - val errorBody: ResponseBody = response.errorBody() ?: return NetworkResponse.ServerError(null, response) +): NetworkResponse.Error { + val errorBody: ResponseBody = + response.errorBody() ?: return NetworkResponse.ServerError(null, response) return try { val convertedBody = errorConverter.convert(errorBody) @@ -62,18 +63,14 @@ private fun parseUnsuccessfulResponse( * - Else return a [NetworkResponse.ServerError] with a null body * - If response body is not null, return [NetworkResponse.Success] with the parsed body */ -private fun parseSuccessfulResponse(response: Response, successType: Type): NetworkResponse { - val responseBody: S? = response.body() - if (responseBody == null) { - if (successType === Unit::class.java) { - @Suppress("UNCHECKED_CAST") - return NetworkResponse.Success(Unit, response) as NetworkResponse +private fun parseSuccessfulResponse(response: Response): NetworkResponse { + return when (val responseBody: S? = response.body()) { + null -> when (response.code()) { + 204 -> NetworkResponse.NoContent(response) + else -> NetworkResponse.ServerError(null, response) } - - return NetworkResponse.ServerError(null, response) + else -> NetworkResponse.OK(responseBody, response) } - - return NetworkResponse.Success(responseBody, response) } /** diff --git a/src/test/kotlin/com/haroldadmin/cnradapter/DeferredNetworkResponseAdapterTest.kt b/src/test/kotlin/com/haroldadmin/cnradapter/DeferredNetworkResponseAdapterTest.kt index 21c878c..c011769 100644 --- a/src/test/kotlin/com/haroldadmin/cnradapter/DeferredNetworkResponseAdapterTest.kt +++ b/src/test/kotlin/com/haroldadmin/cnradapter/DeferredNetworkResponseAdapterTest.kt @@ -13,12 +13,13 @@ import okhttp3.OkHttpClient import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy -import retrofit2.* +import retrofit2.CallAdapter +import retrofit2.Retrofit import retrofit2.http.GET import java.io.IOException import java.time.Duration -class DeferredNetworkResponseAdapterTest : DescribeSpec({ +public class DeferredNetworkResponseAdapterTest : DescribeSpec({ describe(DeferredNetworkResponseAdapter::class.java.simpleName) { it("should return success type correctly") { val converterFactory = StringConverterFactory() @@ -90,7 +91,7 @@ class DeferredNetworkResponseAdapterTest : DescribeSpec({ ) val response = service.getTextAsync().await() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Test Message" } @@ -103,20 +104,19 @@ class DeferredNetworkResponseAdapterTest : DescribeSpec({ ) val response = service.getTextAsync().await() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Not Found" } it("should handle 200 (with body) and 204 (no body) responses correctly") { server.enqueue(MockResponse().setBody("Test Message").setResponseCode(200)) val response = service.getTextAsync().await() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Test Message" server.enqueue(MockResponse().setResponseCode(204)) val noBodyResponse = service.getTextAsync().await() - noBodyResponse.shouldBeInstanceOf>() - noBodyResponse.body shouldBe null + noBodyResponse.shouldBeInstanceOf() } it("should return network error response as NetworkResponse.NetworkError") { @@ -125,7 +125,7 @@ class DeferredNetworkResponseAdapterTest : DescribeSpec({ ) val response = service.getTextAsync().await() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } } diff --git a/src/test/kotlin/com/haroldadmin/cnradapter/ExtensionsTest.kt b/src/test/kotlin/com/haroldadmin/cnradapter/ExtensionsTest.kt index 852fada..c129e9a 100644 --- a/src/test/kotlin/com/haroldadmin/cnradapter/ExtensionsTest.kt +++ b/src/test/kotlin/com/haroldadmin/cnradapter/ExtensionsTest.kt @@ -15,28 +15,28 @@ import retrofit2.http.GET import java.io.IOException import java.time.Duration -class ExtensionsTest : DescribeSpec({ +public class ExtensionsTest : DescribeSpec({ context("Overloaded Invoke Operator") { it("should return the underlying body for NetworkResponse.Success") { - val response = NetworkResponse.Success("Test Message", Response.success("Test Message")) + val response = NetworkResponse.OK("Test Message", Response.success("Test Message")) val body = response() body shouldBe "Test Message" } it("should return null for NetworkResponse.ServerError") { - val response = NetworkResponse.ServerError(null, null) + val response = NetworkResponse.ServerError(null, null) val body = response() body shouldBe null } it("should return null for NetworkResponse.NetworkError") { - val response = NetworkResponse.NetworkError(IOException()) + val response = NetworkResponse.NetworkError(IOException()) val body = response() body shouldBe null } it("should return null for NetworkResponse.UnknownError") { - val response = NetworkResponse.UnknownError(Exception(), null) + val response = NetworkResponse.UnknownError(Exception(), null) val body = response() body shouldBe null } @@ -77,7 +77,7 @@ class ExtensionsTest : DescribeSpec({ service.getTextAsync().await() } - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Hi!" } @@ -92,7 +92,7 @@ class ExtensionsTest : DescribeSpec({ service.getText() } - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Hi!" } } diff --git a/src/test/kotlin/com/haroldadmin/cnradapter/MoshiApplicationTest.kt b/src/test/kotlin/com/haroldadmin/cnradapter/MoshiApplicationTest.kt index 6aaeab9..cc6015b 100644 --- a/src/test/kotlin/com/haroldadmin/cnradapter/MoshiApplicationTest.kt +++ b/src/test/kotlin/com/haroldadmin/cnradapter/MoshiApplicationTest.kt @@ -90,7 +90,7 @@ internal class TestApplication( } } -class MoshiApplicationTest : DescribeSpec({ +public class MoshiApplicationTest : DescribeSpec({ val server = MockWebServer() val moshi = Moshi.Builder() @@ -128,7 +128,7 @@ class MoshiApplicationTest : DescribeSpec({ ) val response = app.getLaunch(validFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body.name shouldContain "FalconSat" response.code shouldBe 200 } @@ -143,7 +143,7 @@ class MoshiApplicationTest : DescribeSpec({ ) val response = app.getLaunch(invalidFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body?.error shouldContain "Not Found" response.code shouldBe 404 } @@ -161,7 +161,7 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.getLaunchWithFailure(validFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } @@ -177,7 +177,7 @@ class MoshiApplicationTest : DescribeSpec({ ) val response = app.getLaunchWithFailure(validFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } @@ -192,7 +192,7 @@ class MoshiApplicationTest : DescribeSpec({ ) val response = app.getLaunchWithFailure(invalidFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } @@ -208,7 +208,7 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.getLaunchAsync(validFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body.name shouldContain "FalconSat" response.code shouldBe 200 } @@ -225,7 +225,7 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.getLaunchAsync(invalidFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body?.error shouldContain "Not Found" response.code shouldBe 404 } @@ -242,7 +242,7 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.getLaunchAsyncInvalid(validFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } @@ -258,11 +258,11 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.getLaunchAsyncInvalid(invalidFlightNumber) - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } - it("should parse empty body as Unit") { + it("should parse empty body as NoContent") { val app = TestApplication(service) server.enqueue( @@ -273,12 +273,11 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.healthCheck() - response.shouldBeInstanceOf>() - response.body shouldBe Unit + response.shouldBeInstanceOf() response.code shouldBe 204 } - it("should parse empty body as Unit for deferred methods too") { + it("should parse empty body as NoContent for deferred methods too") { val app = TestApplication(service) server.enqueue( @@ -289,8 +288,7 @@ class MoshiApplicationTest : DescribeSpec({ val response = app.deferredHealthCheck() - response.shouldBeInstanceOf>() - response.body shouldBe Unit + response.shouldBeInstanceOf() response.code shouldBe 204 } }) diff --git a/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterFactoryTest.kt b/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterFactoryTest.kt index 02e6f99..19f0ea4 100644 --- a/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterFactoryTest.kt +++ b/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterFactoryTest.kt @@ -9,7 +9,7 @@ import kotlinx.coroutines.Deferred import retrofit2.Call import retrofit2.Retrofit -class NetworkResponseAdapterFactoryTest : DescribeSpec({ +public class NetworkResponseAdapterFactoryTest : DescribeSpec({ describe(NetworkResponseAdapterFactory::class.java.simpleName) { it("should return null for non NetworkResponse return types") { val factory = NetworkResponseAdapterFactory() diff --git a/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterTest.kt b/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterTest.kt index 5c9dec6..a0c3d15 100644 --- a/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterTest.kt +++ b/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseAdapterTest.kt @@ -1,11 +1,6 @@ package com.haroldadmin.cnradapter -import com.haroldadmin.cnradapter.utils.CompletableCall -import com.haroldadmin.cnradapter.utils.CrashObjectConversionError -import com.haroldadmin.cnradapter.utils.CrashyObject -import com.haroldadmin.cnradapter.utils.CrashyObjectConverterFactory -import com.haroldadmin.cnradapter.utils.StringConverterFactory -import com.haroldadmin.cnradapter.utils.typeOf +import com.haroldadmin.cnradapter.utils.* import io.kotest.core.spec.style.DescribeSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe @@ -21,7 +16,7 @@ import retrofit2.http.GET import java.io.IOException import java.time.Duration -class NetworkResponseAdapterTest : DescribeSpec({ +public class NetworkResponseAdapterTest : DescribeSpec({ describe(NetworkResponseAdapter::class.java.simpleName) { it("should return success type correctly") { val converterFactory = StringConverterFactory() @@ -92,7 +87,7 @@ class NetworkResponseAdapterTest : DescribeSpec({ ) val response = service.getText() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Test Message" } @@ -105,20 +100,19 @@ class NetworkResponseAdapterTest : DescribeSpec({ ) val response = service.getText() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Not Found" } it("should handle 200 (with body) and 204 (no body) responses correctly") { server.enqueue(MockResponse().setBody("Test Message").setResponseCode(200)) val response = service.getText() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf>() response.body shouldBe "Test Message" server.enqueue(MockResponse().setResponseCode(204)) val noBodyResponse = service.getText() - noBodyResponse.shouldBeInstanceOf>() - noBodyResponse.body shouldBe null + noBodyResponse.shouldBeInstanceOf() } it("should return network error response as NetworkResponse.NetworkError") { @@ -127,7 +121,7 @@ class NetworkResponseAdapterTest : DescribeSpec({ ) val response = service.getText() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } @@ -135,7 +129,7 @@ class NetworkResponseAdapterTest : DescribeSpec({ server.enqueue(MockResponse().setBody("Ignore").setResponseCode(200)) val response = service.getCrashyObject() - response.shouldBeInstanceOf>() + response.shouldBeInstanceOf() response.error.shouldBeInstanceOf() } } diff --git a/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseCallTest.kt b/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseCallTest.kt index 657d3a1..35d025a 100644 --- a/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseCallTest.kt +++ b/src/test/kotlin/com/haroldadmin/cnradapter/NetworkResponseCallTest.kt @@ -12,12 +12,13 @@ import okhttp3.ResponseBody.Companion.toResponseBody import retrofit2.* import java.io.IOException -class NetworkResponseCallTest : DescribeSpec({ +public class NetworkResponseCallTest : DescribeSpec({ describe(NetworkResponseCall::class.java.simpleName) { it("should return a response asynchronously when using 'enqueue'") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) val completable = CompletableDeferred>() networkResponseCall.enqueue(object : Callback> { @@ -41,20 +42,21 @@ class NetworkResponseCallTest : DescribeSpec({ retrofitCall.complete("Test Message") val networkResponse = completable.await() - networkResponse.shouldBeTypeOf>() + networkResponse.shouldBeTypeOf>() networkResponse.body shouldBe "Test Message" } it("should return a response synchronously when using `execute`") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) retrofitCall.complete("Test Message") val response = networkResponseCall.awaitResponse() response.isSuccessful shouldBe true - response.body().shouldBeInstanceOf>() + response.body().shouldBeInstanceOf>() val networkResponse = response.body() as NetworkResponse.Success networkResponse.body shouldBe "Test Message" @@ -63,7 +65,8 @@ class NetworkResponseCallTest : DescribeSpec({ it("should cancel backing call when cancelled") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) networkResponseCall.isCanceled shouldBe false @@ -75,20 +78,22 @@ class NetworkResponseCallTest : DescribeSpec({ it("should parse a successful response as NetworkResponse.Success") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) retrofitCall.complete("Test Message") val networkResponse = networkResponseCall.awaitResponse().body() networkResponse shouldNotBe null - networkResponse.shouldBeInstanceOf>() + networkResponse.shouldBeInstanceOf>() networkResponse.body shouldBe "Test Message" } it("should parse an HTTPException as NetworkResponse.ServerError") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) retrofitCall.completeWithException( HttpException( @@ -101,32 +106,34 @@ class NetworkResponseCallTest : DescribeSpec({ val networkResponse = networkResponseCall.awaitResponse().body() networkResponse shouldNotBe null - networkResponse.shouldBeInstanceOf>() + networkResponse.shouldBeInstanceOf>() networkResponse.body shouldBe "Test Message" } it("should parse an IOException as NetworkResponse.NetworkError") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) retrofitCall.completeWithException(IOException()) val networkResponse = networkResponseCall.awaitResponse().body() networkResponse shouldNotBe null - networkResponse.shouldBeInstanceOf>() + networkResponse.shouldBeInstanceOf() } it("should parse an unknown exception as NetworkResponse.UnknownError") { val errorConverter = Converter { it.string() } val retrofitCall = CompletableCall() - val networkResponseCall = NetworkResponseCall(retrofitCall, errorConverter, String::class.java) + val networkResponseCall = + NetworkResponseCall(retrofitCall, errorConverter, String::class.java) retrofitCall.completeWithException(Exception()) val networkResponse = networkResponseCall.awaitResponse().body() networkResponse shouldNotBe null - networkResponse.shouldBeInstanceOf>() + networkResponse.shouldBeInstanceOf() } } })