From f9dfffcafc8b1f3a31f56545841435e5181bb849 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 30 Aug 2024 09:39:53 -0700 Subject: [PATCH] Autorest respect clientName in more places (#1455) fix [#442](https://github.com/Azure/typespec-azure/issues/442) `@clientName` is respected for: - definition names - enum values names - parameters names(for body it replace the `name` for other it adds `x-ms-client-name`) Doing this now as this will help getting rid of some use of `@extension` This makes this doc accurate now https://azure.github.io/typespec-azure/docs/next/migrate-swagger/faq/breakingchange#createorupdate-put-apis as well --- .../autorest-clientname-2024-7-30-11-5-20.md | 7 +++ packages/typespec-autorest/src/openapi.ts | 34 ++++++++--- .../typespec-autorest/test/models.test.ts | 6 ++ .../typespec-autorest/test/parameters.test.ts | 57 ++++++++++++------- .../test/union-schema.test.ts | 18 +++++- 5 files changed, 92 insertions(+), 30 deletions(-) create mode 100644 .chronus/changes/autorest-clientname-2024-7-30-11-5-20.md diff --git a/.chronus/changes/autorest-clientname-2024-7-30-11-5-20.md b/.chronus/changes/autorest-clientname-2024-7-30-11-5-20.md new file mode 100644 index 0000000000..bf77e013b0 --- /dev/null +++ b/.chronus/changes/autorest-clientname-2024-7-30-11-5-20.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-autorest" +--- + +Respect `@clientName` for definition names(model, enums, union, etc.), enum and union member and for parameters \ No newline at end of file diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index de7d391472..9238ea10f2 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -16,7 +16,10 @@ import { isAzureResource, isConditionallyFlattened, } from "@azure-tools/typespec-azure-resource-manager"; -import { shouldFlattenProperty } from "@azure-tools/typespec-client-generator-core"; +import { + getClientNameOverride, + shouldFlattenProperty, +} from "@azure-tools/typespec-client-generator-core"; import { ArrayModelType, BooleanLiteral, @@ -877,6 +880,7 @@ export async function getOpenAPIForService( ); delete header.in; delete header.name; + delete header["x-ms-client-name"]; delete header.required; return header; } @@ -1255,8 +1259,10 @@ export async function getOpenAPIForService( required: !param.optional, description: getDoc(program, param), }; - if (param.name !== base.name) { - base["x-ms-client-name"] = param.name; + + const clientName = getClientName(context, param); + if (name !== clientName) { + base["x-ms-client-name"] = clientName; } attachExtensions(param, base); @@ -1269,11 +1275,18 @@ export async function getOpenAPIForService( name?: string, bodySchema?: any ): OpenAPI2BodyParameter { - return { + const result: OpenAPI2BodyParameter = { in: "body", ...getOpenAPI2ParameterBase(param, name), schema: bodySchema, }; + + // For body parameter the only value of the name is in the client so no need to keep the original one + if (result["x-ms-client-name"]) { + result.name = result["x-ms-client-name"]; + delete result["x-ms-client-name"]; + } + return result; } function getOpenAPI2FormDataParameter( @@ -1459,7 +1472,11 @@ export async function getOpenAPIForService( // TYPESPEC type. for (const group of processedSchemas.values()) { for (const [visibility, processed] of group) { - let name = getOpenAPITypeName(program, processed.type, typeNameOptions); + let name = getClientNameOverride(context.tcgcSdkContext, processed.type); + if (name === undefined) { + name = getOpenAPITypeName(program, processed.type, typeNameOptions); + } + if (processed.getSchemaNameOverride !== undefined) { name = processed.getSchemaNameOverride(name, visibility); } else if (group.size > 1) { @@ -1676,8 +1693,10 @@ export async function getOpenAPIForService( let foundCustom = false; for (const [name, member] of e.flattenedMembers.entries()) { const description = getDoc(program, member.type); + const memberClientName = getClientNameOverride(context.tcgcSdkContext, member.type); + values.push({ - name: typeof name === "string" ? name : `${member.value}`, + name: memberClientName ?? (typeof name === "string" ? name : `${member.value}`), value: member.value, description, }); @@ -2252,9 +2271,10 @@ export async function getOpenAPIForService( let foundCustom = false; for (const member of type.members.values()) { const description = getDoc(program, member); + const memberClientName = getClientName(context, member); values.push({ name: member.name, - value: member.value ?? member.name, + value: member.value ?? memberClientName, description, }); diff --git a/packages/typespec-autorest/test/models.test.ts b/packages/typespec-autorest/test/models.test.ts index 7d82b7678d..58f8341dc8 100644 --- a/packages/typespec-autorest/test/models.test.ts +++ b/packages/typespec-autorest/test/models.test.ts @@ -22,6 +22,12 @@ describe("typespec-autorest: model definitions", () => { }); }); + it("change definition name with @clientName", async () => { + const res = await openApiFor(`@clientName("ClientFoo") model Foo {};`); + expect(res.definitions).toHaveProperty("ClientFoo"); + expect(res.definitions).not.toHaveProperty("Foo"); + }); + it(`@projectedName("json", <>) updates the property name and set "x-ms-client-name" with the original name - (LEGACY)`, async () => { const res = await oapiForModel( "Foo", diff --git a/packages/typespec-autorest/test/parameters.test.ts b/packages/typespec-autorest/test/parameters.test.ts index e140ff7380..72ab9b104b 100644 --- a/packages/typespec-autorest/test/parameters.test.ts +++ b/packages/typespec-autorest/test/parameters.test.ts @@ -24,6 +24,16 @@ describe("path parameters", () => { expect(res.paths).toHaveProperty("/{my-custom-path}"); }); + it("set x-ms-client-name with @clientName", async () => { + const param = await getPathParam( + `op test(@clientName("myParamClient") @path myParam: string): void;` + ); + expect(param).toMatchObject({ + name: "myParam", + "x-ms-client-name": "myParamClient", + }); + }); + describe("setting reserved expansion attribute applies the x-ms-skip-url-encoding property", () => { it("with option", async () => { const param = await getPathParam( @@ -71,6 +81,16 @@ describe("query parameters", () => { }); }); + it("set x-ms-client-name with @clientName", async () => { + const param = await getQueryParam( + `op test(@clientName("myParamClient") @query myParam: string): void;` + ); + expect(param).toMatchObject({ + name: "myParam", + "x-ms-client-name": "myParamClient", + }); + }); + describe("setting parameter explode modifier set collectionFormat to multi", () => { it("with option", async () => { const param = await getQueryParam( @@ -267,6 +287,16 @@ describe("header parameters", () => { strictEqual(res.paths["/"].get.parameters[0].in, "query"); strictEqual(res.paths["/"].get.parameters[0].name, "top"); }); + + it("set x-ms-client-name with @clientName", async () => { + const res = await openApiFor( + `op test(@clientName("myParamClient") @header myParam: string): void;` + ); + expect(res.paths["/"].get.parameters[0]).toMatchObject({ + name: "my-param", + "x-ms-client-name": "myParamClient", + }); + }); }); describe("body parameters", () => { @@ -275,6 +305,11 @@ describe("body parameters", () => { deepStrictEqual(res.paths["/"].post.parameters, []); }); + it("set name with @clientName", async () => { + const res = await openApiFor(`op test(@body @clientName("bar") foo: string): void;`); + expect(res.paths["/"].post.parameters[0]).toMatchObject({ in: "body", name: "bar" }); + }); + it("using @body ignore any metadata property underneath", async () => { const res = await openApiFor(`@get op read( @body body: { @@ -445,27 +480,5 @@ describe("misc", () => { }); }); }); - - it("@query/@header/@path names & @projectedName on body parameter are honored (LEGACY)", async () => { - const res = await openApiFor( - ` - @route("/{x-ms-arg-3}") - op test( - @query("x-ms-arg-1") @doc("my-doc") arg1: string, - @header("x-ms-arg-2") @doc("my-doc") arg2: string, - @path("x-ms-arg-3") @doc("my-doc") arg3: string): void; - - @put - op test2( - #suppress "deprecated" "for testing" - @projectedName("json", "x-body") @body @doc("my-doc") arg: string): void; - - ` - ); - strictEqual(res.paths["/{x-ms-arg-3}"].get.parameters[0].name, "x-ms-arg-1"); - strictEqual(res.paths["/{x-ms-arg-3}"].get.parameters[1].name, "x-ms-arg-2"); - strictEqual(res.paths["/{x-ms-arg-3}"].get.parameters[2].name, "x-ms-arg-3"); - strictEqual(res.paths["/"].put.parameters[0].name, "x-body"); - }); }); }); diff --git a/packages/typespec-autorest/test/union-schema.test.ts b/packages/typespec-autorest/test/union-schema.test.ts index abe4b5a2e8..e04908002f 100644 --- a/packages/typespec-autorest/test/union-schema.test.ts +++ b/packages/typespec-autorest/test/union-schema.test.ts @@ -1,6 +1,6 @@ import { expectDiagnostics } from "@typespec/compiler/testing"; import { deepStrictEqual, strictEqual } from "assert"; -import { describe, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { diagnoseOpenApiFor, openApiFor } from "./test-host.js"; describe("typespec-autorest: union schema", () => { @@ -40,6 +40,12 @@ describe("typespec-autorest: union schema", () => { }); describe("unions as enum", () => { + it("change definition name with @clientName", async () => { + const res = await openApiFor(`@clientName("ClientFoo") union Foo {"a"};`); + expect(res.definitions).toHaveProperty("ClientFoo"); + expect(res.definitions).not.toHaveProperty("Foo"); + }); + it("emit enum for simple union of string literals", async () => { const res = await openApiFor(`union Test {"one" , "two"}`); deepStrictEqual(res.definitions.Test, { @@ -104,6 +110,16 @@ describe("typespec-autorest: union schema", () => { }); }); + it("change x-ms-enum.values names with @clientName", async () => { + const res = await openApiFor( + `union Test {@clientName("OneClient") One: "one" , @clientName("TwoClient") Two: "two"};` + ); + expect(res.definitions.Test["x-ms-enum"].values).toEqual([ + { value: "one", name: "OneClient" }, + { value: "two", name: "TwoClient" }, + ]); + }); + it("include the description the x-ms-enum values", async () => { const res = await openApiFor( `union Test {@doc("Doc for one") "one" , @doc("Doc for two") Two: "two"}`