Avoid creating a Jsonb each time in Mongo Panache

[loicmathieu]

This is part of #4193

I'm not sure it's the best way to do this. Maybe someone from Jsonb / resteasy can give me feedback ?

[geoand]

Not an expert in either, but I do think this is the right thing to do. It is essentially what I have done here: https://github.com/quarkusio/quarkus/pull/3553/files#diff-9a1fe9d42ef5018e9ebc5cda3e45b9f9R22

[gsmet]

But how does it work when we have multiple extensions doing that? Or the user willing to add their own serializers?

I don't think this approach will fly.

Note: this comment isn't about the fix but on the general approach.

[geoand]

I agree with @gsmet that if we don't do something more centrally this will get out of hand since every extension will want to do it's own thing.

We could do the following:

Provide a CDI producer for Jsonb (which I also did in my PR) and just inject the Jsonb bean into the ContextResolver.
We could also provide "hooks" for users to customize the the default jsonb bean instead of replacing it entirely.

If you like, I can create a PR showing what I have in mind

[geoand]

Another thing we should do is align the handling between Jackson and JSON-B in RESTEasy

[gsmet]

Provide a CDI producer for Jsonb (which I also did in my PR) and just inject the Jsonb bean into the ContextResolver.

I agree with that.

We could also provide "hooks" for users to customize the the default jsonb bean instead of replacing it entirely.

I'm thinking of 3 things:

• discover serializers automatically via Jandex (and other things if they make sense)
• have build items for extensions to push serializers
• then I don't know how we can allow the user to really tweak the JsonbConfig because in some cases, they might need to. Should we allow to entirely overwrite the jsonb bean via @DefaultBean (but that could break some extensions or the user would have to figure out what the extensions are doing)

[geoand]

Provide a CDI producer for Jsonb (which I also did in my PR) and just inject the Jsonb bean into the ContextResolver.

I agree with that.

We could also provide "hooks" for users to customize the the default jsonb bean instead of replacing it entirely.

I'm thinking of 2 things:

* discover serializers automatically via Jandex (and other things if they make sense)

Great idea!

* have build items for extensions to push serializers

Makes perfect sense

* then I don't know how we can allow the user to really tweak the JsonbConfig because in some cases, they might need to. Should we allow to entirely overwrite the jsonb bean via `@DefaultBean` (but that could break some extensions or the user would have to figure out what the extensions are doing)

That's a good question. IMHO, users shouldn't be prevented from doing such things if they really know what they are doing.
Ideally they would only configure the Jsonb and not override it. If we make it easy to configure it and make it explicit in the docs that overriding could break things depending on what extensions are being used, then I think it's a good compromise.
WDYT?

[gsmet]

If we make it easy to configure it

yeah, for me, that's the missing piece. Have you something in mind?

[geoand]

If we make it easy to configure it

yeah, for me, that's the missing piece. Have you something in mind?

Yes. I was thinking of having something like the following:

public interface JsonbCustomizer {
   
    void customize(Jsonb jsonb);
}

Users (or extensions for that matter) would then simply add their own JsonbCustomizer beans which woould be applied with something like:

@Singleton
public class JsonbProducer {

     @Produces
     public Jsonb jsonb(List<JsonbCustomizer> customizers) {
         Jsonb jsonb = // create a default;
         for(JsonbCustomizer customizer: customizers) {
            customizer.customize(jsonb);
         }
         return jsonb;
     }
}

Does that sounds reasonable?

I was thinking that this would be in the vanilla json-b module.
The resteasy-jsonb module would then generate the necessary ContextResolver - the reason I think we need this to be generated is that a very advanced user could supply their own...

Ditto for Jackson

[gsmet]

You would pass JsonbConfig not Jsonb to your JsonbCustomizer, right?

Other than that, it looks good. And yes, we would need something similar for Jackson.

[geoand]

You would pass JsonbConfig not Jsonb to your JsonbCustomizer, right?

Ah yes, you're right!

[geoand]

@loicmathieu WDYT? Does the proposed plan sound good to you?

If we all agree I can cook something up (probably tomorrow)

[gsmet]

BTW, I think we should merge this PR anyway. It fixes a bad behavior and, this way, we can let the other solution mature.

[geoand]

BTW, I think we should merge this PR anyway. It fixes a bad behavior and, this way, we can let the other solution mature.

Makes sense. If we go ahead with the other solution, I'll make sure to update this as well

[loicmathieu]

@gsmet @geoand there is a task on my TODO list #4193 to implements a better solution for providing Serializer/Deserializer for both JsonB and Jackson.

My first idea was to provide a CDI bean (like it is done on Jackson) that can be overriden by a user ... but this is not very convenient as what we want is to provides a default bean that the user can use to add more configuration and not replace, because if a user replace it, he will need to register the serializer that we register on the default one ...

Anyway, @geoand if you have some ideas around this please go ahead and keep me posted.

[geoand]

@loicmathieu the solution I proposed above should take care of the customization vs replacement problem.

I'll go ahead and start something soon, then we can discuss on something concrete.