From 052337b3f9bcde152e8860f569d08ee92a4fdf4b Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 26 Jul 2024 14:02:26 -0700 Subject: [PATCH 01/12] Uptake uri templates --- core | 2 +- packages/typespec-azure-core/src/lro-info.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core b/core index eb245109c0..c38a8d64af 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit eb245109c05e98e10fdd75c48040d33c432a6a98 +Subproject commit c38a8d64afdaa135b68143eb818998ae7b2abee6 diff --git a/packages/typespec-azure-core/src/lro-info.ts b/packages/typespec-azure-core/src/lro-info.ts index 1997d23486..ce0868f0b3 100644 --- a/packages/typespec-azure-core/src/lro-info.ts +++ b/packages/typespec-azure-core/src/lro-info.ts @@ -10,13 +10,14 @@ import { compilerAssert, createDiagnosticCollector, getEffectiveModelType, + ignoreDiagnostics, isErrorType, isType, } from "@typespec/compiler"; import { HttpOperationResponse, getHeaderFieldName, - getOperationParameters, + getHttpOperation, getResponsesForOperation, isHeader, isMetadata, @@ -128,7 +129,7 @@ export function getLroOperationInfo( ): [LroOperationInfo | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const targetResponses = diagnostics.pipe(getResponsesForOperation(program, targetOperation)); - const targetParameters = diagnostics.pipe(getOperationParameters(program, targetOperation)); + const targetParameters = ignoreDiagnostics(getHttpOperation(program, targetOperation)).parameters; const targetProperties = new Map(); const parameterMap = new Map(); const unmatchedParameters = new Set(targetParameters.parameters.flatMap((p) => p.name)); @@ -146,7 +147,7 @@ export function getLroOperationInfo( } } const sourceResponses = diagnostics.pipe(getResponsesForOperation(program, sourceOperation)); - const sourceParameters = diagnostics.pipe(getOperationParameters(program, sourceOperation)); + const sourceParameters = ignoreDiagnostics(getHttpOperation(program, sourceOperation)).parameters; const sourceBodyProperties = new Map(); if (sourceParameters.body && sourceParameters.body.type.kind === "Model") { for (const [sourceName, sourceProp] of getAllProperties(sourceParameters.body.type)) { From a0bbbf2cf16b8abf1a76003b9d60c42c1334c60f Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 26 Jul 2024 14:10:48 -0700 Subject: [PATCH 02/12] Create uptake-uri-templates-2024-6-26-21-7-49.md --- .../changes/uptake-uri-templates-2024-6-26-21-7-49.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/uptake-uri-templates-2024-6-26-21-7-49.md diff --git a/.chronus/changes/uptake-uri-templates-2024-6-26-21-7-49.md b/.chronus/changes/uptake-uri-templates-2024-6-26-21-7-49.md new file mode 100644 index 0000000000..693b68556f --- /dev/null +++ b/.chronus/changes/uptake-uri-templates-2024-6-26-21-7-49.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: internal +packages: + - "@azure-tools/typespec-azure-core" +--- + +Call correct HTTP library API From 8f4dfc7270bd41ab19ea2285c329dd9d09b2b44b Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 26 Jul 2024 14:16:18 -0700 Subject: [PATCH 03/12] Update --- core | 2 +- docs/release-notes/release-2023-04-11.md | 4 +--- packages/samples/specs/misc/appconfig/keyvalues.tsp | 12 +++--------- packages/samples/specs/misc/appconfig/labels.tsp | 4 +--- packages/samples/specs/misc/appconfig/revisions.tsp | 4 +--- packages/typespec-azure-core/lib/models.tsp | 12 +++--------- 6 files changed, 10 insertions(+), 28 deletions(-) diff --git a/core b/core index c38a8d64af..661a1bb5da 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit c38a8d64afdaa135b68143eb818998ae7b2abee6 +Subproject commit 661a1bb5dae435013257d8646c102b4934aab238 diff --git a/docs/release-notes/release-2023-04-11.md b/docs/release-notes/release-2023-04-11.md index 3d89633778..c99495549f 100644 --- a/docs/release-notes/release-2023-04-11.md +++ b/docs/release-notes/release-2023-04-11.md @@ -59,9 +59,7 @@ op list(@collectionFormat("multi") @query select: string[]): Pet[]; // Now op list( - @query({ - format: "multi", - }) + @query(#{ explode: true }) select: string[], ): Pet[]; ``` diff --git a/packages/samples/specs/misc/appconfig/keyvalues.tsp b/packages/samples/specs/misc/appconfig/keyvalues.tsp index a6d4595bae..d03c04651c 100644 --- a/packages/samples/specs/misc/appconfig/keyvalues.tsp +++ b/packages/samples/specs/misc/appconfig/keyvalues.tsp @@ -44,9 +44,7 @@ namespace KeyValuesResource { After: plainDate, @doc("Used to select what fields are present in the returned resource(s).") - @query({ - format: "multi", - }) + @query(#{ explode: true }) $Select?: KeyField[], ): { ...Response<200>; @@ -70,9 +68,7 @@ namespace KeyValuesResource { After: plainDate, @doc("Used to select what fields are present in the returned resource(s).") - @query({ - format: "multi", - }) + @query(#{ explode: true }) $Select?: KeyField[], ): { ...Response<200>; @@ -88,9 +84,7 @@ namespace KeyValuesResource { ...KeyWithFilters, @doc("Used to select what fields are present in the returned resource(s).") - @query({ - format: "multi", - }) + @query(#{ explode: true }) $Select?: KeyField[], ): { ...Response<200>; diff --git a/packages/samples/specs/misc/appconfig/labels.tsp b/packages/samples/specs/misc/appconfig/labels.tsp index 60f934c00f..a8546d1386 100644 --- a/packages/samples/specs/misc/appconfig/labels.tsp +++ b/packages/samples/specs/misc/appconfig/labels.tsp @@ -34,9 +34,7 @@ namespace LabelsResource { @query name?: string, @query after?: string, - @query({ - format: "multi", - }) + @query(#{ explode: true }) $Select?: LabelField[], ): { ...Response<200>; diff --git a/packages/samples/specs/misc/appconfig/revisions.tsp b/packages/samples/specs/misc/appconfig/revisions.tsp index 75a2696d29..bfcff59472 100644 --- a/packages/samples/specs/misc/appconfig/revisions.tsp +++ b/packages/samples/specs/misc/appconfig/revisions.tsp @@ -14,9 +14,7 @@ namespace RevisionsResource { ...AcceptDatetimeHeader, @doc("Used to select what fields are present in the returned resource(s).") - @query({ - format: "multi", - }) + @query(#{ explode: true }) $Select?: KeyField[], @doc("A filter used to match labels") diff --git a/packages/typespec-azure-core/lib/models.tsp b/packages/typespec-azure-core/lib/models.tsp index f34f2adbc5..308bf98d00 100644 --- a/packages/typespec-azure-core/lib/models.tsp +++ b/packages/typespec-azure-core/lib/models.tsp @@ -90,9 +90,7 @@ model FilterParameter { @doc("Provides the standard 'orderby' query parameter for list operations.") model OrderByQueryParameter { #suppress "@azure-tools/typespec-azure-core/prefer-csv-collection-format" "By design" - @query({ - format: "multi", - }) + @query(#{ explode: true }) @doc("Expressions that specify the order of returned results.") orderby?: string[]; } @@ -107,9 +105,7 @@ model FilterQueryParameter { @doc("Provides the standard 'select' query parameter for list operations.") model SelectQueryParameter { #suppress "@azure-tools/typespec-azure-core/prefer-csv-collection-format" "By design" - @query({ - format: "multi", - }) + @query(#{ explode: true }) @doc("Select the specified fields to be included in the response.") select?: string[]; } @@ -117,9 +113,7 @@ model SelectQueryParameter { @doc("Provides the standard 'expand' query parameter for list operations.") model ExpandQueryParameter { #suppress "@azure-tools/typespec-azure-core/prefer-csv-collection-format" "By design" - @query({ - format: "multi", - }) + @query(#{ explode: true }) @doc("Expand the indicated resources into the response.") expand?: string[]; } From 2a741b0679096211e564434ce20abd95ffd1d937 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 5 Aug 2024 19:01:16 -0700 Subject: [PATCH 04/12] Add new linter rule --- core | 2 +- .../azure-core/rules/no-query-explode.md | 35 ++++++++++++++++++ .../src/rules/no-query-explode.ts | 26 +++++++++++++ .../src/rules/prefer-csv-collection-format.ts | 8 +--- .../test/rules/no-query-explode.test.ts | 37 +++++++++++++++++++ .../prefer-csv-collection-format.test.ts | 15 +------- 6 files changed, 102 insertions(+), 21 deletions(-) create mode 100644 docs/libraries/azure-core/rules/no-query-explode.md create mode 100644 packages/typespec-azure-core/src/rules/no-query-explode.ts create mode 100644 packages/typespec-azure-core/test/rules/no-query-explode.test.ts diff --git a/core b/core index cc7f4f319b..6d950fbc33 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit cc7f4f319bb9c04a9dccc480ab82459a6344eb1f +Subproject commit 6d950fbc336b53363f1013d859adab0b6362f45b diff --git a/docs/libraries/azure-core/rules/no-query-explode.md b/docs/libraries/azure-core/rules/no-query-explode.md new file mode 100644 index 0000000000..f431555ab6 --- /dev/null +++ b/docs/libraries/azure-core/rules/no-query-explode.md @@ -0,0 +1,35 @@ +--- +title: "no-query-explode" +--- + +```text title="Full name" +@azure-tools/typespec-azure-core/no-query-explode +``` + +Azure services favor serializing query parameter of array type using the simple style(Where each values is joined by a `,`) +Using the `*` uri template modifier or `explode: true` property specify that each individual value of the array should be sent as a separate query parameter. + +#### ❌ Incorrect + +```tsp +op list( + @query(#{ explode: true }) + select: string[], +): Pet[]; +``` + +#### ✅ Correct + +```tsp +op list( + @query + select: string[], +): Pet[]; +``` + +```tsp +op list( + @query(#{ explode: false }) + select: string[], +): Pet[]; +``` diff --git a/packages/typespec-azure-core/src/rules/no-query-explode.ts b/packages/typespec-azure-core/src/rules/no-query-explode.ts new file mode 100644 index 0000000000..b4a6a299bd --- /dev/null +++ b/packages/typespec-azure-core/src/rules/no-query-explode.ts @@ -0,0 +1,26 @@ +import { createRule, ignoreDiagnostics, Operation } from "@typespec/compiler"; +import { getHttpOperation } from "@typespec/http"; + +export const noQueryExplodeRule = createRule({ + name: "no-query-explode", + description: "It is recommended to serialize query parameter without explode: true", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-query-explode", + messages: { + default: `It is preferred to not use explode: true for query parameters`, + }, + create(context) { + return { + operation: (operation: Operation) => { + const httpOperation = ignoreDiagnostics(getHttpOperation(context.program, operation)); + for (const prop of httpOperation.parameters.properties.filter((x) => x.kind === "query")) { + if (prop.options.explode === true) { + context.reportDiagnostic({ + target: prop.property, + }); + } + } + }, + }; + }, +}); diff --git a/packages/typespec-azure-core/src/rules/prefer-csv-collection-format.ts b/packages/typespec-azure-core/src/rules/prefer-csv-collection-format.ts index 41264730b5..7c5af55c6c 100644 --- a/packages/typespec-azure-core/src/rules/prefer-csv-collection-format.ts +++ b/packages/typespec-azure-core/src/rules/prefer-csv-collection-format.ts @@ -1,5 +1,5 @@ import { ModelProperty, createRule } from "@typespec/compiler"; -import { getHeaderFieldOptions, getQueryParamOptions } from "@typespec/http"; +import { getHeaderFieldOptions } from "@typespec/http"; import { isExcludedCoreType } from "./utils.js"; export const preferCsvCollectionFormatRule = createRule({ @@ -15,11 +15,7 @@ export const preferCsvCollectionFormatRule = createRule({ if (isExcludedCoreType(context.program, property)) return; const headerOptions = getHeaderFieldOptions(context.program, property); - const queryOptions = getQueryParamOptions(context.program, property); - if ( - (headerOptions?.format !== undefined && headerOptions?.format !== "csv") || - (queryOptions?.format !== undefined && queryOptions?.format !== "csv") - ) { + if (headerOptions?.format !== undefined && headerOptions?.format !== "csv") { context.reportDiagnostic({ target: property, }); diff --git a/packages/typespec-azure-core/test/rules/no-query-explode.test.ts b/packages/typespec-azure-core/test/rules/no-query-explode.test.ts new file mode 100644 index 0000000000..8c191ea954 --- /dev/null +++ b/packages/typespec-azure-core/test/rules/no-query-explode.test.ts @@ -0,0 +1,37 @@ +import { + BasicTestRunner, + LinterRuleTester, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; +import { noQueryExplodeRule } from "../../src/rules/no-query-explode.js"; +import { createAzureCoreTestRunner } from "../test-host.js"; + +let runner: BasicTestRunner; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await createAzureCoreTestRunner(); + tester = createLinterRuleTester(runner, noQueryExplodeRule, "@azure-tools/typespec-azure-core"); +}); + +it("emit warning if using * modifier", async () => { + await tester.expect(`@route("abc{?select*}") op foo(select: string[]): void;`).toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/no-query-explode", + }); +}); + +it("emit warning if using explode: true", async () => { + await tester + .expect(`op foo(@query(#{explode: true}) select: string[]): void;`) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/no-query-explode", + }); +}); + +it("is ok if query param doesn't specify explode", async () => { + await tester.expect(`op foo(@query select: string[]): void;`).toBeValid(); +}); +it("is ok if query param specify explode: false", async () => { + await tester.expect(`op foo(@query(#{explode: false}) select: string[]): void;`).toBeValid(); +}); diff --git a/packages/typespec-azure-core/test/rules/prefer-csv-collection-format.test.ts b/packages/typespec-azure-core/test/rules/prefer-csv-collection-format.test.ts index 0703beae53..d22ee384ef 100644 --- a/packages/typespec-azure-core/test/rules/prefer-csv-collection-format.test.ts +++ b/packages/typespec-azure-core/test/rules/prefer-csv-collection-format.test.ts @@ -20,8 +20,7 @@ describe("typespec-azure-core: prefer-csv-collection-format rule", () => { ); }); - // Header doesn't allow anything but csv for now so can't be testing this - it.skip("emit warning if using another format for a header ", async () => { + it("emit warning if using another format for a header ", async () => { await tester .expect(`op foo(@header({format: "tsv"}) select: string[]): void;`) .toEmitDiagnostics({ @@ -29,19 +28,7 @@ describe("typespec-azure-core: prefer-csv-collection-format rule", () => { }); }); - it("emit warning if using another format for a query parameter", async () => { - await tester - .expect(`op foo(@query({format: "tsv"}) select: string[]): void;`) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/prefer-csv-collection-format", - }); - }); - it("is ok if header param doesn't need format", async () => { await tester.expect(`op foo(@header filter: string): void;`).toBeValid(); }); - - it("is ok if query param doesn't need format", async () => { - await tester.expect(`op foo(@query filter: string): void;`).toBeValid(); - }); }); From 598768d1957261733ee4e363c03ac6aef44543e1 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 5 Aug 2024 19:59:09 -0700 Subject: [PATCH 05/12] fix --- packages/typespec-client-generator-core/src/http.ts | 6 +++++- .../typespec-client-generator-core/test/package.test.ts | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 81b3cb6d3a..476118455e 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -596,7 +596,11 @@ function getCollectionFormat( ? getHeaderFieldOptions(program, type) : undefined )?.format; - if (tspCollectionFormat === "form" || tspCollectionFormat === "simple") { + if ( + tspCollectionFormat === "form" || + tspCollectionFormat === "simple" || + tspCollectionFormat === "csv" + ) { return undefined; } return tspCollectionFormat; diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 666ea72ea3..9323901c0c 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -1185,7 +1185,8 @@ describe("typespec-client-generator-core: package", () => { await runner.compile(`@server("http://localhost:3000", "endpoint") @service({}) namespace My.Service; - + + #suppress "deprecated" "Legacy test" op myOp(@query({format: "multi"}) query: string): void; `); const sdkPackage = runner.context.sdkPackage; From 042fa8338e888321e68fb7af4e619176aaa8ab08 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 09:34:29 -0700 Subject: [PATCH 06/12] Add support for uri templates in autorest emitter --- packages/typespec-autorest/src/openapi.ts | 161 +++++---- .../typespec-autorest/test/metadata.test.ts | 6 +- .../typespec-autorest/test/parameters.test.ts | 307 +++++++++++------- 3 files changed, 274 insertions(+), 200 deletions(-) diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 12fe3a8dce..1ebdb1fec9 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -95,7 +95,10 @@ import { HttpOperation, HttpOperationBody, HttpOperationMultipartBody, + HttpOperationParameter, HttpOperationParameters, + HttpOperationPathParameter, + HttpOperationQueryParameter, HttpOperationResponse, HttpStatusCodeRange, HttpStatusCodesEntry, @@ -106,7 +109,6 @@ import { getAllHttpServices, getAuthentication, getHeaderFieldOptions, - getQueryParamOptions, getServers, getStatusCodeDescription, getVisibilitySuffix, @@ -141,7 +143,6 @@ import { OpenAPI2Operation, OpenAPI2Parameter, OpenAPI2ParameterBase, - OpenAPI2ParameterType, OpenAPI2PathItem, OpenAPI2PathParameter, OpenAPI2QueryParameter, @@ -426,10 +427,20 @@ export async function getOpenAPIForService( } const parameters: OpenAPI2PathParameter[] = []; for (const prop of server.parameters.values()) { - const param = getOpenAPI2Parameter(prop, "path", { - visibility: Visibility.Read, - ignoreMetadataAnnotations: false, - }); + const param = getOpenAPI2Parameter( + { + param: prop, + type: "path", + name: prop.name, + explode: false, + style: "simple", + allowReserved: false, + }, + { + visibility: Visibility.Read, + ignoreMetadataAnnotations: false, + } + ); if ( prop.type.kind === "Scalar" && ignoreDiagnostics( @@ -853,8 +864,7 @@ export async function getOpenAPIForService( } function getResponseHeader(prop: ModelProperty): OpenAPI2HeaderDefinition { - const header: any = {}; - populateParameter(header, prop, "header", { + const header: any = getOpenAPI2HeaderParameter(prop, { visibility: Visibility.Read, ignoreMetadataAnnotations: false, }); @@ -1050,11 +1060,8 @@ export async function getOpenAPIForService( if (httpOpParam.type === "header" && isContentTypeHeader(program, httpOpParam.param)) { continue; } - emitParameter( - httpOpParam.param, - httpOpParam.type, - { visibility, ignoreMetadataAnnotations: false }, - httpOpParam.name + emitParameter(httpOpParam.param, () => + getOpenAPI2Parameter(httpOpParam, { visibility, ignoreMetadataAnnotations: false }) ); } @@ -1104,16 +1111,14 @@ export async function getOpenAPIForService( compilerAssert(bodyModelType.kind === "Model", "Body should always be a Model."); if (bodyModelType) { for (const param of bodyModelType.properties.values()) { - emitParameter(param, "formData", schemaContext, getJsonName(param)); + emitParameter(param, () => + getOpenAPI2FormDataParameter(param, schemaContext, getJsonName(param)) + ); } } } else if (body.property) { - emitParameter( - body.property, - "body", - { visibility, ignoreMetadataAnnotations: false }, - getJsonName(body.property), - schema + currentEndpoint.parameters.push( + getOpenAPI2BodyParameter(body.property, getJsonName(body.property), schema) ); } else { currentEndpoint.parameters.push({ @@ -1166,23 +1171,17 @@ export async function getOpenAPIForService( return undefined; } - function emitParameter( - param: ModelProperty, - kind: OpenAPI2ParameterType, - schemaContext: SchemaContext, - name?: string, - typeOverride?: any - ) { - if (isNeverType(param.type)) { + function emitParameter(prop: ModelProperty, resolve: () => OpenAPI2Parameter) { + if (isNeverType(prop.type)) { return; } - const ph = getParamPlaceholder(param); + const ph = getParamPlaceholder(prop); currentEndpoint.parameters.push(ph); // If the parameter already has a $ref, don't bother populating it if (!("$ref" in ph)) { - populateParameter(ph, param, kind, schemaContext, name, typeOverride); + Object.assign(ph, resolve()); } } @@ -1244,10 +1243,7 @@ export async function getOpenAPIForService( } } - function getOpenAPI2ParameterBase( - param: ModelProperty, - name: string | undefined - ): OpenAPI2ParameterBase { + function getOpenAPI2ParameterBase(param: ModelProperty, name?: string): OpenAPI2ParameterBase { const base: OpenAPI2ParameterBase = { name: name ?? param.name, required: !param.optional, @@ -1308,41 +1304,52 @@ export async function getOpenAPIForService( } } - function getOpenAPI2QueryParameter( - param: ModelProperty, - schemaContext: SchemaContext, - name?: string - ): OpenAPI2QueryParameter { - const base = getOpenAPI2ParameterBase(param, name); - let collectionFormat = getQueryParamOptions(program, param).format; + function getQueryCollectionFormat(param: HttpOperationQueryParameter): string | undefined { + if (param.explode) { + return "multi"; + } + let collectionFormat = param.format; if (collectionFormat && !["csv", "ssv", "tsv", "pipes", "multi"].includes(collectionFormat)) { collectionFormat = undefined; - reportDiagnostic(program, { code: "invalid-multi-collection-format", target: param }); + reportDiagnostic(program, { code: "invalid-multi-collection-format", target: param.param }); } + return collectionFormat; + } + function getOpenAPI2QueryParameter( + param: HttpOperationQueryParameter, + schemaContext: SchemaContext + ): OpenAPI2QueryParameter { + const base = getOpenAPI2ParameterBase(param.param, param.name); + const collectionFormat = getQueryCollectionFormat(param); + const schema = getSimpleParameterSchema(param.param, schemaContext, base.name); return { in: "query", - collectionFormat: collectionFormat as any, - default: param.defaultValue && getDefaultValue(param.defaultValue), + collectionFormat: + collectionFormat === "csv" && schema.items === undefined // If csv + ? undefined + : (collectionFormat as any), + default: param.param.defaultValue && getDefaultValue(param.param.defaultValue), ...base, - ...getSimpleParameterSchema(param, schemaContext, base.name), + ...schema, }; } function getOpenAPI2PathParameter( - param: ModelProperty, - schemaContext: SchemaContext, - name?: string + param: HttpOperationPathParameter, + schemaContext: SchemaContext ): OpenAPI2PathParameter { - const base = getOpenAPI2ParameterBase(param, name); + const base = getOpenAPI2ParameterBase(param.param, param.name); return { in: "path", - default: param.defaultValue && getDefaultValue(param.defaultValue), + "x-ms-skip-url-encoding": param.allowReserved ?? undefined, + default: param.param.defaultValue && getDefaultValue(param.param.defaultValue), ...base, - ...getSimpleParameterSchema(param, schemaContext, base.name), + ...getSimpleParameterSchema(param.param, schemaContext, base.name), }; } + function getOpenAPI2HeaderParameter( param: ModelProperty, schemaContext: SchemaContext, @@ -1363,58 +1370,40 @@ export async function getOpenAPIForService( }; } - function getOpenAPI2ParameterInternal( - param: ModelProperty, - kind: T, - schemaContext: SchemaContext, - name?: string, - bodySchema?: any - ): OpenAPI2Parameter & { in: T } { - switch (kind) { - case "body": - return getOpenAPI2BodyParameter(param, name, bodySchema) as any; - case "formData": - return getOpenAPI2FormDataParameter(param, schemaContext, name) as any; + function getOpenAPI2ParameterInternal( + param: HttpOperationParameter, + schemaContext: SchemaContext + ): OpenAPI2Parameter & { in: "query" | "path" | "header" } { + switch (param.type) { case "query": - return getOpenAPI2QueryParameter(param, schemaContext, name) as any; + return getOpenAPI2QueryParameter(param, schemaContext); case "path": - return getOpenAPI2PathParameter(param, schemaContext, name) as any; + return getOpenAPI2PathParameter(param, schemaContext); case "header": - return getOpenAPI2HeaderParameter(param, schemaContext, name) as any; + return getOpenAPI2HeaderParameter(param.param, schemaContext, param.name); default: - const _assertNever: never = kind; + const _assertNever: never = param; compilerAssert(false, "Unreachable"); } } function getOpenAPI2Parameter( - param: ModelProperty, - kind: T, - schemaContext: SchemaContext, - name?: string, - bodySchema?: any + param: HttpOperationParameter & { type: T }, + schemaContext: SchemaContext ): OpenAPI2Parameter & { in: T } { - const value = getOpenAPI2ParameterInternal(param, kind, schemaContext, name, bodySchema); + const value = getOpenAPI2ParameterInternal(param, schemaContext); // Apply decorators to a copy of the parameter definition. We use // Object.assign here because applyIntrinsicDecorators returns a new object // based on the target object and we need to apply its changes back to the // original parameter. Object.assign( value, - applyIntrinsicDecorators(param, { type: (value as any).type, format: (value as any).format }) + applyIntrinsicDecorators(param.param, { + type: (value as any).type, + format: (value as any).format, + }) ); - return value; - } - - function populateParameter( - ph: OpenAPI2Parameter, - param: ModelProperty, - kind: OpenAPI2ParameterType, - schemaContext: SchemaContext, - name?: string, - bodySchema?: any - ) { - Object.assign(ph, getOpenAPI2Parameter(param, kind, schemaContext, name, bodySchema)); + return value as any; } function emitParameters() { diff --git a/packages/typespec-autorest/test/metadata.test.ts b/packages/typespec-autorest/test/metadata.test.ts index 89a31560ba..aa50c0f0b3 100644 --- a/packages/typespec-autorest/test/metadata.test.ts +++ b/packages/typespec-autorest/test/metadata.test.ts @@ -559,9 +559,6 @@ describe("typespec-autorest: metadata", () => { post: { operationId: "Create", parameters: [ - { - $ref: "#/parameters/Pet.id", - }, { name: "h1", in: "header", @@ -580,6 +577,9 @@ describe("typespec-autorest: metadata", () => { schema: { $ref: "#/definitions/PetCreate" }, required: true, }, + { + $ref: "#/parameters/Pet.id", + }, ], responses: { "200": { diff --git a/packages/typespec-autorest/test/parameters.test.ts b/packages/typespec-autorest/test/parameters.test.ts index 6865ed6bea..e140ff7380 100644 --- a/packages/typespec-autorest/test/parameters.test.ts +++ b/packages/typespec-autorest/test/parameters.test.ts @@ -1,17 +1,58 @@ import { expectDiagnostics } from "@typespec/compiler/testing"; import { deepStrictEqual, ok, strictEqual } from "assert"; import { describe, expect, it } from "vitest"; -import { OpenAPI2HeaderParameter, OpenAPI2QueryParameter } from "../src/openapi2-document.js"; +import { + OpenAPI2HeaderParameter, + OpenAPI2PathParameter, + OpenAPI2QueryParameter, +} from "../src/openapi2-document.js"; import { diagnoseOpenApiFor, ignoreUseStandardOps, openApiFor } from "./test-host.js"; -describe("typespec-autorest: parameters", () => { +describe("path parameters", () => { + async function getPathParam(code: string, name = "myParam"): Promise { + const res = await openApiFor(code); + return res.paths[`/{${name}}`].get.parameters[0]; + } + + it("figure out the route parameter from the name of the param", async () => { + const res = await openApiFor(`op test(@path myParam: string): void;`); + expect(res.paths).toHaveProperty("/{myParam}"); + }); + + it("uses explicit name provided from @path", async () => { + const res = await openApiFor(`op test(@path("my-custom-path") myParam: string): void;`); + expect(res.paths).toHaveProperty("/{my-custom-path}"); + }); + + describe("setting reserved expansion attribute applies the x-ms-skip-url-encoding property", () => { + it("with option", async () => { + const param = await getPathParam( + `op test(@path(#{allowReserved: true}) myParam: string[]): void;` + ); + expect(param).toMatchObject({ + "x-ms-skip-url-encoding": true, + }); + }); + it("with uri template", async () => { + const param = await getPathParam(`@route("{+myParam}") op test(myParam: string[]): void;`); + expect(param).toMatchObject({ + "x-ms-skip-url-encoding": true, + }); + }); + }); +}); + +describe("query parameters", () => { + async function getQueryParam(code: string): Promise { + const res = await openApiFor(code); + const param = res.paths[`/`].get.parameters[0]; + strictEqual(param.in, "query"); + return param; + } + it("create a query param", async () => { - const res = await openApiFor( - ` - op test(@query arg1: string): void; - ` - ); - deepStrictEqual(res.paths["/"].get.parameters[0], { + const param = await getQueryParam(`op test(@query arg1: string): void;`); + deepStrictEqual(param, { in: "query", name: "arg1", required: true, @@ -19,9 +60,39 @@ describe("typespec-autorest: parameters", () => { }); }); - it("create a query param of array type", async () => { + it("create a query param with different name", async () => { + const param = await getQueryParam(`op test(@query("$select") select: string): void;`); + deepStrictEqual(param, { + in: "query", + name: "$select", + "x-ms-client-name": "select", + required: true, + type: "string", + }); + }); + + describe("setting parameter explode modifier set collectionFormat to multi", () => { + it("with option", async () => { + const param = await getQueryParam( + `op test(@query(#{explode: true}) myParam: string[]): void;` + ); + expect(param).toMatchObject({ + collectionFormat: "multi", + }); + }); + + it("with uri template", async () => { + const param = await getQueryParam(`@route("{?myParam*}") op test(myParam: string[]): void;`); + expect(param).toMatchObject({ + collectionFormat: "multi", + }); + }); + }); + + it("LEGACY specify the format", async () => { const res = await openApiFor( ` + #suppress "deprecated" "test" op test(@query({format: "multi"}) arg1: string[], @query({format: "csv"}) arg2: string[]): void; ` ); @@ -43,22 +114,6 @@ describe("typespec-autorest: parameters", () => { }); }); - it("create a header param of array type", async () => { - const res = await openApiFor( - ` - op test(@header({format: "csv"}) arg1: string[]): void; - ` - ); - deepStrictEqual(res.paths["/"].get.parameters[0], { - in: "header", - name: "arg1", - required: true, - type: "array", - items: { type: "string" }, - collectionFormat: "csv", - }); - }); - it("create a query param that is a model property", async () => { const res = await openApiFor( ` @@ -96,27 +151,23 @@ describe("typespec-autorest: parameters", () => { ); strictEqual(res.paths["/"].get.parameters[0].description, "my-doc"); }); +}); - it("@query/@header/@path names & @projectedName on body parameter are honored (LEGACY)", async () => { +describe("header parameters", () => { + it("create a header param of array type", 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; - + op test(@header({format: "csv"}) arg1: 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"); + deepStrictEqual(res.paths["/"].get.parameters[0], { + in: "header", + name: "arg1", + required: true, + type: "array", + items: { type: "string" }, + collectionFormat: "csv", + }); }); it("errors on duplicate parameter keys", async () => { @@ -216,7 +267,9 @@ describe("typespec-autorest: parameters", () => { strictEqual(res.paths["/"].get.parameters[0].in, "query"); strictEqual(res.paths["/"].get.parameters[0].name, "top"); }); +}); +describe("body parameters", () => { it("omit request body if type is void", async () => { const res = await openApiFor(`op test(@body foo: void): void;`); deepStrictEqual(res.paths["/"].post.parameters, []); @@ -288,74 +341,84 @@ describe("typespec-autorest: parameters", () => { required: ["name"], }); }); +}); - describe("content type parameter", () => { - it("header named with 'Content-Type' gets resolved as content type for operation.", async () => { - const res = await openApiFor( - ` - op test( - @header("Content-Type") explicitContentType: "application/octet-stream", - @body foo: string - ): void; - ` - ); - deepStrictEqual(res.paths["/"].post.consumes, ["application/octet-stream"]); - }); +describe("content type parameter", () => { + it("header named with 'Content-Type' gets resolved as content type for operation.", async () => { + const res = await openApiFor( + ` + op test( + @header("Content-Type") explicitContentType: "application/octet-stream", + @body foo: string + ): void; + ` + ); + deepStrictEqual(res.paths["/"].post.consumes, ["application/octet-stream"]); + }); - it("header named contentType gets resolved as content type for operation.", async () => { - const res = await openApiFor( - ` - op test( - @header contentType: "application/octet-stream", - @body foo: string - ): void; - ` - ); - deepStrictEqual(res.paths["/"].post.consumes, ["application/octet-stream"]); - }); + it("header named contentType gets resolved as content type for operation.", async () => { + const res = await openApiFor( + ` + op test( + @header contentType: "application/octet-stream", + @body foo: string + ): void; + ` + ); + deepStrictEqual(res.paths["/"].post.consumes, ["application/octet-stream"]); + }); + + it("query named contentType doesn't get resolved as the content type parmaeter.", async () => { + const res = await openApiFor( + ` + op test( + @query contentType: "application/octet-stream", + @body foo: string + ): void; + ` + ); + strictEqual(res.paths["/"].post.consumes, undefined); + }); +}); - it("query named contentType doesn't get resolved as the content type parmaeter.", async () => { +describe("misc", () => { + describe("type can only be primitive items without $ref", () => { + async function testParameter( + decorator: string, + type: string + ): Promise { const res = await openApiFor( ` - op test( - @query contentType: "application/octet-stream", - @body foo: string - ): void; + union NamedUnion {"one", "two"} + enum Foo {one, two} + op test(${decorator} arg1: ${type}): void; ` ); - strictEqual(res.paths["/"].post.consumes, undefined); - }); - - describe("type can only be primitive items without $ref", () => { - async function testParameter( - decorator: string, - type: string - ): Promise { - const res = await openApiFor( - ` - union NamedUnion {"one", "two"} - enum Foo {one, two} - op test(${decorator} arg1: ${type}): void; - ` - ); - return res.paths["/"].get.parameters[0]; - } - - ["query", "header"].forEach((kind) => { - describe(kind, () => { - it("enum is kept inline", async () => { - deepStrictEqual(await testParameter(`@${kind}`, "Foo"), { - in: kind, - name: "arg1", - required: true, - type: "string", - enum: ["one", "two"], - "x-ms-enum": { modelAsString: false, name: "Foo" }, - }); + return res.paths["/"].get.parameters[0]; + } + + ["query", "header"].forEach((kind) => { + describe(kind, () => { + it("enum is kept inline", async () => { + deepStrictEqual(await testParameter(`@${kind}`, "Foo"), { + in: kind, + name: "arg1", + required: true, + type: "string", + enum: ["one", "two"], + "x-ms-enum": { modelAsString: false, name: "Foo" }, }); + }); - it("array of enum is kept inline", async () => { - deepStrictEqual(await testParameter(`@${kind}({format: "csv"})`, "Foo[]"), { + it("array of enum is kept inline", async () => { + deepStrictEqual( + await testParameter( + ` + #suppress "deprecated" "For tests" + @${kind}({format: "csv"})`, + "Foo[]" + ), + { in: kind, name: "arg1", required: true, @@ -366,21 +429,43 @@ describe("typespec-autorest: parameters", () => { enum: ["one", "two"], "x-ms-enum": { modelAsString: false, name: "Foo" }, }, - }); - }); + } + ); + }); - it("named union is kept inline", async () => { - deepStrictEqual(await testParameter(`@${kind}`, "NamedUnion"), { - in: kind, - name: "arg1", - required: true, - type: "string", - enum: ["one", "two"], - "x-ms-enum": { modelAsString: false, name: "NamedUnion" }, - }); + it("named union is kept inline", async () => { + deepStrictEqual(await testParameter(`@${kind}`, "NamedUnion"), { + in: kind, + name: "arg1", + required: true, + type: "string", + enum: ["one", "two"], + "x-ms-enum": { modelAsString: false, name: "NamedUnion" }, }); }); }); }); + + 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"); + }); }); }); From 83c4d5264a320b82382120672f118214c01ee600 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 09:37:36 -0700 Subject: [PATCH 07/12] ADd autorest support --- core | 2 +- packages/typespec-autorest/src/openapi.ts | 9 +++++++-- packages/typespec-autorest/test/metadata.test.ts | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/core b/core index 6d950fbc33..08fc78126b 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 6d950fbc336b53363f1013d859adab0b6362f45b +Subproject commit 08fc78126b7cc7278550ec5b977d145d6c11e36f diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 1ebdb1fec9..b34958938f 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -1341,13 +1341,18 @@ export async function getOpenAPIForService( ): OpenAPI2PathParameter { const base = getOpenAPI2ParameterBase(param.param, param.name); - return { + const result: OpenAPI2PathParameter = { in: "path", - "x-ms-skip-url-encoding": param.allowReserved ?? undefined, default: param.param.defaultValue && getDefaultValue(param.param.defaultValue), ...base, ...getSimpleParameterSchema(param.param, schemaContext, base.name), }; + + if (param.allowReserved) { + result["x-ms-skip-url-encoding"] = true; + } + + return result; } function getOpenAPI2HeaderParameter( diff --git a/packages/typespec-autorest/test/metadata.test.ts b/packages/typespec-autorest/test/metadata.test.ts index aa50c0f0b3..4402517980 100644 --- a/packages/typespec-autorest/test/metadata.test.ts +++ b/packages/typespec-autorest/test/metadata.test.ts @@ -571,15 +571,15 @@ describe("typespec-autorest: metadata", () => { required: true, type: "string", }, + { + $ref: "#/parameters/Pet.id", + }, { name: "body", in: "body", schema: { $ref: "#/definitions/PetCreate" }, required: true, }, - { - $ref: "#/parameters/Pet.id", - }, ], responses: { "200": { From fde849d4af27b7182243cb2b7610499d75947b29 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 09:38:28 -0700 Subject: [PATCH 08/12] Changelog --- .chronus/changes/uptake-uri-templates-2024-7-6-9-38-18.md | 6 ++++++ .chronus/changes/uptake-uri-templates-2024-7-6-9-38-7.md | 7 +++++++ 2 files changed, 13 insertions(+) create mode 100644 .chronus/changes/uptake-uri-templates-2024-7-6-9-38-18.md create mode 100644 .chronus/changes/uptake-uri-templates-2024-7-6-9-38-7.md diff --git a/.chronus/changes/uptake-uri-templates-2024-7-6-9-38-18.md b/.chronus/changes/uptake-uri-templates-2024-7-6-9-38-18.md new file mode 100644 index 0000000000..40b3c23d94 --- /dev/null +++ b/.chronus/changes/uptake-uri-templates-2024-7-6-9-38-18.md @@ -0,0 +1,6 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-client-generator-core" +--- + diff --git a/.chronus/changes/uptake-uri-templates-2024-7-6-9-38-7.md b/.chronus/changes/uptake-uri-templates-2024-7-6-9-38-7.md new file mode 100644 index 0000000000..2a4e348bd1 --- /dev/null +++ b/.chronus/changes/uptake-uri-templates-2024-7-6-9-38-7.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-autorest" +--- + +Add support for URI templates in routes From a1c11941622fd9687a4a7904e8afa122ed17f3d1 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 09:58:06 -0700 Subject: [PATCH 09/12] updates --- .../openapi3/openapi.2022-08-31.yaml | 1 - .../openapi3/openapi.2022-08-31.yaml | 1 - .../openapi3/openapi.2022-08-31.yaml | 1 - .../openapi3/openapi.2023-02-07.yaml | 1 - .../openapi3/openapi.2022-08-31.yaml | 11 +- .../appconfig/@typespec/openapi3/openapi.yaml | 5 - .../test/metadata.test.ts | 666 ------------------ 7 files changed, 5 insertions(+), 681 deletions(-) delete mode 100644 packages/typespec-autorest-canonical/test/metadata.test.ts diff --git a/packages/samples/test/output/azure/core/data-plane/api-path-parameter/@typespec/openapi3/openapi.2022-08-31.yaml b/packages/samples/test/output/azure/core/data-plane/api-path-parameter/@typespec/openapi3/openapi.2022-08-31.yaml index 2d260b3707..7b9d53b04d 100644 --- a/packages/samples/test/output/azure/core/data-plane/api-path-parameter/@typespec/openapi3/openapi.2022-08-31.yaml +++ b/packages/samples/test/output/azure/core/data-plane/api-path-parameter/@typespec/openapi3/openapi.2022-08-31.yaml @@ -725,7 +725,6 @@ components: type: array items: type: string - style: form explode: true Azure.Core.SkipQueryParameter: name: skip diff --git a/packages/samples/test/output/azure/core/data-plane/custom-error-type/@typespec/openapi3/openapi.2022-08-31.yaml b/packages/samples/test/output/azure/core/data-plane/custom-error-type/@typespec/openapi3/openapi.2022-08-31.yaml index 508d595759..5b221b5c78 100644 --- a/packages/samples/test/output/azure/core/data-plane/custom-error-type/@typespec/openapi3/openapi.2022-08-31.yaml +++ b/packages/samples/test/output/azure/core/data-plane/custom-error-type/@typespec/openapi3/openapi.2022-08-31.yaml @@ -375,7 +375,6 @@ components: type: array items: type: string - style: form explode: true Azure.Core.SkipQueryParameter: name: skip diff --git a/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2022-08-31.yaml b/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2022-08-31.yaml index cee3ed7f6d..a62eaa5237 100644 --- a/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2022-08-31.yaml +++ b/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2022-08-31.yaml @@ -642,7 +642,6 @@ components: type: array items: type: string - style: form explode: true Azure.Core.SkipQueryParameter: name: skip diff --git a/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2023-02-07.yaml b/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2023-02-07.yaml index 80daf12544..24fd671ccf 100644 --- a/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2023-02-07.yaml +++ b/packages/samples/test/output/azure/core/data-plane/trait-versioning/@typespec/openapi3/openapi.2023-02-07.yaml @@ -726,7 +726,6 @@ components: type: array items: type: string - style: form explode: true Azure.Core.SkipQueryParameter: name: skip diff --git a/packages/samples/test/output/azure/core/data-plane/widget-manager/@typespec/openapi3/openapi.2022-08-31.yaml b/packages/samples/test/output/azure/core/data-plane/widget-manager/@typespec/openapi3/openapi.2022-08-31.yaml index 414a2a5497..d7d8262283 100644 --- a/packages/samples/test/output/azure/core/data-plane/widget-manager/@typespec/openapi3/openapi.2022-08-31.yaml +++ b/packages/samples/test/output/azure/core/data-plane/widget-manager/@typespec/openapi3/openapi.2022-08-31.yaml @@ -1181,6 +1181,11 @@ paths: '202': description: The request has been accepted for processing, but processing has not yet completed. headers: + x-ms-request-id: + required: false + description: An opaque, globally-unique, server-generated string identifier for the request. + schema: + $ref: '#/components/schemas/Azure.Core.uuid' Operation-Location: required: true description: The location for monitoring the operation state. @@ -1197,11 +1202,6 @@ paths: description: An opaque, globally-unique, client-generated string identifier for the request. schema: $ref: '#/components/schemas/Azure.Core.uuid' - x-ms-request-id: - required: false - description: An opaque, globally-unique, server-generated string identifier for the request. - schema: - $ref: '#/components/schemas/Azure.Core.uuid' content: application/json: schema: @@ -1349,7 +1349,6 @@ components: type: array items: type: string - style: form explode: true Azure.Core.SkipQueryParameter: name: skip diff --git a/packages/samples/test/output/azure/core/misc/appconfig/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/azure/core/misc/appconfig/@typespec/openapi3/openapi.yaml index 60e41db0dc..b9ba87e303 100644 --- a/packages/samples/test/output/azure/core/misc/appconfig/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/azure/core/misc/appconfig/@typespec/openapi3/openapi.yaml @@ -101,7 +101,6 @@ paths: type: array items: $ref: '#/components/schemas/KeyField' - style: form explode: true responses: '200': @@ -147,7 +146,6 @@ paths: type: array items: $ref: '#/components/schemas/KeyField' - style: form explode: true responses: '200': @@ -182,7 +180,6 @@ paths: type: array items: $ref: '#/components/schemas/KeyField' - style: form explode: true responses: '200': @@ -464,7 +461,6 @@ paths: type: array items: $ref: '#/components/schemas/LabelField' - style: form explode: true responses: '200': @@ -570,7 +566,6 @@ paths: type: array items: $ref: '#/components/schemas/KeyField' - style: form explode: true - name: label in: query diff --git a/packages/typespec-autorest-canonical/test/metadata.test.ts b/packages/typespec-autorest-canonical/test/metadata.test.ts deleted file mode 100644 index 5ccca863f5..0000000000 --- a/packages/typespec-autorest-canonical/test/metadata.test.ts +++ /dev/null @@ -1,666 +0,0 @@ -import { deepStrictEqual, strictEqual } from "assert"; -import { describe, it } from "vitest"; -import { openApiFor } from "./test-host.js"; - -it("will expose create visibility properties on PATCH model using @requestVisibility", async () => { - const res = await openApiFor(` - model M { - @visibility("read") r: string; - @visibility("read", "create") rc?: string; - @visibility("read", "update", "create") ruc?: string; - } - @parameterVisibility("create", "update") - @route("/") @patch op createOrUpdate(...M): M; - `); - - const response = res.paths["/"].patch.responses["200"].schema; - const request = res.paths["/"].patch.parameters[0].schema; - - deepStrictEqual(response, { $ref: "#/definitions/M" }); - deepStrictEqual(request, { $ref: "#/definitions/MCreateOrUpdate" }); - deepStrictEqual(res.definitions, { - M: { - type: "object", - properties: { - r: { type: "string", readOnly: true }, - rc: { type: "string", "x-ms-mutability": ["read", "create"] }, - ruc: { type: "string", "x-ms-mutability": ["read", "update", "create"] }, - }, - required: ["r"], - }, - MCreateOrUpdate: { - type: "object", - properties: { - rc: { type: "string", "x-ms-mutability": ["read", "create"] }, - ruc: { type: "string", "x-ms-mutability": ["read", "update", "create"] }, - }, - }, - }); -}); - -it("will expose create visibility properties on PUT model", async () => { - const res = await openApiFor(` - model M { - @visibility("read") r: string; - @visibility("read", "create") rc?: string; - @visibility("read", "update", "create") ruc?: string; - } - @route("/") @put op createOrUpdate(...M): M; - `); - - const response = res.paths["/"].put.responses["200"].schema; - const request = res.paths["/"].put.parameters[0].schema; - - deepStrictEqual(response, { $ref: "#/definitions/M" }); - deepStrictEqual(request, { $ref: "#/definitions/M" }); - deepStrictEqual(res.definitions, { - M: { - type: "object", - properties: { - r: { type: "string", readOnly: true }, - rc: { type: "string", "x-ms-mutability": ["read", "create"] }, - ruc: { type: "string", "x-ms-mutability": ["read", "update", "create"] }, - }, - required: ["r"], - }, - }); -}); - -it("ensures properties are required for array updates", async () => { - const res = await openApiFor(` - model Person { - @visibility("read") id: string; - @visibility("create") secret: string; - name: string; - - @visibility("read", "create") - test: string; - - @visibility("read", "update") - other: string; - - @visibility("read", "create", "update") - relatives: PersonRelative[]; - } - - model PersonRelative { - person: Person; - relationship: string; - } - @route("/") @patch op update(...Person): Person; - `); - - const response = res.paths["/"].patch.responses["200"].schema; - const request = res.paths["/"].patch.parameters[0].schema; - - deepStrictEqual(response, { $ref: "#/definitions/Person" }); - deepStrictEqual(request, { $ref: "#/definitions/PersonUpdate" }); - deepStrictEqual(res.definitions.PersonUpdate, { - type: "object", - properties: { - name: { type: "string" }, - other: { type: "string", "x-ms-mutability": ["read", "update"] }, - relatives: { - type: "array", - "x-ms-identifiers": [], - "x-ms-mutability": ["read", "update", "create"], - items: { $ref: "#/definitions/PersonRelative" }, - }, - }, - }); - deepStrictEqual(res.definitions.PersonRelative, { - type: "object", - properties: { - person: { $ref: "#/definitions/Person" }, - relationship: { type: "string" }, - }, - required: ["person", "relationship"], - }); - deepStrictEqual(res.definitions.Person, { - type: "object", - properties: { - id: { type: "string", readOnly: true }, - name: { type: "string" }, - other: { type: "string", "x-ms-mutability": ["read", "update"] }, - relatives: { - type: "array", - "x-ms-identifiers": [], - "x-ms-mutability": ["read", "update", "create"], - items: { $ref: "#/definitions/PersonRelative" }, - }, - secret: { type: "string", "x-ms-mutability": ["create"] }, - test: { type: "string", "x-ms-mutability": ["read", "create"] }, - }, - required: ["id", "secret", "name", "test", "other", "relatives"], - }); -}); - -it("can share read, update, create visibility via x-ms-mutability or readonly", async () => { - const res = await openApiFor(` - model M { - @visibility("read") r?: string; - @visibility("create") c?: string; - @visibility("update") u?: string; - @visibility("update", "create") uc?: string; - @visibility("read", "update", "create") ruc?: string; - - } - @route("/") @post op create(...M): M; -`); - - const request = res.paths["/"].post.parameters[0].schema; - deepStrictEqual(request, { $ref: "#/definitions/M" }); - - const response = res.paths["/"].post.responses["200"].schema; - deepStrictEqual(response, { $ref: "#/definitions/M" }); - - deepStrictEqual(res.definitions, { - M: { - type: "object", - properties: { - r: { type: "string", readOnly: true }, // x-ms-mutability: ["read"] not used when readOnly: true can suffice - c: { type: "string", "x-ms-mutability": ["create"] }, - u: { type: "string", "x-ms-mutability": ["update"] }, - uc: { type: "string", "x-ms-mutability": ["update", "create"] }, - ruc: { type: "string", "x-ms-mutability": ["read", "update", "create"] }, - }, - }, - }); -}); - -it("does not emit invisible, unshared readonly properties", async () => { - const res = await openApiFor(` - model M { - @visibility("read") r?: string; - @visibility("query") q?: string; - @visibility("delete") d?: string; - } - @route("/") @get op get(...M): M; -`); - - const request = res.paths["/"].get.parameters[0].schema; - deepStrictEqual(request, { $ref: "#/definitions/MQuery" }); - - const response = res.paths["/"].get.responses["200"].schema; - deepStrictEqual(response, { $ref: "#/definitions/M" }); - - deepStrictEqual(res.definitions, { - MQuery: { - type: "object", - properties: { - q: { type: "string" }, - }, - }, - M: { - type: "object", - properties: { - r: { type: "string", readOnly: true }, - }, - }, - }); -}); - -it("bubbles up visibility changes to referencers", async () => { - const res = await openApiFor( - ` - model M { - @visibility("read") r?: string; - @visibility("create") c?: string; - @visibility("update") u?: string; - @visibility("delete") d?: string; - @visibility("query") q?: string; - } - - // base model - model D extends M {} - - // property type - model R { - m?: M; - } - // array element type - model A { - a?: M[]; - } - - @route("/M") - interface IM { - @get get(...M): M; - @post create(...M): M; - @put createOrUpdate(...M): M; - @patch update(...M): M; - @delete delete(...M): void; - } - - @route("/D") - interface ID { - @get get(...D): D; - @post create(...D): D; - @put createOrUpdate(...D): D; - @patch update(...D): D; - @delete delete(...D): void; - } - - @route("/R") - interface IR { - @get op get(id: string): R; - @post op create(...R): R; - @put op createOrUpdate(...R): R; - @patch op update(...R): R; - @delete op delete(...D): void; - } - - `, - { "omit-unreachable-types": true } - ); - - deepStrictEqual(res.definitions, { - M: { - type: "object", - properties: { - r: { - type: "string", - readOnly: true, - }, - c: { - type: "string", - "x-ms-mutability": ["create"], - }, - u: { - type: "string", - "x-ms-mutability": ["update"], - }, - }, - }, - MDelete: { - type: "object", - properties: { - d: { - type: "string", - }, - }, - }, - MQuery: { - type: "object", - properties: { - q: { - type: "string", - }, - }, - }, - D: { - type: "object", - allOf: [ - { - $ref: "#/definitions/M", - }, - ], - }, - DDelete: { - type: "object", - allOf: [ - { - $ref: "#/definitions/MDelete", - }, - ], - }, - DQuery: { - type: "object", - allOf: [ - { - $ref: "#/definitions/MQuery", - }, - ], - }, - R: { - type: "object", - properties: { - m: { - $ref: "#/definitions/M", - }, - }, - }, - }); -}); - -it("puts inapplicable metadata in schema", async () => { - const res = await openApiFor( - ` - model Parameters { - @query q: string; - @path p: string; - @header h: string; - } - @route("/single") @get op single(...Parameters): string; - @route("/batch") @get op batch(@bodyRoot body: Parameters[]): string; - ` - ); - deepStrictEqual(res.paths, { - "/single/{p}": { - get: { - operationId: "Single", - parameters: [ - { $ref: "#/parameters/Parameters.q" }, - { $ref: "#/parameters/Parameters.p" }, - { $ref: "#/parameters/Parameters.h" }, - ], - responses: { - "200": { - description: "The request has succeeded.", - schema: { type: "string" }, - }, - }, - }, - }, - "/batch": { - get: { - operationId: "Batch", - responses: { - "200": { - description: "The request has succeeded.", - schema: { type: "string" }, - }, - }, - parameters: [ - { - in: "body", - name: "body", - required: true, - schema: { - type: "array", - items: { $ref: "#/definitions/Parameters" }, - "x-ms-identifiers": [], - }, - }, - ], - }, - }, - }); - deepStrictEqual(res.parameters, { - "Parameters.q": { - name: "q", - in: "query", - required: true, - type: "string", - "x-ms-parameter-location": "method", - }, - "Parameters.p": { - name: "p", - in: "path", - required: true, - type: "string", - "x-ms-parameter-location": "method", - }, - "Parameters.h": { - name: "h", - in: "header", - required: true, - type: "string", - "x-ms-parameter-location": "method", - }, - }); - - deepStrictEqual(res.definitions, { - Parameters: { - properties: { - h: { - type: "string", - }, - p: { - type: "string", - }, - q: { - type: "string", - }, - }, - required: ["q", "p", "h"], - type: "object", - }, - }); -}); - -it("uses item suffix if array element has inapplicable metadata and is used with more than one visibility.", async () => { - const res = await openApiFor( - ` - model Thing { - @header etag: string; - name: string; - @visibility("delete") d: string; - } - @route("/") @post op createMultiple(...Thing): Thing[]; - ` - ); - - const request = res.paths["/"].post.parameters[1].schema; - deepStrictEqual(request, { $ref: "#/definitions/Thing" }); - - const response = res.paths["/"].post.responses["200"].schema; - deepStrictEqual(response, { - type: "array", - items: { $ref: "#/definitions/ThingItem" }, - "x-ms-identifiers": [], - }); - - deepStrictEqual(res.parameters, { - "Thing.etag": { - name: "etag", - in: "header", - required: true, - type: "string", - "x-ms-parameter-location": "method", - }, - }); - - deepStrictEqual(res.definitions, { - Thing: { - type: "object", - properties: { - name: { type: "string" }, - }, - required: ["name"], - }, - ThingItem: { - type: "object", - properties: { - etag: { type: "string" }, - name: { type: "string" }, - }, - required: ["etag", "name"], - }, - }); -}); - -it("handles cycle in untransformed model", async () => { - const res = await openApiFor( - ` - model Thing { - inner?: Thing; - } - @route("/") @get op get(): Thing; - ` - ); - - const response = res.paths["/"].get.responses["200"].schema; - deepStrictEqual(response, { $ref: "#/definitions/Thing" }); - - deepStrictEqual(res.definitions, { - Thing: { - type: "object", - properties: { - inner: { - $ref: "#/definitions/Thing", - }, - }, - }, - }); -}); - -it("handles cycle in transformed model", async () => { - const res = await openApiFor( - ` - model Thing { - @visibility("delete") d?: string; - @visibility("query") q?: string; - inner?: Thing; - } - - @route("/") @get op get(...Thing): Thing; - ` - ); - - const request = res.paths["/"].get.parameters[0].schema; - deepStrictEqual(request, { $ref: "#/definitions/ThingQuery" }); - - const response = res.paths["/"].get.responses["200"].schema; - deepStrictEqual(response, { $ref: "#/definitions/Thing" }); - - deepStrictEqual(res.definitions, { - Thing: { - type: "object", - properties: { - inner: { $ref: "#/definitions/Thing" }, - }, - }, - ThingQuery: { - type: "object", - properties: { - q: { type: "string" }, - inner: { $ref: "#/definitions/ThingQuery" }, - }, - }, - }); -}); - -it("supports nested metadata and removes properties with @bodyIgnore", async () => { - const res = await openApiFor( - ` - model Pet { - @bodyIgnore headers: { - @header h1: string; - moreHeaders: { - @header h2: string; - } - }; - - @path - id: string; - name: string; - } - - @route("/pets") - @post op create(...Pet): Pet; - ` - ); - - deepStrictEqual(res.paths, { - "/pets/{id}": { - post: { - operationId: "Create", - parameters: [ - { - $ref: "#/parameters/Pet.id", - }, - { - name: "h1", - in: "header", - required: true, - type: "string", - }, - { - name: "h2", - in: "header", - required: true, - type: "string", - }, - { - name: "body", - in: "body", - schema: { $ref: "#/definitions/PetCreate" }, - required: true, - }, - ], - responses: { - "200": { - description: "The request has succeeded.", - headers: { - h1: { - type: "string", - }, - h2: { - type: "string", - }, - }, - schema: { - $ref: "#/definitions/Pet", - }, - }, - }, - }, - }, - }); - - deepStrictEqual(res.definitions, { - PetCreate: { - type: "object", - properties: { - name: { - type: "string", - }, - }, - required: ["name"], - }, - Pet: { - type: "object", - properties: { - id: { - type: "string", - }, - name: { - type: "string", - }, - }, - required: ["id", "name"], - }, - }); -}); - -describe("body required", () => { - it("implicit body is marked as required", async () => { - const res = await openApiFor( - ` - model Pet { - name: string; - age: int32; - } - op single(...Pet): void; - ` - ); - strictEqual(res.paths["/"].post.parameters[0].required, true); - }); - - it("explicit body is marked as required if property is required", async () => { - const res = await openApiFor( - ` - model Pet { - name: string; - age: int32; - } - op single(@body pet: Pet): void; - ` - ); - strictEqual(res.paths["/"].post.parameters[0].required, true); - }); - - it("explicit body is marked as optional if property is optional", async () => { - const res = await openApiFor( - ` - model Pet { - name: string; - age: int32; - } - op single(@body pet?: Pet): void; - ` - ); - strictEqual(res.paths["/"].post.parameters[0].required, false); - }); -}); From e43f99084664677f274ed415080fb72c6ad82461 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 10:02:05 -0700 Subject: [PATCH 10/12] fix --- packages/typespec-autorest/src/openapi.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index b34958938f..470b3da3e7 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -868,6 +868,13 @@ export async function getOpenAPIForService( visibility: Visibility.Read, ignoreMetadataAnnotations: false, }); + Object.assign( + header, + applyIntrinsicDecorators(prop, { + type: (header as any).type, + format: (header as any).format, + }) + ); delete header.in; delete header.name; delete header.required; @@ -1117,9 +1124,8 @@ export async function getOpenAPIForService( } } } else if (body.property) { - currentEndpoint.parameters.push( - getOpenAPI2BodyParameter(body.property, getJsonName(body.property), schema) - ); + const prop = body.property; + emitParameter(prop, () => getOpenAPI2BodyParameter(prop, getJsonName(prop), schema)); } else { currentEndpoint.parameters.push({ name: "body", From 6001013fa042899d8761e3a89ecf8b49b9df12ab Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 10:46:58 -0700 Subject: [PATCH 11/12] update core --- core | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core b/core index 08fc78126b..5522c8177e 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 08fc78126b7cc7278550ec5b977d145d6c11e36f +Subproject commit 5522c8177e96f82f60de36b88d382a4e2b7134fc From 8288ac2aadf7d3680133dca6ca9ea2c970ec9c8b Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 6 Aug 2024 12:19:30 -0700 Subject: [PATCH 12/12] Use main core and bump cadl-ranch to 0.35.0 --- core | 2 +- packages/e2e-tests/cadl-ranch-specs/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core b/core index 5522c8177e..e492ff7d7b 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 5522c8177e96f82f60de36b88d382a4e2b7134fc +Subproject commit e492ff7d7ba4aac89aac73e594d547d267d05b02 diff --git a/packages/e2e-tests/cadl-ranch-specs/package.json b/packages/e2e-tests/cadl-ranch-specs/package.json index 06e31b8344..d1f6fa6db3 100644 --- a/packages/e2e-tests/cadl-ranch-specs/package.json +++ b/packages/e2e-tests/cadl-ranch-specs/package.json @@ -1,7 +1,7 @@ { "name": "@azure-tools/typespec-e2e-cadl-ranch-specs", "dependencies": { - "@azure-tools/cadl-ranch-specs": "0.34.10" + "@azure-tools/cadl-ranch-specs": "0.35.0" }, "type": "module", "private": true