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

[Bug]: TCGC reports wrong diagnostics "conflicting-multipart-model-usage" for valid multipart cases #1510

Closed
4 tasks done
msyyc opened this issue Sep 11, 2024 · 3 comments · Fixed by #1511
Closed
4 tasks done
Assignees
Labels
bug Something isn't working lib:tcgc Issues for @azure-tools/typespec-client-generator-core library

Comments

@msyyc
Copy link
Member

msyyc commented Sep 11, 2024

Describe the bug

For the valid cases, TCGC reports wrong diagnostics "conflicting-multipart-model-usage"

Reproduction

    @service({title: "Test Service"}) namespace TestService;

    model MultiPartRequest {
      id: string;
      profileImage: bytes;
    }
    
    @post op basic1(@header contentType: "multipart/form-data", @body body: MultiPartRequest): NoContentResponse;
    @put op basic2(@header contentType: "multipart/form-data", @body body: MultiPartRequest): NoContentResponse;

Checklist

  • Follow our Code of Conduct
  • Check that this issue is about the Azure libraries for typespec. For bug in the typespec language or core libraries file it in the TypeSpec repo
  • Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@msyyc
Copy link
Member Author

msyyc commented Sep 11, 2024

The root cause is that TCGC always set a model with UsageFlags.Input:

updateUsageOrAccessOfModel(context, UsageFlags.Input, sdkType);
so it is not proper to distinguish whether a model is used both in common content-type and multipart content-type:
// this part should be put before setting current body's usage because it is based on the previous usage
if (
sdkType.kind === "model" &&
((!multipartOperation && (sdkType.usage & UsageFlags.MultipartFormData) > 0) ||
(multipartOperation &&
(sdkType.usage & UsageFlags.Input) > 0 &&
(sdkType.usage & UsageFlags.Input & UsageFlags.MultipartFormData) === 0))
) {
// This means we have a model that is used both for formdata input and for regular body input
diagnostics.add(
.

There are 2 solutions:

  • make usage Input and MultipartFormData excluded
  • add new usage like NonMultiPart

@weidongxu-microsoft
Copy link
Member

Personal opinion:
Make MultipartFormData exclusive to JSON/XML, as all of these relates to the serialization of the model.

@msyyc
Copy link
Member Author

msyyc commented Sep 12, 2024

Personal opinion: Make MultipartFormData exclusive to JSON/XML, as all of these relates to the serialization of the model.

After discussion, we pretend to use this solution instead of adding new usage kind. Let me make a PR to fix it later.

@tadelesh tadelesh added the lib:tcgc Issues for @azure-tools/typespec-client-generator-core library label Sep 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 20, 2024
…ommon content-type and multipart content-type (#1511)

fix #1510

---------

Co-authored-by: tadelesh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib:tcgc Issues for @azure-tools/typespec-client-generator-core library
Projects
None yet
3 participants