add support for shared responses defs
#87
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kosta-Github
commented
Jun 26, 2019
| * @method | ||
| * @name ResponsesApi#get_person | ||
| */ | ||
| get_person(parameters: {} & CommonRequestOptions): Promise < ResponseWithBody < 200, void > | ResponseWithBody < 201, Response_get_person_201 > | ResponseWithBody < 202, direct_schema_ref > | ResponseWithBody < 203, void > | ResponseWithBody < 204, regular_schema > | ResponseWithBody < 205, some_def > | ResponseWithBody < 206, same_name > | ResponseWithBody < 207, Response_same_name_clash >> { |
There was a problem hiding this comment.
ResponseWithBody always provides the correct type now; no more untyped object declarations
| "204": { "$ref": "#/responses/regular_schema" }, | ||
| "205": { "$ref": "#/responses/indirect_schema" }, | ||
| "206": { "$ref": "#/responses/same_name" }, | ||
| "207": { "$ref": "#/responses/same_name_clash" } |
There was a problem hiding this comment.
response codes 200 to 207 should enumerate all possible combinations with respect to response schema definitions
Markionium
reviewed
Jul 1, 2019
There was a problem hiding this comment.
Thanks a lot for doing the work and apologies for the somewhat slow response. @mtennoe is on holiday and it's been a pretty last couple days for me.
| @@ -76,6 +77,65 @@ export function getViewForSwagger2(opts: CodeGenOptions): ViewData { | |||
| }; | |||
| } | |||
|
|
|||
| function normalizeResponseDefinitions(swagger: any): any { | |||
| // ensure that the optional swagger.responses and swagger.definitions fields are present | |||
| swagger.responses = swagger.responses || {}; | |||
There was a problem hiding this comment.
It might be nicer to use a local variable here? We're creating this optional empty object on the swagger object and then later delete it.
In any case we're mutating the parameter which isn't really necessary.
There was a problem hiding this comment.
I'm ok to merge this as is for now.
| @@ -76,6 +77,65 @@ export function getViewForSwagger2(opts: CodeGenOptions): ViewData { | |||
| }; | |||
| } | |||
|
|
|||
| function normalizeResponseDefinitions(swagger: any): any { | |||
There was a problem hiding this comment.
We can probably use CodeGenOptions['swagger'] here and for the return type.
There was a problem hiding this comment.
@Kosta-Github I think once we adjust this type and resolve the conflict we can merge this. Since you mentioned you do not have so much time now, if you could resolve the conflict I can fix this up separately.
| ([path, httpVerb, op]) => { | ||
| const responses = op.responses; | ||
|
|
||
| Object.entries<any>(responses).forEach(([resCode, resDef]) => { |
There was a problem hiding this comment.
This probably has a better type once we use the correct type for the function parameter?
There was a problem hiding this comment.
I think we won't need here once we have the type in the comment above fixed.
|
@Markionium based on your comments, I am not sure if I should change something in this PR to get it accepted... |
* `swagger` supports the definition of shared `responses` objects which can be ref'ed by several endpoints * see: https://github.com/reverb/swagger-spec/blob/4678d9fbe5723cb926b50fe0d1ae877c34e44a42/schemas/v2.0/schema.json#L81 * in order to deal with this potential additional level of indirection the swagger schema is normalized with respect to the `responses` defs before the existing algorithm is triggered E.g. sample input schema: ```json { "swagger": "2.0", "info": { "version": "0.0.1", "title": "your title" }, "paths": { "/persons": { "get": { "operationId": "get_person", "description": "Gets `Person` object.", "responses": { "200": { "description": "empty schema" }, "201": { "description": "inline schema", "schema": { "type": "object", "properties": { "inline_schema": { "type": "string" } } } }, "202": { "description": "inline schema ref", "schema": { "$ref": "#/definitions/direct_schema_ref" } }, "203": { "$ref": "#/responses/empty_schema" }, "204": { "$ref": "#/responses/regular_schema" }, "205": { "$ref": "#/responses/indirect_schema" }, "206": { "$ref": "#/responses/same_name" }, "207": { "$ref": "#/responses/same_name_clash" } } } } }, "responses": { "empty_schema": { "description": "empty schema" }, "regular_schema": { "description": "regular schema", "schema": { "type": "object", "properties": { "regular_schema": { "type": "string" } } } }, "indirect_schema": { "description": "indirect schema", "schema": { "$ref": "#/definitions/some_def" } }, "same_name": { "description": "same name schema", "schema": { "$ref": "#/definitions/same_name" } }, "same_name_clash": { "description": "same name clash schema", "schema": { "type": "object", "properties": { "same_name_clash_response": { "type": "string" } } } } }, "definitions": { "direct_schema_ref": { "type": "object", "properties": { "direct_schema_ref": { "type": "string" } } }, "some_def": { "type": "object", "properties": { "some_def": { "type": "string" } } }, "same_name": { "type": "object", "properties": { "same_name": { "type": "string" } } }, "same_name_clash": { "type": "object", "properties": { "same_name_clash_definition": { "type": "string" } } } } } ``` gets normalized to: ```json { "swagger": "2.0", "info": { "version": "0.0.1", "title": "your title" }, "paths": { "/persons": { "get": { "operationId": "get_person", "description": "Gets `Person` object.", "responses": { "200": { "description": "empty schema" }, "201": { "description": "inline schema", "schema": { "$ref": "#/definitions/Response_get_person_201" } }, "202": { "description": "inline schema ref", "schema": { "$ref": "#/definitions/direct_schema_ref" } }, "203": { "description": "empty schema" }, "204": { "description": "regular schema", "schema": { "$ref": "#/definitions/regular_schema" } }, "205": { "description": "indirect schema", "schema": { "$ref": "#/definitions/some_def" } }, "206": { "description": "same name schema", "schema": { "$ref": "#/definitions/same_name" } }, "207": { "description": "same name clash schema", "schema": { "$ref": "#/definitions/Response_same_name_clash" } } } } } }, "definitions": { "direct_schema_ref": { "type": "object", "properties": { "direct_schema_ref": { "type": "string" } } }, "some_def": { "type": "object", "properties": { "some_def": { "type": "string" } } }, "same_name": { "type": "object", "properties": { "same_name": { "type": "string" } } }, "same_name_clash": { "type": "object", "properties": { "same_name_clash_definition": { "type": "string" } } }, "regular_schema": { "type": "object", "properties": { "regular_schema": { "type": "string" } } }, "Response_same_name_clash": { "type": "object", "properties": { "same_name_clash_response": { "type": "string" } } }, "Response_get_person_201": { "type": "object", "properties": { "inline_schema": { "type": "string" } } } } } ``` * inline response types will be removed and be replaced an explicit response type definition (either using an existing response type name) or generating it for the corresponding endpoint, method, and response code * this will also improve the response type definitions from something like `ResponseWithBody < 200, object >` to a more meaningful declaration like: `ResponseWithBody < 200, Response_findById_200 >`
Kosta-Github
force-pushed
the
response-defs
branch
from
5c2293a
to
03651aa
Compare
|
@Markionium can you please check again? |
|
Thanks a lot! |
|
Published as |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
swaggersupports the definition of sharedresponsesobjects which can be ref'ed by several endpointsresponsesdefs before the existing algorithm is triggeredE.g. sample input schema:
{ "swagger": "2.0", "info": { "version": "0.0.1", "title": "your title" }, "paths": { "/persons": { "get": { "operationId": "get_person", "description": "Gets `Person` object.", "responses": { "200": { "description": "empty schema" }, "201": { "description": "inline schema", "schema": { "type": "object", "properties": { "inline_schema": { "type": "string" } } } }, "202": { "description": "inline schema ref", "schema": { "$ref": "#/definitions/direct_schema_ref" } }, "203": { "$ref": "#/responses/empty_schema" }, "204": { "$ref": "#/responses/regular_schema" }, "205": { "$ref": "#/responses/indirect_schema" }, "206": { "$ref": "#/responses/same_name" }, "207": { "$ref": "#/responses/same_name_clash" } } } } }, "responses": { "empty_schema": { "description": "empty schema" }, "regular_schema": { "description": "regular schema", "schema": { "type": "object", "properties": { "regular_schema": { "type": "string" } } } }, "indirect_schema": { "description": "indirect schema", "schema": { "$ref": "#/definitions/some_def" } }, "same_name": { "description": "same name schema", "schema": { "$ref": "#/definitions/same_name" } }, "same_name_clash": { "description": "same name clash schema", "schema": { "type": "object", "properties": { "same_name_clash_response": { "type": "string" } } } } }, "definitions": { "direct_schema_ref": { "type": "object", "properties": { "direct_schema_ref": { "type": "string" } } }, "some_def": { "type": "object", "properties": { "some_def": { "type": "string" } } }, "same_name": { "type": "object", "properties": { "same_name": { "type": "string" } } }, "same_name_clash": { "type": "object", "properties": { "same_name_clash_definition": { "type": "string" } } } } }gets normalized to:
{ "swagger": "2.0", "info": { "version": "0.0.1", "title": "your title" }, "paths": { "/persons": { "get": { "operationId": "get_person", "description": "Gets `Person` object.", "responses": { "200": { "description": "empty schema" }, "201": { "description": "inline schema", "schema": { "$ref": "#/definitions/Response_get_person_201" } }, "202": { "description": "inline schema ref", "schema": { "$ref": "#/definitions/direct_schema_ref" } }, "203": { "description": "empty schema" }, "204": { "description": "regular schema", "schema": { "$ref": "#/definitions/regular_schema" } }, "205": { "description": "indirect schema", "schema": { "$ref": "#/definitions/some_def" } }, "206": { "description": "same name schema", "schema": { "$ref": "#/definitions/same_name" } }, "207": { "description": "same name clash schema", "schema": { "$ref": "#/definitions/Response_same_name_clash" } } } } } }, "definitions": { "direct_schema_ref": { "type": "object", "properties": { "direct_schema_ref": { "type": "string" } } }, "some_def": { "type": "object", "properties": { "some_def": { "type": "string" } } }, "same_name": { "type": "object", "properties": { "same_name": { "type": "string" } } }, "same_name_clash": { "type": "object", "properties": { "same_name_clash_definition": { "type": "string" } } }, "regular_schema": { "type": "object", "properties": { "regular_schema": { "type": "string" } } }, "Response_same_name_clash": { "type": "object", "properties": { "same_name_clash_response": { "type": "string" } } }, "Response_get_person_201": { "type": "object", "properties": { "inline_schema": { "type": "string" } } } } }ResponseWithBody < 200, object >to a more meaningful declaration like:ResponseWithBody < 200, Response_findById_200 >