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

[TCGC] a helper function to show whether a type is wrapped by HttpPart #1488

Open
msyyc opened this issue Sep 5, 2024 · 4 comments
Open
Assignees
Labels
design:proposed Proposal has been added and ready for discussion

Comments

@msyyc
Copy link
Member

msyyc commented Sep 5, 2024

For the property temperature, TCGC emits its type to a model. The model contains properties for body and contentType. Emitters may only care about type of body. Then how to judge whether we need to parse the property type specially?

op float(
        @header contentType: "multipart/form-data",
        @multipartBody body: {
          temperature: HttpPart<{
            @body body: float64;
            @header contentType: "text/plain";
          }>;
        },
      ): NoContentResponse;

One solution is that judge its kind like here. However, it is not that robust. Since common body may could also define property with prop: {@body body: string} like here.

So maybe TCGC need provide a helper funcion (e.g. isWrappedByHttpPart to help emitters judge whether need special logic for the property type.

@msyyc msyyc added design:proposed Proposal has been added and ready for discussion and removed needs-area labels Sep 5, 2024
@msyyc msyyc self-assigned this Sep 5, 2024
@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Sep 5, 2024

This is somewhat like the discussion of

addresses: HttpPart<Address[]>

that currently returns SdkArray<SdkModel{Address}>, but better be e.g. SdkMultiHttpPart<SdkModel{Address}>, to be explicit about the multiple entries of same field name in multipart request (that it behaves differently from an SdkArray).

This may better be SdkHttpPart<SdkBuiltIn{Float64}> instead of SdkModel{HttpPart}.

Alternatively, if all information of this HttpPart is already captured in the MultipartOptions in the property, and no SDK want to actually generate the unnamed HttpPart model, we can also have TCGC strip this HttpPart for emitter.

@msyyc
Copy link
Member Author

msyyc commented Sep 6, 2024

This may better be SdkHttpPart<SdkBuiltIn{Float64}> instead of SdkModel{HttpPart}.
-> TCGC could do it. But in discussion meeting before, some languages don't hope TCGC do too much for this kind of scenario and they want some original info, so as trade-off, TCGC only peel off HttpPart and emit the content wrapped by it directly. MultipartOptions

Alternatively, if all information of this HttpPart is already captured in the MultipartOptions in the property, and no SDK want to actually generate the unnamed HttpPart model, we can also have TCGC strip this HttpPart for emitter.
-> Yes, actually MultipartOptions already captures the contentType for emitters. So if all languages agree, TCGC could do the additional parse logic to reduce duplicated work for emitters.
image

@msyyc
Copy link
Member Author

msyyc commented Sep 9, 2024

In one word, there are 2 solutions here:

  • TCGC provide helper function for emitters which emitters could use together with additional logic for this scenarios. The cons are all emitters may need duplicated logic
  • Since multipartOptions already extract enough info for emitters so emitters don't need original typespec definition. If all languages agree, TCGC could help extract the @body of HttpPart then all language emitters don't need similar logic.

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Sep 23, 2024

Or add a usage=HttpPart to the wrapping model.

There is already some other usage on model #1518

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design:proposed Proposal has been added and ready for discussion
Projects
None yet
Development

No branches or pull requests

2 participants