v0.6.0: Nullable body handling (#21)

- `.await()` and `.awaitResult()` can be used now only with non-nullable types of result and throw NullPointerException in case of null body - `.toString()` for `Result` classes #13
gildor · Jul 4, 2017 · 96aae36 · 96aae36
1 parent 04b13c4
commit 96aae36
Show file tree

Hide file tree

Showing 9 changed files with 224 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,17 @@
 # CHANGELOG
 
+## Version 0.6.0 (2017-07-04)
+
+Notice: This release backward incompatible with previous versions:
+
+- `.await()` and `.awaitResult()` can be used now only with non-nullable types of result 
+and throw NullPointerException in case of null body. 
+See [examples in Readme](README.md#Nullable body)
+- [#13](https://github.com/gildor/kotlin-coroutines-retrofit/issues/13) `.toString()` for `Result` classes
+
 ## Version 0.5.1 (2017-06-27)
 
+- [Retrofit 2.3.0](https://github.com/square/retrofit/blob/parent-2.3.0/CHANGELOG.md#version-230-2017-05-13)
 - [kotlinx.coroutines 0.16](https://github.com/Kotlin/kotlinx.coroutines/releases/tag/0.16)
 - Compiled against Kotlin 1.1.3
 

diff --git a/README.md b/README.md
@@ -10,7 +10,7 @@ Download the [JAR](https://bintray.com/gildor/maven/kotlin-coroutines-retrofit#f
 Gradle:
 
 ```groovy
-compile 'ru.gildor.coroutines:kotlin-coroutines-retrofit:0.5.1'
+compile 'ru.gildor.coroutines:kotlin-coroutines-retrofit:0.6.0'
 ```
 
 Maven:
@@ -19,7 +19,7 @@ Maven:
 <dependency>
   <groupId>ru.gildor.coroutines</groupId>
   <artifactId>kotlin-coroutines-retrofit</artifactId>
-  <version>0.5.1</version>
+  <version>0.6.0</version>
 </dependency>
 ```
 
@@ -143,3 +143,35 @@ fun main(args: Array<String>) = runBlocking {
   }
 }
 ```
+
+## Nullable body
+
+To prevent unexpected behavior with nullable body of response `Call<Body?>`
+extensions `.await()` and `.awaitResult()` awailable only for 
+non nullable `Call<Body>` or platform `Call<Body!>` body types:
+
+```kotlin
+fun main(args: Array<String>) = runBlocking {
+  val user: Call<User> = api.getUser("username")
+  val userOrNull: Call<User?> = api.getUserOrNull("username")
+
+  // Doesn't work, because User is nullable
+  // userOrNull.await()
+
+  // Works for non-nullable type
+  try {
+      val result: User = user.await()  
+  } catch (e: NullPointerException) {
+      // If body will be null you will get NullPointerException
+  }
+
+  // You can use .awaitResult() to catch possible problems with nullable body
+  val nullableResult = api.getUser("username").awaitResult().getOrNull()
+  // But type of body should be non-nullable
+  // api.getUserOrNull("username").awaitResult()
+
+  // If you still want to use nullable body to clarify your api
+  // use awaitResponse() instead:
+  val responseBody: User? = userOrNull.awaitResponse().body()
+}
+``` 
diff --git a/build.gradle b/build.gradle
@@ -4,14 +4,12 @@ plugins {
 }
 
 group 'ru.gildor.coroutines'
-version '0.5.1'
+version '0.6.0'
 
 repositories {
     jcenter()
 }
 
-apply plugin: 'kotlin'
-
 targetCompatibility = '1.6'
 sourceCompatibility = '1.6'
 

diff --git a/src/main/kotlin/ru/gildor/coroutines/retrofit/CallAwait.kt b/src/main/kotlin/ru/gildor/coroutines/retrofit/CallAwait.kt
@@ -12,12 +12,19 @@ import retrofit2.Response
  *
  * @return Result of request or throw exception
  */
-suspend fun <T> Call<T>.await(): T {
+public suspend fun <T : Any> Call<T>.await(): T {
     return suspendCancellableCoroutine { continuation ->
         enqueue(object : Callback<T> {
-            override fun onResponse(call: Call<T>?, response: Response<T>) {
+            override fun onResponse(call: Call<T>?, response: Response<T?>) {
                 if (response.isSuccessful) {
-                    continuation.resume(response.body() as T)
+                    val body = response.body()
+                    if (body == null) {
+                        continuation.resumeWithException(
+                                NullPointerException("Response body is null")
+                        )
+                    } else {
+                        continuation.resume(body)
+                    }
                 } else {
                     continuation.resumeWithException(HttpException(response))
                 }
@@ -39,7 +46,7 @@ suspend fun <T> Call<T>.await(): T {
  *
  * @return Response for request or throw exception
  */
-suspend fun <T> Call<T>.awaitResponse(): Response<T> {
+public suspend fun <T : Any?> Call<T>.awaitResponse(): Response<T> {
     return suspendCancellableCoroutine { continuation ->
         enqueue(object : Callback<T> {
             override fun onResponse(call: Call<T>?, response: Response<T>) {
@@ -63,13 +70,18 @@ suspend fun <T> Call<T>.awaitResponse(): Response<T> {
  * @return sealed class [Result] object that can be
  *         casted to [Result.Ok] (success) or [Result.Error] (HTTP error) and [Result.Exception] (other errors)
  */
-suspend fun <T> Call<T>.awaitResult(): Result<T> {
+public suspend fun <T : Any> Call<T>.awaitResult(): Result<T> {
     return suspendCancellableCoroutine { continuation ->
         enqueue(object : Callback<T> {
             override fun onResponse(call: Call<T>?, response: Response<T>) {
                 continuation.resume(
                         if (response.isSuccessful) {
-                            Result.Ok(response.body() as T, response.raw())
+                            val body = response.body()
+                            if (body == null) {
+                                Result.Exception(NullPointerException("Response body is null"))
+                            } else {
+                                Result.Ok(body, response.raw())
+                            }
                         } else {
                             Result.Error(HttpException(response), response.raw())
                         }
@@ -97,4 +109,3 @@ private fun Call<*>.registerOnCompletion(continuation: CancellableContinuation<*
             }
     }
 }
-
diff --git a/src/main/kotlin/ru/gildor/coroutines/retrofit/Result.kt b/src/main/kotlin/ru/gildor/coroutines/retrofit/Result.kt
@@ -4,66 +4,78 @@ import okhttp3.Response
 import retrofit2.HttpException
 
 /**
- * Sealed class of Http result
+ * Sealed class of HTTP result
  */
 @Suppress("unused")
-sealed class Result<out T> {
+public sealed class Result<out T : Any> {
     /**
      * Successful result of request without errors
      */
-    class Ok<out T>(
-            val value: T,
+    public class Ok<out T : Any>(
+            public val value: T,
             override val response: Response
-    ) : Result<T>(), ResponseResult
+    ) : Result<T>(), ResponseResult {
+        override fun toString(): String {
+            return "Result.Ok{value=$value, response=$response}"
+        }
+    }
 
     /**
      * HTTP error
      */
-    class Error(
+    public class Error(
             override val exception: HttpException,
             override val response: Response
-    ) : Result<Nothing>(), ErrorResult, ResponseResult
+    ) : Result<Nothing>(), ErrorResult, ResponseResult {
+        override fun toString(): String {
+            return "Result.Error{exception=$exception}"
+        }
+    }
 
     /**
      * Network exception occurred talking to the server or when an unexpected
      * exception occurred creating the request or processing the response
      */
-    class Exception(
+    public class Exception(
             override val exception: Throwable
-    ) : Result<Nothing>(), ErrorResult
+    ) : Result<Nothing>(), ErrorResult {
+        override fun toString(): String {
+            return "Result.Exception{$exception}"
+        }
+    }
 
 }
 
 /**
  * Interface for [Result] classes with [okhttp3.Response]: [Result.Ok] and [Result.Error]
  */
-interface ResponseResult {
+public interface ResponseResult {
     val response: Response
 }
 
 /**
  * Interface for [Result] classes that contains [Throwable]: [Result.Error] and [Result.Exception]
  */
-interface ErrorResult {
+public interface ErrorResult {
     val exception: Throwable
 }
 
 /**
  * Returns [Result.Ok.value] or `null`
  */
-fun <T> Result<T>.getOrNull() =
+public fun <T : Any> Result<T>.getOrNull() =
         if (this is Result.Ok) this.value else null
 
 /**
  * Returns [Result.Ok.value] or [default]
  */
-fun <T> Result<T>.getOrDefault(default: T) =
+public fun <T : Any> Result<T>.getOrDefault(default: T) =
         getOrNull() ?: default
 
 /**
  * Returns [Result.Ok.value] or throw [throwable] or [ErrorResult.exception]
  */
-fun <T> Result<T>.getOrThrow(throwable: Throwable? = null): T {
+public fun <T : Any> Result<T>.getOrThrow(throwable: Throwable? = null): T {
     return when (this) {
         is Result.Ok -> value
         is Result.Error -> throw throwable ?: exception

diff --git a/src/test/kotlin/ru/gildor/coroutines/retrofit/CallAwaitTest.kt b/src/test/kotlin/ru/gildor/coroutines/retrofit/CallAwaitTest.kt
@@ -7,6 +7,7 @@ import org.junit.Assert.*
 import org.junit.Test
 import retrofit2.HttpException
 import ru.gildor.coroutines.retrofit.util.MockedCall
+import ru.gildor.coroutines.retrofit.util.NullBodyCall
 import ru.gildor.coroutines.retrofit.util.errorResponse
 
 private const val DONE = "Done!"
@@ -19,12 +20,17 @@ class CallAwaitTest {
 
     @Test(expected = HttpException::class)
     fun asyncHttpException() = testBlocking {
-        MockedCall(error = HttpException(errorResponse())).await()
+        MockedCall<String>(error = HttpException(errorResponse<String>())).await()
+    }
+
+    @Test(expected = NullPointerException::class)
+    fun asyncNullBody() = testBlocking {
+        NullBodyCall<String>().await()
     }
 
     @Test(expected = IllegalArgumentException::class)
     fun asyncException() = testBlocking {
-        MockedCall(exception = IllegalArgumentException("wrong get param")).await()
+        MockedCall<String>(exception = IllegalArgumentException("wrong get param")).await()
     }
 
     @Test
@@ -35,15 +41,28 @@ class CallAwaitTest {
 
     @Test
     fun asyncResponseError() = testBlocking {
-        val result = MockedCall(error = HttpException(errorResponse(500))).awaitResponse()
+        val result = MockedCall<String>(error = HttpException(errorResponse<String>(500))).awaitResponse()
         assertEquals(500, result.code())
     }
 
+    @Test
+    fun asyncResponseNullBody() = testBlocking {
+        val result = NullBodyCall<String>().awaitResponse()
+        assertNull(result.body())
+    }
+
+    @Test
+    fun asyncResponseNullableBody() = testBlocking {
+        //Check that we can call awaitResponse() on nullable body
+        val result = NullBodyCall<String?>().awaitResponse()
+        assertNull(result.body())
+    }
+
     @Test
     fun asyncResponseFailure() = testBlocking {
         val exception = IllegalStateException()
         try {
-            MockedCall(exception = exception).awaitResult()
+            MockedCall<String>(exception = exception).awaitResult()
         } catch (e: Exception) {
             assertSame(e, exception)
         }
@@ -62,6 +81,18 @@ class CallAwaitTest {
         }
     }
 
+    @Test
+    fun asyncResultNullBody() = testBlocking {
+        val result = NullBodyCall<String>().awaitResult()
+        assertNull(result.getOrNull())
+    }
+
+    @Test(expected = NullPointerException::class)
+    fun asyncResultNullPointerForNullBody() = testBlocking {
+        val result = NullBodyCall<String>().awaitResult()
+        assertNull(result.getOrThrow())
+    }
+
     @Test
     fun asyncResultByType() = testBlocking {
         val result = MockedCall(DONE).awaitResult()
@@ -87,9 +118,9 @@ class CallAwaitTest {
 
     @Test
     fun resultErrorTypes() = testBlocking {
-        val errorResponse = errorResponse(500)
+        val errorResponse = errorResponse<String>(500)
         val httpException = HttpException(errorResponse)
-        val errorResult = MockedCall(error = httpException).awaitResult()
+        val errorResult = MockedCall<String>(error = httpException).awaitResult()
 
         if (errorResult is ResponseResult) {
             assertEquals(500, errorResult.response.code())
@@ -107,7 +138,7 @@ class CallAwaitTest {
     @Test
     fun resultExceptionTypes() = testBlocking {
         val exception = IllegalStateException()
-        val errorResult = MockedCall(exception = exception).awaitResult()
+        val errorResult = MockedCall<String>(exception = exception).awaitResult()
 
         if (errorResult is ErrorResult) {
             assertEquals(exception, errorResult.exception)
@@ -119,8 +150,8 @@ class CallAwaitTest {
 
     @Test
     fun asyncResultError() = testBlocking {
-        val error = HttpException(errorResponse(500))
-        val result = MockedCall(error = error).awaitResult()
+        val error = HttpException(errorResponse<String>(500))
+        val result = MockedCall<String>(error = error).awaitResult()
         when (result) {
             is Result.Error -> {
                 assertEquals(error.code(), result.exception.code())
@@ -136,7 +167,7 @@ class CallAwaitTest {
     @Test
     fun asyncResultException() = testBlocking {
         val exception = IllegalArgumentException("wrong argument")
-        val result = MockedCall(exception = exception).awaitResult()
+        val result = MockedCall<String>(exception = exception).awaitResult()
         when (result) {
             is Result.Exception -> {
                 assertEquals(exception::class.java, result.exception::class.java)
@@ -151,4 +182,4 @@ class CallAwaitTest {
 
 private fun testBlocking(block: suspend CoroutineScope.() -> Unit) {
     runBlocking(Unconfined, block)
-}
+}