Merge pull request #712 from postmanlabs/feature/fix-options-merge

Removed unnecessary options merging with each util function.

Author: VShingala

lib/schemaUtils.js

@@ -16,7 +16,6 @@ const { formatDataPath, checkIsCorrectType, isKnownType } = require('./common/sc
   openApiErr = require('./error.js'),
   ajvValidationError = require('./ajValidation/ajvValidationError'),
   utils = require('./utils.js'),
-  defaultOptions = require('../lib/options.js').getOptions('use'),
   { Node, Trie } = require('./trie.js'),
   { validateSchema } = require('./ajValidation/ajvValidation'),
   inputValidation = require('./30XUtils/inputValidation'),
@@ -523,7 +522,6 @@ module.exports = {
    * @returns {*} combined requestParams from operation and path params.
    */
   getRequestParams: function(operationParam, pathParam, components, options) {
-    options = _.merge({}, defaultOptions, options);
     if (!Array.isArray(operationParam)) {
       operationParam = [];
     }
@@ -579,7 +577,6 @@ module.exports = {
    * @returns {Object} - The final object consists of the tree structure
    */
   generateTrieFromPaths: function (spec, options, fromWebhooks = false) {
-    options = _.merge({}, defaultOptions, options);
     let concreteUtils = getConcreteSchemaUtils({ type: 'json', data: spec }),
       specComponentsAndUtils = {
         concreteUtils
@@ -981,8 +978,6 @@ module.exports = {
    * @returns {Array<object>} returns an array of sdk.Variable
    */
   convertPathVariables: function(type, providedPathVars, commonPathVars, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
-
     var variables = [];
     // converting the base uri path variables, if any
     // commonPathVars is an object for type = root/method
@@ -1036,7 +1031,6 @@ module.exports = {
    */
   convertChildToItemGroup: function (openapi, child, components, options,
     schemaCache, variableStore, fromWebhooks = false) {
-    options = _.merge({}, defaultOptions, options);
 
     var resource = child,
       itemGroup,
@@ -1398,8 +1392,6 @@ module.exports = {
    * @return {object} responseBody, contentType header needed
    */
   convertToPmResponseBody: function(contentObj, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
-
     var responseBody, cTypeHeader, hasComputedType, cTypes,
       isJsonLike = false;
     if (!contentObj) {
@@ -1512,8 +1504,6 @@ module.exports = {
    * @returns {*} first example in the input map type
    */
   getExampleData: function(exampleObj, components, options) {
-    options = _.merge({}, defaultOptions, options);
-
     var example,
       exampleKey;
 
@@ -1556,7 +1546,6 @@ module.exports = {
   convertToPmBodyData: function(bodyObj, requestType, contentType, parameterSourceOption,
     indentCharacter, components, options, schemaCache) {
 
-    options = _.merge({}, defaultOptions, options);
     var bodyData = '',
       schemaFormat = SCHEMA_FORMATS.DEFAULT,
       schemaType,
@@ -1672,7 +1661,6 @@ module.exports = {
    * @returns {array} converted queryparam
    */
   convertToPmQueryParameters: function(param, requestType, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
     var pmParams = [],
       paramValue,
       resolveTo = this.resolveToExampleOrSchema(requestType, options.requestParametersResolution,
@@ -1848,8 +1836,6 @@ module.exports = {
    * @returns {Object} instance of a Postman SDK Header
    */
   convertToPmHeader: function(header, requestType, parameterSource, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
-
     var fakeData,
       convertedHeader,
       reqHeader,
@@ -1891,7 +1877,6 @@ module.exports = {
    * @returns {Object} - Postman requestBody and Content-Type Header
    */
   convertToPmBody: function(requestBody, requestType, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
     var contentObj, // content is required
       bodyData,
       param,
@@ -2175,7 +2160,6 @@ module.exports = {
    * @returns {Object} postman response
    */
   convertToPmResponse: function(response, code, originalRequest, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
     var responseHeaders = [],
       previewLanguage = 'text',
       responseBodyWrapper,
@@ -2287,17 +2271,21 @@ module.exports = {
    * @param {object} components - components defined in the OAS spec. These are used to
    * resolve references while generating params.
    * @param {object} options - a standard list of options that's globally passed around. Check options.js for more.
+   * @param {*} seenRef - References that are repeated. Used to identify circular references.
    * @returns {Object} reference object from the saved components
    * @no-unit-tests
    */
-  getRefObject: function($ref, components, options) {
-    options = _.merge({}, defaultOptions, options);
+  getRefObject: function($ref, components, options, seenRef = {}) {
     var refObj, savedSchema;
 
     if (typeof $ref !== 'string') {
       return { value: `Invalid $ref: ${$ref} was found` };
     }
 
+    if (seenRef[$ref]) {
+      return { value: `Error: "${$ref}" contains circular references in it.` };
+    }
+
     savedSchema = $ref.split('/').slice(1).map((elem) => {
       // https://swagger.io/docs/specification/using-ref#escape
       // since / is the default delimiter, slashes are escaped with ~1
@@ -2328,10 +2316,16 @@ module.exports = {
       return { value: `reference ${$ref} not found in the given specification` };
     }
 
+    // Mark current $ref as seen while processing it further
+    seenRef[$ref] = true;
+
     if (refObj.$ref) {
-      return this.getRefObject(refObj.$ref, components, options);
+      return this.getRefObject(refObj.$ref, components, options, seenRef);
     }
 
+    // Unmark current $ref once resolved
+    seenRef[$ref] = false;
+
     return refObj;
   },
 
@@ -2461,7 +2455,6 @@ module.exports = {
   */
   convertRequestToItem: function(openapi, operationItem, components,
     options, schemaCache, variableStore, fromWebhooks = false) {
-    options = _.merge({}, defaultOptions, options);
     var reqName,
       pathVariables = openapi.baseUrlVariables,
       operation = operationItem.properties,
@@ -2825,7 +2818,6 @@ module.exports = {
   * @returns {*} array of all query params
   */
   convertToPmQueryArray: function(reqParams, requestType, components, options, schemaCache) {
-    options = _.merge({}, defaultOptions, options);
     let requestQueryParams = [];
     _.forEach(reqParams.query, (queryParam) => {
       this.convertToPmQueryParameters(queryParam, requestType, components, options, schemaCache).forEach((pmParam) => {

test/data/valid_openapi/recursiveRefComponents.yaml

@@ -0,0 +1,35 @@
+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
+      tags:
+        - pets
+      parameters:
+        - $ref: "#/components/parameters/rec-param"
+      responses:
+        "401":
+          $ref: "#/components/responses/Unauthorized"
+        default:
+          $ref: "#/components/responses/paramA"
+components:
+  parameters:
+    rec-param:
+      $ref: "#/components/parameters/rec-param"
+  responses:
+    Unauthorized:
+      $ref: "#/components/responses/Unauthorized"
+    paramA:
+      $ref: "#/components/responses/paramB"
+    paramB:
+      $ref: "#/components/responses/paramC"
+    paramC:
+      $ref: "#/components/responses/paramA"

test/unit/base.test.js

@@ -92,7 +92,9 @@ describe('CONVERT FUNCTION TESTS ', function() {
       specWithNullParams =
         path.join(__dirname, VALID_OPENAPI_PATH, '/specWithNullParams.yaml'),
       acceptHeaderExample =
-        path.join(__dirname, VALID_OPENAPI_PATH, '/acceptHeaderExample.json');
+        path.join(__dirname, VALID_OPENAPI_PATH, '/acceptHeaderExample.json'),
+      recursiveRefComponents =
+        path.join(__dirname, VALID_OPENAPI_PATH, '/recursiveRefComponents.yaml');
 
 
     it('Should add collection level auth with type as `bearer`' +
@@ -1888,6 +1890,35 @@ describe('CONVERT FUNCTION TESTS ', function() {
           done();
         });
     });
+
+    it('Should handle recursive references for non-schema $refs correctly', function(done) {
+      var openapi = fs.readFileSync(recursiveRefComponents, 'utf8');
+      Converter.convert({ type: 'string', data: openapi }, {},
+        (err, conversionResult) => {
+          expect(err).to.be.null;
+          expect(conversionResult.result).to.equal(true);
+          expect(conversionResult.output.length).to.equal(1);
+          expect(conversionResult.output[0].type).to.equal('collection');
+          expect(conversionResult.output[0].data).to.have.property('info');
+          expect(conversionResult.output[0].data).to.have.property('item');
+          expect(conversionResult.output[0].data.item.length).to.equal(1);
+
+          const item = conversionResult.output[0].data.item[0];
+
+          expect(item.request.header).to.be.undefined;
+          expect(item.request.url.query).to.be.empty;
+          expect(item.response.length).to.eql(2);
+          expect(item.response[0].header.length).to.eql(1);
+          expect(item.response[0].header[0].key).to.eql('Content-Type');
+          expect(item.response[0].header[0].value).to.eql('text/plain');
+          expect(item.response[0].body).to.be.empty;
+          expect(item.response[1].header.length).to.eql(1);
+          expect(item.response[1].header[0].key).to.eql('Content-Type');
+          expect(item.response[1].header[0].value).to.eql('text/plain');
+          expect(item.response[1].body).to.be.empty;
+          done();
+        });
+    });
   });
 
   describe('Converting swagger 2.0 files', function() {

test/unit/convertV2.test.js

@@ -87,7 +87,9 @@ const expect = require('chai').expect,
   specWithNullParams =
     path.join(__dirname, VALID_OPENAPI_PATH, '/specWithNullParams.yaml'),
   acceptHeaderExample =
-    path.join(__dirname, VALID_OPENAPI_PATH, '/acceptHeaderExample.json');
+    path.join(__dirname, VALID_OPENAPI_PATH, '/acceptHeaderExample.json'),
+  recursiveRefComponents =
+    path.join(__dirname, VALID_OPENAPI_PATH, '/recursiveRefComponents.yaml');
 
 
 describe('The convert v2 Function', function() {
@@ -2179,4 +2181,29 @@ describe('The convert v2 Function', function() {
         done();
       });
   });
+
+  it('Should handle recursive references for non-schema $refs correctly', function(done) {
+    var openapi = fs.readFileSync(recursiveRefComponents, 'utf8');
+    Converter.convertV2({ type: 'string', data: openapi }, {},
+      (err, conversionResult) => {
+        expect(err).to.be.null;
+        expect(conversionResult.result).to.equal(true);
+        expect(conversionResult.output.length).to.equal(1);
+        expect(conversionResult.output[0].type).to.equal('collection');
+        expect(conversionResult.output[0].data).to.have.property('info');
+        expect(conversionResult.output[0].data).to.have.property('item');
+        expect(conversionResult.output[0].data.item.length).to.equal(1);
+
+        const item = conversionResult.output[0].data.item[0].item[0];
+
+        expect(item.request.header).to.be.undefined;
+        expect(item.request.url.query).to.be.empty;
+        expect(item.response.length).to.eql(2);
+        expect(item.response[0].header).to.be.empty;
+        expect(item.response[0].body).to.be.undefined;
+        expect(item.response[1].header).to.be.empty;
+        expect(item.response[1].body).to.be.undefined;
+        done();
+      });
+  });
 });