Refactoring, comments, moving trie lib to a separate file

lib/trie.js

@@ -0,0 +1,38 @@
+/**
+ * Class for the node of the tree containing the folders
+ * @param {object} options - Contains details about the folder/collection node
+ * @returns {void}
+ */
+function Node (options) {
+  // human-readable name
+  this.name = options ? options.name : '/';
+
+  // number of requests in the sub-trie of this node
+  this.requestCount = options ? options.requestCount : 0;
+
+  this.type = options ? options.type : 'item';
+
+  // stores all direct folder descendants of this node
+  this.children = {};
+
+  this.requests = options ? options.requests : []; // request will be an array of objects
+
+  this.addChildren = function (child, value) {
+    this.children[child] = value;
+  };
+
+  this.addMethod = function (method) {
+    this.requests.push(method);
+  };
+}
+
+class Trie {
+  constructor(node) {
+    this.root = node;
+  }
+}
+
+module.exports = {
+  Trie: Trie,
+  Node: Node
+};

lib/util.js

@@ -1,11 +1,11 @@
-var sdk = require('postman-collection'),
+const sdk = require('postman-collection'),
   schemaFaker = require('../assets/json-schema-faker.js'),
   parse = require('./parse.js'),
   deref = require('./deref.js'),
   _ = require('lodash'),
-  openApiErr = require('./error.js');
+  openApiErr = require('./error.js'),
-
+  { Node, Trie } = require('./trie.js'),
-const URLENCODED = 'application/x-www-form-urlencoded',
+  URLENCODED = 'application/x-www-form-urlencoded',
   APP_JSON = 'application/json',
   APP_JS = 'application/javascript',
   APP_XML = 'application/xml',
@@ -13,6 +13,9 @@ const URLENCODED = 'application/x-www-form-urlencoded',
   TEXT_PLAIN = 'text/plain',
   TEXT_HTML = 'text/html',
   FORM_DATA = 'multipart/form-data',
+
+  // These are the methods supported in the PathItem schema
+  // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#pathItemObject
   METHODS = ['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace'];
 
 // See https://github.com/json-schema-faker/json-schema-faker/tree/master/docs#available-options
@@ -62,47 +65,12 @@ function safeSchemaFaker(oldSchema, components) {
   }
 }
 
-/**
- * Class for the node of the tree containing the folders
- * @param {object} options - Contains details about the folder/collection node
- * @returns {void}
- */
-function Node (options) {
-  // human-readable name
-  this.name = options ? options.name : '/';
-
-  // number of requests in the sub-trie of this node
-  this.requestCount = options ? options.requestCount : 0;
-
-  this.type = options ? options.type : 'item';
-
-  // stores all direct folder descendants of this node
-  this.children = {};
-
-  this.requests = options ? options.requests : []; // request will be an array of objects
-
-  this.addChildren = function (child, value) {
-    this.children[child] = value;
-  };
-
-  this.addMethod = function (method) {
-    this.requests.push(method);
-  };
-}
-
-class Trie {
-  constructor(node) {
-    this.root = node;
-  }
-}
-
 module.exports = {
   // list of predefined schemas in components
   components: {},
   options: {},
   safeSchemaFaker: safeSchemaFaker,
 
-
   /**
   * Changes the {} around scheme and path variables to :variable
   * @param {string} url - the url string
@@ -129,7 +97,8 @@ module.exports = {
   },
 
   /**
-   * Adds the neccessary server variables to the collection
+   * Converts the neccessary server variables to the
+   * something that can be added to the collection
    * TODO: Figure out better description
    * @param {object} serverVariables - Object containing the server variables at the root/path-item level
    * @param {string} level - root / path-item level
@@ -161,7 +130,7 @@ module.exports = {
   /**
    * Parses an OAS string/object as a YAML or JSON
    * @param {YAML/JSON} openApiSpec - The OAS 3.x specification specified in either YAML or JSON
-   * @returns {Object} - Contains the parsed JSON-version of the OAS spec
+   * @returns {Object} - Contains the parsed JSON-version of the OAS spec, or an error
    */
   parseSpec: function (openApiSpec) {
     var openApiObj = openApiSpec,
@@ -268,7 +237,6 @@ module.exports = {
       currentPathObject = '',
       commonParams = '',
       collectionVariables = {},
-      method,
       operationItem,
       pathLevelServers = '',
       pathLength,
@@ -282,11 +250,14 @@ module.exports = {
       trie = new Trie(new Node({
         name: '/'
       })),
-      // returns a list of methods supported at each path
+
+      // returns a list of methods supported at each pathItem
+      // some pathItem props are not methods
+      // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#pathItemObject
       getPathMethods = function(pathKeys) {
         var methods = [];
         pathKeys.forEach(function(element) {
-          if (METHODS.indexOf(element) !== -1) {
+          if (METHODS.includes(element)) {
             methods.push(element);
           }
         });
@@ -302,18 +273,19 @@ module.exports = {
           path = path.substring(1);
         }
 
-        // split the path into indiv. segments for trie-generation
+        // split the path into indiv. segments for trie generation
         currentPath = path.split('/');
         pathLength = currentPath.length;
 
-        // stores names of methods in path to pathMethods, if they're present in METHODS.
+        // get method names available for this path
         pathMethods = getPathMethods(Object.keys(currentPathObject));
+
         // the number of requests under this node
         currentPathRequestCount = pathMethods.length;
         currentNode = trie.root;
 
         // adding children for the nodes in the trie
-        // starts at the top-level and does a DFS
+        // start at the top-level and do a DFS
         for (i = 0; i < pathLength; i++) {
           if (!currentNode.children[currentPath[i]]) {
             // if the currentPath doesn't already exist at this node,
@@ -334,7 +306,6 @@ module.exports = {
         // extracting common parameters for all the methods in the current path item
         if (currentPathObject.hasOwnProperty('parameters')) {
           commonParams = currentPathObject.parameters;
-          delete currentPathObject.parameters;
         }
 
         // storing common path/collection vars from the server object at the path item level
@@ -344,10 +315,9 @@ module.exports = {
           delete currentPathObject.servers;
         }
 
-        // if this path has direct methods under it
+        // add methods to node
-        for (method in currentPathObject) {
+        // eslint-disable-next-line no-loop-func
-          // check for valid methods
+        _.each(pathMethods, (method) => {
-          if (currentPathObject.hasOwnProperty(method) && METHODS.indexOf(method) !== -1) {
           // base operationItem
           operationItem = currentPathObject[method];
           // params - these contain path/header/body params
@@ -355,7 +325,6 @@ module.exports = {
           // auth info - local security object takes precedence over the parent object
           operationItem.security = operationItem.security || spec.security;
           summary = operationItem.summary || operationItem.description;
-
           currentNode.addMethod({
             name: summary,
             method: method,
@@ -364,8 +333,7 @@ module.exports = {
             type: 'item',
             servers: pathLevelServers || undefined
           });
-          }
+        });
-        }
         pathLevelServers = undefined;
         commonParams = [];
       }
@@ -383,7 +351,7 @@ module.exports = {
    * method: request(operation)-level, root: spec-level,  param: url-level
    * @param {Array<object>} providedPathVars - Array of path variables
    * @param {object} commonPathVariables - Object of path variables taken from the specification
-   * @returns {Array<object>} returns array of sdk.Variable
+   * @returns {Array<object>} returns an array of sdk.Variable
    */
   convertPathVariables: function(type, providedPathVars, commonPathVariables) {
     var variables = providedPathVars;
@@ -438,17 +406,17 @@ module.exports = {
    */
   insertSpacesInName: function (string) {
     if (!string) {
-      return null;
+      return '';
     }
 
     return string
       .replace(/([a-z])([A-Z])/g, '$1 $2') // convert createUser to create User
       .replace(/([A-Z])([A-Z][a-z])/g, '$1 $2') // convert NASAMission to NASA Mission
-      .replace(/(_+)([A-Za-z0-9])/g, ' $2'); // convert create_user to create user
+      .replace(/(_+)([a-zA-Z0-9])/g, ' $2'); // convert create_user to create user
   },
 
   /**
-   * convert childItem from openAPI to Postman itemGroup if requestCount(no of requests inside childitem)>1
+   * convert childItem from OpenAPI to Postman itemGroup if requestCount(no of requests inside childitem)>1
    * otherwise return postman request
    * @param {*} openapi object with root-level data like pathVariables baseurl
    * @param {*} child object is of type itemGroup or request
@@ -462,28 +430,31 @@ module.exports = {
       i,
       requestCount;
 
+    // it's a folder
     if (resource.type === 'item-group') {
       // 3 options:
 
       // 1. folder with more than one request in its subtree
+      // (immediate children or otherwise)
       if (resource.requestCount > 1) {
         // only return a Postman folder if this folder has>1 requests in its subtree
-        // otherwise we can end up with 10 levels of folders
+        // otherwise we can end up with 10 levels of folders with 1 request in the end
-        // with 1 request in the end
         itemGroup = new sdk.ItemGroup({
           name: this.insertSpacesInName(resource.name)
           // TODO: have to add auth here (but first, auth to be put into the openapi tree)
         });
 
+        // recurse over child leaf nodes
+        // and add as children to this folder
         for (i = 0, requestCount = resource.requests.length; i < requestCount; i++) {
-          // recurse over child requests
           itemGroup.items.add(
             this.convertChildToItemGroup(openapi, resource.requests[i])
           );
         }
 
+        // recurse over child folders
+        // and add as child folders to this folder
         for (subChild in resource.children) {
-          // recurese over child folders
           if (resource.children.hasOwnProperty(subChild) && resource.children[subChild].requestCount > 0) {
             itemGroup.items.add(
               this.convertChildToItemGroup(openapi, resource.children[subChild])
@@ -494,13 +465,13 @@ module.exports = {
         return itemGroup;
       }
 
-      // 2. it's a folder with just one request in its subtree
+      // 2. it has only 1 direct request of its own
-      // recurse DFS-ly to find that one request
       if (resource.requests.length === 1) {
         return this.convertChildToItemGroup(openapi, resource.requests[0]);
       }
 
-      // 3. it has only request
+      // 3. it's a folder that has no child request
+      // but one request somewhere in its child folders
       for (subChild in resource.children) {
         if (resource.children.hasOwnProperty(subChild) && resource.children[subChild].requestCount === 1) {
           return this.convertChildToItemGroup(openapi, resource.children[subChild]);
@@ -508,6 +479,7 @@ module.exports = {
       }
     }
 
+    // it's a request item
     item = this.convertRequestToItem(openapi, resource);
     return item;
   },
@@ -516,7 +488,9 @@ module.exports = {
    * Gets helper object based on the root spec and the operation.security object
    * @param {*} openapi
    * @param {Array<object>} securitySet
-   * @returns {object} The authHelper to use while constructing the Postman Request
+   * @returns {object} The authHelper to use while constructing the Postman Request. This is
+   * not directly supported in the SDK - the caller needs to determine the header/body based on the return
+   * value
    */
   getAuthHelper: function(openapi, securitySet) {
     var securityDef,
@@ -532,7 +506,10 @@ module.exports = {
 
     securitySet.forEach((security) => {
       securityDef = openapi.securityDefs[Object.keys(security)[0]];
-      if (securityDef.type === 'http') {
+      if (!securityDef) {
+        return false;
+      }
+      else if (securityDef.type === 'http') {
         helper = {
           type: securityDef.scheme
         };
@@ -554,12 +531,14 @@ module.exports = {
   },
 
   /**
-   * conerts into POSTMAN Response body
+   * Converts a 'content' object into Postman response body. Any content-type header determined
-   * @param {*} contentObj response content
+   * from the body is returned as well
-   * @return {string} responseBody
+   * @param {*} contentObj response content - this is the content property of the response body
+   * https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#responseObject
+   * @return {object} responseBody, contentType header needed
    */
   convertToPmResponseBody: function(contentObj) {
-    var responseBody, cTypeHeader;
+    var responseBody, cTypeHeader, hasComputedType, cTypes;
     if (!contentObj) {
       return {
         contentTypeHeader: null,
@@ -567,32 +546,44 @@ module.exports = {
       };
     }
 
-    if (contentObj[APP_JSON]) {
+    _.each([APP_JSON, APP_XML], (supportedCType) => {
-      cTypeHeader = APP_JSON;
+      // these are the content-types that we'd prefer to generate a body for
-      responseBody = this.convertToPmBodyData(contentObj[APP_JSON]);
+      // in this order
+      if (contentObj[supportedCType]) {
+        cTypeHeader = supportedCType;
+        hasComputedType = true;
+        return false;
       }
-    else if (contentObj[APP_XML]) {
+    });
-      cTypeHeader = APP_XML;
+
-      responseBody = this.convertToPmBodyData(contentObj[APP_XML]);
+    // if no JSON or XML, take whatever we have
+    if (!hasComputedType) {
+      cTypes = Object.keys(contentObj);
+      if (cTypes.length > 0) {
+        cTypeHeader = cTypes[0];
+        hasComputedType = true;
       }
-    else if (contentObj[APP_JS]) {
+      else {
-      cTypeHeader = APP_JS;
+        // just an empty object - can't convert anything
-      responseBody = this.convertToPmBodyData(contentObj[APP_JS]);
+        return {
+          contentTypeHeader: null,
+          responseBody: ''
+        };
       }
-    else if (contentObj[TEXT_PLAIN]) {
-      cTypeHeader = TEXT_PLAIN;
-      responseBody = this.convertToPmBodyData(contentObj[TEXT_PLAIN]);
     }
-    // TODO: Need to figure out the else case
+
+    responseBody = this.convertToPmBodyData(contentObj[cTypeHeader]);
 
     return {
       contentTypeHeader: cTypeHeader,
+      // what if it's not JSON?
+      // TODO: The indentation must be an option
       responseBody: JSON.stringify(responseBody, null, 4)
     };
   },
 
   /**
-   *  map for creating parameters specific for a request
+   * Create parameters specific for a request
    * @param {*} localParams parameters array
    * @returns {Object} with three arrays of query, header and path as keys.
    */
@@ -648,23 +639,25 @@ module.exports = {
    * @param {*} bodyObj is MediaTypeObject
    * @returns {*} postman body data
    */
+  // TODO: We also need to accept the content type
+  // and generate the body accordingly
+  // right now, even if the content-type was XML, we'll generate
+  // a JSON example/schema
   convertToPmBodyData: function(bodyObj) {
     var bodyData = '';
 
-    // This part is to remove format:binary from any string-type properties
+    if (bodyObj.example) {
-    // will cause schemaFaker to crash if left untreated
-    if (bodyObj.hasOwnProperty('example')) {
       bodyData = bodyObj.example;
       // return example value if present else example is returned
       if (bodyData.value) {
         bodyData = bodyData.value;
       }
     }
-    else if (bodyObj.hasOwnProperty('examples')) {
+    else if (bodyObj.examples) {
-      bodyData = this.getExampleData(bodyObj.examples);
       // take one of the examples as the body and not all
+      bodyData = this.getExampleData(bodyObj.examples);
     }
-    else if (bodyObj.hasOwnProperty('schema')) {
+    else if (bodyObj.schema) {
       if (bodyObj.schema.hasOwnProperty('$ref')) {
         bodyObj.schema = this.getRefObject(bodyObj.schema.$ref);
       }
@@ -704,13 +697,27 @@ module.exports = {
     return pmParams;
   },
 
+  /**
+   * Returns an array of parameters
+   * Handles array/object/string param types
+   * @param {*} param - the param object, as defined in
+   * https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#parameterObject
+   * @param {any} paramValue
+   * @returns {array} parameters. One param with type=array might lead to multiple params
+   * in the return value
+   * The styles are documented at
+   * https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#style-values
+   */
   convertParamsWithStyle: function(param, paramValue) {
     var paramType = param.schema.type,
       paramNameArray,
       pmParams = [],
+      // converts: {a: [1,2,3]} to:
+      // [{key: a, val: 1}, {key: a, val: 2}, {key: a, val: 3}] if explodeFlag
+      // else to [{key:a, val: 1,2,3}]
       handleExplode = (explodeFlag, paramValue, paramName) => {
         if (explodeFlag) {
-          paramNameArray = Array.from(Array(paramValue.length), () => { return paramName; });
+          paramNameArray = _.times(paramValue.length, _.constant(paramName));
           pmParams.push(...paramNameArray.map((value, index) => {
             return {
               key: value,
@@ -728,15 +735,13 @@ module.exports = {
         }
         return pmParams;
       };
-    // checking the type of the query parameter
+
     if (paramType === 'array') {
-      // which style is there ?
+      // paramValue will be an array
       if (param.style === 'form') {
-        // check for the truthness of explode
         pmParams = handleExplode(param.explode, paramValue, param.name);
       }
       else if (param.style === 'spaceDelimited') {
-        // explode parameter doesn't really affect anything here
         pmParams.push({
           key: param.name,
           value: paramValue.join(' '),
@@ -769,16 +774,17 @@ module.exports = {
       }
     }
     else if (paramType === 'object') {
-      // following similar checks as above
       if (param.hasOwnProperty('style')) {
         if (param.style === 'form') {
+          // converts paramValue = {a:1, b:2} to:
+          // [{key: a, val: 1, desc}, {key: b, val: 2, desc}] if explode
+          // else to [{key: paramName, value: a,1,b,2}]
           if (param.explode) {
-            // if explode is true
             paramNameArray = Object.keys(paramValue);
-            pmParams.push(...paramNameArray.map((value, index) => {
+            pmParams.push(...paramNameArray.map((keyName) => {
               return {
-                key: value,
+                key: keyName,
-                value: Object.values(paramValue)[index],
+                value: paramValue[keyName],
                 description: param.description
               };
             }));
@@ -835,9 +841,9 @@ module.exports = {
   },
 
   /**
-   * converts param with in='header' and response header to POSTMAN header
+   * converts params with in='header' to a Postman header object
    * @param {*} header param with in='header'
-   * @returns {Object} instance of POSTMAN sdk Header
+   * @returns {Object} instance of a Postman SDK Header
    */
   convertToPmHeader: function(header) {
     var fakeData,
@@ -860,9 +866,9 @@ module.exports = {
   },
 
   /**
-   * converts operation item requestBody to POSTMAN request body
+   * converts operation item requestBody to a Postman request body
    * @param {*} requestBody in operationItem
-   * @returns {Object} - postman requestBody and Content-Type Header
+   * @returns {Object} - Postman requestBody and Content-Type Header
    */
   convertToPmBody: function(requestBody) {
     var contentObj, // content is required
@@ -1067,19 +1073,21 @@ module.exports = {
   },
 
   /**
-   * @param {*} $ref of reference object
+   * @param {*} $ref reference object
-   * @returns {Object} reference object in components
+   * @returns {Object} reference object from the saved components
    */
   getRefObject: function($ref) {
     var refObj, savedSchema;
 
     savedSchema = $ref.split('/').slice(2);
+    // must have 2 segments after "#/components"
     if (savedSchema.length !== 2) {
       console.warn(`ref ${$ref} not found.`);
       return null;
     }
 
-    refObj = _.get(this.components, [savedSchema[0], savedSchema[1]]);
+    // at this point, savedSchema is similar to ['schemas','Address']
+    refObj = _.get(this.components, savedSchema);
     if (!refObj) {
       console.warn(`ref ${$ref} not found.`);
       return null;
@@ -1185,6 +1193,7 @@ module.exports = {
     });
 
     // using the auth helper
+    // TODO: Figure out what happens if type!=api-key
     if (authHelper.type === 'api-key') {
       if (authHelper.properties.in === 'header') {
         item.request.addHeader(this.convertToPmHeader(authHelper.properties));