johnzon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: Improving "Missing @JsonbCreator argument" message
Date Thu, 23 Jul 2020 05:17:56 GMT
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 <david.blevins@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
>
>

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