BeanieODM / beanie

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

alias _id Causing Casting issues with fastapi #130

Closed zrothberg closed 1 year ago

zrothberg commented 3 years ago
class OutTestModel(BaseModel):
    id: Optional[PydanticObjectId]
    name:str

class TestModel(Outtestmodel,Document):
    id:Optional[PydanticObjectId]
    secret_message:str

@Router.get("/failtest",response_model=OutTestModel)
async def wontwork():
  mymodel = TestModel(name="jeff",secret_message="this is gonna break")
  await mymodel.save()
  return mymodel

The above code OutTestModel will not get the id value from TestModel.

I believe the cause is https://github.com/roman-right/beanie/blob/1641dd81be64dd1dc11af667deb2e50feb2de2be/beanie/odm/documents.py#L922-L927

How hard would it be to optionally just use _id for to and from mongo parts? The only other work around is to not us alias for the response model which opens a whole other can of worms.

roman-right commented 3 years ago

Hm, this is weird a little.

The next code:

import asyncio
from typing import Optional

from pydantic import BaseModel
from beanie import PydanticObjectId, Document, init_beanie

class OutTestModel(BaseModel):
    id: Optional[PydanticObjectId]
    name: str

class TestModel(OutTestModel, Document):
    id: Optional[PydanticObjectId]
    secret_message: str

async def main():
    await init_beanie(
        connection_string="YOUR_CONNECTION_STRING",
        document_models=[TestModel])
    mymodel = TestModel(name="jeff", secret_message="this is gonna break")
    await mymodel.save()
    print(OutTestModel.parse_obj(mymodel))

asyncio.run(main())

prints out:

id=ObjectId('61696ef6805c50ca86d286c5') name='jeff'

I'll check with fastapi a little later, why is this breaking there. But from the pydantic side, it should work.

zrothberg commented 3 years ago

So I believe it is more or less under the hood this: OutTestModel.parse_obj(mymodel.dict(alias=True)) Because by default pydantic has allow_population_by_field_name = False. So you need to use the alias to feed back into the next object.

import asyncio
from typing import Optional
from configfile import dburl
from pydantic import BaseModel
from beanie import PydanticObjectId, Document, init_beanie

class OutTestModel(BaseModel):
    id: Optional[PydanticObjectId]
    name: str

class TestModel(OutTestModel, Document):
    id: Optional[PydanticObjectId]
    secret_message: str

async def main():
    await init_beanie(
        connection_string=dburl,
        document_models=[TestModel])
    mymodel = TestModel(name="jeff", secret_message="this is gonna break")
    await mymodel.save()
    print(OutTestModel.parse_obj(mymodel))
    print(OutTestModel.parse_obj(mymodel.dict(by_alias=True)))

asyncio.run(main())

prints

app_1       | id=ObjectId('616979d0be470f49d46fd7cc') name='jeff'
app_1       | id=None name='jeff'
roman-right commented 3 years ago

I see. For now, I don't know, how to fix it beautifully. But this guy is in my TODO now

douyahu commented 2 years ago

fastapi中,我最后是这样解决的 image image image image

zrothberg commented 2 years ago

The issue is casting between types not what you posted.

frankie567 commented 2 years ago

Hey there 👋

I'll bump into this issue since it was reported to me on https://github.com/fastapi-users/fastapi-users/issues/1000.

Reproducible example

The problem indeed occurs when you return a Beanie document and use response_model to "downcast" it to another Pydantic model. Here is a small reproducible example:

from typing import Optional

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

DATABASE_URL = "mongodb://localhost:27017"
client = motor.motor_asyncio.AsyncIOMotorClient(
    DATABASE_URL, uuidRepresentation="standard"
)
db = client["database_name"]

class User(Document):
    email: str
    hashed_password: str

class UserRead(BaseModel):
    id: Optional[PydanticObjectId]
    email: str

app = FastAPI()

@app.get("/user", response_model=UserRead)
async def get_user():
    user = await User.all().first_or_none()
    return user

@app.on_event("startup")
async def on_startup():
    await init_beanie(
        database=db,
        document_models=[
            User,
        ],
    )
    user = User(email="foo@bar.com", hashed_password="abc")
    await user.save()

If you run this and make a request to GET /user, the id is null:

{
  "id": null,
  "email": "foo@bar.com"
}

Root cause in FastAPI

I was able to track down this in FastAPI and I think this behavior is caused by this part:

if isinstance(res, BaseModel):
        read_with_orm_mode = getattr(res.__config__, "read_with_orm_mode", None)
        if read_with_orm_mode:
            # Let from_orm extract the data from this model instead of converting
            # it now to a dict.
            # Otherwise there's no way to extract lazy data that requires attribute
            # access instead of dict iteration, e.g. lazy relationships.
            return res
        return res.dict(
            by_alias=True,
            exclude_unset=exclude_unset,
            exclude_defaults=exclude_defaults,
            exclude_none=exclude_none,
        )

When FastAPI detects that a response value is a Pydantic BaseModel (which a Beanie Document is), it'll first transform it to a dictionary with the .dict method and parameter by_alias to True. This is where we retrieve a _id instead of id.

Workarounds

I found two workarounds for this.

1. Leverage the read_with_orm_mode option

This is something that was implemented specifically for SQLModel, so may be not the best solution. But as you see in the code above, it forces FastAPI to bypass the .dict transformation.

class User(Document):
    email: str
    hashed_password: str

    class Config:
        read_with_orm_mode = True

2. Implement a root_validator to fill id with _id

Quite verbose though and not very intuitive:

class UserRead(BaseModel):
    id: Optional[PydanticObjectId]
    email: str

    @root_validator(pre=True)
    def fill_id(cls, values):
        id = values.get("_id")
        if id is not None:
            values["id"] = id
        return values

3. Use .from_orm to manually build the response object

Probably the cleanest solution:

class UserRead(BaseModel):
    id: Optional[PydanticObjectId]
    email: str

    class Config:
        orm_mode = True

# ...

@app.get("/user")
async def get_user():
    user = await User.all().first_or_none()
    return UserRead.from_orm(user)

All in all, I'm not sure how it could be fixed in Beanie and even if it's an issue that has to be fixed by Beanie. Maybe a warning in the docs explaining this could be enough.

github-actions[bot] commented 1 year ago

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

github-actions[bot] commented 1 year ago

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