django-ninja icon indicating copy to clipboard operation
django-ninja copied to clipboard

[BUG] openapi schema errors when using optional type with alpha release

Open sasacocic opened this issue 2 years ago • 5 comments

Describe the bug

When I create a schema that has an optional value e.g.

from ninja.schema import Schema
class GetUserResponse(Schema):
    user: int | None = None

And try to validate it (I used this one online) I get get a bunch of errors, but specifically with the schemas:

  #/components/schemas/GetUserResponse/properties/user/anyOf/1/type must be equal to one of the allowed values
  #/components/schemas/GetUserResponse/properties/user/anyOf/1 must have required property '$ref'
  #/components/schemas/GetUserResponse/properties/user/anyOf/1 must match exactly one schema in oneOf
  #/components/schemas/GetUserResponse/properties/user must have required property '$ref'
  #/components/schemas/GetUserResponse/properties/user must match exactly one schema in oneOf
  #/components/schemas/GetUserResponse must have required property '$ref'
  #/components/schemas/GetUserResponse must match exactly one schema in oneOf

The component in question looks like this

      "GetUserResponse": {
        "properties": {
          "user": {
            "anyOf": [
              {
                "type": "integer"
              },
              {
                "type": "null"
              }
            ],
            "title": "User"
          }
        },
        "title": "GetUserResponse",
        "type": "object"
      }

If I change the schema to:

from ninja.schema import Schema
class GetUserResponse(Schema):
    user: int

There are no issues

Versions (please complete the following information):

  • Python version: 3.11.3
  • Django version: 3.2.20
  • Django-Ninja version: 1.0a3
  • Pydantic version: 2.3.0

sasacocic avatar Sep 04 '23 14:09 sasacocic

After further digging this just seems to be an issue with the openapi definition that django ninja generates. The schema that is generated is not valid for version 3.0, but it is valid for openapi schema version 3.1. In order to have a type that can be null you'd need to use the nullable key.

It seems like there are solutions forward (from what I can see)

  1. Bump the openapi version to v3.1 (this seems like what ninja is trying to do - at least in terms of generating schemas in the openapi spec - i.e valid json schema)
  2. keep the openapi spec as v3.0.x, but change the property to be nullable i.e. you'd potentially get something that looks like:

      "GetUserResponse": {
        "properties": {
          "user": {
            "nullable": true,
            "anyOf": [
              {
                "type": "integer"
              },
            ],
            "title": "User"
          }
        },
        "title": "GetUserResponse",
        "type": "object"
      }

some links talking about this

  1. openapi initiative - under the Swap nullable for type arrays section
  2. swagger null docs

@vitalik just wanted to bring this to your attention if you're not already aware of it.

EDIT: also tried this with the beta release and still ran into the same issue

sasacocic avatar Sep 16 '23 22:09 sasacocic

@sasacocic well django ninja here fully rely on pydantic...

can you also try with

user: Optional[int] = None

vitalik avatar Sep 17 '23 13:09 vitalik

I encountered another issue with nullable. Using anyOf leads to a new type definition generated by a sdk generator that I use.

class UserInfoResponseSchema(Schema):
    first_name: str | None
    ...

Creates following definition:

"UserInfoResponseSchema": {
        "properties": {
          "first_name": {
            "anyOf": [{ "type": "string" }, { "type": "null" }],
            "title": "First Name"
          },
        ...

And a redundant type definition is generated by @openapitools/openapi-generator-cli generate -g typescript-fetch ...:

export interface FirstName {
}

It's pretty annoying to deal with and I'm not sure who should fix it in the first place:

  1. I don't want switching from first_name: str | None to first_name: Optional[str], also didn't try if it works.
  2. It might be an issue with @openapitools/openapi-generator-cli but then each lang should handle it correctly and requires a separate fix.
  3. And it could be fixed by django-ninja - sounds like the most reasonable place to fix.

WDYT?

POD666 avatar Apr 11 '24 19:04 POD666

Seems to be an issue with pydantic: https://github.com/pydantic/pydantic/issues/7161

POD666 avatar Apr 12 '24 14:04 POD666

I endup with a following fix, seems to work well for me:

from ninja.schema import NinjaGenerateJsonSchema
from pydantic.json_schema import JsonSchemaValue


def patch():
    """
    Better support for nullable fields in OpenAPI schema generation
    https://github.com/pydantic/pydantic/issues/7161
    https://stackoverflow.com/questions/48111459/how-to-define-a-property-that-can-be-string-or-null-in-openapi-swagger
    """
    _get_flattened_anyof = NinjaGenerateJsonSchema.get_flattened_anyof

    def get_flattened_anyof(self, schemas: list[JsonSchemaValue]) -> JsonSchemaValue:
        result = _get_flattened_anyof(self, schemas)
        members = result.get('anyOf')
        if members and len(members) == 2 and {'type': 'null'} in members:
            return {'type': [t['type'] for t in members]}
        return result
    
    NinjaGenerateJsonSchema.get_flattened_anyof = get_flattened_anyof

This way I ensure that, e.g.:

# this is generated
"first_name": { "title": "First Name", "type": ["string", "null"] },
# instead of the 
"first_name": {
            "anyOf": [{ "type": "string" }, { "type": "null" }],
            "title": "First Name"
          },

Autogenerated sdk seems to work as expected, producing firstName: string | null; instead of creating some custom type: firstName: FirstName;.

POD666 avatar Apr 16 '24 12:04 POD666