BeanieODM / beanie

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

[BUG] RevisionIdWasChanged error when trying to update model #833

Open RamonGiovane opened 5 months ago

RamonGiovane commented 5 months ago

Describe the bug RevisionIdWasChanged error is raised when save_changes is called on an instance created by get.

To Reproduce

import asyncio
from typing import List, Optional

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

class OtherModel(Document):
    bar: str

    class Settings: 
        name = 'other_models'
        use_revision = True
        use_state_management = True
        state_management_save_previous = True
        validate_on_save = True

class FlowEvent(Document):
    content: str
    flow: Link['Flow']

    class Settings:  
        name = 'flow_events'
        use_revision = True
        use_state_management = True
        state_management_save_previous = True
        validate_on_save = True

class Flow(Document):
    data: Optional[str] = None
    other_model: Optional[Link['OtherModel']] = None
    events: List[BackLink['FlowEvent']] = Field(original_field='flow', default=[])

    class Settings: 
        name = 'flows'
        is_root = True
        use_revision = True
        use_state_management = True
        validate_on_save = True

class DoctorFlow(Flow):
    foo: str

    class Settings:
        name = 'doctor_flows'
        use_revision = True
        use_state_management = True
        state_management_save_previous = True
        validate_on_save = True

async def _init_db():
    DB_URI ='mongodb://localhost:27017'
    DB_NAME ='testing-beanie'
    client = AsyncIOMotorClient(DB_URI)
    database = client[DB_NAME]

    # Beanie initialization
    await init_beanie(database, document_models=[OtherModel, FlowEvent, Flow, DoctorFlow])

async def _busines_logic():
    # Inserting one DoctorFlow with a FlowEvent. Relationship one-to-many
    doc_flow = DoctorFlow(foo='foo', data='some_data', other_model=OtherModel(bar='bar'))
    await doc_flow.insert(link_rule=WriteRules.WRITE)
    await FlowEvent(content='content', flow=Link(doc_flow.to_ref(), document_class=Flow)).insert()

    # Retrieving a fresh new instance from the inserted Flow in the DB
    db_flow = await Flow.get(doc_flow.id, fetch_links=True, with_children=True)
    db_flow.data = 'Some change'
    await db_flow.save_changes() # ERROR!!! RevisionIdWasChanged

async def _main():
    await _init_db()
    await _busines_logic()

asyncio.run(_main())

Expected behavior The update operation should work. This is a common use of case: insert data into the database, then the collection is fetched to another instance of the class and the data is updated depending on the business logic.

Additional context I'm afraid that fetch_links or with_children may be affecting the query, though I'm not sure. Version: 1.24.0

mikeckennedy commented 4 months ago

I've been getting crazy unexpected errors like this too. But my code is different:

podcasts = sorted(list(set(user.podcasts.copy() + [podcast.id])))
user.podcasts.clear()
user.podcasts.extend(podcasts)
await user.save()

I started with just await user.save() but that didn't work, so I added ignore_revision=True and still got the revision_id changed error.

podcasts = sorted(list(set(user.podcasts.copy() + [podcast.id])))
user.podcasts.clear()
user.podcasts.extend(podcasts)
await user.save(ignore_revision=True)

So added the save original id, set it back, and STILL get the error.

revision_id = user.revision_id
podcasts = sorted(list(set(user.podcasts + [podcast.id])))
user.podcasts.clear()
user.podcasts.extend(podcasts)
user.revision_id = revision_id
await user.save(ignore_revision=True)

All of this is on top of the model saying no revision tracking.

class User(beanie.Document):
    created_date: datetime.datetime = pydantic.Field(default_factory=datetime.datetime.now)
    ...
    class Settings:
        name = 'users'
        use_revision = False   

The document itself doesn't seem to have a revision_id in the DB at all:

{
    "_id" : "65c456d339bfbd7af9f4b622",
    "created_date" : "2024-02-07T20:21:39.439+0000",
    "email" : "michael@test.com",
    "is_admin" : false,
    "last_login" : "2024-02-07T20:21:39.439+0000",
    "name" : "Michael",
    "password_hash" : "$argon2id$v=19$m=65536,t=3,p=4$fg+hdO6dkjljljkLubfWug$QIUF4JPR/G3R5ElkjlkjlkjOIAZSvzyTwYAjmcdljljlkjg",
    "podcasts" : [
        "talk-python-to-me"
    ]
}

@roman-right What's happening? Any help? :)

roman-right commented 4 months ago

I have to doublecheck, In general RevisionIdWasChanged can be raised, when insert part of upsert is trying to insert an object with already existing id, the native error there is that id already exists. It is a bit problematic to split all the valid cases and corner cases, where the actual duplication id error should be raised. I'll check both your cases. Thank you for the catch.

RamonGiovane commented 1 month ago

I was doing some investigation myself and I found out that revision_id is being lost between calls. More precisely when we call save_changes(), the self object no longer has revision_id:

> /home/ramon/Repos/beanie/error-test.py(73)_busines_logic()
-> await db_flow.save_changes() # ERROR!!! RevisionIdWasChanged
(Pdb) l
 68  
 69         # Retrieving a fresh new instance from the inserted Flow in the DB
 70         db_flow = await Flow.get(doc_flow.id, fetch_links=True, with_children=True)
 71         db_flow.data = 'Some change'
 72         breakpoint()
 73  ->     await db_flow.save_changes() # ERROR!!! RevisionIdWasChanged
 74  
 75     async def _main():
 76         await _init_db()
 77         await _busines_logic()
 78  
(Pdb) db_flow.revision_id
UUID('fef90667-4d0d-4030-b372-001a085f36b1')
> /home/ramon/Repos/beanie/beanie/odm/documents.py(623)save_changes()
-> if not self.is_changed:
(Pdb) l
618             :param ignore_revision: bool - ignore revision id, if revision is turned on
619             :param bulk_writer: "BulkWriter" - Beanie bulk writer
620             :return: None
621             """
622             breakpoint()
623  ->         if not self.is_changed:
624                 return None
625             changes = self.get_changes()
626             if self.get_settings().keep_nulls is False:
627                 return await self.update(
628                     SetOperator(changes),
(Pdb) self.revision_id
(Pdb) 

@roman-right is this a expected behavior?

Tested on 1.26.0.