You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by David Blevins <da...@gmail.com> on 2020/07/23 02:39:52 UTC
Improving "Missing @JsonbCreator argument" message
Hey All!
I'd really like to improve the above error message, but I would like to do it in a way that has the highest chance to be merged so first asking for guidance.
Ideally the message would tell me:
1) the class annotated with @JsonbCreator
2) the arguments that are missing
A very optional, but would be slick:
3) that the `johnzon.failOnMissingCreatorValues` flag exists
The most obvious trick to this is that the validation is done in a lambda here:
- https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L223
However the data we would need to produce an informative message is here:
- https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L322
This is me brainstorming as I type, but I can imagine a few ways this could be solved:
1) Move the lambda down to after the `if` block that sets `final String[] params`, then we have what we need to build a better message.
2) Replace the lambda `factoryValidator.accept(params)` with a plain method call `validateFactory(clazz, params, args)`. We'd need to rename `public Object create(final Object[] params)` to public Object create(final Object[] args)` so there's no conflict with the `String[] params` in the outter method signature.
3) Make the `factoryValidator` lambda throw a subclass of JsonbException that has a field indicating the `args` array positions that are null. Then make `public Object create(final Object[] params)` method catch it, match up the missing arg positions with the names and wrap the exception with a more detailed exception.
4) Change the lambda from `final Consumer<Object[]> factoryValidator` to something like `final Consumer<FactoryMetadata> factoryValidator` and introduce a small inner class FactoryMetadata that has clazz, params, args as fields.
Possibly some other options jump out at others.
If I don't hear anything, I'll pick one and see how it goes. While I'm at it I'd like to improve the other error messages in the area so at minimum they say the affected class:
- https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L192
- https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L206
- https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L292
-David
AW: Improving "Missing @JsonbCreator argument" message
Posted by Arne Limburg <ar...@openknowledge.de>.
Of course, so at least mentioning the flag in the exception would be great, too.
OPEN KNOWLEDGE GmbH
Poststraße 1, 26122 Oldenburg
Mobil: +49 151 - 108 22 942
Tel: +49 441 - 4082-154
Fax: +49 441 - 4082-111
arne.limburg@openknowledge.de
www.openknowledge.de <https://www.openknowledge.de/>
Registergericht: Amtsgericht Oldenburg, HRB 4670
Geschäftsführer: Lars Röwekamp, Jens Schumann
Treffen Sie uns auf kommenden Konferenzen und Workshops:
Zu unseren Events<https://www.openknowledge.de/event/>
________________________________
Von: Romain Manni-Bucau <rm...@gmail.com>
Gesendet: Donnerstag, 23. Juli 2020 08:35
An: dev@johnzon.apache.org
Betreff: Re: Improving "Missing @JsonbCreator argument" message
Le jeu. 23 juil. 2020 à 08:02, Arne Limburg <ar...@openknowledge.de>
a écrit :
> Hi Romain,
>
>
> This will happen in production regularly when a clients sends wrong
> payload (you can't expect that not to happen).
>
Yes and no, only for buggy code.
Basically this must not happen cause jsonb is not the one which should do
the validation, this is why we can disable the spec requirement to enable
business messages.
A technical message is wrong anyway and not i18n.
Indeed does not mean we must not enhance it, we must, but maybe we should
document to not use that default behavior too?
> Personally I would go for option 5:
>
>
> Add a functional interface FactoryValidator, that has a validate method
> with three parameters (class, method, args) and replace the Consumer with
> that interface
>
>
> It's litte code change and satisfies the need.
>
>
> Just my two cent,
>
> Cheers,
>
> Arne
>
>
> OPEN KNOWLEDGE GmbH
> Poststraße 1, 26122 Oldenburg
> Mobil: +49 151 - 108 22 942
> Tel: +49 441 - 4082-154
> Fax: +49 441 - 4082-111
> arne.limburg@openknowledge.de
> www.openknowledge.de<http://www.openknowledge.de> <https://www.openknowledge.de/>
>
> Registergericht: Amtsgericht Oldenburg, HRB 4670
> Geschäftsführer: Lars Röwekamp, Jens Schumann
>
> Treffen Sie uns auf kommenden Konferenzen und Workshops:
>
> Zu unseren Events<https://www.openknowledge.de/event/>
>
>
>
>
>
> ________________________________
> Von: Romain Manni-Bucau <rm...@gmail.com>
> Gesendet: Donnerstag, 23. Juli 2020 07:17
> An: dev@johnzon.apache.org
> Betreff: Re: Improving "Missing @JsonbCreator argument" message
>
> Hi David,
>
> Good catch - looks like a too fast "make it pass the tck" ;)
>
> A few comments inline.
>
> Le jeu. 23 juil. 2020 à 04:40, David Blevins <da...@gmail.com> a
> écrit :
>
> > Hey All!
> >
> > I'd really like to improve the above error message, but I would like to
> do
> > it in a way that has the highest chance to be merged so first asking for
> > guidance.
> >
> > Ideally the message would tell me:
> >
> > 1) the class annotated with @JsonbCreator
> > 2) the arguments that are missing
> >
>
> +1
>
>
> > A very optional, but would be slick:
> >
> > 3) that the `johnzon.failOnMissingCreatorValues` flag exists
> >
>
> Maybe more "you didnt set this flag and params are missing" in case the dev
> knows about it (rather than "it exists")?
>
>
> > The most obvious trick to this is that the validation is done in a lambda
> > here:
> >
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L223
> >
> > However the data we would need to produce an informative message is here:
> >
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L322
> >
> > This is me brainstorming as I type, but I can imagine a few ways this
> > could be solved:
> >
> > 1) Move the lambda down to after the `if` block that sets `final
> String[]
> > params`, then we have what we need to build a better message.
> >
> > 2) Replace the lambda `factoryValidator.accept(params)` with a plain
> > method call `validateFactory(clazz, params, args)`. We'd need to rename
> > `public Object create(final Object[] params)` to public Object
> create(final
> > Object[] args)` so there's no conflict with the `String[] params` in the
> > outter method signature.
> >
> > 3) Make the `factoryValidator` lambda throw a subclass of JsonbException
> > that has a field indicating the `args` array positions that are null.
> Then
> > make `public Object create(final Object[] params)` method catch it, match
> > up the missing arg positions with the names and wrap the exception with a
> > more detailed exception.
> >
> > 4) Change the lambda from `final Consumer<Object[]> factoryValidator` to
> > something like `final Consumer<FactoryMetadata> factoryValidator` and
> > introduce a small inner class FactoryMetadata that has clazz, params,
> args
> > as fields.
> >
>
> Think I'd just catch the exception - we know its type and only exception
> which can be thrown - and recreate it as needed. This is not a case which
> will happen in prod until there is a bug so should be good enough and avoid
> new classes or breaking changes.
>
>
> > Possibly some other options jump out at others.
> >
> > If I don't hear anything, I'll pick one and see how it goes. While I'm
> at
> > it I'd like to improve the other error messages in the area so at minimum
> > they say the affected class:
> >
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L192
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L206
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L292
>
>
> +1
>
>
> >
> >
> >
> >
> >
> >
> > -David
> >
> >
>
Re: Improving "Missing @JsonbCreator argument" message
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le jeu. 23 juil. 2020 à 08:02, Arne Limburg <ar...@openknowledge.de>
a écrit :
> Hi Romain,
>
>
> This will happen in production regularly when a clients sends wrong
> payload (you can't expect that not to happen).
>
Yes and no, only for buggy code.
Basically this must not happen cause jsonb is not the one which should do
the validation, this is why we can disable the spec requirement to enable
business messages.
A technical message is wrong anyway and not i18n.
Indeed does not mean we must not enhance it, we must, but maybe we should
document to not use that default behavior too?
> Personally I would go for option 5:
>
>
> Add a functional interface FactoryValidator, that has a validate method
> with three parameters (class, method, args) and replace the Consumer with
> that interface
>
>
> It's litte code change and satisfies the need.
>
>
> Just my two cent,
>
> Cheers,
>
> Arne
>
>
> OPEN KNOWLEDGE GmbH
> Poststraße 1, 26122 Oldenburg
> Mobil: +49 151 - 108 22 942
> Tel: +49 441 - 4082-154
> Fax: +49 441 - 4082-111
> arne.limburg@openknowledge.de
> www.openknowledge.de <https://www.openknowledge.de/>
>
> Registergericht: Amtsgericht Oldenburg, HRB 4670
> Geschäftsführer: Lars Röwekamp, Jens Schumann
>
> Treffen Sie uns auf kommenden Konferenzen und Workshops:
>
> Zu unseren Events<https://www.openknowledge.de/event/>
>
>
>
>
>
> ________________________________
> Von: Romain Manni-Bucau <rm...@gmail.com>
> Gesendet: Donnerstag, 23. Juli 2020 07:17
> An: dev@johnzon.apache.org
> Betreff: Re: Improving "Missing @JsonbCreator argument" message
>
> Hi David,
>
> Good catch - looks like a too fast "make it pass the tck" ;)
>
> A few comments inline.
>
> Le jeu. 23 juil. 2020 à 04:40, David Blevins <da...@gmail.com> a
> écrit :
>
> > Hey All!
> >
> > I'd really like to improve the above error message, but I would like to
> do
> > it in a way that has the highest chance to be merged so first asking for
> > guidance.
> >
> > Ideally the message would tell me:
> >
> > 1) the class annotated with @JsonbCreator
> > 2) the arguments that are missing
> >
>
> +1
>
>
> > A very optional, but would be slick:
> >
> > 3) that the `johnzon.failOnMissingCreatorValues` flag exists
> >
>
> Maybe more "you didnt set this flag and params are missing" in case the dev
> knows about it (rather than "it exists")?
>
>
> > The most obvious trick to this is that the validation is done in a lambda
> > here:
> >
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L223
> >
> > However the data we would need to produce an informative message is here:
> >
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L322
> >
> > This is me brainstorming as I type, but I can imagine a few ways this
> > could be solved:
> >
> > 1) Move the lambda down to after the `if` block that sets `final
> String[]
> > params`, then we have what we need to build a better message.
> >
> > 2) Replace the lambda `factoryValidator.accept(params)` with a plain
> > method call `validateFactory(clazz, params, args)`. We'd need to rename
> > `public Object create(final Object[] params)` to public Object
> create(final
> > Object[] args)` so there's no conflict with the `String[] params` in the
> > outter method signature.
> >
> > 3) Make the `factoryValidator` lambda throw a subclass of JsonbException
> > that has a field indicating the `args` array positions that are null.
> Then
> > make `public Object create(final Object[] params)` method catch it, match
> > up the missing arg positions with the names and wrap the exception with a
> > more detailed exception.
> >
> > 4) Change the lambda from `final Consumer<Object[]> factoryValidator` to
> > something like `final Consumer<FactoryMetadata> factoryValidator` and
> > introduce a small inner class FactoryMetadata that has clazz, params,
> args
> > as fields.
> >
>
> Think I'd just catch the exception - we know its type and only exception
> which can be thrown - and recreate it as needed. This is not a case which
> will happen in prod until there is a bug so should be good enough and avoid
> new classes or breaking changes.
>
>
> > Possibly some other options jump out at others.
> >
> > If I don't hear anything, I'll pick one and see how it goes. While I'm
> at
> > it I'd like to improve the other error messages in the area so at minimum
> > they say the affected class:
> >
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L192
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L206
> > -
> >
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L292
>
>
> +1
>
>
> >
> >
> >
> >
> >
> >
> > -David
> >
> >
>
AW: Improving "Missing @JsonbCreator argument" message
Posted by Arne Limburg <ar...@openknowledge.de>.
Hi Romain,
This will happen in production regularly when a clients sends wrong payload (you can't expect that not to happen).
Personally I would go for option 5:
Add a functional interface FactoryValidator, that has a validate method with three parameters (class, method, args) and replace the Consumer with that interface
It's litte code change and satisfies the need.
Just my two cent,
Cheers,
Arne
OPEN KNOWLEDGE GmbH
Poststraße 1, 26122 Oldenburg
Mobil: +49 151 - 108 22 942
Tel: +49 441 - 4082-154
Fax: +49 441 - 4082-111
arne.limburg@openknowledge.de
www.openknowledge.de <https://www.openknowledge.de/>
Registergericht: Amtsgericht Oldenburg, HRB 4670
Geschäftsführer: Lars Röwekamp, Jens Schumann
Treffen Sie uns auf kommenden Konferenzen und Workshops:
Zu unseren Events<https://www.openknowledge.de/event/>
________________________________
Von: Romain Manni-Bucau <rm...@gmail.com>
Gesendet: Donnerstag, 23. Juli 2020 07:17
An: dev@johnzon.apache.org
Betreff: Re: Improving "Missing @JsonbCreator argument" message
Hi David,
Good catch - looks like a too fast "make it pass the tck" ;)
A few comments inline.
Le jeu. 23 juil. 2020 à 04:40, David Blevins <da...@gmail.com> a
écrit :
> Hey All!
>
> I'd really like to improve the above error message, but I would like to do
> it in a way that has the highest chance to be merged so first asking for
> guidance.
>
> Ideally the message would tell me:
>
> 1) the class annotated with @JsonbCreator
> 2) the arguments that are missing
>
+1
> A very optional, but would be slick:
>
> 3) that the `johnzon.failOnMissingCreatorValues` flag exists
>
Maybe more "you didnt set this flag and params are missing" in case the dev
knows about it (rather than "it exists")?
> The most obvious trick to this is that the validation is done in a lambda
> here:
>
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L223
>
> However the data we would need to produce an informative message is here:
>
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L322
>
> This is me brainstorming as I type, but I can imagine a few ways this
> could be solved:
>
> 1) Move the lambda down to after the `if` block that sets `final String[]
> params`, then we have what we need to build a better message.
>
> 2) Replace the lambda `factoryValidator.accept(params)` with a plain
> method call `validateFactory(clazz, params, args)`. We'd need to rename
> `public Object create(final Object[] params)` to public Object create(final
> Object[] args)` so there's no conflict with the `String[] params` in the
> outter method signature.
>
> 3) Make the `factoryValidator` lambda throw a subclass of JsonbException
> that has a field indicating the `args` array positions that are null. Then
> make `public Object create(final Object[] params)` method catch it, match
> up the missing arg positions with the names and wrap the exception with a
> more detailed exception.
>
> 4) Change the lambda from `final Consumer<Object[]> factoryValidator` to
> something like `final Consumer<FactoryMetadata> factoryValidator` and
> introduce a small inner class FactoryMetadata that has clazz, params, args
> as fields.
>
Think I'd just catch the exception - we know its type and only exception
which can be thrown - and recreate it as needed. This is not a case which
will happen in prod until there is a bug so should be good enough and avoid
new classes or breaking changes.
> Possibly some other options jump out at others.
>
> If I don't hear anything, I'll pick one and see how it goes. While I'm at
> it I'd like to improve the other error messages in the area so at minimum
> they say the affected class:
>
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L192
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L206
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L292
+1
>
>
>
>
>
>
> -David
>
>
Re: Improving "Missing @JsonbCreator argument" message
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi David,
Good catch - looks like a too fast "make it pass the tck" ;)
A few comments inline.
Le jeu. 23 juil. 2020 à 04:40, David Blevins <da...@gmail.com> a
écrit :
> Hey All!
>
> I'd really like to improve the above error message, but I would like to do
> it in a way that has the highest chance to be merged so first asking for
> guidance.
>
> Ideally the message would tell me:
>
> 1) the class annotated with @JsonbCreator
> 2) the arguments that are missing
>
+1
> A very optional, but would be slick:
>
> 3) that the `johnzon.failOnMissingCreatorValues` flag exists
>
Maybe more "you didnt set this flag and params are missing" in case the dev
knows about it (rather than "it exists")?
> The most obvious trick to this is that the validation is done in a lambda
> here:
>
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L223
>
> However the data we would need to produce an informative message is here:
>
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L322
>
> This is me brainstorming as I type, but I can imagine a few ways this
> could be solved:
>
> 1) Move the lambda down to after the `if` block that sets `final String[]
> params`, then we have what we need to build a better message.
>
> 2) Replace the lambda `factoryValidator.accept(params)` with a plain
> method call `validateFactory(clazz, params, args)`. We'd need to rename
> `public Object create(final Object[] params)` to public Object create(final
> Object[] args)` so there's no conflict with the `String[] params` in the
> outter method signature.
>
> 3) Make the `factoryValidator` lambda throw a subclass of JsonbException
> that has a field indicating the `args` array positions that are null. Then
> make `public Object create(final Object[] params)` method catch it, match
> up the missing arg positions with the names and wrap the exception with a
> more detailed exception.
>
> 4) Change the lambda from `final Consumer<Object[]> factoryValidator` to
> something like `final Consumer<FactoryMetadata> factoryValidator` and
> introduce a small inner class FactoryMetadata that has clazz, params, args
> as fields.
>
Think I'd just catch the exception - we know its type and only exception
which can be thrown - and recreate it as needed. This is not a case which
will happen in prod until there is a bug so should be good enough and avoid
new classes or breaking changes.
> Possibly some other options jump out at others.
>
> If I don't hear anything, I'll pick one and see how it goes. While I'm at
> it I'd like to improve the other error messages in the area so at minimum
> they say the affected class:
>
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L192
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L206
> -
> https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java#L292
+1
>
>
>
>
>
>
> -David
>
>