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: beanie.PydanticObjectId cannot be a path parameter anymore #2158

Closed
4 tasks
gsakkis opened this issue Aug 13, 2023 · 9 comments
Closed
4 tasks

Bug: beanie.PydanticObjectId cannot be a path parameter anymore #2158

gsakkis opened this issue Aug 13, 2023 · 9 comments
Labels
Bug 🐛 This is something that is not working as expected

Comments

@gsakkis
Copy link
Contributor

gsakkis commented Aug 13, 2023

Description

I tried upgrading from v2.0.0beta2 to the latest main and found that using a beanie.PydanticObjectId as path parameter raises validation error. Using git bisect I traced it down to b32088f.

URL to code causing the issue

No response

MCVE

from beanie import PydanticObjectId
from litestar import Litestar, get


@get("/test/{id:str}")
async def test(id: PydanticObjectId) -> dict[str, PydanticObjectId]:
    return {"id": id}


app = Litestar(route_handlers=[test], type_encoders={PydanticObjectId: str})

Steps to reproduce

Right before b32088f (66700b7):

curl -s 'localhost:8000/test/64d6088586da7ecbfe53340f' | jq .
{
  "id": "64d6088586da7ecbfe53340f"
}

At b32088f:

curl -s 'localhost:8000/test/64d6088586da7ecbfe53340f' | jq .
{
  "status_code": 400,
  "detail": "Validation failed for GET http://localhost:8000/test/64d6088586da7ecbfe53340f",
  "extra": [
    {
      "message": "Unsupported type: <class 'str'>",
      "key": "id"
    }
  ]
}


### Screenshots

```bash
"![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

No response

Litestar Version

b32088f

Platform

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

Funding

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@gsakkis gsakkis added Bug 🐛 This is something that is not working as expected Triage Required 🏥 This requires triage labels Aug 13, 2023
@Goldziher
Copy link
Contributor

Hi, have you tried defining a type decoder for the beanie type?

@gsakkis
Copy link
Contributor Author

gsakkis commented Aug 13, 2023

No, how? I don't see a type_decoders parameter.

Also why I have to do something extra? The beanie type validates strings already:

In [1]: from beanie import PydanticObjectId

In [2]: PydanticObjectId("64d6088586da7ecbfe53340f")
Out[2]: ObjectId('64d6088586da7ecbfe53340f')

In [3]: PydanticObjectId("123")
...
InvalidId: '123' is not a valid ObjectId, it must be a 12-byte input or a 24-character hex string

@Goldziher
Copy link
Contributor

No, how? I don't see a type_decoders parameter.

Also why I have to do something extra? The beanie type validates strings already:

In [1]: from beanie import PydanticObjectId

In [2]: PydanticObjectId("64d6088586da7ecbfe53340f")
Out[2]: ObjectId('64d6088586da7ecbfe53340f')

In [3]: PydanticObjectId("123")
...
InvalidId: '123' is not a valid ObjectId, it must be a 12-byte input or a 24-character hex string

Msgspec doesn't know how to handle this type.

There is a type_decoders and type_encoders kwargs on all levels of the app.

@Goldziher
Copy link
Contributor

  • You need encoders for request and decoders for marshaling json

@gsakkis
Copy link
Contributor Author

gsakkis commented Aug 13, 2023

Ah I was at the breaking commit, type_decoders was added later. I'll dig in the code as there are no docs for it afaict.

@provinzkraut
Copy link
Member

I'll dig in the code as there are no docs for it afaict.

Yeah, we need to add those. We're lacking docs for a lot of serialisation intricacies in general.

@gsakkis
Copy link
Contributor Author

gsakkis commented Aug 13, 2023

This seems to do the trick:

app = Litestar(
    route_handlers=[test],
    type_encoders={PydanticObjectId: str},
    type_decoders=[
        (
            lambda target_type: issubclass(target_type, PydanticObjectId),
            lambda target_type, value: target_type.validate(value),
        )
    ],
)

Wondering if it would be possible to support automatically all Pydantic custom data types with __get_validators__ and/or the equivalent for Pydantic v2.

@provinzkraut
Copy link
Member

Wondering if it would be possible to support automatically all Pydantic custom data types with __get_validators__ and/or the equivalent for Pydantic v2.

Should be. We'd just need to add logic for those to the Pydantic plugin.

@gsakkis
Copy link
Contributor Author

gsakkis commented Aug 13, 2023

Thanks, closing this.

@gsakkis gsakkis closed this as completed Aug 13, 2023
@provinzkraut provinzkraut removed the Triage Required 🏥 This requires triage label Aug 13, 2023
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
Projects
None yet
Development

No branches or pull requests

3 participants