Skip to content
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.

Sign up for GitHub

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

Fix default values and matching schemas when oneOf / anyOf subschemas contain references #2272

Merged
merged 21 commits into from
Apr 13, 2021
Merged

Fix default values and matching schemas when oneOf / anyOf subschemas contain references #2272

Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e7a7705
fix: title from refs schema before fallback value
NixBiks Mar 10, 2021
50d92d3
Added test for #2270
NixBiks Mar 10, 2021
0a0c4c4
After review
NixBiks Mar 11, 2021
ac7f428
Add formData when dereferencing
NixBiks Mar 11, 2021
bd50850
Moved logic to SchemaField, so props.options are already dereferenced
NixBiks Mar 12, 2021
18874be
Removed wrong comment
NixBiks Mar 12, 2021
54ce2ac
Fix: refs should be included when determining matching option
NixBiks Mar 12, 2021
2bf49c0
Integrated tests from PR #2245
NixBiks Mar 16, 2021
bbf4a94
Added a new test
NixBiks Mar 22, 2021
8094961
Updated tests after review
NixBiks Mar 23, 2021
7992cae
Update isValid to take rootSchema
NixBiks Mar 24, 2021
46b8a4d
* use global ajv in isValid
NixBiks Mar 24, 2021
ce9be42
Merge branch 'master' into schema-from-formdata
NixBiks Mar 24, 2021
6e92854
Added test and fixed withIdRefPrefix for arrays
NixBiks Mar 24, 2021
126982c
* Cleaned up
NixBiks Mar 24, 2021
677fb3e
* Clean up
NixBiks Mar 25, 2021
90968fd
Added upgrade note
NixBiks Mar 26, 2021
1662f0f
Change title of test in validate_test
NixBiks Apr 13, 2021
1e3eed9
Change title of test in validate_test
NixBiks Apr 13, 2021
7212263
Added test with ref within properties
NixBiks Apr 13, 2021
ff93e1f
Merge branch 'master' into schema-from-formdata
NixBiks Apr 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions packages/core/src/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,25 +263,17 @@ 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"
NixBiks marked this conversation as resolved.
Show resolved Hide resolved
* This is used in isValid to make references to the rootSchema
*/
function withIdRefPrefix(schema) {
NixBiks marked this conversation as resolved.
Show resolved Hide resolved
const obj = { ...schema };
for (let key of Object.keys(obj)) {
const value = obj[key];
if (key === "$ref") {
obj[key] = "__rjsf_rootSchema" + value;
NixBiks marked this conversation as resolved.
Show resolved Hide resolved
} else if (isDict(value)) {
} else if (value.constructor === Object) {
obj[key] = withIdRefPrefix(value);
}
}
Expand All @@ -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) {
NixBiks marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rootSchema is always given -- so maybe we can just remove this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - except for some tests, but I've just updated those tests to add the schema as rootSchema as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively I could leave the tests as is and have this instead

return ajv
  .addSchema(rootSchema || schema, ROOT_SCHEMA_PREFIX)
  .validate(withIdRefPrefix(schema), data);

So if no rootSchema is provided then the schema itself will be used. Any preferences with one or the other?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to just go ahead and update the tests rather than adding unnecessary functionality to this function.

return createAjvInstance()
const result = ajv
NixBiks marked this conversation as resolved.
Show resolved Hide resolved
.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;
}
NixBiks marked this conversation as resolved.
Show resolved Hide resolved
}