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: true on optional fields #3

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

avandecreme
Copy link

@avandecreme avandecreme commented Feb 9, 2023

Related to biocad#71

I made all Maybe fields both optional (not in the required list) and nullable because this is what Aeson does.

This is easily done when the definition is inlined, you just add nullable: true to the definition.

When we have Ref, OpenApi 3.0 sucks. See:
OAI/OpenAPI-Specification#1368

In the end I followed the recommendation made here: OpenAPITools/openapi-generator#14181 (comment)

The first commit should be mergeable to upstream should anyone want to.
The second commit is specific to this fork since it simplifies #2 (by relying on the first commit changes). It also fixes some doctests failures introduced by #2.
The third commit should be mergeable to upstream as well. It fixes the validation in presence of nullable true.

@teto teto merged commit d58d09c into novainsilico:master Mar 14, 2023
@shinzui
Copy link

shinzui commented Apr 27, 2023

@avandecreme This PR generates incorrect OpenAPI schema for nullable sum types such as Color. For example, the following JSON snippet shows the incorrect output:

"Color": {
    "anyOf": [
        {
            "$ref": "#/components/schemas/Color"
        },
        {
            "nullable": true,
            "type": "object"
        }
    ]
},

This specifies that the API accepts either a Color object or any nullable object, which is incorrect.

Instead, the OpenAPI schema should use "allOf" and a separate "nullable": true" property to specify that the API accepts a nullable Color object only, like so:

"Color": {
    "allOf": [
        {
            "$ref": "#/components/schemas/Color"
        }
    ],
    "nullable": true
},

This accurately specifies that the API accepts a Color object that can also be null.

@avandecreme
Copy link
Author

The following comment disagree with you: OpenAPITools/openapi-generator#14181 (comment)

The spec says A true value adds "null" to the allowed type specified by the type keyword, only if type is explicitly defined within the same Schema Object.
There is no type in the Schema Object where you want to set "nullable": true.

@avandecreme avandecreme deleted the nullable branch April 28, 2023 07:16
@shinzui
Copy link

shinzui commented May 2, 2023

I got the solution from this issue on the OpenAPI repo, and this stack overflow answer since the typescript generator was generating invalid types.

I'll take another look to see if the generator is wrong. The solution is much simpler if openapi3 supported the 3.1 spec.

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.

3 participants