From 11c971b9eb0396974e488bfa13ab53ae88f8b3b5 Mon Sep 17 00:00:00 2001 From: Allen Zhang Date: Wed, 10 Jul 2024 17:05:52 -0700 Subject: [PATCH] Remove 'x-ms-client-flatten' in typespec-azure-resource-manager templates and make it conditional in autorest (#1105) Closes #709 --- ...g_RemoveClientFlatten-2024-6-1-16-58-12.md | 8 +++ .../typespec-autorest/reference/emitter.md | 6 ++ .../common-types/src/private-links.tsp | 3 + .../specs/resource-manager/tspconfig.yaml | 1 + packages/typespec-autorest/README.md | 6 ++ packages/typespec-autorest/src/emit.ts | 1 + packages/typespec-autorest/src/lib.ts | 13 ++++ packages/typespec-autorest/src/openapi.ts | 12 +++- .../typespec-autorest/test/arm/arm.test.ts | 67 +++++++++++++++++++ .../lib/common-types/private-links-ref.tsp | 3 + .../lib/common-types/private-links.tsp | 2 - .../lib/foundations/arm.foundations.tsp | 4 +- .../lib/models.tsp | 6 +- .../lib/private.decorators.tsp | 8 +++ .../src/commontypes.private.decorators.ts | 6 ++ .../src/index.ts | 2 + .../src/private.decorators.ts | 15 +++++ .../src/state.ts | 3 + 18 files changed, 158 insertions(+), 8 deletions(-) create mode 100644 .chronus/changes/azhang_RemoveClientFlatten-2024-6-1-16-58-12.md diff --git a/.chronus/changes/azhang_RemoveClientFlatten-2024-6-1-16-58-12.md b/.chronus/changes/azhang_RemoveClientFlatten-2024-6-1-16-58-12.md new file mode 100644 index 0000000000..fa923b0e30 --- /dev/null +++ b/.chronus/changes/azhang_RemoveClientFlatten-2024-6-1-16-58-12.md @@ -0,0 +1,8 @@ +--- +changeKind: breaking +packages: + - "@azure-tools/typespec-autorest" + - "@azure-tools/typespec-azure-resource-manager" +--- + +`x-ms-client-flatten` extension on some of resource properties property is now configurable to be emitted by autorest emitter. Default is false which will skip emission of that extension. diff --git a/docs/emitters/typespec-autorest/reference/emitter.md b/docs/emitters/typespec-autorest/reference/emitter.md index 4bc24450c4..638a5f4ae9 100644 --- a/docs/emitters/typespec-autorest/reference/emitter.md +++ b/docs/emitters/typespec-autorest/reference/emitter.md @@ -125,3 +125,9 @@ Create read-only property schema for lro status **Type:** `"none" | "final-state-only" | "all"` Determine whether and how to emit x-ms-long-running-operation-options for lro resolution + +### `arm-resource-flattening` + +**Type:** `boolean` + +Back-compat flag. If true, continue to emit `x-ms-client-flatten` in for some of the ARM resource properties. diff --git a/packages/samples/common-types/src/private-links.tsp b/packages/samples/common-types/src/private-links.tsp index a5e3a7ca46..7544b3eb33 100644 --- a/packages/samples/common-types/src/private-links.tsp +++ b/packages/samples/common-types/src/private-links.tsp @@ -9,3 +9,6 @@ namespace Azure.ResourceManager.CommonTypes; @useRef("./types.json#/definitions/Resource") model Resource {} + +@@OpenAPI.extension(PrivateEndpointConnection.properties, "x-ms-client-flatten", true); +@@OpenAPI.extension(PrivateLinkResource.properties, "x-ms-client-flatten", true); diff --git a/packages/samples/specs/resource-manager/tspconfig.yaml b/packages/samples/specs/resource-manager/tspconfig.yaml index a71f4f4ea4..eefd13f217 100644 --- a/packages/samples/specs/resource-manager/tspconfig.yaml +++ b/packages/samples/specs/resource-manager/tspconfig.yaml @@ -4,6 +4,7 @@ options: "@azure-tools/typespec-autorest": use-read-only-status-schema: true arm-types-dir: "{project-root}/common-types" + arm-resource-flattening: true linter: extends: - "@azure-tools/typespec-azure-rulesets/resource-manager" diff --git a/packages/typespec-autorest/README.md b/packages/typespec-autorest/README.md index d0e5b6e3d1..4034866642 100644 --- a/packages/typespec-autorest/README.md +++ b/packages/typespec-autorest/README.md @@ -130,6 +130,12 @@ Create read-only property schema for lro status Determine whether and how to emit x-ms-long-running-operation-options for lro resolution +#### `arm-resource-flattening` + +**Type:** `boolean` + +Back-compat flag. If true, continue to emit `x-ms-client-flatten` in for some of the ARM resource properties. + ## Decorators ### Autorest diff --git a/packages/typespec-autorest/src/emit.ts b/packages/typespec-autorest/src/emit.ts index abeecf1f7a..fec710c699 100644 --- a/packages/typespec-autorest/src/emit.ts +++ b/packages/typespec-autorest/src/emit.ts @@ -91,6 +91,7 @@ export function resolveAutorestOptions( armTypesDir, useReadOnlyStatusSchema: resolvedOptions["use-read-only-status-schema"], emitLroOptions: resolvedOptions["emit-lro-options"], + armResourceFlattening: resolvedOptions["arm-resource-flattening"], }; } diff --git a/packages/typespec-autorest/src/lib.ts b/packages/typespec-autorest/src/lib.ts index 11d002f7fd..2f69084ab9 100644 --- a/packages/typespec-autorest/src/lib.ts +++ b/packages/typespec-autorest/src/lib.ts @@ -90,6 +90,12 @@ export interface AutorestEmitterOptions { * @default "final-state-only" */ "emit-lro-options"?: "none" | "final-state-only" | "all"; + + /** + * Back-compat flag. If true, continue to emit `x-ms-client-flatten` in for some of the + * ARM resource properties. + */ + "arm-resource-flattening"?: boolean; } const EmitterOptionsSchema: JSONSchemaType = { @@ -193,6 +199,13 @@ const EmitterOptionsSchema: JSONSchemaType = { description: "Determine whether and how to emit x-ms-long-running-operation-options for lro resolution", }, + "arm-resource-flattening": { + type: "boolean", + nullable: true, + default: false, + description: + "Back-compat flag. If true, continue to emit `x-ms-client-flatten` in for some of the ARM resource properties.", + }, }, required: [], }; diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 52c1d06557..d5daa7c2d3 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -14,6 +14,7 @@ import { getArmCommonTypeOpenAPIRef, isArmCommonType, isAzureResource, + isConditionallyFlattened, } from "@azure-tools/typespec-azure-resource-manager"; import { shouldFlattenProperty } from "@azure-tools/typespec-client-generator-core"; import { @@ -196,6 +197,11 @@ export interface AutorestDocumentEmitterOptions { * @default "final-state-only" */ readonly emitLroOptions?: "none" | "final-state-only" | "all"; + + /** + * readOnly property ARM resource flattening + */ + readonly armResourceFlattening?: boolean; } /** @@ -1849,7 +1855,11 @@ export async function getOpenAPIForService( propSchema = getSchemaOrRef(prop.type, context); } - return applyIntrinsicDecorators(prop, propSchema); + if (options.armResourceFlattening && isConditionallyFlattened(program, prop)) { + return { ...applyIntrinsicDecorators(prop, propSchema), "x-ms-client-flatten": true }; + } else { + return applyIntrinsicDecorators(prop, propSchema); + } } function attachExtensions(type: Type, emitObject: any) { diff --git a/packages/typespec-autorest/test/arm/arm.test.ts b/packages/typespec-autorest/test/arm/arm.test.ts index 35d3b5cfb5..7118a0beac 100644 --- a/packages/typespec-autorest/test/arm/arm.test.ts +++ b/packages/typespec-autorest/test/arm/arm.test.ts @@ -236,3 +236,70 @@ it("can use ResourceNameParameter for default name parameter definition", async strictEqual(openapi.paths[privateEndpointGet].get.parameters[1].pattern, "^[a-zA-Z0-9-]{3,24}$"); ok(openapi.paths[privateEndpointGet].get.parameters[1]); }); + +it("can emit x-ms-client-flatten with optional configuration", async () => { + const openapi = await openApiFor( + `@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + @armProviderNamespace + @armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5) + namespace Microsoft.Contoso; + + model Employee is TrackedResource { + ...ResourceNameParameter; + } + model EmployeeProperties { + age?: int32; + city?: string; + @visibility("read") + provisioningState?: ResourceProvisioningState; + } + @parentResource(Employee) + model Dependent is ProxyResource { + ...ResourceNameParameter; + } + model DependentProperties { + age?: int32; + } + `, + undefined, + { + "arm-resource-flattening": true, + } + ); + + ok(openapi.definitions.Employee.properties.properties["x-ms-client-flatten"]); + ok(openapi.definitions.Dependent.properties.properties["x-ms-client-flatten"]); +}); + +it("no x-ms-client-flatten emitted with default configuration", async () => { + const openapi = await openApiFor( + `@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + @armProviderNamespace + @armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5) + namespace Microsoft.Contoso; + + model Employee is TrackedResource { + ...ResourceNameParameter; + } + model EmployeeProperties { + age?: int32; + city?: string; + @visibility("read") + provisioningState?: ResourceProvisioningState; + } + @parentResource(Employee) + model Dependent is ProxyResource { + ...ResourceNameParameter; + } + model DependentProperties { + age?: int32; + } + ` + ); + + strictEqual(openapi.definitions.Employee.properties.properties["x-ms-client-flatten"], undefined); + strictEqual( + openapi.definitions.Dependent.properties.properties["x-ms-client-flatten"], + undefined + ); +}); diff --git a/packages/typespec-azure-resource-manager/lib/common-types/private-links-ref.tsp b/packages/typespec-azure-resource-manager/lib/common-types/private-links-ref.tsp index 2fca19e18d..c3b47dff03 100644 --- a/packages/typespec-azure-resource-manager/lib/common-types/private-links-ref.tsp +++ b/packages/typespec-azure-resource-manager/lib/common-types/private-links-ref.tsp @@ -4,6 +4,9 @@ using Azure.ResourceManager.CommonTypes.Private; namespace Azure.ResourceManager.CommonTypes; +@@Azure.ResourceManager.Private.conditionalClientFlatten(PrivateLinkResource.properties); +@@Azure.ResourceManager.Private.conditionalClientFlatten(PrivateEndpointConnection.properties); + /** The private endpoint */ @@armCommonDefinition(PrivateEndpoint, "PrivateEndpoint", diff --git a/packages/typespec-azure-resource-manager/lib/common-types/private-links.tsp b/packages/typespec-azure-resource-manager/lib/common-types/private-links.tsp index 694bb96022..b9deea57d3 100644 --- a/packages/typespec-azure-resource-manager/lib/common-types/private-links.tsp +++ b/packages/typespec-azure-resource-manager/lib/common-types/private-links.tsp @@ -18,7 +18,6 @@ model PrivateEndpoint { /** A private link resource. */ model PrivateLinkResource extends Resource { /** Resource properties. */ - @extension("x-ms-client-flatten", true) properties?: PrivateLinkResourceProperties; } @@ -45,7 +44,6 @@ model PrivateLinkResourceListResult { /** The private endpoint connection resource */ model PrivateEndpointConnection extends Resource { /** The private endpoint connection properties */ - @extension("x-ms-client-flatten", true) properties?: PrivateEndpointConnectionProperties; } diff --git a/packages/typespec-azure-resource-manager/lib/foundations/arm.foundations.tsp b/packages/typespec-azure-resource-manager/lib/foundations/arm.foundations.tsp index 454caec07a..13aa7f8e48 100644 --- a/packages/typespec-azure-resource-manager/lib/foundations/arm.foundations.tsp +++ b/packages/typespec-azure-resource-manager/lib/foundations/arm.foundations.tsp @@ -120,7 +120,7 @@ model ResourceUpdateModel< Resource, "Name" | "name" | "properties" >>> { - @extension("x-ms-client-flatten", true) + @conditionalClientFlatten properties?: ResourceUpdateModelProperties; } @@ -184,7 +184,7 @@ model ProxyResourceUpdateModel< Resource extends Foundations.Resource, Properties extends TypeSpec.Reflection.Model > { - @extension("x-ms-client-flatten", true) + @conditionalClientFlatten properties?: ResourceUpdateModelProperties; } diff --git a/packages/typespec-azure-resource-manager/lib/models.tsp b/packages/typespec-azure-resource-manager/lib/models.tsp index fb0bfca16e..db8633c4b8 100644 --- a/packages/typespec-azure-resource-manager/lib/models.tsp +++ b/packages/typespec-azure-resource-manager/lib/models.tsp @@ -46,7 +46,7 @@ model ResourceNameParameter< @includeInapplicableMetadataInPayload(false) model TrackedResource extends Foundations.TrackedResource { @doc("The resource-specific properties for this resource.") - @extension("x-ms-client-flatten", true) + @conditionalClientFlatten properties?: Properties; } @@ -61,7 +61,7 @@ model TrackedResource extends Foundations.TrackedResource @includeInapplicableMetadataInPayload(false) model ProxyResource extends Foundations.ProxyResource { @doc("The resource-specific properties for this resource.") - @extension("x-ms-client-flatten", true) + @conditionalClientFlatten properties?: Properties; } @@ -77,7 +77,7 @@ model ProxyResource extends Foundations.ProxyResource { @includeInapplicableMetadataInPayload(false) model ExtensionResource extends Foundations.ExtensionResource { @doc("The resource-specific properties for this resource.") - @extension("x-ms-client-flatten", true) + @conditionalClientFlatten properties?: Properties; } //#region diff --git a/packages/typespec-azure-resource-manager/lib/private.decorators.tsp b/packages/typespec-azure-resource-manager/lib/private.decorators.tsp index 036396a14c..3be685b32b 100644 --- a/packages/typespec-azure-resource-manager/lib/private.decorators.tsp +++ b/packages/typespec-azure-resource-manager/lib/private.decorators.tsp @@ -74,3 +74,11 @@ extern dec enforceConstraint(target: Operation | Model, sourceType: Model, const * It is *not* meant to be used directly by a spec author, */ extern dec azureResourceBase(target: Model); + +/** + * Please DO NOT USE in RestAPI specs. + * Internal decorator that deprecated direct usage of `x-ms-client-flatten` OpenAPI extension. + * It will programatically enabled/disable client flattening with @flattenProperty with autorest + * emitter flags to maintain compatibility in swagger. + */ +extern dec conditionalClientFlatten(target: ModelProperty); diff --git a/packages/typespec-azure-resource-manager/src/commontypes.private.decorators.ts b/packages/typespec-azure-resource-manager/src/commontypes.private.decorators.ts index 326a380203..2b4e7e67fd 100644 --- a/packages/typespec-azure-resource-manager/src/commontypes.private.decorators.ts +++ b/packages/typespec-azure-resource-manager/src/commontypes.private.decorators.ts @@ -127,6 +127,12 @@ export function $armCommonDefinition( storeCommonTypeRecord(context, entity, "definitions", definitionName, version, referenceFile); } +/** + * Specify the ARM commont type version reference for a particular spec version or namespace. + * @param DecoratorContext context DecoratorContext object + * @param enumType entity Decorator target type. Must be `Model` + * @returns void + */ export function $armCommonTypesVersions(context: DecoratorContext, enumType: Enum) { context.program.stateMap(ArmStateKeys.armCommonTypesVersions).set(enumType, { type: enumType, diff --git a/packages/typespec-azure-resource-manager/src/index.ts b/packages/typespec-azure-resource-manager/src/index.ts index 7d458b5e47..57d3df6fea 100644 --- a/packages/typespec-azure-resource-manager/src/index.ts +++ b/packages/typespec-azure-resource-manager/src/index.ts @@ -21,6 +21,8 @@ export * from "./resource.js"; export { $lib } from "./lib.js"; export { $linter } from "./linter.js"; +export { isConditionallyFlattened } from "./private.decorators.js"; + export const $flags = definePackageFlags({ decoratorArgMarshalling: "new", }); diff --git a/packages/typespec-azure-resource-manager/src/private.decorators.ts b/packages/typespec-azure-resource-manager/src/private.decorators.ts index c4baaa398f..5ac4b55d11 100644 --- a/packages/typespec-azure-resource-manager/src/private.decorators.ts +++ b/packages/typespec-azure-resource-manager/src/private.decorators.ts @@ -359,3 +359,18 @@ export function isAzureResource(program: Program, resourceType: Model): boolean const isResourceBase = program.stateMap(ArmStateKeys.azureResourceBase).get(resourceType); return isResourceBase ?? false; } + +/** + * Please DO NOT USE in RestAPI specs. + * Internal decorator that deprecated direct usage of `x-ms-client-flatten` OpenAPI extension. + * It will programatically enabled/disable client flattening with @flattenProperty with autorest + * emitter flags to maintain compatibility in swagger. + */ +export function $conditionalClientFlatten(context: DecoratorContext, entity: ModelProperty) { + context.program.stateMap(ArmStateKeys.armConditionalClientFlatten).set(entity, true); +} + +export function isConditionallyFlattened(program: Program, entity: ModelProperty): boolean { + const flatten = program.stateMap(ArmStateKeys.armConditionalClientFlatten).get(entity); + return flatten ?? false; +} diff --git a/packages/typespec-azure-resource-manager/src/state.ts b/packages/typespec-azure-resource-manager/src/state.ts index e8d5d68922..e6ba7365cd 100644 --- a/packages/typespec-azure-resource-manager/src/state.ts +++ b/packages/typespec-azure-resource-manager/src/state.ts @@ -23,6 +23,9 @@ export const ArmStateKeys = { // private.decorator.ts azureResourceBase: azureResourceManagerCreateStateSymbol("azureResourceBase"), + armConditionalClientFlatten: azureResourceManagerCreateStateSymbol("armConditionalClientFlatten"), + + // commontypes.private.decorators.ts armCommonDefinitions: azureResourceManagerCreateStateSymbol("armCommonDefinitions"), armCommonParameters: azureResourceManagerCreateStateSymbol("armCommonParameters"), armCommonTypesVersions: azureResourceManagerCreateStateSymbol("armCommonTypesVersions"),