2015-05-13 10:52 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
>
>
> On Wed, May 13, 2015 at 8:56 AM, Romain MB <rmannibucau_at_tomitribe.com>
> wrote:
>>
>> 2015-05-13 8:44 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
>> > I have the following notes to the event handling (alternatively custom
>> > error
>> > handling):
>> >
>> > +1 for enum for severities
>> >
>> > Makes sense to align with JsonLocation and get rid of URI.
>> >
>> > There is no list of events yet. I am not sure if we need it.
>> >
>> > The reason behing custom error handling is to allow to turn off some
>> > fail-fast errors.
>> >
>> > The Optional is stressing that given method is not required to return
>> > value.
>> >
>>
>> Sure but does it bring anything compared to the -1 of json-p? (just
>> trying to stay consistent accross specs)
>>
>
> json-p was designed to be SE6 compatible, json-b minimal requirement is SE8.
> The benefit is the explicity. Personally don't have problem with -1, but
> also don't see strong reason why not use Optional in this case.
>
>> > I prefer to return boolean from handle method and not to introduce
>> > handled()
>> > method into Event.
>> >
>> > I am not that perfectly happy getting rid of event handling mechanism. I
>> > think this is still needed to support even simple usecases where default
>> > is
>> > too restrictive.
>> >
>>
>> Is there any major mapper supporting it in their API (googled quickly
>> and didnt find but can have missed it)?
>>
>
> I am not aware of such mapper, but the idea is taken from JAX-B and
> generalized little. The benefit is the ability to consume events (e.g.
> handle error event and effectively override fail-fast behavior) and the
> possibility to integrate nicely with other specifications, e.g. Bean
> Validation. It is also possible for implementations to introduce additional
> events without changing the specification.
>
I got the idea but my guess is we don't need it for a 1.0. If we
integrate with bean validation it would surely be after the mapping
and not during IMO (to work on an object).
My point was that if no implementation supports it maybe it is not
needed. I prefer to need to enrich the API in a coming versions than
putting a lot of things in a 1.0 which are useless in practise (also
allows to be more adapted to the coming use cases).
>>
>> > MartinV
>> >
>> > On Wed, Apr 29, 2015 at 11:37 PM, Eugen Cepoi <cepoi.eugen_at_gmail.com>
>> > wrote:
>> >>
>> >> Ahh I didn't read correctly your comment :) I thought you where saying
>> >> "add a handled method to the event handler" not the event... my fault.
>> >> The event can be immutable, lets keep it this way. Returning true/false
>> >> looks like a common pattern to indicate success/failure or
>> >> hanlded/unhandled. But if we were to drop this error handling system
>> >> I'd be
>> >> fine too...not sure if people do really need it.
>> >>
>> >> 2015-04-29 23:03 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
>> >>>
>> >>> 2015-04-29 22:13 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
>> >>> >
>> >>> >
>> >>> > 2015-04-29 21:56 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
>> >>> >>
>> >>> >> Hi Martin,
>> >>> >>
>> >>> >> here are my feedbacks on the code:
>> >>> >>
>> >>> >> 1. severities should be an enum (I think it will not be open)
>> >>> >> 2. JsonbEventLocator more or less exists in JSON-P,any way to merge
>> >>> >> them? (ie have a json-common api -
>> >>> >>
>> >>> >>
>> >>> >> https://docs.oracle.com/javaee/7/api/javax/json/stream/JsonLocation.html).
>> >>> >
>> >>> >
>> >>> > I would prefer to not see a json-common :)
>> >>> > If jsonb was built on top of jsonp we wouldn't have this dilemma.
>> >>> >
>> >>> >>
>> >>> >> Plus I wouldn't use Optional here, it will mainly be used for
>> >>> >> logging
>> >>> >> and optional API is just too verbose for the most common cases IMO.
>> >>> >> Finally does the URI really means anything? We eat streams/readers
>> >>> >> so
>> >>> >> I wouldnt see URI as so important (in particularly since on write
>> >>> >> side
>> >>> >> it doesnt mean anything most of the time for the mapper framework).
>> >>> >
>> >>> >
>> >>> > It could be an optional<XXLocationXXX> and the location just contain
>> >>> > the
>> >>> > basic position infos. I don't suggest here to define another
>> >>> > "Location"
>> >>> > for
>> >>> > jsonb.
>> >>> >
>> >>> >>
>> >>> >> 3. on the handler signature: I'd return void and add a handled()
>> >>> >> method to the event to make the API more explicit
>> >>> >
>> >>> >
>> >>> > No I think it's better to return true/false, otherwise the impl
>> >>> > would
>> >>> > be
>> >>> > stateful and not thread safe.
>> >>> >
>> >>>
>> >>> the event is already stateful anyway. it doesnt change the handler
>> >>> chain so not sure i get your point
>> >>>
>> >>> >>
>> >>> >>
>> >>> >> On the overall solution I have 2 points:
>> >>> >> 1. is it really needed for 1.0? Can't we just use direct exceptions
>> >>> >> like in JSON-P? I'm very tempted to wait for users feedbacks and
>> >>> >> see
>> >>> >> if it is missing before adding it.
>> >>> >>
>> >>> >> 2. Do we already have somewhere a list of events? They should be in
>> >>> >> an
>> >>> >> enum + in TCK to ensure portability otherwise kept open it is not
>> >>> >> something portable so not something the spec should handle directly
>> >>> >> IMO (as a user I would then expect to unwrap my mapper to a vendor
>> >>> >> type to access it, not to use directly the spec).
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> Romain Manni-Bucau
>> >>> >> @rmannibucau
>> >>> >> http://www.tomitribe.com
>> >>> >> http://rmannibucau.wordpress.com
>> >>> >> https://github.com/rmannibucau
>> >>> >>
>> >>> >>
>> >>> >> 2015-04-29 19:53 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
>> >>> >> > Hi Experts,
>> >>> >> >
>> >>> >> > I have pushed changes which include proposal to customize error
>> >>> >> > handling.
>> >>> >> >
>> >>> >> > Summary of the proposal:
>> >>> >> >
>> >>> >> > Erros, warning and other issues should be (similarly to JAXB)
>> >>> >> > propagated
>> >>> >> > as
>> >>> >> > events. It will be possible to intercept (and to ignore) these
>> >>> >> > events by
>> >>> >> > providing custom JsonbEventHandler implementation.
>> >>> >> >
>> >>> >> > Details could be found in specification and in code (api +
>> >>> >> > examples).
>> >>> >> >
>> >>> >> > MartinV
>> >>> >
>> >>> >
>> >>
>> >>
>> >
>
>