Deserializing JSON with missing field fails using @JsonbCreator

[emattheis]

When I attempt to deserialize a JSON document with missing fields as a Java type using @JsonbCreator I get the following exception:

javax.json.bind.JsonbException: JsonbCreator parameter PARAM_NAME is missing in json document.

Looking into the source, I found this which indicates that the error condition is mandated by the spec. However, I can't find anything in the spec that requires all parameters in a creator to be present. Am I missing it?

Here's a test class that illustrates the issue:

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

import javax.json.bind.Jsonb;
import javax.json.bind.annotation.JsonbCreator;
import javax.json.bind.annotation.JsonbProperty;

import org.eclipse.yasson.internal.JsonBindingBuilder;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class JsonbCreatorTest {
    public static class Mutable {
        private String test = "default";

        public String getTest() {
            return test;
        }

        public void setTest(String test) {
            this.test = test;
        }
    }

    public static class Immutable {
        private final String test;

        @JsonbCreator
        public Immutable(@JsonbProperty("test") String test) {
            this.test = test;
        }

        public String getTest() {
            return test;
        }
    }

    private static final String OBJECT_WITH_FIELD = "{\"test\":\"test\"}";

    private static final String EMPTY_OBJECT = "{}";

    private Jsonb jsonb;

    @BeforeEach
    public void createJsonBuilder() {
        jsonb = new JsonBindingBuilder().build();
    }

    @Test
    public void testDeserializeMutableFromObjectWithField() {
        Mutable mutable = jsonb.fromJson(OBJECT_WITH_FIELD, Mutable.class);
        assertEquals("test", mutable.getTest());
    }

    @Test
    public void testDeserializeImmutableFromObjectWithField() {
        Immutable immutable = jsonb.fromJson(OBJECT_WITH_FIELD, Immutable.class);
        assertEquals("test", immutable.getTest());
    }

    @Test
    public void testDeserializeMutableFromEmptyOBject() {
        Mutable mutable = jsonb.fromJson(EMPTY_OBJECT, Mutable.class);
        // spec says the setter MUST NOT be called when the property is absent
        assertEquals("default", mutable.getTest());
    }

    @Test
    public void testDeserializeImmutableFromEmptyOBject() {
        Immutable immutable = jsonb.fromJson(EMPTY_OBJECT, Immutable.class);
        // spec does not seem to mention how to handle this - null seems like the only sane choice
        assertNull(immutable.getTest());
    }
}

Ultimately, I think this needs to be clarified in the spec, so I will raise it there as well, but I would like a way to get null passed in to creators when fields are missing instead of exceptions. I don't want to have to resort to mutable POJOs just to support this use case.

[bravehorsie]

Its section 4.5

In case a field required for a parameter mapping doesn’t exist in JSON document, JsonbException MUST be thrown.

The complicated thing with this example is that Yasson will never know the field is missing intentionally or not. When parsing "{\"test\":null}", JsonParser encounters a test field for a constructor or setter and Yasson propagates the null value. However parsing "{}" there is no callback into Yasson for property to be set. In case of @JsonbCreator Yasson can be "smart" and use null because it has a constructor to call but there are no values for it, but what if properties are missed in JSON unintentionally?

[emattheis]

I followed the link from the JSON-B website which leads here. Now I realize that is the public review draft. 🙄 Anyway, I found the final spec and I see the reference.

This is disappointing since it basically means you cannot deserialize to a truly immutable type if your JSON documents have optional fields which is quite common in my experience, but I understand Yasson's position on spec-compliance, so I'll raise the issue in the json-b api project.

[misl]

Wouldn't it still be compliant with JSON-B specification if only Optional<> JsonbCreator arguments were allowed to be missing? In this case missing none Optional<> arguments would still throw the MessageKeys.JSONB_CREATOR_MISSING_PROPERTY error.

I did something similar in #276. I only noticed how similar both solutions were after I was done.

[m0mus]

@aguibert @Verdent Can you folks review it. I am also interested in is TCK will pass with this PR merged.

[aguibert]

@m0mus before we merge this PR or the similar PR that @misl proposed, we need to address this at the JSON-B spec level over at https://github.com/eclipse-ee4j/jsonb-api/issues/121 Lets focus our efforts on that JSON-B spec issue and then we can do the appropriate impl in Yasson.

[m0mus]

We discussed it with @Verdent today. We both agree that the issue exists and it makes sense to fix it. The problem is that it goes against the spec which explicitly restricts this behavior. Thanks @aguibert for creating a task in JSONB spec issues tracker. We will address it when we will start working on a new version of the spec. It doesn't mean that it cannot be fixed in Yasson now. We can add a Yasson specific configuration property called for example ALLOW_JSONB_CREATOR_OPTIONAL_PARAMS. By default this property is not set which makes Yasson 100% spec compliant. If this property is set JsonbCreator behavior is switched to what's been suggested above. Setting configuration property is easy:

JsonbConfig config = new JsonbConfig()
    .setProperty("ALLOW_JSONB_CREATOR_OPTIONAL_PARAMS", true);

@aguibert @emattheis what do you think? If you agree please change a PR.

[emattheis]

Personally, my interest in Yasson is limited to its role as the JSON-B reference implementation, so I am happy to wait until this becomes specified behavior. If I can't write portable code, then I'm going to use a more mature JSON framework anyway.

I don't necessarily object to making this a configurable behavior of Yasson, and I imagine it won't be difficult to add, but I think it dilutes the value of the JSON-B standard when all the implementations support a common feature in a non-standard way. It would be interesting to know if the TCK specifically enforces spec-compliance in this case. If it doesn't, perhaps a minor change to the spec could be fast-tracked somehow?

[aguibert]

@m0mus I hesitate to add a yasson-specific switch to enable non-spec compliant behavior for the reasons that @emattheis outlined and also because it creates more long-term code maintenance for Yasson. Especially if JSON-B decides to solve this in a completely different way, then we would have to support both ways in Yasson and hope they don't conflict.

We can't fast-track a change to the JSON-B spec unfortuantely. I think this would be a must-have feature for the next version of JSON-B though. Dmitry, do you know if we are allowed to start working on JSON-B v1.1 now? Or do we have to wait until JakartaEE 8 finalizes before we can do any work at all?

[m0mus]

@emattheis in this case we should close this issue. Andy already added a task to JSONB issues tracker. There is no need to keep it open.

@aguibert Sure, we can start working on JSONB 1.1 now in master branch. It's fine to define a scope and feature set and start working on updating APIs. We cannot release a final version without going through the spec approval process (but we can release non-final versions such as betas, RCs, etc.). It's also unclear about javax->jakarta namespace conversion at the moment. I guess we can change all packages to jakarta.* now and start thinking about how we implement it in Yasson.

[oliviercailloux]

The corresponding issue is marked as solved. I am sure that many developers are eager to use this feature. Is Yasson going to implement this soon?

@aguibert wrote, on 29 July 2019: “IMO optional constructor properties are one of the top 3 most needed features for JSON-B (…), my preference is to wait until this is defined at the spec level (which will hopefully be soon).”

[Verdent]

@oliviercailloux sure thing. I am already having working implementation of this feature in my local branch. We will try to get this branch to the master soon. This feature will be released as soon as the new JSONB is released.

[mkarg]

What is the current status of this?