Skip to content

Commit

Permalink
Autorest respect clientName in more places (Azure#1455)
Browse files Browse the repository at this point in the history
fix [Azure#442](Azure#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
  • Loading branch information
timotheeguerin authored and markcowl committed Sep 7, 2024
1 parent c7b5506 commit f9dfffc
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 30 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/autorest-clientname-2024-7-30-11-5-20.md
Original file line number Diff line number Diff line change
@@ -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
34 changes: 27 additions & 7 deletions packages/typespec-autorest/src/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});

Expand Down
6 changes: 6 additions & 0 deletions packages/typespec-autorest/test/models.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
57 changes: 35 additions & 22 deletions packages/typespec-autorest/test/parameters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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: {
Expand Down Expand Up @@ -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");
});
});
});
18 changes: 17 additions & 1 deletion packages/typespec-autorest/test/union-schema.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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"}`
Expand Down

0 comments on commit f9dfffc

Please sign in to comment.