Quarkus REST fails to write polymorphic type property in JSON

[danielbobbert]

Describe the bug

When using inheritance with Jackson, Quarkus REST fails to write the type id property.
The problem was NOT present in quarkus-resteasy-jackson, but it was present in quarkus-resteasy-reactive-jackson (checked back to 3.8.6) and it is present in quarkus-rest 3.15.1

Expected behavior

The JSON representation of derived classes should contain the annotated type id property.

Actual behavior

The JSON representation of derived classes does not contain the annotated type id property. For example, RestAssured fails with error "Could not resolve subtype of [simple type, class org.acme.model.BaseClass]: missing type id property 'type'"

How to Reproduce?

See the attached reproducer project. It contains two tests:

1. The first test uses Jackson manually to serialize a derived class. It contains a "type" property, as expected.
2. The second test calls a REST endpoint which formally returns a list of base objects, which at runtime contains two instances of derived classes. This test fails, because the "type" property is missing from the rest objects.

The root of the problem must by Quarkus REST (reactive), because

1. Serialization works correctly with plain Jackson
2. The tests all succeed if you switch to "quarkus-resteasy-jackson" (the old sync API) in the pom.xml
3. But it fails with "quarkus-resteasy-reactive-jackson" (no matter, what version of Quarkus you use. I tested 3.8.6 up to 3.15.1)
4. If also fails with "quarkus-rest-jackson" 3.15.1 (which is the successor to "quarkus-resteasy-reactive-jackson", so that's not a surprise)

So the reactive REST stack seems to do JSON serialization a bit differently than the old non-reactive "resteasy" stack.

Output of uname -a or ver

No response

Output of java -version

Java 21

Quarkus version or git rev

Quarkus 3.8.6 - 3.15.1

Build tool (ie. output of mvnw --version or gradlew --version)

mvn 3.9.9

Additional information

polymorphic-json-bug.zip

[geoand]

@mariofusco I wonder if you want to check this out at some point

[mariofusco]

@geoand sure, I will give a look, feel free to assign this to me.

[geoand]

🙏🏽

[danielbobbert]

I even checked back with Quarkus 3.2.1.Final
Same result: It works with "quarkus-resteasy-jackson", but it fails with "quarkus-resteasy-reactive-jackson".

[gsmet]

I can confirm the reproducer allows to reproduce the problem easily and that we are somehow missing the types (we should have a type property in the JSON) from the serialized output:

[
    {
        "id": "ID A",
        "aProperty": "aProperty"
    },
    {
        "id": "ID B",
        "bProperty": "bProperty"
    }
]

Now as to why, this is the mystery we need to solve.

Also just to clarify, this isn't a native executable oddity, the problem happens with plain JVM.

[danielbobbert]

I think, I found something. If you add the following test to the reproducer, it will always fail, no matter what version/kind of rest easy stack you are using:

@Test
void testJacksonList() throws IOException {
    DerivedClassA a = new DerivedClassA();
    a.id = "id";
    a.aProperty = "property";

    DerivedClassB b = new DerivedClassB();
    b.id = "id";
    b.bProperty = "property";

    String json = objectMapper.writeValueAsString(List.of(a, b));
    Assertions.assertTrue(json.contains("\"type\":\"A\""));
    Assertions.assertTrue(json.contains("\"type\":\"B\""));
}

So, Jackson is really missing the "type" property already, if you serialize a list. (My bad, that the Jackson-Test in the reproducer only serialized a single element, while the REST test was using a list).

But the following works:

@Test
void testJacksonListGeneric() throws IOException {
    DerivedClassA a = new DerivedClassA();
    a.id = "id";
    a.aProperty = "property";

    DerivedClassB b = new DerivedClassB();
    b.id = "id";
    b.bProperty = "property";

    String json = objectMapper.writerFor(new TypeReference<List<BaseClass>>() {
    }).writeValueAsString(List.of(a, b));

    Assertions.assertTrue(json.contains("\"type\":\"A\""));
    Assertions.assertTrue(json.contains("\"type\":\"B\""));
}

So it seems, that Jackson needs the hint about the generic type of list, in order to decide that it needs to generate the "type" properties.

My guess would be that quarkus-resteasy-jackson was taking the generic return type of the method into account. And in fact, in Quarkus 3.14.1, org.jboss.resteasy.core.ServerResponseWriter, lines 111-125, look like this:

            Class type = jaxrsResponse.getEntityClass();
            Type generic = jaxrsResponse.getGenericType();
            Annotation[] annotations = jaxrsResponse.getAnnotations();
            MediaType mt = jaxrsResponse.getMediaType();
            // A filter could set the entity without setting the media type. While the java doc for Response.setEntity(Object)
            // states "The existing entity annotations and media type are preserved.", we need the media type for the
            // writer. We'll go ahead and set a default.
            if (mt == null) {
                mt = getDefaultContentType(request, jaxrsResponse, providerFactory, method);
                if (mt != null) {
                    jaxrsResponse.getHeaders().putSingle(HttpHeaders.CONTENT_TYPE, mt);
                }
            }
            MessageBodyWriter writer = providerFactory.getMessageBodyWriter(
                    type, generic, annotations, mt);

So the "generic" response type is taken into account when resolving the MessageBodyWriter.

But in quarkus-rest, a "one-for-all" message writer is constructed in JacksonMessageBodyWriterUtil.createDefaultWriter(). And in io.quarkus.resteasy.reactive.jackson.runtime.serialisers.BasicServerJacksonMessageBodyWriter, lines 57-67, the generic Type is not taken into account (even though it is available as a method parameter):

public void writeResponse(Object o, Type genericType, ServerRequestContext context) throws WebApplicationException, IOException {
  OutputStream stream = context.getOrCreateOutputStream();
  if (o instanceof String) {
    stream.write(((String)o).getBytes(StandardCharsets.UTF_8));
  } else {
    this.defaultWriter.writeValue(stream, o);
  }
  // we don't use try-with-resources because that results in writing to the http output without the exception mapping coming into play
  stream.close();
}

Chosing the proper writer here might solve the problem. I'm thinking along the lines of

@Override
public void writeResponse(Object o, Type genericType, ServerRequestContext context)
        throws WebApplicationException, IOException {
    OutputStream stream = context.getOrCreateOutputStream();
    if (o instanceof String) { // YUK: done in order to avoid adding extra quotes...
        stream.write(((String) o).getBytes(StandardCharsets.UTF_8));
    } else if (genericType == null) {
        // no generic type information available. Use default writer.
        defaultWriter.writeValue(stream, o);
    } else {
        TypeReference<Object> t = new TypeReference<>() {
            public Type getType() {
                return genericType;
            }
        };
        this.defaultWriter.forType(t).writeValue(stream, o);
    }
    // we don't use try-with-resources because that results in writing to the http output without the exception mapping coming into play
    stream.close();
}

I don't known whether this has any negative performance implications and/or some (caching) factory must/should be used to resolve the proper writer.
But I hope that my findings can help to find a proper solution more quickly.

[gsmet]

@danielbobbert definitely some nice detective work.

I think you probably nailed it. From what I can see it's going to have some performance consequences though. Now if genericType is actually only set when you have generics, it might be fine as we probably need correctness more than speed in this case.

@geoand will probably have a lot of interest in your findings but we are currently preparing Devoxx so it might have to wait a bit.

[geoand]

Excellent work @danielbobbert !

I won't have time now, but feel free to open a PR with your proposed solution and we can discuss it when time permits

[danielbobbert]

I have created a PR, including an in-memory cache of specialized writers to make sure that each generic writer is created only once, to mitigate any performance concerns.