From 30c4cf4b9ac93a1cf3a62f163dfc428fcf6e5a48 Mon Sep 17 00:00:00 2001 From: iscai-msft <43154838+iscai-msft@users.noreply.github.com> Date: Sun, 24 Mar 2024 20:43:47 -0400 Subject: [PATCH] [tcgc] add UsageFlags.MultipartFormData and deprecate `.isFormDataType` (#502) --- .../formdata_usage-2024-2-22-14-50-1.md | 7 ++++++ .../src/decorators.ts | 4 +-- .../src/interfaces.ts | 6 +++++ .../src/internal-utils.ts | 24 +++++++++++++++++- .../src/types.ts | 25 +++++++------------ .../test/types.test.ts | 10 ++++++++ 6 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 .chronus/changes/formdata_usage-2024-2-22-14-50-1.md diff --git a/.chronus/changes/formdata_usage-2024-2-22-14-50-1.md b/.chronus/changes/formdata_usage-2024-2-22-14-50-1.md new file mode 100644 index 0000000000..774bdca419 --- /dev/null +++ b/.chronus/changes/formdata_usage-2024-2-22-14-50-1.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +add UsageFlags.MultipartFormData to represent whether a model is used as form data \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/decorators.ts b/packages/typespec-client-generator-core/src/decorators.ts index 01906360ca..89635544d9 100644 --- a/packages/typespec-client-generator-core/src/decorators.ts +++ b/packages/typespec-client-generator-core/src/decorators.ts @@ -524,12 +524,12 @@ export function $convenientAPI( export function shouldGenerateProtocol(context: TCGCContext, entity: Operation): boolean { const value = getScopedDecoratorData(context, protocolAPIKey, entity); - return value ?? !!context.generateProtocolMethods; + return value ?? Boolean(context.generateProtocolMethods); } export function shouldGenerateConvenient(context: TCGCContext, entity: Operation): boolean { const value = getScopedDecoratorData(context, convenientAPIKey, entity); - return value ?? !!context.generateConvenienceMethods; + return value ?? Boolean(context.generateConvenienceMethods); } const excludeKey = createStateSymbol("exclude"); diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index 145a6c914e..8b3eccaf2e 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -271,6 +271,9 @@ export interface SdkModelType extends SdkTypeBase { kind: "model"; properties: SdkModelPropertyType[]; name: string; + /** + * @deprecated This property is deprecated. Check the bitwise and value of UsageFlags.MultipartFormData nad the `.usage` property on this model + */ isFormDataType: boolean; /** * @deprecated This property is deprecated. You should not need to check whether a model is an error model. @@ -540,5 +543,8 @@ export enum UsageFlags { Input = 1 << 1, Output = 1 << 2, ApiVersionEnum = 1 << 3, + // Input will also be set when JsonMergePatch is set JsonMergePatch = 1 << 4, + // Input will also be set when MultipartFormData is set + MultipartFormData = 1 << 5, } diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts index 03bb40b743..e5d92c84f7 100644 --- a/packages/typespec-client-generator-core/src/internal-utils.ts +++ b/packages/typespec-client-generator-core/src/internal-utils.ts @@ -29,6 +29,7 @@ import { } from "./interfaces.js"; import { getCrossLanguageDefinitionId, + getEffectivePayloadType, getHttpOperationWithCache, isApiVersion, } from "./public-utils.js"; @@ -288,7 +289,7 @@ export function getAllResponseBodies(responses: Record) export function isNullable(type: Type | SdkServiceOperation): boolean { if (type.kind === "Union") { if (getNonNullOptions(type).length < type.variants.size) return true; - return !!ignoreDiagnostics(getUnionAsEnum(type))?.nullable; + return Boolean(ignoreDiagnostics(getUnionAsEnum(type))?.nullable); } if (type.kind === "http") { return getAllResponseBodiesAndNonBodyExists(type.responses).nonBodyExists; @@ -304,3 +305,24 @@ export function isNullable(type: Type | SdkServiceOperation): boolean { export function createGeneratedName(type: Namespace | Operation, suffix: string): string { return `${getCrossLanguageDefinitionId(type).split(".").at(-1)}${suffix}`; } + +function isOperationBodyType(context: TCGCContext, type: Type, operation?: Operation): boolean { + if (type.kind !== "Model") return false; + if (!isHttpOperation(context, operation)) return false; + const httpBody = operation + ? getHttpOperationWithCache(context, operation).parameters.body + : undefined; + return Boolean( + httpBody && + httpBody.type.kind === "Model" && + getEffectivePayloadType(context, httpBody.type) === getEffectivePayloadType(context, type) + ); +} + +export function isMultipartFormData( + context: TCGCContext, + type: Type, + operation?: Operation +): boolean { + return isMultipartOperation(context, operation) && isOperationBodyType(context, type, operation); +} diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index e79b09a95c..a8f6709139 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -92,7 +92,7 @@ import { getSdkTypeBaseHelper, intOrFloat, isAzureCoreModel, - isHttpOperation, + isMultipartFormData, isMultipartOperation, isNullable, updateWithApiVersionInformation, @@ -498,18 +498,6 @@ function addDiscriminatorToModelType( return diagnostics.wrap(undefined); } -function isOperationBodyType(context: TCGCContext, type: Model, operation?: Operation): boolean { - if (!isHttpOperation(context, operation)) return false; - const httpBody = operation - ? getHttpOperationWithCache(context, operation).parameters.body - : undefined; - return ( - !!httpBody && - httpBody.type.kind === "Model" && - getEffectivePayloadType(context, httpBody.type) === type - ); -} - export function getSdkModel( context: TCGCContext, type: Model, @@ -543,8 +531,7 @@ export function getSdkModelWithDiagnostics( usage: UsageFlags.None, // dummy value since we need to update models map before we can set this crossLanguageDefinitionId: getCrossLanguageDefinitionId(type), apiVersions: getAvailableApiVersions(context, type), - isFormDataType: - isMultipartOperation(context, operation) && isOperationBodyType(context, type, operation), + isFormDataType: isMultipartFormData(context, type, operation), isError: isErrorModel(context.program, type), }; updateModelsMap(context, type, sdkType, operation); @@ -1364,6 +1351,11 @@ function updateTypesFromOperation( }); } } + if (isMultipartFormData(context, httpBody.type, operation)) { + bodies.forEach((body) => { + updateUsageOfModel(context, UsageFlags.MultipartFormData, body); + }); + } } for (const response of httpOperation.responses) { for (const innerResponse of response.responses) { @@ -1468,9 +1460,10 @@ function verifyNoConflictingMultipartModelUsage( const diagnostics = createDiagnosticCollector(); for (const [operation, modelMap] of context.operationModelsMap!) { for (const [type, sdkType] of modelMap.entries()) { + const isMultipartFormData = (sdkType.usage & UsageFlags.MultipartFormData) > 0; if ( sdkType.kind === "model" && - sdkType.isFormDataType !== isMultipartOperation(context, operation) + isMultipartFormData !== isMultipartOperation(context, operation) ) { // This means we have a model that is used both for formdata input and for regular body input diagnostics.add( diff --git a/packages/typespec-client-generator-core/test/types.test.ts b/packages/typespec-client-generator-core/test/types.test.ts index 962b134d55..c09dd58016 100644 --- a/packages/typespec-client-generator-core/test/types.test.ts +++ b/packages/typespec-client-generator-core/test/types.test.ts @@ -2791,7 +2791,9 @@ describe("typespec-client-generator-core: types", () => { strictEqual(models.length, 1); const model = models[0]; strictEqual(model.kind, "model"); + // eslint-disable-next-line deprecation/deprecation strictEqual(model.isFormDataType, true); + ok((model.usage & UsageFlags.MultipartFormData) > 0); strictEqual(model.name, "MultiPartRequest"); strictEqual(model.properties.length, 2); const id = model.properties.find((x) => x.name === "id"); @@ -2841,7 +2843,9 @@ describe("typespec-client-generator-core: types", () => { const modelA = models.find((x) => x.name === "A"); ok(modelA); strictEqual(modelA.kind, "model"); + // eslint-disable-next-line deprecation/deprecation strictEqual(modelA.isFormDataType, true); + ok((modelA.usage & UsageFlags.MultipartFormData) > 0); strictEqual(modelA.properties.length, 1); const modelAProp = modelA.properties[0]; strictEqual(modelAProp.kind, "property"); @@ -2850,7 +2854,9 @@ describe("typespec-client-generator-core: types", () => { const modelB = models.find((x) => x.name === "B"); ok(modelB); strictEqual(modelB.kind, "model"); + // eslint-disable-next-line deprecation/deprecation strictEqual(modelB.isFormDataType, false); + ok((modelB.usage & UsageFlags.MultipartFormData) === 0); strictEqual(modelB.properties.length, 1); strictEqual(modelB.properties[0].type.kind, "bytes"); }); @@ -2935,12 +2941,16 @@ describe("typespec-client-generator-core: types", () => { const pictureWrapper = models.find((x) => x.name === "PictureWrapper"); ok(pictureWrapper); + // eslint-disable-next-line deprecation/deprecation strictEqual(pictureWrapper.isFormDataType, true); + ok((pictureWrapper.usage & UsageFlags.MultipartFormData) > 0); const errorResponse = models.find((x) => x.name === "ErrorResponse"); ok(errorResponse); strictEqual(errorResponse.kind, "model"); + // eslint-disable-next-line deprecation/deprecation strictEqual(errorResponse.isFormDataType, false); + ok((errorResponse.usage & UsageFlags.MultipartFormData) === 0); }); }); describe("SdkTupleType", () => {