users@jsonb-spec.java.net

[jsonb-spec users] [jsr367-experts] Re: Re: Re: [33-I-JSON Compatibility] Proposal

From: Romain MB <rmannibucau_at_tomitribe.com>
Date: Fri, 24 Apr 2015 20:59:04 +0200

2015-04-24 20:54 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
> I think that the result of the discussion is as follows:
>
> 1. support of string values for big numbers by default
> 2. support the override of properties by default
> 3. fail fast when non-unique property (after override and rename) is found
> 4. define default algorithm for override (based on java mechanism) and check
> uniqueness of properties (not affecting performance)
> 5. Do not enable I-JSON by default due to the following recommendations
> - support only root objects/arrays by default
> - serialize binary data to base64 by default
> 6. There will be static final constant in Jsonb to define I-JSON
> compatibility mode
>
> WDYT? Could we agree on that or is there some point(s) we should discuss
> further?
>

+1 on the defaults, only question for me is: should we propose I-JSON
at all finally? I mean we did our best for the defaults, should we go
further (ie having this constant + forcing providers to implement it)?
Thinking to it I guess we could just keep the defaults we deduced from
this study and skip for the 1.0 I-JSON full compatibility.

> Thanks,
> MartinV
>
> On Mon, Apr 20, 2015 at 11:24 PM, Romain MB <rmannibucau_at_tomitribe.com>
> wrote:
>>
>> 2015-04-20 23:11 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>> >
>> >
>> > 2015-04-20 22:22 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
>> >>
>> >> 2015-04-20 20:35 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>> >> >
>> >> >
>> >> > 2015-04-20 19:40 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
>> >> >>
>> >> >> I understand, that's where we don't see it the exact same way. You
>> >> >> merge it by name where I would merge it by java model with standard
>> >> >> order. So basically I'd do a reduce (java) then validation (json
>> >> >> side
>> >> >> if we can call it this way).
>> >> >
>> >> >
>> >> > Ah ok, so you would only merge by the name of the field or method.
>> >> > This is viable but would force to make a distinction between resolved
>> >> > name
>> >> > using java bean convention and other naming strategies (annotation,
>> >> > custom
>> >> > user strategy etc). For Genson I prefered to have only one name
>> >> > resolution
>> >> > system and use it always the same way, there are less "rules"
>> >> > involved
>> >> > making things easier to understand.
>> >> > But it can have some advantages to make the distinction...
>> >> >
>> >>
>> >> yes basically all is resolved on java side then we can check
>> >> @JsonProperty (supposed it is the right name) to check the name to
>> >> use, if not present use the one deduced from java. Merge is less
>> >> aggressive and allows few validations.
>> >
>> >
>> > So by java side resolution you mean we use the same strategy as java?
>> > This means that fields/methods from class A overrides what matches in
>> > its
>> > super class.
>> > In short:
>> > 1) reduce accessors/mutators by name and definition order in class
>> > hierarchy
>> > 2) apply any "renaming" strategy on the result of 1)
>> > 3) reduce the result of 2) using the same strategy as in 1)
>> >
>> > If yes then I think we can simplify things by directly applying the
>> > renaming
>> > and then doing a single "reduce" that overrides or throws an exception.
>> >
>>
>> well while we have this reduce phase it is fine. Surely an
>> implementation detail then.
>>
>> > Allowing to override has some advantages, for example you can extend a
>> > class
>> > of which you don't have control and redefine how some property would be
>> > ser/de - a quick solution vs implementing a full custom ser/de for that
>> > enclosing type.
>> >
>> > Would be interesting to know what others think.
>> >
>>
>> +1
>>
>> >>
>> >> >>
>> >> >> Doesnt impact performances.
>> >> >
>> >> >
>> >> > True, doing it on the resolved java model (with renamed or not
>> >> > properties)
>> >> > doesn't change the perfs.
>> >> >
>> >> >>
>> >> >>
>> >> >> Now the real question is: do we gain anything compared to what you
>> >> >> propose ie ensuring we don't have conflicts. I still think it can
>> >> >> make
>> >> >> sense to validate there isn't a "model conflict". Implicit merging
>> >> >> can
>> >> >> be hard to debug from my experience.
>> >> >
>> >> >
>> >> > So you would throw an exception for example if there are two
>> >> > properties
>> >> > with
>> >> > the same name in a class and its parent?
>> >> > If yes then we should make sure of the "rules" applied as there might
>> >> > be
>> >> > ambiguous situations. How do you think we should distinguish valid
>> >> > from
>> >> > invalid conflicts for renamed properties?
>> >> >
>> >>
>> >> We can have rules like "an overriden method is not a conflict if a
>> >> field is not hiding parent one" etc but we can also do like in bean
>> >> validation and define inheritance API. A bit more verbose but clearer
>> >> IMO (+ does it really happen often in real life?). Something like
>> >> @OverrideJsonAttribute("parentName").
>> >>
>> >
>> > Having a specific API for overriding looks overkill, I'd prefer just to
>> > agree on a good default behaviour.
>> >
>>
>> Let's try - agree on your statement. I guess releasing a 1.0 without
>> such API would be good and we can still add it in 1.1 if needed/asked.
>>
>> >>
>> >> >>
>> >> >> That said if we ensure the
>> >> >> framework logs it properly (level = warn + logger name =
>> >> >> javax.json.something) then it would work for me as well.
>> >> >
>> >> >
>> >> > I would prefer to just throw an exception or consider it as valid
>> >> > rather
>> >> > than log a warn.
>> >> >
>> >>
>> >> Works, just want it to be visible and not too implicit.
>> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >> Romain Manni-Bucau
>> >> >> @rmannibucau
>> >> >> http://www.tomitribe.com
>> >> >> http://rmannibucau.wordpress.com
>> >> >> https://github.com/rmannibucau
>> >> >>
>> >> >>
>> >> >> 2015-04-20 19:34 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>> >> >> >
>> >> >> >
>> >> >> > 2015-04-20 19:28 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
>> >> >> >>
>> >> >> >> well not supporting the validation for custom serialization code
>> >> >> >> doesnt sound like a big issue for me but it doesn't prevent to
>> >> >> >> validate the model. I didn't speak of the merging but for
>> >> >> >> instance 2
>> >> >> >> different methods/fields decorated with
>> >> >> >> @JsonProperty(name="foo").
>> >> >> >
>> >> >> >
>> >> >> > Yes I was including renamed properties in that and by merging I
>> >> >> > was
>> >> >> > meaning
>> >> >> > take N properties (field/method/ctr) and reduce it to one (how you
>> >> >> > achieve
>> >> >> > it is up to the impl).
>> >> >> > To me renamed or original name is the same, as I first resolve the
>> >> >> > names
>> >> >> > and
>> >> >> > then do all the selection/merging stuff.
>> >> >> > Amongst others, this allows to override in subclasses things from
>> >> >> > parent
>> >> >> > classes - by using the same name.
>> >> >> >
>> >> >> >>
>> >> >> >>
>> >> >> >> For me just doing it already matches the intention behind I-JSON.
>> >> >> >>
>> >> >> >> Romain Manni-Bucau
>> >> >> >> @rmannibucau
>> >> >> >> http://www.tomitribe.com
>> >> >> >> http://rmannibucau.wordpress.com
>> >> >> >> https://github.com/rmannibucau
>> >> >> >>
>> >> >> >>
>> >> >> >> 2015-04-20 19:24 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>> >> >> >> > I think doing it on the model is something else as basically
>> >> >> >> > you
>> >> >> >> > already
>> >> >> >> > merge detected properties, so you end up with only one.
>> >> >> >> > At least that's what I do in Genson, I give preference to the
>> >> >> >> > detected
>> >> >> >> > properties (methods/fields/whatever else) by hierarchy (from
>> >> >> >> > most
>> >> >> >> > concrete
>> >> >> >> > to the "highest" parent). And the compiler provides already
>> >> >> >> > some
>> >> >> >> > safeguards.
>> >> >> >> >
>> >> >> >> > Still all that doesn't solve the problem of colliding
>> >> >> >> > properties
>> >> >> >> > being
>> >> >> >> > written by the user (via some custom ser. code).
>> >> >> >> > But well... we can't always prevent people from doing stupid
>> >> >> >> > things
>> >> >> >> > :)
>> >> >> >> >
>> >> >> >> > 2015-04-20 19:11 GMT+02:00 Romain MB
>> >> >> >> > <rmannibucau_at_tomitribe.com>:
>> >> >> >> >>
>> >> >> >> >> 2015-04-20 19:05 GMT+02:00 Eugen Cepoi
>> >> >> >> >> <cepoi.eugen_at_gmail.com>:
>> >> >> >> >> >
>> >> >> >> >> > 2015-04-19 19:44 GMT+02:00 Romain MB
>> >> >> >> >> > <rmannibucau_at_tomitribe.com>:
>> >> >> >> >> >>
>> >> >> >> >> >> Hi Martin
>> >> >> >> >> >>
>> >> >> >> >> >> for me it makes sense to support it by default with a flag
>> >> >> >> >> >> like
>> >> >> >> >> >> the
>> >> >> >> >> >> one you proposed to switch it off (a bit off topic but do
>> >> >> >> >> >> we
>> >> >> >> >> >> have
>> >> >> >> >> >> constants for these flags, something like
>> >> >> >> >> >> Jsonb.I_JSON_COMPLIANCE?).
>> >> >> >> >> >>
>> >> >> >> >> >> UTF8 by default is quite mandatory IMO, must-ignore as well
>> >> >> >> >> >> (already
>> >> >> >> >> >> discussed IIRC). ISO8601 is the standard for dates of most
>> >> >> >> >> >> of
>> >> >> >> >> >> frameworks, big numbers need to be string whatever we do so
>> >> >> >> >> >> finally
>> >> >> >> >> >> open points from my window are:
>> >> >> >> >> >>
>> >> >> >> >> >> - byte data as base64: on johnzon we discussed it and I
>> >> >> >> >> >> didn't
>> >> >> >> >> >> want
>> >> >> >> >> >> it
>> >> >> >> >> >> cause it was far from Java - that said it is not a super
>> >> >> >> >> >> common
>> >> >> >> >> >> type
>> >> >> >> >> >> so not sure it is that important
>> >> >> >> >> >> - top level construct: don't recall the spec but I think
>> >> >> >> >> >> primitives
>> >> >> >> >> >> (string, numbers) can be supported as well. Jackson does it
>> >> >> >> >> >> at
>> >> >> >> >> >> least
>> >> >> >> >> >> so +1 to not support this fail fast behavior
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > I don't think there is much value in being too restrictive,
>> >> >> >> >> > +1
>> >> >> >> >> > for
>> >> >> >> >> > it.
>> >> >> >> >> > I don't remember any discussion about accepting to deser.
>> >> >> >> >> > numbers
>> >> >> >> >> > as
>> >> >> >> >> > strings, strings as numbers etc when possible, but it would
>> >> >> >> >> > be
>> >> >> >> >> > nice.
>> >> >> >> >> >
>> >> >> >> >> >>
>> >> >> >> >> >> - no members with duplicate name: the only point I don't
>> >> >> >> >> >> know.
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > This one is painful as it has an impact on performances
>> >> >> >> >> > (esp.
>> >> >> >> >> > if
>> >> >> >> >> > we
>> >> >> >> >> > want
>> >> >> >> >> > to
>> >> >> >> >> > prevent user custom code to generate such output)... at the
>> >> >> >> >> > moment
>> >> >> >> >> > I
>> >> >> >> >> > don't
>> >> >> >> >> > support it in Genson for this reason.
>> >> >> >> >> >
>> >> >> >> >>
>> >> >> >> >> Can't we do it on the model (ie once for the whole application
>> >> >> >> >> lifecycle)? I guess here we are lucky compared to JSON-P.
>> >> >> >> >>
>> >> >> >> >> >>
>> >> >> >> >> >> Intellij Idea makes this kind of JON invalid. This is
>> >> >> >> >> >> something
>> >> >> >> >> >> we
>> >> >> >> >> >> would need to be able to switch off but I think it makes
>> >> >> >> >> >> sense
>> >> >> >> >> >> to
>> >> >> >> >> >> have
>> >> >> >> >> >> it by default to detect wrong models.
>> >> >> >> >> >>
>> >> >> >> >> >>
>> >> >> >> >> >> Romain Manni-Bucau
>> >> >> >> >> >> @rmannibucau
>> >> >> >> >> >> http://www.tomitribe.com
>> >> >> >> >> >> http://rmannibucau.wordpress.com
>> >> >> >> >> >> https://github.com/rmannibucau
>> >> >> >> >> >>
>> >> >> >> >> >>
>> >> >> >> >> >> 2015-04-18 19:53 GMT+02:00 Martin Vojtek
>> >> >> >> >> >> <voytoo_at_gmail.com>:
>> >> >> >> >> >> > Hi Experts,
>> >> >> >> >> >> >
>> >> >> >> >> >> >
>> >> >> >> >> >> > JSON-B should provide support or some level of compliance
>> >> >> >> >> >> > with
>> >> >> >> >> >> > I-JSON
>> >> >> >> >> >> > specification.
>> >> >> >> >> >> >
>> >> >> >> >> >> > https://tools.ietf.org/html/rfc7493
>> >> >> >> >> >> >
>> >> >> >> >> >> > There are several things to discuss.
>> >> >> >> >> >> >
>> >> >> >> >> >> > Should JSON-B support I-JSON by default? My proposal is
>> >> >> >> >> >> > to
>> >> >> >> >> >> > not
>> >> >> >> >> >> > support
>> >> >> >> >> >> > all
>> >> >> >> >> >> > the recommendations of I-JSON by default.
>> >> >> >> >> >> >
>> >> >> >> >> >> > If we agree on that, what specific parts of I-JSON should
>> >> >> >> >> >> > JSON-B
>> >> >> >> >> >> > provide
>> >> >> >> >> >> > by
>> >> >> >> >> >> > default?
>> >> >> >> >> >> >
>> >> >> >> >> >> >
>> >> >> >> >> >> > Proposal of parts (of I-JSON) to support by JSON-B by
>> >> >> >> >> >> > default:
>> >> >> >> >> >> >
>> >> >> >> >> >> > Should be supported by default:
>> >> >> >> >> >> > - Encoding and Characters (UTF-8 by default)
>> >> >> >> >> >> > - Object constraints (no members with duplicate name)
>> >> >> >> >> >> > - MUST-IGNORE policy - partial mapping
>> >> >> >> >> >> > - Time and Date Handling - serialize accoring to I-JSON
>> >> >> >> >> >> >
>> >> >> >> >> >> > Should not be supported by default:
>> >> >> >> >> >> > - Numbers - serialize big number(s) into string (and
>> >> >> >> >> >> > deser
>> >> >> >> >> >> > given
>> >> >> >> >> >> > strings
>> >> >> >> >> >> > into number)
>> >> >> >> >> >> > - Top-Level Constructs - fail fast when ser/deser
>> >> >> >> >> >> > something
>> >> >> >> >> >> > different
>> >> >> >> >> >> > than
>> >> >> >> >> >> > object/array as top-level JSON
>> >> >> >> >> >> > - binary data encoded as a string in base64url
>> >> >> >> >> >> >
>> >> >> >> >> >> >
>> >> >> >> >> >> > To support I-JSON fully in some case, I propose to
>> >> >> >> >> >> > provide
>> >> >> >> >> >> > I-JSON
>> >> >> >> >> >> > compatibility mode available via config property.
>> >> >> >> >> >> >
>> >> >> >> >> >> > JsonbConfig config = new
>> >> >> >> >> >> > JsonbConfig().setProperty("jsonb.i-json.compliance",
>> >> >> >> >> >> > true);
>> >> >> >> >> >> > Jsonb jsonb = JsonbBuilder.create(config);
>> >> >> >> >> >> >
>> >> >> >> >> >> > Looking forward to your feedback.
>> >> >> >> >> >> >
>> >> >> >> >> >> > MartinV
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >
>> >> >> >> >
>> >> >> >
>> >> >> >
>> >> >
>> >> >
>> >
>> >
>
>