Use Bson and not Document in MongoDB with Panache methods #42726
Conversation
|
As discussed here I prefere to replace existing method parameters instead of adding overloads as there is already a lot of overloads for these classes and interfaces that already have a lot of methods. This implies non-binary compatibility: code needs to be recompiled to work properly. |
gsmet
changed the title
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| protected PanacheUpdate createUpdate(MongoCollection collection, Class<?> entityClass, Document docUpdate) { | ||
| protected PanacheUpdate createUpdate(MongoCollection collection, Class<?> entityClass, Bson docUpdate) { |
There was a problem hiding this comment.
Does it make sense to use Bson here? I would expect the docUpdate to be a Document? But I'm not familiar with the Bson API.
There was a problem hiding this comment.
Yes, under the cover it uses PanacheUpdateImpl that was already using Bson in its signature. Always better to use interfaces instead of implementation type.
There was a problem hiding this comment.
Well, in this case, the interface is low level and the Document has a real semantic (it's a Document, not some random Bson object).
Anyway, I won't oppose to it, I was just asking.
| @@ -155,12 +156,12 @@ protected Stream<?> stream(Object queryType) { | |||
| private static class CustomReactiveMongoOperations extends ReactiveMongoOperations<Object, PanacheUpdate> { | |||
|
|
|||
| @Override | |||
| protected Object createQuery(ReactiveMongoCollection collection, Document query, Document sortDoc) { | |||
| protected Object createQuery(ReactiveMongoCollection collection, Bson query, Bson sortDoc) { | |||
There was a problem hiding this comment.
So for sort, it also makes sense to have Bson?
There was a problem hiding this comment.
Same as for document update, under the cover it uses already Bson.
Moreover, there is a com.mongodb.client.model.Sorts helper that return the Bson interface.
So you can for ex do Sorts.ascending("fileName").
|
@gsmet a bit of backgroud, the I don't remember if we started MongoDB with Panache prior to 4.2 or not, but I was not aware of this interface at this time, whether it was not yet created or it was brand new I don't know. It's been a long time since I wanted to switch to exposing the interface as it unites 2 types, and there are helpers inside the driver that use it. So now that we are doing it, I think it's time to use it in all exposed methods of our API. By the way, we use both classes inside the implementation, so it's a clear sign that we should use the |
Status for workflow
|
Fixes #42658