BeanieODM / beanie

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

[BUG] When doing Annotated[str, Indexed(str, unique=True)] the uniqueness is not enforced #1036

Open guyzmo opened 1 month ago

guyzmo commented 1 month ago

Describe the bug

As described in this issue And discussed in this issue And implemented in this PR And according to beanie's documentation as well as pymongo's documentation

we can declare indexes with the following syntax:

class User
    user_id: Annotated[str, Indexed(str)]

To Reproduce

But if we want to declare an index with the uniqueness property, it is not enforced:

class User
    user_id: Annotated[str, Indexed(str, unique=True)]

user_1 = User(user_id: "foo").create()
user_2 = User(user_id: user_1.id).create()

assert len(User.find_all(User.user_id == user_1.user_id)) == 2

Expected behavior

There should be only one user with a given user_id, not two:

class User
    user_id: Annotated[str, Indexed(str, unique=True)]

user_1 = User(user_id: "foo").create()
user_2 = User(user_id: user_1.id).create()

assert len(User.find_all(User.user_id == user_1.id)) == 1

Additional context

I'm still running an older version of beanie (v1.11.6), but it looks like this issue is still actual. Cf discord.

guyzmo commented 1 month ago

It looks like this issue has been fixed in the PR #762. Found it thanks to the community on discord šŸ«¶

Though, I'm not up to date, so I'm upgrading and come back here to tell whether it fixed my issue or not. I hope it does šŸ¤ž

MrEarle commented 1 month ago

When used with Annotated, you shouldn't provide the type to the Indexed function. Try this instead:

class User
    user_id: Annotated[str, Indexed(unique=True)]
staticxterm commented 4 weeks ago

Hi @MrEarle, thank you for the explanation. I just looked at the code as was having a hard time to understand why we can't also provide the type in this case also, but I see now that this is by design. I believe this ticket should still stay open as at least we should clarify this usage in the Indexes docs, to avoid any future confusion.

guyzmo commented 1 week ago

ok, I did upgrade to latest, and that did solve the issue, so yes:

thank you šŸ™

staticxterm commented 1 week ago

Hi @guyzmo, glad to see the issue resolved for you and thank you for opening this ticket.

However, I would like to leave this ticket as open until we make a note about this in our documentation.