fix(downgrader): convert shared schemas once in dereferenced documents

packages/downgrader/README.md

@@ -23,7 +23,7 @@
 Every converter follows the same contract:
 
 - **Never throws.** Malformed parts are deep-copied through unchanged instead of failing the whole conversion. Cyclic object graphs, such as the output of a `$ref` dereferencer, convert with their cycles preserved. Only pathologically deep nesting (thousands of levels) can still exhaust the call stack.
-- **Never mutates.** The input is left untouched and the result is a new object.
+- **Never mutates.** The input is left untouched and the result is a new object. Objects shared within the input, such as a dereferenced schema used in several places, may stay shared within the result.
 - **Preserves extensions, never invents them.** `x-` keys and unknown keys survive. Constructs the target version cannot express are converted where an equivalent exists and removed otherwise.
 
 ## Usage

packages/downgrader/src/shared.test.ts

@@ -1,3 +1,5 @@
+import type { FieldTable } from './shared'
+
 import { dig } from '../tests/helpers'
 import {
   convertRecord,
@@ -133,6 +135,11 @@ describe('deepClone', () => {
     expect(clone.x).not.toBe(shared)
     expect(clone.x).toBe(clone.y)
   })
+
+  it('returns a fresh copy on every call', () => {
+    const shared = { a: 1 }
+    expect(deepClone(shared)).not.toBe(deepClone(shared))
+  })
 })
 
 describe('convertRecord', () => {
@@ -236,16 +243,50 @@ describe('convertRecord', () => {
     expect(second.self).toBe(second)
   })
 
-  it('converts shared acyclic references at every occurrence', () => {
+  it('converts a shared reference once per call and reuses the result', () => {
     const shared = { name: 'x' }
+    const fields: FieldTable = { name: () => 'converted' }
+    const convert = (item: unknown) => convertRecord(item, fields)
+    const result = convertRecord({ a: shared, b: shared }, { a: convert, b: convert })
+    expect(result).toEqual({ a: { name: 'converted' }, b: { name: 'converted' } })
+    expect(dig(result, 'b')).toBe(dig(result, 'a'))
+  })
+
+  it('clones a shared reference once per call', () => {
+    const shared = { deep: true }
+    const result = convertRecord({ a: shared, b: [shared] }, {})
+    expect(result).toEqual({ a: { deep: true }, b: [{ deep: true }] })
+    expect(dig(result, 'b', '0')).toBe(dig(result, 'a'))
+    expect(dig(result, 'a')).not.toBe(shared)
+  })
+
+  it('reuses a finished result only for the same field table and finish', () => {
+    const shared = { name: 'x' }
+    const fields: FieldTable = { name: () => 'converted' }
+    const wrap = (out: Record<string, unknown>) => ({ wrapped: out })
     const result = convertRecord(
-      { a: shared, b: shared },
+      { a: shared, b: shared, c: shared, d: shared },
       {
-        a: item => convertRecord(item, { name: () => 'a' }),
-        b: item => convertRecord(item, { name: () => 'b' }),
+        a: item => convertRecord(item, fields, wrap),
+        b: item => convertRecord(item, fields, wrap),
+        c: item => convertRecord(item, fields),
+        d: item => convertRecord(item, {}),
       },
     )
-    expect(result).toEqual({ a: { name: 'a' }, b: { name: 'b' } })
+    expect(result).toEqual({
+      a: { wrapped: { name: 'converted' } },
+      b: { wrapped: { name: 'converted' } },
+      c: { name: 'converted' },
+      d: { name: 'x' },
+    })
+    expect(dig(result, 'b')).toBe(dig(result, 'a'))
+  })
+
+  it('returns fresh results on every call', () => {
+    const shared = { name: 'x' }
+    const fields: FieldTable = { name: () => 'converted' }
+    expect(convertRecord(shared, fields)).not.toBe(convertRecord(shared, fields))
+    expect(dig(convertRecord({ a: shared }, {}), 'a')).not.toBe(dig(convertRecord({ a: shared }, {}), 'a'))
   })
 
   it('releases the cycle guard when a converter throws', () => {
@@ -259,6 +300,25 @@ describe('convertRecord', () => {
     ).toThrow('boom')
     expect(convertRecord(value, { a: () => 2 })).toEqual({ a: 2 })
   })
+
+  it('forgets reused results and clones when a converter throws', () => {
+    const shared = { name: 'x' }
+    const convert = vi.fn(() => 'converted')
+    const fields: FieldTable = { name: convert }
+    let clone: unknown
+    expect(() =>
+      convertRecord({ a: shared }, {
+        a: (item) => {
+          convertRecord(item, fields)
+          clone = deepClone(item)
+          throw new Error('boom')
+        },
+      }),
+    ).toThrow('boom')
+    const result = convertRecord({ a: shared, b: shared }, { a: item => convertRecord(item, fields) })
+    expect(convert).toHaveBeenCalledTimes(2)
+    expect(dig(result, 'b')).not.toBe(clone)
+  })
 })
 
 describe('operationFields', () => {

packages/downgrader/src/shared.ts

@@ -41,7 +41,19 @@ export function setOwn(object: object, key: PropertyKey, value: unknown): void {
   }
 }
 
-function cloneValue(value: unknown, seen: WeakMap<object, unknown>): unknown {
+type Finish = (out: Record<string, unknown>, source: Record<string, unknown>) => unknown
+
+interface Conversion {
+  done: boolean
+  fields: FieldTable
+  finish: Finish | undefined
+  result: unknown
+}
+
+const conversions = new Map<object, Conversion>()
+const clones = new Map<object, unknown>()
+
+function cloneValue(value: unknown, seen: Map<object, unknown>): unknown {
   if (!(Array.isArray(value) || isRecord(value))) {
     return value
   }
@@ -69,21 +81,21 @@ export function deepClone<T>(value: T): T {
   if (!(Array.isArray(value) || isRecord(value))) {
     return value
   }
-  return cloneValue(value, new WeakMap()) as T
+  return cloneValue(value, conversions.size > 0 ? clones : new Map()) as T
 }
 
-const converting = new WeakMap<object, Record<string, unknown>>()
-
-export function convertRecord(value: unknown, fields: FieldTable, finish?: (out: Record<string, unknown>, source: Record<string, unknown>) => unknown): unknown {
+export function convertRecord(value: unknown, fields: FieldTable, finish?: Finish): unknown {
   if (!isRecord(value)) {
     return deepClone(value)
   }
-  const inProgress = converting.get(value)
-  if (inProgress !== undefined) {
-    return inProgress
+  const known = conversions.get(value)
+  if (known !== undefined && (!known.done || (known.fields === fields && known.finish === finish))) {
+    return known.result
   }
   const out: Record<string, unknown> = {}
-  converting.set(value, out)
+  const conversion: Conversion = { done: false, fields, finish, result: out }
+  const outermost = conversions.size === 0
+  conversions.set(value, conversion)
   try {
     for (const [key, item] of Object.entries(value)) {
       const convert = Object.hasOwn(fields, key) ? fields[key] : undefined
@@ -95,10 +107,15 @@ export function convertRecord(value: unknown, fields: FieldTable, finish?: (out:
         setOwn(out, key, converted)
       }
     }
-    return finish === undefined ? out : finish(out, value)
+    conversion.result = finish === undefined ? out : finish(out, value)
+    conversion.done = true
+    return conversion.result
   }
   finally {
-    converting.delete(value)
+    if (outermost) {
+      conversions.clear()
+      clones.clear()
+    }
   }
 }
 

packages/downgrader/src/v3.1-to-v3.0.test.ts

@@ -766,6 +766,20 @@ describe('downgradeSpecV31ToV30', () => {
       expect(dig(result, 'get', 'responses')).toEqual({ default: { description: '' } })
       expect(dig(result, 'get', 'callbacks', 'cb', 'expr')).toBe(result)
     })
+
+    it('converts a dereferenced schema shared across the document once', () => {
+      const pet = { properties: { name: { type: ['string', 'null'] } }, type: 'object' }
+      const result = convertSpec({
+        components: { schemas: { Pet: pet } },
+        paths: { '/pets': { get: { responses: { 200: { content: { 'application/json': { schema: pet } }, description: 'ok' } } } } },
+      })
+      const schema = dig(result, 'components', 'schemas', 'Pet')
+      expect(schema).toEqual({
+        properties: { name: { nullable: true, type: 'string' } },
+        type: 'object',
+      })
+      expect(dig(result, 'paths', '/pets', 'get', 'responses', '200', 'content', 'application/json', 'schema')).toBe(schema)
+    })
   })
 })
 
@@ -1414,6 +1428,20 @@ describe('downgradeSchemaV31ToV30', () => {
       expect(node.type).toEqual(['object', 'null'])
     })
 
+    it('converts a dereferenced schema reached along many paths once', () => {
+      let node: OpenAPIV3_1.SchemaObject = { type: ['string', 'null'] }
+      for (let index = 0; index < 64; index += 1) {
+        node = { properties: { left: node, right: node }, type: 'object' }
+      }
+      const result = convertSchema(node)
+      expect(dig(result, 'properties', 'left')).toBe(dig(result, 'properties', 'right'))
+      let leaf = result
+      for (let index = 0; index < 64; index += 1) {
+        leaf = dig(leaf, 'properties', 'left')
+      }
+      expect(leaf).toEqual({ nullable: true, type: 'string' })
+    })
+
     it('points the array variant of a cyclic multi-type schema at the converted schema', () => {
       const node: Record<string, unknown> = { type: ['array', 'object'] }
       node.items = node

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

@@ -1256,6 +1256,18 @@ describe('downgradeSpecV32ToV31', () => {
       expect(dig(result, 'get', 'responses', '200')).toEqual({ description: 'ok' })
       expect(dig(result, 'get', 'callbacks', 'cb', 'expr')).toBe(result)
     })
+
+    it('copies a dereferenced schema shared across the document once', () => {
+      const 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' } } } } },
+      })
+      const schema = dig(result, 'components', 'schemas', 'Pet')
+      expect(schema).toEqual(pet)
+      expect(schema).not.toBe(pet)
+      expect(dig(result, 'paths', '/pets', 'get', 'responses', '200', 'content', 'application/json', 'schema')).toBe(schema)
+    })
   })
 })
 
@@ -1318,6 +1330,11 @@ describe('downgradeSchemaV32ToV31', () => {
     })
   })
 
+  it('returns a fresh copy on every call', () => {
+    const schema: OpenAPIV3_2.SchemaObject = { properties: { a: { type: 'string' } }, type: 'object' }
+    expect(downgradeSchemaV32ToV31(schema)).not.toBe(downgradeSchemaV32ToV31(schema))
+  })
+
   it('never mutates the input schema', () => {
     const schema: OpenAPIV3_2.SchemaObject = {
       discriminator: { defaultMapping: 'Dog', propertyName: 'kind' },