diff --git a/lib/bundle.js b/lib/bundle.js index d5578bf1..96be2e1f 100644 --- a/lib/bundle.js +++ b/lib/bundle.js @@ -43,40 +43,36 @@ function crawl (parent, key, path, pathFromRoot, indirections, inventory, $refs, if ($Ref.isAllowed$Ref(obj)) { inventory$Ref(parent, key, path, pathFromRoot, indirections, inventory, $refs, options); } - else { - // Crawl the object in a specific order that's optimized for bundling. - // This is important because it determines how `pathFromRoot` gets built, - // which later determines which keys get dereferenced and which ones get remapped - let keys = Object.keys(obj) - .sort((a, b) => { - // Most people will expect references to be bundled into the the "definitions" property, - // so we always crawl that property first, if it exists. - if (a === "definitions") { - return -1; - } - else if (b === "definitions") { - return 1; - } - else { - // Otherwise, crawl the keys based on their length. - // This produces the shortest possible bundled references - return a.length - b.length; - } - }); - - // eslint-disable-next-line no-shadow - for (let key of keys) { - let keyPath = Pointer.join(path, key); - let keyPathFromRoot = Pointer.join(pathFromRoot, key); - let value = obj[key]; - - if ($Ref.isAllowed$Ref(value)) { - inventory$Ref(obj, key, path, keyPathFromRoot, indirections, inventory, $refs, options); + // Crawl the object in a specific order that's optimized for bundling. + // This is important because it determines how `pathFromRoot` gets built, + // which later determines which keys get dereferenced and which ones get remapped + let keys = Object.keys(obj) + .sort((a, b) => { + // Most people will expect references to be bundled into the the "definitions" property, + // so we always crawl that property first, if it exists. + if (a === "definitions") { + return -1; + } + else if (b === "definitions") { + return 1; } else { - crawl(obj, key, keyPath, keyPathFromRoot, indirections, inventory, $refs, options); + // Otherwise, crawl the keys based on their length. + // This produces the shortest possible bundled references + return a.length - b.length; } + }); + + // eslint-disable-next-line no-shadow + for (let key of keys) { + let keyPath = Pointer.join(path, key); + let keyPathFromRoot = Pointer.join(pathFromRoot, key); + let value = obj[key]; + + if ($Ref.isAllowed$Ref(value)) { + inventory$Ref(obj, key, path, keyPathFromRoot, indirections, inventory, $refs, options); } + crawl(obj, key, keyPath, keyPathFromRoot, indirections, inventory, $refs, options); } } } diff --git a/lib/dereference.js b/lib/dereference.js index 0a4c919a..f87bd612 100644 --- a/lib/dereference.js +++ b/lib/dereference.js @@ -51,38 +51,37 @@ function crawl (obj, path, pathFromRoot, parents, processedObjects, dereferenced result.circular = dereferenced.circular; result.value = dereferenced.value; } - else { - for (const key of Object.keys(obj)) { - let keyPath = Pointer.join(path, key); - let keyPathFromRoot = Pointer.join(pathFromRoot, key); - let value = obj[key]; - let circular = false; - - if ($Ref.isAllowed$Ref(value, options)) { - dereferenced = dereference$Ref(value, keyPath, keyPathFromRoot, parents, processedObjects, dereferencedCache, $refs, options); - circular = dereferenced.circular; - // Avoid pointless mutations; breaks frozen objects to no profit - if (obj[key] !== dereferenced.value) { - obj[key] = dereferenced.value; - } + for (const key of Object.keys(obj)) { + let keyPath = Pointer.join(path, key); + let keyPathFromRoot = Pointer.join(pathFromRoot, key); + let value = obj[key]; + let circular = false; + const matches = {}; + + if ($Ref.isAllowed$Ref(value, options)) { + dereferenced = dereference$Ref(value, keyPath, keyPathFromRoot, parents, processedObjects, dereferencedCache, $refs, options); + circular = circular || dereferenced.circular; + // Avoid pointless mutations; breaks frozen objects to no profit + if (obj[key] !== dereferenced.value && typeof dereferenced.value === "string") { + obj[key] = dereferenced.value; + matches.isAllowed$Ref = true; } - else { - if (!parents.has(value)) { - dereferenced = crawl(value, keyPath, keyPathFromRoot, parents, processedObjects, dereferencedCache, $refs, options); - circular = dereferenced.circular; - // Avoid pointless mutations; breaks frozen objects to no profit - if (obj[key] !== dereferenced.value) { - obj[key] = dereferenced.value; - } - } - else { - circular = foundCircularReference(keyPath, $refs, options); - } + } + if (!parents.has(value)) { + dereferenced = crawl(value, keyPath, keyPathFromRoot, parents, processedObjects, dereferencedCache, $refs, options); + circular = circular || dereferenced.circular; + // Avoid pointless mutations; breaks frozen objects to no profit + if (obj[key] !== dereferenced.value && !matches.isAllowed$Ref) { + obj[key] = dereferenced.value; + matches.isNotInParents = true; } - - // Set the "isCircular" flag if this or any other property is circular - result.circular = result.circular || circular; } + else { + circular = foundCircularReference(keyPath, $refs, options); + } + + // Set the "isCircular" flag if this or any other property is circular + result.circular = result.circular || circular; } parents.delete(obj); diff --git a/lib/resolve-external.js b/lib/resolve-external.js index 0d0a8648..cea87160 100644 --- a/lib/resolve-external.js +++ b/lib/resolve-external.js @@ -61,18 +61,14 @@ function crawl (obj, path, $refs, options, seen) { if ($Ref.isExternal$Ref(obj)) { promises.push(resolve$Ref(obj, path, $refs, options)); } - else { - for (let key of Object.keys(obj)) { - let keyPath = Pointer.join(path, key); - let value = obj[key]; - - if ($Ref.isExternal$Ref(value)) { - promises.push(resolve$Ref(value, keyPath, $refs, options)); - } - else { - promises = promises.concat(crawl(value, keyPath, $refs, options, seen)); - } + for (let key of Object.keys(obj)) { + let keyPath = Pointer.join(path, key); + let value = obj[key]; + + if ($Ref.isExternal$Ref(value)) { + promises.push(resolve$Ref(value, keyPath, $refs, options)); } + promises = promises.concat(crawl(value, keyPath, $refs, options, seen)); } } diff --git a/test/specs/external-from-external/bundled.js b/test/specs/external-from-external/bundled.js new file mode 100644 index 00000000..76088940 --- /dev/null +++ b/test/specs/external-from-external/bundled.js @@ -0,0 +1,92 @@ +"use strict"; + +module.exports = { + $schema: "http://json-schema.org/draft-07/schema#", + type: "object", + definitions: { + externalOne: { type: "string", enum: ["EXTERNAL", "ONE"]}, + internalOne: { $ref: "#/definitions/externalOne" }, + }, + properties: { + pageOne: { + type: "object", + properties: { + nestedProperty: { + $ref: "#/definitions/externalOne", + $bespokeKey: { + append: { + component: "Callout", + props: { type: "warning" }, + on: [ + "GB", + [ + "PROHIBITED", + { + $ref: + "#/properties/pageOne/properties/nestedProperty/not/enum", + }, + ], + ], + }, + }, + not: { enum: ["EXTERNAL", "TWO"]}, + }, + }, + allOf: [ + { + if: { + properties: { + nestedProperty: { type: "string", enum: ["EXTERNAL", "THREE"]}, + }, + }, + then: { + required: ["otherBooleanProperty"], + properties: { otherBooleanProperty: { type: "boolean" }}, + }, + }, + { + if: { + properties: { + nestedProperty: { + type: "string", + enum: { + $ref: + "#/properties/pageOne/allOf/0/if/properties/nestedProperty/enum", + }, + }, + otherBooleanProperty: { const: false }, + }, + }, + then: { + properties: { + nestedOtherProperty: { + $ref: "#/definitions/externalOne", + $bespokeKey: { + append: { + component: "Callout", + props: { type: "warning" }, + on: [ + [ + "PROHIBITED", + { + $ref: + "#/properties/pageOne/properties/nestedProperty/not/enum", + }, + ], + ], + }, + }, + not: { + enum: { + $ref: + "#/properties/pageOne/properties/nestedProperty/not/enum", + }, + }, + }, + }, + }, + }, + ], + }, + }, +}; diff --git a/test/specs/external-from-external/dereferenced.js b/test/specs/external-from-external/dereferenced.js new file mode 100644 index 00000000..1903dd84 --- /dev/null +++ b/test/specs/external-from-external/dereferenced.js @@ -0,0 +1,72 @@ +"use strict"; + +module.exports = { + $schema: "http://json-schema.org/draft-07/schema#", + type: "object", + definitions: { + externalOne: { type: "string", enum: ["EXTERNAL", "ONE"]}, + internalOne: { type: "string", enum: ["EXTERNAL", "ONE"]}, + }, + properties: { + pageOne: { + type: "object", + properties: { + nestedProperty: { + $bespokeKey: { + append: { + component: "Callout", + props: { type: "warning" }, + on: ["GB", ["PROHIBITED", ["EXTERNAL", "TWO"]]], + }, + }, + not: { enum: ["EXTERNAL", "TWO"]}, + type: "string", + enum: ["EXTERNAL", "ONE"], + }, + }, + allOf: [ + { + if: { + properties: { + nestedProperty: { + type: "string", + enum: ["EXTERNAL", "THREE"], + }, + }, + }, + then: { + required: ["otherBooleanProperty"], + properties: { otherBooleanProperty: { type: "boolean" }}, + }, + }, + { + if: { + properties: { + nestedProperty: { + type: "string", + enum: ["EXTERNAL", "THREE"], + }, + otherBooleanProperty: { const: false }, + }, + }, + then: { + properties: { + nestedOtherProperty: { + $bespokeKey: { + append: { + component: "Callout", + props: { type: "warning" }, + on: [["PROHIBITED", ["EXTERNAL", "TWO"]]], + }, + }, + not: { enum: ["EXTERNAL", "TWO"]}, + type: "string", + enum: ["EXTERNAL", "ONE"], + }, + }, + }, + }, + ], + }, + }, +}; diff --git a/test/specs/external-from-external/external-from-external.spec.js b/test/specs/external-from-external/external-from-external.spec.js new file mode 100644 index 00000000..2c287382 --- /dev/null +++ b/test/specs/external-from-external/external-from-external.spec.js @@ -0,0 +1,62 @@ +"use strict"; + +const { expect } = require("chai"); +const $RefParser = require("../../../lib"); +const helper = require("../../utils/helper"); +const path = require("../../utils/path"); +const parsedSchema = require("./parsed"); +const dereferencedSchema = require("./dereferenced"); +const bundledSchema = require("./bundled"); + +describe("Schema with external $refs to nested external files", () => { + it("should parse successfully", async () => { + let parser = new $RefParser(); + const schema = await parser.parse( + path.abs("specs/external-from-external/external-from-external.yaml") + ); + expect(schema).to.equal(parser.schema); + + expect(schema).to.deep.equal(parsedSchema.schema); + expect(parser.$refs.paths()).to.deep.equal([ + path.abs("specs/external-from-external/external-from-external.yaml"), + ]); + }); + + it( + "should resolve successfully", + helper.testResolve( + path.rel("specs/external-from-external/external-from-external.yaml"), + path.abs("specs/external-from-external/external-from-external.yaml"), + parsedSchema.schema, + path.abs("specs/external-from-external/page-one.yaml"), + parsedSchema.pageOne, + path.abs("specs/external-from-external/external-one.yaml"), + parsedSchema.externalOne, + path.abs("specs/external-from-external/external-two.yaml"), + parsedSchema.externalTwo, + path.abs("specs/external-from-external/external-three.yaml"), + parsedSchema.externalThree, + ) + ); + + it("should dereference successfully", async () => { + let parser = new $RefParser(); + const schema = await parser.dereference( + path.rel("specs/external-from-external/external-from-external.yaml") + ); + expect(schema).to.equal(parser.schema); + expect(schema).to.deep.equal(dereferencedSchema); + // The "circular" flag should NOT be set + expect(parser.$refs.circular).to.equal(false); + }); + + it("should bundle successfully", async () => { + let parser = new $RefParser(); + const schema = await parser.bundle( + path.rel("specs/external-from-external/external-from-external.yaml") + ); + console.log(JSON.stringify(schema)); + expect(schema).to.equal(parser.schema); + expect(schema).to.deep.equal(bundledSchema); + }); +}); diff --git a/test/specs/external-from-external/external-from-external.yaml b/test/specs/external-from-external/external-from-external.yaml new file mode 100644 index 00000000..9c248d5e --- /dev/null +++ b/test/specs/external-from-external/external-from-external.yaml @@ -0,0 +1,12 @@ +"$schema": http://json-schema.org/draft-07/schema# +type: object +definitions: + externalOne: + type: string + enum: + "$ref": "./external-one.yaml" + internalOne: + "$ref": "#/definitions/externalOne" +properties: + pageOne: + "$ref": "./page-one.yaml" diff --git a/test/specs/external-from-external/external-one.yaml b/test/specs/external-from-external/external-one.yaml new file mode 100644 index 00000000..1c5c8f43 --- /dev/null +++ b/test/specs/external-from-external/external-one.yaml @@ -0,0 +1,2 @@ +- EXTERNAL +- ONE diff --git a/test/specs/external-from-external/external-three.yaml b/test/specs/external-from-external/external-three.yaml new file mode 100644 index 00000000..7d3b486d --- /dev/null +++ b/test/specs/external-from-external/external-three.yaml @@ -0,0 +1,2 @@ +- EXTERNAL +- THREE diff --git a/test/specs/external-from-external/external-two.yaml b/test/specs/external-from-external/external-two.yaml new file mode 100644 index 00000000..7bf973d0 --- /dev/null +++ b/test/specs/external-from-external/external-two.yaml @@ -0,0 +1,2 @@ +- EXTERNAL +- TWO diff --git a/test/specs/external-from-external/page-one.yaml b/test/specs/external-from-external/page-one.yaml new file mode 100644 index 00000000..5bbffa74 --- /dev/null +++ b/test/specs/external-from-external/page-one.yaml @@ -0,0 +1,52 @@ +type: object +properties: + nestedProperty: + "$ref": "./external-from-external.yaml#/definitions/externalOne" + "$bespokeKey": + append: + component: Callout + props: + type: warning + 'on': + - GB + - - PROHIBITED + - "$ref": "./external-two.yaml" + not: + enum: + "$ref": "./external-two.yaml" +allOf: +- if: + properties: + nestedProperty: + type: string + enum: + "$ref": "./external-three.yaml" + then: + required: + - otherBooleanProperty + properties: + otherBooleanProperty: + type: boolean +- if: + properties: + nestedProperty: + type: string + enum: + "$ref": "./external-three.yaml" + otherBooleanProperty: + const: false + then: + properties: + nestedOtherProperty: + "$ref": "./external-from-external.yaml#/definitions/externalOne" + "$bespokeKey": + append: + component: Callout + props: + type: warning + 'on': + - - PROHIBITED + - "$ref": "./external-two.yaml" + not: + enum: + "$ref": "./external-two.yaml" diff --git a/test/specs/external-from-external/parsed.js b/test/specs/external-from-external/parsed.js new file mode 100644 index 00000000..69c3f9b9 --- /dev/null +++ b/test/specs/external-from-external/parsed.js @@ -0,0 +1,134 @@ +"use strict"; + +module.exports = { + schema: { + $schema: "http://json-schema.org/draft-07/schema#", + type: "object", + definitions: { + externalOne: { + type: "string", + enum: { + $ref: "./external-one.yaml", + }, + }, + internalOne: { + $ref: "#/definitions/externalOne", + }, + }, + properties: { + pageOne: { + $ref: "./page-one.yaml", + }, + }, + }, + + pageOne: { + type: "object", + properties: { + nestedProperty: { + $ref: "./external-from-external.yaml#/definitions/externalOne", + $bespokeKey: { + append: { + component: "Callout", + props: { + type: "warning", + }, + on: [ + "GB", + [ + "PROHIBITED", + { + $ref: "./external-two.yaml", + }, + ], + ], + }, + }, + not: { + enum: { + $ref: "./external-two.yaml", + }, + }, + }, + }, + allOf: [ + { + if: { + properties: { + nestedProperty: { + type: "string", + enum: { + $ref: "./external-three.yaml", + }, + }, + }, + }, + then: { + required: ["otherBooleanProperty"], + properties: { + otherBooleanProperty: { + type: "boolean", + }, + }, + }, + }, + { + if: { + properties: { + nestedProperty: { + type: "string", + enum: { + $ref: "./external-three.yaml", + }, + }, + otherBooleanProperty: { + const: false, + }, + }, + }, + then: { + properties: { + nestedOtherProperty: { + $ref: "./external-from-external.yaml#/definitions/externalOne", + $bespokeKey: { + append: { + component: "Callout", + props: { + type: "warning", + }, + on: [ + [ + "PROHIBITED", + { + $ref: "./external-two.yaml", + }, + ], + ], + }, + }, + not: { + enum: { + $ref: "./external-two.yaml", + }, + }, + }, + }, + }, + }, + ], + }, + + externalOne: [ + "EXTERNAL", + "ONE" + ], + + externalTwo: [ + "EXTERNAL", + "TWO" + ], + externalThree: [ + "EXTERNAL", + "THREE" + ] +}; diff --git a/test/specs/missing-pointers/missing-pointers.spec.js b/test/specs/missing-pointers/missing-pointers.spec.js index 4270bee9..c0670cfc 100644 --- a/test/specs/missing-pointers/missing-pointers.spec.js +++ b/test/specs/missing-pointers/missing-pointers.spec.js @@ -67,7 +67,9 @@ describe("Schema with missing pointers", () => { expect(err.files.$refs._root$Ref.value).to.deep.equal({ foo: { internal1: null, - internal2: null, + internal2: { + $ref: "#/external" + }, } }); expect(err.message).to.have.string("1 error occurred while reading '");