diff --git a/src/commands/oas-sync.js b/src/commands/oas-sync.js index 4dcc0d0..9aea4b7 100644 --- a/src/commands/oas-sync.js +++ b/src/commands/oas-sync.js @@ -56,30 +56,104 @@ function generateOperationId(method, pathStr) { } /** - * Extract operations from an OAS spec. - * Returns a Map of operationId -> { summary, description, tag, path, operationId }. - * For operations without an operationId, a synthetic one is generated from the method and path. + * Identity key for an operation record (or an existing page's frontmatter), + * used everywhere operations/pages are looked up by operationId. `paths` and + * `webhooks` are separate namespaces in an OAS document, but both can have + * operationId omitted, so their synthetic `_` ids can + * legitimately collide (e.g. `POST /orders` and webhook `POST orders` both + * synthesize to `post_orders`) — the isWebhook flag disambiguates them so + * neither silently overwrites the other in an operationId-only Map. + */ +export function operationKey({ operationId, isWebhook }) { + return `${isWebhook ? 'webhook' : 'path'}:${operationId}`; +} + +/** + * Resolve a `paths`/`webhooks` entry that's a Reference Object (OAS 3.1, + * `{ $ref: '#/components/pathItems/Name' }`) against the spec's own + * `components.pathItems`, following chained refs (a pathItem that is itself + * a $ref to another) until a literal Path Item is reached. Only same-document + * refs in that exact form are supported; anything else (external files, + * other pointer shapes, an unresolvable name, or a cycle) is left unresolved + * and quietly skipped by the caller, same as before this existed. + */ +function resolveLocalPathItemRef(entry, spec) { + const seen = new Set(); + let current = entry; + // Sibling fields (e.g. an inline operation) alongside a $ref are explicitly + // allowed in an OAS 3.1 Path Item Object — accumulate them from every hop + // in the chain so they aren't discarded once $ref is followed. A field + // declared at an outer/earlier hop wins over the same field found deeper + // in the chain (OAS itself leaves this "undefined" when both define it). + let overrides = {}; + const finish = () => ({ ...current, ...overrides }); + + while (current && typeof current.$ref === 'string') { + const { $ref, ...siblings } = current; + overrides = { ...siblings, ...overrides }; + + if (seen.has(current.$ref)) return finish(); + seen.add(current.$ref); + + const match = current.$ref.match(/^#\/components\/pathItems\/(.+)$/); + if (!match) return finish(); + + let name; + try { + name = decodeURIComponent(match[1]).replace(/~1/g, '/').replace(/~0/g, '~'); + } catch { + // Malformed percent-escape — leave unresolved rather than throwing and + // aborting the whole sync/lint run over one bad $ref. + return finish(); + } + const resolved = spec.components?.pathItems?.[name]; + if (!resolved) return finish(); + + current = resolved; + } + + return finish(); +} + +/** + * Extract operations from an OAS spec's `paths`, plus its OAS 3.1 `webhooks` + * (callouts the API itself makes to a client-registered URL, not endpoints the + * API exposes — a separate top-level sibling of `paths` with the same + * Operation Object shape). The platform pages a webhook the same way it pages + * a path operation: a synthetic `post_` operationId when none is given, + * grouped by its own tag or, absent one, its own category keyed by its raw + * name — never merged with `paths` operations of the same name. + * Returns a Map keyed by `operationKey()` -> { summary, description, tag, + * path, operationId, isWebhook }. For operations without an operationId, a + * synthetic one is generated from the method and path (or webhook name). */ export function extractOperations(spec) { const ops = new Map(); - const paths = spec.paths || {}; - for (const [pathStr, methods] of Object.entries(paths)) { - for (const [method, operation] of Object.entries(methods)) { - if (!HTTP_METHODS.has(method)) continue; + function collect(entries, isWebhook) { + for (const [pathStr, rawItem] of Object.entries(entries)) { + const methods = resolveLocalPathItemRef(rawItem, spec); + + for (const [method, operation] of Object.entries(methods)) { + if (!HTTP_METHODS.has(method)) continue; - const operationId = operation.operationId || generateOperationId(method, pathStr); + const operationId = operation.operationId || generateOperationId(method, pathStr); - ops.set(operationId, { - operationId, - summary: operation.summary || null, - description: operation.description || null, - tag: (operation.tags && operation.tags[0]) || null, - path: pathStr, - }); + ops.set(operationKey({ operationId, isWebhook }), { + operationId, + summary: operation.summary || null, + description: operation.description || null, + tag: (operation.tags && operation.tags[0]) || null, + path: pathStr, + isWebhook, + }); + } } } + collect(spec.paths || {}, false); + collect(spec.webhooks || {}, true); + return ops; } @@ -190,11 +264,15 @@ function stringifyFrontmatter(frontmatter) { return matter.stringify('', frontmatter).replace(/\n+$/, ''); } -function buildPageContent({ oasFilename, operationId }) { +function buildPageContent({ oasFilename, operationId, isWebhook }) { const frontmatter = { api: { file: oasFilename, operationId, + // Marks the page as a webhook (the API calling out to the client) + // rather than a path operation (the client calling the API), matching + // what the platform stamps on a page generated from `webhooks`. + ...(isWebhook ? { webhook: true } : {}), }, // Mirror the platform's OAS-upload behavior: a newly added endpoint is // always written `hidden: false`, even when its tag and siblings are @@ -338,7 +416,10 @@ function syncOneOas(refDir, oasFilename, spec, takenSlugs) { const pagesByOpId = new Map(); for (const page of existingPages) { - pagesByOpId.set(page.data.api.operationId, page); + pagesByOpId.set( + operationKey({ operationId: page.data.api.operationId, isWebhook: !!page.data.api.webhook }), + page, + ); } const changes = { added: [], deleted: [], skipped: [] }; @@ -426,26 +507,26 @@ function syncOneOas(refDir, oasFilename, spec, takenSlugs) { // Adds: operation pages with no page yet. Title/excerpt are owned by the OAS // spec at render time, so generated pages carry only the api reference. Slugs // are lowercased to match the platform's OAS-upload output. - for (const [opId, op] of specOps) { - if (pagesByOpId.has(opId)) continue; + for (const [key, op] of specOps) { + if (pagesByOpId.has(key)) continue; const { folder } = operationGroup(op); const pageDir = path.join(refDir, infoTitle, folder); // Reference slugs share one flat namespace, so uniquify against every slug // already in reference/ — a collision (or the reserved `index` slug) gets a // numeric suffix rather than being skipped. - const slug = reserveSlug(takenSlugs, safeSegment(opId, 'operation').toLowerCase()); + const slug = reserveSlug(takenSlugs, safeSegment(op.operationId, 'operation').toLowerCase()); const pagePath = path.join(pageDir, `${slug}.md`); // Guard against a spec-crafted name escaping reference/, or a stale slug set // vs. disk. reserveSlug already prevents slug collisions. if (!isWithin(refDir, pagePath) || fs.existsSync(pagePath)) { - changes.skipped.push({ path: path.relative(refDir, pagePath), operationId: opId }); + changes.skipped.push({ path: path.relative(refDir, pagePath), operationId: op.operationId }); continue; } fs.mkdirSync(pageDir, { recursive: true }); - const content = buildPageContent({ oasFilename, operationId: opId }); + const content = buildPageContent({ oasFilename, operationId: op.operationId, isWebhook: op.isWebhook }); fs.writeFileSync(pagePath, content); addToOrder(path.join(pageDir, '_order.yaml'), slug); diff --git a/src/validators/oas-reference.js b/src/validators/oas-reference.js index 409450a..a95d4f2 100644 --- a/src/validators/oas-reference.js +++ b/src/validators/oas-reference.js @@ -1,7 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; import matter from 'gray-matter'; -import { findOasFiles, extractOperations, collectExistingPages, syncOas } from '../commands/oas-sync.js'; +import { findOasFiles, extractOperations, collectExistingPages, syncOas, operationKey } from '../commands/oas-sync.js'; export const name = 'oas-reference'; @@ -34,6 +34,7 @@ export function validateAll(files, gitRoot, { fix } = {}) { const oasFilename = data.api.file; const operationId = data.api.operationId; + const isWebhook = !!data.api.webhook; const oas = oasMap.get(oasFilename); // Check: OAS file doesn't exist. @@ -50,7 +51,7 @@ export function validateAll(files, gitRoot, { fix } = {}) { if (!operationId) continue; // Check: operationId doesn't exist in the spec. - if (!oas.ops.has(operationId)) { + if (!oas.ops.has(operationKey({ operationId, isWebhook }))) { results.push({ file: relPath, rule: name, @@ -67,15 +68,19 @@ export function validateAll(files, gitRoot, { fix } = {}) { const existingPages = collectExistingPages(refDir); for (const [oasFilename, { ops }] of oasMap) { const pagesForOas = existingPages.filter((p) => p.data.api.file === oasFilename); - const coveredOps = new Set(pagesForOas.map((p) => p.data.api.operationId)); - - for (const [opId] of ops) { - if (!coveredOps.has(opId)) { + const coveredOps = new Set( + pagesForOas.map((p) => + operationKey({ operationId: p.data.api.operationId, isWebhook: !!p.data.api.webhook }), + ), + ); + + for (const op of ops.values()) { + if (!coveredOps.has(operationKey(op))) { results.push({ file: `reference/${oasFilename}`, rule: name, severity: 'warning', - message: `Missing page: no reference page found for operation "${opId}"`, + message: `Missing page: no reference page found for operation "${op.operationId}"`, fixable: true, }); } diff --git a/test/oas-reference.test.js b/test/oas-reference.test.js index 9ac2991..704b24e 100644 --- a/test/oas-reference.test.js +++ b/test/oas-reference.test.js @@ -41,3 +41,53 @@ test('operation not found is still reported', () => { rmRepo(root); } }); + +test('a page for a spec webhook is not reported as "Operation not found"', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + paymentCompleted: { + post: { summary: 'Sent when a payment settles' }, + }, + }, + }); + const root = makeRepo({ + 'reference/payments.json': spec, + 'reference/Payments/paymentcompleted/post_paymentcompleted.md': + '---\napi:\n file: payments.json\n operationId: post_paymentcompleted\n webhook: true\nhidden: false\n---\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(!res.some((r) => r.message.includes('Operation not found'))); + } finally { + rmRepo(root); + } +}); + +test('a path page and a webhook page sharing an operationId are both recognized, neither flagged missing', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + paths: { + '/orders': { post: { operationId: 'sharedId' } }, + }, + webhooks: { + orderCreated: { post: { operationId: 'sharedId' } }, + }, + }); + const root = makeRepo({ + 'reference/payments.json': spec, + 'reference/Payments/orders/sharedid.md': + '---\napi:\n file: payments.json\n operationId: sharedId\nhidden: false\n---\n', + 'reference/Payments/ordercreated/sharedid.md': + '---\napi:\n file: payments.json\n operationId: sharedId\n webhook: true\nhidden: false\n---\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(!res.some((r) => r.message.includes('Operation not found'))); + assert.ok(!res.some((r) => r.message.includes('Missing page'))); + } finally { + rmRepo(root); + } +}); diff --git a/test/oas-sync.test.js b/test/oas-sync.test.js index fd0b0e4..7d94144 100644 --- a/test/oas-sync.test.js +++ b/test/oas-sync.test.js @@ -575,3 +575,263 @@ test('deleting a legacy operation page literally named index.md releases its fol rmRepo(root); } }); + +test('sync generates a page for a webhook, marked with api.webhook: true', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + paymentCompleted: { + post: { summary: 'Sent when a payment settles' }, + }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + syncOas(root); + const page = path.join(root, 'reference/Payments/paymentcompleted/post_paymentcompleted.md'); + assert.ok(fs.existsSync(page), 'expected a generated webhook page'); + const { data } = matter(fs.readFileSync(page, 'utf-8')); + assert.equal(data.api.file, 'payments.json'); + assert.equal(data.api.operationId, 'post_paymentcompleted'); + assert.equal(data.api.webhook, true); + + // The category page's title is the webhook's own name, not the folder. + const index = matter( + fs.readFileSync(path.join(root, 'reference/Payments/paymentcompleted/index.md'), 'utf-8'), + ).data; + assert.equal(index.title, 'paymentCompleted'); + } finally { + rmRepo(root); + } +}); + +test('a path operation is not stamped api.webhook', () => { + const root = makeRepo({ 'reference/pets.json': SPEC }); + try { + syncOas(root); + const { data } = matter( + fs.readFileSync(path.join(root, 'reference/Pets/pets/listpets.md'), 'utf-8'), + ); + assert.equal('webhook' in data.api, false); + } finally { + rmRepo(root); + } +}); + +test('sync no longer deletes an existing webhook page on every run', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + paymentCompleted: { + post: { summary: 'Sent when a payment settles' }, + }, + }, + }); + const root = makeRepo({ + 'reference/payments.json': spec, + 'reference/Payments/paymentcompleted/post_paymentcompleted.md': + '---\napi:\n file: payments.json\n operationId: post_paymentcompleted\n webhook: true\nhidden: false\n---\n', + }); + try { + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + assert.ok( + fs.existsSync(path.join(root, 'reference/Payments/paymentcompleted/post_paymentcompleted.md')), + ); + } finally { + rmRepo(root); + } +}); + +test('an untagged webhook and an untagged path operation with the same sanitized name both get pages', () => { + // "/orders" and webhook "orders" sanitize to the same untagged group + // ("orders"), same as two untagged paths would; each still gets its own + // distinct operation page (they have different operationIds). + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + paths: { + '/orders': { get: { operationId: 'listOrders' } }, + }, + webhooks: { + orders: { + post: { summary: 'Sent when an order changes' }, + }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + syncOas(root); + const refDir = path.join(root, 'reference/Payments'); + assert.ok(fs.existsSync(path.join(refDir, 'orders/listorders.md'))); + assert.ok(fs.existsSync(path.join(refDir, 'orders/post_orders.md'))); + } finally { + rmRepo(root); + } +}); + +test('a path and a webhook whose synthesized operationIds collide both still get pages', () => { + // Neither declares an operationId, both are POST, and both sanitize to + // the same synthetic id: post_orders. + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + paths: { + '/orders': { post: { summary: 'Create an order' } }, + }, + webhooks: { + orders: { post: { summary: 'Sent when an order changes' } }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + const [result] = syncOas(root); + // Both pages generated — neither silently dropped by an internal Map + // collision keyed only on the (identical) synthesized operationId. + const added = result.changes.added.filter((p) => !p.endsWith('index.md')); + assert.equal(added.length, 2, `expected 2 pages, got: ${JSON.stringify(added)}`); + + const refDir = path.join(root, 'reference/Payments/orders'); + const pathPage = matter(fs.readFileSync(path.join(refDir, 'post_orders.md'), 'utf-8')).data; + const webhookPage = matter( + fs.readFileSync(path.join(refDir, 'post_orders-1.md'), 'utf-8'), + ).data; + assert.equal('webhook' in pathPage.api, false); + assert.equal(webhookPage.api.webhook, true); + } finally { + rmRepo(root); + } +}); + +test('a webhook that is a $ref to components.pathItems is resolved', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + paymentCompleted: { $ref: '#/components/pathItems/PaymentCompleted' }, + }, + components: { + pathItems: { + PaymentCompleted: { + post: { operationId: 'onPaymentCompleted', summary: 'Sent when a payment settles' }, + }, + }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + syncOas(root); + const page = path.join(root, 'reference/Payments/paymentcompleted/onpaymentcompleted.md'); + assert.ok(fs.existsSync(page), 'expected the $ref-resolved webhook to generate a page'); + const { data } = matter(fs.readFileSync(page, 'utf-8')); + assert.equal(data.api.operationId, 'onPaymentCompleted'); + assert.equal(data.api.webhook, true); + } finally { + rmRepo(root); + } +}); + +test('a $ref to a pathItem that is itself a $ref is followed to the literal Path Item', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + // Chained: paymentCompleted -> Alias -> the literal Path Item. + paymentCompleted: { $ref: '#/components/pathItems/Alias' }, + }, + components: { + pathItems: { + Alias: { $ref: '#/components/pathItems/PaymentCompleted' }, + PaymentCompleted: { + post: { operationId: 'onPaymentCompleted', summary: 'Sent when a payment settles' }, + }, + }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + syncOas(root); + const page = path.join(root, 'reference/Payments/paymentcompleted/onpaymentcompleted.md'); + assert.ok(fs.existsSync(page), 'expected the chained $ref to be followed to the literal Path Item'); + assert.equal(matter(fs.readFileSync(page, 'utf-8')).data.api.operationId, 'onPaymentCompleted'); + } finally { + rmRepo(root); + } +}); + +test('a circular pathItem $ref is left unresolved rather than looping forever', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + paymentCompleted: { $ref: '#/components/pathItems/A' }, + }, + components: { + pathItems: { + A: { $ref: '#/components/pathItems/B' }, + B: { $ref: '#/components/pathItems/A' }, + }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + // Must return (not hang) and simply generate nothing for the cycle. + const [result] = syncOas(root); + assert.equal(result.changes.added.filter((p) => !p.endsWith('index.md')).length, 0); + } finally { + rmRepo(root); + } +}); + +test('a pathItem $ref with a malformed percent-escape is left unresolved rather than throwing', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + // "%zz" is not a valid percent-escape — decodeURIComponent throws on it. + paymentCompleted: { $ref: '#/components/pathItems/%zz' }, + }, + components: { pathItems: {} }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + // Must not throw; the malformed ref is simply left unresolved. + const [result] = syncOas(root); + assert.equal(result.changes.added.filter((p) => !p.endsWith('index.md')).length, 0); + } finally { + rmRepo(root); + } +}); + +test('an inline operation alongside a $ref sibling is not discarded', () => { + // OAS 3.1 explicitly permits sibling fields (like an inline operation) + // alongside $ref in a Path Item Object. + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Payments' }, + webhooks: { + paymentCompleted: { + $ref: '#/components/pathItems/Base', + // Inline sibling operation, alongside the $ref. + put: { operationId: 'inlineUpdate', summary: 'Inline sibling op' }, + }, + }, + components: { + pathItems: { + Base: { post: { operationId: 'onPaymentCompleted', summary: 'From the referenced pathItem' } }, + }, + }, + }); + const root = makeRepo({ 'reference/payments.json': spec }); + try { + syncOas(root); + const refDir = path.join(root, 'reference/Payments/paymentcompleted'); + // Both the referenced pathItem's operation and the inline sibling exist. + assert.ok(fs.existsSync(path.join(refDir, 'onpaymentcompleted.md')), 'expected the referenced operation'); + assert.ok(fs.existsSync(path.join(refDir, 'inlineupdate.md')), 'expected the inline sibling operation'); + } finally { + rmRepo(root); + } +});