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

Microprofile rest client - Can't inject config/beans to providers #5752

Closed
marcinczeczko opened this issue Nov 25, 2019 · 22 comments · Fixed by #6283
Closed

Microprofile rest client - Can't inject config/beans to providers #5752

marcinczeczko opened this issue Nov 25, 2019 · 22 comments · Fixed by #6283
Assignees
Labels
area/rest-client kind/bug Something isn't working
Milestone

Comments

@marcinczeczko
Copy link
Contributor

marcinczeczko commented Nov 25, 2019

Describe the bug
Any provider added to rest client via @Provider annotation is unable to inject config bean or any other application scoped bean.

@RegisterRestClient
@RegisterProvider(RequestFilter.class)
public interface EchoService {....}

And example the provider impl:

public class RequestFilter implements ClientRequestFilter {
  @Inject
  AppConfig config;  //is always null

  @Inject
  SomeBean bean; //is always null

  @Override
  public void filter(ClientRequestContext requestContext) throws IOException {...}

Expected behavior
Config or application scope bean injected and not null

Actual behavior
Any bean injected via @Inject is alway null (no errors in logs during argumentation nor during runtime.

To Reproduce
Steps to reproduce the behavior:

  1. Use reproducer: https://github.com/marcinczeczko/quarkus-rest-providers-reproducer
  2. mvn compile quarkus:dev
  3. curl http://localhost:8080/echo
  4. Check the logs - the injected config & app bean shows null reference

Configuration

  • See reproducer

Screenshots
N/A

Environment (please complete the following information):

  • Output of uname -a or ver: 18.7.0 Darwin Kernel Version 18.7.0: Sat Oct 12 00:02:19 PDT 2019; root:xnu-4903.278.12~1/RELEASE_X86_64 x86_64
  • Output of java -version:
java version "1.8.0_192"
Java(TM) SE Runtime Environment (build 1.8.0_192-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.192-b12, mixed mode)
  • GraalVM version (if different from Java): Didn't check on graalvm
  • Quarkus version or git rev: 1.0.0.Final

Additional context

I found out there is a closed issue for similar problem, but looks like the fix doesn't work, see: #2773

@marcinczeczko marcinczeczko added the kind/bug Something isn't working label Nov 25, 2019
@mkouba mkouba self-assigned this Nov 26, 2019
@mkouba
Copy link
Contributor

mkouba commented Nov 26, 2019

Ok, so an easy workaround is to annotate the provider class with @Provider but this would probably defeat the purpose of the @RegisterProvider annotation because the provider would be discovered and automatically registered for all rest client interfaces and even server-side resources.

Looking at the RestClientBuilderImpl it seems that the creation of providers registered through @RegisterProvider is hard-coded and is using Class.newInstance(). See https://github.com/resteasy/Resteasy/blob/master/resteasy-client-microprofile/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java#L205-L207 and https://github.com/resteasy/Resteasy/blob/master/resteasy-client-microprofile/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java#L452-L458.

I believe that this should be changed so that the provided ResteasyProviderFactory is used instead. @geoand @kenfinnigan WDYT?

@geoand
Copy link
Contributor

geoand commented Nov 26, 2019

I believe that this should be changed so that the provided ResteasyProviderFactory is used instead. @geoand @kenfinnigan WDYT?

+1 for me

@mkouba
Copy link
Contributor

mkouba commented Nov 26, 2019

@geoand would you care to send a PR to resteasy? ;-)

@geoand
Copy link
Contributor

geoand commented Nov 26, 2019

@mkouba sure, I'll most likely get to it this week

@marcinczeczko
Copy link
Contributor Author

marcinczeczko commented Nov 26, 2019

@mkouba @geoand By the way, I updated my reproducer to verify @RegisterClientHeaders to add ClientHeadersFactory to the client - and in fact it has the same issue with no beans injected.

@gsmet
Copy link
Member

gsmet commented Nov 26, 2019

@geoand it looks like something we would want for 1.1.0 so please ask for a RESTEasy release including that one before December 6th.

Thanks!

@geoand
Copy link
Contributor

geoand commented Nov 26, 2019

@gsmet Sure!

Once I finish my current task, I'll pick up this one.

@geoand
Copy link
Contributor

geoand commented Nov 27, 2019

I opened a PR in RESTEasy that forms part of the solution.
The other part will be to register the classes used in @RegisterProvider as beans.

@geoand geoand assigned geoand and unassigned mkouba Nov 27, 2019
@mkouba
Copy link
Contributor

mkouba commented Nov 27, 2019

@geoand Thanks!

@geoand
Copy link
Contributor

geoand commented Nov 27, 2019

@mkouba YW!

@geoand
Copy link
Contributor

geoand commented Dec 12, 2019

@asoldano when do you expect RESTEasy 4.5.0.Final will be released?
@gsmet if it's out soon enough, do you think we can have it for Quarkus 1.1.0.Final or are we too close to release?

@gsmet
Copy link
Member

gsmet commented Dec 12, 2019

I'm not yet sure of when we will be able to release given all the problems we have.

But I'm not sure either if RESTEasy 4.5.0.Final will be an easy upgrade.

So I would say: release it, prepare a PR and we'll see how it goes :).

@asoldano
Copy link
Contributor

@geoand , @gsmet , the current plan is to release another micro from resteasy master, so 4.4.2.Final, by the end of this week.

@gsmet
Copy link
Member

gsmet commented Dec 16, 2019

Ok, that one will be for 1.2 then.

@geoand
Copy link
Contributor

geoand commented Dec 16, 2019

Or if we need a 1.1.1 for some other reason, we can get this fix in as well

@asoldano
Copy link
Contributor

4.4.2.Final is released

@geoand
Copy link
Contributor

geoand commented Dec 20, 2019

@asoldano Thanks!

Do you plan to create a Quarkus PR to bump the RESTEasy version?

@geoand
Copy link
Contributor

geoand commented Dec 20, 2019

PR bumping the version is here: #6283

@rsvoboda
Copy link
Member

RESTEasy update PR was merged 10 days ago, is this issue fixed now ?

@geoand
Copy link
Contributor

geoand commented Jan 15, 2020

@rsvoboda yes, I'll close it.

@geoand geoand closed this as completed Jan 15, 2020
@gsmet gsmet added this to the 1.2.0 milestone Jan 20, 2020
@sassko
Copy link

sassko commented Mar 3, 2020

From my perspective this issue is still present. Even if I upgrade the project from here to 1.2.1.Final all injected beans are still "null"

@gsmet
Copy link
Member

gsmet commented Mar 3, 2020

@sassko can you open a new issue with your reproducer linked? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest-client kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants