[Resteasy-2285] Use LocalResteasyProviderFactory instead of ResteasyProviderFactoryImpl

[NicoNes]

No description provided.

[asoldano]

I'm not convinced about this. By calling withConfig, you're effectively causing new allocations, as the contents from the provided providerFactory has to be copied into a new provider factory. Yes, that would now be a LocalResteasyProviderFactory, but still stuff would be copied, while with the current code, the provided object is directly linked.

[NicoNes]

Hey @asoldano,

You're right few improvments can be done.

Actually my point about calling withConfig was to prevent following case:

      ResteasyClientBuilder resteasyClientBuilder=new ResteasyClientBuilderImpl();
      resteasyClientBuilder.providerFactory(new ResteasyProviderFactoryImpl());
      resteasyClientBuilder.register(MyFeature.class); 

Because of usage of ResteasyClientBuilderImpl two intances of MyFeature.class will be created: one by ClientHelper and the other by ServerHelper.
In a standalone client the instance created by ServerHelper is useless.
So initially I wanted to prevent this creation. Then now I realized it's up to the user to call resteasyClientBuilder.providerFactory() with the most relevant ResteasyProviderFactory subtype. In a standalone client it will be the LocalResteasyProviderFactory and in other cases the ResteasyProviderFactoryImpl.

I'll do the fix 👍

Thanks

[asoldano]

This is fine, nice catch :-)

[asoldano]

Hi @NicoNes , the proposed changes are basically fine now, thanks. The only concern I have is with the position of the new ResteasyProviderFactoryDelegate class. Is it really needed in the resteasy-core-spi? Are we expecting users to really rely on that? Perhaps it could live in resteasy-core, in the same package of the ResteasyProviderFactoryImpl class.

[NicoNes]

Hi @asoldano ,

I did not pay attention to that but you're right there is no good reason for this class to be in resteasy-core-spi. I moved it into the package you said.
Since registerBuiltin() is protected, the delegate cannot invoke resteasyProviderFactoryDelegator.registerBuiltin(). So I just throw an UnsupportedOperationException for this method. Let me know if you think it is acceptable.

Thanks

[asoldano]

@NicoNes , I've thought about this... and I believe at then of the day it's acceptable, yes.