-
-
Notifications
You must be signed in to change notification settings - Fork 6.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
Kotlin-Spring template fixes/improvements #3007
Conversation
…case last element is empty.
…n definition. Allowing also other responses 201, 202 ...
…n definition. Allowing also other responses 201, 202 ...
@drejc thanks for the PR. Can you please run |
cc @jimschubert (2017/09), @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) |
I have run the ./bin/kotlin-springboot-petstore-server.sh and the newly generated files are "broken" due to missing success response definitions in /2_0/petstore.yaml file. Where for instance there is the following definition: delete:
tags:
- user
summary: Delete user
description: This can only be done by the logged in user.
operationId: deleteUser
produces:
- application/xml
- application/json
parameters:
- name: username
in: path
description: The name that needs to be deleted
required: true
type: string
responses:
'400':
description: Invalid username supplied
'404':
description: User not found The generated code is: @ApiOperation(
value = "Deletes a pet",
nickname = "deletePet",
notes = "",
authorizations = [Authorization(value = "petstore_auth", scopes = [AuthorizationScope(scope = "write:pets", description = "modify pets in your account"), AuthorizationScope(scope = "read:pets", description = "read your pets")])])
@ApiResponses(
value = [ApiResponse(code = 400, message = "Invalid pet value")])
@RequestMapping(
value = ["/pet/{petId}"],
method = [RequestMethod.DELETE])
fun deletePet(@ApiParam(value = "Pet id to delete", required=true) @PathVariable("petId") petId: Long
,@ApiParam(value = "" ) @RequestHeader(value="api_key", required=false) apiKey: String?
): ResponseEntity<Unit> {
return ResponseEntity(service.deletePet(petId, apiKey), HttpStatus.valueOf(400))
} Previously it was: HttpStatus.OK Now I didn't dive into swagger definition details if 200 is by default the expected default response, nevertheless either:
|
Updated APIs
I've seen many spec without 200 response defined in the endpoints so I think we need a better way to handle this. |
Yes I agree ... and I have seen a similar approach in /JavaSpring/api.mustache where response codes are generated: @ApiResponses(value = { {{#responses}}
@ApiResponse(code = {{{code}}}, message = "{{{message}}}"{{#baseType}}, response = {{{baseType}}}.class{{/baseType}}{{#containerType}}, responseContainer = "{{{containerType}}}"{{/containerType}}){{#hasMore}},{{/hasMore}}{{/responses}} })
{{#implicitHeaders}} And the generated code: @ApiOperation(value = "Delete purchase order by ID", nickname = "deleteOrder", notes = "For valid response try integer IDs with value < 1000. Anything above 1000 or nonintegers will generate API errors", tags={ "store", })
@ApiResponses(value = {
@ApiResponse(code = 400, message = "Invalid ID supplied"),
@ApiResponse(code = 404, message = "Order not found") })
@RequestMapping(value = "/store/order/{order_id}",
method = RequestMethod.DELETE)
default ResponseEntity<Void> deleteOrder(@ApiParam(value = "ID of the order that needs to be deleted",required=true) @PathVariable("order_id") String orderId) {
return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);
} I would suggest to manually add the 200 response as the first default response in case there is no other non-error response definition, as this is the expected behaviour. This allows for customisation of 2** responses, and will not break existing APIs |
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.
LGTM. I'm agree that petstore spec need to be improved to cover at least one successful status code for each operation. I will try to fill a PR for this purpose.
FIY https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#responsesObject
|
@karismann please ping me via Gitter to have a chat on this before filing a PR. |
Description of the PR
Fixed consumes/produces comma placement.
Sometimes following happened: produces = ["application/json", ],
Changed default response code in interface returnValue from HttpStatus.OK to first response code in definition. Allowing other responses, like created, accepted ...