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.

Sign up for GitHub

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

Jump to bottom

Ensure that resteasy rest-client uses QuarkusInjectorFactory as well #4234

Merged
merged 1 commit into from
Sep 27, 2019
Merged

Ensure that resteasy rest-client uses QuarkusInjectorFactory as well #4234

merged 1 commit into from
Sep 27, 2019
+73 −41

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Sep 27, 2019 •
edited
Loading

This is done in order to be able to use @Provider annotated ContextResolver classes as beans - same as is the case for RESTEasy server extensions.

Fixes #2773

@geoand geoand requested review from mkouba and gsmet September 27, 2019 06:56
@geoand
Copy link
Contributor Author

geoand commented Sep 27, 2019

The only thing I am a little concerned about is setting QuarkusInjectorFactory.CONTAINER twice (if the rest client extension is in play). I don't think it will ever be a problem but if you think we should add some synchronization, then I can of course do that

gsmet
gsmet requested changes Sep 27, 2019
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

It looks good to me, nice work.

I just had a comment about a comment where you totally changed the meaning of the initial wording, and the new meaning is IMHO incorrect. Can you check?

...ent/src/main/java/io/quarkus/resteasy/common/deployment/ResteasyInjectionReadyBuildItem.java Outdated Show resolved Hide resolved
@geoand
Ensure that resteasy rest-client uses QuarkusInjectorFactory as well
729114b 
This is done in order to have providers as beans - same as is the case for
resteasy server.

Fixes quarkusio#2773
@geoand geoand requested a review from gsmet September 27, 2019 11:11
@gsmet
Copy link
Member

gsmet commented Sep 27, 2019

It's OK for me. @mkouba I let you review the latest changes and approve too?

@geoand geoand mentioned this pull request Sep 27, 2019
Merged
mkouba
mkouba approved these changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

@geoand That's exactly what I meant. Good job!

@mkouba mkouba merged commit e1f7a18 into quarkusio:master Sep 27, 2019
@geoand
Copy link
Contributor Author

geoand commented Sep 27, 2019

Thanks for your help @mkouba and @gsmet !

@geoand geoand deleted the #2773 branch September 27, 2019 13:07
@mkouba mkouba added this to the 0.24.0 milestone Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet approved these changes

@mkouba mkouba mkouba approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

NullPointerException using @Provider in MP Rest Client
3 participants
@geoand @gsmet @mkouba