-
-
Notifications
You must be signed in to change notification settings - Fork 266
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
Support declaring that all defined properties are required #659
Comments
Supporting |
There are concerns over this, including:
On the plus side:
I'm at least tentatively open to this idea. Let's see how others chime in. |
I've been using JSON schemas for defining JSON-based communication protocols for a couple of years, and one of the more common and recurrent problem that I face is with mandatory properties. Most of the time we have JSON objects with dozens if not hundreds of properties, all of then mandatory, and maintaining the In short: If I could vote, adding a simple way to indicate that all the properties under Extending the Using annotations for the interaction between As a consequence, either the annotation info will have to be extended to additionally include some kind of optional "type" to differentiate between "matched" and "defined" annotations for I personally think that either option of using annotations would end up with a entangled natural language specification in every place were the annotation for Finally, another option could be defining a new keyword |
BTW, if just modifying slightly the
|
I've actually seen more than one instance where someone has put Cany anyone see any issues with allowing I haven't read @jgonzalezdr suggested patch in detail, but this IS more than an n = 1 problem, which seems to have a reasonably simple solution. |
As commented in the PR: Maybe the solution could be creating a new With this more meaningful keyword name the expected / implicit semantics would be a bit more evident (also explicitly indicating that the keyword applies exclusively to properties): The |
I mentioned elsewhere
@handrews Do you remember what you commented on that idea, if any? |
@Relequestual: I've also seen schema writers misinterpret it that way. If the keyword was |
@awwright I probably objected to the fact that However, we've worked with keyword classification a lot more since then, and the idea is worth revisiting. @jgonzalezdr I'm not sure I understand this concern:
In terms of validation, there is no practical difference between using the matched vs defined properties. For code or UI generation, there is no concept of matched properties (matched against what? the whole point of these use cases is that there is not yet an instance). We will need to address that issue for what I call "generative" use cases anyway, and I don't think it would be any harder here than it already is. Additionally, much of the work on generative vocabularies will be adding keywords that disambiguate problematic validation keywords that really only function usefully in the context of an instance. This may be another case where generative use cases simply use an additional keyword that has no effect in instance-based use cases. |
Ugh, I meant that Addressing @jgonzalezdr 's point:
I'm not entirely sure where I'd put it. My first impulse was core, but then reading this I can see the argument for validation. This is a bit of a warning "design smell", but I'm not sure that the problem is with the keyword itself. I'll think on this more. |
I don't have any problem with a little bit of sugar for authoring convenience. We typically try to factor apart keywords as much as possible (e.g. there's a reason you have to specify If we wanted to take this to its logical conclusion, we could specify a keyword like Unlike my |
@awwright @jgonzalezdr my thought right now is to collect more community feedback here on the two approaches: a "magic" boolean value for So I'd probably prefer not to put it into draft-08 unless we quickly get a lot of feedback. I want to focus on finishing Draft-09 will be much smaller, as draft-08 really dug into the hardest problems. I expect it will be a bug fix / feedback incorporation draft, plus features like this that just need a little more discussion. I know we slipped in @awwright I strongly agree with your code smell comment for setting the bar for this sort of keyword- the problem of keeping |
BTW I am open to putting a solution into draft-08, I'm just unsure of which and would like to be cautious in such a core area. |
We have come across the a similar use case as @jgonzalezdr a couple of times recently in the HCA metadata: During schema edits, These schemas were not flagged as invalid but as all our schemas use We would appreciate any advice on how to handle this short of hand-rolling hacks into our validation process. |
Another option for solving this issue, maybe a bit more less disruptive, would be extending the current behaviour such that This approach would be a lot more flexible, being almost backwards compatible (except rare property names clashing with regex special characters). However, some dependency on the contents of a sibling Some examples:
|
For me, this is something that should be solved at the tools level. JSONBuddy as a schema editor gives you a warning if a required property is not defined at the related schema level. This way you will see immediately the issue while you are editing the schema and this helps saving time. Another advantage, you can catch this error even if you don't have all possible combinations of sample data available (which will rarely be the case) and validated. Maybe your test data doesn't cover all of the schema definitions? From a practical perspective, you need a tool to solve this. Anything else can only be handled at the time of validating the JSON data which is already at production level in the worst case. |
A mistyped property can be detected by a authoring helper tool, but forgetting to mark a property as required when you already have several dozen properties cannot be detected by a tool. My personal experience when defining protocols or storage formats not only with JSON, but also with other languages like XML or ASN.1, is that 95% of the time properties (or their equivalents in other languages) are required. I think that not by chance, most schema languages by default consider that defined elements are mandatory, unless explicitly defined as optional (e.g. DTD, XSD, ASN.1). However, JSON Schema design is to make properties optional. This is fine, it's a design decision that has most probably a good reason behind. However, given that many schema authors will need many times all of the properties to be required, I think it is very interesting to let that design decision being reflected in a formal and machine verifiable way in the schema. I also think that we should take in account that many times schemas are also read by humans, so keeping verbosity reduced is also good: If I have to review a schema and understand which of the 83 properties are required and which not... IMO, it is much more dangerous going to production with something that should be required marked as optional, than the other way around. As I said before, many times, because of the lack of time, only positive cases ("happy path") are tested. When that happens, properties that should be optional are detected, but properties that shouldn't be optional are not detected. |
For those following this thread through email, edited: "IMO, it is much more dangerous going to production with something that should be optional marked as required, than the other way around" => "IMO, it is much more dangerous going to production with something that should be required marked as optional, than the other way around". |
@jgonzalezdr I think there's multiple use-cases for JSON Schema and that's one of them. Especially in comparison to a database, where you have to specifically label a column as NULLable. But there's also many cases where the default is nothing is required, all unknown . Especially protocols, including the biggest application protocols like MIME and HTTP. I just want to be careful to not prefer one style over the other. Also note, in previous versions of JSON Schema, you could do this: {
patternProperties: {
"^js-": { required: true }
}
} |
@awwright what did |
... good question |
This issue is re-summarized and reopened at #846 |
I would like a way to make all properties defined in a schema to be required without having to explicitly list them.
The text was updated successfully, but these errors were encountered: