[R] improve binary/text response handling (#20131)

* [R client] better support for binary/compressed responses * cleanup * revert change after PR review * update samples * fix R tests * move private api methods to api-client, revert breaking method name change
OpenAPITools · Nov 24, 2024 · 6399a7a · 6399a7a
1 parent c33d3aa
commit 6399a7a
Show file tree

Hide file tree

Showing 34 changed files with 1,522 additions and 1,101 deletions.
diff --git a/modules/openapi-generator/src/main/resources/r/ApiResponse.mustache b/modules/openapi-generator/src/main/resources/r/ApiResponse.mustache
@@ -50,9 +50,6 @@ ApiResponse <- R6::R6Class(
         self$response <- charToRaw(jsonlite::toJSON("NULL"))
       }
       text_response <- iconv(readBin(self$response, character()), from = from_encoding, to = to_encoding)
-      if (is.na(text_response)) {
-        warning("The response is binary and will not be converted to text.")
-      }
       return(text_response)
     }
   )

diff --git a/modules/openapi-generator/src/main/resources/r/api.mustache b/modules/openapi-generator/src/main/resources/r/api.mustache
@@ -153,13 +153,13 @@
 
       {{/vendorExtensions.x-streaming}}
       if (local_var_response$status_code >= 200 && local_var_response$status_code <= 299) {
-        local_var_response$content
+        return(local_var_response$content)
       } else if (local_var_response$status_code >= 300 && local_var_response$status_code <= 399) {
-        local_var_response
+        return(local_var_response)
       } else if (local_var_response$status_code >= 400 && local_var_response$status_code <= 499) {
-        local_var_response
+        return(local_var_response)
       } else if (local_var_response$status_code >= 500 && local_var_response$status_code <= 599) {
-        local_var_response
+        return(local_var_response)
       }
     },
 
@@ -543,24 +543,21 @@
       if (local_var_resp$status_code >= 200 && local_var_resp$status_code <= 299) {
         {{#returnType}}
         {{#isPrimitiveType}}
-        local_var_content <- local_var_resp$response
-          local_var_resp, "text", encoding = "UTF-8", simplifyVector = FALSE
-        )
         # save response in a file
         if (!is.null(data_file)) {
-          write(local_var_content, data_file)
+          self$api_client$WriteFile(local_var_resp, data_file)
         }
 
         ApiResponse$new(content,resp)
         {{/isPrimitiveType}}
         {{^isPrimitiveType}}
         # save response in a file
         if (!is.null(data_file)) {
-          write(local_var_resp$response, data_file)
+          self$api_client$WriteFile(local_var_resp, data_file)
         }
 
         deserialized_resp_obj <- tryCatch(
-          self$api_client$deserialize(local_var_resp$response_as_text(), "{{returnType}}", loadNamespace("{{packageName}}")),
+          self$api_client$DeserializeResponse(local_var_resp, "{{returnType}}"),
           error = function(e) {
             {{#useDefaultExceptionHandling}}
             stop("Failed to deserialize response")
@@ -579,10 +576,13 @@
         {{! Returning the ApiResponse object with NULL object when the endpoint doesn't return anything}}
         local_var_resp$content <- NULL
         {{/returnType}}
-        local_var_resp
-      } else if (local_var_resp$status_code >= 300 && local_var_resp$status_code <= 399) {
+        return(local_var_resp)
+      } 
+      
+      local_var_error_msg <- local_var_resp$response_as_text()      
+      if (local_var_resp$status_code >= 300 && local_var_resp$status_code <= 399) {
       {{#returnExceptionOnFailure}}
-        local_var_error_msg <- local_var_resp$response
+        
         if (local_var_error_msg == "") {
           local_var_error_msg <- paste("Server returned ", local_var_resp$status_code, " response status code.")
         }
@@ -600,7 +600,6 @@
         {{/returnExceptionOnFailure}}
       } else if (local_var_resp$status_code >= 400 && local_var_resp$status_code <= 499) {
         {{#returnExceptionOnFailure}}
-        local_var_error_msg <- local_var_resp$response
         if (local_var_error_msg == "") {
           local_var_error_msg <- "Api client exception encountered."
         }
@@ -618,7 +617,6 @@
         {{/returnExceptionOnFailure}}
       } else if (local_var_resp$status_code >= 500 && local_var_resp$status_code <= 599) {
         {{#returnExceptionOnFailure}}
-        local_var_error_msg <- local_var_resp$response
         if (local_var_error_msg == "") {
           local_var_error_msg <- "Api server exception encountered."
         }
@@ -635,7 +633,7 @@
         if (is.null(local_var_resp$response) || local_var_resp$response == "") {
           local_var_resp$response <- "API server error"
         }
-        local_var_resp
+        return(local_var_resp)
         {{/returnExceptionOnFailure}}
       }
     }{{^-last}},{{/-last}}

diff --git a/modules/openapi-generator/src/main/resources/r/api_client.mustache b/modules/openapi-generator/src/main/resources/r/api_client.mustache
@@ -430,6 +430,52 @@ ApiClient <- R6::R6Class(
         # not json mime type, simply return the first one
         return(headers[1])
       }
+    },
+
+    #' @description
+    #' Deserialize the response
+    #' 
+    #' @param local_var_resp The API response
+    #' @param return_type The target return type for the endpoint (e.g., `"object"`). If `NULL` text will be left as-is.
+    #' @return If the raw response is corecable to text, return the text. Otherwise return the raw resposne.
+    DeserializeResponse = function(local_var_resp, return_type = NULL) {
+      text <- local_var_resp$response_as_text()
+      if (is.na(text)) {
+        return(local_var_resp$response)
+      } else if (is.null(return_type)) {
+        return(text)
+      }
+      return(self$deserialize(text, return_type, loadNamespace("{{packageName}}")))
+    },
+
+    #' @description
+    #' Write response to a file
+    #' 
+    #' The function will write out data. 
+    #' 
+    #' 1. If binary data is detected it will use `writeBin`
+    #' 2. If the raw response is coercable to text, the text will be written to a file
+    #' 3. If the raw response is not coercable to text, the raw response will be written
+    #' 
+    #' @param local_var_resp The API response
+    #' @param file The name of the data file to save the result
+    WriteFile = function(local_var_resp, file) {
+      if (self$IsBinary(local_var_resp$response)) {
+        writeBin(local_var_resp$response, file)
+      } else {
+        response <- self$DeserializeResponse(local_var_resp)
+        base::write(response, file)
+      }
+    },
+
+    #' @description
+    #' Check response for binary content
+    #' 
+    #' @param local_var_resp The API response
+    IsBinary = function(x) {
+      # ref: https://stackoverflow.com/a/17098690/1785752
+      b <- readBin(x, "int", n = 1000, size=1, signed=FALSE)
+      return(max(b) > 128)
     }
   )
 )
diff --git a/modules/openapi-generator/src/main/resources/r/api_exception.mustache b/modules/openapi-generator/src/main/resources/r/api_exception.mustache
@@ -32,15 +32,15 @@ ApiException <- R6::R6Class(
     initialize = function(status = NULL, reason = NULL, http_response = NULL) {
       if (!is.null(http_response)) {
         self$status <- http_response$status_code
-        errorMsg <- http_response$response
+        errorMsg <- http_response$response_as_text()
         if (is.null(errorMsg) || errorMsg == "") {
           errorMsg <- "Api exception encountered. No details given."
         }
         self$body <- errorMsg
         self$headers <- http_response$headers
         self$reason <- http_response$http_status_desc
         {{#errorObjectType}}
-        self$error_object <- {{errorObjectType}}$new()$fromJSONString(http_response$response)
+        self$error_object <- {{errorObjectType}}$new()$fromJSONString(http_response$response_as_text())
         {{/errorObjectType}}
       } else {
         self$status <- status

diff --git a/modules/openapi-generator/src/main/resources/r/libraries/httr2/api_client.mustache b/modules/openapi-generator/src/main/resources/r/libraries/httr2/api_client.mustache
@@ -443,6 +443,52 @@ ApiClient  <- R6::R6Class(
         # not json mime type, simply return the first one
         return(headers[1])
       }
+    },
+
+    #' @description
+    #' Deserialize the response
+    #' 
+    #' @param local_var_resp The API response
+    #' @param return_type The target return type for the endpoint (e.g., `"object"`). If `NULL` text will be left as-is.
+    #' @return If the raw response is corecable to text, return the text. Otherwise return the raw resposne.
+    DeserializeResponse = function(local_var_resp, return_type = NULL) {
+      text <- local_var_resp$response_as_text()
+      if (is.na(text)) {
+        return(local_var_resp$response)
+      } else if (is.null(return_type)) {
+        return(text)
+      }
+      return(self$deserialize(text, return_type, loadNamespace("{{packageName}}")))
+    },
+
+    #' @description
+    #' Write response to a file
+    #' 
+    #' The function will write out data. 
+    #' 
+    #' 1. If binary data is detected it will use `writeBin`
+    #' 2. If the raw response is coercable to text, the text will be written to a file
+    #' 3. If the raw response is not coercable to text, the raw response will be written
+    #' 
+    #' @param local_var_resp The API response
+    #' @param file The name of the data file to save the result
+    WriteFile = function(local_var_resp, file) {
+      if (self$IsBinary(local_var_resp$response)) {
+        writeBin(local_var_resp$response, file)
+      } else {
+        response <- self$DeserializeResponse(local_var_resp)
+        base::write(response, file)
+      }
+    },
+
+    #' @description
+    #' Check response for binary content
+    #' 
+    #' @param local_var_resp The API response
+    IsBinary = function(x) {
+      # ref: https://stackoverflow.com/a/17098690/1785752
+      b <- readBin(x, "int", n = 1000, size=1, signed=FALSE)
+      return(max(b) > 128)
     }
   )
 )
diff --git a/samples/client/echo_api/r/R/api_client.R b/samples/client/echo_api/r/R/api_client.R
@@ -382,6 +382,52 @@ ApiClient <- R6::R6Class(
         # not json mime type, simply return the first one
         return(headers[1])
       }
+    },
+
+    #' @description
+    #' Deserialize the response
+    #' 
+    #' @param local_var_resp The API response
+    #' @param return_type The target return type for the endpoint (e.g., `"object"`). If `NULL` text will be left as-is.
+    #' @return If the raw response is corecable to text, return the text. Otherwise return the raw resposne.
+    DeserializeResponse = function(local_var_resp, return_type = NULL) {
+      text <- local_var_resp$response_as_text()
+      if (is.na(text)) {
+        return(local_var_resp$response)
+      } else if (is.null(return_type)) {
+        return(text)
+      }
+      return(self$deserialize(text, return_type, loadNamespace("openapi")))
+    },
+
+    #' @description
+    #' Write response to a file
+    #' 
+    #' The function will write out data. 
+    #' 
+    #' 1. If binary data is detected it will use `writeBin`
+    #' 2. If the raw response is coercable to text, the text will be written to a file
+    #' 3. If the raw response is not coercable to text, the raw response will be written
+    #' 
+    #' @param local_var_resp The API response
+    #' @param file The name of the data file to save the result
+    WriteFile = function(local_var_resp, file) {
+      if (self$IsBinary(local_var_resp$response)) {
+        writeBin(local_var_resp$response, file)
+      } else {
+        response <- self$DeserializeResponse(local_var_resp)
+        base::write(response, file)
+      }
+    },
+
+    #' @description
+    #' Check response for binary content
+    #' 
+    #' @param local_var_resp The API response
+    IsBinary = function(x) {
+      # ref: https://stackoverflow.com/a/17098690/1785752
+      b <- readBin(x, "int", n = 1000, size=1, signed=FALSE)
+      return(max(b) > 128)
     }
   )
 )
diff --git a/samples/client/echo_api/r/R/api_response.R b/samples/client/echo_api/r/R/api_response.R
@@ -57,9 +57,6 @@ ApiResponse <- R6::R6Class(
         self$response <- charToRaw(jsonlite::toJSON("NULL"))
       }
       text_response <- iconv(readBin(self$response, character()), from = from_encoding, to = to_encoding)
-      if (is.na(text_response)) {
-        warning("The response is binary and will not be converted to text.")
-      }
       return(text_response)
     }
   )

diff --git a/samples/client/echo_api/r/R/auth_api.R b/samples/client/echo_api/r/R/auth_api.R
@@ -78,13 +78,13 @@ AuthApi <- R6::R6Class(
     TestAuthHttpBasic = function(data_file = NULL, ...) {
       local_var_response <- self$TestAuthHttpBasicWithHttpInfo(data_file = data_file, ...)
       if (local_var_response$status_code >= 200 && local_var_response$status_code <= 299) {
-        local_var_response$content
+        return(local_var_response$content)
       } else if (local_var_response$status_code >= 300 && local_var_response$status_code <= 399) {
-        local_var_response
+        return(local_var_response)
       } else if (local_var_response$status_code >= 400 && local_var_response$status_code <= 499) {
-        local_var_response
+        return(local_var_response)
       } else if (local_var_response$status_code >= 500 && local_var_response$status_code <= 599) {
-        local_var_response
+        return(local_var_response)
       }
     },
 
@@ -133,26 +133,29 @@ AuthApi <- R6::R6Class(
       if (local_var_resp$status_code >= 200 && local_var_resp$status_code <= 299) {
         # save response in a file
         if (!is.null(data_file)) {
-          write(local_var_resp$response, data_file)
+          self$api_client$WriteFile(local_var_resp, data_file)
         }
 
         deserialized_resp_obj <- tryCatch(
-          self$api_client$deserialize(local_var_resp$response_as_text(), "character", loadNamespace("openapi")),
+          self$api_client$DeserializeResponse(local_var_resp, "character"),
           error = function(e) {
             stop("Failed to deserialize response")
           }
         )
         local_var_resp$content <- deserialized_resp_obj
-        local_var_resp
-      } else if (local_var_resp$status_code >= 300 && local_var_resp$status_code <= 399) {
+        return(local_var_resp)
+      } 
+
+      local_var_error_msg <- local_var_resp$response_as_text()      
+      if (local_var_resp$status_code >= 300 && local_var_resp$status_code <= 399) {
         ApiResponse$new(paste("Server returned ", local_var_resp$status_code, " response status code."), local_var_resp)
       } else if (local_var_resp$status_code >= 400 && local_var_resp$status_code <= 499) {
         ApiResponse$new("API client error", local_var_resp)
       } else if (local_var_resp$status_code >= 500 && local_var_resp$status_code <= 599) {
         if (is.null(local_var_resp$response) || local_var_resp$response == "") {
           local_var_resp$response <- "API server error"
         }
-        local_var_resp
+        return(local_var_resp)
       }
     },
 
@@ -166,13 +169,13 @@ AuthApi <- R6::R6Class(
     TestAuthHttpBearer = function(data_file = NULL, ...) {
       local_var_response <- self$TestAuthHttpBearerWithHttpInfo(data_file = data_file, ...)
       if (local_var_response$status_code >= 200 && local_var_response$status_code <= 299) {
-        local_var_response$content
+        return(local_var_response$content)
       } else if (local_var_response$status_code >= 300 && local_var_response$status_code <= 399) {
-        local_var_response
+        return(local_var_response)
       } else if (local_var_response$status_code >= 400 && local_var_response$status_code <= 499) {
-        local_var_response
+        return(local_var_response)
       } else if (local_var_response$status_code >= 500 && local_var_response$status_code <= 599) {
-        local_var_response
+        return(local_var_response)
       }
     },
 
@@ -221,26 +224,29 @@ AuthApi <- R6::R6Class(
       if (local_var_resp$status_code >= 200 && local_var_resp$status_code <= 299) {
         # save response in a file
         if (!is.null(data_file)) {
-          write(local_var_resp$response, data_file)
+          self$api_client$WriteFile(local_var_resp, data_file)
         }
 
         deserialized_resp_obj <- tryCatch(
-          self$api_client$deserialize(local_var_resp$response_as_text(), "character", loadNamespace("openapi")),
+          self$api_client$DeserializeResponse(local_var_resp, "character"),
           error = function(e) {
             stop("Failed to deserialize response")
           }
         )
         local_var_resp$content <- deserialized_resp_obj
-        local_var_resp
-      } else if (local_var_resp$status_code >= 300 && local_var_resp$status_code <= 399) {
+        return(local_var_resp)
+      } 
+
+      local_var_error_msg <- local_var_resp$response_as_text()      
+      if (local_var_resp$status_code >= 300 && local_var_resp$status_code <= 399) {
         ApiResponse$new(paste("Server returned ", local_var_resp$status_code, " response status code."), local_var_resp)
       } else if (local_var_resp$status_code >= 400 && local_var_resp$status_code <= 499) {
         ApiResponse$new("API client error", local_var_resp)
       } else if (local_var_resp$status_code >= 500 && local_var_resp$status_code <= 599) {
         if (is.null(local_var_resp$response) || local_var_resp$response == "") {
           local_var_resp$response <- "API server error"
         }
-        local_var_resp
+        return(local_var_resp)
       }
     }
   )