-
Notifications
You must be signed in to change notification settings - Fork 167
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
draft 4 schema doesn't require #38
Comments
Just for anyone else who wants to get around this as well, here is a quick fix option that will set the required flag on all properties based upon the required root object array: var enhanceRequired = function(root){
var required;
if(typeof(root)!=='object'){
return;
}
if((root.required instanceof Array)||(root.required&&root.required.length)){
required = Array.prototype.slice.call(root.required);
}else{
required = [];
}
if(root.properties){
var keys = Object.keys(root.properties), value;
keys.forEach(function(key){
value = root.properties[key];
enhanceRequired(value);
if(required.indexOf(key)>-1){
value.required = true;
}
});
}
}
enhanceRequired(schema); |
Why was "required" changed from an object property to be a separate array on the schema in v4? It's a horrible change. It's much simpler and cleanly encapsulated in the v3 spec. |
krisnye, in Version 4, the key "required" has been removed from inside the property because in some cases you need to say that a "foo" property is required even if it is not formally declared as property of the schema. An object can be set as "AdditionalProperties = true" and require the "foo" property, which is not declared. |
That is even worse. Why don't you just declare the foo property if you need it required? By putting it into the required property you are essentially making a mini-declaration about that property. Now instead of having a single place to look for property declarations you have two potential places. |
The fact of not wanting to declare the property "foo" is just one example among many. Let's assume you have a "customer" object, and inside it you have the customer data and various addresses like "commercial, residential, contact...". In this case you will declare only one address scheme on "definitions" so all point to it using "$ref", e.g. "{commercial:{$ref:"/definitions/address"}}". If you say "/definitions/address" is required using a "required" key within it, all addresses properties that point to it would be required, but in this case you just want the "residential" address to be required, not all. It makes more sense to think "I'm an object that requires ["X","Y","Z"] properties"(like v4) than "I'm a required property of any object that i'm inside"(like it was in v3). |
That makes sense, but why not be able to override referenced schemas. {commercial:{$ref:"/definitions/address", required: true}} or {commercial:{$ref:"/definitions/address", required: false}} |
You can't do that. The purpose of "$ref" is to replace the current value by the referenced value, so, the atribute "required" would be ignored. The key "$ref" is based on JSON Reference (http://tools.ietf.org/html/draft-pbryan-zyp-json-ref-03) where it says: "Any members other than "$ref" in a JSON Reference object SHALL be ignored.". |
I know, but it would be nice. Any properties specified on the object containing the ref would define or override existing properties in the referenced schema. It should be functionally equivalent to copying the referenced schema and applying a JSON-merge-patch containing the other properties to it. |
Taking the sample schema from json-schema.org directly:
And calling validate on an invalid object, will result in a return saying its valid:
Output is an empty array: []
The text was updated successfully, but these errors were encountered: