From 4703850bd3c455a93c0d45b19feacf4ea561c425 Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 14 Jun 2023 10:27:23 +0200 Subject: [PATCH] Fix resolving custom ObjectMapper at deserialization in Resteasy Reactive Fix https://github.com/quarkusio/quarkus/issues/34008 --- .../ResteasyReactiveJacksonProcessor.java | 11 +- .../test/CustomObjectMapperTest.java | 99 ++++++++++++ ...eaturedServerJacksonMessageBodyReader.java | 143 ++++++++++++++++++ .../ClientJacksonMessageBodyReader.java | 54 +++++-- 4 files changed, 294 insertions(+), 13 deletions(-) create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/CustomObjectMapperTest.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/runtime/src/main/java/io/quarkus/resteasy/reactive/jackson/runtime/serialisers/FullyFeaturedServerJacksonMessageBodyReader.java diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/main/java/io/quarkus/resteasy/reactive/jackson/deployment/processor/ResteasyReactiveJacksonProcessor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/main/java/io/quarkus/resteasy/reactive/jackson/deployment/processor/ResteasyReactiveJacksonProcessor.java index a92e88117b6c8..fcb10ce690838 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/main/java/io/quarkus/resteasy/reactive/jackson/deployment/processor/ResteasyReactiveJacksonProcessor.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/main/java/io/quarkus/resteasy/reactive/jackson/deployment/processor/ResteasyReactiveJacksonProcessor.java @@ -54,6 +54,7 @@ import io.quarkus.resteasy.reactive.jackson.runtime.mappers.NativeInvalidDefinitionExceptionMapper; import io.quarkus.resteasy.reactive.jackson.runtime.security.SecurityCustomSerialization; import io.quarkus.resteasy.reactive.jackson.runtime.serialisers.BasicServerJacksonMessageBodyWriter; +import io.quarkus.resteasy.reactive.jackson.runtime.serialisers.FullyFeaturedServerJacksonMessageBodyReader; import io.quarkus.resteasy.reactive.jackson.runtime.serialisers.FullyFeaturedServerJacksonMessageBodyWriter; import io.quarkus.resteasy.reactive.jackson.runtime.serialisers.ServerJacksonMessageBodyReader; import io.quarkus.resteasy.reactive.jackson.runtime.serialisers.vertx.VertxJsonArrayMessageBodyReader; @@ -122,6 +123,7 @@ AdditionalBeanBuildItem beans() { // make these beans to they can get instantiated with the Quarkus CDI configured ObjectMapper object return AdditionalBeanBuildItem.builder() .addBeanClass(ServerJacksonMessageBodyReader.class.getName()) + .addBeanClass(FullyFeaturedServerJacksonMessageBodyReader.class) .addBeanClass(BasicServerJacksonMessageBodyWriter.class) // This will not be needed in most cases, but it will not be involved in serialization // just because it's a bean. @@ -141,7 +143,9 @@ void additionalProviders(ContextResolversBuildItem contextResolversBuildItem, additionalReaders .produce( - new MessageBodyReaderBuildItem.Builder(ServerJacksonMessageBodyReader.class.getName(), + new MessageBodyReaderBuildItem.Builder( + getJacksonMessageBodyReader( + hasObjectMapperContextResolver || specialJacksonFeaturesUsed), Object.class.getName()) .setMediaTypeStrings(HANDLED_MEDIA_TYPES) .setBuiltin(true).setRuntimeType(RuntimeType.SERVER).build()); @@ -194,6 +198,11 @@ private String getJacksonMessageBodyWriter(boolean needsFullFeatureSet) { : BasicServerJacksonMessageBodyWriter.class.getName(); } + private String getJacksonMessageBodyReader(boolean needsFullFeatureSet) { + return needsFullFeatureSet ? FullyFeaturedServerJacksonMessageBodyReader.class.getName() + : ServerJacksonMessageBodyReader.class.getName(); + } + @Record(ExecutionTime.STATIC_INIT) @BuildStep void handleJsonAnnotations(Optional resourceScanningResultBuildItem, diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/CustomObjectMapperTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/CustomObjectMapperTest.java new file mode 100644 index 0000000000000..99e66a254a9fa --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/CustomObjectMapperTest.java @@ -0,0 +1,99 @@ +package io.quarkus.resteasy.reactive.jackson.deployment.test; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.CoreMatchers.equalTo; + +import java.util.Objects; + +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.ext.ContextResolver; +import jakarta.ws.rs.ext.Provider; + +import org.apache.http.HttpStatus; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; + +import io.quarkus.arc.Unremovable; +import io.quarkus.test.QuarkusUnitTest; +import io.restassured.http.ContentType; + +public class CustomObjectMapperTest { + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest().withEmptyApplication(); + + /** + * Because we have configured the server Object Mapper instance with: + * `objectMapper.enable(SerializationFeature.WRAP_ROOT_VALUE);` + */ + @Test + void serverShouldUnwrapRootElement() { + given().body("{\"Request\":{\"value\":\"good\"}}") + .contentType(ContentType.JSON) + .post("/server") + .then() + .statusCode(HttpStatus.SC_OK) + .body(equalTo("good")); + } + + @Path("/server") + public static class MyResource { + @POST + @Consumes(MediaType.APPLICATION_JSON) + public String post(Request request) { + return request.value; + } + } + + public static class Request { + private String value; + + public Request() { + + } + + public Request(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Request request = (Request) o; + return Objects.equals(value, request.value); + } + + @Override + public int hashCode() { + return Objects.hash(value); + } + } + + @Provider + @Unremovable + public static class CustomObjectMapperContextResolver implements ContextResolver { + + @Override + public ObjectMapper getContext(final Class type) { + final ObjectMapper objectMapper = new ObjectMapper(); + objectMapper.enable(DeserializationFeature.UNWRAP_ROOT_VALUE); + return objectMapper; + } + } +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/runtime/src/main/java/io/quarkus/resteasy/reactive/jackson/runtime/serialisers/FullyFeaturedServerJacksonMessageBodyReader.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/runtime/src/main/java/io/quarkus/resteasy/reactive/jackson/runtime/serialisers/FullyFeaturedServerJacksonMessageBodyReader.java new file mode 100644 index 0000000000000..6ccb2595e3492 --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/runtime/src/main/java/io/quarkus/resteasy/reactive/jackson/runtime/serialisers/FullyFeaturedServerJacksonMessageBodyReader.java @@ -0,0 +1,143 @@ +package io.quarkus.resteasy.reactive.jackson.runtime.serialisers; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.annotation.Annotation; +import java.lang.reflect.Type; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; + +import jakarta.inject.Inject; +import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.MultivaluedMap; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.ext.ContextResolver; +import jakarta.ws.rs.ext.Providers; + +import org.jboss.resteasy.reactive.common.util.StreamUtil; +import org.jboss.resteasy.reactive.server.jackson.JacksonBasicMessageBodyReader; +import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo; +import org.jboss.resteasy.reactive.server.spi.ServerMessageBodyReader; +import org.jboss.resteasy.reactive.server.spi.ServerRequestContext; + +import com.fasterxml.jackson.core.exc.StreamReadException; +import com.fasterxml.jackson.databind.DatabindException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; + +public class FullyFeaturedServerJacksonMessageBodyReader extends JacksonBasicMessageBodyReader + implements ServerMessageBodyReader { + + private final Providers providers; + private final ConcurrentMap contextResolverMap = new ConcurrentHashMap<>(); + + @Inject + public FullyFeaturedServerJacksonMessageBodyReader(ObjectMapper mapper, Providers providers) { + super(mapper); + this.providers = providers; + } + + @Override + public Object readFrom(Class type, Type genericType, Annotation[] annotations, MediaType mediaType, + MultivaluedMap httpHeaders, InputStream entityStream) throws IOException, WebApplicationException { + try { + return doReadFrom(type, genericType, mediaType, entityStream); + } catch (MismatchedInputException | InvalidDefinitionException e) { + /* + * To extract additional details when running in dev mode or test mode, Quarkus previously offered the + * DefaultMismatchedInputException(Mapper). That mapper provides additional details about bad input, + * beyond Jackson's default, when running in Dev or Test mode. To preserve that behavior, we rethrow + * MismatchedInputExceptions we encounter. + * + * An InvalidDefinitionException is thrown when there is a problem with the way a type is + * set up/annotated for consumption by the Jackson API. We don't wrap it in a WebApplicationException + * (as a Server Error), since unhandled exceptions will end up as a 500 anyway. In addition, this + * allows built-in features like the NativeInvalidDefinitionExceptionMapper to be registered and + * communicate potential Jackson integration issues, and potential solutions for resolving them. + */ + throw e; + } catch (StreamReadException | DatabindException e) { + /* + * As JSON is evaluated, it can be invalid due to one of two reasons: + * 1) Malformed JSON. Un-parsable JSON results in a StreamReadException + * 2) Valid JSON that violates some binding constraint, i.e., a required property, mismatched data types, etc. + * Violations of these types are captured via a DatabindException. + */ + throw new WebApplicationException(e, Response.Status.BAD_REQUEST); + } + } + + @Override + public boolean isReadable(Class type, Type genericType, Annotation[] annotations, MediaType mediaType) { + return isReadable(mediaType, type); + } + + @Override + public boolean isReadable(Class type, Type genericType, ResteasyReactiveResourceInfo lazyMethod, MediaType mediaType) { + return isReadable(mediaType, type); + } + + @Override + public Object readFrom(Class type, Type genericType, MediaType mediaType, ServerRequestContext context) + throws WebApplicationException, IOException { + return readFrom(type, genericType, null, mediaType, null, context.getInputStream()); + } + + private Object doReadFrom(Class type, Type genericType, MediaType responseMediaType, InputStream entityStream) + throws IOException { + if (StreamUtil.isEmpty(entityStream)) { + return null; + } + try { + ObjectReader reader = getEffectiveReader(type, responseMediaType); + return reader.forType(reader.getTypeFactory().constructType(genericType != null ? genericType : type)) + .readValue(entityStream); + } catch (MismatchedInputException e) { + if (isEmptyInputException(e)) { + return null; + } + throw e; + } + } + + private boolean isEmptyInputException(MismatchedInputException e) { + // this isn't great, but Jackson doesn't have a specific exception for empty input... + return e.getMessage().startsWith("No content"); + } + + private ObjectReader getEffectiveReader(Class type, MediaType responseMediaType) { + ObjectMapper effectiveMapper = getObjectMapperFromContext(type, responseMediaType); + if (effectiveMapper == null) { + return getEffectiveReader(); + } + + return contextResolverMap.computeIfAbsent(effectiveMapper, new Function<>() { + @Override + public ObjectReader apply(ObjectMapper objectMapper) { + return objectMapper.reader(); + } + }); + } + + private ObjectMapper getObjectMapperFromContext(Class type, MediaType responseMediaType) { + if (providers == null) { + return null; + } + + ContextResolver contextResolver = providers.getContextResolver(ObjectMapper.class, + responseMediaType); + if (contextResolver == null) { + // TODO: not sure if this is correct, but Jackson does this as well... + contextResolver = providers.getContextResolver(ObjectMapper.class, null); + } + if (contextResolver != null) { + return contextResolver.getContext(type); + } + + return null; + } +} diff --git a/extensions/resteasy-reactive/rest-client-reactive-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java b/extensions/resteasy-reactive/rest-client-reactive-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java index 5939422b47b3a..ad7e481b8405b 100644 --- a/extensions/resteasy-reactive/rest-client-reactive-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java +++ b/extensions/resteasy-reactive/rest-client-reactive-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java @@ -13,11 +13,14 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.ext.ContextResolver; +import jakarta.ws.rs.ext.Providers; import org.jboss.logging.Logger; import org.jboss.resteasy.reactive.ClientWebApplicationException; import org.jboss.resteasy.reactive.client.impl.RestClientRequestContext; import org.jboss.resteasy.reactive.client.spi.ClientRestHandler; +import org.jboss.resteasy.reactive.common.util.EmptyInputStream; import org.jboss.resteasy.reactive.server.jackson.JacksonBasicMessageBodyReader; import com.fasterxml.jackson.core.JsonParseException; @@ -42,7 +45,13 @@ public ClientJacksonMessageBodyReader(ObjectMapper mapper) { public Object readFrom(Class type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap httpHeaders, InputStream entityStream) throws IOException, WebApplicationException { try { - return super.readFrom(type, genericType, annotations, mediaType, httpHeaders, entityStream); + if (entityStream instanceof EmptyInputStream) { + return null; + } + ObjectReader reader = getEffectiveReader(type, mediaType); + return reader.forType(reader.getTypeFactory().constructType(genericType != null ? genericType : type)) + .readValue(entityStream); + } catch (JsonParseException e) { log.debug("Server returned invalid json data", e); throw new ClientWebApplicationException(e, Response.Status.OK); @@ -56,23 +65,44 @@ public void handle(RestClientRequestContext requestContext) { this.context = requestContext; } - @Override - protected ObjectReader getEffectiveReader() { - if (context == null) { - // no context injected when reader is not running within a rest client context - return super.getEffectiveReader(); - } - - ObjectMapper objectMapper = context.getConfiguration().getFromContext(ObjectMapper.class); - if (objectMapper == null) { - return super.getEffectiveReader(); + private ObjectReader getEffectiveReader(Class type, MediaType responseMediaType) { + ObjectMapper effectiveMapper = getObjectMapperFromContext(type, responseMediaType); + if (effectiveMapper == null) { + return getEffectiveReader(); } - return contextResolverMap.computeIfAbsent(objectMapper, new Function<>() { + return contextResolverMap.computeIfAbsent(effectiveMapper, new Function<>() { @Override public ObjectReader apply(ObjectMapper objectMapper) { return objectMapper.reader(); } }); } + + private ObjectMapper getObjectMapperFromContext(Class type, MediaType responseMediaType) { + Providers providers = getProviders(); + if (providers == null) { + return null; + } + + ContextResolver contextResolver = providers.getContextResolver(ObjectMapper.class, + responseMediaType); + if (contextResolver == null) { + // TODO: not sure if this is correct, but Jackson does this as well... + contextResolver = providers.getContextResolver(ObjectMapper.class, null); + } + if (contextResolver != null) { + return contextResolver.getContext(type); + } + + return null; + } + + private Providers getProviders() { + if (context != null && context.getClientRequestContext() != null) { + return context.getClientRequestContext().getProviders(); + } + + return null; + } }