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

ObjectMapperCustomizer not working with JAX-RS client #12104

Closed
knutwannheden opened this issue Sep 15, 2020 · 14 comments
Closed

ObjectMapperCustomizer not working with JAX-RS client #12104

knutwannheden opened this issue Sep 15, 2020 · 14 comments
Labels
kind/bug Something isn't working triage/out-of-date This issue/PR is no longer valid or relevant

Comments

@knutwannheden
Copy link
Contributor

Describe the bug
When using the JAX-RS client obtained through JAX-RS' ClientBuilder (e.g. ClientBuilder.newClient()) any custom ObjectMapperCustomizers are not respected as they are with the MicroProfile REST client.

Expected behavior
The ObjectMapper used by a JAX-RS client constructed using ClientBuilder.newClient() should also have been customized by any registered ObjectMapperCustomizers.

Actual behavior
For a reproducible example see https://github.com/knutwannheden/quarkus-bug-jaxrs-object-mapper, where the ObjectMapper is customized by calling objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES). The repo contains a test case demonstrating that this works as expected for MicroProfile REST client but fails with the JAX-RS client.

To Reproduce
Steps to reproduce the behavior:

  1. git clone https://github.com/knutwannheden/quarkus-bug-jaxrs-object-mapper
  2. cd quarkus-bug-jaxrs-object-mapper/
  3. mvn clean verify

Environment (please complete the following information):

  • Output of uname -a or ver: MSYS_NT-10.0-19042 DESKTOP-HR8N4ES 3.1.4-340.x86_64 2020-05-19 12:55 UTC x86_64 Msys
  • Output of java -version: openjdk version "11.0.7" 2020-04-14 LTS
  • GraalVM version (if different from Java):
  • Quarkus version or git rev: 1.8.0.CR1
  • Build tool (ie. output of mvnw --version or gradlew --version): Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
@knutwannheden knutwannheden added the kind/bug Something isn't working label Sep 15, 2020
@geoand
Copy link
Contributor

geoand commented Sep 15, 2020

This is certainly reasonable, although I don't think we can do much here. My guess is that the JAX-RS when obtaining ObjectMapper does not obtain it from CDI.

@gsmet
Copy link
Member

gsmet commented Sep 15, 2020

Well, we should at least check if we can do something about it :).

@knutwannheden
Copy link
Contributor Author

If there is nothing which can be done about this (does this also apply to Jackson serializers / deserializers registered by build steps?), then it could be documented, which would also be good.

@geoand
Copy link
Contributor

geoand commented Sep 15, 2020

If there is nothing which can be done about this (does this also apply to Jackson serializers / deserializers registered by build steps?), then it could be documented, which would also be good.

It applies generally to any piece of code that doesn't use CDI to obtain the ObjectMapper.
I you want to contribute a docs PR, that would be splendid.

@knutwannheden
Copy link
Contributor Author

knutwannheden commented Sep 15, 2020

Yes, I could look into it. But I was hoping that something could be done behind the scenes at the RestEasy level.

Alternatively there could possibly be a flag to allow registering a CDI provider for ClientConfig or ClientBuilder which could be injected to get a "fully configured" client.

@geoand
Copy link
Contributor

geoand commented Sep 15, 2020

I doubt there is any good hook we could use, but if someone has an idea, I'd like to hear it

@gsmet
Copy link
Member

gsmet commented Sep 15, 2020

I will have a look tomorrow. It bugs me.

@gsmet
Copy link
Member

gsmet commented Sep 15, 2020

Ok, after some code reading, I think it's just that ClientBuilder.newClient() is not really what we support for REST Client.

You should use the MicroProfile REST Client. And AFAICS, in this case, we propagate the customized ObjectMapper.

The funny thing is that it looks like it would work when building a native executable because of https://github.com/quarkusio/quarkus/blob/master/extensions/rest-client/runtime/src/main/java/io/quarkus/restclient/runtime/graal/ClientBuilderReplacement.java but I see no way to do the same in pure JVM mode (well except doing it manually when you initialize your builder).

@geoand
Copy link
Contributor

geoand commented Sep 15, 2020

@gsmet yeah, we don't really do much with the pure JAX-RS client (I don't even know if we even mention it in the docs)

@kenfinnigan
Copy link
Member

I was hoping #11828 would actually fix it.

But it looks like we need to wait for the next version of RESTEasy that will have the ability to statically set the JAX-RS Client provider factory just like we do for MP REST Client

@gsmet
Copy link
Member

gsmet commented Sep 16, 2020

Yes exactly, we need a static thingy.

@geoand
Copy link
Contributor

geoand commented Sep 16, 2020

All in due time :)

@knutwannheden
Copy link
Contributor Author

Yes, that would make sense. Currently there appears to be a discrepancy between native and JVM mode in this area.

@geoand
Copy link
Contributor

geoand commented Jul 27, 2021

I am going to close this as this should work properly with managed quarkus-jaxrs-client-reactive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working triage/out-of-date This issue/PR is no longer valid or relevant
Projects
None yet
Development

No branches or pull requests

4 participants