-
-
Notifications
You must be signed in to change notification settings - Fork 887
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
Do you support "Ban unknown properties mode (v5 proposal)" #238
Comments
It does not. I don't plan to support it for several reasons. |
You can achieve all it does by correctly structuring your schemas. This feature is at least inefficient in terms of performance, also there are cases when it is not clear what it should mean (inside keywords oneOf and not, which were part of the purpose to add it). |
Hm, how could I achieve the following behaviour without "Ban unknown properties": {
"definitions": {
"first": {
"type": "object",
"properties": {
"prop1": {"type": "string"}
}
},
"second": {
"type": "object",
"properties": {
"prop2": {"type": "string"}
}
}
},
"allOf": [
{"$ref": "#/definitions/first"},
{"$ref": "#/definitions/second"}
]
} Here I want to mix two "definitions", and I don't want my json-object to have "additionalProperties". In the example, only "prop1" and "prop2" are allowed. |
Actually I like |
Thank you There are two ways to address this requirement: One is by manually merging allowed properties in one subschema (you don't have to repeat their definitions) and use additionalProperties:
It's super easy to define custom keyword Another is by using the proposed "$merge/$patch" keywords. See #231 - I plan to implement it at some point. I prefer these approaches as it keeps validation logic in the schema without delegating it to the validator option and essentially changing JSON schema semantics in a big way. There are some options that affect validation, but these options are either mentioned in the standard or are very focussed on particular keywords. banUnknownProperties creates the whole parallel universe where JSON schema is a different standard... I've seen the tests you are referring to - there are only tests for very simple cases, they don't even cover patternProperties. With $refs (particularly [mutually] recursive ones) it becomes much messier to support/implement. |
The only reason I see to implement this feature is to help people migrate from tv4... I will try to investigate how many tv4 users who would consider migrating are using this feature. In any case, an alternative to supporting different validation mode would be to provide a migration utility/library that transforms the schemas to equivalent schemas but not relying on this option. |
Thanks |
How do I implement such a logic using $merge or $patch? For example I have more then two definitions of objects. lets say four with a few properties. And I want to 'merge' all the four together to get one. The other details as in the above example. The main restrictions: I don't want to have additional properties and I don't want to duplicate fields (even their names). I tried $merge. It works for two objects but I did not find a easy way to merge four object at once. Maybe $patch can help but I did not managed to get it working in the use case at all. Maybe I'm just missing something. |
@zag2art $merge can't merge multiple schemas indeed. It's possible to improve it to support this: https://runkit.com/esp/588249924ddfcf001488f5e9 But a better option could be to implement an array syntax when $merge value is an array of schemas/objects to merge. |
Wouldn't it be simplest for somebody to just implement a module that returns a copy of a given schema with |
@stuartpb the problem is that additionalProperties is shallow, it doesn't take into account properties that exist on the same level in data but are defined in different schema objects. Please see FAQ on multiple related issues and also this #134 (comment) about possible workarounds. At the moment there is no consensus in JSON-Schema-org about what should be the standard solution to these issues. |
If I understand what you're saying, then I don't see how that isn't solved by recursively setting |
I mean, I get the cases where it breaks |
Because some properties can be defined on top level of schema and some inside anyOf subschemas, but they need to be combined. Just adding additionalProperties would make schema more restrictive than you would like to - it wouldn't allow properties defined on top level. |
Well, I don't see how another layer that copies the top-level properties into the subschemas of |
Could not find. Does ajv support Ban unknown properties mode?
The text was updated successfully, but these errors were encountered: