diff --git a/packages/downgrader/README.md b/packages/downgrader/README.md index 84c6569..09a454c 100644 --- a/packages/downgrader/README.md +++ b/packages/downgrader/README.md @@ -137,7 +137,7 @@ Removed, with no 3.0 equivalent: `$schema`, `$id`, `$defs`, `$anchor`, `$dynamic Both converters treat local `$ref`s the same way: - A `$ref` whose target is removed or moved is replaced by its converted target, following the reference chain until it leaves the removed part. Beside other schema keywords, the target joins `allOf`. When a Path Item `$ref` is replaced, its own fields win over the target's. This covers `webhooks`, `components.pathItems`, `components.mediaTypes`, `$defs`, `itemSchema`, `query` and `additionalOperations` operations, and parameter lists that lost entries. -- A target inlined in several places is converted once and shared. Where it refers back to itself, the inner reference becomes `{}` in a schema, keeps only its own fields on a Path Item, and is removed elsewhere. +- A target inlined in several places is converted once and shared. Where it refers back to itself, the inner reference becomes `{}` in a schema, keeps only its own fields on a Path Item, and is removed elsewhere. In 3.2 → 3.1, only the first copy of a repeated schema keeps its `$id`, `$anchor`, and `$dynamicAnchor`, so each identifier stays unique. - A `$ref` to an object the target version cannot express, such as a `querystring` parameter or a `mutualTLS` scheme, is removed with it. So are Links and discriminator `mapping` entries that point into a removed part. - Inlining ignores the Reference Object's own fields, such as `summary`, `description`, and extensions. - Left as written, even if they then dangle: external references, `$anchor` references, references that already dangle, `$ref` chains that loop, references to values other than objects and boolean schemas, and Path Item `$ref`s whose target is not a Path Item. The exception is a `$ref` in a 3.2 `content` map, which is removed because 3.1 cannot hold a reference there. @@ -145,7 +145,7 @@ Both converters treat local `$ref`s the same way: ## Known limitations - Both: a Link that names a removed operation (`query`, `additionalOperations`, a webhook) by `operationId` is kept, and a Path Item inlined in several places repeats its `operationId`s. -- 3.2 → 3.1: security requirements keyed by URI, `$self`-relative references, and a `$schema` naming the 3.2 dialect pass through unchanged. Where recursion becomes `{}`, an enclosing `not`, `oneOf`, `if`, or `unevaluated*` can reject values the original accepts. +- 3.2 → 3.1: security requirements keyed by URI, `$self`-relative references, and a `$schema` naming the 3.2 dialect pass through unchanged. Where recursion becomes `{}`, an enclosing `not`, `oneOf`, `if`, or `unevaluated*` can reject values the original accepts. A repeated schema copy that loses its `$id` resolves its relative `$ref`s against the enclosing base instead. - 3.1 → 3.0: `$ref`s to an `$anchor` or resolved against an `$id` base are left as written and dangle, so rewrite them as JSON pointers first. A `not` or `oneOf` that reaches a loosened schema through a `$ref` kept in the output can reject values the original accepts. Non-standard schema keywords are kept, although the official 3.0 schema forbids them. ## Sponsors diff --git a/packages/downgrader/src/shared.test.ts b/packages/downgrader/src/shared.test.ts index 20de578..6878cde 100644 --- a/packages/downgrader/src/shared.test.ts +++ b/packages/downgrader/src/shared.test.ts @@ -31,6 +31,7 @@ function createContext(root: unknown = {}): Context { converting: [], copies: new Map(), dangles: () => false, + identified: new Set(), inlined: new Map(), inlining: new Set(), isRemovedPart: () => false, @@ -68,11 +69,11 @@ function convertItem(value: unknown, ctx: Context): unknown { return isRecord(value) && value.drop === true ? DROP : convertObject(value, ctx, ITEM_FIELDS) } -function convertDocument(value: unknown, removed?: string[]): { out: any, passes: number } { +function convertDocument(value: unknown, removed?: string[], fields = DOCUMENT_FIELDS): { out: any, passes: number } { let passes = 0 const out = downgrade(value, (item, ctx) => { passes += 1 - return convertObject(item, ctx, DOCUMENT_FIELDS) + return convertObject(item, ctx, fields) }, removed) return { out, passes } } @@ -707,6 +708,19 @@ describe('downgrade', () => { expect(out.items[0]).toBe(out.items[2]) }) + it('converts a target again for a later place when its conversion identified a value', () => { + const fields = defineFields({ + items: list(refOr((value, ctx) => { + const repeated = ctx.identified.has(value) + ctx.identified.add(value) + return { repeated } + })), + }) + const { out } = convertDocument({ items: [{ $ref: '#/removed/a' }, { $ref: '#/removed/a' }, { $ref: '#/removed/a' }], removed: { a: {} } }, ['#/removed/'], fields) + expect(out.items).toEqual([{ repeated: false }, { repeated: true }, { repeated: true }]) + expect(out.items[2]).toBe(out.items[1]) + }) + it('resolves references nested in inlined targets without a pass per level', () => { const removed = Object.fromEntries(Array.from({ length: 20 }, (_, index) => [`r${index}`, { next: { $ref: `#/removed/r${index + 1}` } }])) const { out, passes } = convertDocument({ items: [{ $ref: '#/removed/r0' }], removed: { ...removed, r20: { value: 'end' } } }) diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index ba72cf2..92468ff 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -10,6 +10,7 @@ export interface Context { readonly markDangling: (ref: string) => void readonly converting: unknown[] readonly copies: Map + readonly identified: Set readonly inlined: Map> readonly inlining: Set readonly removals: Map @@ -217,10 +218,13 @@ export function inline(ref: string, ctx: Context, convert: Convert): unknown { if (cache.has(target)) { return cache.get(target) } + const identified = ctx.identified.size ctx.inlining.add(target) const out = convert(target, { ...ctx, seen: new Map() }) ctx.inlining.delete(target) - cache.set(target, out) + if (ctx.identified.size === identified) { + cache.set(target, out) + } return out } @@ -229,7 +233,7 @@ function convertsToDrop(ref: string, ctx: Context, convert: Convert): boolean { return ctx.removals.get(ref) === true } ctx.removals.set(ref, undefined) - const removed = inline(ref, { ...ctx, converting: [], inlined: new Map(), inlining: new Set(), seen: new Map() }, convert) === DROP + const removed = inline(ref, { ...ctx, converting: [], identified: new Set(), inlined: new Map(), inlining: new Set(), seen: new Map() }, convert) === DROP ctx.removals.set(ref, removed) return removed } @@ -440,6 +444,7 @@ export function downgrade(root: unknown, convert: Convert, removed: readonly str } return dangling.has(ref) }, + identified: new Set(), inlined: new Map(), inlining: new Set(), isRemovedPart, diff --git a/packages/downgrader/src/v3.2-to-v3.1.test.ts b/packages/downgrader/src/v3.2-to-v3.1.test.ts index 6f8e7bd..faec5aa 100644 --- a/packages/downgrader/src/v3.2-to-v3.1.test.ts +++ b/packages/downgrader/src/v3.2-to-v3.1.test.ts @@ -1870,6 +1870,63 @@ describe('downgradeSpecV32ToV31', () => { expect(dig(result, 'components', 'schemas', 'S')).toEqual({ allOf: [{ type: 'string' }], description: 'b' }) }) + it('keeps $id and $anchor on the first copy of a schema inlined in several places', () => { + const pet = { $id: 'https://example.com/pet', properties: { name: { $anchor: 'name', type: 'string' } }, type: 'object' } + const result = convertSpec({ + components: { + mediaTypes: { Pet: { schema: pet } }, + schemas: { Named: { $ref: '#/components/mediaTypes/Pet/schema', description: 'named' } }, + }, + paths: { + '/a': { + get: { + responses: { + 200: { + content: { 'application/json': { $ref: '#/components/mediaTypes/Pet' } }, + description: 'ok', + }, + }, + }, + }, + }, + }) + expect(dig(result, 'components', 'schemas', 'Named')).toEqual({ allOf: [pet], description: 'named' }) + expect(dig(result, 'paths', '/a', 'get', 'responses', '200', 'content')).toEqual({ + 'application/json': { schema: { properties: { name: { type: 'string' } }, type: 'object' } }, + }) + }) + + it('keeps identifiers on a moved or shifted original rather than on the copies inlined from it', () => { + expect(convertPathItem({ + get: { + parameters: [ + { content: { 'text/plain': {} }, in: 'querystring', name: 'q' }, + { in: 'query', name: 'p', schema: { $dynamicAnchor: 'p', type: 'string' } }, + ], + responses: { + 200: { content: { 'application/jsonl': { itemSchema: { $id: 'https://example.com/item' } } }, description: 'ok' }, + }, + }, + post: { + parameters: [{ $ref: '#/paths/~1a/get/parameters/1' }], + requestBody: { + content: { 'application/json': { schema: { $ref: '#/paths/~1a/get/responses/200/content/application~1jsonl/itemSchema' } } }, + }, + }, + })).toEqual({ + get: { + parameters: [{ in: 'query', name: 'p', schema: { $dynamicAnchor: 'p', type: 'string' } }], + responses: { + 200: { content: { 'application/jsonl': { schema: { items: { $id: 'https://example.com/item' }, type: 'array' } } }, description: 'ok' }, + }, + }, + post: { + parameters: [{ in: 'query', name: 'p', schema: { type: 'string' } }], + requestBody: { content: { 'application/json': { schema: {} } } }, + }, + }) + }) + it('inlines a path item $ref that points into a removed operation, keeping own fields', () => { const callbacks = { c: { '{$url}': { description: 'inlined', summary: 'Inlined' } } } const result = convertSpec({ @@ -1978,7 +2035,7 @@ describe('downgradeSpecV32ToV31', () => { }) it('copies a dereferenced schema shared across the document once', () => { - const pet = { properties: { name: { type: 'string' } }, type: 'object' } + const pet = { $anchor: 'pet', $id: 'https://example.com/pet', properties: { name: { type: 'string' } }, type: 'object' } const result = convertSpec({ components: { schemas: { Pet: pet } }, paths: { '/pets': { get: { responses: { 200: { content: { 'application/json': { schema: pet } }, description: 'ok' } } } } }, diff --git a/packages/downgrader/src/v3.2-to-v3.1.ts b/packages/downgrader/src/v3.2-to-v3.1.ts index 4122c3d..504d86c 100644 --- a/packages/downgrader/src/v3.2-to-v3.1.ts +++ b/packages/downgrader/src/v3.2-to-v3.1.ts @@ -188,6 +188,14 @@ function convertTag(value: unknown, ctx: Context): unknown { } function finishSchema(out: Record, schema: Record, ctx: Context): unknown { + if (ctx.identified.has(schema)) { + delete out.$id + delete out.$anchor + delete out.$dynamicAnchor + } + else if ('$id' in schema || '$anchor' in schema || '$dynamicAnchor' in schema) { + ctx.identified.add(schema) + } if (typeof schema.$ref === 'string' && !('$ref' in out)) { const target = inlineSchema(schema.$ref, ctx, convertSchema) if (target !== DROP) { diff --git a/packages/downgrader/tests/e2e.test.ts b/packages/downgrader/tests/e2e.test.ts index 5e70005..6995f03 100644 --- a/packages/downgrader/tests/e2e.test.ts +++ b/packages/downgrader/tests/e2e.test.ts @@ -387,6 +387,50 @@ describe('3.2 example documents downgraded to 3.1 and chained to 3.0', () => { expect(doc).toEqual(before) }) + it('keeps schema identifiers unique when it inlines a schema in several places', async () => { + const doc: OpenAPIV3_2.OpenAPIObject = { + components: { + mediaTypes: { + Pet: { + schema: { + $id: 'https://example.com/pet', + properties: { name: { $anchor: 'name', type: 'string' } }, + type: 'object', + }, + }, + }, + }, + info: { title: 'Identifiers', version: '1.0.0' }, + openapi: '3.2.0', + paths: { + '/pets': { + get: { + responses: { 200: { content: { 'application/json': { $ref: '#/components/mediaTypes/Pet' } }, description: 'Pet' } }, + }, + post: { + requestBody: { content: { 'application/json': { $ref: '#/components/mediaTypes/Pet' } } }, + responses: { + 201: { + content: { 'application/json': { schema: { $ref: '#/components/mediaTypes/Pet/schema/properties/name' } } }, + description: 'Name', + }, + }, + }, + }, + }, + } + const before = structuredClone(doc) + + const v31 = downgradeSpecV32ToV31(doc) + const serialized = JSON.stringify(v31) + expect(serialized.match(/"\$id"/g)).toHaveLength(1) + expect(serialized.match(/"\$anchor"/g)).toHaveLength(1) + await expectValidAs(v31, '3.1') + + await expectValidAs(downgradeSpecV31ToV30(v31), '3.0') + expect(doc).toEqual(before) + }) + it('converts the 3.2 mega document, removing the discriminator defaultMapping from the schema', async () => { const before = structuredClone(mega32) const v31 = downgradeSpecV32ToV31(mega32)