From e3f880647b65acf5746a2418afc2d3f9c46cbe98 Mon Sep 17 00:00:00 2001 From: Drew Powers Date: Wed, 26 Apr 2023 13:12:59 -0600 Subject: [PATCH] Fix missing parameters in operation object --- CONTRIBUTING.md | 10 +-- src/transform/path-item-object.ts | 23 +++--- src/types.ts | 2 +- test/index.test.ts | 112 ++++++++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6c033a007..c2a5343a7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,15 +40,17 @@ This will compile the code as you change automatically. **Please fill out the template!** It’s a very lightweight template 🙂. -### TDD +### Use Test-driven Development! -This library is a great usecase for [test-driven development (TDD)](https://en.wikipedia.org/wiki/Test-driven_development). If you’re new to this, the basic workflow is: +Contributing to this library is hard-bordering-on-impossible without a [test-driven development (TDD)](https://en.wikipedia.org/wiki/Test-driven_development) strategy. If you’re new to this, the basic workflow is: 1. First, write a [test](#testing) that fully outlines what you’d _like_ the output to be. -2. Make sure this test **fails** when you run `npm test` (yes, _fails!_) +2. Next, make sure this test **fails** when you run `npm test` (yes, _fails!_) 3. Then, make changes to `src/` until the tests pass. -_Code generation is hard!_ And for that reason, starting with a very clear expectation of your end-goal can make working easier. +Reasoning about code generation can be quite difficult until you “invert your thinking” and approach it output-first. Adopting TDD can turn very unclear/abstract problems into concrete ones with clear steps to resolution. + +✨ When starting any task, **write a failing test first!** ✨ #### Updating snapshot tests diff --git a/src/transform/path-item-object.ts b/src/transform/path-item-object.ts index be1a719b3..1f5915c86 100644 --- a/src/transform/path-item-object.ts +++ b/src/transform/path-item-object.ts @@ -24,25 +24,32 @@ export default function transformPathItemObject( if (!operationObject) continue; const c = getSchemaObjectComment(operationObject, indentLv); if (c) output.push(indent(c, indentLv)); + + // fold top-level PathItem parameters into method-level, with the latter overriding the former + const keyedParameters: Record = {}; + if (!("$ref" in operationObject)) { + // important: OperationObject parameters come last, and will override any conflicts with PathItem parameters + for (const parameter of [...(pathItem.parameters ?? []), ...(operationObject.parameters ?? [])]) { + // note: the actual key doesn’t matter here, as long as it can match between PathItem and OperationObject + keyedParameters["$ref" in parameter ? parameter.$ref : parameter.name] = parameter; + } + } + if ("$ref" in operationObject) { output.push(indent(`${method}: ${operationObject.$ref}`, indentLv)); } // if operationId exists, move into an `operations` export and pass the reference in here else if (operationObject.operationId) { - const operationType = transformOperationObject(operationObject, { path, ctx: { ...ctx, indentLv: 1 } }); + const operationType = transformOperationObject( + { ...operationObject, parameters: Object.values(keyedParameters) }, + { path, ctx: { ...ctx, indentLv: 1 } } + ); ctx.operations[operationObject.operationId] = { operationType, comment: getSchemaObjectComment(operationObject, 1), }; output.push(indent(`${method}: operations[${escStr(operationObject.operationId)}];`, indentLv)); } else { - // fold top-level PathItem parameters into method-level, with the latter overriding the former - const keyedParameters: Record = {}; - // important: OperationObject parameters come last, and will override any conflicts with PathItem parameters - for (const parameter of [...(pathItem.parameters ?? []), ...(operationObject.parameters ?? [])]) { - // note: the actual key doesn’t matter here, as long as it can match between PathItem and OperationObject - keyedParameters["$ref" in parameter ? parameter.$ref : parameter.name] = parameter; - } const operationType = transformOperationObject( { ...operationObject, parameters: Object.values(keyedParameters) }, { path, ctx: { ...ctx, indentLv } } diff --git a/src/types.ts b/src/types.ts index fda2c4c93..a7b582fd1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -276,7 +276,7 @@ export interface RequestBodyObject extends Extensable { */ export interface MediaTypeObject extends Extensable { /** The schema defining the content of the request, response, or parameter. */ - schema?: SchemaObject; + schema?: SchemaObject | ReferenceObject; /** Example of the media type. The example object SHOULD be in the correct format as specified by the media type. The example field is mutually exclusive of the examples field. Furthermore, if referencing a schema which contains an example, the example value SHALL override the example provided by the schema. */ example?: any; /** Examples of the media type. Each example object SHOULD match the media type and specified schema if present. The examples field is mutually exclusive of the example field. Furthermore, if referencing a schema which contains an example, the examples value SHALL override the example provided by the schema. */ diff --git a/test/index.test.ts b/test/index.test.ts index 9319fa232..ed13374a3 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -33,6 +33,7 @@ describe("openapiTS", () => { }); describe("3.0", () => { + /** test that custom $refs are ignored rather than throw errors */ test("custom properties", async () => { const generated = await openapiTS({ openapi: "3.0", @@ -74,6 +75,7 @@ export type operations = Record; `); }); + /** test that component examples aren’t parsed (they may be invalid / pseudocode) */ test("components.examples are skipped", async () => { const generated = await openapiTS({ openapi: "3.0", @@ -124,6 +126,7 @@ export type operations = Record; `); }); + /** test that $ref’d parameters make it into the paths object correctly */ test("parameter $refs", async () => { const generated = await openapiTS(new URL("./fixtures/parameters-test.yaml", import.meta.url)); expect(generated).toBe(`${BOILERPLATE} @@ -177,6 +180,115 @@ export type operations = Record; `); }); + /** test that $ref’d parameters, operation parameters, and method parameters all make it into the operation object correctly */ + test("operations keep all parameters", async () => { + const schema: OpenAPI3 = { + openapi: "3.0", + info: { title: "Test", version: "1.0" }, + paths: { + "/post/{id}": { + get: { + operationId: "getPost", + parameters: [ + { name: "format", in: "query", schema: { type: "string" } }, + { $ref: "#/components/parameters/post_id" }, + ], + responses: { + 200: { + description: "OK", + content: { + "application/json": { schema: { $ref: "#/components/schemas/Post" } }, + }, + }, + }, + }, + parameters: [{ name: "revision", in: "query", schema: { type: "number" } }], + }, + }, + components: { + schemas: { + Post: { + type: "object", + properties: { + id: { type: "number" }, + title: { type: "string" }, + body: { type: "string" }, + published_at: { type: "number" }, + }, + required: ["id", "title", "body"], + }, + }, + parameters: { + post_id: { + name: "post_id", + in: "path", + schema: { type: "string" }, + required: true, + }, + }, + }, + }; + const generated = await openapiTS(schema); + expect(generated).toBe(`${BOILERPLATE} +export interface paths { + "/post/{id}": { + get: operations["getPost"]; + parameters: { + query: { + revision?: number; + }; + }; + }; +} + +export type webhooks = Record; + +export interface components { + schemas: { + Post: { + id: number; + title: string; + body: string; + published_at?: number; + }; + }; + responses: never; + parameters: { + post_id: string; + }; + requestBodies: never; + headers: never; + pathItems: never; +} + +export type external = Record; + +export interface operations { + + getPost: { + parameters: { + query: { + revision?: number; + format?: string; + }; + path: { + post_id: components["parameters"]["post_id"]; + }; + }; + responses: { + /** @description OK */ + 200: { + content: { + "application/json": components["schemas"]["Post"]; + }; + }; + }; + }; +} +`); + }); + + /** test that remote $refs are loaded correctly */ test("remote $refs", async () => { const generated = await openapiTS(new URL("./fixtures/remote-ref-test.yaml", import.meta.url)); expect(generated).toBe(`${BOILERPLATE}