From 1831df91223045710d4fc1b8eca31df3afcaa708 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 26 Jul 2024 16:00:52 -0700 Subject: [PATCH 1/6] Refactor parameter handling in swagger emitter --- packages/typespec-autorest/src/openapi.ts | 194 +++++++++++++----- .../src/openapi2-document.ts | 16 +- .../typespec-autorest/test/parameters.test.ts | 4 +- 3 files changed, 161 insertions(+), 53 deletions(-) diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 6ff51569a0..cf2ac6f2f7 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -131,16 +131,20 @@ import { getExamples, getRef } from "./decorators.js"; import { sortWithJsonSchema } from "./json-schema-sorter/sorter.js"; import { createDiagnostic, reportDiagnostic } from "./lib.js"; import { + OpenAPI2BodyParameter, OpenAPI2Document, OpenAPI2FileSchema, OpenAPI2FormDataParameter, OpenAPI2HeaderDefinition, + OpenAPI2HeaderParameter, OpenAPI2OAuth2FlowType, OpenAPI2Operation, OpenAPI2Parameter, + OpenAPI2ParameterBase, OpenAPI2ParameterType, OpenAPI2PathItem, OpenAPI2PathParameter, + OpenAPI2QueryParameter, OpenAPI2Response, OpenAPI2Schema, OpenAPI2SchemaProperty, @@ -1208,7 +1212,7 @@ export async function getOpenAPIForService( type: Type, schemaContext: SchemaContext, paramName: string - ): Omit | undefined { + ): PrimitiveItems | undefined { if (isBytes(type)) { return { type: "file" }; } @@ -1240,66 +1244,164 @@ export async function getOpenAPIForService( } } - function getOpenAPI2Parameter( + function getOpenAPI2PArameterBase( param: ModelProperty, - kind: T, - schemaContext: SchemaContext, - name?: string, - bodySchema?: any - ): OpenAPI2Parameter & { in: T } { - const ph: any = { + name: string | undefined + ): OpenAPI2ParameterBase { + const base: OpenAPI2ParameterBase = { name: name ?? param.name, - in: kind, required: !param.optional, description: getDoc(program, param), }; - if (param.name !== ph.name) { - ph["x-ms-client-name"] = param.name; - } - if (param.defaultValue) { - ph.default = getDefaultValue(param.defaultValue); + if (param.name !== base.name) { + base["x-ms-client-name"] = param.name; } - if (ph.in === "body") { - compilerAssert(bodySchema, "bodySchema argument is required to populate body parameter"); - ph.schema = bodySchema; - } else if (ph.in === "formData") { - Object.assign(ph, getFormDataSchema(param.type, schemaContext, ph.name)); + attachExtensions(param, base); + + return base; + } + + function getOpenAPI2BodyParameter( + param: ModelProperty, + name?: string, + bodySchema?: any + ): OpenAPI2BodyParameter { + return { + in: "body", + ...getOpenAPI2PArameterBase(param, name), + schema: bodySchema, + }; + } + + function getOpenAPI2FormDataParameter( + param: ModelProperty, + schemaContext: SchemaContext, + name?: string + ): OpenAPI2FormDataParameter { + const base = getOpenAPI2PArameterBase(param, name); + return { + in: "formData", + ...base, + ...(getFormDataSchema(param.type, schemaContext, base.name) as any), + default: param.defaultValue && getDefaultValue(param.defaultValue), + }; + } + + function getSimpleParameterSchema( + param: ModelProperty, + schemaContext: SchemaContext, + name: string + ): Pick< + OpenAPI2QueryParameter | OpenAPI2HeaderParameter | OpenAPI2PathParameter, + "type" | "items" + > { + if (param.type.kind === "Model" && isArrayModelType(program, param.type)) { + const itemSchema = getSchemaForPrimitiveItems(param.type.indexer.value, schemaContext, name); + const schema = itemSchema && { + ...itemSchema, + }; + delete (schema as any).description; + return { type: "array", items: schema }; } else { - const collectionFormat = ( - kind === "query" - ? getQueryParamOptions(program, param) - : kind === "header" - ? getHeaderFieldOptions(program, param) - : undefined - )?.format; - if (collectionFormat === "multi" && !["query", "header", "formData"].includes(ph.in)) { - reportDiagnostic(program, { code: "invalid-multi-collection-format", target: param }); - } - if (collectionFormat) { - ph.collectionFormat = collectionFormat; - } - - if (param.type.kind === "Model" && isArrayModelType(program, param.type)) { - ph.type = "array"; - const schema = { - ...getSchemaForPrimitiveItems(param.type.indexer.value, schemaContext, ph.name), - }; - delete (schema as any).description; - ph.items = schema; - } else { - Object.assign(ph, getSchemaForPrimitiveItems(param.type, schemaContext, ph.name)); - } + return getSchemaForPrimitiveItems(param.type, schemaContext, name) as any; } + } - attachExtensions(param, ph); + function getOpenAPI2QueryParameter( + param: ModelProperty, + schemaContext: SchemaContext, + name?: string + ): OpenAPI2QueryParameter { + const base = getOpenAPI2PArameterBase(param, name); + let collectionFormat = getQueryParamOptions(program, param).format; + if (collectionFormat && !["csv", "ssv", "tsv", "pipes", "multi"].includes(collectionFormat)) { + collectionFormat = undefined; + } + return { + in: "query", + collectionFormat: collectionFormat as any, + default: param.defaultValue && getDefaultValue(param.defaultValue), + ...base, + ...getSimpleParameterSchema(param, schemaContext, base.name), + }; + } + + function getOpenAPI2PathParameter( + param: ModelProperty, + schemaContext: SchemaContext, + name?: string + ): OpenAPI2PathParameter { + const base = getOpenAPI2PArameterBase(param, name); + + return { + in: "path", + default: param.defaultValue && getDefaultValue(param.defaultValue), + ...base, + ...getSimpleParameterSchema(param, schemaContext, base.name), + }; + } + function getOpenAPI2HeaderParameter( + param: ModelProperty, + schemaContext: SchemaContext, + name?: string + ): OpenAPI2HeaderParameter { + const base = getOpenAPI2PArameterBase(param, name); + let collectionFormat = getHeaderFieldOptions(program, param).format; + if (collectionFormat && !["csv", "ssv", "tsv", "pipes"].includes(collectionFormat)) { + collectionFormat = undefined; + } + return { + in: "header", + default: param.defaultValue && getDefaultValue(param.defaultValue), + ...base, + collectionFormat: collectionFormat as any, + ...getSimpleParameterSchema(param, schemaContext, base.name), + }; + } + + 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; + case "query": + return getOpenAPI2QueryParameter(param, schemaContext, name) as any; + case "path": + return getOpenAPI2PathParameter(param, schemaContext, name) as any; + case "header": + return getOpenAPI2HeaderParameter(param, schemaContext, name) as any; + default: + const _assertNever: never = kind; + compilerAssert(false, "Unreachable"); + } + } + + function getOpenAPI2Parameter( + param: ModelProperty, + kind: T, + schemaContext: SchemaContext, + name?: string, + bodySchema?: any + ): OpenAPI2Parameter & { in: T } { + const value = getOpenAPI2ParameterInternal(param, kind, schemaContext, name, bodySchema); // 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(ph, applyIntrinsicDecorators(param, { type: ph.type, format: ph.format })); - return ph; + Object.assign( + value, + applyIntrinsicDecorators(param, { type: (value as any).type, format: (value as any).format }) + ); + return value; } function populateParameter( diff --git a/packages/typespec-autorest/src/openapi2-document.ts b/packages/typespec-autorest/src/openapi2-document.ts index 1afef8fb0b..48dc35a582 100644 --- a/packages/typespec-autorest/src/openapi2-document.ts +++ b/packages/typespec-autorest/src/openapi2-document.ts @@ -301,10 +301,10 @@ export type OpenAPI2ParameterType = OpenAPI2Parameter["in"]; export interface OpenAPI2HeaderDefinition { type: "string" | "number" | "integer" | "boolean" | "array"; - items?: OpenAPI2Schema; collectionFormat?: "csv" | "ssv" | "tsv" | "pipes"; description?: string; format?: string; + items?: PrimitiveItems; } export type OpenAPI2Parameter = @@ -315,6 +315,10 @@ export type OpenAPI2Parameter = | OpenAPI2PathParameter; export interface OpenAPI2ParameterBase extends Extensions { + name: string; + description?: string; + required?: boolean; + /** * Provide a different name to be used in the client. */ @@ -323,11 +327,8 @@ export interface OpenAPI2ParameterBase extends Extensions { } export interface OpenAPI2BodyParameter extends OpenAPI2ParameterBase { - name: string; in: "body"; schema: OpenAPI2Schema; - description?: string; - required?: boolean; allowEmptyValue?: boolean; example?: unknown; @@ -338,6 +339,7 @@ export interface OpenAPI2HeaderParameter extends OpenAPI2HeaderDefinition, OpenA name: string; in: "header"; required?: boolean; + default?: unknown; } export interface OpenAPI2FormDataParameter extends OpenAPI2ParameterBase { @@ -359,7 +361,7 @@ export interface OpenAPI2FormDataParameter extends OpenAPI2ParameterBase { "x-ms-client-flatten"?: boolean; } -export interface PrimitiveItems extends OpenAPI2ParameterBase { +export interface PrimitiveItems { type: "string" | "number" | "integer" | "boolean" | "array" | "file"; format?: string; items?: PrimitiveItems; @@ -376,7 +378,9 @@ export interface OpenAPI2PathParameter extends OpenAPI2ParameterBase { required?: boolean; format?: string; enum?: string[]; + items?: PrimitiveItems; "x-ms-skip-url-encoding"?: boolean; + default?: unknown; } export interface OpenAPI2QueryParameter extends OpenAPI2ParameterBase { @@ -389,6 +393,8 @@ export interface OpenAPI2QueryParameter extends OpenAPI2ParameterBase { required?: boolean; format?: string; enum?: string[]; + items?: PrimitiveItems; + default?: unknown; } export type HttpMethod = "get" | "put" | "post" | "delete" | "options" | "head" | "patch" | "trace"; diff --git a/packages/typespec-autorest/test/parameters.test.ts b/packages/typespec-autorest/test/parameters.test.ts index 0d5e71dd34..6865ed6bea 100644 --- a/packages/typespec-autorest/test/parameters.test.ts +++ b/packages/typespec-autorest/test/parameters.test.ts @@ -355,11 +355,11 @@ describe("typespec-autorest: parameters", () => { }); it("array of enum is kept inline", async () => { - deepStrictEqual(await testParameter(`@${kind}({format: "multi"})`, "Foo[]"), { + deepStrictEqual(await testParameter(`@${kind}({format: "csv"})`, "Foo[]"), { in: kind, name: "arg1", required: true, - collectionFormat: "multi", + collectionFormat: "csv", type: "array", items: { type: "string", From b94e1372a9ed9e3ee8940e8d91c6006ca59c4430 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 26 Jul 2024 16:06:42 -0700 Subject: [PATCH 2/6] Fix --- ...ctor-autorest-parameter-resolution-2024-6-26-16-6-16.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .chronus/changes/refactor-autorest-parameter-resolution-2024-6-26-16-6-16.md diff --git a/.chronus/changes/refactor-autorest-parameter-resolution-2024-6-26-16-6-16.md b/.chronus/changes/refactor-autorest-parameter-resolution-2024-6-26-16-6-16.md new file mode 100644 index 0000000000..66cba2f484 --- /dev/null +++ b/.chronus/changes/refactor-autorest-parameter-resolution-2024-6-26-16-6-16.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-autorest" +--- + +Fix issue what allowed `multi` format on a header From 0705fcbbb899c34859f3cfd701ec3e110d8f6099 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 26 Jul 2024 16:22:14 -0700 Subject: [PATCH 3/6] missing diagnostic report --- packages/typespec-autorest/src/openapi.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index cf2ac6f2f7..56f44495ae 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -1317,6 +1317,7 @@ export async function getOpenAPIForService( let collectionFormat = getQueryParamOptions(program, param).format; if (collectionFormat && !["csv", "ssv", "tsv", "pipes", "multi"].includes(collectionFormat)) { collectionFormat = undefined; + reportDiagnostic(program, { code: "invalid-multi-collection-format", target: param }); } return { @@ -1351,6 +1352,7 @@ export async function getOpenAPIForService( let collectionFormat = getHeaderFieldOptions(program, param).format; if (collectionFormat && !["csv", "ssv", "tsv", "pipes"].includes(collectionFormat)) { collectionFormat = undefined; + reportDiagnostic(program, { code: "invalid-multi-collection-format", target: param }); } return { in: "header", From bdb2234ea60fcd3b3f81bf0b37710f808c65ffee Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 29 Jul 2024 08:52:31 -0700 Subject: [PATCH 4/6] Fix invalid spec --- .../core/optional/@azure-tools/typespec-autorest/openapi.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/samples/test/output/core/optional/@azure-tools/typespec-autorest/openapi.json b/packages/samples/test/output/core/optional/@azure-tools/typespec-autorest/openapi.json index 432b7830d6..8b2ee79c99 100644 --- a/packages/samples/test/output/core/optional/@azure-tools/typespec-autorest/openapi.json +++ b/packages/samples/test/output/core/optional/@azure-tools/typespec-autorest/openapi.json @@ -38,8 +38,7 @@ "schema": { "type": "integer", "format": "int64" - }, - "default": 123 + } } ], "responses": { From 1bc1878096286b99915598ab4cca1f3eb9917c20 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 30 Jul 2024 08:56:00 -0700 Subject: [PATCH 5/6] Update packages/typespec-autorest/src/openapi.ts Co-authored-by: Christopher Radek <14189820+chrisradek@users.noreply.github.com> --- packages/typespec-autorest/src/openapi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 56f44495ae..ec67df5817 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -1244,7 +1244,7 @@ export async function getOpenAPIForService( } } - function getOpenAPI2PArameterBase( + function getOpenAPI2ParameterBase( param: ModelProperty, name: string | undefined ): OpenAPI2ParameterBase { From cd2b712dd4d27aed98217ef31a2e6f1ad1994662 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 30 Jul 2024 08:59:04 -0700 Subject: [PATCH 6/6] Fix --- packages/typespec-autorest/src/openapi.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index ec67df5817..c66b4ef4c9 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -1269,7 +1269,7 @@ export async function getOpenAPIForService( ): OpenAPI2BodyParameter { return { in: "body", - ...getOpenAPI2PArameterBase(param, name), + ...getOpenAPI2ParameterBase(param, name), schema: bodySchema, }; } @@ -1279,7 +1279,7 @@ export async function getOpenAPIForService( schemaContext: SchemaContext, name?: string ): OpenAPI2FormDataParameter { - const base = getOpenAPI2PArameterBase(param, name); + const base = getOpenAPI2ParameterBase(param, name); return { in: "formData", ...base, @@ -1313,7 +1313,7 @@ export async function getOpenAPIForService( schemaContext: SchemaContext, name?: string ): OpenAPI2QueryParameter { - const base = getOpenAPI2PArameterBase(param, name); + const base = getOpenAPI2ParameterBase(param, name); let collectionFormat = getQueryParamOptions(program, param).format; if (collectionFormat && !["csv", "ssv", "tsv", "pipes", "multi"].includes(collectionFormat)) { collectionFormat = undefined; @@ -1334,7 +1334,7 @@ export async function getOpenAPIForService( schemaContext: SchemaContext, name?: string ): OpenAPI2PathParameter { - const base = getOpenAPI2PArameterBase(param, name); + const base = getOpenAPI2ParameterBase(param, name); return { in: "path", @@ -1348,7 +1348,7 @@ export async function getOpenAPIForService( schemaContext: SchemaContext, name?: string ): OpenAPI2HeaderParameter { - const base = getOpenAPI2PArameterBase(param, name); + const base = getOpenAPI2ParameterBase(param, name); let collectionFormat = getHeaderFieldOptions(program, param).format; if (collectionFormat && !["csv", "ssv", "tsv", "pipes"].includes(collectionFormat)) { collectionFormat = undefined;