Hi Olena,
1. In case of null Java fields, it is agreed that the value will be set to
null. Only if the value is absent, the setter will not be called. So to
align with this behavior it maybe makes sense to fail in such a case.
2. and 3. I see it could simplify things a little and maybe improve
performance (no check for time). Don't see any problem with more options to
set default behavior. There is already JsonbDateFormat annotation, which
changes format on more granular level. In case of builder-level it is not
so simple, because you need to specify what types you are customizing (only
Date? Calendar? LocalDate? ...).
What default format should JSON Binding specify? With time or without time?
And why? In some cases it makes sense to format date with time. In other
cases without time.
Thank you,
Martin
On Sun, Jun 21, 2015 at 12:41 PM, Olena Syrota <sirotae_at_gmail.com> wrote:
> Hi guys,
>
> More comments from JUG UA.
>
> 1. What do you think, is it worth to specify default behavior and
> customization for unmarshalling nulls to primitives value? Default behavior
> will be to ignore nulls as it is agreed for null Java fields in 3.14.1.
> Customization may be fail on null for primitive value.
>
> 2. [3.5.1. Marshalling] 1. Proposition to setup default format for
> Date|Calendar|GregorianCalendar marshalling instead of run-time desision on
> ISO_DATE|ISO_DATE_TIME format regarding of time presence in value. 2.
> Proposition to consider customization of marshalling date format on
> Builder-level (additionally to current cistomization for field)
>
> 3. [3.5.1. Unmarshalling] 1. Proposition to setup default format for
> Date|Calendar|GregorianCalendar unmarshaling instread of chosing
> corresponding format at run-time. 2. Proposition to consider customization
> on Builder-level (additionally to current customization for field).
>
> Thank you
> Olena
>
> 2015-06-20 18:49 GMT+03:00 Martin Vojtek <voytoo_at_gmail.com>:
>
>> Hi Oleg,
>>
>> it was quite a big list :)
>>
>> comments below.
>>
>> Thank you,
>> MartinV
>>
>> On Tue, Jun 2, 2015 at 11:30 PM, Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>
>> wrote:
>>
>>> Hi guys,
>>>
>>> It's been for a while since our last email, so we have quite a few
>>> outstanding comments/questions reg JSON-B spec.
>>>
>>> I decided to provide majority of them in one email so it might be easier
>>> for you to comment on them since all of you pretty much in context of
>>> recent changes in spec including 'Customizing Mapping'. Also I grouped our
>>> comments/questions by topics:
>>>
>>> Forgotten stuff:
>>>
>>> 1) [TYPO] Runtime "Use an explicit implementation of JsonbProvider"
>>> example should be changed to explicitly instantiate provider instead of
>>> builder itself I believe.
>>>
>>> DefaultMappingGenerics:
>>>
>>> 2) There are still no examples to show use cases for Jsonb#toJson(Object,
>>> Type) method to deal with particular special cases where otherwise it
>>> won't work
>>>
>>>
>> I think this is the last example in toJson method -
>> List<java.util.Optional<String>>.
>>
>>
>>> 3) [TYPO]
>>> assertEquals("{\"boundedSet\":[3],\"superList\":[{\"radius\":2.5}]}",
>>> jsonb.toJson(boundedGenericClass));
>>>
>>> missing area property of Shape super class I guess
>>>
>>>
>> Fixed.
>>
>>
>>> 4) In DefaultMappingGenerics examples we shouldn't use explicit asserts
>>> for LinkedHashMap default implementation of Map as we agreed to relax this
>>> constraint in spec as I remember
>>>
>>>
>> Fixed
>>
>>
>>> 5) In DefaultMappingGenerics examples for incompatible types it will be
>>> ClassCastException later on instead of JsonbException
>>>
>>> DefaultMappingDates:
>>>
>>
>> Fixed
>>
>>
>>> 6) No negative test case examples to throw JsonbException if JSON
>>> date/time is not in supported ISO format
>>>
>>>
>> Fixed
>>
>>
>>> SPEC:
>>>
>>> 7) Formally in 3.4.3 chapter it's not mentioned about ser/deser of
>>> Optional's as JsonValues, despite behavior is the same as for object
>>> instance properties
>>>
>>>
>> Fixed
>>
>>
>>> 8) [TYPO] in JSB-3.6-1 java.math.Integer—Long—BigDecimal
>>>
>>>
>> Fixed
>>
>>
>>> 9) In 'Scope and Field access strategy' 3.7.1 section nothing said
>>> about getters/setters without actual field as we have examples for it
>>>
>>>
>> Fixed
>>
>>
>>> 10) In 'Null Java field' 3.14.1 section it is not explicitly stated
>>> about unmarshalling of null values from JSON and there is no such case in
>>> examples by the way
>>>
>>>
>> Fixed
>>
>>
>>> 11) [TYPO] javax.json.bin. in several places in spec
>>>
>>>
>> Fixed
>>
>>
>>> 12) @JsonbPropertyOrder annotation usage is not mentioned in spec
>>>
>>>
>> Fixed
>>
>>
>>> 13) There is no word about @JsonbTransient annotation in spec
>>>
>>>
>> Fixed
>>
>>
>>> CustomizingMapping:
>>>
>>> 14) Why @Retention is missed on @JsonbTransient annotation?
>>>
>>>
>> Fixed
>>
>>
>>> 15) It is stated that @JsonbTransient is mutually exclusive with other
>>> annotations. What gonna happen if user specify contradictive annotation on
>>> field lets say?
>>>
>>>
>> Fixed
>>
>>
>>> 16) I suspect in CustomMapping#toJson_nillable() examples it should be
>>> 'null' instead of 'nill' values?
>>>
>>>
>> Fixed
>>
>>
>>> 17) I think JsonConfig#withSkippedNullValues should really be
>>> #withNullValues and there is no mentioning of default value in Javadoc
>>> which should be 'true' in this case because we are skipping null values by
>>> default?
>>>
>>>
>> Makes sense. Fixed.
>>
>>
>>> 18) There is no example of @JsonbProperty usage on Parameter level and
>>> I'm not sure it's required IMO. Only one valid usage I can think of is
>>> together with @JsonbCreator methods/constructors which hasn't been agreed
>>> yet.
>>>
>>>
>> Yes, it is meant of @JsonbCreator methods/constructors. We try to change
>> the process from discussion -> implementation to implementation proposal ->
>> discussion to help discussion about the topics.
>>
>>
>>> 19) Do we really feel strong need in @JsonbCreator functionality? If
>>> yes, it should probably be integrated in main default mapping spec chapters
>>> rather then simply in customization mapping chapter, cause it influence
>>> constraints for POJOs being deserialized. WDYT?
>>>
>>>
>> Default mapping is about no JSON Binding annotations present in the java
>> model. I think JsonbCreator could solve some problems in certain
>> situations. Personally can live without this functionality in version 1.0.
>>
>>
>>> 20) There are no examples of @JsonbValue usage which might be ambiguous
>>> IMO. Will be good to hear some good use cases justifying it? Also same
>>> effect could be achieved using TypeAdapters.
>>>
>>>
>> If there are some ambiguities, these should be fixed in SPEC. The
>> examples are just illustrative to support discussion about the topics. Not
>> providing examples is a good way how to force people to read SPEC IMO :)
>> Please could you specify what ambiguities do you see?
>>
>>
>>> 21) [TYPO] public identifiers from
>>> PropertyNamingStartegy/PropertyVisibilityStrategy interfaces should be
>>> removed to be consistent
>>>
>>>
>> Fixed
>>
>>
>>> 22) There are no examples of JsonbVisibilityStrategy usage and again I
>>> don't feel it's really needed. I would rather relax constraints for public
>>> class members in first place. However even if we do want to have it,
>>> probably it's better to define some meaningfull default strategies here if
>>> you can think of any?
>>>
>>>
>> There was long discussion about these constraints and I am happy not to
>> open it for some time :)
>>
>>
>>> 23) BinaryDataStrategy could be an enum?
>>>
>>>
>> I have switched from enums to Strings. The reason is that enum is not an
>> interface and extending enum for another option could break some (bad
>> written) existing code and it is quite easy to live with strings. It will
>> be easier to add another option in the future.
>>
>>
>>> 24) In PropertyOrderStrategy I would rename REFLECTION constant to
>>> FIELDS and update corresponding javadocs to not rely on some particular
>>> reflection API method as stated in javadocs currently
>>>
>>>
>> There was discussion in EG and I plan to rename REFLECTION to ANY.
>>
>>
>>> 25) Don't we want to introduce smh like JSONB_STRICT_DESERIALIZATION
>>> config param to handle missing JSON properties differently than default
>>> skip strategy and corresponding @JsonbStrictDeserialization annotation on
>>> TYPE level?
>>>
>>>
>> According to RFC 7493 https://tools.ietf.org/html/rfc7493 tt is
>> recommended to skip missing properties. Strict deserialization could be
>> addressed by some preprocessor and there was no strong agreement in EG to
>> support strict deserialization. Maybe it could be add in some later JSON
>> Binding version.
>>
>>
>>> 26) JSONB_EVENT_HANDLER could be removed in JsonbConfig class
>>>
>>>
>> Fixed
>>
>>
>>>
>>> Thank you,
>>> Oleg
>>>
>>
>>
>