Lagom 1.6.5 silently performs only partial deserialization when using custom deserializers

[lapidus79]

Lagom Version (1.2.x / 1.3.x / etc)

1.6.5

API (Scala / Java / Neither / Both)

Java

Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)

MacOs Catalina 10.15.5

JDK (Oracle 1.8.0_112, OpenJDK 1.8.x, Azul Zing)

openjdk version "11.0.6" 2020-01-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.6+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.6+10, mixed mode)

Problem

• We have the RootEntityState class which we try to deserialize as illustrated below.
• Our class contains a child field, which class uses a custom deserializer.
• In lagom 1.6.5 this custom deserializer will result in all fields below child (i.e. updatedAt and updatedBy fields) being silently deserialized to null values
• In lagom 1.6.4 the deserialization worked just fine and updatedAt and updatedBy got set correct values.
• This issue is particularly dangerous since no error is thrown (unless we have for example lombok NonNull annotations, as in the example below, in which case an exception is thrown)

@Value
@Builder
@With
public class RootEntityState implements Jsonable {

    // this is deserialzed successfully, since above `child`
    @NonNull Long createdAt;
    // We have the child object that uses a custom deserializer
    Child child;
    // after the the custom deserializer we have other fields below.
    // These fields will resolve to NULL values in lagom 1.6.5.
    // Because we have @Nonnull annotations an exception
    // is thrown. Without the NonNull it would just silently set
    // the rest of the fields to null
    @NonNull Long updatedAt;
    @NonNull String updatedBy;

}

@Value
@Builder
@With
@JsonDeserialize(using = ChildDeserializer.class)
public class Child {

    @NonNull Long updatedAt;
    @NonNull String updatedBy;

}

public class ChildDeserializer extends StdDeserializer<Child> {

    public ChildDeserializer() {
        this(null);
    }

    public ChildDeserializer(Class<?> vc) {
        super(vc);
    }

    @Override
    public Child deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException {
        JsonNode node = jp.readValueAsTree();
        String updatedBy = node.get("updatedBy").asText();
        Long updatedAt = node.get("updatedAt").asLong();

        return Child.builder()
                .updatedAt(updatedAt)
                .updatedBy(updatedBy)
                .build();
    }
}

Reproducible Test Case

There is a fully reproducible test case here https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue

• It will display everything working on 1.6.4 and failing on 1.6.5.
• I also tried to run on 1.6.4 but so that jackson was overridden to 2.11.4, and it still worked on 1.6.4. Which leads be to believe it is not directly related to jackson.

To reproduce run RootEntityStateTest and test run will fail.

After that edit the project/plugins.sbt and set lagom version to 1.6.4.. Now reload and run test again and it will pass.

Also tried to debug 1.6.4 vs 1.6.5 and there is a a difference which might help:

https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue/blob/master/lagom164_breakpoint_in_deser.png
https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue/blob/master/lagom165_breakpoint_in_deser.png

https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue/blob/master/lagom_164_processing_continues_since_FIELD_NAME.png
https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue/blob/master/lagom165_where_processing_stops.png

[ignasi35]

Hi @lapidus79, at first I thought the issue was related to this line and akka/akka#29797. But even as I tuned the Visibility of the ObjectMapper to be as open and accepting as possible (ANY), your reproducer failed consistently.

As I debugged your reproducer a bit more, I noticed the error was trigger after parsing child and trying to parse updatedAt from RootEntityState (not the property in child with the same name). In a blind shot I found that the following workaround makes the test pass:

@Value
@Builder
@With
public class RootEntityState implements Jsonable {

    @NonNull Long createdAt;
// don't put the child class with a custom serializer halfway on the list of fields
    // Child child;  
    @NonNull Long updatedAt;
    @NonNull String updatedBy;

	// Putting the child class that uses a custom serializer at 
    // the end prevents the issue
    Child child;
}

And I don't know what is going on.

[metsavir]

Maybe the issue is related to this: FasterXML/jackson-databind#2904.

[lapidus79]

@ignasi35 yes, for some reason the cursor does not seem to move in the same way in 1.6.5. The problem with setting the class with the custom deserializer last is that will work only if you have exactly one such field in the root class. In case you have

• List(Child) etc
• any other classes that use a custom deserializer
• your child class resides for example down the entity tree. For example RootEntityState.somefield.child

=> then things will fall apart

Could this issue also affect for example migrations 🤔?

The FasterXML/jackson-databind#2904 issue does really look like similar to this. The thing that perplexes me is that I tried to override jackson deps to 2.11.4 in lagom 1.6.4 and it worked. It is possible that is somehow have messed up the override commands and that maybe jackson 2.10.4 was still used?. It would be cool if someone who knows how to override in sbt could verify that it was indeed 2.11.4 used and not 2.10.4. In this way we would at least be sure that the statement below not lead us astray:

I also tried to run on 1.6.4 but so that jackson was overridden to 2.11.4, and it still worked on 1.6.4. Which leads be to believe it is not directly related to jackson.

[lapidus79]

The problem occurs in play.utils.JsonNodeDeserializer.scala

Lagom 1.6.4 will use fasterxml JsonNodeDeserializer.java
Lagom 1.6.5 will use JsonNodeDeserializer.scala

I pushed a new commit to https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue that disables the JsonNodeDeserializer.scala when starting the server. After this

• RootEntityStateTest will still fail because this test doesn't read the application.conf
• JacksontestServiceTest succeeds since I've now disabled the JsonNodeDeserializer.scala at https://github.com/lapidus79/lagom-1-6-5-custom-deserializer-issue/blob/master/jacksontest-impl/src/main/resources/application.conf#L9-L14

It seems that the TreeTraversingParser state is different after calling JsonNodeDeserializer.deserialize(jp: JsonParser, ctxt: DeserializationContext): vs the equivalent fasterxml impl.

[ignasi35]

@lapidus79 I've created a unit test in the Play codebase testing the custom JsonNodeDeserializer but it is not failing.

I think the problem is a combination of JsonNodeDeserializer with other features specific to Lagom but I'm a bit lost for ideas. Any finding you make would be helpful.

Edit: the reading to JSONNode works as expected, the binding fails. So, yeah, the error is on Play's custom JSONNodeDeserializer

[ignasi35]

@lapidus79 I put together a fix but while it's not released there's also a workaround you could use in your code. In ChildDeserializer, instead of:

    JsonNode node = jp.readValueAsTree();

You should use:

    JsonNode node = JsonNodeDeserializer.getDeserializer(ObjectNode.class).deserialize(jp, ctxt);

[lapidus79]

Brilliant @ignasi35! You are the GOAT 🙇‍♂️

[ignasi35]

Credit goes to @jrudolph 😅