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:51:52 +0300

yeah, but in terms of Java API we have date/time patterns we will be using
during ser/deser anyway

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

> Actually, there is no date standard for JSON... so what I just wrote is
> not that useful.
>
> Is there any ISO standardization on going to standardize date in JSON?
> Otherwise, it's just a String :)
>
> On Mon, Jul 6, 2015 at 9:40 AM, Przemyslaw Bielicki <pbielicki_at_gmail.com>
> wrote:
>
>> 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
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>