koxudaxi / datamodel-code-generator

Pydantic model and dataclasses.dataclass generator for easy conversion of JSON, OpenAPI, JSON Schema, and YAML data sources.
https://koxudaxi.github.io/datamodel-code-generator/
MIT License
2.65k stars 296 forks source link

OpenAPI with oneOf prevents direct access to generated model's fields #1779

Open GC-Elia opened 9 months ago

GC-Elia commented 9 months ago

Describe the bug Given the following OpenAPI yaml:

openapi: 3.0.0
components:
  schemas:
    InventoryItem:
      type: object
      properties:
        item-id:
          type: string
        external-ids:
          type: array
          items:
            type: string
      oneOf:
        - required: [ 'item-id' ]
        - required: [ 'external-ids' ]

And the following generation commands:

datamodel-codegen --input inventory.yaml --input-file-type openapi --output inventory.py --output-model-type pydantic_v2.BaseModel

The resulted model is:

from __future__ import annotations
from typing import List, Optional, Union
from pydantic import BaseModel, Field, RootModel

class InventoryItem1(BaseModel):
    item_id: str = Field(..., alias='item-id')
    external_ids: Optional[List[str]] = Field(None, alias='external-ids')

class InventoryItem2(BaseModel):
    item_id: Optional[str] = Field(None, alias='item-id')
    external_ids: List[str] = Field(..., alias='external-ids')

class InventoryItem(RootModel[Union[InventoryItem1, InventoryItem2]]):
    root: Union[InventoryItem1, InventoryItem2]

The following action fails:

>>> from inventory import InventoryItem
>>> InventoryItem(**{'item-id': '123'}).item_id

Traceback (most recent call last):
  File "$PYDIR/site-packages/pydantic/main.py", line 761, in __getattr__
    raise AttributeError(f'{type(self).__name__!r} object has no attribute {item!r}')
AttributeError: 'InventoryItem' object has no attribute 'item_id'

While the following succeeds:

>>> from inventory import InventoryItem
>>> InventoryItem(**{'item-id': '123'}).root.item_id
123

Or using the following generation command for pydantic v1 models

datamodel-codegen --input inventory.yaml --input-file-type openapi --output inventory.py

The resulted model is:

from __future__ import annotations
from typing import List, Optional, Union
from pydantic import BaseModel, Field

class InventoryItem1(BaseModel):
    item_id: str = Field(..., alias='item-id')
    external_ids: Optional[List[str]] = Field(None, alias='external-ids')

class InventoryItem2(BaseModel):
    item_id: Optional[str] = Field(None, alias='item-id')
    external_ids: List[str] = Field(..., alias='external-ids')

class InventoryItem(BaseModel):
    __root__: Union[InventoryItem1, InventoryItem2]

The following action fails:

>>> from inventory import InventoryItem
>>> InventoryItem(**{'item-id': '123'}).item_id

Traceback (most recent call last):
  "$PYDIR/site-packages/pydantic/_internal/_model_construction.py", line 92, in __new__
    private_attributes = inspect_namespace(
  File "$PYDIR/site-packages/pydantic/_internal/_model_construction.py", line 316, in inspect_namespace
    raise TypeError("To define root models, use `pydantic.RootModel` rather than a field called '__root__'")
TypeError: To define root models, use `pydantic.RootModel` rather than a field called '__root__'

Expected behavior Being able to access pydantic v2 model properties directly such as model.item_id and not model.root.item_id.

Version:

koxudaxi commented 9 months ago

@GC-Elia

Being able to access pydantic v2 model properties directly such as model.item_id and not model.root.item_id.

How do we access to root attribute without root attribute? I think RootModel provides a root attribute to access the content.

RootModel is the feature of pydantic. datamodel-code-generator is not owened. https://docs.pydantic.dev/latest/concepts/models/#rootmodel-and-custom-root-types

You may want Type Adapter to parse the object. https://docs.pydantic.dev/latest/concepts/type_adapter/

...

InventoryAdapter = TypeAdapter(Union[InventoryItem1, InventoryItem2])
print(InventoryAdapter.validate_python({'item-id': '123'}).item_id)
# 123
GC-Elia commented 9 months ago

Hi @koxudaxi, This change seem to have been introduced in https://github.com/koxudaxi/datamodel-code-generator/pull/1682 as previously oneOf generated both fields as Optional, albeit it was not enforcing the oneOf requirement, direct field access was supported (model.item_id). Other than breaking behavior, I believe it also introduces inconsistency in the generated models where some support direct field access and others not.

Meanwhile I've modified RootModel.jinja2 (simple addition as seen below) to support direct field access, and I wonder if this is something that may be added by default to retain previous behavior, or if you see any downside with this approach.

    def __getattr__(self, attr: str):
        return getattr(self.root, attr)
koxudaxi commented 9 months ago

@GC-Elia

This change seem to have been introduced in https://github.com/koxudaxi/datamodel-code-generator/pull/1682 as previously oneOf generated both fields as Optional,

I'm sorry. The implementation at this time was completely wrong. It does not follow the correct oneof definition because all the attributes of multiple models are implemented as optional.

Other than breaking behavior, I believe it also introduces inconsistency in the generated models where some support direct field access and others not.

I agree with you. It is true that breaking change should be avoided as much as possible, but due to Python and Pydantic restrictions on rootModel and .root attributes, it would be difficult to reproduce the exact same structure as OpenAPI.

Meanwhile I've modified RootModel.jinja2 (simple addition as seen below) to support direct field access, and I wonder if this is something that may be added by default to retain previous behavior, or if you see any downside with this approach.

Doesn't this hack completely destroy the type-checking mechanism?

I believe it also introduces inconsistency in the generated models where some support direct field access and others not.

It is indeed inconsistent from this point of view, but from the type-checking point of view, it can check if the root model definitely exists, and type checking also shows that there is a union set up within it.

Long story short, it means that you should run the IDE type-chekcing or mypy before you run the code.