diff --git a/.chronus/changes/tsp-openapi3-common-params-2024-9-11-14-26-2.md b/.chronus/changes/tsp-openapi3-common-params-2024-9-11-14-26-2.md new file mode 100644 index 0000000000..1bd62f1866 --- /dev/null +++ b/.chronus/changes/tsp-openapi3-common-params-2024-9-11-14-26-2.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/openapi3" +--- + +Updates tsp-openapi3 to include path-level parameters in generated typespec operations. \ No newline at end of file diff --git a/packages/openapi3/src/cli/actions/convert/interfaces.ts b/packages/openapi3/src/cli/actions/convert/interfaces.ts index eeb04eb2c3..c4e6897cea 100644 --- a/packages/openapi3/src/cli/actions/convert/interfaces.ts +++ b/packages/openapi3/src/cli/actions/convert/interfaces.ts @@ -112,6 +112,7 @@ export interface TypeSpecOperation extends TypeSpecDeclaration { export interface TypeSpecOperationParameter { name: string; + in: string; doc?: string; decorators: TypeSpecDecorator[]; isOptional: boolean; diff --git a/packages/openapi3/src/cli/actions/convert/transforms/transform-paths.ts b/packages/openapi3/src/cli/actions/convert/transforms/transform-paths.ts index a6480c1054..e616aef113 100644 --- a/packages/openapi3/src/cli/actions/convert/transforms/transform-paths.ts +++ b/packages/openapi3/src/cli/actions/convert/transforms/transform-paths.ts @@ -29,6 +29,7 @@ export function transformPaths( const operations: TypeSpecOperation[] = []; for (const route of Object.keys(paths)) { + const routeParameters = paths[route].parameters?.map(transformOperationParameter) ?? []; const path = paths[route]; for (const verb of supportedHttpMethods) { const operation = path[verb]; @@ -48,7 +49,7 @@ export function transformPaths( { name: "route", args: [route] }, { name: verb, args: [] }, ], - parameters, + parameters: dedupeParameters([...routeParameters, ...parameters]), doc: operation.description, operationId: operation.operationId, requestBodies: transformRequestBodies(operation.requestBody), @@ -61,6 +62,30 @@ export function transformPaths( return operations; } +function dedupeParameters( + parameters: Refable[], +): Refable[] { + const seen = new Set(); + const dedupeList: Refable[] = []; + + // iterate in reverse since more specific-scoped parameters are added last + for (let i = parameters.length - 1; i >= 0; i--) { + // ignore resolving the $ref for now, unlikely to be able to resolve + // issues without user intervention if a duplicate is present except in + // very simple cases. + const param = parameters[i]; + + const identifier = "$ref" in param ? param.$ref : `${param.in}.${param.name}`; + + if (seen.has(identifier)) continue; + seen.add(identifier); + + dedupeList.unshift(param); + } + + return dedupeList; +} + function transformOperationParameter( parameter: Refable, ): Refable { @@ -70,6 +95,7 @@ function transformOperationParameter( return { name: printIdentifier(parameter.name), + in: parameter.in, doc: parameter.description, decorators: getParameterDecorators(parameter), isOptional: !parameter.required, diff --git a/packages/openapi3/test/tsp-openapi3/paths.test.ts b/packages/openapi3/test/tsp-openapi3/paths.test.ts new file mode 100644 index 0000000000..667d0ce2a9 --- /dev/null +++ b/packages/openapi3/test/tsp-openapi3/paths.test.ts @@ -0,0 +1,269 @@ +import assert from "assert"; +import { expect, it } from "vitest"; +import { OpenAPI3Response } from "../../src/types.js"; +import { expectDecorators } from "./utils/expect.js"; +import { tspForOpenAPI3 } from "./utils/tsp-for-openapi3.js"; + +const response: OpenAPI3Response = { + description: "test response", + content: { + "application/json": { + schema: { + type: "object", + properties: { + message: { type: "string" }, + }, + }, + }, + }, +}; + +it("generates operations with no params", async () => { + const serviceNamespace = await tspForOpenAPI3({ + paths: { + "/": { + get: { + operationId: "rootGet", + parameters: [], + responses: { + "200": response, + }, + }, + }, + }, + }); + + const operations = serviceNamespace.operations; + + expect(operations.size).toBe(1); + + /* @route("/") @get op rootGet(): rootGet200ApplicationJsonResponse; */ + const rootGet = operations.get("rootGet"); + assert(rootGet, "rootGet operation not found"); + + /* @get @route("/") */ + expectDecorators(rootGet.decorators, [{ name: "get" }, { name: "route", args: ["/"] }]); + // verify no operation parameters + expect(rootGet.parameters.properties.size).toBe(0); + assert(rootGet.returnType.kind === "Model", "Expected model return type"); + expect(rootGet.returnType.name).toBe("rootGet200ApplicationJsonResponse"); +}); + +it("generates operations without common params", async () => { + const serviceNamespace = await tspForOpenAPI3({ + paths: { + "/{id}": { + get: { + operationId: "idGet", + parameters: [{ name: "id", in: "path", required: true, schema: { type: "string" } }], + responses: { + "200": response, + }, + }, + }, + }, + }); + + const operations = serviceNamespace.operations; + + expect(operations.size).toBe(1); + + /* @route("/{id}") @get op idGet(@path id: string): idGet200ApplicationJsonResponse; */ + const idGet = operations.get("idGet"); + assert(idGet, "idGet operation not found"); + + /* @get @route("/{id}") */ + expectDecorators(idGet.decorators, [{ name: "get" }, { name: "route", args: ["/{id}"] }]); + + expect(idGet.parameters.properties.size).toBe(1); + const idParam = idGet.parameters.properties.get("id")!; + expect(idParam).toMatchObject({ + optional: false, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(idParam.decorators, [{ name: "path" }]); + + assert(idGet.returnType.kind === "Model", "Expected model return type"); + expect(idGet.returnType.name).toBe("idGet200ApplicationJsonResponse"); +}); + +it("generates operations with common params", async () => { + const serviceNamespace = await tspForOpenAPI3({ + paths: { + "/{id}": { + parameters: [{ name: "id", in: "path", required: true, schema: { type: "string" } }], + get: { + operationId: "idGet", + parameters: [], + responses: { + "200": response, + }, + }, + }, + }, + }); + + const operations = serviceNamespace.operations; + + expect(operations.size).toBe(1); + + /* @route("/{id}") @get op idGet(@path id: string): idGet200ApplicationJsonResponse; */ + const idGet = operations.get("idGet"); + assert(idGet, "idGet operation not found"); + + /* @get @route("/{id}") */ + expectDecorators(idGet.decorators, [{ name: "get" }, { name: "route", args: ["/{id}"] }]); + + expect(idGet.parameters.properties.size).toBe(1); + const idParam = idGet.parameters.properties.get("id")!; + expect(idParam).toMatchObject({ + optional: false, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(idParam.decorators, [{ name: "path" }]); + + assert(idGet.returnType.kind === "Model", "Expected model return type"); + expect(idGet.returnType.name).toBe("idGet200ApplicationJsonResponse"); +}); + +it("generates operations with common and specific params", async () => { + const serviceNamespace = await tspForOpenAPI3({ + paths: { + "/{id}": { + parameters: [{ name: "id", in: "path", required: true, schema: { type: "string" } }], + get: { + operationId: "idGet", + parameters: [{ name: "foo", in: "query", schema: { type: "string" } }], + responses: { + "200": response, + }, + }, + }, + }, + }); + + const operations = serviceNamespace.operations; + + expect(operations.size).toBe(1); + + /* @route("/{id}") @get op idGet(@path id: string, @query foo?: string): idGet200ApplicationJsonResponse; */ + const idGet = operations.get("idGet"); + assert(idGet, "idGet operation not found"); + + /* @get @route("/{id}") */ + expectDecorators(idGet.decorators, [{ name: "get" }, { name: "route", args: ["/{id}"] }]); + + /* (@path id: string, @query foo?: string) */ + expect(idGet.parameters.properties.size).toBe(2); + const idParam = idGet.parameters.properties.get("id")!; + expect(idParam).toMatchObject({ + optional: false, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(idParam.decorators, { name: "path" }); + + const fooParam = idGet.parameters.properties.get("foo")!; + expect(fooParam).toMatchObject({ + optional: true, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(fooParam.decorators, { name: "query" }); + + assert(idGet.returnType.kind === "Model", "Expected model return type"); + expect(idGet.returnType.name).toBe("idGet200ApplicationJsonResponse"); +}); + +it("supports overriding common params with operation params", async () => { + const serviceNamespace = await tspForOpenAPI3({ + paths: { + "/{id}": { + parameters: [ + { name: "id", in: "path", required: true, schema: { type: "string" } }, + { name: "x-header", in: "header", required: false, schema: { type: "string" } }, + ], + get: { + operationId: "idGet", + parameters: [ + { name: "foo", in: "query", schema: { type: "string" } }, + { name: "x-header", in: "header", required: true, schema: { type: "string" } }, + ], + responses: { + "200": response, + }, + }, + put: { + operationId: "idPut", + parameters: [], + responses: { + "200": response, + }, + }, + }, + }, + }); + + const operations = serviceNamespace.operations; + + expect(operations.size).toBe(2); + + // `idGet` overrides the common `x-header` parameter with it's own, making it required + /* @route("/{id}") @get op idGet(@path id: string, @query foo?: string, @header `x-header`: string): idGet200ApplicationJsonResponse; */ + const idGet = operations.get("idGet"); + assert(idGet, "idGet operation not found"); + + /* @get @route("/{id}") */ + expectDecorators(idGet.decorators, [{ name: "get" }, { name: "route", args: ["/{id}"] }]); + + /* (@path id: string, @query foo?: string, @header `x-header`: string) */ + expect(idGet.parameters.properties.size).toBe(3); + const idParam = idGet.parameters.properties.get("id")!; + expect(idParam).toMatchObject({ + optional: false, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(idParam.decorators, { name: "path" }); + + const fooParam = idGet.parameters.properties.get("foo")!; + expect(fooParam).toMatchObject({ + optional: true, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(fooParam.decorators, { name: "query" }); + + const xHeaderParam = idGet.parameters.properties.get("x-header")!; + expect(xHeaderParam).toMatchObject({ + optional: false, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(xHeaderParam.decorators, { name: "header" }); + + assert(idGet.returnType.kind === "Model", "Expected model return type"); + expect(idGet.returnType.name).toBe("idGet200ApplicationJsonResponse"); + + // `idPut` uses the common `x-header` parameter, which is marked optional + /* @route("/{id}") @put op idPut(@path id: string, @header `x-header`: string): idPut200ApplicationJsonResponse; */ + const idPut = operations.get("idPut"); + assert(idPut, "idPut operation not found"); + + /* @put @route("/{id}") */ + expectDecorators(idPut.decorators, [{ name: "put" }, { name: "route", args: ["/{id}"] }]); + + /* (@path id: string, @header `x-header`?: string) */ + expect(idPut.parameters.properties.size).toBe(2); + const idPutParam = idPut.parameters.properties.get("id")!; + expect(idPutParam).toMatchObject({ + optional: false, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(idPutParam.decorators, [{ name: "path" }]); + + const xHeaderSharedParam = idPut.parameters.properties.get("x-header")!; + expect(xHeaderSharedParam).toMatchObject({ + optional: true, + type: { kind: "Scalar", name: "string" }, + }); + expectDecorators(xHeaderSharedParam.decorators, { name: "header" }); + + assert(idPut.returnType.kind === "Model", "Expected model return type"); + expect(idPut.returnType.name).toBe("idPut200ApplicationJsonResponse"); +}); diff --git a/packages/openapi3/test/tsp-openapi3/utils/expect.ts b/packages/openapi3/test/tsp-openapi3/utils/expect.ts new file mode 100644 index 0000000000..55df73a60f --- /dev/null +++ b/packages/openapi3/test/tsp-openapi3/utils/expect.ts @@ -0,0 +1,57 @@ +import { DecoratorApplication, isType, Numeric } from "@typespec/compiler"; +import { expect } from "vitest"; + +export interface DecoratorMatch { + /** + * The name of the decorator without the "@" prefix. + */ + name: string; + + /** + * The arguments passed into the decorator. + */ + args?: any[]; +} + +export interface ExpectDecoratorsOptions { + strict?: boolean; +} + +export function expectDecorators( + decorators: DecoratorApplication[], + matches: DecoratorMatch | DecoratorMatch[], + options: ExpectDecoratorsOptions = { strict: true }, +) { + const expectations = Array.isArray(matches) ? matches : [matches]; + + if (options.strict) { + expect(decorators).toHaveLength(expectations.length); + } + + for (let i = 0; i < expectations.length; i++) { + const decorator = decorators[i]; + const expectation = expectations[i]; + + if (expectation.name) { + expect(decorator.definition?.name).toBe(`@${expectation.name}`); + } + + if (expectation.args) { + const args = expectation.args.map(transformDecoratorArg); + expect(decorator.args).toMatchObject(args); + } + } +} + +function transformDecoratorArg(arg: any) { + if (isType(arg)) return arg; + + if (typeof arg === "string") { + return { jsValue: arg }; + } + if (typeof arg === "number") { + return { jsValue: Numeric(`${arg}`) }; + } + + return arg; +} diff --git a/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts b/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts index 533fdfa1b9..fe8031062f 100644 --- a/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts +++ b/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts @@ -7,6 +7,7 @@ import { OpenAPI3TestLibrary } from "../../../src/testing/index.js"; import { OpenAPI3Document, OpenAPI3Parameter, + OpenAPI3PathItem, OpenAPI3Schema, Refable, } from "../../../src/types.js"; @@ -20,16 +21,17 @@ function wrapCodeInTest(code: string): string { export interface OpenAPI3Options { schemas?: Record>; parameters?: Record>; + paths?: Record; } -export async function tspForOpenAPI3({ parameters, schemas }: OpenAPI3Options) { +export async function tspForOpenAPI3({ parameters, paths, schemas }: OpenAPI3Options) { const openApi3Doc: OpenAPI3Document = { info: { title: "Test Service", version: "1.0.0", }, openapi: "3.0.0", - paths: {}, + paths: { ...paths }, components: { schemas: { ...(schemas as any),