jsr367-experts@jsonb-spec.java.net

[jsr367-experts] Re: [jsonb-spec users] Re: Re: Re: Re: [17-Customizing names]

From: Eugen Cepoi <cepoi.eugen_at_gmail.com>
Date: Mon, 1 Jun 2015 17:08:50 +0200

2015-06-01 16:38 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:

>
> Le 1 juin 2015 02:56, "Eugen Cepoi" <cepoi.eugen_at_gmail.com> a écrit :
> >
> >
> >
> > 2015-06-01 2:55 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
> >>
> >> Hi
> >>
> >> mainly agree with Eugen but added few comments inline.
> >>
> >>
> >> Few more comments:
> >> - JsonbProperty has nillable so it should get transient IMO (or
> >> nillable should be removed)
> >> - JsonbCreator misses parameter (like ConstructorProperties)
> >
> >
> > I was hesitant about this one too, but we already have JsonbProperty. It
> could be used on the ctr parameters.
> >
>
> Hmm then metadata (nillable) should be there to avoid potential conflicts.
> We could of course validate it but not needing it would make the API better.
>
>
> >> >> General
> >> >> Instead of having all options as strings, I think it would be better
> to
> >> >> provide them as implementations.
> >> >> The advantages of doing so:
> >> >> - The spec provides stronger guarantees on the default behaviour
> >> >> - This avoids having the same code being rewritten over and over by
> >> >> implementations
> >> >> - Makes the spec more useful
> >> >>
> >>
> >> Using strings have the advantage to make it config friendly so I guess
> >> we would need both.
> >>
> >> About having defaults or not I think it shouldn't be in the spec since
> >> it is up to the implementations (even if some code canbe trivial).
> >
> >
> > But what value is there in implementing those inside the implementations
> of the spec?
> > Assuming we would all end up with very similar code.
> >
>
> Who knows? Plus better to not put logic in the API to let it be switchable.
>
IMO this ok in that case, the logic is very isolated and achieves specific
goals. Having some default impl. be part of the spec doesn't prevent
modularization. People can still implement their own strategies and
register them (same for implementors).
Lets see what others think.

> >>
> >>
> >> >> This is something that can be done once and for all.
> >> >>
> >> >>
> >> >> Adapter
> >> >> The methods in Adapter could be called adapt or adaptFrom/adaptTo.
> >> >>
> >>
> >> why not using Java8 naming: apply?
> >>
> >> >> Why not use type parameters for Adapter, this is much nicer. The end
> user
> >> >> will have to do a cast inside his ser/de methods,
> >> >> impl ~5 methods just to provide some dummy ser/de? IMO that's way
> too
> >> >> much.
> >> >>
> >> >> Something like:
> >> >>
> >> >> interface Adapter<BoundType, ValueType> {
> >> >> ValueType adaptTo(BoundyType obj);
> >> >> BoundType adaptFrom(ValueType json);
> >> >> }
> >> >>
> >>
> >> +1, even wonder if splitting both sides wouldn't makes sense. I'm
> >> thinking to a rest service (server only) where you don't care of read
> >> side. So would be @JsonbJavaTypeAdapter(reader = MyReader.class,
> >> writer = MyWriter.class) (or from/to or any other naming convention).
> >
> >
> > In theory yes in practice I find that separating those is not necessary
> (but yes it gives more flexibility). People can throw an
> UnsupportedOperationException from the method they don't want to implement
> and voila.
> >
>
> Yes and no. Using java 8 defaults can help surely.
>
> Wdyt?
>
>
Using defaults for both methods could be fine. I like having an error when
I implement some interface so I can generate the methods skeleton with my
IDE, but maybe it would work with default impls. too.

> >>
> >>
> >> >>
> >> >> PropertyVisibilityStrategy
> >> >> Member does not provide access to annotations, handling things like
> >> >> @JsonTransient requires access to those.
> >> >>
> >>
> >> javax.json.bind.config.PropertyVisibilityStrategy#isVisible: agree we
> >> should have annotations but type as well. Is it expected to be casted?
> >> Can't we add a bit like in CDI an abstraction for that? Would be
> >> Property { type, annotations, declaringClass, name, member }. Wonder
> >> if synthetic makes sense or if we skip them before calling the method.
> >
> >
> > This would be one option. But it is a bit like the TypeToken story, we
> end up with clones all around...
> > The advantage of a property class I see is to merge the field/get/set
> into a single property and then use this property all around.
> > The other solution could be to have separate methods using field/method.
> >
>
> Not real clones here, we are quite more specific and having an
> abstraction will avoid to impl twice the same logic.
>
Mh, I don't know. I wouldn't be surprised by an API that exposes access to
a field/method and requires me to do some checks on it (esp. as those have
been around for long time and people got used to it?). Depending on some of
the points I mentioned the impl. could be different for a method and a
field (check if it is a set/get?).

Having a property would be fine of course (we all do that anyway), just
feels to me a bit like the typetoken story...something that is lacking in
the jdk.

> >>
> >>
> >> >>
> >> >> PropertyOrderStrategy
> >> >> Reflection gives perhaps to much details/constraints. ANY could
> perhaps be
> >> >> better = we don't provide any ordering guarantee, don't rely on it.
> >> >> Is it called with the "renamed" name or the originally resolved one?
> >> >> Only the class might not be enough information. How would one match
> the
> >> >> name to some property if he needs an annotation from it or whatever
> else?
> >> >> But anyway, except of providing a consistent ordering I don't think
> there
> >> >> is much value in making ordering customizable.
> >> >>
> >>
> >> +1
> >>
> >> If we go with a "Property" API we should replace the Class here by
> >> something close to be able to change metadata easily just wrapping a
> >> strategy.
> >>
> >> >>
> >> >> I'll have a look at the rest later.
> >> >>
> >> >> Eugen
> >> >>
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> 2015-05-30 13:12 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
> >> >>>
> >> >>> Hi Experts,
> >> >>>
> >> >>> I have pushed initial version of custom mapping api specification.
> >> >>> Javadocs will be improved asap.
> >> >>>
> >> >>> I am looking forward to receiving your feedback.
> >> >>>
> >> >>> Thanks,
> >> >>>
> >> >>> MartinV
> >> >>>
> >> >>> On Wed, May 13, 2015 at 6:21 PM, Martin Grebac <
> martin.grebac_at_oracle.com>
> >> >>> wrote:
> >> >>>>
> >> >>>> On 13.05.15 14:53, Martin Vojtek wrote:
> >> >>>>
> >> >>>> On Wed, May 13, 2015 at 1:34 PM, Eugen Cepoi <
> cepoi.eugen_at_gmail.com>
> >> >>>> wrote:
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>> 2015-05-13 8:40 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
> >> >>>>>>
> >> >>>>>> Hi Experts,
> >> >>>>>>
> >> >>>>>> we should get some conclusion to this discussion.
> >> >>>>>>
> >> >>>>>> I have the following notes:
> >> >>>>>>
> >> >>>>>> 1. Nillable, JsonbProperty, JsonbTransient
> >> >>>>>>
> >> >>>>>> I don't see any problem adding nillable attribute to
> JsonbProperty
> >> >>>>>> annotation. The situation with JsonbTransient is different,
> because
> >> >>>>>> transient field is effectively not a JsonbProperty. I think
> JsonbTransient
> >> >>>>>> should stay as is.
> >> >>>>>>
> >> >>>>>>
> >> >>>>>> Adding target Property to JsonbProperty makes sense.
> >> >>>>>>
> >> >>>>>> 2. Property Naming
> >> >>>>>>
> >> >>>>>>
> >> >>>>>> I see concept of Mappers equivalent to JsonbAdapter (analogy to
> >> >>>>>> XmlAdapter). It means that JsonbAdapter should see the context
> to be able to
> >> >>>>>> do complex mapping between type and JSON fragment.
> >> >>>>>
> >> >>>>>
> >> >>>>> What is the context to you? From memory there is no context in
> >> >>>>> XmlAdapter. Also I think it could make sense to allow users to
> register
> >> >>>>> Adapters directly to JsonB (using the builder or config) vs only
> via
> >> >>>>> annotations.
> >> >>>>
> >> >>>>
> >> >>>> Right now I don't know exactly what information JsonbAdapter
> needs, but
> >> >>>> the idea behind context is that JsonbAdapter should be able to
> override as
> >> >>>> much as possible behavior of default mapping. One option is to
> give access
> >> >>>> to JsonbConfig. It allows to change behavior when some property is
> set. The
> >> >>>> other option is to have access to information what has already been
> >> >>>> processed. For example if there is dependency between objects in
> JSON.
> >> >>>> Another interesting option is possibility to skip some part of
> json, or to
> >> >>>> change some behavior during processing of the JSON. But maybe
> there should
> >> >>>> be no context and to keep the concept of JsonbAdapter simple in
> version 1.0.
> >> >>>>
> >> >>>>>>
> >> >>>>>>
> >> >>>>>> Composition of Mappings looks like an advanced feature and maybe
> it
> >> >>>>>> should be postponed to future spec version.
> >> >>>>>
> >> >>>>>
> >> >>>>> By composition of mappings do you think of allowing users to
> provide an
> >> >>>>> Adapter/Mapper for some type that uses another registered
> adapters/mappers?
> >> >>>>>
> >> >>>>> For example I provide an PersonsAdapter<List<Person>> but inside I
> >> >>>>> don't want to deal with the mapping of each entry and want to
> delegate to
> >> >>>>> the default adapter of person.
> >> >>>>>
> >> >>>>>>
> >> >>>>>> WDYT?
> >> >>>>
> >> >>>> IMO we should keep it simple, unless there's obvious reason to not
> do
> >> >>>> so. I'd like to see clearer need for the context and more complex
> usecases
> >> >>>> here.
> >> >>>>
> >> >>>>>> 3. Property Order
> >> >>>>>>
> >> >>>>>> Case insensitive mapping is special, because it is not 1:1
> mapping. To
> >> >>>>>> make PropertyOrderStrategy simple, it should support only 1:1
> mapping. In
> >> >>>>>> that case PropertyOrderPolicy should not implement
> PropertyOrderStrategy. I
> >> >>>>>> am not sure if there is a need to introduce Accessor class. In
> this case
> >> >>>>>> simple String -> String mapping (accessor name -> JSON key)
> should work.
> >> >>>>>> Implementations could provide some internal mechanism to call
> this function
> >> >>>>>> only once per property.
> >> >>>>>
> >> >>>>>
> >> >>>>> I guess you are talking about naming and mean
> PropertyNamingStrategy
> >> >>>>> and PropertyNamingPolicy.
> >> >>>>>
> >> >>>> You are right, I was thining about different things at once and at
> the
> >> >>>> end I have messed it into one point ...
> >> >>>>
> >> >>>>>
> >> >>>>> Even if I like the names "strategy" and "policy" (who doesn't,
> this
> >> >>>>> sounds pretty cool :)) I find them confusing when used together.
> >> >>>>> I think both mean the same thing :)
> >> >>>>>
> >> >>>>
> >> >>>> I'm not that good in inventing names :) If you have better names,
> it is
> >> >>>> still possible to change that.
> >> >>>>
> >> >>>>>
> >> >>>>> To me there are two things with some common parts:
> >> >>>>> - Resolve names (using Java Bean convetion/Annotations/Other)
> >> >>>>> - Translate the string to another string
> >> >>>>> (lower/upper/dashed/underscore/blabla)
> >> >>>>>
> >> >>>>
> >> >>>> +1
> >> >>>>
> >> >>>>>
> >> >>>>> The first one could deal directly with "properties" where the
> second
> >> >>>>> one takes the resolved name and translates it.
> >> >>>>> So here we have the choice of:
> >> >>>>> - Have two separate concepts. One interface
> PropertyNameResolver that
> >> >>>>> deals with properties or some abstraction. And a
> >> >>>>> PropertyNamingStrategy/Policy interface that takes a string and
> returns
> >> >>>>> another string (and provides the default impls as static
> attributes of the
> >> >>>>> interface).
> >> >>>>> - Or we consider that the name resolution is relatively fixed
> and
> >> >>>>> handled inside the spec impl, thus we expose only the naming
> >> >>>>> strategy/policy. Though I think this too is probably even more
> fixed...
> >> >>>>> - Or we expose only the name resolution API
> >> >>>>>
> >> >>>>> No matter the choice, I'd be for providing default impls for all
> those
> >> >>>>> inside the spec. There aren't many ways to implement a name
> resolution from
> >> >>>>> java bean or annotations, neither to translate camel case to
> underscore...
> >> >>>>
> >> >>>>
> >> >>>> Agree with default impls + two separate concepts.
> >> >>>>
> >> >>>> My first comment applies here, too. If we're thinking the most of
> the
> >> >>>> need is going to be covered by the spec implementation, then is
> there really
> >> >>>> a need for the flexibility? What are the usecases which this would
> be
> >> >>>> covering?
> >> >>>>
> >> >>>> MartiNG
> >> >>>>
> >> >>>
> >> >>
> >> >
> >
> >
>