-
-
Notifications
You must be signed in to change notification settings - Fork 308
Keyword for extending a schema #907
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
@awwright To start off with It's also not clear to me why this is better than my proposal regarding |
@awwright thanks for fixing the syntax, this is more clear to me. What I don't understand here is how this solves the problem that {
"$schema": "https://json-schema.org/draft/2019-09/hyper-schema",
"$id": "https://json-schema.org/draft/2019-09/hyper-schema",
"$recursiveAnchor": true,
"allOf": [
{"$ref": "https://json-schema.org/draft/2019-09/schema"},
{"$ref": "https://json-schema.org/draft/2019-09/meta/hyper-schema"}
]
} In your proposal, I think it would need to be written as: {
"$schema": "https://json-schema.org/draft/2019-09/hyper-schema",
"$id": "https://json-schema.org/draft/2019-09/hyper-schema",
"$extends": "https://json-schema.org/draft/2019-09/schema",
"$ref": "https://json-schema.org/draft/2019-09/meta/hyper-schema",
"properties": {
"additionalItems": { "$ref": "#", "$extends": "schema#" },
"additionalProperties": { "$ref": "#", "$extends": "schema#"},
"dependencies": {
"additionalProperties": {
"anyOf": [
{ "$ref": "#", "$extends": "schema#" },
{ "type": "array" }
]
}
},
"items": {
"anyOf": [
{ "$ref": "#", "$extends": "schema#" },
{ "$ref": "#/definitions/schemaArray" }
]
},
"definitions": {
"additionalProperties": { "$ref": "#", "$extends": "schema#" }
},
"patternProperties": {
"additionalProperties": { "$ref": "#", "$extends": "schema#" }
},
"properties": {
"additionalProperties": { "$ref": "#", "$extends": "schema#" }
},
"if": { "$ref": "#", "$extends": "schema#" },
"then": { "$ref": "#", "$extends": "schema#" },
"else": { "$ref": "#", "$extends": "schema#" },
"allOf": { "$ref": "#/$defs/schemaArray" },
"anyOf": { "$ref": "#/$defs/schemaArray" },
"oneOf": { "$ref": "#/$defs/schemaArray" },
"not": { "$ref": "#", "$extends": "schema#" },
"contains": { "$ref": "#", "$extends": "schema#" },
"propertyNames": { "$ref": "#", "$extends": "schema#" }
},
"$defs": {
"schemaArray": {
"type": "array",
"items": { "$ref": "#", "$extends": "schema#" }
}
}
} The whole point of |
@handrews And there's no need to re-declare any of the applicator keywords. If the base schema refers back to the target of $extends, it goes back to the extending schema instead; and it passes this fact along when validating sub-schemas. Re-declaring the applicator keywords in the extended schema is likely only necessary if multiple schemas need to be in the "extends map". So, we should have merely this: {
"$schema": "https://json-schema.org/draft/2019-09/hyper-schema",
"$id": "https://json-schema.org/draft/2019-09/hyper-schema",
"$extends": "https://json-schema.org/draft/2019-09/schema",
"$ref": "https://json-schema.org/draft/2019-09/meta/hyper-schema"
} And here's what goes on, let's validate a simple schema:
|
@awwright thanks- while I still don't quite get it, I'm working through it and will comment more later. Just to check, regardless of the syntax I think the primary difference here is that you are passing some sort of additional context down into the base schema, so that certain references resolve into that context (here I'm fairly certain you can implement that "higher" marker as a passed-down context in some way, and IIRC @gregsdennis might actually do something like that in his implementation? Whether he does or not, I'd love for his input on which of the way things are now, vs my proposal in #909, vs this proposal, seems simplest to him. |
I'm not sure I follow this proposal either. It may be due to my unfamiliarity with hyper-schema, though, but I'm still failing to see how this solves a problem that |
As to handling of these keywords in Manatee.Json, I pass a context object through the validation process that keeps track of the current recursive anchor so that the recursive ref can resolve to it. |
@awwright I think I'm starting to understand this. I want to do an example in a bit more detail, and I'll refer to the following (meta-)schemas, presented below in abridged form. I'll go through the actual example in the next comment, this is just the set of files being used. The only notable difference here is that the dialect schemas extend multiple other schemas, so I changed Hyper-Schema dialect{
"$schema": "https://json-schema.org/draft/2019-09/hyper-schema",
"$id": "https://json-schema.org/draft/2019-09/hyper-schema",
"$extends": [
"https://json-schema.org/draft/2019-09/schema",
"https://json-schema.org/draft/2019-09/meta/hyper-schema"
]
} Standard dialect{
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://json-schema.org/draft/2019-09/schema",
"type": ["boolean", "object"],
"$extends": [
"https://json-schema.org/draft/2019-09/meta/core",
"https://json-schema.org/draft/2019-09/meta/applicator",
"https://json-schema.org/draft/2019-09/meta/validation",
"https://json-schema.org/draft/2019-09/meta/content",
"https://json-schema.org/draft/2019-09/meta/format",
"https://json-schema.org/draft/2019-09/meta/meta-data"
]
} Applicator vocabulary{
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://json-schema.org/draft/2019-09/meta/applicator",
"properties": {
"not": {"$ref": "#"}
}
} Hyper-Schema vocabulary{
"$schema": "https://json-schema.org/draft/2019-09/hyper-schema",
"$id": "https://json-schema.org/draft/2019-09/meta/hyper-schema",
"properties": {
"links": {
"type": "array",
"items": {"$ref": "https://json-schema.org/draft/2019-09/links"
}
}
} Link Description Object{
"$schema": "https://json-schema.org/draft/2019-09/hyper-schema",
"$id": "https://json-schema.org/draft/2019-09/links",
"type": "object",
"properties": {
"targetSchema": {"$ref": "https://json-schema.org/draft/2019-09/meta/hyper-schema"}
}
} |
@handrews Thanks for trying to grok this, nice catch with the array form |
@awwright I got distracted but will come back with my take from a walk-through with the above schemas tomorrow. |
If I understand this correctly, this looks like it maps to the current The important difference is that all I like that this cleans up the syntax, but if I understand this correctly, I don't think it's flexible enough. |
OK I managed to go through a couple of different paths here. TL;DR:
Does it do what it says?The most interesting path is through the If we start from the hyper-schema dialect meta-schema, by the first time we get to the links schema, the map looks something like this (with relative URIs for readability):
So when we get to
This does produce the desired behavior. The double-lookup could be optimized by modifying the map rather than just adding to it, although that could cause its own complications. Starting from the links schema, on the first pass there is no map, so the reference to This is also the correct behavior. So I agree that this proposal does what it says it does. Is that the right thing to do?I do think that the issue raised by @jdesrosiers is a concern. A key piece of the Of these three points, the "saying you are extensible" part is arguably the least important. The idea was that not all schema designers may want to allow extension, so be default it was disallowed. I have no idea if that's a real concern or not. The "say it is an extension" part is obviously important. This proposal requires you to specify who you are extending, and subsumes the The "reference says it respects extension" is, I think, critically important. We discussed this with the original However, this could easily be "fixed" by pairing So: This is very close to what we've previously said we want, and I think by adding Is this the right terminology?Names are hard.
I'm pretty sure if we have an I'd really rather avoid OO terminology. The fact that What about the proposed
|
Then you didn't understand the point I was making because you proceeded to make almost exactly the same argument just with a lot more words. |
@jdesrosiers I'm confused. I said "I DO think that the issue raised by @jdesrosiers IS a concern. And then I used a lot more words. As I usually do. But to elaborate on my thinking. I don't know where I stand on this so laying out my thought process was the best I could do right now. |
Good points all around and I will address them on more sleep. ... Well, fine... two quick points: (1) This doesn't have to be an OO system, however it seems reasonable to me for JSON Schema to support unmarshalling onto an OO type system. (2) I was designing this so that being "extended" is obligatory — though if sometimes you need to recurse back to the literal $ref target and not the extending schema, it's possible to work around that with $defs or something. If the goal is a cooperative system, that's fine too. But in my understanding, that actually makes this a bit more complicated, not less. I think the next step is trying a few different proof-of-concepts. This is a very complicated proposal. Hopefully I can look at the other ideas, and rework this some, until we've got something that's intuitive. |
@handrews Ha! Sorry, I read that wrong. |
We've literally been talking about supporting this for the past year or two, and it's the primary driving use case for This is a thing we want, but there's been a ton of conceptual work done, which has been validated to a significant degree with JSON Schema's largest "customer" community. If we want to talk about OO type system support, that is a far larger conversation and needs to build on the work that has already been done across both the JSON Schema and OpenAPI communities.
In terms of implementation, it's essentially the same. If you're doing it as passing information down during evaluation, then:
In terms of usage, it depends on whether you think explicit or implicit is more clear. I still strongly dispute that schemas are inherently polymorphic- that's not the model that we've used to date, it's contrary to the model we've promoted over the last two years, and it's an enormous philosophical change. In that sense, I think the ease-of-use of obligate extension is problematic. And we can solve OO type system problems without it by decoupling structure and semantics. The cooperative model is less intuitive to the user, but more explicit. More importantly, it satisfies the principle of least power in that it solves exactly the mechanical problem of recursive references without saying anything about the semantic rationales for which such references are used.
Keep in mind that @gregsdennis and (I think) @jdesrosiers have already implemented the existing |
Yes, I've implemented everything from draft-04 to draft 2019-09 (plus OAS 3.1) with the exception of |
@jdesrosiers that's what appeals to me about #909. As you noted over there, aligning Talking about base URIs seems to always confuse a lot of people, and the use of a boolean in the target schemas doesn't have any clear analogue anywhere that I know of. I think I was just trying to make it all as limited as possible. |
@awwright given that the changes in #909 have been fairly well-received, and that it represents a much smaller change to the approach we already have, and (most importantly) given that we expect a lot of work on code generation that will fit very well with OO extension concepts, would you be OK if we go with #909 for the next draft, and keep this open for consideration when we get to code gen? I don't mean that this proposal should be restricted to code gen, I just think it's best considered alongside of it because of the OO implications. My biggest concern is whether you think that #909, as I explain it three comments up from this one, is reasonably easy to implement. If so, I'd like to proceed with that, and defer the question of whether we want obligate extension polymorphism until the future. Cooperative recursion is in some ways more complicated, but it is intended as a very much advanced under-the-hood feature. I'd like to find a way to make it a little easier without moving to an OO paradigm or rushing work that we should be doing in coordination with OpenAPI post-3.1. |
@handrews Sure, I guess. I was hoping to get a proof-of-concept out the door sooner than I am. Then I could experiment more, "oh this works... this doesn't... I understand what's going on here" |
@awwright I'm not sure what you mean about a proof of concept for the other issue, two people have implemented the current version and said that that proposal would work fine in their implementation. What more proof are you asking for for #909? I'm trying to make sure I'm not steamrolling over a real problem here, and this draft has a constraint of matching the next OAS release, which is hitting RC1 as early as today. So we can't just mess around with this on our own schedule, and I've devoted most of my JSON Schema time to this discussion this week, which means I've made zero progress on the long and growing set of issues that need to be finished for the draft. We need to be wrapping up this draft, not opening it up, right now. |
Yeah, I would strongly appose adding any more I'm going to leave this issue open but highlight it's low priority and not part of I really want to put this issue on ice till draft 2020-whatever is out the door. |
I've been trying to implement $recursiveRoot and $recursiveRef myself, and it's pretty gnarly. I think we were trying to keep it super simple for implement, but it ended up having so many rules it's actually difficult to comprehend what's going on.
I have an idea for a different mechanism for "extending" recursive schemas, let's call it
"$extends"
.When a validator encounters the "$extends" keyword, it follows the reference to the schema with the given URI, and applies that schema as an applicator keyword, the same way "$ref" works—but with one change in behavior:
Any time a sub-instance applies the extended schema (because of a $ref or $extends recursive reference), the extending schema is also applied.
The mechanism to perform this is, I think, straightforward: the validation function just needs to accept an optional argument, a mapping of
schema → set of schemas
, by default whatever the parent value was, or an empty map (at the document root). The behavior: any validations against a schema in the map, will also validate that instance against all schemas in the mapped set.This should even allow multiple recursive levels, for example, where one property recurses back to the parent, and a different property recurses up to the grandparent. As long as you use "$extends" for both cases, you should be able to write an extending schema that always gets applied when the base is; and so (afaict) unevaluatedProperties should still work properly.
An example for adventurous parties to wrap their heads around:
An instance, validated against
hyper
:The text was updated successfully, but these errors were encountered: