users@jsonb-spec.java.net

[jsonb-spec users] Re: [jsr367-experts] Re: Re: [2-DefaultMapping] Proposal

From: Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>
Date: Wed, 8 Apr 2015 10:26:40 +0300

Hi Martin,

Not sure this differentiation brings much benefit and it will be tricky to
implement, cause naturally during serialization when you spot Optional type
you simply unwrap it and serialize it's value in the field and it can be
done via standard extension mechanisms like type adapters.
However in all cases (simple NULL, Optional.empty(), ofNullable(null))
actual value after unwrapping will be NULL, so wether to put an element to
JSON or not will depend on global configuration and behavior will be the
same for all three cases anyway.
Let me know if I'm missing smth.

Regards,
Oleg

2015-04-07 17:25 GMT+03:00 Martin Vojtek <voytoo_at_gmail.com>:

> Hi Oleg,
>
> from my point of view it is quite consistent. If optionalField is null,
> there is no element in JSON. If there is optionalField with
> Optional.empty() or ofNullable(null), then it is null in JSON.
>
> The idea is to make the difference between null field and Optional.empty().
>
> Thank you,
> MartinV
>
>
> On Tue, Apr 7, 2015 at 2:00 PM, Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>
> wrote:
>
>> Thnx Martin for your answers!
>>
>> Agree with Romain reg default behavior though.
>>
>> Forgot one more question:
>>
>> I noticed diff specified behavior for *marshalling* *of simple NULL
>> fields and NULL Optional fields in POJOs *according to examples.
>>
>> I know that it will be configurable how to serialize NULL POJO fields,
>> but if by default we are not adding element into JSON doc as per below:
>>
>> POJO pojo = new POJO();
>> pojo.id = 1;
>> pojo.name = null;
>> assertEquals("*{\"id\":1}*", jsonb.toJson(pojo));
>>
>> similarly in examples for serialization of empty Optional fields we
>> shouldn't add NULL fields at all:
>>
>> OptionalClass nullOptionalClass = new OptionalClass();
>> nullOptionalClass.optionalField = Optional.ofNullable(null);
>> assertEquals("*{}*", jsonb.toJson(nullOptionalClass));
>>
>> instead of current example:
>>
>> assertEquals("*{\"optionalField\":null}*",
>> jsonb.toJson(nullOptionalClass));
>>
>> otherwise simple POJO example should be as well:
>>
>> POJO pojo = new POJO();
>> pojo.id = 1;
>> pojo.name = null;
>> assertEquals("*{\"id\":1, **\"name\":null**}*", jsonb.toJson(pojo));
>>
>> The main point is that regardless of default behavior it should be
>> consistent/same for simple fields and Optional fields.
>> Handling of ser/deser fields of Optional type presumably will be done
>> using kind of TypeAdapters mechanism by diff JSONB providers and it will
>> be absolutely not straightforward to implement this kind of diff behaviour
>> for Optional nullable fields and nullable fields of other types...
>>
>> Thank you,
>> Oleg
>>
>> 2015-04-06 21:36 GMT+03:00 Romain Manni-Bucau <rmannibucau_at_tomitribe.com>
>> :
>>
>>> hi
>>>
>>> comments inline
>>>
>>> 2015-04-06 19:17 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
>>>
>>>> Hi Oleg,
>>>>
>>>> thank you very much for your feedback!
>>>>
>>>> Response is inlined below.
>>>>
>>>> MartinV
>>>>
>>>> On Sun, Apr 5, 2015 at 5:37 PM, Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com
>>>> > wrote:
>>>>
>>>>> Hi guys,
>>>>>
>>>>> More feedback on default mapping:
>>>>> 1) We have following examples for JAXP structures mapping:
>>>>>
>>>>> JsonStructure jsonStructure = jsonb.fromJson("{\"name\":\"unknown
>>>>> object\"}", JsonStructure.class);
>>>>>
>>>>> JsonObject jsonStructure = jsonb.fromJson("{\"name\":\"unknown
>>>>> object\"}", JsonObject.class);
>>>>>
>>>>> Does it makes sense to have following example:
>>>>>
>>>>> JsonObject jsonStructure =
>>>>> (JsonObject)jsonb.fromJson("{\"name\":\"unknown object\"}",
>>>>> JsonStructure.class);
>>>>>
>>>>> just to emphasize that in runtime correct JsonStructutre
>>>>> instantiated?
>>>>> Or it should be part of TCK rather then another example?
>>>>>
>>>>
>>>> Examples are just supporting discussion leading to specification. TCK
>>>> will be implemented based on specification, and examples (not included in
>>>> pdf) will be not part of that specification. The content of the
>>>> specification will be the single source of the truth.
>>>>
>>>> I this case, there is one sentence in the proposed specification:
>>>> Unmarshalling into supported javax.json.* objects/interfaces/fields MUST
>>>> have the same result as unmarshalling into such objects with
>>>> javax.json.JsonReader.
>>>>
>>>> You could (and it would be really great) propose new statements or
>>>> changes in the current statements.
>>>>
>>>>
>>>>> 2) I didn't find examples for deserializing JSON with missing fields
>>>>> into POJO which should result in null values I expect such as:
>>>>>
>>>>> POJO pojo = jsonb.fromJson("{\"id\":1}", POJO.class);
>>>>>
>>>>> assertEquals(1, pojo.id);
>>>>>
>>>>> assertNull(pojo.name);
>>>>>
>>>>>
>>>> There is one example in fromJson_nullValues(Jsonb jsonb)
>>>>
>>>> //java object
>>>> POJOWithInitialValue pojoWithInitialValue = jsonb.fromJson("{\"name\":\"newName\"}", POJOWithInitialValue.class);
>>>> assert(pojoWithInitialValue.id.intValue() == 4);
>>>> assertEquals("newName", pojoWithInitialValue.name);
>>>>
>>>> In specification, there is following statement:
>>>>
>>>> Unmarshalling operation of a property absent in JSON document MUST not set the value
>>>> of the field, setter (if available) MUST not be called, thus original value of the field MUST be preserved.
>>>>
>>>>
>>>> 3) I didn't find example that shows is it allowed to use POJOs with
>>>>> private fields without getters/setters?
>>>>>
>>>>
>>>> There is no such example.
>>>>
>>>> There is no restriction on private, so it is definitely supported
>>>> (maybe it should be specified explicitly):
>>>>
>>>> For unmarshalling operation for a Java property, if a matching
>>>> setter method exists, the method is called to set the value of the
>>>> property, otherwise direct field assignment is used.
>>>>
>>>> 4) Agree on restricting usage of static/final/transient fields and
>>>>> private/overloaded constructors for POJOs used for ser/deser JSON data,
>>>>> however disagree with not supporting deserialization of unknown fields.
>>>>> Why not ignore all not mapped content of JSON doc during
>>>>> deserialization rather then dictate strict one-to-one correspondence of
>>>>> JSON doc to POJO?
>>>>> It will give user much more flexibility where he/she wants to map only
>>>>> part of JSON doc he/she interested in.
>>>>>
>>>>
>>>> The proposed default mapping strategy is fail fast with strict (in one
>>>> way) correspondence. I intend to propose customization which can change
>>>> this behavior.
>>>>
>>>> It would be helpful to hear some other opinions on this topic.
>>>>
>>>>
>>> it is super common to map a sub tree of the whole JSon (check github
>>> rest endpoints for instance, you never use everything) so by default I
>>> think it should deserialize what it can. All missing mapping should be
>>> ignored. That is also what do (almost) all mappers - including jaxb - AFAIK.
>>>
>>>
>>>>
>>>>
>>>>> 5) [TYPO]: Function DefaultMapping#toJson_Anonymous_Class. Look like
>>>>> instead of:
>>>>>
>>>>> assertEquals("{\"id\":1,\"name\":\"pojoName\"}", new POJO() {...})
>>>>>
>>>>> it should be
>>>>>
>>>>> assertEquals("{\"id\":1,\"name\":\"pojoName\"}", jsonb.toJson(new
>>>>> POJO() {...}))
>>>>>
>>>>> 6) [TYPO]: DefaultMapping line 834:
>>>>>
>>>>> java.net.URI uri = new java.net.URI("mailto:users_at_jsonb-spec.java.net
>>>>> ");
>>>>>
>>>>> assertEquals("\"https://www.jcp.org/en/jsr/detail?id=367#3\"",
>>>>> jsonb.toJson(uri));
>>>>>
>>>>> 7) [TYPO]: DefaultMapping line 1034. Should be:
>>>>>
>>>>> assertEquals("{\"optionalField\":\"value\"}",
>>>>> jsonb.toJson(optionalClass));
>>>>>
>>>>> instead of
>>>>>
>>>>> assertEquals("{\"optionalField\":\"value\"}", optionalClass);
>>>>>
>>>>>
>>>>>
>>>> Typos will be fixed asap :)
>>>>
>>>>
>>>>> Have a great Easter!
>>>>>
>>>>> Thank you,
>>>>> Oleg
>>>>>
>>>>>
>>>>
>>>
>>
>