Skip to content

Commit

Permalink
Merge pull request #36126 from geoand/#36067
Browse files Browse the repository at this point in the history 
Properly cache ContextResolver usage for ObjectMapper
  • Loading branch information
@geoand
geoand authored Sep 25, 2023
2 parents 87966d9 + 1c65e20 commit 705de0b
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 67 deletions.
100 changes: 92 additions & 8 deletions ...est/java/io/quarkus/resteasy/reactive/jackson/deployment/test/CustomObjectMapperTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package io.quarkus.resteasy.reactive.jackson.deployment.test;

import static io.restassured.RestAssured.given;
import static io.restassured.RestAssured.when;
import static org.hamcrest.CoreMatchers.equalTo;

import java.util.Objects;
import java.util.concurrent.atomic.AtomicLong;

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.core.MediaType;
Expand All @@ -18,6 +21,7 @@

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;

import io.quarkus.arc.Unremovable;
import io.quarkus.test.QuarkusUnitTest;
Expand All @@ -32,26 +36,99 @@ public class CustomObjectMapperTest {
* `objectMapper.enable(SerializationFeature.WRAP_ROOT_VALUE);`
*/
@Test
void serverShouldUnwrapRootElement() {
given().body("{\"Request\":{\"value\":\"good\"}}")
void test() {
given().body("{\"Request\":{\"value\":\"FIRST\"}}")
.contentType(ContentType.JSON)
.post("/server")
.post("/server/dummy")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("good"));
.body(equalTo("0"));

// ContextResolver was invoked for both reader and writer
when().get("/server/count")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("2"));

given().body("{\"Request2\":{\"value\":\"FIRST\"}}")
.contentType(ContentType.JSON)
.post("/server/dummy2")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("0"));

// ContextResolver was invoked for both reader and writer because different types where used
when().get("/server/count")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("4"));

given().body("{\"Request\":{\"value\":\"FIRST\"}}")
.contentType(ContentType.JSON)
.post("/server/dummy")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("0"));

// ContextResolver was not invoked because the types have already been cached
when().get("/server/count")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("4"));

given().body("{\"Request2\":{\"value\":\"FIRST\"}}")
.contentType(ContentType.JSON)
.post("/server/dummy2")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("0"));

// ContextResolver was not invoked because the types have already been cached
when().get("/server/count")
.then()
.statusCode(HttpStatus.SC_OK)
.body(equalTo("4"));
}

private static void doTest() {

}

@Path("/server")
public static class MyResource {
@POST
@Consumes(MediaType.APPLICATION_JSON)
public String post(Request request) {
return request.value;
@Path("dummy")
public Dummy dummy(Request request) {
return Dummy.valueOf(request.value);
}

@POST
@Consumes(MediaType.APPLICATION_JSON)
@Path("dummy2")
public Dummy2 dummy2(Request2 request) {
return Dummy2.valueOf(request.value);
}

@GET
@Path("count")
public long count() {
return CustomObjectMapperContextResolver.COUNT.get();
}
}

public enum Dummy {
FIRST,
SECOND
}

public enum Dummy2 {
FIRST,
SECOND
}

public static class Request {
private String value;
protected String value;

public Request() {

Expand Down Expand Up @@ -85,14 +162,21 @@ public int hashCode() {
}
}

public static class Request2 extends Request {
}

@Provider
@Unremovable
public static class CustomObjectMapperContextResolver implements ContextResolver<ObjectMapper> {

static final AtomicLong COUNT = new AtomicLong();

@Override
public ObjectMapper getContext(final Class<?> type) {
COUNT.incrementAndGet();
final ObjectMapper objectMapper = new ObjectMapper();
objectMapper.enable(DeserializationFeature.UNWRAP_ROOT_VALUE);
objectMapper.enable(DeserializationFeature.UNWRAP_ROOT_VALUE)
.enable(SerializationFeature.WRITE_ENUMS_USING_INDEX);
return objectMapper;
}
}
Expand Down
16 changes: 13 additions & 3 deletions ...asy/reactive/jackson/runtime/serialisers/FullyFeaturedServerJacksonMessageBodyReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public class FullyFeaturedServerJacksonMessageBodyReader extends JacksonBasicMes
private final Providers providers;
private final ConcurrentMap<String, ObjectReader> perMethodReader = new ConcurrentHashMap<>();
private final ConcurrentMap<String, ObjectReader> perTypeReader = new ConcurrentHashMap<>();
private final ConcurrentMap<ObjectMapper, ObjectReader> contextResolverMap = new ConcurrentHashMap<>();
private final ConcurrentMap<Class<?>, ObjectMapper> contextResolverMap = new ConcurrentHashMap<>();
private final ConcurrentMap<ObjectMapper, ObjectReader> objectReaderMap = new ConcurrentHashMap<>();

@Inject
public FullyFeaturedServerJacksonMessageBodyReader(ObjectMapper mapper, Providers providers) {
Expand Down Expand Up @@ -154,7 +155,7 @@ private ObjectReader getEffectiveReader(Class<Object> type, Type genericType, Me
ObjectReader effectiveReader = defaultReader;
if (effectiveMapper != originalMapper) {
// Effective reader based on the context
effectiveReader = contextResolverMap.computeIfAbsent(effectiveMapper, new Function<>() {
effectiveReader = objectReaderMap.computeIfAbsent(effectiveMapper, new Function<>() {
@Override
public ObjectReader apply(ObjectMapper objectMapper) {
return objectMapper.reader();
Expand Down Expand Up @@ -201,7 +202,16 @@ private ObjectMapper getEffectiveMapper(Class<Object> type, MediaType responseMe
contextResolver = providers.getContextResolver(ObjectMapper.class, null);
}
if (contextResolver != null) {
return contextResolver.getContext(type);
var cr = contextResolver;
ObjectMapper result = contextResolverMap.computeIfAbsent(type, new Function<>() {
@Override
public ObjectMapper apply(Class<?> aClass) {
return cr.getContext(type);
}
});
if (result != null) {
return result;
}
}

return originalMapper;
Expand Down
13 changes: 10 additions & 3 deletions ...asy/reactive/jackson/runtime/serialisers/FullyFeaturedServerJacksonMessageBodyWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public class FullyFeaturedServerJacksonMessageBodyWriter extends ServerMessageBo
private final ObjectWriter defaultWriter;
private final ConcurrentMap<String, ObjectWriter> perMethodWriter = new ConcurrentHashMap<>();
private final ConcurrentMap<String, ObjectWriter> perTypeWriter = new ConcurrentHashMap<>();
private final ConcurrentMap<ObjectMapper, ObjectWriter> contextResolverMap = new ConcurrentHashMap<>();
private final ConcurrentMap<Class<?>, ObjectMapper> contextResolverMap = new ConcurrentHashMap<>();
private final ConcurrentMap<ObjectMapper, ObjectWriter> objectWriterMap = new ConcurrentHashMap<>();

@Inject
public FullyFeaturedServerJacksonMessageBodyWriter(ObjectMapper mapper, Providers providers) {
Expand Down Expand Up @@ -112,7 +113,7 @@ private ObjectWriter getEffectiveWriter(ObjectMapper effectiveMapper) {
if (effectiveMapper == originalMapper) {
return defaultWriter;
}
return contextResolverMap.computeIfAbsent(effectiveMapper, new Function<>() {
return objectWriterMap.computeIfAbsent(effectiveMapper, new Function<>() {
@Override
public ObjectWriter apply(ObjectMapper objectMapper) {
return createDefaultWriter(effectiveMapper);
Expand All @@ -133,7 +134,13 @@ private ObjectMapper getEffectiveMapper(Object o, ServerRequestContext context)
contextResolver = providers.getContextResolver(ObjectMapper.class, null);
}
if (contextResolver != null) {
ObjectMapper mapperFromContextResolver = contextResolver.getContext(o.getClass());
var cr = contextResolver;
ObjectMapper mapperFromContextResolver = contextResolverMap.computeIfAbsent(o.getClass(), new Function<>() {
@Override
public ObjectMapper apply(Class<?> aClass) {
return cr.getContext(o.getClass());
}
});
if (mapperFromContextResolver != null) {
effectiveMapper = mapperFromContextResolver;
}
Expand Down
34 changes: 23 additions & 11 deletions ...uarkus/rest/client/reactive/jackson/test/DifferentObjectMapperForClientAndServerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
import static io.restassured.RestAssured.given;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.net.URI;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;

import jakarta.inject.Singleton;
import jakarta.ws.rs.GET;
Expand Down Expand Up @@ -70,10 +68,17 @@ void serverShouldWrapRootElement() {
*/
@Test
void shouldClientUseCustomObjectMapperUnwrappingRootElement() {
assertFalse(ClientObjectMapperUnwrappingRootElement.USED.get());
AtomicLong count = ClientObjectMapperUnwrappingRootElement.COUNT;
assertEquals(0, count.get());
Request request = clientUnwrappingRootElement.get();
assertEquals("good", request.value);
assertTrue(ClientObjectMapperUnwrappingRootElement.USED.get());
assertEquals(1, count.get());

assertEquals("good", clientUnwrappingRootElement.get().value);
assertEquals("good", clientUnwrappingRootElement.get().value);
assertEquals("good", clientUnwrappingRootElement.get().value);
// count should not change as the resolution of the ObjectMapper should be cached
assertEquals(1, count.get());
}

/**
Expand All @@ -82,10 +87,17 @@ void shouldClientUseCustomObjectMapperUnwrappingRootElement() {
*/
@Test
void shouldClientUseCustomObjectMapperNotUnwrappingRootElement() {
assertFalse(MyClientNotUnwrappingRootElement.CUSTOM_OBJECT_MAPPER_USED.get());
AtomicLong count = MyClientNotUnwrappingRootElement.CUSTOM_OBJECT_MAPPER_COUNT;
assertEquals(0, count.get());
Request request = clientNotUnwrappingRootElement.get();
assertNull(request.value);
assertTrue(MyClientNotUnwrappingRootElement.CUSTOM_OBJECT_MAPPER_USED.get());
assertEquals(1, count.get());

assertNull(clientNotUnwrappingRootElement.get().value);
assertNull(clientNotUnwrappingRootElement.get().value);
assertNull(clientNotUnwrappingRootElement.get().value);
// count should not change as the resolution of the ObjectMapper should be cached
assertEquals(1, count.get());
}

@Path("/server")
Expand All @@ -108,14 +120,14 @@ public interface MyClientUnwrappingRootElement {
@Path("/server")
@Produces(MediaType.APPLICATION_JSON)
public interface MyClientNotUnwrappingRootElement {
AtomicBoolean CUSTOM_OBJECT_MAPPER_USED = new AtomicBoolean(false);
AtomicLong CUSTOM_OBJECT_MAPPER_COUNT = new AtomicLong();

@GET
Request get();

@ClientObjectMapper
static ObjectMapper objectMapper(ObjectMapper defaultObjectMapper) {
CUSTOM_OBJECT_MAPPER_USED.set(true);
CUSTOM_OBJECT_MAPPER_COUNT.incrementAndGet();
return defaultObjectMapper.copy()
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.disable(DeserializationFeature.UNWRAP_ROOT_VALUE);
Expand Down Expand Up @@ -158,11 +170,11 @@ public int hashCode() {
}

public static class ClientObjectMapperUnwrappingRootElement implements ContextResolver<ObjectMapper> {
static final AtomicBoolean USED = new AtomicBoolean(false);
static final AtomicLong COUNT = new AtomicLong();

@Override
public ObjectMapper getContext(Class<?> type) {
USED.set(true);
COUNT.incrementAndGet();
return new ObjectMapper().enable(DeserializationFeature.UNWRAP_ROOT_VALUE);
}
}
Expand Down
38 changes: 6 additions & 32 deletions ...rkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.quarkus.rest.client.reactive.jackson.runtime.serialisers;

import static io.quarkus.rest.client.reactive.jackson.runtime.serialisers.JacksonUtil.getObjectMapperFromContext;

import java.io.IOException;
import java.io.InputStream;
import java.lang.annotation.Annotation;
Expand All @@ -13,8 +15,6 @@
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;
Expand All @@ -33,7 +33,8 @@ public class ClientJacksonMessageBodyReader extends JacksonBasicMessageBodyReade

private static final Logger log = Logger.getLogger(ClientJacksonMessageBodyReader.class);

private final ConcurrentMap<ObjectMapper, ObjectReader> contextResolverMap = new ConcurrentHashMap<>();
private final ConcurrentMap<ResolverMapKey, ObjectMapper> contextResolverMap = new ConcurrentHashMap<>();
private final ConcurrentMap<ObjectMapper, ObjectReader> objectReaderMap = new ConcurrentHashMap<>();
private RestClientRequestContext context;

@Inject
Expand Down Expand Up @@ -66,43 +67,16 @@ public void handle(RestClientRequestContext requestContext) {
}

private ObjectReader getEffectiveReader(Class<Object> type, MediaType responseMediaType) {
ObjectMapper effectiveMapper = getObjectMapperFromContext(type, responseMediaType);
ObjectMapper effectiveMapper = getObjectMapperFromContext(type, responseMediaType, context, contextResolverMap);
if (effectiveMapper == null) {
return getEffectiveReader();
}

return contextResolverMap.computeIfAbsent(effectiveMapper, new Function<>() {
return objectReaderMap.computeIfAbsent(effectiveMapper, new Function<>() {
@Override
public ObjectReader apply(ObjectMapper objectMapper) {
return objectMapper.reader();
}
});
}

private ObjectMapper getObjectMapperFromContext(Class<Object> type, MediaType responseMediaType) {
Providers providers = getProviders();
if (providers == null) {
return null;
}

ContextResolver<ObjectMapper> 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;
}
}
Loading

0 comments on commit 705de0b

Please sign in to comment.