Skip to content

#226: fix for external-from-external dependencies (exposed by addition of dereferencedCache) #230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
56 changes: 26 additions & 30 deletions lib/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
57 changes: 28 additions & 29 deletions lib/dereference.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 7 additions & 11 deletions lib/resolve-external.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
92 changes: 92 additions & 0 deletions test/specs/external-from-external/bundled.js
Original file line number Diff line number Diff line change
@@ -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",
},
},
},
},
},
},
],
},
},
};
72 changes: 72 additions & 0 deletions test/specs/external-from-external/dereferenced.js
Original file line number Diff line number Diff line change
@@ -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"],
},
},
},
},
],
},
},
};
62 changes: 62 additions & 0 deletions test/specs/external-from-external/external-from-external.spec.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading