From 03be17c1b06a78c83d881b04b600f32f55771300 Mon Sep 17 00:00:00 2001 From: Vladimir Gorej Date: Tue, 7 Nov 2023 00:05:55 +0100 Subject: [PATCH] fix(reference): fix handling cycles in all dereference strategies Refs https://github.com/swagger-api/swagger-ui/issues/9337 --- .../strategies/asyncapi-2/visitor.ts | 76 ++++++++++++++-- .../strategies/openapi-3-0/visitor.ts | 57 ++++++++++-- .../strategies/openapi-3-1/visitor.ts | 87 ++++++++++++++++--- .../apidom-reference/src/dereference/util.ts | 19 +++- .../cycle-internal-advanced/root.json | 35 ++++++++ .../openapi-3-1/schema-object/index.ts | 38 ++++++-- 6 files changed, 269 insertions(+), 43 deletions(-) create mode 100644 packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/fixtures/cycle-internal-advanced/root.json diff --git a/packages/apidom-reference/src/dereference/strategies/asyncapi-2/visitor.ts b/packages/apidom-reference/src/dereference/strategies/asyncapi-2/visitor.ts index 3a0533f3b2..e51523bdbd 100644 --- a/packages/apidom-reference/src/dereference/strategies/asyncapi-2/visitor.ts +++ b/packages/apidom-reference/src/dereference/strategies/asyncapi-2/visitor.ts @@ -10,6 +10,7 @@ import { isStringElement, visit, toValue, + refractorPluginElementIdentity, } from '@swagger-api/apidom-core'; import { ApiDOMError } from '@swagger-api/apidom-error'; import { evaluate, uriToPointer } from '@swagger-api/apidom-json-pointer'; @@ -19,6 +20,7 @@ import { isChannelItemElementExternal, isReferenceElementExternal, isReferenceLikeElement, + isBooleanJsonSchemaElement, keyMap, ReferenceElement, } from '@swagger-api/apidom-ns-asyncapi-2'; @@ -34,6 +36,24 @@ import Reference from '../../../Reference'; // @ts-ignore const visitAsync = visit[Symbol.for('nodejs.util.promisify.custom')]; +/** + * Predicate for detecting if element was created by merging referencing + * element with particular element identity with a referenced element. + */ +const wasReferencedBy = + (referencingElement: T) => + (element: U) => { + // @ts-ignore + return ( + element.meta.hasKey('ref-referencing-element-id') && + element.meta.get('ref-referencing-element-id').equals(toValue(referencingElement.id)) + ); + }; + +// initialize element identity plugin +const elementIdentity = refractorPluginElementIdentity()(); +elementIdentity.pre(); + const AsyncApi2DereferenceVisitor = stampit({ props: { indirections: [], @@ -55,7 +75,7 @@ const AsyncApi2DereferenceVisitor = stampit({ * Compute full ancestors lineage. * Ancestors are flatten to unwrap all Element instances. */ - const directAncestors = new WeakSet(ancestors.filter(isElement)); + const directAncestors = new Set(ancestors.filter(isElement)); const ancestorsLineage = new AncestorLineage(...this.ancestors, directAncestors); return [ancestorsLineage, directAncestors]; @@ -174,6 +194,24 @@ const AsyncApi2DereferenceVisitor = stampit({ this.indirections.pop(); + // Boolean JSON Schemas + if (isBooleanJsonSchemaElement(referencedElement)) { + const booleanJsonSchemaElement = cloneDeep(referencedElement); + // annotate referenced element with info about original referencing element + booleanJsonSchemaElement.setMetaProperty('ref-fields', { + $ref: toValue(referencingElement.$ref), + }); + // annotate referenced element with info about origin + booleanJsonSchemaElement.setMetaProperty('ref-origin', reference.uri); + // annotate fragment with info about referencing element + booleanJsonSchemaElement.setMetaProperty( + 'ref-referencing-element-id', + cloneDeep(referencingElement.id), + ); + + return booleanJsonSchemaElement; + } + const mergeAndAnnotateReferencedElement = (refedElement: T): T => { const copy = cloneShallow(refedElement); @@ -187,14 +225,24 @@ const AsyncApi2DereferenceVisitor = stampit({ return copy; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } @@ -301,14 +349,24 @@ const AsyncApi2DereferenceVisitor = stampit({ return mergedElement; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } diff --git a/packages/apidom-reference/src/dereference/strategies/openapi-3-0/visitor.ts b/packages/apidom-reference/src/dereference/strategies/openapi-3-0/visitor.ts index 2f2b637c50..b793a6dc17 100644 --- a/packages/apidom-reference/src/dereference/strategies/openapi-3-0/visitor.ts +++ b/packages/apidom-reference/src/dereference/strategies/openapi-3-0/visitor.ts @@ -12,6 +12,7 @@ import { cloneDeep, toValue, isMemberElement, + refractorPluginElementIdentity, } from '@swagger-api/apidom-core'; import { ApiDOMError } from '@swagger-api/apidom-error'; import { evaluate, uriToPointer } from '@swagger-api/apidom-json-pointer'; @@ -41,6 +42,24 @@ import Reference from '../../../Reference'; // @ts-ignore const visitAsync = visit[Symbol.for('nodejs.util.promisify.custom')]; +/** + * Predicate for detecting if element was created by merging referencing + * element with particular element identity with a referenced element. + */ +const wasReferencedBy = + (referencingElement: T) => + (element: U) => { + // @ts-ignore + return ( + element.meta.hasKey('ref-referencing-element-id') && + element.meta.get('ref-referencing-element-id').equals(toValue(referencingElement.id)) + ); + }; + +// initialize element identity plugin +const elementIdentity = refractorPluginElementIdentity()(); +elementIdentity.pre(); + // eslint-disable-next-line @typescript-eslint/naming-convention const OpenApi3_0DereferenceVisitor = stampit({ props: { @@ -97,7 +116,7 @@ const OpenApi3_0DereferenceVisitor = stampit({ * Compute full ancestors lineage. * Ancestors are flatten to unwrap all Element instances. */ - const directAncestors = new WeakSet(ancestors.filter(isElement)); + const directAncestors = new Set(ancestors.filter(isElement)); const ancestorsLineage = new AncestorLineage(...this.ancestors, directAncestors); return [ancestorsLineage, directAncestors]; @@ -196,14 +215,24 @@ const OpenApi3_0DereferenceVisitor = stampit({ return copy; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } @@ -310,14 +339,24 @@ const OpenApi3_0DereferenceVisitor = stampit({ return mergedElement; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } diff --git a/packages/apidom-reference/src/dereference/strategies/openapi-3-1/visitor.ts b/packages/apidom-reference/src/dereference/strategies/openapi-3-1/visitor.ts index 918f7c5af2..f7e74f0a92 100644 --- a/packages/apidom-reference/src/dereference/strategies/openapi-3-1/visitor.ts +++ b/packages/apidom-reference/src/dereference/strategies/openapi-3-1/visitor.ts @@ -13,6 +13,7 @@ import { cloneDeep, toValue, Element, + refractorPluginElementIdentity, } from '@swagger-api/apidom-core'; import { ApiDOMError } from '@swagger-api/apidom-error'; import { evaluate as jsonPointerEvaluate, uriToPointer } from '@swagger-api/apidom-json-pointer'; @@ -52,6 +53,24 @@ import EvaluationJsonSchemaUriError from '../../../errors/EvaluationJsonSchemaUr // @ts-ignore const visitAsync = visit[Symbol.for('nodejs.util.promisify.custom')]; +/** + * Predicate for detecting if element was created by merging referencing + * element with particular element identity with a referenced element. + */ +const wasReferencedBy = + (referencingElement: T) => + (element: U) => { + // @ts-ignore + return ( + element.meta.hasKey('ref-referencing-element-id') && + element.meta.get('ref-referencing-element-id').equals(toValue(referencingElement.id)) + ); + }; + +// initialize element identity plugin +const elementIdentity = refractorPluginElementIdentity()(); +elementIdentity.pre(); + // eslint-disable-next-line @typescript-eslint/naming-convention const OpenApi3_1DereferenceVisitor = stampit({ props: { @@ -111,7 +130,7 @@ const OpenApi3_1DereferenceVisitor = stampit({ * Compute full ancestors lineage. * Ancestors are flatten to unwrap all Element instances. */ - const directAncestors = new WeakSet(ancestors.filter(isElement)); + const directAncestors = new Set(ancestors.filter(isElement)); const ancestorsLineage = new AncestorLineage(...this.ancestors, directAncestors); return [ancestorsLineage, directAncestors]; @@ -228,14 +247,24 @@ const OpenApi3_1DereferenceVisitor = stampit({ return copy; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } @@ -342,14 +371,24 @@ const OpenApi3_1DereferenceVisitor = stampit({ return mergedElement; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } @@ -592,6 +631,12 @@ const OpenApi3_1DereferenceVisitor = stampit({ }); // annotate referenced element with info about origin booleanJsonSchemaElement.setMetaProperty('ref-origin', reference.uri); + // annotate fragment with info about referencing element + booleanJsonSchemaElement.setMetaProperty( + 'ref-referencing-element-id', + cloneDeep(referencingElement.id), + ); + return booleanJsonSchemaElement; } @@ -616,22 +661,36 @@ const OpenApi3_1DereferenceVisitor = stampit({ }); // annotate fragment with info about origin mergedElement.setMetaProperty('ref-origin', reference.uri); + // annotate fragment with info about referencing element + mergedElement.setMetaProperty( + 'ref-referencing-element-id', + cloneDeep(referencingElement.id), + ); return mergedElement; }; + // assigning element identity if not already assigned + if (referencingElement.id.equals('')) { + elementIdentity.visitor.enter(referencingElement); + } + // attempting to create cycle - if (ancestorsLineage.includes(referencedElement)) { + if ( + ancestorsLineage.includes(referencingElement) || + ancestorsLineage.includes(referencedElement) + ) { + const replaceWith = + ancestorsLineage.findItem(wasReferencedBy(referencingElement)) ?? + mergeAndAnnotateReferencedElement(referencedElement); if (isMemberElement(parent)) { - parent.value = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent.value = replaceWith; // eslint-disable-line no-param-reassign } else if (Array.isArray(parent)) { - parent[key] = mergeAndAnnotateReferencedElement(referencedElement); // eslint-disable-line no-param-reassign + parent[key] = replaceWith; // eslint-disable-line no-param-reassign } - return false; } - // transclude referencing element with merged referenced element return mergeAndAnnotateReferencedElement(referencedElement); }, }, diff --git a/packages/apidom-reference/src/dereference/util.ts b/packages/apidom-reference/src/dereference/util.ts index 5472aa60b2..8fb5c3871e 100644 --- a/packages/apidom-reference/src/dereference/util.ts +++ b/packages/apidom-reference/src/dereference/util.ts @@ -1,15 +1,26 @@ -import { Element } from '@swagger-api/apidom-core'; +import { Element, isElement } from '@swagger-api/apidom-core'; // eslint-disable-next-line import/prefer-default-export -export class AncestorLineage extends Array> { +export class AncestorLineage extends Array> { includesCycle(element: T) { return this.filter((ancestors) => ancestors.has(element)).length > 1; } - includes(searchElement: WeakSet | T, fromIndex?: number): boolean { - if (searchElement instanceof WeakSet) { + includes(searchElement: Set | T, fromIndex?: number): boolean { + if (searchElement instanceof Set) { return super.includes(searchElement, fromIndex); } return this.some((ancestors) => ancestors.has(searchElement)); } + + findItem(predicate: (item: T) => boolean): T | undefined { + for (const set of this) { + for (const item of set) { + if (isElement(item) && predicate(item)) { + return item; + } + } + } + return undefined; + } } diff --git a/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/fixtures/cycle-internal-advanced/root.json b/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/fixtures/cycle-internal-advanced/root.json new file mode 100644 index 0000000000..41d5459812 --- /dev/null +++ b/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/fixtures/cycle-internal-advanced/root.json @@ -0,0 +1,35 @@ +{ + "openapi": "3.1.0", + "components": { + "schemas": { + "PlatformMenuTree": { + "$ref": "#/components/schemas/PlatformMenuTreeNode" + }, + "PlatformMenuTreeNode": { + "properties": { + "children": { + "type": "array", + "items": { + "$ref": "#/components/schemas/PlatformMenuTreeNode" + } + }, + "resources": { + "type": "array", + "items": { + "$ref": "#/components/schemas/PlatformMenuTreeResourceNode" + } + } + } + }, + "PlatformMenuTreeResourceNode": { + "properties": { + "id": { + "type": "integer", + "format": "int64", + "description": "ID" + } + } + } + } + } +} diff --git a/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/index.ts b/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/index.ts index 0c1db6aedc..590e319d1a 100644 --- a/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/index.ts +++ b/packages/apidom-reference/test/dereference/strategies/openapi-3-1/schema-object/index.ts @@ -98,11 +98,35 @@ describe('dereference', function () { const dereferenced = await dereference(rootFilePath, { parse: { mediaType: mediaTypes.latest('json') }, }); - const parent = evaluate('/0/components/schemas/User/properties/parent', dereferenced); - const cyclicParent = evaluate( + const parent = evaluate( '/0/components/schemas/User/properties/parent/properties/parent', dereferenced, ); + const cyclicParent = evaluate( + '/0/components/schemas/User/properties/parent/properties/parent/properties/parent/properties/parent', + dereferenced, + ); + + assert.strictEqual(parent, cyclicParent); + }); + }); + + context('given Schema Objects with advanced internal cycles', function () { + const fixturePath = path.join(rootFixturePath, 'cycle-internal-advanced'); + + specify('should dereference', async function () { + const rootFilePath = path.join(fixturePath, 'root.json'); + const dereferenced = await dereference(rootFilePath, { + parse: { mediaType: mediaTypes.latest('json') }, + }); + const parent = evaluate( + '/0/components/schemas/PlatformMenuTree/properties/children/items/properties/children/items', + dereferenced, + ); + const cyclicParent = evaluate( + '/0/components/schemas/PlatformMenuTree/properties/children/items/properties/children/items/properties/children/items', + dereferenced, + ); assert.strictEqual(parent, cyclicParent); }); @@ -134,12 +158,13 @@ describe('dereference', function () { const dereferenced = await dereference(rootFilePath, { parse: { mediaType: mediaTypes.latest('json') }, }); + const parent = evaluate( - '/0/components/schemas/User/properties/profile/properties/parent', + '/0/components/schemas/User/properties/profile/properties/parent/properties/parent', dereferenced, ); const cyclicParent = evaluate( - '/0/components/schemas/User/properties/profile/properties/parent/properties/parent', + '/0/components/schemas/User/properties/profile/properties/parent/properties/parent/properties/parent', dereferenced, ); @@ -155,13 +180,12 @@ describe('dereference', function () { const dereferenced = await dereference(rootFilePath, { parse: { mediaType: mediaTypes.latest('json') }, }); - const user = evaluate( - '/0/components/schemas/User/properties/profile/properties/user', + '/0/components/schemas/User/properties/profile/properties/user/properties/profile', dereferenced, ); const cyclicUserInProfile = evaluate( - '/0/components/schemas/User/properties/profile/properties/user/properties/profile/properties/user', + '/0/components/schemas/User/properties/profile/properties/user/properties/profile/properties/user/properties/profile', dereferenced, );