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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions schemas/v3.1/meta/extensions.json
Original file line number Diff line number Diff line change
@@ -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.

"$schema": "https://json-schema.org/draft/2019-09/schema",
"$vocabulary": {
"https://spec.openapis.org/oas/3.1/vocab/extensions/2019-10": true
},
"$recursiveAnchor": true,

"type": ["object", "boolean"],
"properties": {
"example": true,
"exclusiveMinimum": {
"type": ["number", "boolean"]
},
"exclusiveMaximum": {
"type": ["number", "boolean"]
},
"nullable": {
"type": "boolean",
"default": false
},
"discriminator": {
"$ref": "#/$defs/Discriminator"
},
"externalDocs": {
"$ref": "#/$defs/ExternalDocs"
},
"xml": {
"$ref": "#/$defs/Xml"
}
},
"patternProperties": {
"^x-": true
},
"allOf": [
{
"if": {
"required": ["exclusiveMinimum"],
"properties": {
"exclusiveMinimum": {"type": "boolean"}
}
},
"then": {
"required": ["minimum"]
}
},
{
"if": {
"required": ["exclusiveMaximum"],
"properties": {
"exclusiveMaximum": {"type": "boolean"}
}
},
"then": {
"required": ["maximum"]
}
}
],
"$defs": {
"Discriminator": {
"type": "object",
"required": ["propertyName"],
"properties": {
"propertyName": {
"type": "string"
},
"mapping": {
"additionalProperties": {
"type": "string"
}
}
},
"additionalProperties": false
},
"ExternalDocs": {
"type": "object",
"required": ["url"],
"properties": {
"url": {
"type": "string",
"format": "uri-reference"
},
"description": {
"type": "string"
}
},
"patternProperties": {
"^x-": true
},
"additionalProperties": false
},
"Xml": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"namespace": {
"type": "string",
"format": "uri"
},
"prefix": {
"type": "string"
},
"attribute": {
"type": "boolean"
},
"wrapped": {
"type": "boolean"
}
},
"patternProperties": {
"^x-": true
},
"additionalProperties": false
}
}
}
28 changes: 28 additions & 0 deletions schemas/v3.1/meta/schema-object.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"$id": "https://spec.openapis.org/oas/3.1/meta/schema-object/2019-10",
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$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!

"https://json-schema.org/draft/2019-09/vocab/meta-data": true,
"https://json-schema.org/draft/2019-09/vocab/format": false,
"https://json-schema.org/draft/2019-09/vocab/content": true,
"https://spec.openapis.org/oas/3.1/vocab/extensions/2019-10": true
},
"$recursiveAnchor": true,

"title": "Core and Validation specifications meta-schema",
"$comment": "Note that while the standard validation vocabulary is used, an alternate meta-schema for that vocabulary is used in order to faciliate compatibility with past versions of the OpenAPI Specification.",
"allOf": [
{"$ref": "https://json-schema.org/draft/2019-09/core"},
{"$ref": "https://json-schema.org/draft/2019-09/applicator"},
{"$ref": "https://json-schema.org/draft/2019-09/meta-data"},
{"$ref": "https://json-schema.org/draft/2019-09/format"},
{"$ref": "https://json-schema.org/draft/2019-09/content"},
{"$ref": "https://spec.openapis.org/oas/3.1/meta/validation/2019-10"},
{"$ref": "https://spec.openapis.org/oas/3.1/meta/extensions/2019-10"}
],
"type": ["object", "boolean"],
"unevaluatedProperties": false
}
94 changes: 94 additions & 0 deletions schemas/v3.1/meta/validation.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
{
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://spec.openapis.org/oas/3.1/meta/validation/2019-10",
"$vocabulary": {
"https://json-schema.org/draft/2019-09/vocab/validation": true
},
"$recursiveAnchor": true,

"$comment": "This is an alternate meta-schema for the standard validation vocabulary. It is identical to the standard validation meta-schema except that it omits exclusiveMinimum and exclusiveMaximum, which are described by the OpenAPI extensions meta-schema.",

"title": "Validation vocabulary meta-schema",
"type": ["object", "boolean"],
"properties": {
"multipleOf": {
"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.

"type": "number"
},
"minimum": {
"type": "number"
},
"maxLength": { "$ref": "#/$defs/nonNegativeInteger" },
"minLength": { "$ref": "#/$defs/nonNegativeIntegerDefault0" },
"pattern": {
"type": "string",
"format": "regex"
},
"maxItems": { "$ref": "#/$defs/nonNegativeInteger" },
"minItems": { "$ref": "#/$defs/nonNegativeIntegerDefault0" },
"uniqueItems": {
"type": "boolean",
"default": false
},
"maxContains": { "$ref": "#/$defs/nonNegativeInteger" },
"minContains": {
"$ref": "#/$defs/nonNegativeInteger",
"default": 1
},
"maxProperties": { "$ref": "#/$defs/nonNegativeInteger" },
"minProperties": { "$ref": "#/$defs/nonNegativeIntegerDefault0" },
"required": { "$ref": "#/$defs/stringArray" },
"dependentRequired": {
"type": "object",
"additionalProperties": {
"$ref": "#/$defs/stringArray"
}
},
"const": true,
"enum": {
"type": "array",
"items": true
},
"type": {
"anyOf": [
{ "$ref": "#/$defs/simpleTypes" },
{
"type": "array",
"items": { "$ref": "#/$defs/simpleTypes" },
"minItems": 1,
"uniqueItems": true
}
]
}
},
"$defs": {
"nonNegativeInteger": {
"type": "integer",
"minimum": 0
},
"nonNegativeIntegerDefault0": {
"$ref": "#/$defs/nonNegativeInteger",
"default": 0
},
"simpleTypes": {
"enum": [
"array",
"boolean",
"integer",
"null",
"number",
"object",
"string"
]
},
"stringArray": {
"type": "array",
"items": { "type": "string" },
"uniqueItems": true,
"default": []
}
}
}