Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions modules/openapi-generator/src/main/resources/r/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,21 @@
{{/requiredParams}}
{{#allParams}}
{{^isNullable}}
if (!missing(`{{paramName}}`) && is.null(`{{paramName}}`)) {
{{#useDefaultExceptionHandling}}
stop("Invalid value for `{{paramName}}` when calling {{classname}}${{operationId}}, `{{paramName}}` is not nullable")
{{/useDefaultExceptionHandling}}
{{#useRlangExceptionHandling}}
rlang::abort(message = "Invalid value for `{{paramName}}` when calling {{classname}}${{operationId}}, `{{paramName}}` is not nullable",
.subclass = "ApiException",
ApiException = ApiException$new(status = 0,
reason = "Invalid value for `{{paramName}}` when calling {{classname}}${{operationId}}, `{{paramName}}` is not nullable"))
{{/useRlangExceptionHandling}}
}
{{/isNullable}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the pr

2 questions:

  1. looks like required parameters are processed again for null check. shall we update the block in line 195 instead?

  2. since this is in the allParams loop/list, that means optional parameters (if defined) will be processed as well. would optional parameters work in this new code block (null check)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update was in response to @Mattias-Sehlstedt above

Parameters have two settings, required and nullable, see for example Data Types. required means that the property must be present, while nullable means whether the property can take the value null.

I was matching "required" to the R missing() check and "nullable" the R is.null() check. I added the !missing({{paramName}}) to the null checks to avoid an error in the event that an a parameter that is "not required" but "is not nullable" and is set to NULL. I cannot think of any cases where an optional parameter should not be nullable though.

To your questions:

  1. Required parameters are only processed for missing in the line 195 block. If I add a null check there then optional and nullable will by handled the same way in R clients. I'm fine with this if you prefer it.
  2. Yes, optional parameters are first checked for missing() and then checked for is.null() if the parameter is not nullable.

I think this is ready to go, but what I actually care about is the fix in a4aac2a. If we need further discussion on the nullable checks I'm happy to move those commits to a fresh PR.

{{#maxLength}}
if (nchar(`{{paramName}}`) > {{maxLength}}) {
if (!is.null(`{{paramName}}`) && nchar(`{{paramName}}`) > {{maxLength}}) {
{{#useDefaultExceptionHandling}}
stop("Invalid length for `{{paramName}}` when calling {{classname}}${{operationId}}, must be smaller than or equal to {{maxLength}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -220,7 +233,7 @@
}
{{/maxLength}}
{{#minLength}}
if (nchar(`{{paramName}}`) < {{minLength}}) {
if (!is.null(`{{paramName}}`) && nchar(`{{paramName}}`) < {{minLength}}) {
{{#useDefaultExceptionHandling}}
stop("Invalid length for `{{paramName}}` when calling {{classname}}${{operationId}}, must be bigger than or equal to {{minLength}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -233,7 +246,7 @@
}
{{/minLength}}
{{#maximum}}
if (`{{paramName}}` >{{#exclusiveMaximum}}={{/exclusiveMaximum}} {{maximum}}) {
if (!is.null(`{{paramName}}`) && `{{paramName}}` > {{#exclusiveMaximum}}={{/exclusiveMaximum}} {{maximum}}) {
{{#useDefaultExceptionHandling}}
stop("Invalid value for `{{paramName}}` when calling {{classname}}${{operationId}}, must be smaller than {{^exclusiveMaximum}}or equal to {{/exclusiveMaximum}}{{maximum}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -246,7 +259,7 @@
}
{{/maximum}}
{{#minimum}}
if (`{{paramName}}` <{{#exclusiveMinimum}}={{/exclusiveMinimum}} {{minimum}}) {
if (!is.null(`{{paramName}}`) && `{{paramName}}` < {{#exclusiveMinimum}}={{/exclusiveMinimum}} {{minimum}}) {
{{#useDefaultExceptionHandling}}
stop("Invalid value for `{{paramName}}` when calling {{classname}}${{operationId}}, must be bigger than {{^exclusiveMinimum}}or equal to {{/exclusiveMinimum}}{{minimum}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -259,7 +272,7 @@
}
{{/minimum}}
{{#pattern}}
if (!str_detect(`{{paramName}}`, "{{{pattern}}}")) {
if (!is.null(`{{paramName}}`) && !stringr::str_detect(`{{paramName}}`, "{{{pattern}}}")) {
{{#useDefaultExceptionHandling}}
stop("Invalid value for `{{paramName}}` when calling {{classname}}${{operationId}}, must conform to the pattern {{{pattern}}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -272,7 +285,7 @@
}
{{/pattern}}
{{#maxItems}}
if (length(`{{paramName}}`) > {{maxItems}}) {
if (!is.null(`{{paramName}}`) && length(`{{paramName}}`) > {{maxItems}}) {
{{#useDefaultExceptionHandling}}
stop("Invalid length for `{{paramName}}` when calling {{classname}}${{operationId}}, number of items must be less than or equal to {{maxItems}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -285,7 +298,7 @@
}
{{/maxItems}}
{{#minItems}}
if (length(`{{paramName}}`) < {{minItems}}) {
if (!is.null(`{{paramName}}`) && length(`{{paramName}}`) < {{minItems}}) {
{{#useDefaultExceptionHandling}}
stop("Invalid length for `{{paramName}}` when calling {{classname}}${{operationId}}, number of items must be greater than or equal to {{minItems}}.")
{{/useDefaultExceptionHandling}}
Expand All @@ -307,7 +320,7 @@
{{#isArray}}
{{#uniqueItems}}
# check if items are unique
if (!identical(`{{{paramName}}}`, unique(`{{{paramName}}}`))) {
if (!is.null(`{{paramName}}`) && !identical(`{{{paramName}}}`, unique(`{{{paramName}}}`))) {
{{#useDefaultExceptionHandling}}
stop("Invalid value for {{{paramName}}} when calling {{classname}}${{operationId}}. Items must be unique.")
{{/useDefaultExceptionHandling}}
Expand Down
27 changes: 27 additions & 0 deletions samples/client/echo_api/r/R/body_api.R
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`body`) && is.null(`body`)) {
stop("Invalid value for `body` when calling BodyApi$TestBodyApplicationOctetstreamBinary, `body` is not nullable")
}

if (!is.null(`body`)) {
local_var_body <- `body`$toJSONString()
Expand Down Expand Up @@ -400,6 +403,9 @@ BodyApi <- R6::R6Class(
stop("Missing required parameter `files`.")
}

if (!missing(`files`) && is.null(`files`)) {
stop("Invalid value for `files` when calling BodyApi$TestBodyMultipartFormdataArrayOfBinary, `files` is not nullable")
}

file_params["files"] <- httr::upload_file(`files`)
local_var_url_path <- "/body/application/octetstream/array_of_binary"
Expand Down Expand Up @@ -491,6 +497,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`my_file`) && is.null(`my_file`)) {
stop("Invalid value for `my_file` when calling BodyApi$TestBodyMultipartFormdataSingleBinary, `my_file` is not nullable")
}

file_params["my-file"] <- httr::upload_file(`my_file`)
local_var_url_path <- "/body/application/octetstream/single_binary"
Expand Down Expand Up @@ -582,6 +591,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`pet`) && is.null(`pet`)) {
stop("Invalid value for `pet` when calling BodyApi$TestEchoBodyAllOfPet, `pet` is not nullable")
}

if (!is.null(`pet`)) {
local_var_body <- `pet`$toJSONString()
Expand Down Expand Up @@ -678,6 +690,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`body`) && is.null(`body`)) {
stop("Invalid value for `body` when calling BodyApi$TestEchoBodyFreeFormObjectResponseString, `body` is not nullable")
}

if (!is.null(`body`)) {
local_var_body <- `body`$toJSONString()
Expand Down Expand Up @@ -774,6 +789,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`pet`) && is.null(`pet`)) {
stop("Invalid value for `pet` when calling BodyApi$TestEchoBodyPet, `pet` is not nullable")
}

if (!is.null(`pet`)) {
local_var_body <- `pet`$toJSONString()
Expand Down Expand Up @@ -870,6 +888,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`pet`) && is.null(`pet`)) {
stop("Invalid value for `pet` when calling BodyApi$TestEchoBodyPetResponseString, `pet` is not nullable")
}

if (!is.null(`pet`)) {
local_var_body <- `pet`$toJSONString()
Expand Down Expand Up @@ -966,6 +987,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`body`) && is.null(`body`)) {
stop("Invalid value for `body` when calling BodyApi$TestEchoBodyStringEnum, `body` is not nullable")
}

if (!is.null(`body`)) {
local_var_body <- `body`$toJSONString()
Expand Down Expand Up @@ -1062,6 +1086,9 @@ BodyApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`tag`) && is.null(`tag`)) {
stop("Invalid value for `tag` when calling BodyApi$TestEchoBodyTagResponseString, `tag` is not nullable")
}

if (!is.null(`tag`)) {
local_var_body <- `tag`$toJSONString()
Expand Down
30 changes: 30 additions & 0 deletions samples/client/echo_api/r/R/form_api.R
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,17 @@ FormApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`integer_form`) && is.null(`integer_form`)) {
stop("Invalid value for `integer_form` when calling FormApi$TestFormIntegerBooleanString, `integer_form` is not nullable")
}

if (!missing(`boolean_form`) && is.null(`boolean_form`)) {
stop("Invalid value for `boolean_form` when calling FormApi$TestFormIntegerBooleanString, `boolean_form` is not nullable")
}

if (!missing(`string_form`) && is.null(`string_form`)) {
stop("Invalid value for `string_form` when calling FormApi$TestFormIntegerBooleanString, `string_form` is not nullable")
}

form_params["integer_form"] <- `integer_form`
form_params["boolean_form"] <- `boolean_form`
Expand Down Expand Up @@ -226,6 +235,9 @@ FormApi <- R6::R6Class(
stop("Missing required parameter `marker`.")
}

if (!missing(`marker`) && is.null(`marker`)) {
stop("Invalid value for `marker` when calling FormApi$TestFormObjectMultipart, `marker` is not nullable")
}

form_params["marker"] <- `marker`
local_var_url_path <- "/form/object/multipart"
Expand Down Expand Up @@ -327,11 +339,29 @@ FormApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`form1`) && is.null(`form1`)) {
stop("Invalid value for `form1` when calling FormApi$TestFormOneof, `form1` is not nullable")
}

if (!missing(`form2`) && is.null(`form2`)) {
stop("Invalid value for `form2` when calling FormApi$TestFormOneof, `form2` is not nullable")
}

if (!missing(`form3`) && is.null(`form3`)) {
stop("Invalid value for `form3` when calling FormApi$TestFormOneof, `form3` is not nullable")
}

if (!missing(`form4`) && is.null(`form4`)) {
stop("Invalid value for `form4` when calling FormApi$TestFormOneof, `form4` is not nullable")
}

if (!missing(`id`) && is.null(`id`)) {
stop("Invalid value for `id` when calling FormApi$TestFormOneof, `id` is not nullable")
}

if (!missing(`name`) && is.null(`name`)) {
stop("Invalid value for `name` when calling FormApi$TestFormOneof, `name` is not nullable")
}

form_params["form1"] <- `form1`
form_params["form2"] <- `form2`
Expand Down
15 changes: 15 additions & 0 deletions samples/client/echo_api/r/R/header_api.R
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,25 @@ HeaderApi <- R6::R6Class(
oauth_scopes <- NULL
is_oauth <- FALSE

if (!missing(`integer_header`) && is.null(`integer_header`)) {
stop("Invalid value for `integer_header` when calling HeaderApi$TestHeaderIntegerBooleanStringEnums, `integer_header` is not nullable")
}

if (!missing(`boolean_header`) && is.null(`boolean_header`)) {
stop("Invalid value for `boolean_header` when calling HeaderApi$TestHeaderIntegerBooleanStringEnums, `boolean_header` is not nullable")
}

if (!missing(`string_header`) && is.null(`string_header`)) {
stop("Invalid value for `string_header` when calling HeaderApi$TestHeaderIntegerBooleanStringEnums, `string_header` is not nullable")
}

if (!missing(`enum_nonref_string_header`) && is.null(`enum_nonref_string_header`)) {
stop("Invalid value for `enum_nonref_string_header` when calling HeaderApi$TestHeaderIntegerBooleanStringEnums, `enum_nonref_string_header` is not nullable")
}

if (!missing(`enum_ref_string_header`) && is.null(`enum_ref_string_header`)) {
stop("Invalid value for `enum_ref_string_header` when calling HeaderApi$TestHeaderIntegerBooleanStringEnums, `enum_ref_string_header` is not nullable")
}

header_params["integer_header"] <- `integer_header`

Expand Down
12 changes: 12 additions & 0 deletions samples/client/echo_api/r/R/path_api.R
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,21 @@ PathApi <- R6::R6Class(
stop("Missing required parameter `enum_ref_string_path`.")
}

if (!missing(`path_string`) && is.null(`path_string`)) {
stop("Invalid value for `path_string` when calling PathApi$TestsPathStringPathStringIntegerPathIntegerEnumNonrefStringPathEnumRefStringPath, `path_string` is not nullable")
}

if (!missing(`path_integer`) && is.null(`path_integer`)) {
stop("Invalid value for `path_integer` when calling PathApi$TestsPathStringPathStringIntegerPathIntegerEnumNonrefStringPathEnumRefStringPath, `path_integer` is not nullable")
}

if (!missing(`enum_nonref_string_path`) && is.null(`enum_nonref_string_path`)) {
stop("Invalid value for `enum_nonref_string_path` when calling PathApi$TestsPathStringPathStringIntegerPathIntegerEnumNonrefStringPathEnumRefStringPath, `enum_nonref_string_path` is not nullable")
}

if (!missing(`enum_ref_string_path`) && is.null(`enum_ref_string_path`)) {
stop("Invalid value for `enum_ref_string_path` when calling PathApi$TestsPathStringPathStringIntegerPathIntegerEnumNonrefStringPathEnumRefStringPath, `enum_ref_string_path` is not nullable")
}

local_var_url_path <- "/path/string/{path_string}/integer/{path_integer}/{enum_nonref_string_path}/{enum_ref_string_path}"
if (!missing(`path_string`)) {
Expand Down
Loading
Loading