Optional not generated for nullable strings in OpenAPI

[languitar]

Describe the bug

When using type: string in combination with nullable: true in an OpenAPI specification, the resulting type should be Optional[str] but it is not.

To Reproduce

Example schema:

openapi: 3.0.3
info:
  version: 1.0.0
  title: testapi
  license:
    name: proprietary
servers: []
paths: {}
components:
  schemas:
    TopLevel:
      type: object
      properties:
        cursors:
          type: object
          properties:
            prev:
              type: string
              nullable: true
          required:
          - prev
      required:
      - cursors

Used commandline:

$ env/bin/datamodel-codegen --input spec.yaml --output test.py

Expected behavior

The generated class should be

class Cursors(BaseModel):
    prev: Optional[str]

but currently it is:

class Cursors(BaseModel):
    prev: str

Version:

• OS: Linux
• Python version: 3.9
• datamodel-code-generator version: 0.7.0

[languitar]

Maybe this is a regression. #8 looks like this used to be implemented.

[goodoldneon]

The behavior around Optional is a little strange. In addition to your problem, making a field non-required and non-nullable with a default results in an Optional type. But if a non-required field has a default, and cannot be set to null, then it should never be None in Python.

[koxudaxi]

@languitar
Thank you for creating this suggestion.
The behavior is not regression. required and nullable have not been supported at the same time yet.

I think that If required and nullable are used then, the generator should return prev: Optional[str] = ...
https://pydantic-docs.helpmanual.io/usage/models/#required-optional-fields

Also, I expect prev: Optional[str] is generated from below

...
type: object
  properties:
    prev:
      type: string

How did you feel it?

[koxudaxi]

@goodoldneon

But if a non-required field has a default, and cannot be set to null, then it should never be None in Python.

Interesting.

OK, I write an example.

...
type: object
  properties:
    prev:
      type: string
      default: exapmle

Did you expect the field from the schema?
prev: str = Field(..., default='example')

Your opinion is correct, But the change is a too huge an impact.
We should provide the behavior as a CLI option.
The option may be --strict-nullable or another name.

What did you think about it?

[languitar]

I think that If required and nullable are used then, the generator should return prev: Optional[str] = ...
https://pydantic-docs.helpmanual.io/usage/models/#required-optional-fields

Also, I expect prev: Optional[str] is generated from below

...
type: object
  properties:
    prev:
      type: string

How did you feel it?

Anything that enables me to also provide None values on the Python side is fine for me. Right now this simply isn't possible.

[goodoldneon]

@goodoldneon

But if a non-required field has a default, and cannot be set to null, then it should never be None in Python.

Interesting.

OK, I write an example.

...
type: object
  properties:
    prev:
      type: string
      default: exapmle

Did you expect the field from the schema?
prev: str = Field(..., default='example')

Your opinion is correct, But the change is a too huge an impact.
We should provide the behavior as a CLI option.
The option may be --strict-nullable or another name.

What did you think about it?

Yes, that's what I'd expect. I think OpenAPI defaults nullable to false, which would mean that prev could never be None in Python.

A CLI argument sounds like a good choice! That'd make sure we don't introduce breaking changes for other people.

[languitar]

Yes, that's what I'd expect. I think OpenAPI defaults nullable to false

That's right.

Is there any change to get a quick fix for this? This is currently blocking one of our projects.

[koxudaxi]

@languitar

Sorry, Nothing!!
I want to add the option. But, I'm working on daily work now.
I will add it after dinner tonight. Is it OK?

[languitar]

That's perfectly fine. Your fixes always extremely fast :)

[koxudaxi]

@languitar @goodoldneon
I just released a new version 0.7.1
The version has --strict-nullable option.

command

$ datamodel-codegen --input openapi.yaml --input-file-type=openapi --strict-nullable

openapi.yaml

openapi: 3.0.3
info:
  version: 1.0.0
  title: testapi
  license:
    name: proprietary
servers: []
paths: {}
components:
  schemas:
    TopLevel:
      type: object
      properties:
        cursors:
          type: object
          properties:
            prev:
              type: string
              nullable: true
          required:
          - prev
      required:
      - cursors
    pet:
      type: object
      properties:
          name:
            type: string
            default: dog

output

# generated by datamodel-codegen:
#   filename:  openapi.yaml
#   timestamp: 2021-02-05T19:57:32+00:00

from __future__ import annotations

from typing import Optional

from pydantic import BaseModel, Field


class Cursors(BaseModel):
    prev: Optional[str] = Field(...)


class TopLevel(BaseModel):
    cursors: Cursors


class Pet(BaseModel):
    name: str = 'dog'

@goodoldneon
Btw, pydantic doesn't support prev: str = Field(..., default='example')
We should write prev: str = 'example' for non-required field.

[languitar]

Thanks again for fast resolution! That version and CLI flag seems to work for the use case I have described. However, now I am also seeing other changes to the generated models that I do not expect. For instance, I am now seeing to following hunk:

The respective OpenAPI type is:

Problem:
  type: object
  properties:
    type:
      type: string
      format: uri
      description: An absolute URI that identifies the problem type. When dereferenced,
        it SHOULD provide human-readable documentation for the problem type (e.g.,
        using HTML).
      default: about:blank
      example: https://zalando.github.io/problem/constraint-violation
    title:
      type: string
      description: 'A short, summary of the problem type. Written in english and
        readable for engineers (usually not suited for non technical stakeholders
        and not localized); example: Service Unavailable'
    status:
      type: integer
      format: int32
      description: The HTTP status code generated by the origin server for this
        occurrence of the problem.
      example: 503
    detail:
      type: string
      description: A human readable explanation specific to this occurrence of
        the problem.
      example: Connection to database timed out
    instance:
      type: string
      format: uri
      description: An absolute URI that identifies the specific occurrence of
        the problem. It may or may not yield further information if dereferenced.

None of the fields are required (there is no required keyword at all in the object type) and hence can be left empty (apart from type which has a default value). So again I would expect that they are generated using Optional. Is this intended?

[koxudaxi]

@languitar

Thank you for testing the new version.

Is this intended?

Yes. But, I can discuss it with you.

I imagined this matrix

required	has default	nullable	result
Yes	No	No	name: str
Yes	Yes	No	name: str = ...
Yes	Yes	Yes	name: Optional[str] = ...
Yes	No	Yes	name: Optional[str] = ...
No	Yes	No	name: str = 'example'
No	Yes	Yes	name: Optional[str] = 'example'
No	No	Yes	name: Optional[str] = None
No	No	No	name: str = None

You said the last row should be name: Optional[str] = None on the table.
But, I understand nullable adds Optional to the field.
Which do users expect behavior? 🤔
If they can use the option clearly then, I will change it.

@goodoldneon
I want to hear your opinion on the problem.

[languitar]

For the last case of your table, OpenAPI explicitly allows an absent value. If there is no Optional in Python for this case, then I cannot represent one case that is explicitly intended by OpenAPI. So I don't see how this can be avoided without being less expressive on the Python-side than with the specification.

[koxudaxi]

Thank you for your comment.
I agree. we should write the table on the document to explain this option.

I'm waiting for other feedback.

[goodoldneon]

Thank you so much for the quick fix, @koxudaxi! The new version fixes my problem when I use the --strict-nullable flag. I really appreciate your responsiveness, and thanks for maintaining such a great tool!

[koxudaxi]

@languitar
I have released a new version as 0.7.2
Could you check it out?

[languitar]

Thank you again for being so quick on this one! This looks now exactly as expected!

[languitar]

Should I close the issue?

[koxudaxi]

I have closed the issue.
Thank you!!