tsp-openapi3: support path-level parameters (#4708)

Fixes #4455 

Previously, the parameters defined as a child of a `path` were ignored.
Now they are added to the operations defined in each of the path's
methods.

The spec states that duplicates are not allowed in the final
operation-level parameter list, and the more specific (operation level)
replaces the less specific (path level) if there is a collision. A
combination of the location and name uniquely identify a parameter for
these purposes.

Example:
```yml
openapi: 3.0.0
info:
  title: (title)
  version: 0.0.0
tags: []
paths:
  /widgets/{id}:
    get:
      operationId: Widgets_read
      responses:
        "200":
          description: The request has succeeded.
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Widget"
    delete:
      operationId: Widgets_delete
      responses:
        "204":
          description: "There is no content to send for this request, but the headers may be useful. "
    parameters:
      - name: id
        in: path
        required: true
        schema:
          type: string
components:
  schemas:
    Widget:
      type: object
      required:
        - id
      properties:
        id:
          type: string

```

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>

Author: chrisradek

.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.

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;

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<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> {
@@ -70,6 +95,7 @@ function transformOperationParameter(
 
   return {
     name: printIdentifier(parameter.name),
+    in: parameter.in,
     doc: parameter.description,
     decorators: getParameterDecorators(parameter),
     isOptional: !parameter.required,

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");
+});