diff --git a/src/commands/oas-sync.js b/src/commands/oas-sync.js index b551960..90ff2b1 100644 --- a/src/commands/oas-sync.js +++ b/src/commands/oas-sync.js @@ -55,19 +55,111 @@ function generateOperationId(method, pathStr) { return `${method.toLowerCase()}_${sanitized}`; } +/** + * JSON Pointer (`#/components/pathItems/Pets`, `#/paths/~1pets`) used by + * OAS path-item and operation `$ref`s. `~1` / `~0` are the spec's escapes + * for `/` and `~`. + */ +function decodeJsonPointerToken(token) { + return token.replace(/~1/g, '/').replace(/~0/g, '~'); +} + +function resolvePointer(root, pointer) { + if (pointer === '#' || pointer === '') return root; + if (typeof pointer !== 'string' || !pointer.startsWith('#/')) return undefined; + const parts = pointer.slice(2).split('/').map(decodeJsonPointerToken); + let current = root; + for (const part of parts) { + if (current == null || typeof current !== 'object') return undefined; + // Own keys only — `in` would treat `__proto__` / `constructor` as hits + // and walk off the document into Object.prototype. + if (!Object.hasOwn(current, part)) return undefined; + current = current[part]; + } + return current; +} + +function ownSiblings(obj) { + const siblings = {}; + for (const [key, value] of Object.entries(obj)) { + if (key !== '$ref') siblings[key] = value; + } + return siblings; +} + +/** + * Follow an internal `#/…` `$ref` (and chains of them). External/file refs + * (`./paths/pets.yaml`) are left as-is so callers can treat them as unresolved. + * + * OAS 3.1 Path Item Objects may keep sibling fields next to `$ref`; those + * overlay the resolved target (local keys win) instead of being dropped. + */ +function resolveRefObject(root, obj, seen = new Set()) { + if (!obj || typeof obj !== 'object' || typeof obj.$ref !== 'string') return obj; + const ref = obj.$ref; + const siblings = ownSiblings(obj); + const hasSiblings = Object.keys(siblings).length > 0; + + if (!ref.startsWith('#/') || seen.has(ref)) return obj; + seen.add(ref); + const target = resolvePointer(root, ref); + if (target == null) return obj; + const resolved = resolveRefObject(root, target, seen); + if (!hasSiblings) return resolved; + if (!resolved || typeof resolved !== 'object' || Array.isArray(resolved)) return resolved; + return { ...resolved, ...siblings }; +} + +/** + * True when following `$ref` (including chained pointers) still lands on a + * `$ref` — external file, cycle, broken pointer, or `#/__proto__`-style miss. + * A single hop that lands on `{ $ref: './other.yaml' }` is unresolved. + */ +function isUnresolvedRef(root, obj) { + if (!obj || typeof obj !== 'object' || typeof obj.$ref !== 'string') return false; + const resolved = resolveRefObject(root, obj); + return !!(resolved && typeof resolved === 'object' && typeof resolved.$ref === 'string'); +} + +/** + * True when `paths` still has a `$ref` we could not inline. Used to skip the + * delete pass — missing operations after a failed resolve are "we couldn't + * see the spec", not "the operation was removed". + */ +export function hasUnresolvedOperationRefs(spec) { + for (const rawPathItem of Object.values(spec.paths || {})) { + if (isUnresolvedRef(spec, rawPathItem)) return true; + const pathItem = resolveRefObject(spec, rawPathItem); + if (!pathItem || typeof pathItem !== 'object') continue; + for (const [method, rawOp] of Object.entries(pathItem)) { + if (!HTTP_METHODS.has(method)) continue; + if (isUnresolvedRef(spec, rawOp)) return true; + } + } + return false; +} + /** * Extract operations from an OAS spec. * Returns a Map of operationId -> { summary, description, tag, operationId }. * For operations without an operationId, a synthetic one is generated from the method and path. + * + * Path-item and operation `$ref`s that point inside the same document + * (`#/components/pathItems/…`, `#/paths/~1pets`) are resolved first. OAS 3.1 + * `components.pathItems` is the documented way to reuse a path; walking the + * raw `$ref` stub would report zero operations and `oas:sync` / `lint --fix` + * would delete every matching reference page. */ 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)) { + for (const [pathStr, rawPathItem] of Object.entries(paths)) { + const methods = resolveRefObject(spec, rawPathItem) || {}; + for (const [method, rawOperation] of Object.entries(methods)) { if (!HTTP_METHODS.has(method)) continue; + const operation = resolveRefObject(spec, rawOperation) || {}; const operationId = operation.operationId || generateOperationId(method, pathStr); ops.set(operationId, { @@ -210,10 +302,14 @@ function syncOneOas(refDir, oasFilename, spec) { } const changes = { added: [], deleted: [], skipped: [] }; + // File $refs (and broken internal pointers) mean we cannot see the real + // operation set. Deleting "missing" pages would wipe valid reference docs. + const skipDeletes = hasUnresolvedOperationRefs(spec); // Deletes: pages referencing operations that no longer exist. for (const [opId, page] of pagesByOpId) { if (!specOps.has(opId)) { + if (skipDeletes) continue; fs.unlinkSync(page.filePath); const pageDir = path.dirname(page.filePath); diff --git a/test/oas-reference.test.js b/test/oas-reference.test.js index 9ac2991..75861c9 100644 --- a/test/oas-reference.test.js +++ b/test/oas-reference.test.js @@ -28,6 +28,33 @@ test('mismatched title/excerpt no longer reported as out of sync', () => { } }); +test('path-item $ref is not reported as a missing operation', () => { + const spec = JSON.stringify({ + openapi: '3.1.0', + info: { title: 'Pets', version: '1.0.0' }, + paths: { + '/pets': { $ref: '#/components/pathItems/Pets' }, + }, + components: { + pathItems: { + Pets: { get: { operationId: 'listPets' } }, + }, + }, + }); + const root = makeRepo({ + 'reference/pets.json': spec, + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\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); + } +}); + test('operation not found is still reported', () => { const root = makeRepo({ 'reference/pets.json': SPEC, diff --git a/test/oas-sync.test.js b/test/oas-sync.test.js index b0b48cc..dd884e6 100644 --- a/test/oas-sync.test.js +++ b/test/oas-sync.test.js @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import matter from 'gray-matter'; -import { syncOas } from '../src/commands/oas-sync.js'; +import { extractOperations, hasUnresolvedOperationRefs, syncOas } from '../src/commands/oas-sync.js'; import { makeRepo, rmRepo } from './helpers.js'; const SPEC = JSON.stringify({ @@ -105,6 +105,269 @@ test('sync does not overwrite an existing page from another spec or author', () } }); +const PATH_ITEMS_SPEC = { + openapi: '3.1.0', + info: { title: 'Pets', version: '1.0.0' }, + paths: { + '/pets': { $ref: '#/components/pathItems/Pets' }, + }, + components: { + pathItems: { + Pets: { + get: { operationId: 'listPets', summary: 'List pets', tags: ['pets'] }, + post: { operationId: 'createPet', summary: 'Create a pet', tags: ['pets'] }, + }, + }, + }, +}; + +test('extractOperations resolves OAS 3.1 components.pathItems $refs', () => { + const ops = extractOperations(PATH_ITEMS_SPEC); + assert.deepEqual([...ops.keys()].sort(), ['createPet', 'listPets']); + assert.equal(ops.get('listPets').tag, 'pets'); + assert.equal(hasUnresolvedOperationRefs(PATH_ITEMS_SPEC), false); +}); + +test('extractOperations resolves a path $ref to another path item', () => { + const spec = { + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { + get: { operationId: 'listPets', summary: 'List pets' }, + }, + '/animals': { $ref: '#/paths/~1pets' }, + }, + }; + const ops = extractOperations(spec); + // Same operationId is reused (Map last-write); the point is the $ref is visible. + assert.equal(ops.has('listPets'), true); + assert.equal(ops.size, 1); +}); + +test('extractOperations uses the resolved operationId on an operation $ref', () => { + const spec = { + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { + get: { $ref: '#/components/x-operations/ListPets' }, + }, + }, + components: { + 'x-operations': { + ListPets: { operationId: 'listPets', summary: 'List pets', tags: ['pets'] }, + }, + }, + }; + const ops = extractOperations(spec); + assert.equal(ops.has('listPets'), true); + assert.equal(ops.has('get_pets'), false); + assert.equal(ops.get('listPets').summary, 'List pets'); +}); + +test('sync does not delete pages when the spec uses path-item $refs', () => { + const root = makeRepo({ + 'reference/pets.json': JSON.stringify(PATH_ITEMS_SPEC), + 'reference/Pets/pets/listPets.md': + '---\ntitle: Custom docs\napi:\n file: pets.json\n operationId: listPets\n---\n\nCUSTOM BODY\n', + 'reference/Pets/pets/_order.yaml': '- listPets\n', + 'reference/Pets/_order.yaml': '- pets\n', + }); + try { + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + const page = path.join(root, 'reference/Pets/pets/listPets.md'); + assert.ok(fs.existsSync(page), 'existing $ref-backed page must survive sync'); + assert.match(fs.readFileSync(page, 'utf-8'), /CUSTOM BODY/); + assert.equal(result.changes.added.includes('Pets/pets/createPet.md'), true); + } finally { + rmRepo(root); + } +}); + +test('JSON Pointer does not follow inherited prototype keys', () => { + const spec = { + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { $ref: '#/__proto__' }, + }, + }; + assert.equal(extractOperations(spec).size, 0); + assert.equal(hasUnresolvedOperationRefs(spec), true); + + const root = makeRepo({ + 'reference/pets.json': JSON.stringify(spec), + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\n---\n\nCUSTOM BODY\n', + }); + try { + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + assert.match( + fs.readFileSync(path.join(root, 'reference/Pets/Other/listPets.md'), 'utf-8'), + /CUSTOM BODY/, + ); + } finally { + rmRepo(root); + } +}); + +test('chained $ref to an external file is unresolved and does not delete pages', () => { + const spec = { + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { $ref: '#/components/pathItems/Pets' }, + }, + components: { + pathItems: { + Pets: { $ref: './paths/pets.yaml' }, + }, + }, + }; + assert.equal(hasUnresolvedOperationRefs(spec), true); + assert.equal(extractOperations(spec).size, 0); + + const root = makeRepo({ + 'reference/pets.json': JSON.stringify(spec), + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\n---\n\nCUSTOM BODY\n', + }); + try { + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + assert.match( + fs.readFileSync(path.join(root, 'reference/Pets/Other/listPets.md'), 'utf-8'), + /CUSTOM BODY/, + ); + } finally { + rmRepo(root); + } +}); + +test('cyclic path-item $ref is unresolved and does not delete pages', () => { + const spec = { + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { $ref: '#/components/pathItems/A' }, + }, + components: { + pathItems: { + A: { $ref: '#/components/pathItems/B' }, + B: { $ref: '#/components/pathItems/A' }, + }, + }, + }; + assert.equal(hasUnresolvedOperationRefs(spec), true); + assert.equal(extractOperations(spec).size, 0); + + const root = makeRepo({ + 'reference/pets.json': JSON.stringify(spec), + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\n---\n\nCUSTOM BODY\n', + }); + try { + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + assert.ok(fs.existsSync(path.join(root, 'reference/Pets/Other/listPets.md'))); + } finally { + rmRepo(root); + } +}); + +test('OAS 3.1 path-item $ref keeps sibling operations', () => { + const spec = { + openapi: '3.1.0', + info: { title: 'Pets', version: '1.0.0' }, + paths: { + '/pets': { + $ref: '#/components/pathItems/Pets', + post: { operationId: 'createPet', tags: ['pets'] }, + }, + }, + components: { + pathItems: { + Pets: { + get: { operationId: 'listPets', tags: ['pets'] }, + }, + }, + }, + }; + const ops = extractOperations(spec); + assert.deepEqual([...ops.keys()].sort(), ['createPet', 'listPets']); + assert.equal(hasUnresolvedOperationRefs(spec), false); + + const root = makeRepo({ + 'reference/pets.json': JSON.stringify(spec), + 'reference/Pets/pets/createPet.md': + '---\napi:\n file: pets.json\n operationId: createPet\n---\n\nSIBLING BODY\n', + 'reference/Pets/pets/_order.yaml': '- createPet\n', + 'reference/Pets/_order.yaml': '- pets\n', + }); + try { + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + assert.match( + fs.readFileSync(path.join(root, 'reference/Pets/pets/createPet.md'), 'utf-8'), + /SIBLING BODY/, + ); + assert.equal(result.changes.added.includes('Pets/pets/listPets.md'), true); + } finally { + rmRepo(root); + } +}); + +test('OAS 3.1 path-item sibling overrides the referenced method', () => { + const spec = { + openapi: '3.1.0', + info: { title: 'Pets', version: '1.0.0' }, + paths: { + '/pets': { + $ref: '#/components/pathItems/Pets', + get: { operationId: 'listPetsV2', tags: ['pets'] }, + }, + }, + components: { + pathItems: { + Pets: { + get: { operationId: 'listPets', tags: ['pets'] }, + }, + }, + }, + }; + const ops = extractOperations(spec); + assert.equal(ops.has('listPetsV2'), true); + assert.equal(ops.has('listPets'), false); +}); + +test('sync does not delete pages when a path $ref points at an external file', () => { + const spec = JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { $ref: './paths/pets.yaml' }, + }, + }); + const root = makeRepo({ + 'reference/pets.json': spec, + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\n---\n\nCUSTOM BODY\n', + }); + try { + assert.equal(hasUnresolvedOperationRefs(JSON.parse(spec)), true); + const [result] = syncOas(root); + assert.deepEqual(result.changes.deleted, []); + const page = path.join(root, 'reference/Pets/Other/listPets.md'); + assert.ok(fs.existsSync(page), 'unresolved $ref must not wipe existing pages'); + assert.match(fs.readFileSync(page, 'utf-8'), /CUSTOM BODY/); + } finally { + rmRepo(root); + } +}); + test('existing reference page title is not overwritten by sync', () => { const root = makeRepo({ 'reference/pets.json': SPEC,