From f202cb29910bd919497d1545879ab1ff7135fccb Mon Sep 17 00:00:00 2001 From: Vishal Shingala Date: Fri, 25 Sep 2020 01:30:52 +0530 Subject: [PATCH] Added optional support for remote refs resolution --- lib/options.js | 18 +++ lib/parse.js | 28 +++++ lib/schemapack.js | 145 ++++++++++++----------- lib/utils.js | 1 + package-lock.json | 31 +++-- package.json | 2 +- scripts/test-unit.sh | 2 +- test/data/valid_openapi/remote-refs.yaml | 29 +++++ test/system/structure.test.js | 14 +++ test/unit/base.test.js | 33 +++++- test/unit/sanity.test.js | 3 - test/unit/validator.test.js | 3 - 12 files changed, 220 insertions(+), 89 deletions(-) create mode 100644 test/data/valid_openapi/remote-refs.yaml diff --git a/lib/options.js b/lib/options.js index b5b98a82d..030638df1 100644 --- a/lib/options.js +++ b/lib/options.js @@ -107,6 +107,24 @@ module.exports = { external: true, usage: ['CONVERSION'] }, + { + name: 'Resolve remote references', + id: 'resolveRemoteRefs', + type: 'boolean', + default: false, + description: 'Select whether to resolve remote references.', + external: true, + usage: ['CONVERSION'] + }, + { + name: 'Source URL of definition', + id: 'sourceUrl', + type: 'string', + default: '', + description: 'Specify source URL of definition to resolve remote references mentioned in it.', + external: true, + usage: ['CONVERSION'] + }, { name: 'Enable Schema Faking', id: 'schemaFaker', diff --git a/lib/parse.js b/lib/parse.js index 9372aba51..cf6a31026 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -177,12 +177,40 @@ module.exports = { cache: [], externals: [], externalRefs: {}, + ignoreIOErrors: true, rewriteRefs: true, openapi: openapi, files: files }); }, + /** + * Resolves remote and URL $ref from OpenAPI definition based on given options + * + * @param {*} openapi - OpenAPI definition + * @param {*} options - options + * @param {*} cb - callback function + * @return {*} - err if present + */ + resolveRemoteRefs: function (openapi, options, cb) { + if (options.resolveRemoteRefs) { + return resolver.resolve(openapi, options.sourceUrl, { + resolve: true, + externals: [], + externalRefs: {}, + ignoreIOErrors: true, + openapi: openapi + }) + .then(function() { + return cb(null); + }) + .catch(function(err) { + return cb(err); + }); + } + return cb(null); + }, + /** Resolves all OpenAPI file references and returns a single OAS Object * * @param {Object} source Root file path diff --git a/lib/schemapack.js b/lib/schemapack.js index 46da3dce5..397752017 100644 --- a/lib/schemapack.js +++ b/lib/schemapack.js @@ -237,90 +237,99 @@ class SchemaPack { return callback(new OpenApiErr('The schema must be validated before attempting conversion')); } - // this cannot be attempted before validation - componentsAndPaths = { - components: this.openapi.components, - paths: this.openapi.paths - }; + parse.resolveRemoteRefs(this.openapi, options, (err) => { + if (err) { + return callback(null, { + result: false, + reason: err.name + ': ' + err.message + }); + } - // create and sanitize basic spec - openapi = this.openapi; - openapi.servers = _.isEmpty(openapi.servers) ? [{ url: '/' }] : openapi.servers; - openapi.securityDefs = _.get(openapi, 'components.securitySchemes', {}); - openapi.baseUrl = _.get(openapi, 'servers.0.url', '{{baseURL}}'); + // this cannot be attempted before validation + componentsAndPaths = { + components: this.openapi.components, + paths: this.openapi.paths + }; - // TODO: Multiple server variables need to be saved as environments - openapi.baseUrlVariables = _.get(openapi, 'servers.0.variables'); + // create and sanitize basic spec + openapi = this.openapi; + openapi.servers = _.isEmpty(openapi.servers) ? [{ url: '/' }] : openapi.servers; + openapi.securityDefs = _.get(openapi, 'components.securitySchemes', {}); + openapi.baseUrl = _.get(openapi, 'servers.0.url', '{{baseURL}}'); - // Fix {scheme} and {path} vars in the URL to :scheme and :path - openapi.baseUrl = schemaUtils.fixPathVariablesInUrl(openapi.baseUrl); + // TODO: Multiple server variables need to be saved as environments + openapi.baseUrlVariables = _.get(openapi, 'servers.0.variables'); - // Creating a new instance of a Postman collection - // All generated folders and requests will go inside this - generatedStore.collection = new sdk.Collection({ - info: { - name: _.get(openapi, 'info.title', COLLECTION_NAME) - } - }); + // Fix {scheme} and {path} vars in the URL to :scheme and :path + openapi.baseUrl = schemaUtils.fixPathVariablesInUrl(openapi.baseUrl); + + // Creating a new instance of a Postman collection + // All generated folders and requests will go inside this + generatedStore.collection = new sdk.Collection({ + info: { + name: _.get(openapi, 'info.title', COLLECTION_NAME) + } + }); - if (openapi.security) { - authHelper = schemaUtils.getAuthHelper(openapi, openapi.security); - if (authHelper) { - generatedStore.collection.auth = authHelper; + if (openapi.security) { + authHelper = schemaUtils.getAuthHelper(openapi, openapi.security); + if (authHelper) { + generatedStore.collection.auth = authHelper; + } } - } - // ---- Collection Variables ---- - // adding the collection variables for all the necessary root level variables - // and adding them to the collection variables - schemaUtils.convertToPmCollectionVariables( - openapi.baseUrlVariables, - 'baseUrl', - openapi.baseUrl - ).forEach((element) => { - generatedStore.collection.variables.add(element); - }); + // ---- Collection Variables ---- + // adding the collection variables for all the necessary root level variables + // and adding them to the collection variables + schemaUtils.convertToPmCollectionVariables( + openapi.baseUrlVariables, + 'baseUrl', + openapi.baseUrl + ).forEach((element) => { + generatedStore.collection.variables.add(element); + }); - generatedStore.collection.describe(schemaUtils.getCollectionDescription(openapi)); + generatedStore.collection.describe(schemaUtils.getCollectionDescription(openapi)); - // Only change the stack limit if the optimizeConversion option is true - if (options.optimizeConversion) { - // Deciding stack limit based on size of the schema, number of refs and number of paths. - analysis = schemaUtils.analyzeSpec(openapi); + // Only change the stack limit if the optimizeConversion option is true + if (options.optimizeConversion) { + // Deciding stack limit based on size of the schema, number of refs and number of paths. + analysis = schemaUtils.analyzeSpec(openapi); - // Update options on the basis of analysis. - options = schemaUtils.determineOptions(analysis, options); - } + // Update options on the basis of analysis. + options = schemaUtils.determineOptions(analysis, options); + } - // ---- Collection Items ---- - // Adding the collection items from openapi spec based on folderStrategy option - // For tags, All operations are grouped based on respective tags object - // For paths, All operations are grouped based on corresponding paths - try { - if (options.folderStrategy === 'tags') { - schemaUtils.addCollectionItemsUsingTags(openapi, generatedStore, componentsAndPaths, options, schemaCache); + // ---- Collection Items ---- + // Adding the collection items from openapi spec based on folderStrategy option + // For tags, All operations are grouped based on respective tags object + // For paths, All operations are grouped based on corresponding paths + try { + if (options.folderStrategy === 'tags') { + schemaUtils.addCollectionItemsUsingTags(openapi, generatedStore, componentsAndPaths, options, schemaCache); + } + else { + schemaUtils.addCollectionItemsUsingPaths(openapi, generatedStore, componentsAndPaths, options, schemaCache); + } } - else { - schemaUtils.addCollectionItemsUsingPaths(openapi, generatedStore, componentsAndPaths, options, schemaCache); + catch (e) { + return callback(e); } - } - catch (e) { - return callback(e); - } - collectionJSON = generatedStore.collection.toJSON(); + collectionJSON = generatedStore.collection.toJSON(); - // this needs to be deleted as even if version is not specified to sdk, - // it returns a version property with value set as undefined - // this fails validation against v2.1 collection schema definition. - delete collectionJSON.info.version; + // this needs to be deleted as even if version is not specified to sdk, + // it returns a version property with value set as undefined + // this fails validation against v2.1 collection schema definition. + delete collectionJSON.info.version; - return callback(null, { - result: true, - output: [{ - type: 'collection', - data: collectionJSON - }] + return callback(null, { + result: true, + output: [{ + type: 'collection', + data: collectionJSON + }] + }); }); } diff --git a/lib/utils.js b/lib/utils.js index 9ead6e460..671062580 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -22,6 +22,7 @@ module.exports = { // check the type of the value of that option came from the user switch (defaultOptions[id].type) { + case 'string': case 'boolean': if (typeof userOptions[id] === defaultOptions[id].type) { retVal[id] = userOptions[id]; diff --git a/package-lock.json b/package-lock.json index f1f088d6a..114a4ee4b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1799,16 +1799,23 @@ } }, "oas-resolver-browser": { - "version": "2.3.3", - "resolved": "https://registry.npmjs.org/oas-resolver-browser/-/oas-resolver-browser-2.3.3.tgz", - "integrity": "sha512-KvggQ6xU7WlUWRYZKEktR90zJtNCHi1wbTAZuUX6oSfmBSdZo/b26rzfg3w2AdPVwQPRXMga6tqLW3OhbUF0Qg==", + "version": "2.5.0", + "resolved": "https://registry.npmjs.org/oas-resolver-browser/-/oas-resolver-browser-2.5.0.tgz", + "integrity": "sha512-2rzB1x62CfH4vBSxaSqpLuVI0kmLzqbhdm0nzLCH3fFpVZzy7uVNlu0S2nWamQw20PClamvmvj4I/GI1+7reOg==", "requires": { "node-fetch-h2": "^2.3.0", "oas-kit-common": "^1.0.8", "path-browserify": "^1.0.1", - "reftools": "^1.1.1", - "yaml": "^1.8.3", + "reftools": "^1.1.6", + "yaml": "^1.10.0", "yargs": "^15.3.1" + }, + "dependencies": { + "yaml": { + "version": "1.10.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-1.10.0.tgz", + "integrity": "sha512-yr2icI4glYaNG+KWONODapy2/jDdMSDnrONSjblABjD9B4Z5LgiircSt8m8sRZFNi08kG9Sm0uSHtEmP3zaEGg==" + } } }, "once": { @@ -2193,9 +2200,9 @@ } }, "reftools": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/reftools/-/reftools-1.1.1.tgz", - "integrity": "sha512-7ySkzK7YpUeJP16rzJqEXTZ7IrAq/AL/p+wWejD9wdKQOe+mYYVAOB3w5ZTs2eoHfmAidwr/6PcC+q+LzPF/DQ==" + "version": "1.1.6", + "resolved": "https://registry.npmjs.org/reftools/-/reftools-1.1.6.tgz", + "integrity": "sha512-rQfJ025lvPjw9qyQuNPqE+cRs5qVs7BMrZwgRJnmuMcX/8r/eJE8f5/RCunJWViXKHmN5K2DFafYzglLOHE/tw==" }, "regenerator-runtime": { "version": "0.13.5", @@ -2770,9 +2777,9 @@ } }, "yargs": { - "version": "15.3.1", - "resolved": "https://registry.npmjs.org/yargs/-/yargs-15.3.1.tgz", - "integrity": "sha512-92O1HWEjw27sBfgmXiixJWT5hRBp2eobqXicLtPBIDBhYB+1HpwZlXmbW2luivBJHBzki+7VyCLRtAkScbTBQA==", + "version": "15.4.1", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-15.4.1.tgz", + "integrity": "sha512-aePbxDmcYW++PaqBsJ+HYUFwCdv4LVvdnhBy78E57PIor8/OVvhMrADFFEDh8DHDFRv/O9i3lPhsENjO7QX0+A==", "requires": { "cliui": "^6.0.0", "decamelize": "^1.2.0", @@ -2784,7 +2791,7 @@ "string-width": "^4.2.0", "which-module": "^2.0.0", "y18n": "^4.0.0", - "yargs-parser": "^18.1.1" + "yargs-parser": "^18.1.2" }, "dependencies": { "ansi-regex": { diff --git a/package.json b/package.json index 2ec5f3d0f..d4a148d1a 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,7 @@ "commander": "2.20.3", "js-yaml": "3.13.1", "lodash": "4.17.20", - "oas-resolver-browser": "2.3.3", + "oas-resolver-browser": "2.5.0", "path-browserify": "1.0.1", "postman-collection": "3.6.6", "yaml": "1.8.3" diff --git a/scripts/test-unit.sh b/scripts/test-unit.sh index 7140dd831..46e2de8e9 100755 --- a/scripts/test-unit.sh +++ b/scripts/test-unit.sh @@ -38,6 +38,6 @@ fi # run test node --max-old-space-size=2048 ./node_modules/.bin/nyc ${COVERAGE_REPORT} --report-dir ./.coverage \ - -x **/assets/** --print both ./node_modules/.bin/_mocha \ + -x **/assets/** --print both ./node_modules/.bin/_mocha --timeout 15000 \ --reporter ${MOCHA_REPORTER} --reporter-options output=${XUNIT_FILE} \ test/unit/*.test.js --recursive --prof --grep "$1"; diff --git a/test/data/valid_openapi/remote-refs.yaml b/test/data/valid_openapi/remote-refs.yaml new file mode 100644 index 000000000..1e98646e2 --- /dev/null +++ b/test/data/valid_openapi/remote-refs.yaml @@ -0,0 +1,29 @@ +openapi: "3.0.0" +info: + version: 1.0.0 + title: Swagger Petstore + license: + name: MIT +servers: + - url: http://petstore.swagger.io/v1 +paths: + /pets: + get: + summary: List all pets + operationId: listPets + parameters: + - $ref: 'https://raw.githubusercontent.com/postmanlabs/openapi-to-postman/develop/test/data/petstore%20separate%20yaml/spec/parameters.yaml#/tagsParam' + - $ref: 'https://raw.githubusercontent.com/postmanlabs/openapi-to-postman/develop/test/data/petstore%20separate%20yaml/spec/parameters.yaml#/limitsParam' + responses: + '200': + description: An paged array of pets + content: + application/json: + schema: + $ref: "./spec/Pet.yaml" + default: + description: unexpected error + content: + application/json: + schema: + $ref: "./common/Error.yaml" diff --git a/test/system/structure.test.js b/test/system/structure.test.js index fe2c4159f..1ee9fb264 100644 --- a/test/system/structure.test.js +++ b/test/system/structure.test.js @@ -8,6 +8,8 @@ const optionIds = [ 'folderStrategy', 'indentCharacter', 'requestNameSource', + 'resolveRemoteRefs', + 'sourceUrl', 'includeAuthInfoInExample', 'shortValidationErrors', 'validationPropertiesToIgnore', @@ -78,6 +80,18 @@ const optionIds = [ ' If “Fallback” is selected, the request will be named after one of the following schema' + ' values: `description`, `operationid`, `url`.' }, + resolveRemoteRefs: { + name: 'Resolve remote references', + type: 'boolean', + default: false, + description: 'Select whether to resolve remote references.' + }, + sourceUrl: { + name: 'Source URL of definition', + type: 'string', + default: '', + description: 'Specify source URL of definition to resolve remote references mentioned in it.' + }, includeAuthInfoInExample: { name: 'Include auth info in example requests', type: 'boolean', diff --git a/test/unit/base.test.js b/test/unit/base.test.js index f3762be73..cad1b9f4d 100644 --- a/test/unit/base.test.js +++ b/test/unit/base.test.js @@ -40,7 +40,8 @@ describe('CONVERT FUNCTION TESTS ', function() { tagsFolderSpec = path.join(__dirname, VALID_OPENAPI_PATH + '/petstore-detailed.yaml'), securityTestCases = path.join(__dirname, VALID_OPENAPI_PATH + '/security-test-cases.yaml'), emptySecurityTestCase = path.join(__dirname, VALID_OPENAPI_PATH + '/empty-security-test-case.yaml'), - rootUrlServerWithVariables = path.join(__dirname, VALID_OPENAPI_PATH + '/root_url_server_with_variables.json'); + rootUrlServerWithVariables = path.join(__dirname, VALID_OPENAPI_PATH + '/root_url_server_with_variables.json'), + remoteRefs = path.join(__dirname, VALID_OPENAPI_PATH + '/remote-refs.yaml'); it('Should add collection level auth with type as `bearer`' + @@ -905,6 +906,36 @@ describe('CONVERT FUNCTION TESTS ', function() { done(); }); }); + + it('should correctly resolve remote refs when option resolveRemoteRefs enabled and sourceUrl is provided', + function (done) { + var openapi = fs.readFileSync(remoteRefs, 'utf8'), + options = { + resolveRemoteRefs: true, + sourceUrl: 'https://raw.githubusercontent.com/postmanlabs/openapi-to-postman' + + '/develop/test/data/petstore%20separate%20yaml/openapi.yaml' + }; + + Converter.convert({ type: 'string', data: openapi }, options, (err, conversionResult) => { + let collection, + collectionRequest; + + expect(err).to.be.null; + expect(conversionResult.result).to.be.true; + + collection = conversionResult.output[0].data; + expect(collection.item).to.have.lengthOf(1); + + collectionRequest = collection.item[0]; + expect(collectionRequest.request.url.query).to.have.lengthOf(2); + expect(collectionRequest.request.url.query[0].key).to.eql('tags'); + expect(collectionRequest.request.url.query[1].key).to.eql('limit'); + + expect(JSON.parse(collectionRequest.response[0].body)).to.have.keys('id', 'name', 'tag'); + expect(JSON.parse(collectionRequest.response[1].body)).to.have.keys('code', 'message'); + done(); + }); + }); }); describe('requestNameSource option', function() { diff --git a/test/unit/sanity.test.js b/test/unit/sanity.test.js index c934be3f4..ce8c438dc 100644 --- a/test/unit/sanity.test.js +++ b/test/unit/sanity.test.js @@ -15,9 +15,6 @@ describe('Runing validation tests for all files in `valid_openapi`', function () it('should generte a valid collection ' + file, function () { let fileData = fs.readFileSync(path.join(__dirname, VALID_OPENAPI_PATH + '/' + file), 'utf8'); - // Increase timeout for larger schema - this.timeout(15000); - Converter.convert({ data: fileData, type: 'string' }, {}, (err, conversionResult) => { expect(err).to.be.null; expect(conversionResult.result).to.equal(true); diff --git a/test/unit/validator.test.js b/test/unit/validator.test.js index d49ec0f0e..3be888378 100644 --- a/test/unit/validator.test.js +++ b/test/unit/validator.test.js @@ -56,9 +56,6 @@ describe('The validator must validate generated collection from schema against s }, schemaPack = new Converter.SchemaPack({ type: 'string', data: fileData }, options); - // Increase timeout for larger schema - this.timeout(15000); - schemaPack.convert((err, conversionResult) => { expect(err).to.be.null; expect(conversionResult.result).to.equal(true);