diff --git a/packages/downgrader/README.md b/packages/downgrader/README.md index b4c64b1..6525cb9 100644 --- a/packages/downgrader/README.md +++ b/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 diff --git a/packages/downgrader/src/shared.test.ts b/packages/downgrader/src/shared.test.ts index ba3d0fb..8e26212 100644 --- a/packages/downgrader/src/shared.test.ts +++ b/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) => ({ 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', () => { diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index 721d285..7909692 100644 --- a/packages/downgrader/src/shared.ts +++ b/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): unknown { +type Finish = (out: Record, source: Record) => unknown + +interface Conversion { + done: boolean + fields: FieldTable + finish: Finish | undefined + result: unknown +} + +const conversions = new Map() +const clones = new Map() + +function cloneValue(value: unknown, seen: Map): unknown { if (!(Array.isArray(value) || isRecord(value))) { return value } @@ -69,21 +81,21 @@ export function deepClone(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>() - -export function convertRecord(value: unknown, fields: FieldTable, finish?: (out: Record, source: Record) => 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 = {} - 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() + } } } diff --git a/packages/downgrader/src/v3.1-to-v3.0.test.ts b/packages/downgrader/src/v3.1-to-v3.0.test.ts index 3755941..4aa859e 100644 --- a/packages/downgrader/src/v3.1-to-v3.0.test.ts +++ b/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 = { type: ['array', 'object'] } node.items = node 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 e17a07b..27eeee9 100644 --- a/packages/downgrader/src/v3.2-to-v3.1.test.ts +++ b/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' },