From e7bfcf128abbc8bce8167bf7f65e792a0b5bdce6 Mon Sep 17 00:00:00 2001 From: Christopher Radek Date: Fri, 11 Oct 2024 16:13:14 -0700 Subject: [PATCH] tsp-openapi3 - dedupe common/operation params --- .../src/cli/actions/convert/interfaces.ts | 1 + .../convert/transforms/transform-paths.ts | 27 ++++- .../openapi3/test/tsp-openapi3/paths.test.ts | 99 ++++++++++++++++++- 3 files changed, 124 insertions(+), 3 deletions(-) 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 26e3534cc4..e616aef113 100644 --- a/packages/openapi3/src/cli/actions/convert/transforms/transform-paths.ts +++ b/packages/openapi3/src/cli/actions/convert/transforms/transform-paths.ts @@ -49,7 +49,7 @@ export function transformPaths( { name: "route", args: [route] }, { name: verb, args: [] }, ], - parameters: [...routeParameters, ...parameters], + parameters: dedupeParameters([...routeParameters, ...parameters]), doc: operation.description, operationId: operation.operationId, requestBodies: transformRequestBodies(operation.requestBody), @@ -62,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 { @@ -71,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 index bb41304785..667d0ce2a9 100644 --- a/packages/openapi3/test/tsp-openapi3/paths.test.ts +++ b/packages/openapi3/test/tsp-openapi3/paths.test.ts @@ -160,15 +160,110 @@ it("generates operations with common and specific params", async () => { optional: false, type: { kind: "Scalar", name: "string" }, }); - expectDecorators(idParam.decorators, [{ name: "path" }]); + 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" }]); + 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"); +});