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] Validation Error on parsing retrieved document's BSON Decimal128 field #691

Closed
adeelsohailahmed opened this issue Sep 5, 2023 · 11 comments
Labels

Comments

@adeelsohailahmed
Copy link

adeelsohailahmed commented Sep 5, 2023

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

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:
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
Copy link

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

@adeelsohailahmed
Copy link
Author

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
Copy link
Member

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
Copy link

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
Copy link

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
Copy link

jirojo2 commented Oct 11, 2023

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
Copy link
Contributor

gsakkis commented Oct 12, 2023

@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
Copy link

@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.

Copy link

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

@github-actions github-actions bot added the Stale label Nov 16, 2023
Copy link

github-actions bot commented Dec 1, 2023

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

@github-actions github-actions bot closed this as completed Dec 1, 2023
@maxktz
Copy link

maxktz commented Dec 2, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants