BeanieODM / beanie

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

[BUG] Frozen fields not supported (Pydantic 2) - exception thrown on `.save()` #863

Open TheBloke opened 7 months ago

TheBloke commented 7 months ago

Describe the bug Beanie does not appear to support frozen fields.

Testing with Beanie 1.25.0 on Pydantic 2.6.1.

Trying to .save() a model with a frozen field throws a validation error. Using insert() does work, but then it will fail if you ever call .save() on it.

Exception thrown:

pydantic_core._pydantic_core.ValidationError: 1 validation error for ConfigModel
creation_datetime
  Field is frozen [type=frozen_field, input_value=datetime.datetime(2024, 2, 17, 11, 16, 52, 604000), input_type=datetime]
    For further information visit https://errors.pydantic.dev/2.6/v/frozen_field

Exception thrown from beanie/odm/utiils/parsing.py:

def merge_models(left: BaseModel, right: BaseModel) -> None:
    """
    Merge two models
    :param left: left model
    :param right: right model
    :return: None
    """
    from beanie.odm.fields import Link

    for k, right_value in right.__iter__():
        left_value = getattr(left, k)
        if isinstance(right_value, BaseModel) and isinstance(
            left_value, BaseModel
        ):
            if get_config_value(left_value, "frozen"):   <------- checks if the model is frozen, but not individual fields?
                left.__setattr__(k, right_value)
            else:
                merge_models(left_value, right_value)
            continue
        if isinstance(right_value, list):
            links_found = False
            for i in right_value:
                if isinstance(i, Link):
                    links_found = True
                    break
            if links_found:
                continue
            left.__setattr__(k, right_value)
        elif not isinstance(right_value, Link):
            left.__setattr__(k, right_value)   <-------------- exception thrown here on frozen=True field

To Reproduce

from pydantic import Field
from motor.motor_asyncio import AsyncIOMotorClient
from beanie import Document, init_beanie

class TestFrozen(Document):
    some_field: str = Field(frozen=True)

    class Settings:
        name = "test_frozen"

async def test_frozen():
    await init_beanie(database=client[DB_NAME], document_models=[TestFrozen])

    test_frozen = TestFrozen(some_field="test")

    # will throw pydantic.validation_error "Field is frozen [type=frozen_field, input_value='test', input_type=str]"
    await test_frozen.save() 
    # test_frozen.insert() does NOT throw, but will if you later .find_one()  and then .save() the row

await test_frozen()

Expected behavior

Frozen fields are handled, and do not break saving. I guess it will be necessary to modify merge_models() to check on a per field basis if a field is frozen, and handle it differently.

Thanks in advance.

TheBloke commented 7 months ago

Frozen models don't work either, but I guess this is expected because of the id field.

It'd be really nice if it was possible to mark a model as ConfigDict(frozen=True) and have this apply to all fields except the id field. But I guess that might be a bigger ask.

For completeness, here's a reproduction case for a frozen=True model in Pydantic 2:

from pydantic import Field, ConfigDict
from motor.motor_asyncio import AsyncIOMotorClient
from beanie import Document, init_beanie

class TestFrozen(Document):
    model_config = ConfigDict(frozen=True)
    some_field: str

    class Settings:
        name = "test_frozen"

async def test_frozen():
    await init_beanie(database=client[DB_NAME], document_models=[TestFrozen])

    test_frozen = TestFrozen(some_field="test")

    await test_frozen.save()
    # also fails with test_frozen.insert()

await test_frozen()

Result:

File ~/venv/pq_dev/lib/python3.11/site-packages/beanie/odm/documents.py:719, in Document.update(self, ignore_revision, session, bulk_writer, skip_actions, skip_sync, *args, **pymongo_kwargs)
    717     if use_revision_id and not ignore_revision and result is None:
    718         raise RevisionIdWasChanged
--> 719     merge_models(self, result)
    720 return self

File ~/venv/pq_dev/lib/python3.11/site-packages/beanie/odm/utils/parsing.py:46, in merge_models(left, right)
     44     left.__setattr__(k, right_value)
     45 elif not isinstance(right_value, Link):
---> 46     left.__setattr__(k, right_value)

File ~/venv/pq_dev/lib/python3.11/site-packages/pydantic/main.py:786, in BaseModel.__setattr__(self, name, value)
    783             self.__pydantic_private__[name] = value
    784     return
--> 786 self._check_frozen(name, value)
    788 attr = getattr(self.__class__, name, None)
    789 if isinstance(attr, property):

File ~/venv/pq_dev/lib/python3.11/site-packages/pydantic/main.py:850, in BaseModel._check_frozen(self, name, value)
    844     return
    845 error: pydantic_core.InitErrorDetails = {
    846     'type': typ,
    847     'loc': (name,),
    848     'input': value,
    849 }
--> 850 raise pydantic_core.ValidationError.from_exception_data(self.__class__.__name__, [error])

ValidationError: 1 validation error for TestFrozen
id
  Instance is frozen [type=frozen_instance, input_value=ObjectId('65d09abca8cf98113d6784ca'), input_type=PydanticObjectId]
    For further information visit https://errors.pydantic.dev/2.6/v/frozen_instance
TheBloke commented 7 months ago

Possible workaround for frozen fields:

I'm still testing this to confirm there's no edge cases, but this might be a usable workaround for me.

I'd still raise the .save() issue as a bug though, because IMHO it should be possible for Beanie to detect frozen fields and not try to update them.

TheBloke commented 7 months ago

Unfortunately I don't think the state_management=True work around works, at least not if links are used.

Because I also get the frozen fields exception when I do something like:


from beanie import Document, Link, WriteRules
from pydantic import Field

class Door(Document):
    height: int = Field(default=2, frozen=True)
    width: int = 1

class House(Document):
    name: str
    door: Link[Door]

door = Door(height=10, width=20)
house = House(name="test", door=door)
await house.insert(link_rule=WriteRules.WRITE)

Even though nothing has changed on Door.height, the act of inserting it with link_rule seems to cause the model to get re-written, triggering the frozen error.

roman-right commented 7 months ago

Hi! Good catch. I'll add a test suite for this and will fix on the next bug-fixing session.

ian-activeloop commented 5 days ago

I'm running into this issue trying to use my_custom_document_instance.set({"not-frozen-field": "new-value"), somehow sending that single field change through either the .set() method is triggering the model's frozen created_by field despite my not having changed the field. I tried switching to the suggested state_management and save_changes, but I still run into the same issue. I see that the .save_changes() method invokes .set() under the hood.

Is that next bug fix session soon?