fix(downgrader): keep schema identifiers unique when inlining 3.2 to 3.1

packages/downgrader/README.md

@@ -137,15 +137,15 @@ 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.
 
 ## 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

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' } } })

packages/downgrader/src/shared.ts

@@ -10,6 +10,7 @@ export interface Context {
   readonly markDangling: (ref: string) => void
   readonly converting: unknown[]
   readonly copies: Map<object, unknown>
+  readonly identified: Set<unknown>
   readonly inlined: Map<Convert, Map<unknown, unknown>>
   readonly inlining: Set<unknown>
   readonly removals: Map<string, boolean | undefined>
@@ -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,

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' } } } } },

packages/downgrader/src/v3.2-to-v3.1.ts

@@ -188,6 +188,14 @@ function convertTag(value: unknown, ctx: Context): unknown {
 }
 
 function finishSchema(out: Record<string, unknown>, schema: Record<string, unknown>, 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) {

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)