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

Remove 'x-ms-client-flatten' in typespec-azure-resource-manager templates and make it conditional in autorest #1105

Merged
merged 6 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
allenjzhang marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions docs/emitters/typespec-autorest/reference/emitter.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 1 addition & 0 deletions packages/samples/specs/resource-manager/tspconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
6 changes: 6 additions & 0 deletions packages/typespec-autorest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/typespec-autorest/src/emit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
};
}

Expand Down
13 changes: 13 additions & 0 deletions packages/typespec-autorest/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AutorestEmitterOptions> = {
Expand Down Expand Up @@ -193,6 +199,13 @@ const EmitterOptionsSchema: JSONSchemaType<AutorestEmitterOptions> = {
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: [],
};
Expand Down
12 changes: 11 additions & 1 deletion packages/typespec-autorest/src/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import {
getArmCommonTypeOpenAPIRef,
isArmCommonType,
isConditionallyFlattened,
} from "@azure-tools/typespec-azure-resource-manager";
import { shouldFlattenProperty } from "@azure-tools/typespec-client-generator-core";
import {
Expand Down Expand Up @@ -195,6 +196,11 @@ export interface AutorestDocumentEmitterOptions {
* @default "final-state-only"
*/
readonly emitLroOptions?: "none" | "final-state-only" | "all";

/**
* readOnly property ARM resource flattening
*/
readonly armResourceFlattening?: boolean;
}

/**
Expand Down Expand Up @@ -1848,7 +1854,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) {
Expand Down
67 changes: 67 additions & 0 deletions packages/typespec-autorest/test/arm/arm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<EmployeeProperties> {
...ResourceNameParameter<Employee>;
}
model EmployeeProperties {
age?: int32;
city?: string;
@visibility("read")
provisioningState?: ResourceProvisioningState;
}
@parentResource(Employee)
model Dependent is ProxyResource<DependentProperties> {
...ResourceNameParameter<Dependent>;
}
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<EmployeeProperties> {
...ResourceNameParameter<Employee>;
}
model EmployeeProperties {
age?: int32;
city?: string;
@visibility("read")
provisioningState?: ResourceProvisioningState;
}
@parentResource(Employee)
model Dependent is ProxyResource<DependentProperties> {
...ResourceNameParameter<Dependent>;
}
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
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ model PrivateEndpoint {
/** A private link resource. */
model PrivateLinkResource extends Resource {
/** Resource properties. */
@extension("x-ms-client-flatten", true)
@Azure.ResourceManager.Private.conditionalClientFlatten
properties?: PrivateLinkResourceProperties;
}

Expand All @@ -45,7 +45,7 @@ model PrivateLinkResourceListResult {
/** The private endpoint connection resource */
model PrivateEndpointConnection extends Resource {
/** The private endpoint connection properties */
@extension("x-ms-client-flatten", true)
@Azure.ResourceManager.Private.conditionalClientFlatten
properties?: PrivateEndpointConnectionProperties;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ model ResourceUpdateModel<
Resource,
"Name" | "name" | "properties"
>>> {
@extension("x-ms-client-flatten", true)
@conditionalClientFlatten
properties?: ResourceUpdateModelProperties<Resource, Properties>;
}

Expand Down Expand Up @@ -184,7 +184,7 @@ model ProxyResourceUpdateModel<
Resource extends Foundations.Resource,
Properties extends TypeSpec.Reflection.Model
> {
@extension("x-ms-client-flatten", true)
@conditionalClientFlatten
properties?: ResourceUpdateModelProperties<Resource, Properties>;
}

Expand Down
6 changes: 3 additions & 3 deletions packages/typespec-azure-resource-manager/lib/models.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ model ResourceNameParameter<
@includeInapplicableMetadataInPayload(false)
model TrackedResource<Properties extends {}> extends Foundations.TrackedResource {
@doc("The resource-specific properties for this resource.")
@extension("x-ms-client-flatten", true)
@conditionalClientFlatten
properties?: Properties;
}

Expand All @@ -61,7 +61,7 @@ model TrackedResource<Properties extends {}> extends Foundations.TrackedResource
@includeInapplicableMetadataInPayload(false)
model ProxyResource<Properties extends {}> extends Foundations.ProxyResource {
@doc("The resource-specific properties for this resource.")
@extension("x-ms-client-flatten", true)
@conditionalClientFlatten
properties?: Properties;
}

Expand All @@ -77,7 +77,7 @@ model ProxyResource<Properties extends {}> extends Foundations.ProxyResource {
@includeInapplicableMetadataInPayload(false)
model ExtensionResource<Properties extends {}> extends Foundations.ExtensionResource {
@doc("The resource-specific properties for this resource.")
@extension("x-ms-client-flatten", true)
@conditionalClientFlatten
properties?: Properties;
}
//#region
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,11 @@ extern dec defaultResourceKeySegmentName(
* Note, this is intended for internal use only for now.
*/
extern dec enforceConstraint(target: Operation | Model, sourceType: Model, constraintType: 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);
allenjzhang marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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}
allenjzhang marked this conversation as resolved.
Show resolved Hide resolved
*/
export function $armCommonTypesVersions(context: DecoratorContext, enumType: Enum) {
context.program.stateMap(ArmStateKeys.armCommonTypesVersions).set(enumType, {
type: enumType,
Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-azure-resource-manager/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,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",
});
Original file line number Diff line number Diff line change
Expand Up @@ -350,3 +350,18 @@ function hasProperty(program: Program, model: Model): boolean {
if (model.baseModel) return hasProperty(program, model.baseModel);
return 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allenjzhang i do not see where to set @flattenProperty decorator, but only see the set of openapi2 x-ms-client-flatten. am i miss sth?

* 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 {
allenjzhang marked this conversation as resolved.
Show resolved Hide resolved
const flatten = program.stateMap(ArmStateKeys.armConditionalClientFlatten).get(entity);
return flatten ?? false;
}
3 changes: 3 additions & 0 deletions packages/typespec-azure-resource-manager/src/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ export const ArmStateKeys = {
armBuiltInResource: azureResourceManagerCreateStateSymbol("armExternalResource"),

// private.decorator.ts
armConditionalClientFlatten: azureResourceManagerCreateStateSymbol("armConditionalClientFlatten"),

// commontypes.private.decorators.ts
armCommonDefinitions: azureResourceManagerCreateStateSymbol("armCommonDefinitions"),
armCommonParameters: azureResourceManagerCreateStateSymbol("armCommonParameters"),
armCommonTypesVersions: azureResourceManagerCreateStateSymbol("armCommonTypesVersions"),
Expand Down
Loading