[BUG] RevisionIdWasChanged is always raised when updating through FastAPI put route

[Ty-Ni]

Describe the bug
I am trying to build a simple CRUD application using FastAPI and beanie, with the setting "use_revision" enabled on the model that I am using for this app. However, it seems that I am unable to update items in the database as the RevisionIdWasChanged error is always raised on calling .save().

To Reproduce

import uvicorn
from beanie import Document, init_beanie, PydanticObjectId
from fastapi import FastAPI
from motor.motor_asyncio import AsyncIOMotorClient


class Foo(Document):
    class Settings:
        use_revision = True
        name = "foos"

    bar: str


app: FastAPI = FastAPI()


@app.post("/create")
async def create() -> PydanticObjectId:
    foo = Foo(bar="bar")
    result = await foo.insert()
    return result.id


@app.put("/update")
async def update(foo: Foo) -> None:
    result = await foo.save()  # <- this always throws RevisionIdWasChanged
    return None


@app.on_event("startup")
async def startup():
    app.mongodb_client = AsyncIOMotorClient("mongodb://mongo-0:27117,mongo-1:27118,mongo-2:27119/?replicaSet=replica-set")
    app.mongodb = app.mongodb_client["foos"]

    await init_beanie(database=app.mongodb, document_models=[Foo])


if __name__ == "__main__":
    uvicorn.run(
        "main:app",
        host="127.0.0.1",
        port=8000,
        reload=True
    )

Using the above server, follow these steps:

1. Perform a POST request to the create endpoint
2. Perform a PUT request to the update endpoint using the ID returned in step 1.
3. The RevisionIdWasChanged error will be raised in the server.

The body used for step 2 is the example body provided by the doc generation of FastAPI ("localhost:8000/docs"):

{
  "_id": "651163927129d9177247c1b7",
  "bar": "string"
}

---

As a separate question; I would expect version_id to be part of the Model that is used for doc generation, but the field is marked as hidden. How are we supposed to check if a document has changed since it was retrieved, if the user does not send the revision_id for the object it was editing?

Even with the following body, the request still fails with RevisionIdWasChanged:

{
  "_id": "651163927129d9177247c1b7",
  "revision_id": "69a4b65b-83a8-4874-a129-b237cc51d11b",
  "bar": "string"
}

where _id and revision_id were copied directly from the database.

---

Expected behavior
I would expect to be able to call the save method on an object that has not been changed since retrieving it. Furthermore, I would expect revision_id to be part of the expected body type generated when using the use_revision = True statement. Lastly, I would expect foo.save() to create a document with a filled in revision_id field: however, it seems to create document without setting that field (I need to use foo.insert() to actually generate a revision_id.

Additional context
This is using beanie with version 1.22.6, fastAPI version 0.103.1, mongodb version 7.

I recognise that I may be using this use_revision parameter in the wrong way, but the documentation on it is very sparse and my interpretation seems intuitive for a CRUD application.

[iterlace]

This appears to be an extension of #707

[roman-right]

Hi! Thank you for the catch. I'll check and fix it this/next week.

[hd10180]

Using the above server, follow these steps:

1. Perform a POST request to the create endpoint
2. Perform a PUT request to the update endpoint using the ID returned in step 1.
3. The RevisionIdWasChanged error will be raised in the server.

after reproduce i found the problem cause by step 2 (i got the same error yesterday)
you pass the id from step 1, the object in endpoint update will generate an new object with the same id, then you call save() cause the error,
we could find_one by id first, and then use new_obj = obj.model_copy(update=foo.model_dump(exclude={"id"})) to update model,
finally call new_obj.save()

[Ty-Ni]

Using the above server, follow these steps:

1. Perform a POST request to the create endpoint
2. Perform a PUT request to the update endpoint using the ID returned in step 1.
3. The RevisionIdWasChanged error will be raised in the server.

after reproduce i found the problem cause by step 2 (i got the same error yesterday) you pass the id from step 1, the object in endpoint update will generate an new object with the same id, then you call save() cause the error, we could find_one by id first, and then use new_obj = obj.model_copy(update=foo.model_dump(exclude={"id"})) to update model, finally call new_obj.save()

Doing it like this would not provide any guarantees that your data wasn't changed since you retrieved it, however. If you always get a fresh revision_id when updating an object, you might as well not use the revision_id. The way it should work is that you supply the revision_id that belongs to your version of the object, and the object itself, to your update endpoint. The save method will then compare the revisions between your supplied one, and the one currently stored in the database, to determine whether or not the object has already been changed since you last retrieved it.

[hd10180]

Using the above server, follow these steps:

1. Perform a POST request to the create endpoint
2. Perform a PUT request to the update endpoint using the ID returned in step 1.
3. The RevisionIdWasChanged error will be raised in the server.

after reproduce i found the problem cause by step 2 (i got the same error yesterday) you pass the id from step 1, the object in endpoint update will generate an new object with the same id, then you call save() cause the error, we could find_one by id first, and then use new_obj = obj.model_copy(update=foo.model_dump(exclude={"id"})) to update model, finally call new_obj.save()

Doing it like this would not provide any guarantees that your data wasn't changed since you retrieved it, however. If you always get a fresh revision_id when updating an object, you might as well not use the revision_id. The way it should work is that you supply the revision_id that belongs to your version of the object, and the object itself, to your update endpoint. The save method will then compare the revisions between your supplied one, and the one currently stored in the database, to determine whether or not the object has already been changed since you last retrieved it.

Thanks for the clarification. Sorry, I must have misunderstood your question.

[Sylver11]

Hi just to pick up on this. Not sure if this makes a difference but we recently migrated to Pydantic V2.

We are experiencing a somewhat similar issue. Though in our case this is being thrown after a pymongo DuplicateKeyError has been raised. Here small example:

try:
     await m.update(
         {
             "$set": update.model_dump(
                 exclude={"id", "_id"} # this does not solve the problem
             )
         },
        upsert=False,
    )
except DuplicateKeyError as e:
    raise ManufacturersDuplicateKeyException(
        description=str(e.details)
    ) from e

We purposefully created a duplicate key error but instead of just throwing the duplicate key error it says the following:

raise DuplicateKeyError(errmsg, code, response, max_wire_version)
...
During handling of the above exception, another exception occurred:
...
raise RevisionIdWasChanged

Resulting in a 500 error instead of a duplicate key error.

[axelmukwena]

Hi just to pick up on this. Not sure if this makes a difference but we recently migrated to Pydantic V2.

We are experiencing a somewhat similar issue. Though in our case this is being thrown after a pymongo DuplicateKeyError has been raised. Here small example:
...

We purposefully created a duplicate key error but instead of just throwing the duplicate key error it says the following:
...
Resulting in a 500 error instead of a duplicate key error.

I the issue highlighted by @Sylver11 seems to be caused by this block of code https://github.com/roman-right/beanie/blob/main/beanie/odm/documents.py#L665

try:
    result = await self.find_one(find_query).update(
        *arguments,
        session=session,
        response_type=UpdateResponse.NEW_DOCUMENT,
        bulk_writer=bulk_writer,
        **pymongo_kwargs,
    )
except DuplicateKeyError:
    raise RevisionIdWasChanged
if bulk_writer is None:
    if use_revision_id and not ignore_revision and result is None:
        raise RevisionIdWasChanged
    merge_models(self, result)

@roman-right It appears that RevisionIdWasChanged is being raised immediately following a DuplicateKeyError, which may not be the intended behavior in this context. This is confusing since a DuplicateKeyError typically signifies a unique constraint violation and shouldn't necessarily be only linked to a revision ID change.

[roman-right]

Hi! Sorry for the late reply.
The problem in the original example is that the revision id was not provided.
It is an interesting situation, as I was asked many times to not provide revision_id and in response body and in the api doc of the endpoint.

I'll think how I can implement both. Maybe with an additional option in the Settings inner class.

BTW for now you can try this approach to return revisions:

from uuid import UUID

import uvicorn
from pydantic import BaseModel

from beanie import Document, init_beanie, PydanticObjectId
from fastapi import FastAPI
from motor.motor_asyncio import AsyncIOMotorClient


class Foo(Document):
    class Settings:
        use_revision = True
        name = "foos"

    bar: str


class ResponseModel(BaseModel):
    id: PydanticObjectId
    bar: str
    revision_id: UUID


app: FastAPI = FastAPI()


@app.post("/create")
async def create() -> ResponseModel:
    foo = Foo(bar="bar")
    result = await foo.insert()
    return result


@app.put("/update")
async def update(foo: Foo) -> None:
    result = await foo.save()  # <- this always throws RevisionIdWasChanged
    return None


@app.on_event("startup")
async def startup():
    app.mongodb_client = AsyncIOMotorClient("mongodb://beanie:beanie@localhost:27017/")
    app.mongodb = app.mongodb_client["foos"]

    await init_beanie(database=app.mongodb, document_models=[Foo])


if __name__ == "__main__":
    uvicorn.run(
        app,
        host="127.0.0.1",
        port=8000,
    )

If you still face revision id problem, please try this PR: #797

[Ty-Ni]

Hi,

The snippet you provided gives me the same error when I try to create and then edit an object. This occurs both when I omit and when I provide the revision_id in the JSON body pf the update request.

On the plus side, I am at least able to retrieve the revision_id from the GET request now, so that at least is progress.

I will have a look at the PR and see if I can get it to work with those changes.

Edit:

It is an interesting situation, as I was asked many times to not provide revision_id and in response body and in the api doc of the endpoint.

I would consider it as something you should return if the use_revision flag is set to True, where anyone can still optionally remove it from the server output by excluding the field, but that's just my 2 cents.

[Ty-Ni]

Apologies. After updating Beanie to the most recent version, I automatically receive the revision_id in the response as requested above. If I send that revision_id to the update endpoint, the update works. The issue is now resolved for me, thank you.

[Ty-Ni]

After some more testing, the issue reappeared for me. I must've already applied some changes from your PR when I mentioned that it was working as expected last week. Just wanted to mention that the changes in that PR are what did it, not the update to 1.23.6.

Do you have sight on when you will release a version with the PR merged?

[roman-right]

Hi! It will be published today.