Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix providers not auto registered for JAX-RS Client #11448

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Aug 18, 2020

Fixes #11424

@kenfinnigan
Copy link
Member

Happened to see this, and I'm not sure how this fix works.

A change has been made in resteasy-server-common, but that artifact isn't used by rest-client. From the issue it seemed this problem occurred with REST Client when the server isn't present, or am I mistaken?

@radcortez
Copy link
Member Author

Hi @kenfinnigan

Common is used here:

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common</artifactId>
</dependency>

The issue only occurs with the programmatic REST Client ClientBuilder.getClient() and when there is not another class annotated with @Path in the application. This prevents the RESTEasy server to start and annotated Providers are not auto registered with the programmatic client.

@radcortez
Copy link
Member Author

The proposal is to use the same solution that was used in Quartz:
https://quarkus.io/guides/quartz#quarkus-quartz_quarkus.quartz.force-start

To have a config that force starts the server.

The programmatic REST Client does work without the RESTEasy server, is just doesn't auto register scanned metadata like Providers. Another possible solution would be to auto start the server if we detect user Provider classes, but it doesn't mean that the programmatic REST Client is going to be used so it may be a waste.

@kenfinnigan
Copy link
Member

@radcortez Yes resteasy-common is used by REST Client, but the change was made in resteasy-server-common, which is a different dependency.

That's why I wasn't sure if the change actually worked when just using REST Client with no JAX-RS endpoints

@radcortez
Copy link
Member Author

Hum, maybe you are right. The dependency is there, but as test in the extension. Let me try it out with a real project.

@radcortez
Copy link
Member Author

radcortez commented Aug 19, 2020

@jaikiran jaikiran requested a review from kenfinnigan August 20, 2020 04:29
@@ -69,6 +69,7 @@ public InjectorFactory getInjectorFactory() {
}

RestClientBuilderImpl.setProviderFactory(clientProviderFactory);
ResteasyProviderFactory.setInstance(clientProviderFactory);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't conflict with the server behavior when both are used? /cc @kenfinnigan

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. When the server starts (if any) it will replace the instance:

https://github.com/resteasy/Resteasy/blob/1570f6825de1fde1ea3c3a950dcb6297e3c9bd4f/resteasy-core/src/main/java/org/jboss/resteasy/core/ResteasyDeploymentImpl.java#L377

And it will work for both server and client with the right factory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the ResteasyDeploymentImpl used in Quarkus, for some reason I'm thinking it isn't, but could easily be wrong?

If we do use it, then I think it should work fine. But certainly worth verifying

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Famous Last Words™

@gsmet gsmet added this to the 1.8.0 - master milestone Aug 26, 2020
@gsmet
Copy link
Member

gsmet commented Aug 26, 2020

@kenfinnigan could you approve it if everything is good for you? Thanks!

@gsmet gsmet changed the title Fix for #11424. Providers not auto registered for JAX-RS Client. Fix providers not auto registered for JAX-RS Client Aug 27, 2020
@gsmet gsmet merged commit 6de3201 into quarkusio:master Aug 27, 2020
@radcortez radcortez deleted the issue-11424 branch January 4, 2021 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JAX-RS Providers in RESTEasy are not initialized unless a @Path is present in the Application
4 participants