Skip to content
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

fix-binary-in-multipart-form-data #2056

Merged
merged 11 commits into from
Oct 24, 2023

Conversation

qiaozha
Copy link
Member

@qiaozha qiaozha commented Oct 13, 2023

fixes #2045
Currently, we will try to treat binary bytes the same as other encoding to use stringToUint8Array and unit8ArrayToString util to convert it, which is incorrect,

For binary in RLC layer,
if it's in input model, we should generate it as string | Uint8Array | ReadableStream<Uint8Array> | NodeJS.ReadableStream,
if it's in output model, we will generate it as Uint8Array

For binary in Moular layer, current implemtation is treat it as Uint8Array regardless input or output.

Another change in this pr is that, previously, we will speculate if a bytes by default means binary data in the context of multipart/form-data request. As this is not yet decided by microsoft/typespec#2370, we will not do such assumption.

One more thing here, in the past swagger generation, because there's no way to express all those parts together in the multipart/form-data request, we have to manual build the body parameter, but in typespec, we naturally support to use model to group all parts for multipart/form-data request, we don't need this manual build body parameter step now. that's why I delete the transformMultiFormBody function in parameters transform.
If that model is defined anonymously, we will just generate a anonymously model.
If that model has a name, we will just give it a name and put it into the models.ts

@qiaozha qiaozha marked this pull request as ready for review October 18, 2023 06:16
@qiaozha qiaozha requested a review from timovv October 18, 2023 07:47
@qiaozha qiaozha merged commit c3d9f9b into Azure:main Oct 24, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support @encode("binary")
4 participants