Do not create additional root for already rooted $refs (#7)

* Inline $refs placed under default root * Remove option we will not need

Author: P0lip

docs/options.md

@@ -85,6 +85,5 @@ The `bundle` options control how JSON Schema $Ref Parser will bundle `$ref` poin
 |Option(s) |Type |Description
 |:---------------------|:-------------------|:------------
 |`generateKey`|`(value: any, file: string, hash: string or null) => string or null`|Used to generate $ref.
-|`shouldInline`|`(pathFromRoot: string) => boolean`|Determines whether a value of given reference should be inlined in the resulting output.
 |`defaultRoot`|`string`|The default root to optimize for.
 

lib/bundle/defaults.js

@@ -6,7 +6,11 @@ function getGenericDefaults (generator) {
  return {
  defaultRoot: generator.root,
 
- generateKey (schema, file, hash) {
+ generateKey (schema, file, hash, pathFromRoot) {
+ if (generator.isUnderDirectRoot(pathFromRoot)) {
+ return null;
+ }
+
  if (!url.isFileSystemPath(file) && !url.isHttp(file)) {
  return null;
  }
@@ -22,7 +26,7 @@ function getGenericDefaults (generator) {
  return null;
  }
 
- return generator.generateKeyForPointer(schema, existingGeneratedKey + hash.slice(1));
+ return generator.generateKeyForPointer(schema, existingGeneratedKey === undefined ? pathFromRoot : existingGeneratedKey + hash.slice(1));
  }
 
  if (url.isHttp(file)) {
@@ -31,9 +35,6 @@ function getGenericDefaults (generator) {
 
  return generator.generateKeyForFilepath(schema, file);
  },
- shouldInline () {
- return false;
- },
  };
 }
 
@@ -50,39 +51,22 @@ module.exports.getDefaultsForNewJsonSchema = function (defaults = getGenericDefa
 module.exports.getDefaultsForOAS2 = function (defaults = getGenericDefaults(new KeyGenerator("#/definitions"))) {
  return {
  ...defaults,
- generateKey (schema, file, hash) {
+ generateKey (schema, file, hash, pathFromRoot) {
+ pathFromRoot = normalizeOasSchemasHash(pathFromRoot, defaults.defaultRoot);
+
  if (hash !== "#" && hash !== null) {
- return defaults.generateKey(schema, file, hash.replace(/\/components\/schemas\//g, "/definitions/"));
+ return defaults.generateKey(schema, file, normalizeOasSchemasHash(hash, defaults.defaultRoot), pathFromRoot);
  }
 
- return defaults.generateKey(schema, file, hash);
+ return defaults.generateKey(schema, file, hash, pathFromRoot);
  },
- shouldInline (pathFromRoot) {
- const parsed = url.safePointerToPath(pathFromRoot);
- return parsed.length === 0 || (parsed[0] !== "definitions" && !parsed.includes("schema"));
- }
  };
 };
 
 module.exports.getDefaultsForOAS3 = function (defaults = getGenericDefaults(new KeyGenerator("#/components/schemas"))) {
- return {
- ...defaults,
- generateKey (schema, file, hash) {
- if (hash !== "#" && hash !== null) {
- return defaults.generateKey(schema, file, hash.replace(/\/definitions\//g, "/components/schemas/"));
- }
-
- return defaults.generateKey(schema, file, hash);
- },
- shouldInline (pathFromRoot) {
- if (pathFromRoot.startsWith("#/components/schemas")) {
- return false;
- }
-
- const parsed = url.safePointerToPath(pathFromRoot);
-
- return parsed.length === 0 || !parsed.includes("schema");
- }
- };
+ return module.exports.getDefaultsForOAS2(defaults);
 };
 
+function normalizeOasSchemasHash (hash, root) {
+ return hash.replace(/\/(?:components\/schemas|definitions)\//g, root.slice(1) + "/");
+}

lib/bundle/index.js

@@ -141,16 +141,15 @@ function inventory$Ref ($refParent, $refKey, path, pathFromRoot, indirections, i
  }
  }
 
- let inlineable = options.bundle.shouldInline(pathFromRoot);
- if (!inlineable && file !== $refs._root$Ref.path) {
+ if (options.bundle.generateKey && file !== $refs._root$Ref.path) {
  if (!customRoots[file]) {
  customRoots[file] = {
- "#": options.bundle.generateKey($refs._root$Ref.value, file, null)
+ "#": options.bundle.generateKey($refs._root$Ref.value, file, null, pathFromRoot)
  };
  }
 
  if (!(hash in customRoots[file])) {
- customRoots[file][hash] = options.bundle.generateKey($refs._root$Ref.value, file, hash);
+ customRoots[file][hash] = options.bundle.generateKey($refs._root$Ref.value, file, hash, pathFromRoot);
  }
 
  pathFromRoot = findClosestRoot(customRoots[file], hash, pathFromRoot);
@@ -170,7 +169,6 @@ function inventory$Ref ($refParent, $refKey, path, pathFromRoot, indirections, i
  extended, // Does this $ref extend its resolved value? (i.e. it has extra properties, in addition to "$ref")
  external, // Does this $ref pointer point to a file other than the main JSON Schema file?
  indirections, // The number of indirect references that were traversed to resolve the value
- inlineable,
  });
 
  // Recursively crawl the resolved value
@@ -216,10 +214,6 @@ function remap (schema, inventory, options, customRoots) {
  // Group all the $refs that point to the same part of the file
  return a.hash < b.hash ? -1 : +1;
  }
- else if (a.inlineable !== b.inlineable) {
- // Group all the $refs that should be inlined. Inlined go last.
- return a.inlineable ? +1 : -1;
- }
  else if (a.circular !== b.circular) {
  // If the $ref points to itself, then sort it higher than other $refs that point to this $ref
  return a.circular ? -1 : +1;
@@ -258,7 +252,7 @@ function remap (schema, inventory, options, customRoots) {
  // console.log('Re-mapping $ref pointer "%s" at %s', entry.$ref.$ref, entry.pathFromRoot);
 
  // if entry is not inlineable and has a custom root linked, we need to remap the properties of the object
- if (!entry.inlineable && customRoots[entry.file] && entry.hash in customRoots[entry.file] && customRoots[entry.file]["#"] !== null) {
+ if (customRoots[entry.file] && entry.hash in customRoots[entry.file] && customRoots[entry.file]["#"] !== null) {
  if (entry.hash === "#") {
  // the whole file is referenced for the first time
  // we need to inject its entire content here

lib/bundle/stoplight/generator.js

@@ -5,14 +5,14 @@ const KeyGenerator = require("../util/key-generator");
 
 function normalizeOpts (opts) {
  if (typeof opts.cwd === "string") {
- opts.cwd = StoplightKeyGenerator.appendSlash(opts.cwd);
+ opts.cwd = KeyGenerator.appendSlash(opts.cwd);
  }
  else {
  opts.cwd = null;
  }
 
  if (typeof opts.endpointUrl === "string") {
- opts.endpointUrl = StoplightKeyGenerator.appendSlash(opts.endpointUrl);
+ opts.endpointUrl = KeyGenerator.appendSlash(opts.endpointUrl);
  }
  else {
  opts.endpointUrl = null;
@@ -26,10 +26,6 @@ function StoplightKeyGenerator (root, opts) {
  this._opts = normalizeOpts({ ...opts });
 }
 
-StoplightKeyGenerator.appendSlash = function (str) {
- return str.replace(/([^/])\/?$/, "$1/");
-};
-
 module.exports = StoplightKeyGenerator;
 
 StoplightKeyGenerator.prototype = Object.create(KeyGenerator.prototype, {

lib/bundle/util/key-generator.js

@@ -32,6 +32,10 @@ function suggestKey (takenKeys, key) {
 
 KeyGenerator.suggestKey = suggestKey;
 
+KeyGenerator.appendSlash = function (str) {
+ return str.replace(/([^/])\/?$/, "$1/");
+};
+
 function KeyGenerator (root) {
  this.root = root;
  this._parsedRoot = safePointerToPath(root);
@@ -142,7 +146,8 @@ Object.assign(KeyGenerator.prototype, {
 
  generateKeyForPointer (schema, pointer) {
  if (!this.hasExistingGeneratedKey(schema, pointer)) {
- let actualPath = pointer.split(this.root.slice(1)).slice(1);
+ let fragment = KeyGenerator.appendSlash(this.root.slice(1));
+ let actualPath = pointer.split(fragment).slice(1);
  let key = this.generateUniqueKey(schema, prettify(actualPath.join("/")));
 
  this.registerNewGeneratedKey(schema, pointer, key);
@@ -165,5 +170,15 @@ Object.assign(KeyGenerator.prototype, {
  }
 
  return true;
+ },
+
+ isUnderDirectRoot (pointer) {
+ let parsedPointer = safePointerToPath(pointer);
+
+ if (parsedPointer.length !== this._parsedRoot.length + 1) {
+ return false;
+ }
+
+ return this.isInRoot(pointer);
  }
 });

lib/index.d.ts

@@ -245,14 +245,6 @@ declare namespace $RefParser {
  */
  generateKey?(value: unknown, file: string, hash: string | null): string | null;
 
- /**
- * Determines whether a value of given reference should be inlined in the resulting output.
- *
- * @param {string} pathFromRoot
- * @return boolean
- */
- shouldInline?(pathFromRoot: string): boolean;
-
  /**
  * The default root to optimize for.
  */

lib/options.js

@@ -84,17 +84,10 @@ $RefParserOptions.defaults = {
  * @param {*} value
  * @param {string} file
  * @param {string|null} hash
- */
- // eslint-disable-next-line no-unused-vars
- generateKey (value, file, hash) { return null; },
-
- /**
- * Determines whether a value of given reference should be inlined in the resulting output
- *
  * @param {string} pathFromRoot
  */
  // eslint-disable-next-line no-unused-vars
- shouldInline (pathFromRoot) { return false; },
+ generateKey: null,
 
  /**
  * The default root to optimize for.

test/specs/custom-bundling-roots-legacy-stoplight/reference/openapi-2-bundled.js

@@ -10,10 +10,7 @@ module.exports = {
  "/flight/{id}": {
  parameters: [
  {
- type: "number",
- name: "id",
- in: "path",
- required: true
+ $ref: "#/definitions/Id"
  }
  ],
  get: {
@@ -155,6 +152,12 @@ module.exports = {
  maxLength: 100,
  example: "747"
  },
+ Id: {
+ in: "path",
+ name: "id",
+ required: true,
+ type: "number"
+ },
  Manufacturer: {
  definitions: {},
  title: "Manufacturer",

test/specs/custom-bundling-roots-legacy-stoplight/reference/openapi-3.bundled.js

@@ -14,10 +14,7 @@ module.exports = {
  "/flight/{id}": {
  parameters: [
  {
- type: "number",
- name: "id",
- in: "path",
- required: true
+ $ref: "#/components/schemas/Id"
  }
  ],
  get: {
@@ -169,6 +166,12 @@ module.exports = {
  maxLength: 100,
  example: "747"
  },
+ Id: {
+ in: "path",
+ name: "id",
+ required: true,
+ type: "number"
+ },
  Manufacturer: {
  definitions: {},
  title: "Manufacturer",

test/specs/custom-bundling-roots/custom-bundling-roots.spec.js

@@ -46,10 +46,7 @@ describe("Custom bundling roots", () => {
  },
  parameters: [
  {
- in: "path",
- name: "id",
- required: true,
- type: "number"
+ $ref: "#/definitions/Id"
  }
  ]
  }
@@ -224,4 +221,49 @@ describe("Custom bundling roots", () => {
  }
  });
  });
+
+ it("should not create redundant roots", async () => {
+ let parser = new $RefParser();
+ const model = {
+ properties: {
+ user: {
+ properties: {
+ id: {
+ $ref: "#/definitions/Id"
+ }
+ },
+ }
+ },
+ definitions: {
+ Id: {
+ $ref: path.rel("specs/custom-bundling-roots/id.json")
+ }
+ }
+ };
+
+ const schema = await parser.bundle(model, {
+ bundle: getDefaultsForOldJsonSchema(),
+ });
+
+ expect(schema).to.equal(parser.schema);
+ expect(schema).to.deep.equal({
+ properties: {
+ user: {
+ properties: {
+ id: {
+ $ref: "#/definitions/Id"
+ }
+ },
+ }
+ },
+ definitions: {
+ Id: {
+ type: "number",
+ name: "id",
+ in: "path",
+ required: true
+ }
+ }
+ });
+ });
 });