litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.64k stars 382 forks source link

Bug: Pydantic model with method named `validate` is passed as a dict for `data` parameter #1117

Closed MatthewNewland closed 1 year ago

MatthewNewland commented 1 year ago

Describe the bug When a pydantic model with a root_validator is passed to the data parameter of a route handler, the data object is passed as a dict, rather than as the type of the model.

To Reproduce Minimal repro:

from typing import Any
from pydantic import BaseModel, root_validator
from starlite import Starlite, post

class Model(BaseModel):
    value: str

    @root_validator(pre=True)
    def validate(cls, values: dict[str, Any]) -> dict[str, Any]:
        return values

@post("/example")
def example_handler(data: Model) -> str:
    return data.value

app = Starlite(
    route_handlers=[example_handler]
)

Run with uvicorn

$ curl -d '{"value":"hello"}' http://localhost:8000/example -s -S -v -H "Content-Type: application/json" -H "Accept: application/json, */*"

{
    "status_code": 500,
    "detail": "AttributeError(\"dict object has no attribute value\")"
}

[!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

MatthewNewland commented 1 year ago

Note that this works fine if I get rid of the root_validator.

wassafshahzad commented 1 year ago

Hey @MatthewNewland I looked into this and for the most part I only got this error when my Model class contained a function with he name of "validate" irrespective of whether it was wrapped with root validator or not. Could you maybe try renaming the function and see if that helps. Looking deeper into this if you remove the validate function (or rename it) then

# line 204 in routes/http.py
parsed_kwargs = route_handler.signature_model.parse_values_from_connection_kwargs(
                connection=request, **kwargs
            )

sets parsed_kwargs to {data: PydanticModel(kwargs)}

However in your case it sets parsed_kwargs to { data: {kwarg : value } }

MatthewNewland commented 1 year ago

@wassafshahzad can confirm that it works when I rename the function to something other than validate. Very strange. I would think that's still a bug, though, so I'll leave it open for now.

peterschutt commented 1 year ago

The first thing that stands out to me with this is that as soon as I run mypy over the repro code I get an error:

main.py:10: error: Signature of "validate" incompatible with supertype "BaseModel"  [override]
Found 1 error in 1 file (checked 3 source files)

Pydantic's BaseModel has its own validate() method (ref) with the following signature:

    @classmethod
    def validate(cls: Type['Model'], value: Any) -> 'Model':

As you can see, BaseModel.validate() should return an instance of the model, but your override returns dict[str, Any].

Here's a reproduction without any starlite interaction at all:

    class ModelWithValidate(BaseModel):
        value: str

        @root_validator(pre=True)
        def validate(cls, values: dict[str, Any]) -> dict[str, Any]:
            return values

    class ModelWithoutValidate(BaseModel):
        value: str

    class ParentModel(BaseModel):
        data_with_validate: ModelWithValidate
        data_without_validate: ModelWithoutValidate

    parent_model = ParentModel(data_with_validate={"value": "hello"}, data_without_validate={"value": "hello"})
    print(repr(parent_model.data_with_validate))  # {'value': 'hello'}
    print(repr(parent_model.data_without_validate))  # ModelWithoutValidate(value='hello')

This pattern is essentially what the signature modelling does. We take the type annotations from the handler function and construct a parent model with the kwargs and type annotations of the handler function as the model attributes. That "signature model" gets instantiated with the kwargs that are extracted from the connection, in this case data={"value": "hello"}, and the rest is left up to pydantic.

I don't think this is something that we can/should try to fix.