Merge pull request #566 from postmanlabs/feat/fixLongPaths20

Feat/fix long paths20

lib/bundle.js

@@ -12,6 +12,7 @@ const {
   traverseUtility = require('traverse'),
   parse = require('./parse.js'),
   { ParseError } = require('./common/ParseError'),
+  Utils = require('./utils'),
   crypto = require('crypto');
 
 let path = require('path'),
@@ -219,9 +220,10 @@ function createComponentMainKey(tempRef, mainKeys) {
  * @param {object} tempRef - The tempRef from the $ref
  * @param {object} mainKeys - The dictionary of the previous keys generated
  * @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, tempRef, mainKeys, version) {
+function getTraceFromParentKeyInComponents(nodeContext, tempRef, mainKeys, version, commonPathFromData) {
   const parents = [...nodeContext.parents].reverse(),
     isArrayKeyRegexp = new RegExp('^\\d$', 'g'),
     key = nodeContext.key,
@@ -234,7 +236,7 @@ function getTraceFromParentKeyInComponents(nodeContext, tempRef, mainKeys, versi
       [key, ...parentKeys],
     nodeTrace = getRootFileTrace(nodeParentsKey),
     componentKey = createComponentMainKey(tempRef, mainKeys),
-    keyTraceInComponents = getKeyInComponents(nodeTrace, componentKey, version);
+    keyTraceInComponents = getKeyInComponents(nodeTrace, componentKey, version, commonPathFromData);
   return keyTraceInComponents;
 }
 
@@ -266,9 +268,11 @@ function handleLocalCollisions(trace, initialMainKeys) {
    * @param {string} parentFilename - The parent's filename
    * @param {object} version - The version of the spec we are bundling
    * @param {object} rootMainKeys - A dictionary with the component keys in local components object and its mainKeys
+   * @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, rootMainKeys) {
+function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, version, rootMainKeys,
+  commonPathFromData) {
   let referencesInNode = [],
     nodeReferenceDirectory = {},
     mainKeys = {};
@@ -287,7 +291,7 @@ function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, ve
       if (hasReferenceTypeKey) {
         const tempRef = calculatePath(parentFilename, property.$ref),
           nodeTrace = handleLocalCollisions(
-            getTraceFromParentKeyInComponents(this, tempRef, mainKeys, version),
+            getTraceFromParentKeyInComponents(this, tempRef, mainKeys, version, commonPathFromData),
             rootMainKeys
           ),
           componentKey = nodeTrace[nodeTrace.length - 1],
@@ -338,9 +342,10 @@ function getReferences (currentNode, isOutOfRoot, pathSolver, parentFilename, ve
    * @param {object} specRoot - root file information
    * @param {string} version - The current version
    * @param {object} rootMainKeys - A dictionary with the local reusable components keys and its mainKeys
+   * @param {string} commonPathFromData - The common path in the file's paths
    * @returns {object} - Detect root files result object
    */
-function getNodeContentAndReferences (currentNode, allData, specRoot, version, rootMainKeys) {
+function getNodeContentAndReferences (currentNode, allData, specRoot, version, rootMainKeys, commonPathFromData) {
   let graphAdj = [],
     missingNodes = [],
     nodeContent;
@@ -358,7 +363,8 @@ function getNodeContentAndReferences (currentNode, allData, specRoot, version, r
     removeLocalReferenceFromPath,
     currentNode.fileName,
     version,
-    rootMainKeys
+    rootMainKeys,
+    commonPathFromData
   );
 
   referencesInNode.forEach((reference) => {
@@ -521,9 +527,13 @@ module.exports = {
       initialMainKeys = getMainKeysFromComponents(initialComponents, version);
     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, initialMainKeys);
+      return getNodeContentAndReferences(currentNode, allData, specRoot, version, initialMainKeys, commonPathFromData);
     });
     components = generateComponentsWrapper(specRoot.parsed.oasObject, version);
     generateComponentsObject(

lib/jsonPointer.js

@@ -51,10 +51,10 @@ function generateObjectName(filePathName, hash = '') {
 * @param {string} traceFromParent the node trace from root.
 * @param {string} mainKey - The generated mainKey for the components
 * @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, mainKey, version) {
+function getKeyInComponents(traceFromParent, mainKey, version, commonPathFromData) {
-  // const localPart = localPath ? `${localPointer}${localPath}` : '',
   const {
     CONTAINERS,
     DEFINITIONS,
@@ -63,9 +63,10 @@ function getKeyInComponents(traceFromParent, mainKey, version) {
     ROOT_CONTAINERS_KEYS
   } = getBundleRulesDataByVersion(version);
   let result,
+    newFPN = mainKey.replace(generateObjectName(commonPathFromData), ''),
     trace = [
       ...traceFromParent,
-      jsonPointerDecodeAndReplace(mainKey)
+      jsonPointerDecodeAndReplace(newFPN)
     ].reverse(),
     traceToKey = [],
     matchFound = false,

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('/');
   }
 };

test/data/toBundleExamples/longPath/client.json

@@ -0,0 +1,14 @@
+{
+    "type": "object",
+    "properties": {
+      "idClient": {
+        "type": "integer"
+      },
+    "clientName": {
+        "type": "string"
+      },
+    "special": {
+        "$ref": "../user/special.yaml"
+      }
+  }
+}

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"
+          }
+        }
+      }
+    }
+  }
+}

test/data/toBundleExamples/longPath/magic.yaml

@@ -0,0 +1,6 @@
+type: object
+properties:
+  magicNumber:
+    type: integer
+  magicString:
+    type: string

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"

test/data/toBundleExamples/longPath/special.yaml

@@ -0,0 +1,6 @@
+type: object
+properties:
+  specialClientId:
+    type: string
+  magic:
+    $ref: ./magic.yaml

test/data/toBundleExamples/longPath/user.yaml

@@ -0,0 +1,8 @@
+type: object
+properties:
+  id:
+    type: integer
+  userName:
+    type: string
+  special:
+    $ref: ./special.yaml

test/data/toBundleExamples/longPath/userSpecial.yaml

@@ -0,0 +1,4 @@
+type: object
+properties:
+  specialUserId:
+    type: string

test/unit/bundle.test.js

@@ -38,6 +38,7 @@ let expect = require('chai').expect,
   compositeOneOf = path.join(__dirname, BUNDLES_FOLDER + '/composite_oneOf'),
   compositeNot = path.join(__dirname, BUNDLES_FOLDER + '/composite_not'),
   compositeAnyOf = path.join(__dirname, BUNDLES_FOLDER + '/composite_anyOf'),
+  longPath = path.join(__dirname, BUNDLES_FOLDER + '/longPath'),
   schemaCollision = path.join(__dirname, BUNDLES_FOLDER + '/schema_collision_from_responses'),
   schemaCollisionWRootComponent = path.join(__dirname, BUNDLES_FOLDER + '/schema_collision_w_root_components');
 
@@ -2002,6 +2003,63 @@ describe('bundle files method - 3.0', function () {
     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);
   });
+
+  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() {
@@ -2051,7 +2109,9 @@ describe('getReferences method when node does not have any reference', function(
         nodeIsRoot,
         removeLocalReferenceFromPath,
         'the/parent/filename',
-        {}
+        '3.0',
+        {},
+        ''
       );
     expect(result.nodeReferenceDirectory).to.be.an('object');
     expect(Object.keys(result.nodeReferenceDirectory).length).to.equal(1);

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');

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('');
+  });
+
+});