tsp-openapi3 - dedupe common/operation params

microsoft · Oct 11, 2024 · e7bfcf1 · e7bfcf1
1 parent a02cc15
commit e7bfcf1
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 3 deletions.
diff --git 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
@@ -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<TypeSpecOperationParameter>[],
+): Refable<TypeSpecOperationParameter>[] {
+  const seen = new Set<string>();
+  const dedupeList: Refable<TypeSpecOperationParameter>[] = [];
+
+  // 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<OpenAPI3Parameter>,
 ): Refable<TypeSpecOperationParameter> {
@@ -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
@@ -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");
+});