users@jsonb-spec.java.net

[jsonb-spec users] Re: JUG UA feedback

From: Przemyslaw Bielicki <pbielicki_at_gmail.com>
Date: Mon, 6 Jul 2015 09:40:58 +0200

I think that adapters would be a good place to start but still, if you
specify custom date pattern, how do you expect the receiver to understand
your message? If you both agree on the format, it's fine but you should not
assume that, I suppose?
Unless you say: "look, this String field will be a date in yyyyMMdd
format". As long as you use String field to do this I don't have any
objections, but if you define a field as Date (or whatever else other than
String) I think that the format should be defined and non modifiable to
guarantee compatibility with ALL possible receivers.

WDYT?



On Mon, Jul 6, 2015 at 9:34 AM, Oleg Tsal-Tsalko <oleg.tsalko_at_gmail.com>
wrote:

> 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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>