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

Deserialization from null to JsonValue #181

Closed
leadpony opened this issue Aug 17, 2019 · 6 comments · Fixed by #273
Closed

Deserialization from null to JsonValue #181

leadpony opened this issue Aug 17, 2019 · 6 comments · Fixed by #273
Labels
enhancement New feature or request
Milestone

Comments

@leadpony
Copy link

According to the current spec, mapping JSON document

{
    "value": null
}

to the following Java class

public class Stuff {
    public JsonValue value;
}

assigns null to the value in the class instead of JsonValue.NULL.

On the other hand, the following JSON document

{
    "value": {
        "value2": null
    }
}

assigns a JSON object to the value and JsonValue.NULL is assigned to its value2 property.

I believe null in JSON should be converted to JsonValue.NULL if the target type is JsonValue, in the same way it is converted specially to Optional.empty() when the target type is Optional.

Please see also eclipse-ee4j/yasson#293

@aguibert
Copy link
Contributor

hi @leadpony, thanks for raising this issue on the spec. I agree it does make sense. My only concern would be backwards compatibility. However, I'm not sure if we'll be doing a 2.0 or a 1.1 release next so it might be OK.

@aguibert aguibert added the enhancement New feature or request label Aug 27, 2019
@m0mus
Copy link
Contributor

m0mus commented Aug 27, 2019

Technically you can assign null to your value field. Java allows it. Although it could not make sense some users may use it and there won't be any option to do it unless you skip the field in your Json.

@leadpony
Copy link
Author

leadpony commented Aug 27, 2019

Java allows for developers to assign null to any value of Optional<T> type. However the JSON-B API assigns Optional.empty() instead of null for it.
The most preferable value to be mapped from JSON null depends on the target type.
For Optional<T> it is Optional.empty() and for JsonValue it is JsonValue.NULL.
Many JSON-P users appreciate the existence of JsonValue.NULL in JSON-P spec, because it frees us from notorious NullPointerException.
It is OK for me that the API change will happen in 2.0 or later. We have a workaround to use setter method forcing to assign JsonValue.NULL when the passed value is null.

@rmannibucau
Copy link

In jsonb, jsonvalue should be a fallback when nothing matches - otherwise you use a deserializer (from jsonb if local) if you need json metadata and not only binding. So we shouldnt encourage it.

Now, the original json really has jsonvalue.null so it must be injected as such for jsonvalue type. However, an empty json would bind java null and not any jsonvalue instance to respect the binding.

Does it sound ok?

@leadpony
Copy link
Author

Yes.
If the input JSON is { }, Stuff.value should not be modified and be kept as initial null (of Java) in this case, which means the property does not exist in the JSON.
If the input JSON is { "value": null }, Stuff.value should be modified to JsonValue.NULL, which means the property actually exists in the JSON and its value is JSON null.

@m0mus
Copy link
Contributor

m0mus commented Aug 28, 2019

After some thinking I am fine to add to 2.0 spec feature set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants