diff --git a/packages/downgrader/README.md b/packages/downgrader/README.md index 0409def..2ffb57a 100644 --- a/packages/downgrader/README.md +++ b/packages/downgrader/README.md @@ -211,7 +211,7 @@ Accepts the default OAS dialect, which adds `discriminator`, `xml`, `externalDoc | [`nullable`][3.0-schema-nullable], a 3.0 keyword | Kept beside a single `type`, where it means in 3.0 what its author meant, and the schema counts as having lost a restriction. Removed otherwise. | | A boolean `exclusiveMinimum` or `exclusiveMaximum`, the 3.0 form, as NestJS writes in 3.1 | Kept beside a numeric `minimum` or `maximum`, where it means in 3.0 what its author meant. Removed otherwise. | | [`readOnly`][3.0-schema-read-only] or [`writeOnly`][3.0-schema-write-only] | Kept. 3.0 requires such a property in responses or in requests only, where 3.1 requires it everywhere, so the schema counts as having lost a restriction, and the property tells no `oneOf` branch apart. | -| [`not`][js-not] over a schema that lost a restriction above | Removed. Negating the looser schema would reject values the original accepts. A keyword that restricts nothing, such as `propertyNames: { type: "string" }`, or a tuple converted exactly, does not count. A schema that loses one only around a cycle, of `$ref`s or of objects, counts too, which converts the input a second time. | +| [`not`][js-not] over a schema that lost a restriction above | Removed. Negating the looser schema would reject values the original accepts. A keyword that restricts nothing, such as `propertyNames: { type: "string" }`, or a tuple converted exactly, does not count. A recursive schema, which a cycle of `$ref`s or of objects reaches again before its conversion is done, counts too. | | [`oneOf`][js-one-of] with a branch that lost a restriction above | `anyOf`. Looser branches may overlap, and then match more than one. It stays a `oneOf` when no two branches can match the same value however loose they become: they have different types, other than `integer` and `number`, or one is an object that requires a property the other holds to other `const` or `enum` values, as OpenAI, Zod and oRPC write a tagged union, also through a `$ref` with only annotations beside it, as Zod writes a literal with its own id, and in a member of an `allOf`, as OpenAI writes a resource. A `$ref` that recursion cuts to `{}`, also in a copy reused later, tells no branch apart. | [js-type]: https://json-schema.org/draft/2020-12/json-schema-validation#name-type @@ -315,7 +315,7 @@ Accepts the default OAS dialect, which adds `discriminator`, `xml`, `externalDoc Each object is converted once (inside an inlined `$ref` target, once per target), however many `$ref`s or shared references reach it, so the work grows with the size of the document, not with the number of paths through it. A cycle of objects in the input, as a dereferencing parser leaves, becomes the same cycle in the output. -Four shapes no generator emits fall outside this. A schema that loses a restriction only around a cycle, of `$ref`s or of objects, converts the input again when it keeps a `not` or `oneOf`, once for each such schema found after the last pass. A chain of `$ref`s that point at `$ref`s is followed again from each place that reaches it, so its cost grows with the square of its length. A `oneOf` with a branch that lost a restriction compares its branches pair by pair, so its cost grows with the square of their number, about 100 ms for a thousand. And conversion is recursive, so input nested, or chained through `$ref`s, more than about a thousand levels deep can throw a `RangeError`. If you convert untrusted documents, limit their size and depth first. +Three shapes no generator emits fall outside this. A chain of `$ref`s that point at `$ref`s is followed again from each place that reaches it, so its cost grows with the square of its length. A `oneOf` with a branch that lost a restriction compares its branches pair by pair, so its cost grows with the square of their number, about 100 ms for a thousand. And conversion is recursive, so input nested, or chained through `$ref`s, more than about a thousand levels deep can throw a `RangeError`. If you convert untrusted documents, limit their size and depth first. [Benchmarks](https://github.com/middleapi/openapi-spec/tree/main/packages/downgrader/benches) cover each converter on the official example documents, a document generated by oRPC, generated APIs, and worst-case reference graphs. [CodSpeed](https://app.codspeed.io/middleapi/openapi-spec) runs them on every pull request to catch regressions. diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index c2aae27..44b4c2a 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -123,7 +123,7 @@ export function convertObject( ctx: Context, fields: Fields, finish?: Finish, - revisit?: (value: object, out: object) => void, + revisit?: (out: object) => void, ): unknown { if (!isRecord(value)) { return clone(value) @@ -135,7 +135,7 @@ export function convertObject( } const known = seen.get(value) if (known !== undefined) { - revisit?.(value, known) + revisit?.(known) return known } const out: Record = {} diff --git a/packages/downgrader/src/v3.1-to-v3.0.ts b/packages/downgrader/src/v3.1-to-v3.0.ts index 2158838..6abac36 100644 --- a/packages/downgrader/src/v3.1-to-v3.0.ts +++ b/packages/downgrader/src/v3.1-to-v3.0.ts @@ -122,10 +122,9 @@ const IN_PLACE_APPLICATORS = [ // Converted schemas that lost a restriction, directly or in a subschema. const LOOSE = new WeakSet() -// Whether a `not` or `oneOf` kept its form, since nothing under it was loose. -// Only such a choice changes when more schemas read as loose, so without one -// the conversion need not run again. -let keptStrict = false +// Converted schemas whose conversion is done. One met again before that, +// around a cycle, counts as loose, since what it loses is not known yet. +const DONE = new WeakSet() // Converted schemas with an `anyOf` or `oneOf` of several branches, for // `mergeEnums` and `mergeNullable`. They run once the whole input is converted, @@ -140,21 +139,6 @@ const unionTargets = new Set() // Input schemas a `$ref` kept as written points to, for `scalarOneOf`. const refTargets = new Set() -// The output of each target a `$ref` kept as written read as not loose. Around -// a cycle, that output was still being converted, and can lose a restriction -// after the `$ref` read it. -const refOutputs = new Map() - -// Targets that did so, which the next pass reads as loose from the start. -const looseTargets = new Set() - -// The same for a cycle of objects, as a dereferencing parser leaves: each -// schema met again, also where a document shares it, with its output, not -// loose yet, and the schemas whose output then lost a restriction, which the -// next pass reads as loose when it meets them again. -const revisits: [object, object][] = [] -const looseInputs = new Set() - // Schemas whose `$ref` a copy cut to `{}` at recursion. The copy is reused // wherever its target is inlined again, or a `$ref` reads the component that // holds it, also once the target it was cut at is no longer being inlined. @@ -416,14 +400,10 @@ function convertSchemaRef(ref: string, ctx: Context, holder: unknown): unknown { refTargets.add(target) // A `$ref` kept as written loses what its target loses. Converting the // target here reuses the copy the components get, and stops at a cycle, - // where the target is still being converted. + // where the target is still being converted, so it counts as loose. const out = { $ref: ref } const converted = inline(ref, ctx, convertSchema, true) - if (isLoose(converted) || looseTargets.has(target)) { - return loosened(out) - } - refOutputs.set(target, converted === DROP ? sharedOutput(target, ctx) : converted) - return out + return isLoose(converted) || (converted === DROP && target !== undefined) ? loosened(out) : out } const out = inline(ref, ctx, convertSchema) if (out === DROP) { @@ -433,26 +413,6 @@ function convertSchemaRef(ref: string, ctx: Context, holder: unknown): unknown { return out } -// The output of a target still being converted. A target that converts to the -// output of the target of its `$ref`, an alias, has none of its own, so it is -// the output of the target its chain of `$ref`s ends at: the copy the -// components get, or for a `$ref` the conversion inlines, its inlined copy. -function sharedOutput(target: unknown, ctx: Context): unknown { - const visited = new Set() - let at = target - let seen: Context['seen'] | undefined = ctx.shared - for ( - let ref = aliasRef(at, ctx); - ref !== undefined && !visited.has(at); - ref = aliasRef(at, ctx) - ) { - visited.add(at) - at = resolvePointer(ref, ctx) - seen = isInlined(ref, ctx) ? ctx.apart.get(at) : ctx.shared - } - return isRecord(at) ? seen?.get(SCHEMA_FIELDS)?.get(at) : undefined -} - // 3.0 `items` applies one schema to every item, so a tuple becomes an array // whose items match any of its item schemas, and those of the items after // them, unless `items: false` or `maxItems` allows none. `items: false` @@ -785,19 +745,13 @@ function loosen( if (isLoose(out.not)) { delete out.not loose = true - } else if (out.not !== undefined) { - keptStrict = true - } - if (Array.isArray(out.oneOf)) { - if (out.oneOf.some(isLoose)) { - if (!isDisjoint(schema.oneOf as unknown[], ctx)) { - addAnyOf(out, out.oneOf) - delete out.oneOf - } - loose = true - } else { - keptStrict = true + } + if (Array.isArray(out.oneOf) && out.oneOf.some(isLoose)) { + if (!isDisjoint(schema.oneOf as unknown[], ctx)) { + addAnyOf(out, out.oneOf) + delete out.oneOf } + loose = true } return ( loose || @@ -1565,15 +1519,12 @@ function finishSchema( if (loose) { LOOSE.add(out) } + DONE.add(out) } -// Met again before its output was done, around a cycle, a schema not loose -// yet may lose a restriction after a `not` or `oneOf` around it read it. -function revisitSchema(value: object, out: object): void { - if (looseInputs.has(value)) { +function revisitSchema(out: object): void { + if (!DONE.has(out)) { LOOSE.add(out) - } else if (!isLoose(out)) { - revisits.push([value, out]) } } @@ -1591,22 +1542,15 @@ function convertSchema(value: unknown, ctx: Context): unknown { if (typeof value === 'boolean') { return value ? {} : { not: {} } } - const ref = aliasRef(value, ctx) - if (ref !== undefined) { - return convertSchemaRef(ref, ctx, value) - } - return convertObject(value, ctx, SCHEMA_FIELDS, finishSchema, revisitSchema) -} - -// The `$ref` of a schema that becomes its target's output: a bare `$ref`, or -// one to a tagged union beside only annotations. -function aliasRef(value: unknown, ctx: Context): string | undefined { const ref = getRef(value) - return ref !== undefined && + if ( + ref !== undefined && (isBareRef(value as Record) || (isAnnotatedRef(value as Record) && isTaggedRef(ref, ctx))) - ? ref - : undefined + ) { + return convertSchemaRef(ref, ctx, value) + } + return convertObject(value, ctx, SCHEMA_FIELDS, finishSchema, revisitSchema) } function convertParameter(value: unknown, ctx: Context): unknown { @@ -1702,20 +1646,7 @@ function convertDocument(value: unknown, ctx: Context): unknown { function downgradeV30(root: unknown, convert: Convert): unknown { try { - let out = downgrade(root, convert) - // A `not` or `oneOf` over a `$ref` around a cycle read its target before - // that target lost a restriction, so the conversion runs again, reading - // such targets as loose from the start, until no other target does so. - while (keptStrict && markLooseTargets()) { - keptStrict = false - nullableUnions.length = 0 - unionTargets.clear() - refTargets.clear() - refOutputs.clear() - cutHolders.clear() - revisits.length = 0 - out = downgrade(root, convert) - } + const out = downgrade(root, convert) for (const [schema, input, key] of nullableUnions) { if (!unionTargets.has(input) && (key === 'oneOf' || dropEqualBranches(schema))) { const isTarget = refTargets.has(input) @@ -1733,34 +1664,11 @@ function downgradeV30(root: unknown, convert: Convert): unknown { return out } finally { // Cleared even when conversion throws, so nothing keeps the input alive. - keptStrict = false nullableUnions.length = 0 unionTargets.clear() refTargets.clear() - refOutputs.clear() - looseTargets.clear() cutHolders.clear() - revisits.length = 0 - looseInputs.clear() - } -} - -// Adds each target whose output became loose after a `$ref` read it. -function markLooseTargets(): boolean { - let added = false - for (const [target, out] of refOutputs) { - if (isLoose(out)) { - looseTargets.add(target) - added = true - } - } - for (const [input, out] of revisits) { - if (isLoose(out) && !looseInputs.has(input)) { - looseInputs.add(input) - added = true - } } - return added } /** Downgrades an OpenAPI 3.1 document to 3.0, setting `openapi` to `3.0.4`. The input is never changed. */ diff --git a/packages/downgrader/tests/v3.1-to-v3.0.test.ts b/packages/downgrader/tests/v3.1-to-v3.0.test.ts index e62188c..d65f439 100644 --- a/packages/downgrader/tests/v3.1-to-v3.0.test.ts +++ b/packages/downgrader/tests/v3.1-to-v3.0.test.ts @@ -3353,8 +3353,9 @@ describe('downgradeSchemaV31ToV30', () => { { type: 'array', items: { type: 'string' } }, ], }) + // A $ref met again while its target is still being converted counts as loose. expect(schemas?.Cycle).toEqual({ - oneOf: [{ $ref: '#/components/schemas/A' }, { type: 'string' }], + anyOf: [{ $ref: '#/components/schemas/A' }, { type: 'string' }], }) }) })