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

Jakarta - EE 9 - Serialization issue in MongoDB IT PojoResourceTest #27506

Closed
gsmet opened this issue Aug 25, 2022 · 14 comments · Fixed by #27618 or #27688
Closed

Jakarta - EE 9 - Serialization issue in MongoDB IT PojoResourceTest #27506

gsmet opened this issue Aug 25, 2022 · 14 comments · Fixed by #27618 or #27688

Comments

@gsmet
Copy link
Member

gsmet commented Aug 25, 2022

The test is failing with:

2022-08-25 15:09:49,202 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-0) HTTP Request to /pojos failed, error id: 76c5f6d0-f5e4-475f-8cd7-dfa13a436fc7-1: jakarta.json.bind.JsonbException: Unable to deserialize property 'optionalString' because of: Incorrect position for processing type: class java.lang.String. Received event: START_OBJECT Allowed: [VALUE_NUMBER, VALUE_FALSE, VALUE_TRUE, VALUE_NULL, VALUE_STRING]
	at org.eclipse.yasson.internal.deserializer.ObjectDeserializer.deserialize(ObjectDeserializer.java:80)
	at org.eclipse.yasson.internal.deserializer.ObjectDeserializer.deserialize(ObjectDeserializer.java:31)
	at org.eclipse.yasson.internal.deserializer.DefaultObjectInstanceCreator.deserialize(DefaultObjectInstanceCreator.java:57)
	at org.eclipse.yasson.internal.deserializer.DefaultObjectInstanceCreator.deserialize(DefaultObjectInstanceCreator.java:29)
	at org.eclipse.yasson.internal.deserializer.PositionChecker.deserialize(PositionChecker.java:85)
	at org.eclipse.yasson.internal.deserializer.PositionChecker.deserialize(PositionChecker.java:34)
	at org.eclipse.yasson.internal.deserializer.NullCheckDeserializer.deserialize(NullCheckDeserializer.java:46)
	at org.eclipse.yasson.internal.deserializer.NullCheckDeserializer.deserialize(NullCheckDeserializer.java:26)
	at org.eclipse.yasson.internal.DeserializationContextImpl.deserializeItem(DeserializationContextImpl.java:138)
	at org.eclipse.yasson.internal.DeserializationContextImpl.deserialize(DeserializationContextImpl.java:127)
	at org.eclipse.yasson.internal.JsonBinding.deserialize(JsonBinding.java:55)
	at org.eclipse.yasson.internal.JsonBinding.fromJson(JsonBinding.java:95)
	at org.jboss.resteasy.reactive.server.jsonb.JsonbMessageBodyReader.doReadFrom(JsonbMessageBodyReader.java:55)
	at org.jboss.resteasy.reactive.server.jsonb.JsonbMessageBodyReader.readFrom(JsonbMessageBodyReader.java:37)
	at org.jboss.resteasy.reactive.server.handlers.RequestDeserializeHandler.readFrom(RequestDeserializeHandler.java:108)
	at org.jboss.resteasy.reactive.server.handlers.RequestDeserializeHandler.handle(RequestDeserializeHandler.java:67)
	at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:102)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:140)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:557)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:829)

If I remove the Optiona<String> in Pojo, it works OK but as this field is supposed to test something specific, we can't really do that.

Not sure if it's due to a Yasson regression or a bug in what we're doing, it needs some digging. We are using the latest version of Yasson.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 25, 2022

/cc @evanchooly, @loicmathieu

@loicmathieu
Copy link
Contributor

I remember a very specific issue with Yasson and Optional that may be why this test exists, it may be two years old so I need to dig a little to remember what it was.

@gsmet
Copy link
Member Author

gsmet commented Aug 25, 2022

@loicmathieu I will send an email to the list early next week with instructions on how to reproduce all these Jakarta issues.

@loicmathieu
Copy link
Contributor

OK, I'll have a look next week then

@loicmathieu
Copy link
Contributor

This issue is strange, when I launch the project locally and test it with curl I saw a NPE inside Yasson (which should never occurs).

Caused by: java.lang.NullPointerException
	at org.eclipse.yasson.internal.serializer.OptionalSerializer.serialize(OptionalSerializer.java:36)
	at org.eclipse.yasson.internal.serializer.ValueGetterSerializer.serialize(ValueGetterSerializer.java:43)
	at org.eclipse.yasson.internal.serializer.ObjectSerializer.lambda$serialize$0(ObjectSerializer.java:41)

The OptionalSerializer code looks like this, the NPE arrises at the delegate.serialize(optional.orElse(null), generator, context); according to me it can only occurs if optional is null which of course is valid (even if bad practice). I don't know if it's normal at this stage (there is a nullserializer that may catch this) but it definitly looks like a bug at yasson side.

    @SuppressWarnings("unchecked")
    @Override
    public void serialize(Object value, JsonGenerator generator, SerializationContextImpl context) {
        Optional<Object> optional = (Optional<Object>) value;
        delegate.serialize(optional.orElse(null), generator, context);

@loicmathieu
Copy link
Contributor

loicmathieu commented Aug 26, 2022

A simple test shows the same issue:

public class OptionalTest {
    @Test
    void optional_null() {
        Jsonb jsonb = JsonbBuilder.create();
        Pojo pojo = new Pojo();
        pojo.description = "description";
        String pojoJson = jsonb.toJson(pojo);
        System.out.println(pojoJson);
        Pojo deserialized = jsonb.fromJson(pojoJson, Pojo.class);
    }
}

This test works on main but fail on the jakarate branch. Seems like an issue on Yasson side.

@gsmet
Copy link
Member Author

gsmet commented Aug 30, 2022

@loicmathieu could you create a Yasson issue with a simple reproducer?

And in the meantime, I suppose we should disable the test entirely (and create an issue to track it and enable it again as soon as it's fixed on the Yasson side).
I think it's going to be easier to disable it in main rather than do some magic in the Jakarta transformation that we might forget about.

@gsmet
Copy link
Member Author

gsmet commented Aug 30, 2022

Maybe create 2 Maven projects: one with the old Yasson that works and the other with the new that fails.

@loicmathieu
Copy link
Contributor

OK, I'll work on it tomorrow.
I'll also create the PR to disable the test on main.

@gsmet
Copy link
Member Author

gsmet commented Aug 30, 2022

Thanks!

@loicmathieu
Copy link
Contributor

I opened and issue at Yasson side: eclipse-ee4j/yasson#575
It looks like it was already reported here: eclipse-ee4j/yasson#573

I openned an issue to disable the test for now: #27619

@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Aug 31, 2022
@loicmathieu
Copy link
Contributor

loicmathieu commented Sep 2, 2022

@gsmet there was issues on Yasson side to handle null Optional (seriaization and deserialization issue) but our test was not failing due to this (or it was not the only cause).
Apparently there is issue with Jakarta JSONB and Restassured.
If I change the test to use manual serialization it works see main...loicmathieu:quarkus:jsonb-restassures

Is Restassured compatible with Jakarata EE 9 ? I remember seeing some lines to configure Restassured to use JSONB but I didn't find them.

@loicmathieu loicmathieu reopened this Sep 2, 2022
@gsmet
Copy link
Member Author

gsmet commented Sep 2, 2022

No, it's not compatible but what is weird is that I usually had big errors about missing classes (that's why I switched most of the testing to Jackson). Not something weird like this.
If what you did fixes the test then let's do it this way.

@loicmathieu
Copy link
Contributor

OK, I'll open a PR on main with manually serializing the Pojo so we can put this behing us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment