From e7a770511abec7f1ef21b80189347037053993c0 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Wed, 10 Mar 2021 21:09:02 +0100 Subject: [PATCH 01/19] fix: title from refs schema before fallback value --- packages/core/src/components/fields/MultiSchemaField.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/fields/MultiSchemaField.js b/packages/core/src/components/fields/MultiSchemaField.js index 11ec9afe81..aefd90c9cf 100644 --- a/packages/core/src/components/fields/MultiSchemaField.js +++ b/packages/core/src/components/fields/MultiSchemaField.js @@ -132,7 +132,10 @@ class AnyOfField extends Component { } const enumOptions = options.map((option, index) => ({ - label: option.title || `Option ${index + 1}`, + label: + option.title || + retrieveSchema(option, registry.rootSchema).title || + `Option ${index + 1}`, value: index, })); From 50d92d3f5f0aa4ad43e0cabea36c7d2e2b203a06 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Wed, 10 Mar 2021 21:37:55 +0100 Subject: [PATCH 02/19] Added test for #2270 --- packages/core/test/anyOf_test.js | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/core/test/anyOf_test.js b/packages/core/test/anyOf_test.js index a7bc43e388..5a4349456b 100644 --- a/packages/core/test/anyOf_test.js +++ b/packages/core/test/anyOf_test.js @@ -639,6 +639,52 @@ describe("anyOf", () => { }); }); + it("should use title from refs schema before using fallback generated value as title", () => { + const schema = { + definitions: { + address: { + title: "Address", + type: "object", + properties: { + street: { + title: "Street", + type: "string", + }, + }, + }, + person: { + title: "Person", + type: "object", + properties: { + name: { + title: "Name", + type: "string", + }, + }, + }, + nested: { + $ref: "#/definitions/person", + }, + }, + anyOf: [ + { + $ref: "#/definitions/address", + }, + { + $ref: "#/definitions/nested", + }, + ], + }; + + const { node } = createFormComponent({ + schema, + }); + + let options = node.querySelectorAll("option"); + expect(options[0].firstChild.nodeValue).eql("Address"); + expect(options[1].firstChild.nodeValue).eql("Person"); + }); + describe("Arrays", () => { it("should correctly render form inputs for anyOf inside array items", () => { const schema = { From 0a0c4c4dcc2668d2b828ad1eb4ea289a1b023191 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Thu, 11 Mar 2021 10:10:37 +0100 Subject: [PATCH 03/19] After review --- .../core/src/components/fields/MultiSchemaField.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/components/fields/MultiSchemaField.js b/packages/core/src/components/fields/MultiSchemaField.js index aefd90c9cf..b989dd83a5 100644 --- a/packages/core/src/components/fields/MultiSchemaField.js +++ b/packages/core/src/components/fields/MultiSchemaField.js @@ -108,7 +108,6 @@ class AnyOfField extends Component { onBlur, onChange, onFocus, - options, registry, uiSchema, schema, @@ -120,6 +119,10 @@ class AnyOfField extends Component { const { widget = "select", ...uiOptions } = getUiOptions(uiSchema); const Widget = getWidget({ type: "number" }, widget, widgets); + // get the dereference schemas + const options = this.props.options.map(option => + retrieveSchema(option, registry.rootSchema) + ); const option = options[selectedOption] || null; let optionSchema; @@ -132,10 +135,7 @@ class AnyOfField extends Component { } const enumOptions = options.map((option, index) => ({ - label: - option.title || - retrieveSchema(option, registry.rootSchema).title || - `Option ${index + 1}`, + label: option.title || `Option ${index + 1}`, value: index, })); From ac7f42819106f6c75e1044377a79537a4e627fd1 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Thu, 11 Mar 2021 10:52:09 +0100 Subject: [PATCH 04/19] Add formData when dereferencing --- packages/core/src/components/fields/MultiSchemaField.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/fields/MultiSchemaField.js b/packages/core/src/components/fields/MultiSchemaField.js index b989dd83a5..c14bbb9489 100644 --- a/packages/core/src/components/fields/MultiSchemaField.js +++ b/packages/core/src/components/fields/MultiSchemaField.js @@ -121,7 +121,7 @@ class AnyOfField extends Component { // get the dereference schemas const options = this.props.options.map(option => - retrieveSchema(option, registry.rootSchema) + retrieveSchema(option, registry.rootSchema, formData) ); const option = options[selectedOption] || null; let optionSchema; From bd50850f7142cd4ceb68f9fabd8fd8482085dcde Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Fri, 12 Mar 2021 12:46:55 +0100 Subject: [PATCH 05/19] Moved logic to SchemaField, so props.options are already dereferenced --- packages/core/src/components/fields/MultiSchemaField.js | 4 +--- packages/core/src/components/fields/SchemaField.js | 8 ++++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core/src/components/fields/MultiSchemaField.js b/packages/core/src/components/fields/MultiSchemaField.js index c14bbb9489..f998fb92a7 100644 --- a/packages/core/src/components/fields/MultiSchemaField.js +++ b/packages/core/src/components/fields/MultiSchemaField.js @@ -108,6 +108,7 @@ class AnyOfField extends Component { onBlur, onChange, onFocus, + options, registry, uiSchema, schema, @@ -120,9 +121,6 @@ class AnyOfField extends Component { const Widget = getWidget({ type: "number" }, widget, widgets); // get the dereference schemas - const options = this.props.options.map(option => - retrieveSchema(option, registry.rootSchema, formData) - ); const option = options[selectedOption] || null; let optionSchema; diff --git a/packages/core/src/components/fields/SchemaField.js b/packages/core/src/components/fields/SchemaField.js index 526c008b28..5f4087be23 100644 --- a/packages/core/src/components/fields/SchemaField.js +++ b/packages/core/src/components/fields/SchemaField.js @@ -362,7 +362,9 @@ function SchemaFieldRender(props) { onBlur={props.onBlur} onChange={props.onChange} onFocus={props.onFocus} - options={schema.anyOf} + options={schema.anyOf.map(_schema => + retrieveSchema(_schema, rootSchema, formData) + )} baseType={schema.type} registry={registry} schema={schema} @@ -380,7 +382,9 @@ function SchemaFieldRender(props) { onBlur={props.onBlur} onChange={props.onChange} onFocus={props.onFocus} - options={schema.oneOf} + options={schema.oneOf.map(_schema => + retrieveSchema(_schema, rootSchema, formData) + )} baseType={schema.type} registry={registry} schema={schema} From 18874be60afd082794f995d642d3e15c2ff08b80 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Fri, 12 Mar 2021 12:50:26 +0100 Subject: [PATCH 06/19] Removed wrong comment --- packages/core/src/components/fields/MultiSchemaField.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/components/fields/MultiSchemaField.js b/packages/core/src/components/fields/MultiSchemaField.js index f998fb92a7..11ec9afe81 100644 --- a/packages/core/src/components/fields/MultiSchemaField.js +++ b/packages/core/src/components/fields/MultiSchemaField.js @@ -120,7 +120,6 @@ class AnyOfField extends Component { const { widget = "select", ...uiOptions } = getUiOptions(uiSchema); const Widget = getWidget({ type: "number" }, widget, widgets); - // get the dereference schemas const option = options[selectedOption] || null; let optionSchema; From 54ce2ac2a619b995983c1e57143a6340d8ac1d50 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Fri, 12 Mar 2021 17:35:22 +0100 Subject: [PATCH 07/19] Fix: refs should be included when determining matching option based on formData --- packages/core/src/utils.js | 15 ++++++++++--- packages/core/test/utils_test.js | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 7bece94742..ac666e0f09 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -1200,7 +1200,16 @@ export function getMatchingOption(formData, options, rootSchema) { })), }; - let augmentedSchema; + // start augmentedSchema as an object to hold only references + let { + type, + anyOf, + $ref, + properties, + items, + title, + ...augmentedSchema + } = rootSchema; // If the "anyOf" keyword already exists, wrap the augmentation in an "allOf" if (option.anyOf) { @@ -1216,9 +1225,9 @@ export function getMatchingOption(formData, options, rootSchema) { shallowClone.allOf.push(requiresAnyOf); - augmentedSchema = shallowClone; + augmentedSchema = Object.assign(augmentedSchema, shallowClone); } else { - augmentedSchema = Object.assign({}, option, requiresAnyOf); + augmentedSchema = Object.assign(augmentedSchema, option, requiresAnyOf); } // Remove the "required" field as it's likely that not all fields have diff --git a/packages/core/test/utils_test.js b/packages/core/test/utils_test.js index 200ce167cc..d7c554141c 100644 --- a/packages/core/test/utils_test.js +++ b/packages/core/test/utils_test.js @@ -29,6 +29,7 @@ import { schemaRequiresTrueValue, canExpand, optionsList, + getMatchingOption, } from "../src/utils"; import { createSandbox } from "./test_utils"; @@ -3827,5 +3828,41 @@ describe("utils", () => { })) ); }); + it("should infer correct anyOf schema based on data", () => { + const rootSchema = { + defs: { + a: { type: "object", properties: { id: { enum: ["a"] } } }, + nested: { + type: "object", + properties: { + id: { enum: ["nested"] }, + child: { $ref: "#/defs/any" }, + }, + }, + any: { anyOf: [{ $ref: "#/defs/a" }, { $ref: "#/defs/nested" }] }, + }, + $ref: "#/defs/any", + }; + const options = [ + { type: "object", properties: { id: { enum: ["a"] } } }, + { + type: "object", + properties: { + id: { enum: ["nested"] }, + child: { $ref: "#/defs/any" }, + }, + }, + ]; + const formData = { + id: "nested", + child: { + id: "nested", + child: { + id: "a", + }, + }, + }; + expect(getMatchingOption(formData, options, rootSchema)).eql(1); + }); }); }); From 2bf49c043ab05cdbac571d58a5ebc42a2106038d Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Tue, 16 Mar 2021 15:24:58 +0100 Subject: [PATCH 08/19] Integrated tests from PR #2245 --- packages/core/test/anyOf_test.js | 47 +++++++++++++++++++++++++++++++- packages/core/test/oneOf_test.js | 47 ++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/packages/core/test/anyOf_test.js b/packages/core/test/anyOf_test.js index 5a4349456b..51e93d03d6 100644 --- a/packages/core/test/anyOf_test.js +++ b/packages/core/test/anyOf_test.js @@ -67,12 +67,17 @@ describe("anyOf", () => { }, }, { + $ref: "#/definitions/bar", + }, + ], + definitions: { + bar: { type: "object", properties: { foo: { type: "string", default: "defaultbar" }, }, }, - ], + }, }, }); sinon.assert.calledWithMatch(onChange.lastCall, { @@ -828,6 +833,46 @@ describe("anyOf", () => { expect(strInputs[1].value).eql("bar"); }); + it("should correctly set the label of the options", () => { + const schema = { + type: "object", + anyOf: [ + { + title: "Foo", + properties: { + foo: { type: "string" }, + }, + }, + { + properties: { + bar: { type: "string" }, + }, + }, + { + $ref: "#/definitions/baz", + }, + ], + definitions: { + baz: { + title: "Baz", + properties: { + baz: { type: "string" }, + }, + }, + }, + }; + + const { node } = createFormComponent({ + schema, + }); + + const $select = node.querySelector("select"); + + expect($select.options[0].text).eql("Foo"); + expect($select.options[1].text).eql("Option 2"); + expect($select.options[2].text).eql("Baz"); + }); + it("should correctly render mixed types for anyOf inside array items", () => { const schema = { type: "object", diff --git a/packages/core/test/oneOf_test.js b/packages/core/test/oneOf_test.js index 2060d99052..f606dfb689 100644 --- a/packages/core/test/oneOf_test.js +++ b/packages/core/test/oneOf_test.js @@ -66,13 +66,16 @@ describe("oneOf", () => { foo: { type: "string", default: "defaultfoo" }, }, }, - { + { $ref: "#/definitions/bar" }, + ], + definitions: { + bar: { type: "object", properties: { foo: { type: "string", default: "defaultbar" }, }, }, - ], + }, }, }); @@ -611,5 +614,45 @@ describe("oneOf", () => { expect($select.value).to.eql($select.options[1].value); }); + + it("should correctly set the label of the options", () => { + const schema = { + type: "object", + oneOf: [ + { + title: "Foo", + properties: { + foo: { type: "string" }, + }, + }, + { + properties: { + bar: { type: "string" }, + }, + }, + { + $ref: "#/definitions/baz", + }, + ], + definitions: { + baz: { + title: "Baz", + properties: { + baz: { type: "string" }, + }, + }, + }, + }; + + const { node } = createFormComponent({ + schema, + }); + + const $select = node.querySelector("select"); + + expect($select.options[0].text).eql("Foo"); + expect($select.options[1].text).eql("Option 2"); + expect($select.options[2].text).eql("Baz"); + }); }); }); From bbf4a9468bf83b785f3fd00bb8f7aa28397112d2 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Mon, 22 Mar 2021 10:51:08 +0100 Subject: [PATCH 09/19] Added a new test --- packages/core/src/utils.js | 6 ++- packages/core/test/anyOf_test.js | 85 ++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index ac666e0f09..13c561fd16 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -1208,6 +1208,8 @@ export function getMatchingOption(formData, options, rootSchema) { properties, items, title, + additionalItems, + additionalProperties, ...augmentedSchema } = rootSchema; @@ -1225,9 +1227,9 @@ export function getMatchingOption(formData, options, rootSchema) { shallowClone.allOf.push(requiresAnyOf); - augmentedSchema = Object.assign(augmentedSchema, shallowClone); + augmentedSchema = { ...augmentedSchema, ...shallowClone }; } else { - augmentedSchema = Object.assign(augmentedSchema, option, requiresAnyOf); + augmentedSchema = { ...augmentedSchema, ...option, ...requiresAnyOf }; } // Remove the "required" field as it's likely that not all fields have diff --git a/packages/core/test/anyOf_test.js b/packages/core/test/anyOf_test.js index 51e93d03d6..b396a00c41 100644 --- a/packages/core/test/anyOf_test.js +++ b/packages/core/test/anyOf_test.js @@ -918,5 +918,90 @@ describe("anyOf", () => { expect(node.querySelectorAll("input#root_foo")).to.have.length.of(1); expect(node.querySelectorAll("input#root_bar")).to.have.length.of(1); }); + + it("should correctly infer the selected option based on value", () => { + const schema = { + $ref: "#/defs/any", + defs: { + chain: { + type: "object", + title: "Chain", + properties: { + id: { + enum: ["chain"], + }, + components: { + type: "array", + items: { $ref: "#/defs/any" }, + }, + }, + }, + + map: { + type: "object", + title: "Map", + properties: { + id: { enum: ["map"] }, + fn: { $ref: "#/defs/any" }, + }, + }, + + to_absolute: { + type: "object", + title: "To Absolute", + properties: { + id: { enum: ["to_absolute"] }, + base_url: { type: "string" }, + }, + }, + + transform: { + type: "object", + title: "Transform", + properties: { + id: { enum: ["transform"] }, + property_key: { type: "string" }, + transformer: { $ref: "#/defs/any" }, + }, + }, + any: { + anyOf: [ + { $ref: "#/defs/chain" }, + { $ref: "#/defs/map" }, + { $ref: "#/defs/to_absolute" }, + { $ref: "#/defs/transform" }, + ], + }, + }, + }; + + const { node } = createFormComponent({ + schema, + formData: { + id: "chain", + components: [ + { + id: "map", + fn: { + id: "transform", + property_key: "uri", + transformer: { + id: "to_absolute", + base_url: "http://localhost", + }, + }, + }, + ], + }, + }); + + const idSelects = node.querySelectorAll("select#root_id"); + + expect(idSelects).to.have.length(4); + expect(idSelects[0].value).eql("chain"); + expect(idSelects[1].value).eql("map"); + expect(idSelects[2].value).eql("transform"); + expect(idSelects[3].value).eql("to_absolute"); + }); }); }); From 8094961006d6febc5f5c11569a906651bc6bdb82 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Tue, 23 Mar 2021 09:53:22 +0100 Subject: [PATCH 10/19] Updated tests after review --- packages/core/test/anyOf_test.js | 34 +++++++++ packages/core/test/oneOf_test.js | 120 +++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/packages/core/test/anyOf_test.js b/packages/core/test/anyOf_test.js index b396a00c41..af9dcc1795 100644 --- a/packages/core/test/anyOf_test.js +++ b/packages/core/test/anyOf_test.js @@ -57,6 +57,40 @@ describe("anyOf", () => { }); it("should assign a default value and set defaults on option change", () => { + const { node, onChange } = createFormComponent({ + schema: { + anyOf: [ + { + type: "object", + properties: { + foo: { type: "string", default: "defaultfoo" }, + }, + }, + { + type: "object", + properties: { + foo: { type: "string", default: "defaultbar" }, + }, + }, + ], + }, + }); + sinon.assert.calledWithMatch(onChange.lastCall, { + formData: { foo: "defaultfoo" }, + }); + + const $select = node.querySelector("select"); + + Simulate.change($select, { + target: { value: $select.options[1].value }, + }); + + sinon.assert.calledWithMatch(onChange.lastCall, { + formData: { foo: "defaultbar" }, + }); + }); + + it("should assign a default value and set defaults on option change when using references", () => { const { node, onChange } = createFormComponent({ schema: { anyOf: [ diff --git a/packages/core/test/oneOf_test.js b/packages/core/test/oneOf_test.js index f606dfb689..7bb795078c 100644 --- a/packages/core/test/oneOf_test.js +++ b/packages/core/test/oneOf_test.js @@ -57,6 +57,41 @@ describe("oneOf", () => { }); it("should assign a default value and set defaults on option change", () => { + const { node, onChange } = createFormComponent({ + schema: { + oneOf: [ + { + type: "object", + properties: { + foo: { type: "string", default: "defaultfoo" }, + }, + }, + { + type: "object", + properties: { + foo: { type: "string", default: "defaultbar" }, + }, + }, + ], + }, + }); + + sinon.assert.calledWithMatch(onChange.lastCall, { + formData: { foo: "defaultfoo" }, + }); + + const $select = node.querySelector("select"); + + Simulate.change($select, { + target: { value: $select.options[1].value }, + }); + + sinon.assert.calledWithMatch(onChange.lastCall, { + formData: { foo: "defaultbar" }, + }); + }); + + it("should assign a default value and set defaults on option change when using refs", () => { const { node, onChange } = createFormComponent({ schema: { oneOf: [ @@ -655,4 +690,89 @@ describe("oneOf", () => { expect($select.options[2].text).eql("Baz"); }); }); + + it("should correctly infer the selected option based on value", () => { + const schema = { + $ref: "#/defs/any", + defs: { + chain: { + type: "object", + title: "Chain", + properties: { + id: { + enum: ["chain"], + }, + components: { + type: "array", + items: { $ref: "#/defs/any" }, + }, + }, + }, + + map: { + type: "object", + title: "Map", + properties: { + id: { enum: ["map"] }, + fn: { $ref: "#/defs/any" }, + }, + }, + + to_absolute: { + type: "object", + title: "To Absolute", + properties: { + id: { enum: ["to_absolute"] }, + base_url: { type: "string" }, + }, + }, + + transform: { + type: "object", + title: "Transform", + properties: { + id: { enum: ["transform"] }, + property_key: { type: "string" }, + transformer: { $ref: "#/defs/any" }, + }, + }, + any: { + oneOf: [ + { $ref: "#/defs/chain" }, + { $ref: "#/defs/map" }, + { $ref: "#/defs/to_absolute" }, + { $ref: "#/defs/transform" }, + ], + }, + }, + }; + + const { node } = createFormComponent({ + schema, + formData: { + id: "chain", + components: [ + { + id: "map", + fn: { + id: "transform", + property_key: "uri", + transformer: { + id: "to_absolute", + base_url: "http://localhost", + }, + }, + }, + ], + }, + }); + + const idSelects = node.querySelectorAll("select#root_id"); + + expect(idSelects).to.have.length(4); + expect(idSelects[0].value).eql("chain"); + expect(idSelects[1].value).eql("map"); + expect(idSelects[2].value).eql("transform"); + expect(idSelects[3].value).eql("to_absolute"); + }); }); From 7992caeee36d7b2846fda1492b820bfc947ec2dd Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Wed, 24 Mar 2021 11:40:16 +0100 Subject: [PATCH 11/19] Update isValid to take rootSchema --- packages/core/src/utils.js | 21 +++++---------------- packages/core/src/validate.js | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 13c561fd16..e53fd07e46 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -1200,18 +1200,7 @@ export function getMatchingOption(formData, options, rootSchema) { })), }; - // start augmentedSchema as an object to hold only references - let { - type, - anyOf, - $ref, - properties, - items, - title, - additionalItems, - additionalProperties, - ...augmentedSchema - } = rootSchema; + let augmentedSchema; // If the "anyOf" keyword already exists, wrap the augmentation in an "allOf" if (option.anyOf) { @@ -1227,19 +1216,19 @@ export function getMatchingOption(formData, options, rootSchema) { shallowClone.allOf.push(requiresAnyOf); - augmentedSchema = { ...augmentedSchema, ...shallowClone }; + augmentedSchema = shallowClone; } else { - augmentedSchema = { ...augmentedSchema, ...option, ...requiresAnyOf }; + augmentedSchema = Object.assign({}, option, requiresAnyOf); } // Remove the "required" field as it's likely that not all fields have // been filled in yet, which will mean that the schema is not valid delete augmentedSchema.required; - if (isValid(augmentedSchema, formData)) { + if (isValid(augmentedSchema, formData, rootSchema)) { return i; } - } else if (isValid(options[i], formData)) { + } else if (isValid(option, formData, rootSchema)) { return i; } } diff --git a/packages/core/src/validate.js b/packages/core/src/validate.js index 73046208df..3973490996 100644 --- a/packages/core/src/validate.js +++ b/packages/core/src/validate.js @@ -263,14 +263,45 @@ export default function validateFormData( }; } +function isDict(v) { + return ( + typeof v === "object" && + v !== null && + !(v instanceof Array) && + !(v instanceof Date) + ); +} + +/** + * Get a similar schema where ref's are prefixed with "__rjsf_rootSchema" + */ +function withIdRefPrefix(schema) { + const obj = { ...schema }; + for (let key of Object.keys(obj)) { + const value = obj[key]; + if (key === "$ref") { + obj[key] = "__rjsf_rootSchema" + value; + } else if (isDict(value)) { + obj[key] = withIdRefPrefix(value); + } + } + return obj; +} + /** * Validates data against a schema, returning true if the data is valid, or * false otherwise. If the schema is invalid, then this function will return * false. */ -export function isValid(schema, data) { +export function isValid(schema, data, rootSchema) { try { - return ajv.validate(schema, data); + if (rootSchema) { + return createAjvInstance() + .addSchema(rootSchema, "__rjsf_rootSchema") + .validate(withIdRefPrefix(schema), data); + } else { + return ajv.validate(schema, data); + } } catch (e) { return false; } From 46b8a4d3cf40642991aa4a81687be238b48cde12 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Wed, 24 Mar 2021 14:02:16 +0100 Subject: [PATCH 12/19] * use global ajv in isValid * simplify withIdRefPrefix * add comments * improve error handling --- packages/core/src/validate.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/core/src/validate.js b/packages/core/src/validate.js index 3973490996..0d3e4dcf52 100644 --- a/packages/core/src/validate.js +++ b/packages/core/src/validate.js @@ -263,17 +263,9 @@ export default function validateFormData( }; } -function isDict(v) { - return ( - typeof v === "object" && - v !== null && - !(v instanceof Array) && - !(v instanceof Date) - ); -} - /** * Get a similar schema where ref's are prefixed with "__rjsf_rootSchema" + * This is used in isValid to make references to the rootSchema */ function withIdRefPrefix(schema) { const obj = { ...schema }; @@ -281,7 +273,7 @@ function withIdRefPrefix(schema) { const value = obj[key]; if (key === "$ref") { obj[key] = "__rjsf_rootSchema" + value; - } else if (isDict(value)) { + } else if (value.constructor === Object) { obj[key] = withIdRefPrefix(value); } } @@ -295,14 +287,28 @@ function withIdRefPrefix(schema) { */ export function isValid(schema, data, rootSchema) { try { + // if rootSchema is given then we add that schema with an id. + // then we rewrite the schema ref's to point to the rootSchema using the id + // this accounts for the case where schema have references to models + // that lives in the rootSchema but not in the schema in question. if (rootSchema) { - return createAjvInstance() + const result = ajv .addSchema(rootSchema, "__rjsf_rootSchema") .validate(withIdRefPrefix(schema), data); + + // make sure we remove the rootSchema from the global ajv instance + ajv.removeSchema("__rjsf_rootSchema"); + return result; } else { return ajv.validate(schema, data); } } catch (e) { + try { + // make sure we also remove the rootSchema if an error occured before removing but after adding + ajv.removeSchema("__rjsf_rootSchema"); + } catch (e) { + return false; + } return false; } } From 6e9285455c20e943ed8193b5ac3008f8999f5e1a Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Wed, 24 Mar 2021 17:12:36 +0100 Subject: [PATCH 13/19] Added test and fixed withIdRefPrefix for arrays --- packages/core/src/validate.js | 33 ++++++++++++++++++----------- packages/core/test/validate_test.js | 20 +++++++++++++++++ 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/core/src/validate.js b/packages/core/src/validate.js index 0d3e4dcf52..3edfae5c8e 100644 --- a/packages/core/src/validate.js +++ b/packages/core/src/validate.js @@ -5,6 +5,7 @@ import { deepEquals, getDefaultFormState } from "./utils"; let formerCustomFormats = null; let formerMetaSchema = null; +const rootSchemaId = "__rjsf_rootSchema"; import { isObject, mergeObjects } from "./utils"; @@ -264,17 +265,25 @@ export default function validateFormData( } /** - * Get a similar schema where ref's are prefixed with "__rjsf_rootSchema" + * Get a similar schema where ref's are prefixed with `rootSchemaId` * This is used in isValid to make references to the rootSchema */ -function withIdRefPrefix(schema) { - const obj = { ...schema }; - for (let key of Object.keys(obj)) { - const value = obj[key]; - if (key === "$ref") { - obj[key] = "__rjsf_rootSchema" + value; - } else if (value.constructor === Object) { - obj[key] = withIdRefPrefix(value); +function withIdRefPrefix(schemaNode) { + let obj = schemaNode; + if (schemaNode.constructor === Object) { + obj = { ...schemaNode }; + for (let key of Object.keys(obj)) { + const value = obj[key]; + if (key === "$ref") { + obj[key] = rootSchemaId + value; + } else { + obj[key] = withIdRefPrefix(value); + } + } + } else if (Array.isArray(schemaNode)) { + obj = [...schemaNode]; + for (var i = 0; i < obj.length; i++) { + obj[i] = withIdRefPrefix(obj[i]); } } return obj; @@ -293,11 +302,11 @@ export function isValid(schema, data, rootSchema) { // that lives in the rootSchema but not in the schema in question. if (rootSchema) { const result = ajv - .addSchema(rootSchema, "__rjsf_rootSchema") + .addSchema(rootSchema, rootSchemaId) .validate(withIdRefPrefix(schema), data); // make sure we remove the rootSchema from the global ajv instance - ajv.removeSchema("__rjsf_rootSchema"); + ajv.removeSchema(rootSchemaId); return result; } else { return ajv.validate(schema, data); @@ -305,7 +314,7 @@ export function isValid(schema, data, rootSchema) { } catch (e) { try { // make sure we also remove the rootSchema if an error occured before removing but after adding - ajv.removeSchema("__rjsf_rootSchema"); + ajv.removeSchema(rootSchemaId); } catch (e) { return false; } diff --git a/packages/core/test/validate_test.js b/packages/core/test/validate_test.js index 70d4e9398a..9abe097bee 100644 --- a/packages/core/test/validate_test.js +++ b/packages/core/test/validate_test.js @@ -35,6 +35,26 @@ describe("Validation", () => { expect(isValid(schema, { foo: "bar" })).to.be.false; }); + + it("should return true if the data is valid against the schema including refs to rootSchema", () => { + const schema = { + anyOf: [{ $ref: "#/defs/foo" }], + }; + const rootSchema = { + defs: { + foo: { + properties: { + name: { type: "string" }, + }, + }, + }, + }; + const formData = { + name: "John Doe", + }; + + expect(isValid(schema, formData, rootSchema)).to.be.true; + }); }); describe("validate.validateFormData()", () => { From 126982c960326fef6c8b746e7c48a4cc9c643d95 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Wed, 24 Mar 2021 23:56:44 +0100 Subject: [PATCH 14/19] * Cleaned up * Fixed bug with $ref being a property * added tests for withIdRefPrefix --- packages/core/src/validate.js | 26 +++++++---------- packages/core/test/validate_test.js | 44 ++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/core/src/validate.js b/packages/core/src/validate.js index 3edfae5c8e..eaf7883760 100644 --- a/packages/core/src/validate.js +++ b/packages/core/src/validate.js @@ -5,7 +5,7 @@ import { deepEquals, getDefaultFormState } from "./utils"; let formerCustomFormats = null; let formerMetaSchema = null; -const rootSchemaId = "__rjsf_rootSchema"; +const ROOT_SCHEMA_PREFIX = "__rjsf_rootSchema"; import { isObject, mergeObjects } from "./utils"; @@ -265,17 +265,17 @@ export default function validateFormData( } /** - * Get a similar schema where ref's are prefixed with `rootSchemaId` + * Recursively prefixes all $ref's in a schema with `ROOT_SCHEMA_PREFIX` * This is used in isValid to make references to the rootSchema */ -function withIdRefPrefix(schemaNode) { +export function withIdRefPrefix(schemaNode) { let obj = schemaNode; if (schemaNode.constructor === Object) { obj = { ...schemaNode }; - for (let key of Object.keys(obj)) { + for (const key in obj) { const value = obj[key]; - if (key === "$ref") { - obj[key] = rootSchemaId + value; + if (key === "$ref" && typeof value === "string") { + obj[key] = ROOT_SCHEMA_PREFIX + value; } else { obj[key] = withIdRefPrefix(value); } @@ -302,22 +302,16 @@ export function isValid(schema, data, rootSchema) { // that lives in the rootSchema but not in the schema in question. if (rootSchema) { const result = ajv - .addSchema(rootSchema, rootSchemaId) + .addSchema(rootSchema, ROOT_SCHEMA_PREFIX) .validate(withIdRefPrefix(schema), data); - - // make sure we remove the rootSchema from the global ajv instance - ajv.removeSchema(rootSchemaId); return result; } else { return ajv.validate(schema, data); } } catch (e) { - try { - // make sure we also remove the rootSchema if an error occured before removing but after adding - ajv.removeSchema(rootSchemaId); - } catch (e) { - return false; - } return false; + } finally { + // make sure we remove the rootSchema from the global ajv instance + ajv.removeSchema(ROOT_SCHEMA_PREFIX); } } diff --git a/packages/core/test/validate_test.js b/packages/core/test/validate_test.js index 9abe097bee..86296c7e1d 100644 --- a/packages/core/test/validate_test.js +++ b/packages/core/test/validate_test.js @@ -3,7 +3,11 @@ import { expect } from "chai"; import sinon from "sinon"; import { Simulate } from "react-dom/test-utils"; -import validateFormData, { isValid, toErrorList } from "../src/validate"; +import validateFormData, { + isValid, + toErrorList, + withIdRefPrefix, +} from "../src/validate"; import { createFormComponent, submitForm } from "./test_utils"; describe("Validation", () => { @@ -57,6 +61,44 @@ describe("Validation", () => { }); }); + describe("validate.withIdRefPrefix", () => { + it("should recursively add id prefix to all refs", () => { + const schema = { + anyOf: [{ $ref: "#/defs/foo" }], + }; + const expected = { + anyOf: [{ $ref: "__rjsf_rootSchema#/defs/foo" }], + }; + + expect(withIdRefPrefix(schema)).to.eql(expected); + }); + + it("shouldn't mutated the schema", () => { + const schema = { + anyOf: [{ $ref: "#/defs/foo" }], + }; + + withIdRefPrefix(schema); + + expect(schema).to.eql({ + anyOf: [{ $ref: "#/defs/foo" }], + }); + }); + + it("should not change the ref since it's a property", () => { + const schema = { + title: "A registration form", + description: "A simple form example.", + type: "object", + properties: { + $ref: { type: "string", title: "First name", default: "Chuck" }, + }, + }; + + expect(withIdRefPrefix(schema)).to.eql(schema); + }); + }); + describe("validate.validateFormData()", () => { describe("No custom validate function", () => { const illFormedKey = "bar.'\"[]()=+*&^%$#@!"; From 677fb3e8277399e85bcbe635db700aa13a0c004b Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Thu, 25 Mar 2021 22:54:26 +0100 Subject: [PATCH 15/19] * Clean up * Check for # in ref * update tests --- packages/core/src/validate.js | 21 ++++++++++----------- packages/core/test/validate_test.js | 6 +++--- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/core/src/validate.js b/packages/core/src/validate.js index eaf7883760..f2bb6b9887 100644 --- a/packages/core/src/validate.js +++ b/packages/core/src/validate.js @@ -274,7 +274,11 @@ export function withIdRefPrefix(schemaNode) { obj = { ...schemaNode }; for (const key in obj) { const value = obj[key]; - if (key === "$ref" && typeof value === "string") { + if ( + key === "$ref" && + typeof value === "string" && + value.startsWith("#") + ) { obj[key] = ROOT_SCHEMA_PREFIX + value; } else { obj[key] = withIdRefPrefix(value); @@ -296,18 +300,13 @@ export function withIdRefPrefix(schemaNode) { */ export function isValid(schema, data, rootSchema) { try { - // if rootSchema is given then we add that schema with an id. - // then we rewrite the schema ref's to point to the rootSchema using the id + // add the rootSchema ROOT_SCHEMA_PREFIX as id. + // then rewrite the schema ref's to point to the rootSchema // this accounts for the case where schema have references to models // that lives in the rootSchema but not in the schema in question. - if (rootSchema) { - const result = ajv - .addSchema(rootSchema, ROOT_SCHEMA_PREFIX) - .validate(withIdRefPrefix(schema), data); - return result; - } else { - return ajv.validate(schema, data); - } + return ajv + .addSchema(rootSchema, ROOT_SCHEMA_PREFIX) + .validate(withIdRefPrefix(schema), data); } catch (e) { return false; } finally { diff --git a/packages/core/test/validate_test.js b/packages/core/test/validate_test.js index 86296c7e1d..fa6afaf0dd 100644 --- a/packages/core/test/validate_test.js +++ b/packages/core/test/validate_test.js @@ -20,7 +20,7 @@ describe("Validation", () => { }, }; - expect(isValid(schema, { foo: "bar" })).to.be.true; + expect(isValid(schema, { foo: "bar" }, schema)).to.be.true; }); it("should return false if the data is not valid against the schema", () => { @@ -31,13 +31,13 @@ describe("Validation", () => { }, }; - expect(isValid(schema, { foo: 12345 })).to.be.false; + expect(isValid(schema, { foo: 12345 }, schema)).to.be.false; }); it("should return false if the schema is invalid", () => { const schema = "foobarbaz"; - expect(isValid(schema, { foo: "bar" })).to.be.false; + expect(isValid(schema, { foo: "bar" }, schema)).to.be.false; }); it("should return true if the data is valid against the schema including refs to rootSchema", () => { From 90968fd2b65bf67a38458b7c737ae78e14fd243b Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Fri, 26 Mar 2021 09:23:09 +0100 Subject: [PATCH 16/19] Added upgrade note --- docs/3.x upgrade guide.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/3.x upgrade guide.md b/docs/3.x upgrade guide.md index ad362ad384..479e7887c4 100644 --- a/docs/3.x upgrade guide.md +++ b/docs/3.x upgrade guide.md @@ -15,3 +15,11 @@ For a slightly more elaborate setup, [@babel/preset-env](https://babeljs.io/docs From `@babel/preset-env`'s docs > We leverage [`browserslist`, `compat-table`, and `electron-to-chromium`] to maintain mappings of which version of our supported target environments gained support of a JavaScript syntax or browser feature, as well as a mapping of those syntaxes and features to Babel transform plugins and core-js polyfills. + +### Dereferenced schemas for `anyOf`/`allOf` options + +`options` prop for `MultiSchemaField` has been changed slightly. + +Before an option could include a `$ref`. + +Now any option with a reference will be resolved/dereferenced when given as props for `MultiSchemaField`. From 1662f0f2bfb9027c0ab023d3437cfd168a912371 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Tue, 13 Apr 2021 09:31:53 +0200 Subject: [PATCH 17/19] Change title of test in validate_test Co-authored-by: Ashwin Ramaswami --- packages/core/test/validate_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/validate_test.js b/packages/core/test/validate_test.js index fa6afaf0dd..6dc528c129 100644 --- a/packages/core/test/validate_test.js +++ b/packages/core/test/validate_test.js @@ -85,7 +85,7 @@ describe("Validation", () => { }); }); - it("should not change the ref since it's a property", () => { + it("should not change a property named '$ref'", () => { const schema = { title: "A registration form", description: "A simple form example.", From 1e3eed91908a3b153e991271c3220b9396868db9 Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Tue, 13 Apr 2021 09:32:13 +0200 Subject: [PATCH 18/19] Change title of test in validate_test Co-authored-by: Ashwin Ramaswami --- packages/core/test/validate_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/validate_test.js b/packages/core/test/validate_test.js index 6dc528c129..9c11778643 100644 --- a/packages/core/test/validate_test.js +++ b/packages/core/test/validate_test.js @@ -73,7 +73,7 @@ describe("Validation", () => { expect(withIdRefPrefix(schema)).to.eql(expected); }); - it("shouldn't mutated the schema", () => { + it("shouldn't mutate the schema", () => { const schema = { anyOf: [{ $ref: "#/defs/foo" }], }; From 7212263e8c26a0c0c7e6e9f2d46b78e0ba60cfdf Mon Sep 17 00:00:00 2001 From: Nicolai Bjerre Pedersen Date: Tue, 13 Apr 2021 09:40:36 +0200 Subject: [PATCH 19/19] Added test with ref within properties --- packages/core/test/anyOf_test.js | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/core/test/anyOf_test.js b/packages/core/test/anyOf_test.js index af9dcc1795..382ed76dc5 100644 --- a/packages/core/test/anyOf_test.js +++ b/packages/core/test/anyOf_test.js @@ -724,6 +724,52 @@ describe("anyOf", () => { expect(options[1].firstChild.nodeValue).eql("Person"); }); + it("should collect schema from $ref even when ref is within properties", () => { + const schema = { + properties: { + address: { + title: "Address", + type: "object", + properties: { + street: { + title: "Street", + type: "string", + }, + }, + }, + person: { + title: "Person", + type: "object", + properties: { + name: { + title: "Name", + type: "string", + }, + }, + }, + nested: { + $ref: "#/properties/person", + }, + }, + anyOf: [ + { + $ref: "#/properties/address", + }, + { + $ref: "#/properties/nested", + }, + ], + }; + + const { node } = createFormComponent({ + schema, + }); + + let options = node.querySelectorAll("option"); + expect(options[0].firstChild.nodeValue).eql("Address"); + expect(options[1].firstChild.nodeValue).eql("Person"); + }); + describe("Arrays", () => { it("should correctly render form inputs for anyOf inside array items", () => { const schema = {