Allow contains to apply to objects as well as arrays#1092
Conversation
| or equal to the "maxContains" value. The second way is if the annotation | ||
| result is a boolean "true" and the instance array length is less than or | ||
| equal to the "maxContains" value. | ||
| result is a boolean "true" and the instance length is less than or equal to |
There was a problem hiding this comment.
Can "length" be applied to objects? Maybe we keep "instance array length" and add "or property count"?
There was a problem hiding this comment.
I think "length" makes sense. It may be an object, but in this context, it's being treated like a map, which does have a concept of length. But I you think it's questionable, others will be confused, so we should be more explicit.
There was a problem hiding this comment.
"number of properties" is explicit and doesn't depend on a language/framework's particular data model.
| than or equal to the "minContains" value. The second way is if the | ||
| annotation result is a boolean "true" and the instance array length is | ||
| greater than or equal to the "minContains" value. | ||
| annotation result is a boolean "true" and the instance length is greater |
|
I think this keyword should be moved out of this section entirely -- as it is presently located in "Keywords for Applying Subschemas to Arrays". It can be moved to a third section after "Keywords for Applying Subschemas to Objects", called "Keywords for Applying Subschemas to Arrays or Objects", or even just "Other Keywords for Applying Subschemas". |
|
What happens if someone wants to specify that an array to contain an item, OR that an object to contain a property value? I would like to suggest these should be different keywords, e.g. |
If they only wanted one or the other, they would need to also specify |
|
Good thinking about adding to the change log! It sounds like this PR might get closed as preference seems to be shifting toward using new keywords. When it's decided, I will make sure whichever one we go with is included in the changelog. |
|
While it sounds like this PR is a bit in-limbo, if that changes and it's re-approved, it would be good to wait to merge until we decide whether to branch for the next major draft. |
|
It appears we have general consensus on this direction, so I'm removing "draft" status to open this up for review. However, @handrews, I will not merge until we decide what's happening with the bug-fix draft. This should not be included in the bug-fix draft. |
|
I had the create a new section for the change log because it doesn't exist yet. I used |
handrews
left a comment
There was a problem hiding this comment.
Excellent work on reaching this solution!
|
I just want to include a quick summary of the discussion around adding this keyword. There was no opposition to adding {
"not": {
"patternProperties": {
"": { "not": { ... schema ... } }
}
}
}Note: I used The only point of contention was whether In the end, the only people expressing strong preferences were for overloading |
|
I noticed there were no changes to meta/applicator.json. Shall we do those in a subsequent PR? |
|
I don't think there are any meta-schema changes needed for this. Am I missing something? |
Resolves #1077
The two ways to go with this were to have
contains/minContains/maxContainsapply to objects, or to introduce three new keywords that apply to objects. I went with the first option because that was the only one anyone expressed a preference for and no one objected.