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