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

Update Schema Objects to JSON Schema Draft 2019-09 #1977

Open
wants to merge 10 commits into
base: v3.1.0-dev
from

Conversation

@philsturgeon
Copy link
Contributor

commented Jul 18, 2019

Fixes #333
Fixes #397
Fixes #470
Fixes #1026
Fixes #1368
Fixes #1389
Fixes #1666
Fixes #1719
Fixes #1766
Fixes #1313
Fixes #1985

Picking up from where #1766 left off, this PR is helping OpenAPI v3.1 make a non-breaking change, to catch up with JSON Schema's latest draft.

Instead of OpenAPI continuing to confuse everyone by being a JSON Schema draft 05 sub-set, and a super-set, with some notable differences to watch out for, it will now be a JSON Schema draft 2019-09 superset. More info on that here.

The goal here is to help people write pure JSON Schema for their data model if they want to, or write OpenAPI-flavoured JSON Schema if they want to, but we are closing the gaps on what these two things mean. The list of differences is now tiny, only 4 extra keywords, with 2 notes about how exlusiveMinimum and exclusiveMaximum can be used in two ways (in order to maintain BC with OpenAPI v3.0).

Alternative Solutions

Alternative Schemas are a big topic which needs more work to be done. We should address the JSON Schema divergence issue to make the v3.x branch a happy place to be, and follow up with Alternative Schemas for v3.2 or v4.0, so our XML using friends can play with XML Schema, and all the other good things.

Risks

There are concerns that tooling vendors who work with static languages will have a hard time adding some of JSON Schemas newer more dynamic features. I understand this in theory, but most of JSON Schema's new features like if/then/else, and the old "type can be an array" issue, are just sugar for allOf/anyOf/oneOf, as can be seen in wework/json-schema-to-openapi-schema.

Seeing as allOf, anyOf, oneOf already exist in OpenAPI v3.0, whatever tooling vendors are doing to support that, should be done to support these keywords. Other new things like const are just an enum of 1.

Nothing should be that scary, yet people seem scared, but I think we can overcome in. We need to. JSON is a dynamic format, a property can contain a string one minute, and an object another, and this is pretty common practice for those using evolution. You might start out with a simple string, then decide you need more information and offer an object, deprecating the old way of doing things, and folks move over to use the object. This was an issue for reading JSON in Go for a while, then better JSON tooling came out and its not an issue.

OpenAPI tooling should be able to deal with this too. For example, ReDoc is an awesome open-source documentation generator, and it has no trouble at all with showing oneOf:

Screen Shot 2019-07-18 at 17 19 32

Screen Shot 2019-07-18 at 17 19 35

The argument I keep hearing is "tooling vendors are complaining about anyOf, allOf, oneOf, because it's hard, so lets not add any more keywords... Meh? Which is the priority here, making lives slightly easier for the tooling vendors by not adding new features, or making lives substantially less confusing for all the users of all the OpenAPI tools by letting them not have to unpack the differences between OpenAPI and JSON Schema? I lose 10% of every day helping people navigate this mess on APIs You Won't Hate, and I'm a tooling vendor at Stoplight.io. I'd rather have a few more complex keywords to figure out for the tooling, than have all the users of that tooling be constantly confused about which keywords they can use when, and how to convert from one to the other because different tools need OpenAPI or JSON Schema.

Anyhow, the tools that support advanced features will end up being more popular than the tools which do not. 🤷‍♂️

Life for tooling vendors trying to add these new keywords can be simplified in a few ways:

  • Provide guidance in the form of blog posts (I got this)
  • Common tooling (Stoplight got this for JS)
  • JSON Schema tools can now be used directly, instead of building OpenAPI-based tooling which is similar then tweaking it... 🤦‍♂

I'd like to dig into this fear of adopting actual JSON Schema further on the OpenAPI slack, so please swing by and throw examples at me. I have heard vague concerns about patternProperties, but would love to sink my teeth into concrete examples.

@philsturgeon philsturgeon changed the base branch from master to v3.1.0-dev Jul 18, 2019

@philsturgeon philsturgeon force-pushed the philsturgeon:json-schema-update branch 2 times, most recently from 3cf26af to dd328b7 Jul 18, 2019

@philsturgeon philsturgeon changed the title Updated OpenAPI Schema Objects to JSON Schema Draft 08 Update Schema Objects to JSON Schema Draft "08" Jul 18, 2019

@philsturgeon philsturgeon force-pushed the philsturgeon:json-schema-update branch from dd328b7 to f01c260 Jul 18, 2019

@handrews

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

@philsturgeon did you mean "picking up from where #1766 left off"? You linked back to this PR in your actual description.

versions/3.1.0.md Outdated Show resolved Hide resolved
Update versions/3.1.0.md
Co-Authored-By: Kyle Fuller <kyle@fuller.li>
- contains
- const
- examples
- if / then / else

This comment has been minimized.

Copy link
@jcarres-mdsol

jcarres-mdsol Jul 23, 2019

Worried that for instance these keys will make creating tooling which supports 3.1 much more difficult than supporting 3.0

This comment has been minimized.

Copy link
@philsturgeon

philsturgeon Jul 23, 2019

Author Contributor

@jcarres-mdsol could you expand on that? What sort of tooling are you concerned about, and how do you think it is likely to make tooling more complicated? Is it simply "more is worse" or are there specific concerns about if / then / else in particular?

Did you see my link to the notes about how if: X, then: Y, else: Z is equivalent to oneOf: [allOf: [X, Y], allOf: [not: X, Z]]?

This comment has been minimized.

Copy link
@handrews

handrews Jul 23, 2019

Contributor

@jcarres-mdsol the other important thing about moving to draft-08 is that it supports extensible vocabularies, and one of the primary use cases for that is to improve OpenAPI tooling support.

For example, it is currently very awkward to use JSON Schema as a data definition language- it is simply not designed for that. The current JSON Schema vocabulary is a constraint system, not a definition system.

However, draft-08 will allow defining keywords that (for example) can disambiguate how to interpret an allOf (is this an inheritance structure where one branch is the base class and therefore code gen tools should generate and connect two classes? or is it just gluing some re-usable fields together and the code-gen tool should ignore the pieces and generate one class from the result of the allOf?)

Ultimately, draft-08 should make tooling dramatically easier to create. That's the whole point of it. But those parts don't show up in this PR yet for a variety of reasons.

This comment has been minimized.

Copy link
@handrews

handrews Jul 23, 2019

Contributor

From a validation perspective, if/then/else says how to figure out which set of rules to use, and how to apply them.

From a data definition perspective, an if/then/else probably means something like "there are two subclasses here."

We need to stop trying to contort generative tooling (code gen, ui gen, doc gen) to fit a constraint system and instead add keywords that support generative use cases. Using the JSON Schema validation vocabulary to generate things is like trying to hammer a nail with a screwdriver. You can probably make it happen if the screwdriver has a nice heavy handle, but it's kind of ridiculous to do.

Up until now, OpenAPI has only had the options of doing that hammering, or breaking compatibility with JSON Schema. And they've tried to strike a balance between those two distasteful options. Draft-08 is designed to move past that problem and provide a real solution.

This comment has been minimized.

Copy link
@jcarres-mdsol

jcarres-mdsol Jul 23, 2019

I was thinking of a couple of use-cases.
One is generation for statically typed languages but as per your explanation of the meaning of if-then-else, if generators already have support for oneOf we have already crossed that bridge so I would rectify my comment, it should not be much extra work.

in-house we have a linter which applies some rules, we have had multiple fixes over oneOf , allOf. I do agree, bringing this vocabulary more in line with draft-8 would be at least less confusing for people implementing the tools.

So, I'd like to change my comment to a +1 on this!

@handrews handrews referenced this pull request Jul 24, 2019
versions/3.1.0.md Outdated Show resolved Hide resolved
versions/3.1.0.md Outdated Show resolved Hide resolved
versions/3.1.0.md Outdated Show resolved Hide resolved
<a name="schemaExample"></a>example | Any | A free-form property to include an example of an instance for this schema. To represent examples that cannot be naturally represented in JSON or YAML, a string value can be used to contain the example with escaping where necessary.
<a name="schemaDeprecated"></a> deprecated | `boolean` | Specifies that a schema is deprecated and SHOULD be transitioned out of usage. Default value is `false`.

This comment has been minimized.

Copy link
@darrelmiller

darrelmiller Aug 3, 2019

Member

What are the consequences of removing this? Will this break anyone?

This comment has been minimized.

Copy link
@philsturgeon

philsturgeon Aug 4, 2019

Author Contributor

This is removed as an “extra keyword” because it exists in JSON Schema and is in the list of “JSON Schema keywords that are supported”.

Seeing as it works the same, everything is fine :)

This comment has been minimized.

Copy link
@handrews

handrews Aug 5, 2019

Contributor

@darrelmiller translation: We shamelessly lifted this keyword (and also writeOnly) from OAS 😛

@philsturgeon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2019

@darrelmiller I have addressed you latest round of feedback, please lmk what you think!

versions/3.1.0.md Outdated Show resolved Hide resolved
versions/3.1.0.md Show resolved Hide resolved
@fmvilas

This comment has been minimized.

Copy link

commented Aug 9, 2019

💯 to this. At AsyncAPI, we'll adopt a superset of Draft-07 for now. It shouldn't be hard to realign again with OpenAPI once this PR is merged.

@darrelmiller

This comment has been minimized.

Copy link
Member

commented Aug 11, 2019

I see nothing wrong with this PR. That doesn't mean it doesn't cause problems that I am not aware of, but I think this is a good place to start trying to determine what the impact of this change would be on existing users. //cc @OAI/tsc

@philsturgeon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

@fmvilas asked a fantastic question:

One last question: is the OpenAPI schema going to allow extra properties as JSON Schema does? I mean, without counting the extensions. For instance:

type: string
whatever: "whatever value"

Would this be a valid OpenAPI schema?

What do you think @OAI/tsc @darrelmiller ?

OpenAPI Schema Objects disallow "extra properties" but JSON Schema has a default of allowing any properties in and only working with the ones it knows. This can be useful for backwards compatibility, but it can potentially lead to situations where a typo means a keyword isn't actually doing what an author thinks it does.

I would lean towards consistency, but if this restriction needs to remain we can have a single note about it elsewhere and not on a per-keyword basis. Basically part of the vocabulary would be "only keywords from OpenAPI Schema Object Vocabulary, JSON Schema Validation, and JSON Schema Core are allowed" and boom that's it.

@handrews

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

@philsturgeon one use case I specifically kept in mind while designing the vocabulary vs meta-schema functionality was the idea that OpenAPI could use a meta-schema to forbid non-x--prefixed extension keywords through patternProperties.

@@ -2329,7 +2324,7 @@ In addition to the JSON Schema fields, the following OpenAPI vocabulary fields M
##### Fixed Fields
Field Name | Type | Description
---|:---:|---
<a name="schemaNullable"></a>nullable | `boolean` | Allows sending a `null` value for the defined schema. Default value is `false`. Deprecated. Type arrays are now preferred, using the new `"null"` type, e.g: `"type": ["string", "null"]`.
<a name="schemaNullable"></a>nullable | `boolean` | Allows sending a `null` value for the defined schema. Default value is `false`. Deprecated. Type arrays are now preferred, using the new `"null"` type, e.g: `"type": ["string", "null"]`. Nullable cannot be used in conjunction with a type array and implementations MUST ignore it.

This comment has been minimized.

Copy link
@handrews

handrews Sep 9, 2019

Contributor

How about:

nullable | boolean | Allows sending a null value for the defined schema when used alongside the type keyword with a single value. Default value is false. Deprecated. Type arrays are now preferred, using the "null" type alongside one or more other types, e.g: "type": ["string", "null"]. nullable cannot be used in conjunction with a type array, or when type is absent, and implementations MUST ignore it in such cases. In particular, the default value of nullable MUST be ignored when type is absent or when type has an array value such as ["string", "null"].

This will ensure that the empty schema, {}, correctly allows the JSON null literal as a valid value. To do otherwise violates a fundamental design principle of JSON Schema, and there is no way we can claim compatibility. The empty schema MUST NOT ever produce an invalid result.

@philsturgeon philsturgeon changed the title Update Schema Objects to JSON Schema Draft "08" Update Schema Objects to JSON Schema Draft 2019-09 Sep 19, 2019

@philsturgeon

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

Awesome progress made on the TSC today. @tedepstein if you can drop those type array / nullable clarifications in here as a comment we can figure out the exact wording and get this PR ready for consideration.

@tedepstein

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2019

Summarizing from today's weekly TSC call, here are my suggestions and comments.

type keyword

Move the type keyword from the as-defined-in-JSON-Schema keywords list to the adjusted-to-OAS list. Suggested language for this entry:


  • type - Used to restrict the allowed types as specified in JSON Schema, but may be modified by the nullable keyword.

nullable keyword

Suggested content for the nullable definition:


Field Name Type Description
nullable boolean Allows or disallows sending a null value for the defined schema. A nullable value of false disallows null unconditionally, equivalent to "not" : {"type" : "null"} . A nullable value of true allows null in addition to other specified type values only if nullable and type are both specified within the same schema context. The effect of "nullable" : true is limited to its expansion of the set of allowed types, within the scope of its containing schema. An enum, const, allOf, or other keyword can independently prohibit null values, effectively overriding "nullable" : true.

Deprecated. The type property now allows "null" as a type, alone or within a type array. This is the standard way to allow null values in JSON Schema. Use of nullable is discouraged, and later versions of this specification may remove it.

Note that there is no default value specified in the above definition. I don't think we need a default value anymore, and the semantics are clearer without it.

This addresses nullable : false and nullable : true as two separate cases:

  • nullable : false is an independent assertion that simply disallows null values, regardless of what may be specified in type. Like all constraining assertions in JSON Schema, it is unconditional.

  • nullable : true works as a modifier or qualifier of the type keyword.

    • As discussed on Slack, nullable : true has no effect in the absence of a type assertion (direct or inherited), since typeless schemas already allow null along with all of the other types.

    • The effect of nullable : true is conditional, subject to other keywords that can disallow null, effectively overriding nullable : true. Tools can detect and warn in cases like this.

    • @philsturgeon , I tried hard to avoid saying that nullable : true "adds 'null' to the type array". That's really how I'm thinking about it now. Hopefully the way it's worded is clear enough, without implying any superpowers.

@philsturgeon philsturgeon force-pushed the philsturgeon:json-schema-update branch from 569f4ef to af1fa6e Sep 20, 2019

@philsturgeon

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2019

I have pushed the exact wording from @tedepstein, with some artistic liberties taken with <br> tags for formatting.

Screen Shot 2019-09-20 at 15 48 27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.