From ad1baee51c7c554b1b31a8d37681f6db22cad817 Mon Sep 17 00:00:00 2001 From: Luis Tejeda <46000487+LuisTejedaS@users.noreply.github.com> Date: Tue, 21 Jun 2022 16:47:41 -0500 Subject: [PATCH 1/2] fix long paths fix long paths --- lib/bundle.js | 25 +++-- lib/jsonPointer.js | 6 +- lib/utils.js | 31 ++++++ .../toBundleExamples/longPath/client.json | 14 +++ .../toBundleExamples/longPath/expected.json | 94 +++++++++++++++++++ .../data/toBundleExamples/longPath/magic.yaml | 6 ++ test/data/toBundleExamples/longPath/root.yaml | 33 +++++++ .../toBundleExamples/longPath/special.yaml | 6 ++ test/data/toBundleExamples/longPath/user.yaml | 8 ++ .../longPath/userSpecial.yaml | 4 + test/unit/bundle.test.js | 60 +++++++++++- test/unit/util.test.js | 28 ++++++ 12 files changed, 304 insertions(+), 11 deletions(-) create mode 100644 test/data/toBundleExamples/longPath/client.json create mode 100644 test/data/toBundleExamples/longPath/expected.json create mode 100644 test/data/toBundleExamples/longPath/magic.yaml create mode 100644 test/data/toBundleExamples/longPath/root.yaml create mode 100644 test/data/toBundleExamples/longPath/special.yaml create mode 100644 test/data/toBundleExamples/longPath/user.yaml create mode 100644 test/data/toBundleExamples/longPath/userSpecial.yaml diff --git a/lib/bundle.js b/lib/bundle.js index 8522f55..29baf99 100644 --- a/lib/bundle.js +++ b/lib/bundle.js @@ -10,7 +10,8 @@ const { } = require('./jsonPointer'), traverseUtility = require('traverse'), parse = require('./parse.js'), - { ParseError } = require('./common/ParseError'); + { ParseError } = require('./common/ParseError'), + Utils = require('./utils'); let path = require('path'), pathBrowserify = require('path-browserify'), @@ -187,9 +188,10 @@ function setValueInComponents(keyInComponents, components, value, componentsKeys * @param {object} nodeContext - The current node we are processing * @param {object} property - The current property that contains the $ref * @param {string} version - The current version of the spec + * @param {string} commonPathFromData - The common path in the file's paths * @returns {array} The trace to the place where the $ref appears */ -function getTraceFromParentKeyInComponents(nodeContext, property, version) { +function getTraceFromParentKeyInComponents(nodeContext, property, version, commonPathFromData) { const parents = [...nodeContext.parents].reverse(), isArrayKeyRegexp = new RegExp('^\\d$', 'g'), key = nodeContext.key, @@ -202,7 +204,7 @@ function getTraceFromParentKeyInComponents(nodeContext, property, version) { [key, ...parentKeys], nodeTrace = getRootFileTrace(nodeParentsKey), [file, local] = property.split(localPointer), - keyTraceInComponents = getKeyInComponents(nodeTrace, file, local, version); + keyTraceInComponents = getKeyInComponents(nodeTrace, file, local, version, commonPathFromData); return keyTraceInComponents; } @@ -213,9 +215,10 @@ function getTraceFromParentKeyInComponents(nodeContext, property, version) { * @param {Function} pathSolver - function to resolve the Path * @param {string} parentFilename - The parent's filename * @param {object} version - The version of the spec we are bundling + * @param {string} commonPathFromData - The common path in the file's paths * @returns {object} - The references in current node and the new content from the node */ -function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, version) { +function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, version, commonPathFromData) { let referencesInNode = [], nodeReferenceDirectory = {}; traverseUtility(currentNode).forEach(function (property) { @@ -232,7 +235,7 @@ function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, ve ); if (hasReferenceTypeKey) { const tempRef = calculatePath(parentFilename, property.$ref), - nodeTrace = getTraceFromParentKeyInComponents(this, tempRef, version), + nodeTrace = getTraceFromParentKeyInComponents(this, tempRef, version, commonPathFromData), referenceInDocument = getJsonPointerRelationToRoot( tempRef, nodeTrace, @@ -276,9 +279,10 @@ function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, ve * @param {Array} allData - array of { path, content} objects * @param {object} specRoot - root file information * @param {string} version - The current version + * @param {string} commonPathFromData - The common path in the file's paths * @returns {object} - Detect root files result object */ -function getNodeContentAndReferences (currentNode, allData, specRoot, version) { +function getNodeContentAndReferences (currentNode, allData, specRoot, version, commonPathFromData) { let graphAdj = [], missingNodes = [], nodeContent; @@ -295,7 +299,8 @@ function getNodeContentAndReferences (currentNode, allData, specRoot, version) { currentNode.fileName !== specRoot.fileName, removeLocalReferenceFromPath, currentNode.fileName, - version + version, + commonPathFromData ); referencesInNode.forEach((reference) => { @@ -437,9 +442,13 @@ module.exports = { } let algorithm = new DFS(), components = {}, + commonPathFromData = '', rootContextData; + commonPathFromData = Utils.findCommonSubpath(allData.map((fileData) => { + return fileData.fileName; + })); rootContextData = algorithm.traverseAndBundle(specRoot, (currentNode) => { - return getNodeContentAndReferences(currentNode, allData, specRoot, version); + return getNodeContentAndReferences(currentNode, allData, specRoot, version, commonPathFromData); }); components = generateComponentsWrapper(specRoot.parsed.oasObject, version); generateComponentsObject( diff --git a/lib/jsonPointer.js b/lib/jsonPointer.js index dd19507..af5d6d4 100644 --- a/lib/jsonPointer.js +++ b/lib/jsonPointer.js @@ -49,9 +49,10 @@ function jsonPointerDecodeAndReplace(filePathName) { * @param {string} filePathName the filePathName of the file * @param {string} localPath the local path that the pointer will reach * @param {string} version - The current spec version +* @param {string} commonPathFromData - The common path in the file's paths * @returns {Array} - the calculated keys in an array representing each nesting property name */ -function getKeyInComponents(traceFromParent, filePathName, localPath, version) { +function getKeyInComponents(traceFromParent, filePathName, localPath, version, commonPathFromData) { const localPart = localPath ? `${localPointer}${localPath}` : '', { CONTAINERS, @@ -61,9 +62,10 @@ function getKeyInComponents(traceFromParent, filePathName, localPath, version) { ROOT_CONTAINERS_KEYS } = getBundleRulesDataByVersion(version); let result, + newFPN = filePathName.replace(commonPathFromData, ''), trace = [ ...traceFromParent, - jsonPointerDecodeAndReplace(`${filePathName}${localPart}`) + jsonPointerDecodeAndReplace(`${newFPN}${localPart}`) ].reverse(), traceToKey = [], matchFound = false, diff --git a/lib/utils.js b/lib/utils.js index be975fc..1d1bcc9 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -111,5 +111,36 @@ module.exports = { return reqName.substring(0, 255); } return reqName; + }, + + /** + * Finds the common subpath from an array of strings starting from the + * strings starts + * @param {Array} stringArrays - pointer to get the name from + * @returns {string} - string: the common substring + */ + findCommonSubpath(stringArrays) { + if (!stringArrays || stringArrays.length === 0) { + return ''; + } + let cleanStringArrays = [], + res = []; + stringArrays.forEach((cString) => { + if (cString) { + cleanStringArrays.push(cString.split('/')); + } + }); + const asc = cleanStringArrays.sort((a, b) => { return a.length - b.length; }); + for (let segmentIndex = 0; segmentIndex < asc[0].length; segmentIndex++) { + const segment = asc[0][segmentIndex]; + let nonCompliant = asc.find((cString) => { + return cString[segmentIndex] !== segment; + }); + if (nonCompliant) { + break; + } + res.push(segment); + } + return res.join('/'); } }; diff --git a/test/data/toBundleExamples/longPath/client.json b/test/data/toBundleExamples/longPath/client.json new file mode 100644 index 0000000..181ffbd --- /dev/null +++ b/test/data/toBundleExamples/longPath/client.json @@ -0,0 +1,14 @@ +{ + "type": "object", + "properties": { + "idClient": { + "type": "integer" + }, + "clientName": { + "type": "string" + }, + "special": { + "$ref": "../user/special.yaml" + } + } +} diff --git a/test/data/toBundleExamples/longPath/expected.json b/test/data/toBundleExamples/longPath/expected.json new file mode 100644 index 0000000..e895b2f --- /dev/null +++ b/test/data/toBundleExamples/longPath/expected.json @@ -0,0 +1,94 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Sample API", + "description": "Optional multiline or single-line description in [CommonMark](http://commonmark.org/help/) or HTML.", + "version": "0.1.9" + }, + "servers": [ + { + "url": "http://api.example.com/v1", + "description": "Optional server description, e.g. Main (production) server" + }, + { + "url": "http://staging-api.example.com", + "description": "Optional server description, e.g. Internal staging server for testing" + } + ], + "paths": { + "/users": { + "get": { + "summary": "Get a user by ID", + "responses": { + "200": { + "description": "A single user.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/_schemas_user_user.yaml" + } + } + } + } + } + } + }, + "/clients": { + "get": { + "summary": "Get a user by ID", + "responses": { + "200": { + "description": "A single user.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/_schemas_client_client.json" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "_schemas_user_user.yaml": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "userName": { + "type": "string" + }, + "special": { + "$ref": "#/components/schemas/_schemas_user_special.yaml" + } + } + }, + "_schemas_client_client.json": { + "type": "object", + "properties": { + "idClient": { + "type": "integer" + }, + "clientName": { + "type": "string" + }, + "special": { + "$ref": "#/components/schemas/_schemas_user_special.yaml" + } + } + }, + "_schemas_user_special.yaml": { + "type": "object", + "properties": { + "specialUserId": { + "type": "string" + } + } + } + } + } +} \ No newline at end of file diff --git a/test/data/toBundleExamples/longPath/magic.yaml b/test/data/toBundleExamples/longPath/magic.yaml new file mode 100644 index 0000000..6e8fc86 --- /dev/null +++ b/test/data/toBundleExamples/longPath/magic.yaml @@ -0,0 +1,6 @@ +type: object +properties: + magicNumber: + type: integer + magicString: + type: string diff --git a/test/data/toBundleExamples/longPath/root.yaml b/test/data/toBundleExamples/longPath/root.yaml new file mode 100644 index 0000000..37594eb --- /dev/null +++ b/test/data/toBundleExamples/longPath/root.yaml @@ -0,0 +1,33 @@ +openapi: 3.0.0 +info: + title: Sample API + description: Optional multiline or single-line description in [CommonMark](http://commonmark.org/help/) or HTML. + version: 0.1.9 + +servers: + - url: http://api.example.com/v1 + description: Optional server description, e.g. Main (production) server + - url: http://staging-api.example.com + description: Optional server description, e.g. Internal staging server for testing + +paths: + /users: + get: + summary: Get a user by ID + responses: + 200: + description: A single user. + content: + application/json: + schema: + $ref: "./schemas/user/user.yaml" + /clients: + get: + summary: Get a user by ID + responses: + 200: + description: A single user. + content: + application/json: + schema: + $ref: "./schemas/client/client.json" diff --git a/test/data/toBundleExamples/longPath/special.yaml b/test/data/toBundleExamples/longPath/special.yaml new file mode 100644 index 0000000..91e0cc5 --- /dev/null +++ b/test/data/toBundleExamples/longPath/special.yaml @@ -0,0 +1,6 @@ +type: object +properties: + specialClientId: + type: string + magic: + $ref: ./magic.yaml diff --git a/test/data/toBundleExamples/longPath/user.yaml b/test/data/toBundleExamples/longPath/user.yaml new file mode 100644 index 0000000..4704746 --- /dev/null +++ b/test/data/toBundleExamples/longPath/user.yaml @@ -0,0 +1,8 @@ +type: object +properties: + id: + type: integer + userName: + type: string + special: + $ref: ./special.yaml diff --git a/test/data/toBundleExamples/longPath/userSpecial.yaml b/test/data/toBundleExamples/longPath/userSpecial.yaml new file mode 100644 index 0000000..85f8831 --- /dev/null +++ b/test/data/toBundleExamples/longPath/userSpecial.yaml @@ -0,0 +1,4 @@ +type: object +properties: + specialUserId: + type: string diff --git a/test/unit/bundle.test.js b/test/unit/bundle.test.js index 3a9c834..73c1ff6 100644 --- a/test/unit/bundle.test.js +++ b/test/unit/bundle.test.js @@ -37,7 +37,8 @@ let expect = require('chai').expect, additionalProperties = path.join(__dirname, BUNDLES_FOLDER + '/additionalProperties'), compositeOneOf = path.join(__dirname, BUNDLES_FOLDER + '/composite_oneOf'), compositeNot = path.join(__dirname, BUNDLES_FOLDER + '/composite_not'), - compositeAnyOf = path.join(__dirname, BUNDLES_FOLDER + '/composite_anyOf'); + compositeAnyOf = path.join(__dirname, BUNDLES_FOLDER + '/composite_anyOf'), + longPath = path.join(__dirname, BUNDLES_FOLDER + '/longPath'); describe('bundle files method - 3.0', function () { it('Should return bundled file as json - schema_from_response', async function () { @@ -1924,6 +1925,63 @@ describe('bundle files method - 3.0', function () { expect(error.message).to.equal('The provided version "Anything" is not valid'); } }); + + it('Should bundle long paths into shorter ones', async function () { + let contentRootFile = fs.readFileSync(longPath + '/root.yaml', 'utf8'), + client = fs.readFileSync(longPath + '/client.json', 'utf8'), + magic = fs.readFileSync(longPath + '/magic.yaml', 'utf8'), + special = fs.readFileSync(longPath + '/special.yaml', 'utf8'), + userSpecial = fs.readFileSync(longPath + '/userSpecial.yaml', 'utf8'), + user = fs.readFileSync(longPath + '/user.yaml', 'utf8'), + expected = fs.readFileSync(longPath + '/expected.json', 'utf8'), + input = { + type: 'multiFile', + specificationVersion: '3.0', + rootFiles: [ + { + path: '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/root.yaml' + } + ], + data: [ + { + 'content': contentRootFile, + 'path': '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/root.yaml' + }, + { + 'content': client, + 'path': '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/schemas' + + '/client/client.json' + }, + { + 'content': magic, + 'path': '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/schemas' + + '/client/magic.yaml' + }, + { + 'content': special, + 'path': '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/schemas' + + '/client/special.yaml' + }, + { + 'content': userSpecial, + 'path': '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/schemas' + + '/user/special.yaml' + }, + { + 'content': user, + 'path': '/pm/openapi-to-postman/test/data/toBundleExamples/same_ref_different_source/schemas/user/user.yaml' + } + ], + options: {}, + bundleFormat: 'JSON' + }; + const res = await Converter.bundle(input); + + expect(res).to.not.be.empty; + expect(res.result).to.be.true; + expect(res.output.specification.version).to.equal('3.0'); + expect(JSON.stringify(JSON.parse(res.output.data[0].bundledContent), null, 2)).to.be.equal(expected); + }); }); describe('getReferences method when node does not have any reference', function() { diff --git a/test/unit/util.test.js b/test/unit/util.test.js index d1a3948..9e9ef55 100644 --- a/test/unit/util.test.js +++ b/test/unit/util.test.js @@ -2927,3 +2927,31 @@ describe('getPostmanUrlSchemaMatchScore function', function() { expect(endpointMatchScore.pathVars[0]).to.eql({ key: 'spaceId', value: ':spaceId' }); }); }); + +describe('findCommonSubpath method', function () { + it('should return aabb with input ["aa/bb/cc/dd", "aa/bb"]', function () { + const result = Utils.findCommonSubpath(['aa/bb/cc/dd', 'aa/bb']); + expect(result).to.equal('aa/bb'); + }); + + it('should return empty string with undefined input', function () { + const result = Utils.findCommonSubpath(); + expect(result).to.equal(''); + }); + + it('should return empty string with empty array input', function () { + const result = Utils.findCommonSubpath([]); + expect(result).to.equal(''); + }); + + it('should return aabb with input ["aa/bb/cc/dd", "aa/bb", undefined]', function () { + const result = Utils.findCommonSubpath(['aa/bb/cc/dd', 'aa/bb', undefined]); + expect(result).to.equal('aa/bb'); + }); + + it('should return "" with input ["aabbccdd", "aabb", "ccddee"]', function () { + const result = Utils.findCommonSubpath(['aa/bb/cc/dd', 'aa/bb', 'ccddee']); + expect(result).to.equal(''); + }); + +}); From 1754339889196223bfab0436ddae3161a3720cb0 Mon Sep 17 00:00:00 2001 From: Luis Tejeda <46000487+LuisTejedaS@users.noreply.github.com> Date: Wed, 22 Jun 2022 12:21:34 -0500 Subject: [PATCH 2/2] Update jsonPointer.test.js Add required parameter --- test/unit/jsonPointer.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/jsonPointer.test.js b/test/unit/jsonPointer.test.js index 89d2843..63c9cfd 100644 --- a/test/unit/jsonPointer.test.js +++ b/test/unit/jsonPointer.test.js @@ -21,7 +21,7 @@ describe('getKeyInComponents function', function () { }); it('should return ["schemas", "_folder_pet.yaml"] when the filename _folder_pet.yaml', function () { - const result = getKeyInComponents(['path', 'schemas'], '_folder_pet.yaml'); + const result = getKeyInComponents(['path', 'schemas'], '_folder_pet.yaml', '3.0', ''); expect(result).to.be.an('array').with.length(2); expect(result[0]).to.equal('schemas'); expect(result[1]).to.equal('_folder_pet.yaml');