users@jsonb-spec.java.net

[jsonb-spec users] Re: JUG UA feedback

From: Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>
Date: Mon, 6 Jul 2015 10:34:13 +0300

Absolutely. I didn't mean passing any kind of meta info along with JSON.
I meant possibility to specify date/time pattern to be used during
marshaling/unmarshaling if necessary...

2015-07-06 10:26 GMT+03:00 Przemyslaw Bielicki <pbielicki_at_gmail.com>:

> Hi,
>
> Just my few cents regarding:
>
> > It really makes sense to let “Custom date format”/“Custom number format”
> to be specified globally via JsonbConfig. WDYT?
>
> I understand this requirement, we could even say that it would be nice to
> customize dates/numbers using locale, but...
> Please remember that JSON is a protocol that has to be understood the same
> way by the sender and the receiver that may be developed in completely
> different technologies. Having custom date and number format seems to
> violate this constraint as we would need to send configuration with which
> current message was generated. I think it's a bad idea.
>
> As long as customizations keep the message readable and legible by both
> endpoints without a need of any other information to be sent, it's OK.
>
> Cheers,
> Przemyslaw
>
> On Sun, Jul 5, 2015 at 8:23 PM, Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>
> wrote:
>
>> Hi Martin,
>>
>> Thnx for your reply and that you incorporated our feedback in spec and
>> code examples.
>> Couple more comments/questions from my side including main Olena's
>> proposal as I haven't seen any replies on her email:
>>
>> 1. [by @Olena] It really makes sense to let “Custom date
>> format”/“Custom number format” to be specified globally via JsonbConfig.
>> WDYT?
>>
>> Agree with all your comments in your reply and changes you've done except:
>>
>> 1. Why have we decided not to leave FIELDS DEFINITION ORDER DRIVEN
>> PropertyOrderStrategy? Is it difficult to achieve from impl point of view?
>> I think it's default behavior expected by users IMHO.
>> 2. Reg last example in DefaultMappingGenerics#toJson_generics() =>
>> jsonb.toJson(expected) will yield same result as
>> jsonb.toJson(expected,DefaultMappingGenerics.class.getField("listOfOptionalStringField").getGenericType()),
>> isn't it?
>>
>> SPEC REVIEW:
>>
>> 1. [POLISHING] "*The default behavior can be customized annotating a
>> given field (or JavaBean property)*" => Probably could be stated
>> better to cover type level annotations
>> 2. [JSB-4.3.1-2] Why have we decided to use diff annotations
>> (@JsonbNillable and @JsonbProperty) to customize NULL handling on type
>> level and on property level? It will be more clear to use @JsonbNillable
>> annotation everywhere. (And even if we do decide to use @JsonbProperty to
>> specify nillable fields, lets update javadoc for @JsonbProperty to reflect
>> NULL handling purpose)
>> 3. [POLISHING] [JSB-4.5-1] *Class can contain at most one mapped
>> property or field that is annotated with
>> javax.json.bind.annotation.JsonbValue,* otherwise JsonbException MUST
>> be thrown.
>> 4. [4.5] I would explicitly specify restriction on method signature
>> @JsonbValue annotation can be applied on otherwise JsonbException MUST be
>> thrown (e.g. public method without parameters)
>> 5. [TYPO] [4.6] "*JsonbCreator annotation can be used to annotation
>> custom constructor or static void factory method*" => JsonbCreator
>> annotation can be used to annotate custom constructor or static factory
>> method. It should be at most one @JsonbCreator annotation otherwise
>> JsonbException MUST be thrown.
>> 6. [4.6] I would add a NOTE that factory method annotated with @JsonbCreator
>> should return instance of particular class this annotation is used for,
>> otherwise JsonbException MUST be thrown. Also I would add a NOTE on how
>> JSON fields will be mapped on parameters of constructor/factory method
>> annotated with @JsonbCreator?
>> 7. [TYPO] [JSB-4.8-2] @JsonbJavaTypeAdapter has been renamed
>>
>> Also is there any discussion going on in private reg JsonbAdapter API,
>> cause I feel like it's hot topic?
>>
>> Thank you,
>> Oleg
>>
>> 2015-06-21 13:41 GMT+03:00 Olena Syrota <sirotae_at_gmail.com>:
>>
>>> 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
>>>>>
>>>>
>>>>
>>>
>>
>