Swagger-codegen: [R] Issues with R Codegen Clients

[cannin]

I tried to make a new API package using the following with code pulled on September 18. :

java -jar modules/swagger-codegen-cli/target/swagger-codegen-cli.jar -i http://petstore.swagger.io/v2/swagger.json -l r -o /tmp/test

Below are a number of suggestions and bugs:

Minor Issue 1: Include Installation Instructions

This would be useful in the README.md.

Set up to use CRAN package repository

setRepositories(ind=1:6)
options(repos="http://cran.rstudio.com/")
if(!require(devtools)) { install.packages("devtools") }

Install package

library(devtools)
install(".")

First Bug: The above doesn't work.

The code needs a NAMESPACE file in the top directory. Any function that had an @export in the template will need to be represented in the NAMESPACE file as below:

export(func1)
export(func2)

Second Bug: Syntax issue

The NAMESPACE can be generated in R, so I tried to do that, but came across this issue when building the NAMESPACE file using roxygen (http://r-pkgs.had.co.nz/namespace.html):

Error in parse(text = lines, n = -1, srcfile = srcfile) :
/tmp/test/R/ApiResponse.r:47:39: unexpected symbol
46: ApiResponseObject <- jsonlite::fromJSON(ApiResponseJson)
47: self$code <- ApiResponseObject$code

The backtick is in the wrong place, it should be after the dollar sign. Also, the backtick is probably not needed at all unless spaces can appear in the strings.

Third Bug: Use API name in DESCRIPTION

This line:

Package: swagger

will end up conflicting if multiple Swagger packages are installed. Libraries in R are loaded as library(swagger) using the package string. Can "title":"Swagger Petstore" be grabbed instead? The space won't work. Some options: CamelCase or just turn it into all lowercase replacing spaces with underscores or dashes?

Background:

• http://r-pkgs.had.co.nz/package.html (names should be unique)
• https://cran.r-project.org/doc/manuals/r-release/R-exts.html#The-DESCRIPTION-file ([Package] should contain only (ASCII) letters, numbers and dot, have at least two characters and start with a letter and not end in a dot.)

Minor Issue 2:

What is git_push.sh for? Is it necessary? Can it be removed from the code generation?

Fourth Bug: Duplicated code

Response and Element classes are duplicated in several files.

[wing328]

@cannin for the improvement in README.md, do you mind submitting a PR? A good starting point is https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/README.mustache

cc @ramnov

[jschelbert]

I've created a PR for some of the errors. There seemed to be some smaller bugs in the mustache files. Hopefully, these are now fixed. However, the issue about NAMESPACE and DESCRIPTION remain.

[albsantosdel]

I have tested this version and most of the errors identified by @cannin have been solved. It is still necessary to create the NAMESPACE file but swagger-codegen generates valid R code. I found though several errors:

I generated the R code for the Pet shop example (http://petstore.swagger.io/) using:
java -jar ../swagger-codegen/modules/swagger-codegen-cli/target/swagger-codegen-cli.jar generate -i http://petstore.swagger.io/v2/swagger.json -l r -o .

Installed the generated R package:

install.packages('devtools')
library(devtools)
install('.')
devtools::document() #Generate NAMESPACE file

Run "Get user by user name" example (http://petstore.swagger.io/#/user/getUserByName):

library(swagger)
userApi = swagger::UserApi$new() 
user = userApi$get_user_by_name(username = "user1")
user$content$id
user$content$username
user$content$email

_Response:_
Error in user$new()$fromJSON(httr::content(resp, "text", encoding = "UTF-8"), :
unused argument (simplifyVector = FALSE)

I deleted this argument from the generated R code (UserApi.r) and run again:
_Response:_
"API client error"

I debugged the actual GET request sent and the path ('/user/') seems to be missing:

resp <- httr::GET(paste0(self$basePath,username),
          httr::add_headers("User-Agent" = self$userAgent, 
                "content-type" = "application/xml"))

I believe the path needs to be added to paste0() here:
https://github.com/jschelbert/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/api.mustache#L17

I added manually the right path (UserApi.r:81):

resp <- httr::GET(paste0(self$basePath,"/user/",username),
          httr::add_headers("User-Agent" = self$userAgent, "content-type" = "application/xml"))

_Response:_
I should have obtained a User object but instead I got an integer:

user
[1] 0

The generated code seems not to work as a one-liner here (UserApi.r:86):
result <- User$new()$fromJSON(httr::content(resp, "text", encoding = "UTF-8"))
Corresponds to:
https://github.com/jschelbert/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/api.mustache#L45

If instead it is divided into 2, it works:

result <- User$new()
result$fromJSON(httr::content(resp, "text", encoding = "UTF-8"))

_Response:_

user$content$id
[1] 0
user$content$username
[1] "user1"
user$content$email
[1] "string"

[wing328]

@albsantosdel thanks for testing the fix. I've merged it into master given that it fixes most of the issues.

[wing328]

@albsantosdel for the missing path issue with https://github.com/jschelbert/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/api.mustache#L17, I think we need to revise it with {{path}} as follows:

      resp <- httr::{{httpMethod}}(paste0(self$basePath, "{{path}}", {{#pathParams}}, {{paramName}}{{/pathParams}}),

Would you have time to submit a PR with the fix (together with the fix for https://github.com/jschelbert/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/api.mustache#L45)?

[wing328]

@albsantosdel Looks like https://github.com/swagger-api/swagger-codegen/pull/6571 will fix the issues you encountered. Do you mind testing the PR when you've time?

git checkout -b brnleehng-fix/r-mustache-templates master
git pull https://github.com/brnleehng/swagger-codegen.git fix/r-mustache-templates

cc @brnleehng

[brnleehng]

The url path needs string replacement instead of appending at the end.

For example, /{{path parameter}}/path will not work for the url path. I've updated my PR for this use case #6571

[albsantosdel]

I have tested the PR #6571 (without the fixes done few minutes ago) and I found several issues:

1. Generated DESCRIPTION file is not correct:
_Response:_

• installing source package ‘Swagger Petstore’ ...
  Error : Invalid DESCRIPTION file

Malformed package name

See section 'The DESCRIPTION file' in the 'Writing R Extensions'
manual.

ERROR: installing package DESCRIPTION failed for package ‘Swagger Petstore’

• removing ‘/Library/Frameworks/R.framework/Versions/3.4/Resources/library/Swagger Petstore’
  Error: Command failed (1)

I went back to Package: {{{packageName}}} and it worked just fine.

2. user$apiClient is NULL:
In UserApi, apiClient is equal to NULL and never initialized, so when running the test you get an error:

library(swagger)
userApi = swagger::UserApi$new() 
user = userApi$get_user_by_name(username = "testUser")

_Response:_
Error in userApi$get_user_by_name(username = "user1") :
attempt to apply non-function

To solve the error, I modified the generated code to include the initialization (UserApi.r:52):
apiClient = ApiClient$new()

3. When the previous error was solved, the request was sent but I got

_Response:_
"API client error"

The generated url is incorrect ("http://petstore.swagger.io/v2/User/testUser") because the path is "User" instead of "user". If this is changed in the generated code (UserApi.r: 51) the test example works:

library(swagger)
userApi = swagger::UserApi$new() 
user = userApi$get_user_by_name(username = "testUser")
class(user)
user$content$id
user$content$username
user$content$email

[brnleehng]

1) Yeah, I'll change the description to use {{{packageName}}} instead of {{{appName}}}
I didn't know you can add additional properties on the command line.

For reference:
java -jar "modules\swagger-codegen-cli\target\swagger-codegen-cli.jar" generate -i "http://petstore.swagger.io/v2/swagger.json" -l "r" -o "c:\temp\petstore_client" --additional-properties "packageName=PetStoreSwagger"

2) Added the suggestion above in my PR #6571

3) Url paths need be string replace instead of append.

Changed to:
urlPath <- "{{path}}"
{{#hasPathParams}}
{{#pathParams}}
if (!missing({{paramName}})) {
urlPath <- gsub(paste0("\{", "{{baseName}}", "\}"), {{paramName}}, urlPath)
}
{{/pathParams}}
{{/hasPathParams}}

[wing328]

https://github.com/swagger-api/swagger-codegen/pull/6571 has been merged into master.

Please pull the latest master to give it a try.

[ramnov]

Hi @wing328 , I pulled the latest master and generated PetStoreSwagger package and was able to install it successfully. Is there some sample code that I can try (say create user).

[wing328]

@ramnov I remember you created https://github.com/swagger-api/swagger-codegen/pull/6307/files#diff-cffab5bb150fcd367f23f42f14993fd6 for testing Petstore. We should probably add it to https://github.com/swagger-api/swagger-codegen/tree/master/samples/client/petstore/R and setup integration tests in CI (travis, shippable, etc)

For sample code, we add auto-generated documentation, which is not yet implemented in the R generator. I'll create a separate task to track it.

[ramnov]

I tested with the following code :

apiClient <- PetStoreSwagger::ApiClient$new("http://petstore.swagger.io/v2")
petStoreClient <- PetStoreSwagger::PetApi$new(apiClient)
petId <- sample(1:1000, 1)
pet <- PetStoreSwagger::Pet$new(petId,
Element$new(0,"test"),
"test",
list("test"),
list(Element$new(0, "test")),
"available")
response <- petStoreClient$addPet(pet)

Got the following error message at the final line :
Error: attempt to apply non-function

Will investigate and let you know when I find more.

[wing328]

For sample code, we add auto-generated documentation, which is not yet implemented in the R generator. I'll create a separate task to track it.

Created https://github.com/swagger-api/swagger-codegen/issues/6697 for tracking.

[ramnov]

I found 3 issues :

1. list is surrounded by 2 quotes rather than a single quote. For example, running the following code :

pet <- PetStoreSwagger::Pet$new(petId,
Element$new(0,"test"),
"test",
list("http://test"),
list(Element$new(0, "test")),
"available")
pet$toJSONString()
[1] "{n \"id\": 1,n \"category\": {\"id\":0,\"name\":\"test\"},n \"name\": \"test\",n \"photoUrls\": [\"\"http://test\"\"],n \"tags\": [{\"id\":0,\"name\":\"test\"}],n \"status\": \"available\"n }"

Note that http://test is surrounded by 2 quotes.

1. For all requests, body must be a JSON string , not json object

if (!missing(body)) {
body <- body$toJSONString()
}

1. I am still trying to figure out how to pass this header : "Content-Type" : "application/json". Currently I have a workaround of using content_type_json() as an additional parameter :

httr::POST("http://petstore.swagger.io/v2/pet", queryParams = NULL, body = pet$toJSONString(), content_type_json())

Issue number 1 and 2 need code fixes. I have opened an issue with httr repo regarding issue 3 : https://github.com/r-lib/httr/issues/489

[cderv]

1. I am still trying to figure out how to pass this header : "Content-Type" : "application/json". Currently I have a workaround of using content_type_json() as an additional parameter :
   httr::POST("http://petstore.swagger.io/v2/pet", queryParams = NULL, body = pet$toJSONString(), content_type_json())

You could try

httr::POST("http://petstore.swagger.io/v2/pet", queryParams = NULL, body = pet$toJSONString(), add_headers(`Content-Type` = "application/json"))

[ramnov]

@cderv : Yes that worked ! I believe this is how we should pass it using ApiClient :

apiClient <- PetStoreSwagger::ApiClient$new("http://petstore.swagger.io/v2", NULL, c(Content-Type = "application/json"))

[ramnov]

I have created a PR for issue 1 and 2 : https://github.com/swagger-api/swagger-codegen/pull/6743

Will test it and let you know if I succeed with adding a pet.

[wing328]

6743 has been merged into master.

[wing328]

For content-type and accept, we can use consumes, produces and write a function to select the proper header value similar to what we did in other generators, e.g. https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/csharp/api.mustache#L212-L228

[ramnov]

Found the real reason why this message "Error: attempt to apply non-function" was thrown :
add_pet looks like this :

add_pet = function(body, ...){
args <- list(...)
body <- NULL
queryParams <- list()
headerParams <- character()
if (!missing(body)) {
body <- body$toJSONString()
}

we are initializing body to NULL and then trying to convert into a JSON String. Error is thrown in body <- body$toJSONString().

@wing328 : do you remember why these variables are set to these values before passing to self$apiClient$callApi

https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/api.mustache#L38-L40

[wing328]

I do not recall the reason. One thing I would like to point out is that the body parameter can be optional so it makes sense to me body is initialized to NULL

I thought the following line is a NULL check:

if (!missing(body)) {
...
}

If it's not, we need to replace it with a proper null check.

Disclaimer: not an expert in R

[cderv]

I am not following everything from the beginning as I recently found this thread and because I am more a R expert than a Java expert.
In the snippet by @ramnov, as body is set to NULL inside the function but is also an argument of the function, the argument value is replace to NULL whatever is provided as argument. It is not used. Seems strange.

missing could be used inside function to test if an argument is provided or nor by a function call. Here if you want to test for NULL value, the function should be is.null. However, in the current code snippet it will always be TRUE because body is set to NULL each time.

If I understand how to generate the code and install the package, I could test and review the R code to see if something is wrong.

[ramnov]

@wing328 : correct code should be like this : if body parameter is passed, convert it to json string, else, use NULL :

if (!missing(body)) {
body <- body$toJSONString()
} else {
body <- NULL
}

@cderv : Here are some steps that I followed to install and test the petstore package :

1. Install maven, java, git, R
2. git clone https://github.com/swagger-api/swagger-codegen
3. cd swagger-codegen
4. mvn clean package
5. java -jar "modules\swagger-codegen-cli\target\swagger-codegen-cli.jar" generate -i "http://petstore.swagger.io/v2/swagger.json" -l "r" -o "c:\temp\petstore_client" --additional-properties "packageName=PetStoreSwagger"
6. R CMD build "C:\temp\petstore_client"
7. R CMD INSTALL PetStoreSwagger_1.0.0.tar.gz
8. library(PetStoreSwagger) in any R IDE

Please try it out and let me know if you succeed.

[wing328]

UPDATE: submitted https://github.com/swagger-api/swagger-codegen/pull/6786 to fix "Error: attempt to apply non-function"

[wing328]

@cderv the fix (#6786) has been merged into master. Please give it a try and let us know if you still see issues.

(if you need more information on using the SNAPSHOT version of the latest, please let me know)

[ramnov]

Hi @wing328

I built the latest master code and tested the following code to add a pet :

pet <- Pet$new(sample(1:1000, 1),
Element$new(1,"testpet"),
"test",
list("test"),
list(Element$new(1, "test")),
"available")
PetApi$new(ApiClient$new())$add_pet(pet, httr::content_type_json())$response

I get a 400 response. Does that mean I am not authorized to add a pet ?

I am going to test all the possible APIs from http://petstore.swagger.io/ and will report if I find any more bugs.

On a side note :
I see 2 other empty parameters in api.mustache

queryParams <- list()
headerParams <- character()

These 2 values are always empty when passing to callApi. Should we add these to ApiClient.R or get these values from user for every single function. If we decide to get it from the user , it will be like this :

create_users_with_array_input = function(body, queryParams, headerParams, ...){
args <- list(...)
if (missing(queryParams)) {
queryParams <- list()
}
if (missing(headerParams)) {
headerParams <- character()
}

[wing328]

There's no permission need to add Pet to the Petstore server.

We need to focus on the following code:

https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/r_test/R/PetApi.r#L60-L87

The code looks good to me although I'm no expert in R.

Can we have another pair of eyes to review the code assuming API client's callApi is doing the right thing?

[wing328]

If we decide to get it from the user , it will be like this :

The user shouldn't need to do that (they don't even need to know the difference between query and header parameters)

[sneumann]

Hi, just starting to look into the R code. A quick stopgap fix
for the above mentioned issue with the NAMESPACE (see https://github.com/swagger-api/swagger-codegen/issues/6520#issuecomment-332220726 )
would be a file NAMESPACE with the fixed content

exportPattern("^[^\\.]")`

before the proper fix would be to enumerate all actual R6 classes and functions generated
by the RClientCodegen.java. I don't yet know how to code that in RClientCodegen.java,
so no PR so far. Yours, Steffen

[sneumann]

[Would you prefer individual small issues, then I can split this off.
That would also allow to reference/close an issue with a commit.]

Using a fresh checkout (acf70d04815c422799ece5c6fb1dfbaf5a4f7144)
and the code generated by bin/r-petstore.sh I fail to generate an object from JSON:

library(petstore)
s <- '{"id": 605, "category": {"id":1,"name":"testpet"}, "name": "test", "photoUrls": ["test"], "tags": [{"id":1,"name":"test"}], "status": "available"}'
p <- Pet$new()
p <- p$fromJSONString(s)

resulting in the above mentioned Error in p$fromJSONString(s) : object 'CategoryObject' not found.

For that I had to manually fix the generated code, because in https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/r/model.mustache#L130
{{datatype}}Object -> {{datatype}}$new()
should be {{datatype}}Object <- {{datatype}}$new()

[wing328]

{{datatype}}Object -> {{datatype}}$new()
should be {{datatype}}Object <- {{datatype}}$new()

@sneumann Do you mind filing a PR with the suggested fix?

[wing328]

Thanks @sneumann for the fix, which has been merged into 2.4.0.

[wing328]

For the NAMESPACE file, isn't that generated by roxygen2: https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/R/NAMESPACE?

We can definitely update the R client generator to auto-generate it but just want to make sure first.

[sneumann]

Hi, for the petstore, there are two generated R codebases:
swagger-codegen/samples/client/petstore/R/ which contains a roxygenerated NAMESPACE,
and /swagger-codegen/samples/client/petstore/r_test/ last modified by you. So I guess
what's missing is the call to run roxygen in r-petstore to generate the NAMESPACE before committing.
I am unsure where/how swagger would call roxygen2::roxygenise() (https://github.com/yihui/roxygen2/blob/master/vignettes/roxygen2.Rmd#running-roxygen) , so I can't readily write s PR.
What you'd need to generate the NAMESPACE is:

library(roxygen2)
roxygenize("/swagger-codegen/samples/client/petstore/r_test/", roclets="namespace")

where roclets defines what gets generated, and indeed there are no r_test/manpages,
so calling roxygenize("/swagger-codegen/samples/client/petstore/r_test/") will generate all
of "collate", "namespace", "rd". Yours, Steffen

[wing328]

@sneumann I've added the auto-generated NAMESPACE file via #7467

Please pull the latest master or use the SNAPSHOT version to give it a try

[wing328]

I'll consolidate swagger-codegen/samples/client/petstore/R/, swagger-codegen/samples/client/petstore/r_test/ into one shortly...

[calbach]

I've found several bugs in the generated R code on 2.4.0-SNAPSHOT - in the interest of avoiding duplicate issue filing is there a more recent version I should be testing against? If not - would you prefer I file these as new distinct issues or add to this cover issue?

[HugoMario]

Hello @calbach, please, file these bugs you find and ping me there. thanks!

[calbach]

Hi @HugoMario, finally got a chance to file issues: #8607 #8608 #8609 #8610 #8611 #8612 #8613

[HugoMario]

thanks a lot @calbach !!!

[HugoMario]

hey @calbach, i've been solving a couple of issues related with R generator. i have some limitations, because i'm not so familiar with this language, if you can give me some guide about the expected output, it would be really helpful to get things done quicker.

[calbach]

Sure, I added expected output to the tickets where it made sense. I'm not an expert on Swagger codegen JSON conventions particularly on when to output null vs undefined, so you may need to consult with someone else on that.