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