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

Add nullable support on schema #204

Closed
wants to merge 1 commit into from

Conversation

carlosrpg
Copy link

fixes issue asyncapi/generator#33
Ideally I would also add the html changes to display to the user the field is nullable but I don't have the time now

@fmvilas
Copy link
Member

fmvilas commented Apr 29, 2019

Hi @carlosrpg. I'm sorry but this is already fixed for version 2: https://github.com/asyncapi/asyncapi/blob/master/versions/next/schema.json#L337.

Version 1.x is closed and not accepting changes anymore. We'll soon release version 2.0.0 with this and many more changes: https://github.com/asyncapi/asyncapi/pull/188/files.

@fmvilas fmvilas closed this Apr 29, 2019
@carlosrpg
Copy link
Author

I can understand that but I imagined the 1.2 should be maintained for a while, specially since 2.0 spec differs so much from the previous one.
I'm trying to make it work for my company and we cannot recreate all the documentation already written on 1.2 spec.
Also, 2.0 was not even released yet. we will have to maintain a for of the cold code to make it work for us.

@fmvilas
Copy link
Member

fmvilas commented Apr 29, 2019

I'm sorry but right now we can't maintain older versions with the resources we have. It's me working full time on AsyncAPI and a bunch of other people making occasional contributions.

Can't you just use an extension like this?

x-nullable: true

Then it's just a matter of adding support for this extension in the existing tooling.

@tedepstein
Copy link

Hey @carlosrpg , @fmvilas & friends,

As it happens, I've just been doing a deep dive into questions and problems around nullable in OpenAPI. The short story is:

  • nullable is very difficult to reconcile with JSON Schema, because it has the effect of expanding the range of allowed values. JSON Schema is a constraint-based validation language, where assertion keywords effectively constrain the value space. In JSON Schema, constraints are cumulative, and cannot be relaxed or overridden.
  • As defined in the OpenAPI spec, nullable is ambiguous. If a schema specifies nullable: true, does that mean that null is always allowed, or is allowed subject to other constraints, like enum? JSON Schema makes it clear that constraints are always cumulative; they never expressly "allow" values, they only restrict values. But since nullable isn't a constraint, this is uncharted territory.
  • Making false the default value of nullable breaks JSON Schema's fundamental operating model. It means that a default schema -- even an empty object schema {} that doesn't specify nullable and doesn't specify type -- is more restrictive than a schema with nullable: true.
  • A default schema like this cannot be overridden to nullable: true via allOf inheritance. null is prohibited by the base schema, so it cannot be allowed in a subtype schema.

This is still being hammered out in OpenAPI, but a suggestion has been made to deprecate nullable and replace it with a limited form of JSON Schema's type array. In this proposed solution, type can either be a string or a 2-element array. If it's a string, it has to be a valid type name (including 'null'). If it's an array, one element must be 'null', and the other element must be a valid type name other than 'null'.

You might want to take a look at these issues:
OAI/OpenAPI-Specification#1389
OAI/OpenAPI-Specification#1368
OAI/OpenAPI-Specification#1900

@carlosrpg
Copy link
Author

carlosrpg commented Apr 29, 2019

I'm going just to share my user scenario.
We already have an OpenAPI documentation written for our REST API, we wont deprecate it.
We want to use asyncapi as a mean to document our WebSocket events and we share some common schema between them (that I already could reuse with 1.2 except for the nullable that was breaking the doc generator)
If the plan is to not going towards OpenAPI but uses your own schema from JSON schema we just cannot go forward with.
I would like to going towards using OpenAPI schema and talk over there if nullable is something that needs to go forward or not. (imo nullable have so much real world usage that I don't see a better way to handle it but that is another topic)

@carlosrpg
Copy link
Author

BTW, I've always imagined nullable = true as oneOf { string with default "null" or other object } but I guess I've never put much thought on it.

@fmvilas
Copy link
Member

fmvilas commented Apr 29, 2019

Thanks, @tedepstein! That helps a lot.

@carlosrpg In version 2.0.0, nullable will be there because we want to keep compatibility with OpenAPI. That said, most probably the way to do that will be specifying that your schema type is OpenAPI/AsyncAPI 1.x (as we'll support multiple schema types) and the default schema type will be JSON Schema Draft 07. OpenAPI — AsyncAPI interoperability (regarding schemas) is one of the things we want to keep maintaining to support scenarios like the one you just described.

I just ask for patience. We're going to release it soon. Tooling will be updated accordingly and we'll offer a migration script. In the meantime, please explore solutions using extensions like the one I described above.

@carlosrpg
Copy link
Author

That is great to hear! having a tool to upgrade it will be awesome!
I should expect the asyncapi-generator to be updated as well? right now I have to use my schema with nullable to make it work for us so I can call the asyncapi-generator and have a proper document page for our websockets.

@fmvilas
Copy link
Member

fmvilas commented Apr 29, 2019

It will be updated. Actually, this is what I'm working on right now.

@WaleedAshraf
Copy link
Contributor

@fmvilas do we support nullable in version 2.0.0?
I don't see any reference for it.

@tedepstein
Copy link

There have been some further developments in OpenAPI. Here is the plan, as of now:

  • We will do an OpenAPI 3.0.3 patch release to clarify nullable semantics. This will resolve the open questions and uncertainties described in my previous comment, and will ensure alignment with JSON Schema. The proposal is here.

  • Starting in OpenAPI 3.1, nullable will be defined the same way, but will be deprecated. OpenAPI 3.1 will support the full JSON Schema Draft 2019-09, including type arrays, so nullable isn't needed anymore. See the pull request here.

@fmvilas
Copy link
Member

fmvilas commented Nov 5, 2019

We don't support nullable anymore. The AsyncAPI schemas are supersets of JSON Schema, meaning we add properties but we don't remove/change the meaning of the existing ones. The idea is that OpenAPI and AsyncAPI will converge into standard JSON Schema. As Ted mentioned, this is going to happen in the upcoming OpenAPI releases 🎉

@WaleedAshraf
Copy link
Contributor

Thanks for clarification @tedepstein @fmvilas 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants