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? 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?.
>> >>
>> >> - 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
>> >>
>> >>
>> >> 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
>> >> >>>>
>> >> >>>
>> >> >>
>> >> >
>> >
>> >
>
>