BeanieODM / beanie

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

[BUG] Beanie projection and Pydantic Schema do not play well together #892

Open sheoak opened 4 months ago

sheoak commented 4 months ago

Describe the bug Beanie projections are expecting an "_id" field, but Pydantic schema expect "id". This makes it impossible to use the same Schema and create duplicated code (unless I’m missing the proper method to do it)

To Reproduce

from motor.motor_asyncio import AsyncIOMotorClient
from pydantic import BaseModel, Field

from beanie import Document, init_beanie, PydanticObjectId

class Author(Document, BaseModel):
    name: str

class AuthorRead(BaseModel):
    id: PydanticObjectId = Field(alias="id")
    name: str

class AuthorProjection(BaseModel):
    # note the underscore
    id: PydanticObjectId = Field(alias="_id")
    name: str

async def example():
    client = AsyncIOMotorClient("mongodb://localhost:27017")
    await init_beanie(database=client.db_name, document_models=[Author])

    dict = { "name": "Joe" }
    joe = Author(**dict)
    await joe.insert()

    # created object contains "id"
    print(AuthorRead(**joe.dict()))

    # Beanie get, also give us an 'id' field, so AuthorRead expect id too
    # (get() method does not have a project() method)
    result = await Author.get(joe.id)
    print(AuthorRead(**joe.dict()))

    # projection is expecting "_id", not "id"
    # we cannot use the same Schema!
    result = await Author.find_all().project(AuthorProjection).to_list()
    print(result)

await example()

Expected behavior A way to use the same Schema for projections, like mapping _id to id during projection

sheoak commented 4 months ago

To give more context, here is my dirty fix on fastapi. I’m really not satisfied with that but that’s all I could find:

@app.get("/hack", response_model=list[AuthorRead])
async def hack() -> list[Any]:
    # using a new Schema just for the hack. AuthorRead should be usable
    return await Author.find_all().project(AuthorProjection).to_list()
CartfordSam commented 4 months ago

I can post a gist in a bit, but I use a base class that overrides the BaseModel and handles the _id conversion there, both directions. Otherwise lots of ways to tackle this, most are so-so, but more of a pydantic quirk than anything else I think!

sheoak commented 3 months ago

@CartfordSam That would be nice thank you! I think it should be included in Beanie nevertheless, it complexify things for no reason.

valentinoli commented 3 months ago

In FastAPI you can use response_model_by_alias=False to declare you want the response model to not use the alias.

That way, you don't even need a projection if the only purpose for it is to ensure you have id and not _id. So your FastAPI path operation can be written like:

@app.get("/hack", response_model=list[Author], response_model_by_alias=False)
async def hack() -> list[Any]:
    return await Author.find_all().to_list()
sheoak commented 3 months ago
response_model_by_alias=False

Thanks, I misunderstood what it was doing, it seems to work indeed. Then i suppose using the format that works for projection is suitable. I feel that this would be a nice thing to add in the documentation maybe?

CartfordSam commented 3 months ago

@CartfordSam That would be nice thank you! I think it should be included in Beanie nevertheless, it complexify things for no reason.

yeah complexify is right lol response_model_by_alias is probably the right way to go, especially if you can set it at the router level, here's my janky workaround YMMV