quarkus: Resteasy Reactive: ContextResolver not used
Describe the bug
When creating a RestClient with a ContextResolver<ObjectMapper> registered, this ContextResolver is never used and thus the wrong ObjectMapper (via CDI) is used. Other implementation approaches would be fine as well, but nothing seems to get this behaviour working. Multiple different ObjectMappers in the application do not seem to be supported.
Expected behavior
The ObjectMapper returned by a class that implements ContextResolver<ObjectMapper> used via RestClientBuilder#register(Object) is used in the given RestClient and only there.
Multiple RestClients can use multiple different ObjectMappers.
Actual behavior
The ObjectMapper of the registered ContextResolver is not used at all. Instead an application scoped ObjectMapper bean is used in all RestClients.
How to Reproduce?
Run the following test class. The test should pass if matching the expected behaviour.
@QuarkusTest
class MyClientTest {
MyClient clientAllowsUnknown;
MyClient clientDisallowsUnknown;
WireMockServer wireMockServer = getWireMockServer();
@BeforeEach
void setUp() throws MalformedURLException {
wireMockServer.resetAll();
clientAllowsUnknown = RestClientBuilder.newBuilder()
.baseUrl(new URL(wireMockServer.baseUrl()))
.register(ClientObjectMapperUnknown.class)
.build(MyClient.class);
clientDisallowsUnknown = RestClientBuilder.newBuilder()
.baseUrl(new URL(wireMockServer.baseUrl()))
.register(ClientObjectMapperNoUnknown.class)
.build(MyClient.class);
}
@Test
void something_withAdditionalIgnoredProperties() {
var json = "{ \"value\": \"someValue\", \"secondValue\": \"toBeIgnored\" }";
wireMockServer.stubFor(
WireMock.get(WireMock.urlMatching("/something"))
.willReturn(okJson(json)));
var result = clientAllowsUnknown.something().await().indefinitely();
// FAIL_ON_UNKNOWN_PROPERTIES disabled
assertThatCode(() -> new ClientObjectMapperUnknown().getContext(ObjectMapper.class).readValue(json, Something.class))
.doesNotThrowAnyException();
assertThat(result).isEqualTo(Something.builder().withValue("someValue").build());
// FAIL_ON_UNKNOWN_PROPERTIES enabled
assertThatThrownBy(() -> new ClientObjectMapperNoUnknown().getContext(ObjectMapper.class).readValue(json, Something.class))
.isInstanceOf(JsonProcessingException.class);
assertThatThrownBy(() -> clientDisallowsUnknown.something().await().indefinitely())
.isInstanceOf(JsonProcessingException.class);
}
@Path("/something")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public interface MyClient {
@GET
Uni<Something> something();
}
@Value
@Builder(toBuilder = true, setterPrefix = "with")
@Jacksonized
public static class Something {
String value;
}
public static class ClientObjectMapperUnknown implements ContextResolver<ObjectMapper> {
@Override
public ObjectMapper getContext(Class<?> type) {
return new ObjectMapper()
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
}
}
public static class ClientObjectMapperNoUnknown implements ContextResolver<ObjectMapper> {
@Override
public ObjectMapper getContext(Class<?> type) {
return new ObjectMapper()
.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.enable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
}
}
public static WireMockServer getWireMockServer() {
var wireMockServer = new WireMockServer(options().port(getAvailableTcpPort(20000, 22000)));
wireMockServer.start();
return wireMockServer;
}
public static int getAvailableTcpPort(int min, int max) {
var ports = IntStream.range(min, max).boxed().collect(Collectors.toList());
Collections.shuffle(ports); // shuffle to get a random order and reduce the probability a port is already in use
for (var port : ports) {
try (ServerSocket serverSocket = new ServerSocket(port)) {
return serverSocket.getLocalPort();
} catch (IOException e) {
// try next
}
}
throw new IllegalStateException(MessageFormat.format("Could not find a free TCP port in range {0}:{1}.", min, max));
}
}
With
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-rest-client-reactive-jackson</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-reactive</artifactId>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>1.18.22</version>
</dependency>
Output of uname -a or ver
Darwin M042112251A.local 20.6.0 Darwin Kernel Version 20.6.0: Wed Nov 10 22:23:07 PST 2021; root:xnu-7195.141.14~1/RELEASE_X86_64 x86_64
Output of java -version
openjdk version “11.0.14.1” 2022-02-08 OpenJDK Runtime Environment Temurin-11.0.14.1+1 (build 11.0.14.1+1) OpenJDK 64-Bit Server VM Temurin-11.0.14.1+1 (build 11.0.14.1+1, mixed mode)
GraalVM version (if different from Java)
not used
Quarkus version or git rev
2.8.0.Final, 2.9.2.Final, 2.10.0.CR1
Build tool (ie. output of mvnw --version or gradlew --version)
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Additional information
No response
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 1
- Comments: 35 (27 by maintainers)
Commits related to this issue
- Introduce support for ContextResolver<ObjectMapper> in server part of RESTEasy Reactive Relates to: #26152 — committed to geoand/quarkus by geoand 2 years ago
- Introduce support for ContextResolver<ObjectMapper> in server part of RESTEasy Reactive Relates to: #26152 — committed to geoand/quarkus by geoand 2 years ago
- Introduce support for ContextResolver<ObjectMapper> in server part of RESTEasy Reactive Relates to: #26152 — committed to geoand/quarkus by geoand 2 years ago
- Merge pull request #26988 from geoand/#26152 Introduce support for ContextResolver<ObjectMapper> in server part of RESTEasy Reactive — committed to quarkusio/quarkus by geoand 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to Sgitario/quarkus by Sgitario 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to Sgitario/quarkus by Sgitario 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to Sgitario/quarkus by Sgitario 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to Sgitario/quarkus by Sgitario 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to Sgitario/quarkus by Sgitario 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to fercomunello/quarkus by Sgitario 2 years ago
- Introduce support for ContextResolver<ObjectMapper> in server part of RESTEasy Reactive Relates to: #26152 — committed to miador/quarkus by geoand 2 years ago
- Rest Client Reactive: Provide custom ObjectMapper in request scope With these changes, we can now register custom object mappers when creating a client programmatically: ``` clientAllowsUnknown = Re... — committed to nenros/quarkus by Sgitario 2 years ago
- Fix client jackson body writer to propagate the client context Fix https://github.com/quarkusio/quarkus/issues/26152 — committed to Sgitario/quarkus by Sgitario a year ago
- Fix client jackson body writer to propagate the client context Fix https://github.com/quarkusio/quarkus/issues/26152 (cherry picked from commit e05af28364c3bea101a8a8ee16e33d3321d2470f) — committed to gsmet/quarkus by Sgitario a year ago
- Update all non-major dependencies (mulk/mulkcms2!21) This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [flow-bin](https://github.com/flowtype/flow-bin)... — committed to benkard/mulkcms2 by deleted user a year ago
Yes, please create a new issue with a reproducer, so we can see what’s going on.
The CDI support was partially fixed by https://github.com/quarkusio/quarkus/commit/b60b23def77cbd4b31896259239ea46784abee25 but, I’ve just spotted another issue that will be fixed in https://github.com/quarkusio/quarkus/pull/31422.
https://github.com/quarkusio/quarkus/pull/26988 takes care of the server part.
For the client part, this comment still applies.
You’re missing my point then 😃 - it might not be enough to justify the work but better make sure you have it. My concern is that people migrating from the REST Client to the Reactive one won’t notice until too late that their
ContextResolverwon’t actually be taken into account. It’s not as if the failure would always be big enough to notice it. For instance, you could just lose a field that you used to persist or something similar. And realize a few days later that the data is missing. That’s my biggest concern about us not supporting the “old” way.We could for example use CDI qualifiers on the class, or even utilize a static (or default) method on the interface that would yield an object mapper.
My point is that there are a lot of ways of making this better than relying on the antiquated and arcane
ContextResolverAPIYeah, I understand that and it’s completely reasonable. The PR mentioned above does not exactly do that, but it could lay the groundwork for making things easier when it comes to addressing this use case.
I personally think
ContextResolveris a very dated API that most people don’t know about anyway so we should strive to come up with something more modern and usable.I have a similar issue. I tried to migrate fom
resteasytoresteasy-reactiveand fromrest-clienttorest-client-reactive. Every Client has it’s own ObjectMapper which is registered via@RegisterProvider(....class). In rest-client this works, in rest-client-reactive it’s not working. If a breakpoint is set in the getContext() method you see that it’s called only whenrest-clientis used.