From 57ff6a504309fb62dbcc3d84b2ed56537e5bce32 Mon Sep 17 00:00:00 2001 From: Lukasz Gornicki Date: Fri, 17 Jul 2020 12:35:14 +0200 Subject: [PATCH] feat: support for handling circular references (#94) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fran Méndez --- README.md | 6 ++ lib/models/asyncapi.js | 127 ++++++++++++++++++++------ lib/models/schema.js | 7 ++ lib/parser.js | 76 ++++++++------- lib/utils.js | 1 - test/good/circular-refs-file-ref.yaml | 13 +++ test/good/circular-refs.yaml | 53 +++++++++++ test/models/schema_test.js | 9 ++ test/parse_test.js | 15 +++ 9 files changed, 243 insertions(+), 64 deletions(-) create mode 100644 test/good/circular-refs-file-ref.yaml create mode 100644 test/good/circular-refs.yaml diff --git a/README.md b/README.md index c2b624a73..f6eb8ebe8 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,12 @@ This package throws a bunch of different error types. All errors contain a `type For more information about the `ParserError` class, [check out the documentation](./API.md#new_ParserError_new). +### Circular references + +Parser dereferences all circular references by default. In addition, to simplify interactions with the parser, the following is added: +- `x-parser-circular` property is added to the root of the AsyncAPI document to indicate that the document contains circular references. Tooling developer that doesn't want to support circular references can use the `hasCircular` method to check the document and provide a proper message to the user. +- `x-parser-circular` property is added to every schema where circular reference starts. You should use `isCircular` method on a Schema model like `document.components().schema('RecursiveSelf').properties()['selfChildren'].isCircular()`. + ### Develop 1. Run tests with `npm test` diff --git a/lib/models/asyncapi.js b/lib/models/asyncapi.js index f3f3d73af..e839b5010 100644 --- a/lib/models/asyncapi.js +++ b/lib/models/asyncapi.js @@ -8,6 +8,7 @@ const Tag = require('./tag'); const xParserMessageName = 'x-parser-message-name'; const xParserSchemaId = 'x-parser-schema-id'; +const xParserCircle = 'x-parser-circular'; /** * Implements functions to deal with the AsyncAPI document. @@ -194,6 +195,13 @@ class AsyncAPIDocument extends Base { return schemas; } + + /** + * @returns {boolean} + */ + hasCircular() { + return !!this._json[String(xParserCircle)]; + } } function assignNameToComponentMessages(doc) { @@ -263,55 +271,68 @@ function addNameToKey(messages, number) { }); } +/** + * Function that indicates that a circular reference was detected. + * + * @param {Schema} schema schema that is currently accessed and need to be checked if it is a first time + * @param {Array} seenObjects list of objects that were already seen during recursion + */ +function isCircular(schema, seenObjects) { + return seenObjects.includes(schema.json()); +} + +/** + * Mark schema as being a circular ref + * + * @param {Schema} schema schema that should be marked as circular + */ +function markCircular(schema) { + schema.json()[String(xParserCircle)] = true; +} /** * Recursively go through each schema and execute callback. * - * @param {Schema} schema found. + * @param {Schema} schemaContent schema. * @param {Function} callback(schema) * the function that is called foreach schema found. * schema {Schema}: the found schema. */ -function recursiveSchema(schema, callback) { - if (schema === null) return; - callback(schema); +function recursiveSchema(schemaContent, callback) { + if (schemaContent === null) return; + const seenObj = []; + + return crawl(schemaContent, seenObj, callback); +} + +/** + * Schema crawler + * + * @param {Schema} schemaContent schema. + * @param {Array} seenObj schema elements that crowler went through already. + * @param {Function} callback(schema) + * the function that is called foreach schema found. + * schema {Schema}: the found schema. + */ +function crawl(schema, seenObj, callback) { + if (schema.isCircular() || isCircular(schema, seenObj)) return true; + seenObj.push(schema.json()); + callback(schema); if (schema.type() !== undefined) { switch (schema.type()) { case 'object': - if (schema.additionalProperties() !== undefined && typeof schema.additionalProperties() !== 'boolean') { - const additionalSchema = schema.additionalProperties(); - recursiveSchema(additionalSchema, callback); - } - if (schema.properties() !== null) { - const props = schema.properties(); - for (const [, propertySchema] of Object.entries(props)) { - recursiveSchema(propertySchema, callback); - } - } + recursiveSchemaObject(schema, seenObj, callback); break; case 'array': - if (schema.additionalItems() !== undefined) { - const additionalArrayItems = schema.additionalItems(); - recursiveSchema(additionalArrayItems, callback); - } - - if (schema.items() !== null) { - if (Array.isArray(schema.items())) { - schema.items().forEach(arraySchema => { - recursiveSchema(arraySchema, callback); - }); - } else { - recursiveSchema(schema.items(), callback); - } - } + recursiveSchemaArray(schema, seenObj, callback); break; } } else { - //check for allOf, oneOf, anyOf + //check for allOf, oneOf, anyOf const checkCombiningSchemas = (combineArray) => { if (combineArray !== null && combineArray.length > 0) { combineArray.forEach(combineSchema => { - recursiveSchema(combineSchema, callback); ; + if (crawl(combineSchema, seenObj, callback)) markCircular(schema); }); } }; @@ -369,4 +390,50 @@ function assignIdToAnonymousSchemas(doc) { schemaDocument(doc, callback); } +/** + * Recursively go through schema of object type and execute callback. + * + * @param {Schema} schema Object type. + * @param {Array} seenObj schema elements that crawler went through already. + * @param {Function} callback(schema) + * the function that is called foreach schema found. + * schema {Schema}: the found schema. + */ +function recursiveSchemaObject(schema, seenObj, callback) { + if (schema.additionalProperties() !== undefined && typeof schema.additionalProperties() !== 'boolean') { + const additionalSchema = schema.additionalProperties(); + if (crawl(additionalSchema, seenObj, callback)) markCircular(schema); + } + if (schema.properties() !== null) { + const props = schema.properties(); + for (const [, propertySchema] of Object.entries(props)) { + if (crawl(propertySchema, seenObj, callback)) markCircular(schema); + } + } +} + +/** + * Recursively go through schema of array type and execute callback. + * + * @param {Schema} schema Array type. + * @param {Array} seenObj schema elements that crowler went through already. + * @param {Function} callback(schema) + * the function that is called foreach schema found. + * schema {Schema}: the found schema. + */ +function recursiveSchemaArray(schema, seenObj, callback) { + if (schema.additionalItems() !== undefined) { + const additionalArrayItems = schema.additionalItems(); + if (crawl(additionalArrayItems, seenObj, callback)) markCircular(schema); + } + + if (schema.items() !== null) { + if (Array.isArray(schema.items())) { + schema.items().forEach(arraySchema => { + if (crawl(arraySchema, seenObj, callback)) markCircular(schema); + }); + } else if (crawl(schema.items(), seenObj, callback)) markCircular(schema); + } +} + module.exports = addExtensions(AsyncAPIDocument); diff --git a/lib/models/schema.js b/lib/models/schema.js index f69a97fae..9a4ef348b 100644 --- a/lib/models/schema.js +++ b/lib/models/schema.js @@ -361,6 +361,13 @@ class Schema extends Base { examples() { return this._json.examples; } + + /** + * @returns {boolean} + */ + isCircular() { + return !!this.ext('x-parser-circular'); + } } module.exports = addExtensions(Schema); diff --git a/lib/parser.js b/lib/parser.js index d7b1a2a4a..394b0f938 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -14,6 +14,8 @@ const OPERATIONS = ['publish', 'subscribe']; //the only security types that can have a non empty array in the server security item const SPECIAL_SECURITY_TYPES = ['oauth2', 'openIdConnect']; const PARSERS = {}; +const xParserCircle = 'x-parser-circular'; +const refParser = new $RefParser; /** * @module Parser @@ -35,7 +37,6 @@ module.exports = { * @param {String} [options.path] Path to the AsyncAPI document. It will be used to resolve relative references. Defaults to current working dir. * @param {Object} [options.parse] Options object to pass to {@link https://apidevtools.org/json-schema-ref-parser/docs/options.html|json-schema-ref-parser}. * @param {Object} [options.resolve] Options object to pass to {@link https://apidevtools.org/json-schema-ref-parser/docs/options.html|json-schema-ref-parser}. - * @param {Object} [options.dereference] Options object to pass to {@link https://apidevtools.org/json-schema-ref-parser/docs/options.html|json-schema-ref-parser}. * @param {Object} [options.applyTraits=true] Whether to resolve and apply traits or not. * @returns {Promise} The parsed AsyncAPI document. */ @@ -76,39 +77,20 @@ async function parse(asyncapiYAMLorJSON, options = {}) { if (options.applyTraits === undefined) options.applyTraits = true; - try { - parsedJSON = await $RefParser.dereference(options.path, parsedJSON, { - parse: options.parse, - resolve: options.resolve, - dereference: options.dereference, - }); - } catch (err) { - throw new ParserError({ - type: 'dereference-error', - title: err.message, - parsedJSON, - refs: findRefs(parsedJSON, err.originalPath || err.path, options.path, initialFormat, asyncapiYAMLorJSON), - }); - } - } catch (e) { - if (e instanceof ParserError) throw e; - throw new ParserError({ - type: 'unexpected-error', - title: e.message, - parsedJSON, - }); - } - - const ajv = new Ajv({ - jsonPointers: true, - allErrors: true, - schemaId: 'id', - logger: false, - }); + //because of Ajv lacks support for circular refs, parser should not resolve them before Ajv validation and first needs to ignore them and leave circular $refs to successfully validate the document + //this is done pair to advice from Ajv creator https://github.com/ajv-validator/ajv/issues/1122#issuecomment-559378449 + //later we perform full dereference of circular refs if they occure + await dereference(parsedJSON, initialFormat, asyncapiYAMLorJSON, { ...options, dereference: { circular: 'ignore' } }); - ajv.addMetaSchema(require('ajv/lib/refs/json-schema-draft-04.json')); - - try { + const ajv = new Ajv({ + jsonPointers: true, + allErrors: true, + schemaId: 'id', + logger: false, + }); + + ajv.addMetaSchema(require('ajv/lib/refs/json-schema-draft-04.json')); + const validate = ajv.compile(asyncapi[parsedJSON.asyncapi]); const valid = validate(parsedJSON); if (!valid) throw new ParserError({ @@ -119,6 +101,7 @@ async function parse(asyncapiYAMLorJSON, options = {}) { }); await customDocumentOperations(parsedJSON, asyncapiYAMLorJSON, initialFormat, options); + if (refParser.$refs.circular) await handleCircularRefs(parsedJSON, initialFormat, asyncapiYAMLorJSON, options); } catch (e) { if (e instanceof ParserError) throw e; throw new ParserError({ @@ -150,6 +133,33 @@ function parseFromUrl(url, fetchOptions = {}, options) { }); } +async function dereference(parsedJSON, initialFormat, asyncapiYAMLorJSON, options) { + try { + return await refParser.dereference(options.path, parsedJSON, { + parse: options.parse, + resolve: options.resolve, + dereference: options.dereference, + }); + } catch (err) { + throw new ParserError({ + type: 'dereference-error', + title: err.message, + parsedJSON, + refs: findRefs(parsedJSON, err.originalPath || err.path, options.path, initialFormat, asyncapiYAMLorJSON), + }); + } +} + +/* + * In case of circular refs, this function dereferences the spec again to dereference circular dependencies + * Last step is to discover those and mark properly with information whey they are pointing +*/ +async function handleCircularRefs(parsedJSON, initialFormat, asyncapiYAMLorJSON, options) { + await dereference(parsedJSON, initialFormat, asyncapiYAMLorJSON, { ...options, dereference: { circular: true } }); + //mark entire document as containing circular references + parsedJSON[String(xParserCircle)] = true; +} + async function customDocumentOperations(js, asyncapiYAMLorJSON, initialFormat, options) { validateServerVariables(js, asyncapiYAMLorJSON, initialFormat); validateServerSecurity(js, asyncapiYAMLorJSON, initialFormat, SPECIAL_SECURITY_TYPES); diff --git a/lib/utils.js b/lib/utils.js index c8f804c8e..07f357deb 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -135,7 +135,6 @@ utils.toJS = (asyncapiYAMLorJSON) => { title: 'The AsyncAPI document has to be either a string or a JS object.', }); } - if (asyncapiYAMLorJSON.trimLeft().startsWith('{')) { try { return { diff --git a/test/good/circular-refs-file-ref.yaml b/test/good/circular-refs-file-ref.yaml new file mode 100644 index 000000000..91fed68cd --- /dev/null +++ b/test/good/circular-refs-file-ref.yaml @@ -0,0 +1,13 @@ +ExternalFile: + type: object + properties: + testExt: + $ref: '#/YetAnother' +YetAnother: + type: object + properties: + children: + type: array + items: + $ref: '#/ExternalFile' + \ No newline at end of file diff --git a/test/good/circular-refs.yaml b/test/good/circular-refs.yaml new file mode 100644 index 000000000..402ca4140 --- /dev/null +++ b/test/good/circular-refs.yaml @@ -0,0 +1,53 @@ +asyncapi: 2.0.0 +info: + title: My Circular API + version: '1.0.0' +channels: + recursive: + subscribe: + message: + payload: + $ref: '#/components/schemas/RecursiveSelf' + external/file: + publish: + message: + payload: + $ref: './good/circular-refs-file-ref.yaml#/ExternalFile' + nonRecursive: + subscribe: + message: + payload: + $ref: '#/components/schemas/NonRecursive' +components: + schemas: + NonRecursive: + type: object + properties: + child: + $ref: '#/components/schemas/NonRecursiveChild' + NonRecursiveChild: + type: object + properties: + value: + type: string + RecursiveSelf: + type: object + properties: + selfChildren: + type: array + items: + $ref: '#/components/schemas/RecursiveSelf' + selfSomething: + type: object + properties: + test: + $ref: '#/components/schemas/RecursiveAncestor' + RecursiveAncestor: + type: object + properties: + ancestorChildren: + type: array + items: + $ref: '#/components/schemas/RecursiveSelf' + ancestorSomething: + type: string \ No newline at end of file diff --git a/test/models/schema_test.js b/test/models/schema_test.js index e08e72e74..549057df6 100644 --- a/test/models/schema_test.js +++ b/test/models/schema_test.js @@ -541,4 +541,13 @@ describe('Schema', function() { expect(d.examples()).to.be.equal(doc.examples); }); }); + + describe('#isCircular()', function() { + it('should return a boolean', function() { + const doc = { 'x-parser-circular': true}; + const d = new Schema(doc); + expect(typeof d.isCircular()).to.be.equal('boolean'); + expect(d.isCircular()).to.be.equal(doc['x-parser-circular']); + }); + }); }); diff --git a/test/parse_test.js b/test/parse_test.js index 37abad2dd..fc5ddc90e 100644 --- a/test/parse_test.js +++ b/test/parse_test.js @@ -11,6 +11,7 @@ const expect = chai.expect; const invalidYAML = fs.readFileSync(path.resolve(__dirname, './wrong/malformed-asyncapi.yaml'), 'utf8'); const inputYAML = fs.readFileSync(path.resolve(__dirname, './good/asyncapi.yaml'), 'utf8'); +const inputYAMLCircular = fs.readFileSync(path.resolve(__dirname, './good/circular-refs.yaml'), 'utf8'); const inputJSON = fs.readFileSync(path.resolve(__dirname, './good/asyncapi.json'), 'utf8'); const invalidAsyncapiYAML = fs.readFileSync(path.resolve(__dirname, './wrong/invalid-asyncapi.yaml'), 'utf8'); const invalidAsyncpiJSON = fs.readFileSync(path.resolve(__dirname, './wrong/invalid-asyncapi.json'), 'utf8'); @@ -377,6 +378,20 @@ describe('parse()', function() { await parser.parse(() => {}); }, type, message); }); + + it('should properly mark circular references', async function() { + const result = await parser.parse(inputYAMLCircular, { path: __filename }); + expect(result.components().schema('RecursiveAncestor').properties()['ancestorChildren'].isCircular()).to.equal(true); + expect(result.components().schema('RecursiveSelf').properties()['selfSomething'].properties()['test'].properties()['ancestorChildren'].isCircular()).to.equal(true); + expect(result.channel('external/file').publish().messages()[0].payload().properties()['testExt'].properties()['children'].isCircular()).to.equal(true); + //not testing on a model level as required xParserCircle value is added before model construction so we need to test through calling parser function + expect(result.hasCircular()).to.equal(true); + //we want false here, even though this schema has some circular refs in some props, it is not circular, but just specific items + expect(result.components().schema('RecursiveSelf').isCircular()).to.equal(false); + expect(result.components().schema('NonRecursive').isCircular()).to.equal(false); + expect(result.components().schema('RecursiveSelf').properties()['selfChildren'].isCircular()).to.equal(true); + expect(result.components().schema('NonRecursive').properties()['child'].isCircular()).to.equal(false); + }); }); it('should not apply traits', async function() {