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

Bug: Parsing tagged unions #3398

Open
1 of 4 tasks
ahrzb opened this issue Apr 17, 2024 · 1 comment
Open
1 of 4 tasks

Bug: Parsing tagged unions #3398

ahrzb opened this issue Apr 17, 2024 · 1 comment
Labels
Bug 🐛 This is something that is not working as expected DTOs This is related to our DTO feature OpenAPI This is related to our OpenAPI schema

Comments

@ahrzb
Copy link

ahrzb commented Apr 17, 2024

Description

When using tagged unions, while the correct OpenAPI schema is generated, decoding the request fails. It also doesn't work with the MsgpackDTO. It seems that the internal litestar.typing.FieldDefinition type doesn't support tagged unions.
A question thou, are tagged unions meant to be supported in litestar? I would be happy to contribute to fix/implement the solution if it's meant to be there.

URL to code causing the issue

No response

MCVE

from typing import Annotated, Literal
from litestar import Controller, Litestar, post
from litestar.contrib.pydantic import PydanticDTO
from pydantic import BaseModel, Field


class DataA(BaseModel):
    name: str
    type: Annotated[Literal["A"], Field("A")]


class DataB(BaseModel):
    name: str
    type: Annotated[Literal["B"], Field("B")]


class DataContainer(BaseModel):
    value: Annotated[DataA | DataB, Field(discriminator="type")]


class TaggedUnionExample(Controller):
    dto = PydanticDTO[DataContainer]

    @post("/data/")
    def post_data(self, data: DataContainer) -> str:
        return data.value.name


app = Litestar(route_handlers=[TaggedUnionExample])

Steps to reproduce

1. `LITESTAR_APP=example:app litestar run --debug`
2. `echo '{"value": {"type": "B", "name": "test"}}' | http post http://localhost:8000/data/`
4. See error

Screenshots

"![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

Traceback (most recent call last):
  File ".../.venv/lib/python3.12/site-packages/litestar/middleware/exceptions/middleware.py", line 219, in __call__
    await self.app(scope, receive, send)
  File ".../.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 82, in handle
    response = await self._get_response_for_request(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 134, in _get_response_for_request
    return await self._call_handler_function(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 154, in _call_handler_function
    response_data, cleanup_group = await self._get_response_data(
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 178, in _get_response_data
    data = await kwargs["data"]
           ^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/_kwargs/extractors.py", line 490, in dto_extractor
    return data_dto(connection).decode_bytes(body)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/contrib/pydantic/pydantic_dto_factory.py", line 100, in decode_bytes
    return super().decode_bytes(value)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/dto/base_dto.py", line 97, in decode_bytes
    return backend.populate_data_from_raw(value, self.asgi_connection)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/dto/_codegen_backend.py", line 144, in populate_data_from_raw
    return self._transfer_to_model_type(self.parse_raw(raw, asgi_connection))
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/dto/_backend.py", line 237, in parse_raw
    result = decode_json(value=raw, target_type=self.annotation, type_decoders=type_decoders)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../.venv/lib/python3.12/site-packages/litestar/serialization/msgspec_hooks.py", line 183, in decode_json
    return msgspec.json.decode(
           ^^^^^^^^^^^^^^^^^^^^
TypeError: If a type union contains multiple Struct types, all Struct types must be tagged (via `tag` or `tag_field` kwarg) - type `typing.Annotated[typing.Union[litestar.dto._backend.PostDataDataContainer_0DataARequestBody, litestar.dto._backend.PostDataDataContainer_1DataBRequestBody], msgspec.Meta(examples=[])]` is not supported

Litestar Version

2.8.2

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@ahrzb ahrzb added the Bug 🐛 This is something that is not working as expected label Apr 17, 2024
@JacobCoffee JacobCoffee added DTOs This is related to our DTO feature OpenAPI This is related to our OpenAPI schema labels Apr 17, 2024
@peterschutt
Copy link
Contributor

Dealing with tagged unions is definitly in scope for the DTOs. I'd like there to be an interface that is agnostic to the DTO type that handles these. Just like AbstractDTO.detect_nested_field() which must be defined by the framework-specific sub-types, we could have something like AbstractDTO.identify_union_tag() which would encapsulate the logic of identifying the tag field and value for a given type with respect to the modelling library that the DTO represents.

For example, for the PydanticDTO, we should be able to automatically handle where the union type on the model is tagged, but not when it isn't.

For msgspec, we should be right to pass the tags of the models that exist in the union type directly through to the generated transfer models for the reconstructed field annotation.

Another one I have my eye on is for sqlalchemy's polymorphic types, where each model has a polymorphic_on/polymorphic_identity mapper args, we should be able to use that to generate model tags.

Some things to consider:

  • if a tag field is excluded/not included from the nested model type via the dto config it should be an error
  • we should look into allowing the tag field and value to be declared by the DTOConfig object, if defined this would take priority over the auto-detection
  • if a union type is encountered where a tag field and value cannot be determined for the transfer model types, then we should raise an error at startup

Feel free to take a bite at this @ahrzb - contributions very welcome.

There is also the case of supporting tagged union types directly at the handler level, e.g.,:

@post(dto=...)
async def post_handler(data: TypeA | TypeB) -> ...: ...

However that will require more of a restructure in how the DTOs fundamentally work, so we should leave that out of scope for this IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 This is something that is not working as expected DTOs This is related to our DTO feature OpenAPI This is related to our OpenAPI schema
Projects
None yet
Development

No branches or pull requests

3 participants