-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Scala] Fix async helper methods when body is optional #7274
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is most likely fine.
I wanted to comment on the "body is optional" comment, though.
I believe None
would get serialized as null
here, and not an empty body. The "required" property of a parameter for Swagger/OpenAPI 2.0 specs says:
Determines whether this parameter is mandatory. If the parameter is in "path", this property is required and its value MUST be true. Otherwise, the property MAY be included and its default value is false.
It's been a few years since I've used json4s, but I believe the way to truly support required=false
for body parameters per swagger spec would be to implement a custom serializer per type and only write when the Option value is defined.
This isn't a deficiency in your PR, it's a confusion of the 2.0 specification, in which required
refers to the json structure and not the data within the structure. This is fixed in Open API 3.0.1 for schema objects using the nullable
attribute (see https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.1.md#schemaNullable).
I might be wrong (haven't actually tested this yet), but it looks like the async methods will write an empty body. |
CircleCI reported the following errors:
I wonder if you can take a look when you've time. UPDATE: I merged with master locally and no longer can repeat the error. I've pushed the branch to CircleCI to verify again. Let's see how it goes. |
CircleCI tests passed via https://circleci.com/gh/swagger-api/swagger-codegen/3321 |
* master: (26 commits) [Scala] Fix async helper methods when body is optional (swagger-api#7274) [Rust] Recommend style based on 'rustfmt' defaults (swagger-api#7335) [Java:vertx] Initialize router in init method and re-use router member to create S… (swagger-api#7234) [Scala] Fix missing json4s import (swagger-api#7271) deploy snapshot version 2.3.1 [Ada] Add Ada support for server code generator swagger-api#6680 (swagger-api#7256) add shijinkui to scala technical committee Generate swagger yaml for go client (swagger-api#7281) use openjdk7 in travis to ensure it works with jdk7 docs(readme): update link to contributing guid (swagger-api#7332) Fix a regression bug that was introduce in a recent commit. Removed the tabs that were causing error in Play Framework (swagger-api#7241) Fix issue swagger-api#7262 with the parameter name in the path. The problem was that camelCase naming was forced only in this part of the code when everywhere else it is configurable. (swagger-api#7313) Java8 fix (swagger-api#7260) update to 2.3.1-SNAPSHOT fix typo, update 2017 to 2018 [Doc] add huawei cloud to companies list swagger-api#7308 (swagger-api#7309) Adding Peatio opensource as reference project (swagger-api#7267) Update README.md (swagger-api#7298) Update README.md (swagger-api#7299) [all] sys props in CodegenConstants ...
* [Scala] Fix async helper methods when body is optional (#7274) * [Scala] Fix async helper methods when body is optional Closes #7272 * Update petstore sample * Standardize all isRestfulxxx methods. Change 'equals' method to 'equalsIgnoreCase' for isRestfulIndex method in Operations. This matches isRestfulCreate, isRestfulShow, isRestfulDestroy
* [Scala] Fix async helper methods when body is optional (swagger-api#7274) * [Scala] Fix async helper methods when body is optional Closes swagger-api#7272 * Update petstore sample * Standardize all isRestfulxxx methods. Change 'equals' method to 'equalsIgnoreCase' for isRestfulIndex method in Operations. This matches isRestfulCreate, isRestfulShow, isRestfulDestroy
Closes #7272
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.3.0.0
branch for changes related to OpenAPI spec 3.0. Default:master
.Description of the PR
Closes #7272