You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by Alessandro Moscatelli <al...@LIVE.COM> on 2018/03/26 18:08:47 UTC
PULL 17
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
R: R: PULL 17
Posted by Alessandro Moscatelli <al...@live.com>.
*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<ma...@live.com>
Inviato: martedì 27 marzo 2018 12:04
A: dev@johnzon.apache.org<ma...@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<ma...@gmail.com>
Inviato: martedì 27 marzo 2018 11:44
A: dev@johnzon.apache.org<ma...@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 <rm...@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<ma...@gmail.com>
> Inviato: lunedì 26 marzo 2018 20:43
> A: dev@johnzon.apache.org<ma...@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
> >
>
Re: PULL 20
Posted by Romain Manni-Bucau <rm...@gmail.com>.
yes, the two blocks at
https://github.com/apache/johnzon/pull/20/files#diff-7f654235e6be2d22d4eb2df20cd40b19R153
and
https://github.com/apache/johnzon/pull/20/files#diff-7f654235e6be2d22d4eb2df20cd40b19R297
concretely we want to bypass the bounds access once we already got them
once because it goes down to byte[] parsing and it slows down artificially
the runtime.
Same for class.isEnum() which leads to a native (and we can cache that
aggressively in the ClassMapping) - that said there is not only the one you
added here when rechecking ;).
If you think it is ok to keep it as it - can be since my comments are from
past experience but was on pre java 8 tests, you can add a bench as a unit
test in a small project which shows we dont go slower (we can accept a
range of 5% of error) for the old and new code paths. It is always the best
way to solve the perf questions ;).
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-04-03 17:43 GMT+02:00 Alessandro Moscatelli <
alessandro.moscatelli@live.com>:
> By “find” do you mean “findObjectConverterReader” ?
>
>
>
> If so, they are called only in 2 very specific cases, and in these cases
> en exception was throwed by :
>
>
>
> if (classMapping == null) {
>
> throw new MapperException("Can't map " + type);
>
> }
>
>
>
> To be accurate I am speaking about :
>
> CASE1) classMapping == null AND ParameterizedType.class.isInstance(type)
> AND fieldArgTypes.length <= 1
> CASE2) classMapping == null AND !ParameterizedType.class.isInstance(type)
> AND TypeVariable.class.isInstance(type)
>
>
>
> These 2 cases ALWAYS ended with errors before my commit.
>
>
> Are you sure you want have a switch for these ?
> Anyway the switch is fine for me.
>
>
>
> Thank you
> AM
>
>
>
> ________________________________
> Da: Romain Manni-Bucau <rm...@gmail.com>
> Inviato: Tuesday, April 3, 2018 5:03:16 PM
> A: dev@johnzon.apache.org
> Oggetto: Re: PULL 20
>
> Hi Alessandro,
>
> Is it possible to reduce all the finds you do at runtime? If not we can
> need a flag to switch if off by default since it will slow down the
> processing in most cases for no gain (for these cases).
> Overall idea is to do all the reflection we can once only since types can
> be cached per mapper/jsonb instance.
>
> Can you check it out to avoid a perf regression please?
>
> otherwise, other parts looks pretty ready to merge.
>
>
> 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-04-03 16:45 GMT+02:00 Alessandro Moscatelli <
> alessandro.moscatelli@live.com>:
>
> > Hi there,
> > this is my third and final (for now) pull.
> > I made some minor fixes to enable borderline scenarios like
> > deserialization of enum from deserializationContext.
> > This jsonb interface can only be access inside jsonb deserializers, so
> > these scenarios maybe are not so common or so much tested.
> > I also made a Test specific for this.
> >
> > I also pushed a small fix for milli/micro/nano seconds being non
> > considerated/truncated and I made a test for this too.
> >
> > Please let me know as soon as you can if there are any problems.
> > Thank you
> > AM
> >
> > Da: Alessandro Moscatelli<ma...@live.com>
> > Inviato: venerdì 30 marzo 2018 16:30
> > A: dev@johnzon.apache.org<ma...@johnzon.apache.org>
> > Oggetto: PULL 19 - complete support for DateFormat
> >
> >
> > Hi there,
> > here is my second pull.
> > It’s not a refactor, I simply extended the already present implementation
> > to provide support and apply formatting for more Date like types
> > (serialization and deserialization) :
> >
> > Date
> > LocalDateTime
> > LocalDate
> > OffsetDateTime
> > ZonedDateTime
> > Calendar
> > GregorianCalendar
> > Instant
> >
> > I also noticed that DateFormat were not used for serialized, in any case
> > (default value was Always used, and this was a blocking bug for me).
> >
> > After this I have a final pull.
> > Please let me know as soon as you can if there are any problems.
> > Greetings
> > AM
> >
> >
>
R: PULL 20
Posted by Alessandro Moscatelli <al...@live.com>.
By “find” do you mean “findObjectConverterReader” ?
If so, they are called only in 2 very specific cases, and in these cases en exception was throwed by :
if (classMapping == null) {
throw new MapperException("Can't map " + type);
}
To be accurate I am speaking about :
CASE1) classMapping == null AND ParameterizedType.class.isInstance(type) AND fieldArgTypes.length <= 1
CASE2) classMapping == null AND !ParameterizedType.class.isInstance(type) AND TypeVariable.class.isInstance(type)
These 2 cases ALWAYS ended with errors before my commit.
Are you sure you want have a switch for these ?
Anyway the switch is fine for me.
Thank you
AM
________________________________
Da: Romain Manni-Bucau <rm...@gmail.com>
Inviato: Tuesday, April 3, 2018 5:03:16 PM
A: dev@johnzon.apache.org
Oggetto: Re: PULL 20
Hi Alessandro,
Is it possible to reduce all the finds you do at runtime? If not we can
need a flag to switch if off by default since it will slow down the
processing in most cases for no gain (for these cases).
Overall idea is to do all the reflection we can once only since types can
be cached per mapper/jsonb instance.
Can you check it out to avoid a perf regression please?
otherwise, other parts looks pretty ready to merge.
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-04-03 16:45 GMT+02:00 Alessandro Moscatelli <
alessandro.moscatelli@live.com>:
> Hi there,
> this is my third and final (for now) pull.
> I made some minor fixes to enable borderline scenarios like
> deserialization of enum from deserializationContext.
> This jsonb interface can only be access inside jsonb deserializers, so
> these scenarios maybe are not so common or so much tested.
> I also made a Test specific for this.
>
> I also pushed a small fix for milli/micro/nano seconds being non
> considerated/truncated and I made a test for this too.
>
> Please let me know as soon as you can if there are any problems.
> Thank you
> AM
>
> Da: Alessandro Moscatelli<ma...@live.com>
> Inviato: venerdì 30 marzo 2018 16:30
> A: dev@johnzon.apache.org<ma...@johnzon.apache.org>
> Oggetto: PULL 19 - complete support for DateFormat
>
>
> Hi there,
> here is my second pull.
> It’s not a refactor, I simply extended the already present implementation
> to provide support and apply formatting for more Date like types
> (serialization and deserialization) :
>
> Date
> LocalDateTime
> LocalDate
> OffsetDateTime
> ZonedDateTime
> Calendar
> GregorianCalendar
> Instant
>
> I also noticed that DateFormat were not used for serialized, in any case
> (default value was Always used, and this was a blocking bug for me).
>
> After this I have a final pull.
> Please let me know as soon as you can if there are any problems.
> Greetings
> AM
>
>
Re: PULL 20
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi Alessandro,
Is it possible to reduce all the finds you do at runtime? If not we can
need a flag to switch if off by default since it will slow down the
processing in most cases for no gain (for these cases).
Overall idea is to do all the reflection we can once only since types can
be cached per mapper/jsonb instance.
Can you check it out to avoid a perf regression please?
otherwise, other parts looks pretty ready to merge.
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-04-03 16:45 GMT+02:00 Alessandro Moscatelli <
alessandro.moscatelli@live.com>:
> Hi there,
> this is my third and final (for now) pull.
> I made some minor fixes to enable borderline scenarios like
> deserialization of enum from deserializationContext.
> This jsonb interface can only be access inside jsonb deserializers, so
> these scenarios maybe are not so common or so much tested.
> I also made a Test specific for this.
>
> I also pushed a small fix for milli/micro/nano seconds being non
> considerated/truncated and I made a test for this too.
>
> Please let me know as soon as you can if there are any problems.
> Thank you
> AM
>
> Da: Alessandro Moscatelli<ma...@live.com>
> Inviato: venerdì 30 marzo 2018 16:30
> A: dev@johnzon.apache.org<ma...@johnzon.apache.org>
> Oggetto: PULL 19 - complete support for DateFormat
>
>
> Hi there,
> here is my second pull.
> It’s not a refactor, I simply extended the already present implementation
> to provide support and apply formatting for more Date like types
> (serialization and deserialization) :
>
> Date
> LocalDateTime
> LocalDate
> OffsetDateTime
> ZonedDateTime
> Calendar
> GregorianCalendar
> Instant
>
> I also noticed that DateFormat were not used for serialized, in any case
> (default value was Always used, and this was a blocking bug for me).
>
> After this I have a final pull.
> Please let me know as soon as you can if there are any problems.
> Greetings
> AM
>
>
PULL 20
Posted by Alessandro Moscatelli <al...@live.com>.
Hi there,
this is my third and final (for now) pull.
I made some minor fixes to enable borderline scenarios like deserialization of enum from deserializationContext.
This jsonb interface can only be access inside jsonb deserializers, so these scenarios maybe are not so common or so much tested.
I also made a Test specific for this.
I also pushed a small fix for milli/micro/nano seconds being non considerated/truncated and I made a test for this too.
Please let me know as soon as you can if there are any problems.
Thank you
AM
Da: Alessandro Moscatelli<ma...@live.com>
Inviato: venerdì 30 marzo 2018 16:30
A: dev@johnzon.apache.org<ma...@johnzon.apache.org>
Oggetto: PULL 19 - complete support for DateFormat
Hi there,
here is my second pull.
It’s not a refactor, I simply extended the already present implementation to provide support and apply formatting for more Date like types (serialization and deserialization) :
Date
LocalDateTime
LocalDate
OffsetDateTime
ZonedDateTime
Calendar
GregorianCalendar
Instant
I also noticed that DateFormat were not used for serialized, in any case (default value was Always used, and this was a blocking bug for me).
After this I have a final pull.
Please let me know as soon as you can if there are any problems.
Greetings
AM
Re: PULL 19 - complete support for DateFormat
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Applied with the ticket https://issues.apache.org/jira/browse/JOHNZON-165
thanks (again) ;)
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-30 16:30 GMT+02:00 Alessandro Moscatelli <
alessandro.moscatelli@live.com>:
>
> Hi there,
> here is my second pull.
> It’s not a refactor, I simply extended the already present implementation
> to provide support and apply formatting for more Date like types
> (serialization and deserialization) :
>
> Date
> LocalDateTime
> LocalDate
> OffsetDateTime
> ZonedDateTime
> Calendar
> GregorianCalendar
> Instant
>
> I also noticed that DateFormat were not used for serialized, in any case
> (default value was Always used, and this was a blocking bug for me).
>
> After this I have a final pull.
> Please let me know as soon as you can if there are any problems.
> Greetings
> AM
>
PULL 19 - complete support for DateFormat
Posted by Alessandro Moscatelli <al...@live.com>.
Hi there,
here is my second pull.
It’s not a refactor, I simply extended the already present implementation to provide support and apply formatting for more Date like types (serialization and deserialization) :
Date
LocalDateTime
LocalDate
OffsetDateTime
ZonedDateTime
Calendar
GregorianCalendar
Instant
I also noticed that DateFormat were not used for serialized, in any case (default value was Always used, and this was a blocking bug for me).
After this I have a final pull.
Please let me know as soon as you can if there are any problems.
Greetings
AM
Re: R: PULL 17
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <
alessandro.moscatelli@live.com>:
> 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<ma...@gmail.com>
> Inviato: martedì 27 marzo 2018 11:44
> A: dev@johnzon.apache.org<ma...@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/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 <
> 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 <rm...@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<ma...@gmail.com>
> > Inviato: lunedì 26 marzo 2018 20:43
> > A: dev@johnzon.apache.org<ma...@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
> > >
> >
>
>
R: R: PULL 17
Posted by Alessandro Moscatelli <al...@live.com>.
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<ma...@gmail.com>
Inviato: martedì 27 marzo 2018 11:44
A: dev@johnzon.apache.org<ma...@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 <rm...@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<ma...@gmail.com>
> Inviato: lunedì 26 marzo 2018 20:43
> A: dev@johnzon.apache.org<ma...@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
> >
>
Re: R: PULL 17
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <rm...@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<ma...@gmail.com>
> Inviato: lunedì 26 marzo 2018 20:43
> A: dev@johnzon.apache.org<ma...@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
> >
>
R: R: PULL 17
Posted by Alessandro Moscatelli <al...@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 <rm...@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<ma...@gmail.com>
Inviato: lunedì 26 marzo 2018 20:43
A: dev@johnzon.apache.org<ma...@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
>
Re: R: PULL 17
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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<ma...@gmail.com>
Inviato: lunedì 26 marzo 2018 20:43
A: dev@johnzon.apache.org<ma...@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
>
R: PULL 17
Posted by Alessandro Moscatelli <al...@live.com>.
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
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 ?
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 ?
4) Here you lost me … completely … what ? “^^
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
AM
Da: Romain Manni-Bucau<ma...@gmail.com>
Inviato: lunedì 26 marzo 2018 20:43
A: dev@johnzon.apache.org<ma...@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
>
Re: PULL 17
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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
>