2015-06-06 12:39 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
> 2015-06-06 11:32 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
> >
> >
> > 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 :)
> >
>
> Do you expect to use bytecode to extract it?
Yes
> I dont think it would be
> good since then users and tools would have a hard time reusing it + do
> we want a bytecode lib for sthg like jsonb?.
>
I think the opposite, users are happy if you are able to support non
default ctr without additional annotations.
Java 8 supports by default parameter name resolution (though it just uses
the same technique as most libs do - via bytecode and debug symbols).
Of course people should be able to disable this feature.
>
> >> >>
> >> >> - 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?
> >>
>
> cause this is no more the format plus I think sticking to long would
> make the model simplerand more understandable
>
I don't understand. What I am suggesting is: let people indicate if they
want the date to be serialized as a long or as a string using some
dateformat.
>
> >> >>
> >> >>
> >> >> 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
> >> >> >>>>
> >> >> >>>
> >> >> >>
> >> >> >
> >> >
> >> >
> >
> >
>