BeanieODM / beanie

Asynchronous Python ODM for MongoDB
http://beanie-odm.dev/
Apache License 2.0
2.06k stars 217 forks source link

[BUG] Validation Error on parsing retrieved document's BSON Decimal128 field #691

Open adeelsohailahmed opened 1 year ago

adeelsohailahmed commented 1 year ago

Describe the bug When using Pydantic v2, Beanie doesn't convert the retrieved document's BSON Decimal128 type to Python's Decimal type. This leads to Pydantic raising validation error.

To Reproduce

```python import asyncio from decimal import Decimal from beanie import Document, init_beanie from bson import Decimal128 from motor.motor_asyncio import AsyncIOMotorClient user = "user" password = "password" # Data as saved in MongoDB decimal_128_data = {"price": Decimal128("125.52")} class Test(Document): price: Decimal async def init(): # Create Motor client client = AsyncIOMotorClient(f"mongodb://{user}:{password}@localhost:27017") # Init beanie with the Test document class await init_beanie(database=client.test, document_models=[Test]) retrieved_data_model = Test(**decimal_128_data) print(f"{retrieved_data_model=}") if __name__ == "__main__": asyncio.run(init()) ```

Expected behavior When using Pydantic v2, BSON Decimal128 type should automatically convert to Python's Decimal type, as defined in the Beanie model.

Additional context The error itself:

pydantic_core._pydantic_core.ValidationError: 1 validation error for Test
price
  Decimal input should be an integer, float, string or Decimal object [type=decimal_type, input_value=Decimal128('125.52'), input_type=Decimal128]
    For further information visit https://errors.pydantic.dev/2.3/v/decimal_type

The above example works fine with Pydantic v1. For Pydantic v2, I'm using the following workaround for the time being:

```python import asyncio from decimal import Decimal from typing import Any from beanie import Document, init_beanie from bson import Decimal128 from motor.motor_asyncio import AsyncIOMotorClient from pydantic import model_validator user = "user" password = "password" # Data as saved in MongoDB decimal_128_data = {"price": Decimal128("125.52")} class Test(Document): price: Decimal @model_validator(mode="before") @classmethod def convert_bson_decimal128_to_decimal(cls, data: dict[str, Any]) -> Any: for field in data: if isinstance(data[field], Decimal128): data[field] = data[field].to_decimal() return data async def init(): # Create Motor client client = AsyncIOMotorClient(f"mongodb://{user}:{password}@localhost:27017") # Init beanie with the Test document class await init_beanie(database=client.test, document_models=[Test]) retrieved_data_model = Test(**decimal_128_data) print(f"{retrieved_data_model=}") if __name__ == "__main__": asyncio.run(init()) ```
gsainsbury86 commented 1 year ago

I stumbled across this problem also yesterday. Thanks for reporting the issue and providing a nice workaround!

adeelsohailahmed commented 1 year ago

Thanks, @gsainsbury86. I'm glad you found it useful!

Apparently, Decimal Annotation needs to be imported directly from Beanie to keep the behavior consistent between Pydantic v1 and Pydantic v2.

The following code will work with both versions of Pydantic:

from beanie import DecimalAnnotation, Document

class Test(Document):
    price: DecimalAnnotation

While this works, it's a bit unclear and required me to dig into Beanie's source code. This should be documented, of course, but I do wonder if there's a better way to deal with it directly within Beanie.

roman-right commented 1 year ago

In Pydantic v2 Decimal type's handling got changed. I had to implement my own type wrapper (used in the @adeelsohailahmed example) to make it work. It is not documented yet, as I'm fixing other type problems (bson.Binary for example). If this will not work as expected, please let me know

gsainsbury86 commented 1 year ago

Thanks @roman-right . Maybe mine is a strange use-case but I have a system where I wanted to re-use my defined pydantic models between the front-end and back-end services in my application, but the front-end does not have access to Mongo. So what I ended up doing was defining my model from BaseModel and then extending that base and beanie.Document as well. The problem in this instance is that while not having beanie imported to the front-end, I had to use decimal.Decimal and so I still needed @adeelsohailahmed 's original workaround with the validator.

shared model:

class BaseProduct(BaseModel):
    code: str
    description: str
    capacity: Decimal
    uom: str
    unit_cost: Decimal
    type: ProductType

    @model_validator(mode="before")
    @classmethod
    def convert_bson_decimal128_to_decimal(cls, data: dict[str, Any]) -> Any:
        for field in data:
            if isinstance(data[field], Decimal128):
                data[field] = data[field].to_decimal()
        return data

back-end:

class Product(Document, BaseProduct):
    pass
nickleman commented 1 year ago

Another use case, I like to use the pydantic.condecimal type so I can also have validation of number of decimals and multiple of: stock: pydantic.condecimal(decimal_places=1, multiple_of=decimal.Decimal("0.5")) The work around works for me for now, would the workaround potentially be a better solution?

Or is this something we should post on pydantic to fix? Since, they should really handle this like they used to.

jirojo2 commented 1 year ago

It doesnt work for me at all now... It crashes both on save and on retrieval with the validation error. I am using Pydantic 2.

EDIT: The issue with the workaround comes back when you have nested models, then you have to apply the workaround for each nested model, not just to the Document one.

This is a simple unit test that crashes:

from decimal import Decimal
from beanie import Document, init_beanie
from mongomock_motor import AsyncMongoMockClient

from pydantic import BaseModel, TypeAdapter
import pytest
import pytest_asyncio

class MehSchema(BaseModel):
    price: Decimal

class Meh(Document, MehSchema):
    pass

@pytest_asyncio.fixture(autouse=True)
async def init_mongo():
    client = AsyncMongoMockClient()
    await init_beanie(document_models=[Meh], database=client.get_database(name="db"))

@pytest.mark.asyncio()
async def test_beanie_decimal128():
    json_data = '{"price": 1.5}'
    doc = TypeAdapter(Meh).validate_json(json_data)
    assert doc.price == Decimal("1.5")

    await doc.save()
    assert doc.price == Decimal("1.5")

    assert Meh.find_one().price == Decimal("1.5")
gsakkis commented 1 year ago

@jirojo2: Use beanie.DecimalAnnotation in place of the Decimal annotation.

@nickleman: As per the Pydantic docs for condecimal:

This function is discouraged in favor of using Annotated with Field instead.

This function will be deprecated in Pydantic 3.0.

The reason is that condecimal returns a type, which doesn't play well with static analysis tools.

@gsainsbury86: You can replace the model_validator workaround with

try:
    from beanie import DecimalAnnotation
except ImportError:
    from decimal import Decimal as DecimalAnnotation

Btw I just pushed a PR that simplifies DecimalAnnotation to one line so you may redefine it instead of trying to import it from Beanie.

nickleman commented 1 year ago

@gsakkis It may work if I use DecimalAnnotation, but then I can't specify the constraints I want in the Field() part of hte pydantic documentation. So, if I use the newer Annotated[Decimal, Field(decimal_places=1, multiple_of=Decimal("0.2"))] then I still need the model_validator workaround from above to make it all work. That's fine, but it should be documented, or the Document class should have a default validator for Decimal128 included to make it transparent to users.

github-actions[bot] commented 11 months ago

This issue is stale because it has been open 30 days with no activity.

github-actions[bot] commented 11 months ago

This issue was closed because it has been stalled for 14 days with no activity.

maxktz commented 11 months ago

@gsakkis It may work if I use DecimalAnnotation, but then I can't specify the constraints I want in the Field() part of hte pydantic documentation. So, if I use the newer Annotated[Decimal, Field(decimal_places=1, multiple_of=Decimal("0.2"))] then I still need the model_validator workaround from above to make it all work. That's fine, but it should be documented, or the Document class should have a default validator for Decimal128 included to make it transparent to users.

I found solution, I guess.. you can use

Annotated[DecimalAnnotation, Field(...)]

Instead of:

Annotated[Decimal, Field(...)]

It works and validates correctly.