johnzon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alessandro Moscatelli <alessandro.moscate...@live.com>
Subject R: R: PULL 17
Date Tue, 27 Mar 2018 10:07:54 GMT
*I meant “so it will work on older container/application server”

I forgot to mention I also made a new test to check that Jsonb Provider is called indeed by
the MessageReader/MessageWriter Provider as you asked for point 5.

Da: Alessandro Moscatelli<mailto:alessandro.moscatelli@live.com>
Inviato: martedì 27 marzo 2018 12:04
A: dev@johnzon.apache.org<mailto:dev@johnzon.apache.org>
Oggetto: R: R: PULL 17

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:rmannibucau@gmail.com>
Inviato: martedì 27 marzo 2018 11:44
A: dev@johnzon.apache.org<mailto:dev@johnzon.apache.org>
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/rmannibucau> |
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 <
alessandro.moscatelli@live.com>:

> 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 <rmannibucau@gmail.com>
> Inviato: Monday, March 26, 2018 11:21:20 PM
> A: dev@johnzon.apache.org
> Oggetto: Re: R: PULL 17
>
> Le 26 mars 2018 23:13, "Alessandro Moscatelli" <
> alessandro.moscatelli@live.com> 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:rmannibucau@gmail.com>
> Inviato: lunedì 26 marzo 2018 20:43
> A: dev@johnzon.apache.org<mailto:dev@johnzon.apache.org>
> 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" <
> alessandro.moscatelli@live.com> 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
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message