quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Querying with id instead of _id in MongoDB with Panache

Open vapostolopoulos opened this issue 2 years ago • 7 comments

Description

Suppose we have this very simple class as a Mongo entity with a custom String as an id

@MongoEntity
public class Person {

   private String id;
   private String name;

   // constructors & getters & setters 

When querying we have to use _id as it is stored in Mongo like: find("_id", id) when it should make sense to use find("id", id)

Implementation ideas

No response

vapostolopoulos avatar Jun 15 '22 09:06 vapostolopoulos

/cc @FroMage, @evanchooly, @loicmathieu

quarkus-bot[bot] avatar Jun 15 '22 09:06 quarkus-bot[bot]

I think I can create a PR trying to improve this one! What do you guys think?

I would check if the object has the field id or _id and pass it forward during the query. What do you think? @loicmathieu

JohnneSouza avatar Jun 21 '22 19:06 JohnneSouza

@JohnneSouza the PR should be even easier ;) We already have a mechanism to replace field name in case @BsonProperty is used, we can add an hardcoded replacement for id -> _id. I don't think it's possible/supported by the POJOCodec to have both id and _id in a collection/POJO.

loicmathieu avatar Jun 27 '22 10:06 loicmathieu

@JohnneSouza do you still plan to provide a PR for this ?

loicmathieu avatar Jul 21 '22 13:07 loicmathieu

Hey @loicmathieu sorry, I totally forgot about this thread haha! I'll take a look on it today or tomorrow and push this stuff to the repo ASAP! Tks for the reminder :~D

JohnneSouza avatar Jul 21 '22 18:07 JohnneSouza

As always, I'm really forgetting things... but I couldn't help to notice that MongoDB by default will create a new document with the "_id" and I'm think if it wouldn't be a little counter intuitive querying with "id" instead of "_id"; Another thing is: If you guys think that is OK to change from _id to id, Not sure that only changing here would be enough. @loicmathieu @vapostolopoulos

JohnneSouza avatar Aug 04 '22 00:08 JohnneSouza

@JohnneSouza I was thinking about this (and an other issue with querying with attributes annotated by @BsonProperty. MongoDB PojoCodec is only used to serialize/deserialize a Pojo to/from a collection, at query time MongoDB only takes fields names.

So if id is used instead of _id, or if @BsonProperty is used, even if it means POJO attributes and collection fields will not be the same it has no effect on the query that must use collection field names.

So I'm not sure allowing someone to use id instead of _id is a good idea, we should mandates the usage of the field (MongoDB with Panache is not an ORM). I need to check what's our current status around this.

loicmathieu avatar Aug 08 '22 07:08 loicmathieu

I'm unclear what that last comment is getting at but, IMO, we should require the use of the mapped names in queries. Replacing java field names with @BsonProperty mapped names gets really tricky without the proper modeling and panache is a bit simplistic in that respect. My vote is to require the mapped names including _id.

evanchooly avatar Aug 15 '22 16:08 evanchooly

@evanchooly yes, this is more or less what I'm thinking of, what you named mapped names I named them collection field names.

I should update the documentation guide and the JavaDoc to clearly states that when querying, the collection field names should always be used even if @BsonProperty is used to change the name as no remapping is done, including _id. (I'll come with a better sentence explaining that)

loicmathieu avatar Aug 16 '22 10:08 loicmathieu