From 3becc74aaa57b2d33ccd913c617ca04910e1e7b5 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Fri, 20 Sep 2024 10:46:10 +0800 Subject: [PATCH] [TCGC] Fix judge logic how to distinguish model is referred both in common content-type and multipart content-type (#1511) fix https://github.com/Azure/typespec-azure/issues/1510 --------- Co-authored-by: tadelesh --- ...ultipart-diagnostics-2024-8-13-13-31-25.md | 7 ++ .../src/types.ts | 41 ++++++------ .../test/types/multipart-types.test.ts | 65 +++++++++++++++++++ 3 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 .chronus/changes/fix-multipart-diagnostics-2024-8-13-13-31-25.md diff --git a/.chronus/changes/fix-multipart-diagnostics-2024-8-13-13-31-25.md b/.chronus/changes/fix-multipart-diagnostics-2024-8-13-13-31-25.md new file mode 100644 index 0000000000..555966ea96 --- /dev/null +++ b/.chronus/changes/fix-multipart-diagnostics-2024-8-13-13-31-25.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Fix logic to check conflicting usage for model of multipart body and regular body \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 5576f735b0..61b3cce7e6 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1602,26 +1602,6 @@ function updateTypesFromOperation( } const multipartOperation = isMultipartOperation(context, operation); - // 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( - createDiagnostic({ - code: "conflicting-multipart-model-usage", - target: httpBody.type, - format: { - modelName: sdkType.name, - }, - }), - ); - } - if (generateConvenient) { if (spread) { updateUsageOrAccessOfModel(context, UsageFlags.Spread, sdkType, { propagation: false }); @@ -1649,7 +1629,28 @@ function updateTypesFromOperation( } const access = getAccessOverride(context, operation) ?? "public"; diagnostics.pipe(updateUsageOrAccessOfModel(context, access, sdkType)); + + // after completion of usage calculation for httpBody, check whether it has + // conflicting usage between multipart and regular body + if (sdkType.kind === "model") { + const isUsedInMultipart = (sdkType.usage & UsageFlags.MultipartFormData) > 0; + const isUsedInOthers = + ((sdkType.usage & UsageFlags.Json) | (sdkType.usage & UsageFlags.Xml)) > 0; + if ((!multipartOperation && isUsedInMultipart) || (multipartOperation && isUsedInOthers)) { + // This means we have a model that is used both for formdata input and for regular body input + diagnostics.add( + createDiagnostic({ + code: "conflicting-multipart-model-usage", + target: httpBody.type, + format: { + modelName: sdkType.name, + }, + }), + ); + } + } } + for (const response of httpOperation.responses) { for (const innerResponse of response.responses) { if (innerResponse.body?.type && !isNeverOrVoidType(innerResponse.body.type)) { diff --git a/packages/typespec-client-generator-core/test/types/multipart-types.test.ts b/packages/typespec-client-generator-core/test/types/multipart-types.test.ts index 2a2635e4f2..25663f8e77 100644 --- a/packages/typespec-client-generator-core/test/types/multipart-types.test.ts +++ b/packages/typespec-client-generator-core/test/types/multipart-types.test.ts @@ -55,6 +55,7 @@ describe("typespec-client-generator-core: multipart types", () => { ok(profileImage.multipartOptions); strictEqual(profileImage.multipartOptions.isFilePart, true); }); + it("multipart conflicting model usage", async function () { await runner.compile( ` @@ -72,6 +73,70 @@ describe("typespec-client-generator-core: multipart types", () => { code: "@azure-tools/typespec-client-generator-core/conflicting-multipart-model-usage", }); }); + + it("multipart conflicting model usage for only multipart operations", async function () { + await runner.compile( + ` + @service({title: "Test Service"}) namespace TestService; + model Address {city: string;} + model MultiPartRequest { + address: Address; + id: string; + profileImage: bytes; + } + + @post + @route("/basic1") + op basic1(@header contentType: "multipart/form-data", @body body: MultiPartRequest): NoContentResponse; + @post + @route("/basic2") + op basic2(@header contentType: "multipart/form-data", @body body: MultiPartRequest): NoContentResponse; + `, + ); + deepEqual(runner.context.diagnostics.length, 0); + const address = runner.context.sdkPackage.models.find((x) => x.name === "Address"); + ok(address); + deepEqual(address.usage, UsageFlags.Input); + const multiPartRequest = runner.context.sdkPackage.models.find( + (x) => x.name === "MultiPartRequest", + ); + ok(multiPartRequest); + deepEqual(multiPartRequest.usage, UsageFlags.MultipartFormData | UsageFlags.Input); + }); + + it("multipart conflicting model usage for mixed operations", async function () { + await runner.compile( + ` + @service({title: "Test Service"}) namespace TestService; + model Address {city: string;} + model RegularRequest { + address: Address; + } + model MultiPartRequest { + address: Address; + id: string; + profileImage: bytes; + } + + @post + @route("/basic1") + op basic1(@body body: RegularRequest): NoContentResponse; + @post + @route("/basic2") + op basic2(@header contentType: "multipart/form-data", @body body: MultiPartRequest): NoContentResponse; + `, + ); + deepEqual(runner.context.diagnostics.length, 0); + const address = runner.context.sdkPackage.models.find((x) => x.name === "Address"); + ok(address); + deepEqual(address.usage, UsageFlags.Input | UsageFlags.Json); + const multiPartRequest = runner.context.sdkPackage.models.find( + (x) => x.name === "MultiPartRequest", + ); + ok(multiPartRequest); + deepEqual(multiPartRequest.usage, UsageFlags.MultipartFormData | UsageFlags.Input); + }); + it("multipart resolving conflicting model usage with spread", async function () { await runner.compileWithBuiltInService( `