Hi Alessandro, comment on github directly. but high level except the "type" issue I mentionned and the formatting it looks good.
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://rmannibucau.metawerx.net/> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book <https://www.packtpub.com/application-development/java-ee-8-high-performance> 2018-03-27 12:04 GMT+02:00 Alessandro Moscatelli < [email protected]>: > Ok I just made a new pull request (#18) containing only support for > injectable JsonbConfig via JAX-RS Provider. > I didn’t use Priorities (so it will work on older provider), I didn’t > create a new library (so it will work out of the box) but I used Priority > to set a value lower than User default one (4900, so that provider created > by user will be used over this by default). > I used geronimo dependencies as you said for point 1. > I fixed what you said regarding point 4 : > Now if context resolver is found, the type passed is always propagated. > If context resolver is not found, type passed is not used, since it was > not used before my change. > > I’ll commit next set of changes as soon as you accept this pull request. > > Please let me know as soon as possible. > > Thank you > AM > > Da: Romain Manni-Bucau<mailto:[email protected]> > Inviato: martedì 27 marzo 2018 11:44 > A: [email protected]<mailto:[email protected]> > Oggetto: Re: R: PULL 17 > > We can do as for our "mapper" jaxrs provider and support another > constructor. > For the priority I wonder why the default/implicit priority doesn't work? > > > Romain Manni-Bucau > @rmannibucau <https://twitter.com/rmannibucau> | Blog > <https://rmannibucau.metawerx.net/> | Old Blog > <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibuca > u> | > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > <https://www.packtpub.com/application-development/java-ee-8- > high-performance> > > 2018-03-27 11:09 GMT+02:00 Alessandro Moscatelli < > [email protected]>: > > > I use Wildfly 12. > > I am going to make a small pull request with Injectable JsonBconfig a few > > minutes from now … > > > > Greetings > > AM > > > > > > > > ________________________________ > > Da: Romain Manni-Bucau <[email protected]> > > Inviato: Monday, March 26, 2018 11:21:20 PM > > A: [email protected] > > Oggetto: Re: R: PULL 17 > > > > Le 26 mars 2018 23:13, "Alessandro Moscatelli" < > > [email protected]> a écrit : > > > > I am not sure I got all you said but let’s divide et impera : > > > > 1) I really tried (I have/had no idea what geronimo dep were but I > figured > > out javaee dependencies were inside them) but at first I didn’t find the > > geronimo dep containing Priority annotation. Now I got it : > > https://mvnrepository.com/artifact/org.apache.geronimo. > > specs/geronimo-annotation_1.3_spec/1.0 > > > > > > Exactly > > > > > > > > 2) I simply didn’t want to reinvent the wheel, but … oh well I am going > to > > find another implementation able to recursively explore superclasses and > > superinterfaces to find the generics type … are maven dependencies ok ? > > > > > > We really try to limit dependencies at maximum. Shading can be an option > is > > size is very small (commons has thus kind of code IIRC) or just do it > from > > scratch maybe, it is not that long i think. > > > > > > 3) I think the best is creating an external library. Jsonb is not really > > related to jax-rs, so maybe a jsonb-jaxrs library is the best option. Is > > this ok for you ? > > > > > > Was in jsonb cause 1. We sere not java 8 in jaxrs module and 2. It must > > work ootb in a jaxrs container. That said we can probably disable it > > somehow. Why container do you use? > > > > > > 4) Here you lost me … completely … what ? “^^ > > > > > > You pass type in the resolver but dont respect this at runtime (let say > > first call uses Book and init jsonb and next one has User) > > > > > > > > 5) Oh … ok I’ll try to make some test for the new supported scenarios … > > > > 6) I’ll cancel the current pull Tomorrow and try with smaller individual > > ones. > > > > > > > > Thanks a LOT for 5 and 6. I appreciate it a lot. > > > > > > Thanks > > AM > > > > Da: Romain Manni-Bucau<mailto:[email protected]> > > Inviato: lunedì 26 marzo 2018 20:43 > > A: [email protected]<mailto:[email protected]> > > Oggetto: Re: PULL 17 > > > > Hi > > > > Alessandro, some general comments: > > > > 1. Please use geronimo spec dependencies (instead of javax) since asf > owns > > it (less legal work + we can tune them if needed) > > 2. Dont use google deps (we dont want any dep for container control on > > dependencies) > > 3. Priority should work on javaee 6 containers so maybe make it optional > or > > dont use priorities (annotation are ignored if missing but not classes > > iirc) > > 4. Your jsonb type key doesnt work since the instance is for all matching > > type no? > > 5. I dont see much tests ;) > > 6. Surely split in as much pr as change to ease the merge and review > > > > Hope it helps, impatient to get it in and congrats for your PR :) > > > > > > Le 26 mars 2018 20:08, "Alessandro Moscatelli" < > > [email protected]> a écrit : > > > > > Hi everybody ! > > > I want to contribute and I made some minor fixes and I am trying to > pull > > > my changes with github. > > > > > > This is my changelist : > > > > > > JAX-RS MessageWriter/MessagerReader with Priority (so that user can > > define > > > and provide his own) > > > JsonbConfig injectable via Jax-RS API > > > Better support for generics types > > > Support for JsonbDeserializers/JsonbSerializers defined in interfaces > or > > > abstract classes > > > Support for default deserialization from string to enum > > > Fix to dateformatting (date format was not properly used in > > > deserialization) > > > > > > I hope you will appreciate … > > > > > > Have a nice evening ! > > > > > > Lemme know what you think, I’d really love to drop my snapshot > dependency > > > soon 😊 > > > > > > Alessandro Moscatelli > > > > > > >
