use org.bson.conversions.Bson instead of org.bson.Document in PanacheMongoEntityBase

[Loki-Afro]

Description

usually one writes filters in like so

org.bson.conversions.Bson filter = com.mongodb.client.model.Filters.eq('someField', "someValue")

for such simple cases this is appropriate:

@MongoEntity(collection = "namespaces")
public class Namespace extends PanacheMongoEntityBase {
    ...
    public Instant activatedUntil;

    public static Optional<Namespace> findByName(String name) {
        return find("name", name).singleResultOptional();
    }

however if i want any more complex stuff i can not put org.bson.conversions.Bson into the find function since its only accepting documents, and org.bson.conversions.Bson dosen't implement that it is rather the opposite.

i think this is an unnecessary barrier because i don't see a reason why one should not be able to call

    public static Set<Namespace> findByActivatedUntilOlderThan(Instant activatedUntil) {
        org.bson.conversions.Bson query = com.mongodb.client.model.Filters..lte("activatedUntil", activatedUntil);
        return find(query); <-- breaks
    }

thank you

Implementation ideas

change occurrences of org.bson.Document to org.bson.conversions.Bson

[quarkus-bot]

/cc @FroMage (panache), @loicmathieu (mongodb,panache)

[loicmathieu]

This is totally legitimate and in fact under the cover we already use org.bson.conversions.Bson internally in some places like for io.quarkus.mongodb.panache.PanacheQuery.

I remember already wanted to do this change but didn't as it's a huge change and I fear for backward compatibility (this is at least not binary compatible).

[gsmet]

Couldn't we keep both methods for a while?

[loicmathieu]

Yes, I'll have a look if there a not too many of them as the interface is already crowded.

[loicmathieu]

@gsmet keeping both method would have the same issue as Java always dispatch to the more precise type parameter so the old method will still be used by existing code using Document and when we remove the old methods it will switch to the new one.

Moreover it would bring unnecessary burden to the user as the only way to force using the new methods instead of the old ones would be to cast!

If I'm not wrong, here is what would happen.

We have this existing method with a Document parameter which override `Bson':

public static <T extends PanacheMongoEntityBase> PanacheQuery<T> find(Document query) {
}

We would add this one and deprecate the old one:

public static <T extends PanacheMongoEntityBase> PanacheQuery<T> find(Bson query) {
}

So now you would be able to use a Bson query which is great:

MyEntity.find(Filters.lte("activatedUntil", activatedUntil))

But existing code will still use he old method except if you cast:

MyEntity.find(new Document().append("category", "category1")); // still use the find(Document query) method
MyEntity.find((Bson) new Document().append("category", "category1")); // cast to force using the ind(Bson query)  method

I'll try to move from Document to Bson and see how it works.

[gsmet]

But my understanding is that using the Document method is not a problem if you have a Document?

Could you clarify why using the old method when you have a Document and the new one for Bson would be an issue?

That being said, we don't have a guarantee of binary compatibility with Quarkus so we can adjust it if it's too cumbersome. We just try to not break people if we can avoid it.
But if we break binary compatibility, we won't backport it in a micro.

[loicmathieu]

It's just that creating new method in Panache has a hight cost as there is already a lot of them (and each implies code generation under the cover) so if we don't need to maintain binary compatibility I would prefer to replace the actual one instead of duplicating them. We're talking about 10 methods in an already crowded class or interface.

I'll open a PR to see what it is about, if you think it's better to duplicate I can do it.

[loicmathieu]

See candidate PR: #42726

[Loki-Afro]

Thanks for adding this change so quickly, didn't expect that,thank you although I didn't go though the code yet 😁👍