-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Details of model composition should be documented #1428
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
Comments
@mkistler based on the JSON Schema spec: "Thing": {
"allOf": [
{
"properties": {
"foo": {
"type": "string"
}
}
},
{
"properties": {
"foo": {
"type": "string"
}
},
"required": [
"foo"
]
}
]
} is equivalent to "Thing": {
"required": ["foo"],
"properties": {
"foo": {
"type": "string"
}
}
} because the "foo" subschemas are identical and "required" only needs to appear once to apply across the board. Whether or not you actually want to perform such transforms depends on what you're trying to do. Code generation? In the latest draft of JSON Schema there is some guidance on how to find and combine all of the schema objects that apply to a given location in an instance, which makes it a bit more clear that you can do this sort of thing. |
@handrews I like your interpretation of composition. My concern is that I do not believe this is clear from the specification. The specification makes some qualifications about
So it's not clear to me if composition conforms to the JSON schema definition or some OpenAPI-specific interpretation. And I don't think I am the only one that is unclear on this, since some current OpenAPI tools, e.g. swagger-editor, do not interpret the So I'm suggesting a small addition the spec something along the lines of:
Or maybe more simply, just state that the composition should be interpreted consistent with the JSON schema interpretation of allOf (though that leaves it up to the reader to understand the nuances of this). |
As one of the editors of the JSON Schema spec, I would prefer this :-) More broadly, I am hoping that we can converge the JSON Schema spec and Open API uses of JSON Schema spec to the point where little or even no special wording on the part of OAI is necessary, primarily through improving the modularity of JSON Schema. Embedding a subset of JSON Schema into a larger document is done in several projects (OAI is the largest that I know of) so we should address that on the JSON Schema spec side. |
@mkistler The allOf definition says that the inline schema must be an OpenAPI Schema Object so as to allow OpenAPI extensions and apply OpenAPI constraints to JSON Schema. However, the definition of Schema Object says:
So, based on the lack of any explicit differentiation between the behavior of JSON Schema and Schema Objects with respect to schema composition, my interpretation of the spec means that JSON Schema rules should be respected. |
I like this answer. I think we are all agreed on the desired interpretation and it's now clearly documented in this issue. |
This actually flows directly from JSON Schema validation as a constraint system. Constraints are always added, never removed. It's probably better to think of this as layering constraints rather than object composition. Composition adds fields, constraints restrict fields. |
Okay ... this opens a new can of worms. @handrews what should be the "composed model" in this scenario:
? I know that the Swagger tools interpret this as a model with three properties, "foo", "bar", and "baz", but this interpretation is not "restricting" in the way you suggest. |
@mkistler As @handrews said, it is much easier to rationalize what JSON Schema will do if you consider it as constraints, not a model description. The combination of the two schemas says that the object must have a property called "foo" that must be a string and if it has properties called "bar" or "baz" then they must be strings. |
@mkistler I understand how it doesn't look that way, but that is actually what is happening. The "problem" here (and it is true in several other uses of JSON Schema) is that in most cases, you can treat JSON Schema as if it were a data definition system instead of a constraint system and it more or less works. I am hoping to pick up on some work that another person started to write vocabulary that handles things specific to data definition better, and clarify how data definition and validation fit with each other. In the meantime, here's how to think about your example. I'm going to link to the latest spec for some definitions- we only just wrote those definitions now, but they hold equally true for the drafts that OpenAPI uses.
So the 1st The 2nd Note that there is nothing that says there can't be other properties. There's nothing that even requires the instance be an object- an integer would successfully validate as a So, this will validate successfully against the 1st
and this will validate against the 2nd but not the 1st:
Neither of these examples will validate against
will. It validates successfully against both branches of the So that is how the constraints work, and how combining schemas with |
Thank you! This does clarify things. In fact, as soon as I read:
the light bulb went on. I was concerned that only the properties defined in the schema could be present -- that But now I see how this works. And likewise, Thanks for setting me straight. |
@mkistler exactly! Another thing we clarified in recent drafts is allowing boolean values for any schema (not just subschemas for When you look at it that way, |
Hopefully adding those definition sections that we added in draft-07 will make this processing model a lot more clear. |
@handrews collapsing allOf schemas like you have done only works if the allOf schemas overlap and one is more restrictive than the other. Should these 3 cases be collapsed? Example 1:
Example 2:
What type is ComposedWithTypeCollision.a? Example 3
Where are the validations for a? When would collapsing down allof properties not work?
For these reasons, I think that the collapsing that you show only works under certain conditions. Do you think these 3 example should be collapsed? |
@spacether it's too late at night here for me to attempt schema algebra but let me sketch out a few things:
As you can see if you poke around in that repo, I did the collapsing because we were using schemas for documentation and having a ton of If you're making use of annotations, which your application might interpret based on their location in the schema (e.g. a Philosophically, I'd like to encourage folks to design annotation vocabularies (now that extension vocabularies are supported in OAS 3.1 and JSON Schema 2020-12) to give tools hints as to how to handle things like **adjacent keywords are keywords in the same schema object |
Heads up, example3 had a typo and was updated to not use oneOf. |
OASv3.0 says:
However, the spec does not define how this composition should be done. In particular, it is unclear how to interpret an "allOf" composition when a property appears in more than one component of the "allOf". For example, in this schema:
it is not clear from the specification whether
foo
is required or optional in theThing
schema.Multiple interpretations are possible, including "first occurrence of the property wins" or "last occurrence of the property wins". However, to be consistent with JSON Schema, it seems that we need a more complicated interpretation.
The JSON Schema spec says:
To be consistent with JSON schema, it seems that we should "compose" like-named properties in a "allOf" in a way that produces the "most restrictive" property definition. For example, a property that is required in any component of the "allOf" would be required in the composed model. Or if one property specifies a maximum, the composed property would have that maximum.
Whether it is the "composed property" interpretation or some other that is appropriate, it should be clearly documented in the spec.
The text was updated successfully, but these errors were encountered: