Skip to content
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

First pass at 3.1 schema object meta-schema #2016

Closed
wants to merge 2 commits into from

Conversation

handrews
Copy link
Member

This is just for the Schema Object, done in JSON Schema 2019-09
with its vocabulary concept.

This just takes a guess at appropriate $id URIs, and puts in
2019-10 as a placeholder for now.

Note that $vocabulary URIs are identifiers- there is no actual file/resource for them. They just identify semantics.

In terms of semantics, the OpenAPI 3.1 Schema Object uses all of the standard core and validation vocabulary semantics from JSON Schema 2019-09, and adds its own extension semantics (discriminator, nullable, example, externalDocs, xml, the boolean form of exclusiveMaximum and exclusiveMinimum, additional values for format, and the designation of x--prefixed fields as legal extension fields)

The meta-schemas describe syntax. They are broken up by vocabulary, and then all of the appropriate per-vocabulary meta-schemas are combined in the top-level meta-schema,

  • Most of the standard 2019-09 meta-schemas are referenced just as they are in the standard general-purpose 2019-09 meta-schema
  • There is a replacement for validation to allow the dual syntax for exclusiveMaximum and exclusiveMinimum
  • The extension schema includes all of the extensions, including the exclusive* syntax. There is nothing special for format because format values do not appear in meta-schemas
  • The top-level Schema Object meta-schema uses "unevaluatedProperties": false to ensure that only ^x- properties (as described in the extension meta-schema) can be added
  • The "$recursiveAnchor": true (and "$recursiveRef": "#" in the standard applicator meta-schema) ensure that keywords in a Schema Object that take a schema value will only accept a valid Schema Object.

Note that if we really want to we can pack all of this and the main OAS schema into a single file, it's just a lot easier to see what's going on with the Schema Object alone this way. I put these things in a "meta" directory because (unlike the OAS schema as a whole) they really do function as meta-schemas.

This is just for the Schema Object, done in JSON Schema 2019-09
with its vocabulary concept.

This just takes a guess at appropriate $id URIs, and puts in
2019-10 as a placeholder for now.
@philsturgeon
Copy link
Contributor

@handrews is this something we can get over the finishing line before the call tomorrow? It would be awesome to get the meta schema in before RC1, the last one was... a year after release? 😅

@handrews
Copy link
Member Author

handrews commented Feb 5, 2020

@philsturgeon no one has provided any feedback on it whatsoever, so... ball is not in my court right now.

@philsturgeon
Copy link
Contributor

That wasall pre.-1977. If this is up to date with the new reality, we just need to make sure paths are optional (#1045), webhook support is in there (@lornajane could you take a look), and... what else are we missing? @darrelmiller can think of any new bits we need to add?

@handrews
Copy link
Member Author

handrews commented Feb 6, 2020

@philsturgeon this isn't a full schema, it's just for the Schema Object, which is the part that I'm involved in changing. If we're good on that then we can look at how to do the rest of it.

@MikeRalphson
Copy link
Member

@handrews can you point us at any tooling which can understand such a vocabulary yet?

@handrews
Copy link
Member Author

handrews commented Feb 6, 2020

@MikeRalphson Manatee.JSON (.NET) is so far the only implementation, although some others are underway. As is the test suite, so we're kind of verifying that implementation as we build up the test cases. The implementation was written alongside the development of the spec.

@handrews
Copy link
Member Author

handrews commented Feb 6, 2020

@MikeRalphson what I'm really looking for here is feedback on the general approach- the way I've structured standard vs extension keywords, how the vocabulary concept shows up, etc. At this stage I'm not super-worried if it validates everything correctly. If the approach makes sense we can put more energy into testing it, but I don't want to do that if folks don't like the approach. It's quite different from the single-document approach for OAS 3.0.

Copy link
Contributor

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been working on a JSON Schema validator with support for Drafts 04, 06, 07, and 2019-09. Even though it's not done yet, I used these schemas as a base to implement OAS 3.1 schema validation as a proof of concept for supporting vocabularies. Found a couple of issues and came back to share what I found. (further comments are in-line)

schemas/v3.1/meta/extensions.json Outdated Show resolved Hide resolved
"$vocabulary": {
"https://json-schema.org/draft/2019-09/vocab/core": true,
"https://json-schema.org/draft/2019-09/vocab/applicator": true,
"https://json-schema.org/draft/2019-09/vocab/validation": true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to reuse the Draft 2019-09 "validation" vocabulary. I had to define a separate "validation" vocabulary for OAS 3.1 in my implementation.

Removing exclusiveMaximum and exclusiveMinimum from the vocabulary-schema doesn't remove the keyword, it removes any meta-validation for the keyword. The vocabulary still provides these keywords. So, when exclusiveMaximum and exclusiveMinimum keywords are provided for the "extensions" vocabulary, we have two vocabularies providing the same keyword.

maximum and minimum lead to conflicts too. Even though the vocabulary-schema doesn't change for these keywords, the functionality of these keywords does change. Specifically, they have a dependency on the "extensions" version of exclusiveMaximum and exclusiveMinimum keywords. Because the implementation can't be reused, maximum and minimum have to be in the "extensions" vocabulary which creates another conflict with the Draft 2019-09 "validation" vocabulary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers the entire point of separating vocabularies and meta-schemas is to specify semantics with vocabularies and exact syntax with meta-schemas.

The semantics of the OAS 3.1 usage of these keywords are compatible with the standard semantics: if the syntax used matches the syntax for the standard semantics, then the standard semantics are correct (they are identical for both standard and OAS vocabularies).

If the syntax matches the old style, then the semantics come from the OAS vocabulary alone.

This is the correct approach, and is specifically one of the cases for which I designed the syntax/semantics separation and the notion of "compatible semantics".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, this is not something I'd recommend doing, but we're shackled by Semantic Versioning into keeping this confusing compatibility. Unless we decide that we could ignore semantic versioning on this point and drop the old boolean forms? Pinging @darrelmiller in case semver conversations going on elsewhere move us in that direction. This is really not an ideal thing to have but it's definitely better than not moving to a new draft. If we could get away with dropping it I think that will be much less confusing as it will be harder to write a schema that doesn't work with standard validators.

Writing a plugin that handles this either-or thing isn't very clear- I might be pushing back on @jdesrosiers' suggestion here but he is 100% correct that this is confusing and weird, and that feedback is something the TSC should take into account. I came up with this justification of either-or handling purely for OAI exclusive* semver-mandated compatibility purposes and would otherwise drop it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the entire point of separating vocabularies and meta-schemas is to specify semantics with vocabularies and exact syntax with meta-schemas.

I understand that. What I'm arguing is that adding semantics is changing the semantics from a vocabulary implementation point of view. I have to provide a different implementation for those 4 keywords than I do for draft 2019-09.

If it were the other way around with draft 2019-09 having extra functionality and OAS constraining it, there wouldn't be a problem. I could use the same implementation and just use the meta-schema to exclude certain usages.

if the syntax used matches the syntax for the standard semantics, then the standard semantics are correct

That makes sense in words, but it's unhelpful from an implementation point of view. I still have to provide an alternate implementation even if some of the semantics overlap with the original.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did eventually come up with something I think could work to implement this as is, but it only serves to make things more complicated with no real benefit. I need to add a way for a vocabulary to explicitly define that one of it's keywords overrides that of another vocabulary. Then I need to rewrite the vocabulary loading functionality to make sure the right keyword implementations get loaded.

Even if there's a reasonable way to implement it this way, it seems harder to understand than it needs to be. By looking at the $vocabulary list, it's not clear that the "validation" vocabulary has been modified by the "extensions" vocabulary. If the OAS "validation" vocabulary has it's own identifier, it's clear that something is different.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not modifying the "validation" vocabulary is an option, that's highly preferred and makes this whole discussion moot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By looking at the $vocabulary list, it's not clear that the "validation" vocabulary has been modified by the "extensions" vocabulary.

Because $vocabulary is intentionally underspecified so that we can get feedback on what is needed to complete the feature.

What is likely is that the $vocabulary URIs will identify some sort of machine-readable file that would say what keywords are part of the vocabulary, something about their classifications (does it act as an assertion? does it provide an annotation, is it an in-place or child applicator? etc.), and something about what "semantic compatibility" means.

That is where an implementation could find out something like "if this keyword produces an error (not a validation failure, but an error) in the standard validation feature, it can alternatively be handled by this vocabulary." Otherwise, the assumption is that a keyword has to work according to all vocabularies in use.

TBH, I'm hoping that by then OAS 4 is out and we can point to this sort of confusion and say "actually we're not going to support it."

There really isn't any substitute for putting something into production and letting people find out how it works or doesn't. As you know better than most, 90% of this stuff is debated by the same 4 or 5 people in the JSON Schema repo, and at some point we have to admit that we're just guessing.

Anyway, thanks for raising all of this, it is exactly part of what "put it into production and see what happens" is supposed to raise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As things stand, full OAS 3.1 compatibility is far more important than eliminating an awkward corner of an underspecified feature that we have said numerous times we expect to tighten up before finalizing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. Full OAS 3.1 compatibility is a given whether you take my suggestion or not. What am I missing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers

I'm not sure what you mean. Full OAS 3.1 compatibility is a given whether you take my suggestion or not. What am I missing?

I meant full JSON Schema compatibility in OAS 3.1- sorry, my mistake, and somehow I missed this earlier. Anyway, see my other comment about hopefully being able to resolve this after all!

"type": "number",
"exclusiveMinimum": 0
},
"maximum": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maximum and minimum should be moved to the "extensions" vocabulary along with exclusiveMaximum and exclusiveMinimum because they are coupled.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because their syntax hasn't changed, and the meta-schema is about syntax.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This recommendation assumes that OAS defines it's own "validation" vocabulary and the modified keywords are all defined in the "extensions" vocabulary. Without that change, you are correct that it is not wrong as it is. It's just awkward.

However, I realize that this recommendation is backwards. If OAS defines it's own "validation" vocabulary, the best place for the modified keywords is in that vocabulary not the "extensions" vocabulary. It would be much more straightforward to have all the validation keywords together.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers OAS is not defining its own validation vocabulary. Meta-schemas do not define vocabularies. At all. That is why the vocabulary URI is not the meta-schema URI. Meta-schemas describe the acceptable syntax for a dialect (a collection of vocabularies- that term, "dialect", will be in the next draft). You often want to break down meta-schemas by vocabulary because that's efficient, but there is no required correlation at all. You could just write a single-file meta-schema for all of your vocabularies if you want to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OAS is not defining its own validation vocabulary.

I understand that. My suggestion is that it should define its own validation vocabulary. My suggestion is that instead of OAS 3.1 defining one vocabulary, it defines two. One vocabulary for added keywords ("extensions") and another for it's custom version of the "validation" vocabulary.

If it were done this way, the modified keywords would be part of the custom "validation" vocabulary and would therefore make the most sense in the custom "validation" meta-schema. Yes, it could be in the "extensions" meta-schema, but it would be a strange choice.

As I said, if you don't take my suggestion of making a custom "validation" vocabulary, the suggestion of moving maximum and minimum definitions can be ignored.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers whether OpenAPI decides to create its own validation vocab in the future (which I do not think it needs to do at all), the point here is moot for the simple reason that minimum and maximum work exactly the same in OpenAPI as they do in JSON Schema.

Defining them twice would be pointless and redundant.

@handrews Let's resolve this thread and crack on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philsturgeon No, the point is that they don't work the same. In OpenAPI, their behavior can change in the presence of exclusiveMaximum/exclusiveMinimum with a boolean value. Their meta-schema doesn't change, but their semantics do. I have to provide an OpenAPI-specific implementation of minimum and maximum. I can't use the standard implementation.

I still think one vocabulary adding semantics to another vocabulary is a bad idea (removing semantics via meta-schema is fine), but if you don't choose to make a separate validation vocabulary, I'd still consider moving minimum and maximum over (even tho it's functionally equivalent), because it helps communicate that they're not the standard keywords and that the extensions vocabulary is overriding something about their standard behavior.

But, I agree that it's time to make a decision and move on. I've shared my experience implementing this vocabulary and my opinions. I'll work with whatever is decided, whether I agree with the decision or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think one vocabulary adding semantics to another vocabulary is a bad idea (removing semantics via meta-schema is fine),

This is a little confusing because the discussion keeps jumping between different threads on a PR. This PR only aims to document the current situation, not change the current situation.

Please make a new issue to discuss the concerns and mark your concerns as resolved here so we can clear up the PR and move forward. That discussion can then be had at greater length with people who actually represent OpenAPI (which is neither myself or Henry).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers as you can see in #2219, we are now considering either changing from 3.1 to 4.0, or dropping SemVer, which would allow us to get rid of this exclusive* mess (and allow the next JSON Schema draft to tighten up the language around combining vocabularies). While this problem is not the only reason the TSC is considering this change, it helped make the case stronger, so thanks for your persistence!

There's still a chance we'll stick with this so I'm not updating this PR yet, but hopefully will be able to do so after the next TSC meeting.

@@ -0,0 +1,118 @@
{
"$id": "https://spec.openapis.org/oas/3.1/meta/extensions/2019-10",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@handrews @MikeRalphson did we settle on an $id in slack? I need one to take a swing at #2017.

@philsturgeon
Copy link
Contributor

Right, with v3.1.0 out this is going to start becoming a popular topic.

@darrelmiller do we have anyone "in charge" of this?

@handrews
Copy link
Member Author

@philsturgeon @darrelmiller I'd recommend closing this and opening a new PR. The conversations above were all resolved and are now more confusing than helpful, and some complexity around maintaining compatibility is no longer necessary. A new PR will be cleaner and easier to follow. I'm not sure how much time I'll have but I can probably at least get this started in a new PR.

@MikeRalphson
Copy link
Member

Thanks @handrews good thoughts (as per).

@jdesrosiers
Copy link
Contributor

I was just going to volunteer to help get this over the finish line. @handrews, if you find you don't have the time, let me know and I can pick it up.

@philsturgeon
Copy link
Contributor

@jdesrosiers please do mate! I’ve got a donation for your preferred charity ready to go if you can knock this out. Not just for the scheme object but the whole meta scheme like the 3.0.json we already have.

@jdesrosiers
Copy link
Contributor

@philsturgeon Ha! I was just volunteering to do the vocabulary, but I suppose I can take a stab at the rest as well. My expertise is in JSON Schema, not OpenAPI, so I'll need some careful review to ensure I'm describing OpenAPI correctly. I figure if I start with the 3.0 schemas, then convert to Draft 2020-12, then make changes based on the change log, that should allow me to get pretty close without having to get into the weeds of every detail of the spec.

@handrews
Copy link
Member Author

handrews commented Feb 17, 2021

@jdesrosiers this PR was only for the schema object- I don't think I ever did a complete OAS 3.1 schema. Please go right ahead with the schema object, I'm sure you can sort it out but if you run into problems or questions about what I had in this PR, please let me know!

@handrews
Copy link
Member Author

For whenever anyone wants to tackle the full OAS 3.1 schema (which I recommend doing separately), here is the gist where I tried to do a 3.0 schema using unevaluatedProperties. I don't recall how accurate it was, but it at least shows the sort of compression and refactoring that is now possible.

Other things to do in that schema would include replacing 1-element enums with const, replacing definitions with $defs, and seeing if things can be further streamlined using $dynamicRef/$dynamicAnchor which we did not have when I wrote that gist.

@philsturgeon
Copy link
Contributor

Good shout @handrews, and it was worth a try @jdesrosiers! :D

I'll take a swing at the full oas 3.1 schema over here because I need it yesterday. #2474

@philsturgeon philsturgeon mentioned this pull request Feb 18, 2021
@handrews handrews deleted the oas3.1-schema branch April 18, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants