2015-06-06 10:10 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
> > Don't know exactly what is missing. Can you please specify it further?
> > Parameters of constructors and methods could be annotation with e.g.
> JsonbProperty.
> >
>
> Property annotation can then be duplicated and conflicting. I wouldnt
> allow it here.
>
IMO the parameter names should be resolved without the user having to do
anything. In such an usage defining the name for each parameter would be
the exception. The most frequent one would be to just "rename" a single
property, in which case JsonbProperty provides greater flexibility. But
those are only suppositions :)
>>
> >> - Can't date and number format annotations be merged? seems it is not
> >> ambiguous for me and simplifies the API.
> >>
> >
> > If we add "long timeInMillis", then these annotations will be different.
> >
>
> True but not sure we should do that.
>
Not sure if we should add an option indicating that the date should be
serialized as a long, why not?
> >>
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau
> >> http://www.tomitribe.com
> >> http://rmannibucau.wordpress.com
> >> https://github.com/rmannibucau
> >>
> >>
> >> 2015-05-31 21:55 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
> >> > Some other comments.
> >> >
> >> > JsonbCreator
> >> > The javadoc could be improved to make it clear what the contract is.
> >> > Only one constructor or static factory method can be annotated with
> it, the
> >> > marked ctr/method will be used to create instances of that object. A
> note
> >> > about ctr/method with args would be nice too (are impls supposed to
> work
> >> > with it or not?).
> >> >
> >> > JsonbDateFormat
> >> > Maybe add an option to ser the property as a timestamp in millis?
> >> > If you don't, then there shouldn't be a default value for "value".
> >>
> >> not sure for timestamp option, using a plain long would make it quite
> >> easy as well
> >>
> >> >
> >> > JsonbJavaTypeAdapter
> >> > Why not only JsonbTypeAdapter.
> >> >
> >> > JsonbValue
> >> > Will this single value be "unwrapped" if it is a class that would
> serialized
> >> > as a json object?
> >> > It might be worth adding an example in the javadoc.
> >> >
> >> > JsonbConfig
> >> > It is maybe not necessary to prefix all the config constants with
> JSONB.
> >> > Though it has the advantage to avoid namespace conflicts with static
> >> > imports.
> >>
> >> since it is part of the API directly I think it is not a big deal.
> >>
> >> Side note: why JSONB_STRICT_IJSON, doesn't seem to match the convention?
> >>
> >> > WithAdapters overrides the previous result, this should be documented
> or be
> >> > merged with previous value (as a user it wouldn't be obivous to me,
> I'd tend
> >> > to think that it merges the adapters).
> >> >
> >>
> >> JsonbConfig#withAdapters should merge adapters (you can still use
> >> setProperty if you want to erase previous ones or we can even add a
> >> real setter).
> >>
> >> Side note: having setters and not "builder methods" (withXX) would
> >> make it easy to integrate with any common IoC config IMO.
> >>
> >> > And again providing some default impls with the spec would be nice
> >> > (adapters, configs etc).
> >> >
> >> > Eugen
> >> >
> >> >
> >> > 2015-05-31 16:07 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
> >> >>
> >> >> Hey Martin,
> >> >>
> >> >> I didn't review all the code yet, but here are my first remarks.
> >> >>
> >> >> 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).
> >>
> >> >> 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).
> >>
> >> >>
> >> >> 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.
> >>
> >> >>
> >> >> 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
> >> >>>>
> >> >>>
> >> >>
> >> >
> >
> >
>