Improve exception used, message, when indicating a required property is not set

[raubel]

I'm using Jackson 2.9.8.

I want to deserialize a payload to a class with JsonProperty(required = true) annotated fields.
If the payload does not contain all necessary fields, I want to inform my API user with a concise (possibly custom) message.
However, the Jackson deserialization only gives me an InvalidDefinitionException ("Cannot construct instance of...") with the root cause, which depends on the underlying technology. For instance, with the AutoValue framework, the root cause is an IllegalStateException ("Missing required properties:...") and with Kotlin data classes, it is an IllegalArgumentException ("Parameter specified as non-null is null").

Since all fields are annotated, I guess Jackson could be able to produce something like a MissingFieldException (with field name, path...). The same way it is able to produce a UnrecognizedFieldException when an unknown field is met.

[cowtowncoder]

I am not sure what to do here, to be honest. Jackson project does not control Auto-Value (or Immutables, Lombok); and Kotlin data classes are handled by Kotlin module.

I agree in that it would be good to surface problem in same or at least similar way, and that exception message should give as much information as possible. But since handling is widely diverging it is necessary to tackle specific problems, I think, starting with what jackson-databind gives.
I am open to improvements here, specifically: parameters for @JsonCreator annotated constructors with required = true setting (since only those are verified -- regular setter/field-based properties do not yet have such verification, for historical reasons).

[raubel]

I don't know jackson-databind enough, but my naive thought was that, since it can report any unexpected property (base on @JsonProperty annotations), it could also report any missing property.
We often have objects with a lot of properties, so instead of using the constructor pattern, with the @JsonCreator annotation, we rather use the builder pattern provided by AutoValue (with @JsonProperty annotations).

[cowtowncoder]

@raubel that makes sense from user perspective, but from implementation perspective things are different -- the problem is that figuring out what is missing requires more bookkeeping internally than just matching what exists. And since required was added much later than core databind (in 2.0 I think), and initially just as metadata for schema generation, there just isn't anything to support it from core. The reason why @JsonCreator is easier is that buffering is required (since all properties passed must be available before call can be made), whereas with regular properties POJO always exists and no buffering is needed -- value can be immediately assigned via setter or field.

So implementing support for validation requires much more thought, to ensure that performance is not significantly affected with added tracking (despite size of API Jackson critical path processing is quite lean and additional data structures will be observable in throughput).

[cowtowncoder]

Note: if someone could just provide a test case in which we get "wrong" exception message -- InvalidDefinitionException should not be used if definition is fine but provided JSON not -- it'd be possible to get started here.
I'll accept PR for just the test to qualify for Hacktoberfest.

[akohli96]

@cowtowncoder , I will take a crack at this. Just to clarify my understanding for time being you simply want a test case of what should happen ie java InvalidDefinitionException should not be used. Should this test case be commented out? Should I create it in the folder with exception tests ?

[cowtowncoder]

@akohli96 exc is not necessarily the best place (it contains tests for (de)serializing Exception types), but I am not too picky on initial location; I can move tests around easily. This is related to deserialization, so somewhere under ".deser."; also related to creators, so could be under "deser.creators".

Now, on test, I'd be interested in finding case(s) where exception message does not give enough indication of missing required property/-ies; use of InvalidDefinitionException is probably not ideal either.

Oh, and on commenting: there is separate "..../failing" directory for tests that would fail; these tests are not run with regular "mvn test" but only when directly referenced. So that might actually be the best location.

[cowtowncoder]

Not sure how to proceed here; and while I am +100 on improving error messages, would need more specific information on what is missing, with a test case against Jackson 2.12.

May be re-opened or re-filed with more information; closing for now.