Completely rewrote the bundling logic to fix APIDevTools/swagger-pars…

…er#16
APIDevTools · Jan 4, 2016 · 32510a3 · tlusk · Jan 5, 2016 · JamesMessinger
1 parent 030f604
commit 32510a3
Show file tree

Hide file tree

Showing 16 changed files with 1,112 additions and 701 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -87,7 +87,7 @@
 
     "accessor-pairs":                   2,                // require corresponding getters for any setters
     "block-scoped-var":                 2,                // treat var statements as if they were block scoped (off by default)
-    "complexity":                      [2, 8],            // specify the maximum cyclomatic complexity allowed in a program (off by default)
+    "complexity":                      [1, 8],            // specify the maximum cyclomatic complexity allowed in a program (off by default)
     "consistent-return":                0,                // require return statements to either always or never specify values
     "curly":                            2,                // specify curly brace conventions for all control statements
     "default-case":                     0,                // require default case in switch statements (off by default)

diff --git a/.jscsrc b/.jscsrc
@@ -70,19 +70,4 @@
   "disallowNewlineBeforeBlockStatements": true,
   "disallowSpaceBeforeComma": true,
   "disallowSpaceBeforeSemicolon": true,
-
-  "jsDoc": {
-    "checkAnnotations": true,
-    "checkParamNames": true,
-    "requireParamTypes": true,
-    "checkRedundantParams": true,
-    "checkReturnTypes": true,
-    "checkRedundantReturns": true,
-    "requireReturnTypes": true,
-    "checkTypes": true,
-    "checkRedundantAccess": "enforceLeadingUnderscore",
-    "leadingUnderscoreAccess": true,
-    "requireHyphenBeforeDescription": true,
-    "requireNewlineAfterDescription": true
-  }
 }
diff --git a/dist/ref-parser.js b/dist/ref-parser.js
diff --git a/dist/ref-parser.js.map b/dist/ref-parser.js.map
diff --git a/dist/ref-parser.min.js b/dist/ref-parser.min.js
diff --git a/dist/ref-parser.min.js.map b/dist/ref-parser.min.js.map
diff --git a/lib/bundle.js b/lib/bundle.js
@@ -22,171 +22,170 @@ module.exports = bundle;
  * @param {$RefParserOptions} options
  */
 function bundle(parser, options) {
-  util.debug('Bundling $ref pointers in %s', parser._basePath);
+  util.debug('Bundling $ref pointers in %s', parser.$refs._basePath);
 
-  optimize(parser.$refs);
-  remap(parser.$refs, options);
-  dereference(parser._basePath, parser.$refs, options);
-}
+  // Build an inventory of all $ref pointers in the JSON Schema
+  var inventory = [];
+  crawl(parser.schema, parser.$refs._basePath + '#', '#', inventory, parser.$refs, options);
 
-/**
- * Optimizes the {@link $Ref#referencedAt} list for each {@link $Ref} to contain as few entries
- * as possible (ideally, one).
- *
- * @example:
- *  {
- *    first: { $ref: somefile.json#/some/part },
- *    second: { $ref: somefile.json#/another/part },
- *    third: { $ref: somefile.json },
- *    fourth: { $ref: somefile.json#/some/part/sub/part }
- *  }
- *
- * In this example, there are four references to the same file, but since the third reference points
- * to the ENTIRE file, that's the only one we care about.  The other three can just be remapped to point
- * inside the third one.
- *
- * On the other hand, if the third reference DIDN'T exist, then the first and second would both be
- * significant, since they point to different parts of the file. The fourth reference is not significant,
- * since it can still be remapped to point inside the first one.
- *
- * @param {$Refs} $refs
- */
-function optimize($refs) {
-  Object.keys($refs._$refs).forEach(function(key) {
-    var $ref = $refs._$refs[key];
-
-    // Find the first reference to this $ref
-    var first = $ref.referencedAt.filter(function(at) { return at.firstReference; })[0];
-
-    // Do any of the references point to the entire file?
-    var entireFile = $ref.referencedAt.filter(function(at) { return at.hash === '#'; });
-    if (entireFile.length === 1) {
-      // We found a single reference to the entire file.  Done!
-      $ref.referencedAt = entireFile;
-    }
-    else if (entireFile.length > 1) {
-      // We found more than one reference to the entire file.  Pick the first one.
-      if (entireFile.indexOf(first) >= 0) {
-        $ref.referencedAt = [first];
-      }
-      else {
-        $ref.referencedAt = entireFile.slice(0, 1);
-      }
-    }
-    else {
-      // There are noo references to the entire file, so optimize the list of reference points
-      // by eliminating any duplicate/redundant ones (e.g. "fourth" in the example above)
-console.log('========================= %s BEFORE =======================', $ref.path, JSON.stringify($ref.referencedAt, null, 2));
-      [first].concat($ref.referencedAt).forEach(function(at) {
-        dedupe(at, $ref.referencedAt);
-      });
-console.log('========================= %s AFTER =======================', $ref.path, JSON.stringify($ref.referencedAt, null, 2));
-    }
-  });
-}
-
-/**
- * Removes redundant entries from the {@link $Ref#referencedAt} list.
- *
- * @param {object} original - The {@link $Ref#referencedAt} entry to keep
- * @param {object[]} dupes - The {@link $Ref#referencedAt} list to dedupe
- */
-function dedupe(original, dupes) {
-  for (var i = dupes.length - 1; i >= 0; i--) {
-    var dupe = dupes[i];
-    if (dupe !== original && dupe.hash.indexOf(original.hash) === 0) {
-      dupes.splice(i, 1);
-    }
-  }
+  // Remap all $ref pointers
+  remap(inventory);
 }
 
 /**
- * Re-maps all $ref pointers in the schema, so that they are relative to the root of the schema.
+ * Recursively crawls the given value, and inventories all JSON references.
  *
+ * @param {*} obj - The value to crawl. If it's not an object or array, it will be ignored.
+ * @param {string} path - The full path of `obj`, possibly with a JSON Pointer in the hash
+ * @param {string} pathFromRoot - The path of `obj` from the schema root
+ * @param {object[]} inventory - An array of already-inventoried $ref pointers
  * @param {$Refs} $refs
  * @param {$RefParserOptions} options
  */
-function remap($refs, options) {
-  var remapped = [];
-
-  // Crawl the schema and determine the re-mapped values for all $ref pointers.
-  // NOTE: We don't actually APPLY the re-mappings yet, since that can affect other re-mappings
-  Object.keys($refs._$refs).forEach(function(key) {
-    var $ref = $refs._$refs[key];
-    crawl($ref.value, $ref.path + '#', $refs, remapped, options);
-  });
+function crawl(obj, path, pathFromRoot, inventory, $refs, options) {
+  if (obj && typeof obj === 'object') {
+    var keys = Object.keys(obj);
 
-  // Now APPLY all of the re-mappings
-  for (var i = 0; i < remapped.length; i++) {
-    var mapping = remapped[i];
-    mapping.old$Ref.$ref = mapping.new$Ref.$ref;
-  }
-}
+    // Most people will expect references to be bundled into the the "definitions" property,
+    // so we always crawl that property first, if it exists.
+    var defs = keys.indexOf('definitions');
+    if (defs > 0) {
+      keys.splice(0, 0, keys.splice(defs, 1)[0]);
+    }
 
-/**
- * Recursively crawls the given value, and re-maps any JSON references.
- *
- * @param {*} obj - The value to crawl. If it's not an object or array, it will be ignored.
- * @param {string} path - The path to use for resolving relative JSON references
- * @param {$Refs} $refs - The resolved JSON references
- * @param {object[]} remapped - An array of the re-mapped JSON references
- * @param {$RefParserOptions} options
- */
-function crawl(obj, path, $refs, remapped, options) {
-  if (obj && typeof obj === 'object') {
-    Object.keys(obj).forEach(function(key) {
+    keys.forEach(function(key) {
       var keyPath = Pointer.join(path, key);
+      var keyPathFromRoot = Pointer.join(pathFromRoot, key);
       var value = obj[key];
 
       if ($Ref.is$Ref(value)) {
-        // We found a $ref, so resolve it
-        util.debug('Re-mapping $ref pointer "%s" at %s', value.$ref, keyPath);
-        var $refPath = url.resolve(path, value.$ref);
-        var pointer = $refs._resolve($refPath, options);
-
-        // Find the path from the root of the JSON schema
-        var hash = util.path.getHash(value.$ref);
-        var referencedAt = pointer.$ref.referencedAt.filter(function(at) {
-          return hash.indexOf(at.hash) === 0;
-        })[0];
-
-console.log(
-  'referencedAt.pathFromRoot =', referencedAt.pathFromRoot,
-  '\nreferencedAt.hash         =', referencedAt.hash,
-  '\nhash                      =', hash,
-  '\npointer.path.hash         =', util.path.getHash(pointer.path)
-);
-
-        // Re-map the value
-        var new$RefPath = referencedAt.pathFromRoot + util.path.getHash(pointer.path).substr(1);
-        util.debug('    new value: %s', new$RefPath);
-        remapped.push({
-          old$Ref: value,
-          new$Ref: {$ref: new$RefPath}  // Note: DON'T name this property `new` (https://github.com/BigstickCarpet/json-schema-ref-parser/issues/3)
-        });
+        // Skip this $ref if we've already inventoried it
+        if (!inventory.some(function(i) { return i.parent === obj && i.key === key; })) {
+          inventory$Ref(obj, key, path, keyPathFromRoot, inventory, $refs, options);
+        }
       }
       else {
-        crawl(value, keyPath, $refs, remapped, options);
+        crawl(value, keyPath, keyPathFromRoot, inventory, $refs, options);
       }
     });
   }
 }
 
 /**
- * Dereferences each external $ref pointer exactly ONCE.
+ * Inventories the given JSON Reference (i.e. records detailed information about it so we can
+ * optimize all $refs in the schema), and then crawls the resolved value.
  *
- * @param {string} basePath
+ * @param {object} $refParent - The object that contains a JSON Reference as one of its keys
+ * @param {string} $refKey - The key in `$refParent` that is a JSON Reference
+ * @param {string} path - The full path of the JSON Reference at `$refKey`, possibly with a JSON Pointer in the hash
+ * @param {string} pathFromRoot - The path of the JSON Reference at `$refKey`, from the schema root
+ * @param {object[]} inventory - An array of already-inventoried $ref pointers
  * @param {$Refs} $refs
  * @param {$RefParserOptions} options
  */
-function dereference(basePath, $refs, options) {
-  basePath = util.path.stripHash(basePath);
+function inventory$Ref($refParent, $refKey, path, pathFromRoot, inventory, $refs, options) {
+  var $ref = $refParent[$refKey];
+  var $refPath = url.resolve(path, $ref.$ref);
+  var pointer = $refs._resolve($refPath, options);
+  var depth = Pointer.parse(pathFromRoot).length;
+  var file = util.path.stripHash(pointer.path);
+  var hash = util.path.getHash(pointer.path);
+  var external = file !== $refs._basePath;
+  var extended = Object.keys($ref).length > 1;
+
+  inventory.push({
+    $ref: $ref,                   // The JSON Reference (e.g. {$ref: string})
+    parent: $refParent,           // The object that contains this $ref pointer
+    key: $refKey,                 // The key in `parent` that is the $ref pointer
+    pathFromRoot: pathFromRoot,   // The path to the $ref pointer, from the JSON Schema root
+    depth: depth,                 // How far from the JSON Schema root is this $ref pointer?
+    file: file,                   // The file that the $ref pointer resolves to
+    hash: hash,                   // The hash within `file` that the $ref pointer resolves to
+    value: pointer.value,         // The resolved value of the $ref pointer
+    circular: pointer.circular,   // Is this $ref pointer DIRECTLY circular? (i.e. it references itself)
+    extended: extended,           // Does this $ref extend its resolved value? (i.e. it has extra properties, in addition to "$ref")
+    external: external            // Does this $ref pointer point to a file other than the main JSON Schema file?
+  });
 
-  Object.keys($refs._$refs).forEach(function(key) {
-    var $ref = $refs._$refs[key];
+  // Recursively crawl the resolved value
+  crawl(pointer.value, pointer.path, pathFromRoot, inventory, $refs, options);
+}
 
-    if ($ref.referencedAt.length > 0) {
-      $refs.set(basePath + $ref.referencedAt[0].pathFromRoot, $ref.value, options);
+/**
+ * Re-maps every $ref pointer, so that they're all relative to the root of the JSON Schema.
+ * Each referenced value is dereferenced EXACTLY ONCE.  All subsequent references to the same
+ * value are re-mapped to point to the first reference.
+ *
+ * @example:
+ *  {
+ *    first: { $ref: somefile.json#/some/part },
+ *    second: { $ref: somefile.json#/another/part },
+ *    third: { $ref: somefile.json },
+ *    fourth: { $ref: somefile.json#/some/part/sub/part }
+ *  }
+ *
+ * In this example, there are four references to the same file, but since the third reference points
+ * to the ENTIRE file, that's the only one we need to dereference.  The other three can just be
+ * remapped to point inside the third one.
+ *
+ * On the other hand, if the third reference DIDN'T exist, then the first and second would both need
+ * to be dereferenced, since they point to different parts of the file. The fourth reference does NOT
+ * need to be dereferenced, because it can be remapped to point inside the first one.
+ *
+ * @param {object[]} inventory
+ */
+function remap(inventory) {
+  // Group & sort all the $ref pointers, so they're in the order that we need to dereference/remap them
+  inventory.sort(function(a, b) {
+    if (a.file !== b.file) {
+      return a.file < b.file ? -1 : +1;   // Group all the $refs that point to the same file
+    }
+    else if (a.hash !== b.hash) {
+      return a.hash < b.hash ? -1 : +1;   // Group all the $refs that point to the same part of the file
+    }
+    else if (a.circular !== b.circular) {
+      return a.circular ? -1 : +1;        // If the $ref points to itself, then sort it higher than other $refs that point to this $ref
+    }
+    else if (a.extended !== b.extended) {
+      return a.extended ? +1 : -1;        // If the $ref extends the resolved value, then sort it lower than other $refs that don't extend the value
+    }
+    else if (a.depth !== b.depth) {
+      return a.depth - b.depth;           // Sort $refs by how close they are to the JSON Schema root
+    }
+    else {
+      // If all else is equal, use the $ref that's in the "definitions" property
+      return b.pathFromRoot.lastIndexOf('/definitions') - a.pathFromRoot.lastIndexOf('/definitions');
     }
   });
+
+  var file, hash, pathFromRoot;
+  inventory.forEach(function(i) {
+    util.debug('Re-mapping $ref pointer "%s" at %s', i.$ref.$ref, i.pathFromRoot);
+
+    if (!i.external) {
+      // This $ref already resolves to the main JSON Schema file
+      i.$ref.$ref = i.hash;
+    }
+    else if (i.file !== file || i.hash.indexOf(hash) !== 0) {
+      // We've moved to a new file or new hash
+      file = i.file;
+      hash = i.hash;
+      pathFromRoot = i.pathFromRoot;
+
+      // This is the first $ref to point to this value, so dereference the value.
+      // Any other $refs that point to the same value will point to this $ref instead
+      i.$ref = i.parent[i.key] = util.dereference(i.$ref, i.value);
+
+      if (i.circular) {
+        // This $ref points to itself
+        i.$ref.$ref = i.pathFromRoot;
+      }
+    }
+    else {
+      // This $ref points to the same value as the prevous $ref
+      i.$ref.$ref = Pointer.join(pathFromRoot, Pointer.parse(i.hash));
+    }
+
+    util.debug('    new value: %s', (i.$ref && i.$ref.$ref) ? i.$ref.$ref : '[object Object]');
+  });
 }
diff --git a/tests/index.html b/tests/index.html
@@ -40,6 +40,11 @@
     <script src="specs/external/external.bundled.js"></script>
     <script src="specs/external/external.spec.js"></script>
 
+    <script src="specs/external-partial/external-partial.parsed.js"></script>
+    <script src="specs/external-partial/external-partial.dereferenced.js"></script>
+    <script src="specs/external-partial/external-partial.bundled.js"></script>
+    <script src="specs/external-partial/external-partial.spec.js"></script>
+
     <script src="specs/circular/circular.parsed.js"></script>
     <script src="specs/circular/circular.dereferenced.js"></script>
     <script src="specs/circular/circular.spec.js"></script>

diff --git a/tests/specs/external-partial/definitions/definitions.json b/tests/specs/external-partial/definitions/definitions.json
@@ -0,0 +1,19 @@
+{
+  "required string": {
+    "$ref": "required-string.yaml"
+  },
+  "string": {
+    "$ref": "#/required%20string/type"
+  },
+  "name": {
+    "$ref": "../definitions/name.yaml"
+  },
+  "age": {
+    "type": "integer",
+    "minimum": 0
+  },
+  "gender": {
+    "type": "string",
+    "enum": ["male", "female"]
+  }
+}
diff --git a/tests/specs/external-partial/definitions/name.yaml b/tests/specs/external-partial/definitions/name.yaml
@@ -0,0 +1,22 @@
+title: name
+type: object
+required:
+  - first
+  - last
+properties:
+  first:
+    $ref: ../definitions/definitions.json#/required string
+  last:
+    $ref: ./required-string.yaml
+  middle:
+    type:
+      $ref: "definitions.json#/name/properties/first/type"
+    minLength:
+      $ref: "definitions.json#/name/properties/first/minLength"
+  prefix:
+    $ref: "../definitions/definitions.json#/name/properties/last"
+    minLength: 3
+  suffix:
+    type: string
+    $ref: "definitions.json#/name/properties/prefix"
+    maxLength: 3
diff --git a/tests/specs/external-partial/definitions/required-string.yaml b/tests/specs/external-partial/definitions/required-string.yaml
@@ -0,0 +1,3 @@
+title: required string
+type: string
+minLength: 1