fix(amplify_api): prevent some fatal REST errors in Android

Travis Sheppard · web-flow · commit ef60beade998 · 2021-08-03T10:13:59.000-07:00
Throw ApiException in android when PUT, POST, and PATCH REST requests have no body to prevent fatal error (#661).
diff --git a/packages/amplify_api/android/src/main/kotlin/com/amazonaws/amplify/amplify_api/FlutterApiRequest.kt b/packages/amplify_api/android/src/main/kotlin/com/amazonaws/amplify/amplify_api/FlutterApiRequest.kt
@@ -15,7 +15,9 @@
 
 package com.amazonaws.amplify.amplify_api
 
+import com.amazonaws.amplify.amplify_api.rest_api.FlutterRestApi
 import com.amplifyframework.AmplifyException
+import com.amplifyframework.api.ApiException
 import com.amplifyframework.api.rest.RestOptions
 import io.flutter.plugin.common.MethodChannel
 
@@ -33,7 +35,7 @@ class FlutterApiRequest {
         private val HEADERS_KEY = "headers"
 
         // ====== Rest API ======
-        fun getCancelToken(request: Map<String, Any>) : String {
+        fun getCancelToken(request: Map<String, Any>): String {
             try {
                 return request[CANCEL_TOKEN_KEY] as String
             } catch (cause: Exception) {
@@ -45,7 +47,7 @@ class FlutterApiRequest {
             return request[CANCEL_TOKEN_KEY] as String
         }
 
-        fun getApiPath(request: Map<String, Any>) : String? {
+        fun getApiPath(request: Map<String, Any>): String? {
             try {
                 val restOptionsMap = request[REST_OPTIONS_KEY] as Map<String, Any>
                 return restOptionsMap[API_NAME_KEY] as String?
@@ -58,7 +60,7 @@ class FlutterApiRequest {
             return request[CANCEL_TOKEN_KEY] as String
         }
 
-        fun getRestOptions(request: Map<String, Any>) : RestOptions {
+        fun getRestOptions(request: Map<String, Any>): RestOptions {
 
             try {
                 val builder: RestOptions.Builder = RestOptions.builder()
@@ -90,16 +92,23 @@ class FlutterApiRequest {
             }
         }
 
+        @JvmStatic
+        fun checkForEmptyBodyIfRequired(options: RestOptions, methodName: String) {
+            if ((methodName == FlutterRestApi.PUT || methodName == FlutterRestApi.POST || methodName == FlutterRestApi.PATCH) && !options.hasData()) {
+                throw ApiException("$methodName request must have a body", "Add a body to the request.")
+            }
+        }
+
         // ====== GraphQL ======
         @JvmStatic
         fun getGraphQLDocument(request: Map<String, Any>): String {
             try {
                 return request["document"] as String
             } catch (cause: Exception) {
-                   throw AmplifyException(
-                            "The graphQL document request argument was not passed as a String",
-                            cause,
-                            "The request should include the graphQL document as a String")
+                throw AmplifyException(
+                        "The graphQL document request argument was not passed as a String",
+                        cause,
+                        "The request should include the graphQL document as a String")
             }
         }
 
diff --git a/packages/amplify_api/android/src/main/kotlin/com/amazonaws/amplify/amplify_api/rest_api/FlutterRestApi.kt b/packages/amplify_api/android/src/main/kotlin/com/amazonaws/amplify/amplify_api/rest_api/FlutterRestApi.kt
@@ -33,6 +33,12 @@ class FlutterRestApi {
     companion object {
         private val LOG = Amplify.Logging.forNamespace("amplify:flutter:api")
         private val handler = Handler(Looper.getMainLooper())
+        const val GET = "get"
+        const val POST = "post"
+        const val PUT = "put"
+        const val DELETE = "delete"
+        const val HEAD = "head"
+        const val PATCH = "patch"
 
         private fun restFunctionHelper(
                 methodName: String,
@@ -41,14 +47,23 @@ class FlutterRestApi {
                 functionWithoutApiName: FunctionWithoutApiName,
                 functionWithApiName: FunctionWithApiName
         ) {
-            var cancelToken : String
+            var cancelToken: String
             var apiName: String?
             var options: RestOptions
 
             try {
                 cancelToken = FlutterApiRequest.getCancelToken(request)
                 apiName = FlutterApiRequest.getApiPath(request)
                 options = FlutterApiRequest.getRestOptions(request)
+
+                // Needed to prevent Android library from throwing a fatal error when body not present in some methods. https://github.com/aws-amplify/amplify-android/issues/1355
+                FlutterApiRequest.checkForEmptyBodyIfRequired(options, methodName)
+            } catch (e: ApiException) {
+                handler.post {
+                    ExceptionUtil.postExceptionToFlutterChannel(flutterResult, "ApiException",
+                            ExceptionUtil.createSerializedError(e))
+                }
+                return
             } catch (e: Exception) {
                 handler.post {
                     ExceptionUtil.postExceptionToFlutterChannel(flutterResult, "ApiException",
@@ -92,7 +107,7 @@ class FlutterRestApi {
                             }
                     )
                 }
-                if(operation != null) {
+                if (operation != null) {
                     OperationsManager.addOperation(cancelToken, operation)
                 }
 
@@ -116,27 +131,27 @@ class FlutterRestApi {
         }
 
         fun get(flutterResult: Result, arguments: Map<String, Any>) {
-            restFunctionHelper("get", flutterResult, arguments, this::get, this::get)
+            restFunctionHelper(GET, flutterResult, arguments, this::get, this::get)
         }
 
         fun post(flutterResult: Result, arguments: Map<String, Any>) {
-            restFunctionHelper("post", flutterResult, arguments, this::post, this::post)
+            restFunctionHelper(POST, flutterResult, arguments, this::post, this::post)
         }
 
         fun put(flutterResult: Result, arguments: Map<String, Any>) {
-            restFunctionHelper("put", flutterResult, arguments, this::put, this::put)
+            restFunctionHelper(PUT, flutterResult, arguments, this::put, this::put)
         }
 
         fun delete(flutterResult: Result, arguments: Map<String, Any>) {
-            restFunctionHelper("delete", flutterResult, arguments, this::delete, this::delete)
+            restFunctionHelper(DELETE, flutterResult, arguments, this::delete, this::delete)
         }
 
         fun head(flutterResult: Result, arguments: Map<String, Any>) {
-            restFunctionHelper("head", flutterResult, arguments, this::head, this::head)
+            restFunctionHelper(HEAD, flutterResult, arguments, this::head, this::head)
         }
 
         fun patch(flutterResult: Result, arguments: Map<String, Any>) {
-            restFunctionHelper("patch", flutterResult, arguments, this::patch, this::patch)
+            restFunctionHelper(PATCH, flutterResult, arguments, this::patch, this::patch)
         }
 
 
diff --git a/packages/amplify_api/android/src/test/kotlin/com/amazonaws/amplify/amplify_api/AmplifyApiRestTest.kt b/packages/amplify_api/android/src/test/kotlin/com/amazonaws/amplify/amplify_api/AmplifyApiRestTest.kt
@@ -17,6 +17,8 @@ package com.amazonaws.amplify.amplify_api
 
 
 import com.amazonaws.amplify.amplify_core.exception.ExceptionMessages
+import com.amazonaws.amplify.amplify_api.AmplifyApiPlugin
+import com.amazonaws.amplify.amplify_api.rest_api.FlutterRestApi
 import com.amplifyframework.api.ApiCategory
 import com.amplifyframework.api.ApiException
 import com.amplifyframework.api.rest.RestOperation
@@ -352,6 +354,40 @@ class AmplifyApiRestTest {
         )
     }
 
+    // PUT PATCH and POST with no body throws error
+    @Test
+    fun required_body_methods_error() {
+        Mockito.doAnswer { _ ->
+            mockRestOperation
+        }.`when`(mockApi).get(
+                any(RestOptions::class.java),
+                any(),
+                any())
+
+        var arguments: Map<String, Any> = mapOf(
+                "restOptions" to mapOf(
+                        "path" to "/items"
+                ),
+                "cancelToken" to "someCode"
+        )
+        var methods = arrayOf<String>(FlutterRestApi.PUT, FlutterRestApi.POST, FlutterRestApi.PATCH)
+        for (method in methods) {
+            flutterPlugin.onMethodCall(
+                    MethodCall(method, arguments),
+                    mockResult
+            )
+
+            verify(mockResult, times(1)).error(
+                    "ApiException",
+                    ExceptionMessages.defaultFallbackExceptionMessage,
+                    mapOf(
+                            "recoverySuggestion" to "Add a body to the request.",
+                            "message" to "$method request must have a body"
+                    )
+            )
+        }
+    }
+
     // Cancellation of ongoing rest operation succeeds
     @Test
     fun test_cancel_get_returns_success(){
