[typescript-fetch] to fix incorrect parsing with additional properties #20923
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -265,7 +265,7 @@ export class {{classname}} extends runtime.BaseAPI { | |
| {{/isEnumRef}} | ||
| {{^isEnumRef}} | ||
| {{^withoutRuntimeChecks}} | ||
| formParams.append('{{baseName}}', new Blob([JSON.stringify({{returnType}}ToJSON(requestParameters['{{paramName}}']))], { type: "application/json", })); | ||
|
Contributor
There was a problem hiding this comment. can we add a fixture (generated sample) to the repository that shows this change? Or have the samples not been regenerated?
Contributor
Author
There was a problem hiding this comment. I think the only real changes I got from running
Contributor
Author
There was a problem hiding this comment. @joscha Okay, so I retried on WSL and it seems the There was a problem hiding this comment. this seems to be only working when additionalProperties exists it is causing an issue where the param ( for the case shown below, it puts in e.g. requestBody:
description: entity for which custom field will be added such as fund, deal, asset, investor etc
content:
multipart/form-data:
schema:
$ref: "#/components/schemas/RequestPayload"
...
RequestPayload:
type: object
properties:
context:
$ref: "#/components/schemas/RequestContext"
...
RequestContext:
type: object
properties:
...
Member
There was a problem hiding this comment. @paras-fundwave thanks for reporting! would you want to send a PR to fix that? There was a problem hiding this comment. sure thing! while I'll familiarise myself with the codebase, can you help confirm the approach I have in mind we'll need to have 2 outputs (for this line) depending on whether from this PR I can see I'll need to add test-cases too, anything else I'm missing that can help but might not be mentioned in the contributing.md?
Member
There was a problem hiding this comment. sg, no objections, although i didnt look into the issue in detail There was a problem hiding this comment. looks like how datatype is processed has changed too in b/w v7.0.0 and v7.1.0 reverting to
Contributor
Author
There was a problem hiding this comment. Ah, I see. It seems like an inverted mustache section might come in handy. Were you able to spot any other issues around this? Thanks for pointing this out
Contributor
There was a problem hiding this comment. Seeing similar issues on 7.14.0 with a discriminated schema where returnType is null and ends up generating invalid code: 7.12.0 generates OK, 7.13.0 onwards is broken. I haven't read up on the original issue fully -- but as the line in question is part of building up the request, using the return type for serializing request parameters seems wrong?
Contributor
There was a problem hiding this comment. Diving into the test I don't think the assertion is right. The schema for the request body is:
The code that gets generated to append the part for the form in the request: But that is surely wrong, as ResponseOfString is the response of the operation, not related to the request body? I have raised a PR to revert: #21542 (FYI @macjohnny) |
||
| {{/withoutRuntimeChecks}}{{#withoutRuntimeChecks}} | ||
| formParams.append('{{baseName}}', new Blob([JSON.stringify(requestParameters['{{paramName}}'])], { type: "application/json", })); | ||
| {{/withoutRuntimeChecks}} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| { | ||
| "openapi": "3.0.1", | ||
| "info": { | ||
| "title": "Sample API", | ||
| "description": "This is sample api", | ||
| "version": "v1" | ||
| }, | ||
| "paths": { | ||
| "/example_api_101": { | ||
| "post": { | ||
| "tags": [ | ||
| "Example" | ||
| ], | ||
| "summary": "Process Auth", | ||
| "parameters": [ | ||
| { | ||
| "name": "Id", | ||
| "in": "query", | ||
| "schema": { | ||
| "type": "integer", | ||
| "format": "int32" | ||
| } | ||
| } | ||
| ], | ||
| "requestBody": { | ||
| "content": { | ||
| "multipart/form-data": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { | ||
| "paRes": { | ||
| "type": "object", | ||
| "additionalProperties": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Form sent for Authentication" | ||
| } | ||
| } | ||
| }, | ||
| "encoding": { | ||
| "paRes": { | ||
| "style": "form" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "responses": { | ||
| "200": { | ||
| "description": "OK", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/ResponseOfString" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "components": { | ||
| "schemas": { | ||
| "ResponseOfString": { | ||
| "type": "object", | ||
| "properties": { | ||
| "message": { | ||
| "type": "string", | ||
| "nullable": true | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
| } | ||
| } | ||
| } |
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.
We should revert these - probably because regeneration was done on windows?
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.
Hello, thank you for the feedback.
I also think it was because of Windows. I'll try using a Linux machine instead to effectively undo my own changes
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.
you can use WSL instead
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.
Thank you. Done