Polymorphic subtype deduction ignores defaultImpl attribute

[drekbour]

Describe the bug
A class with @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION, defaultImpl = Animal.class) that fails to deduce a solo candidate should use defaultImpl

Version information
2.12.0

To Reproduce
TBA see #2976

Expected behavior
In the absence of a single candidate, defaultImpl should be the target type regardless of suitability.

Additional context
The current deduction process does not support pojo-hierarchies such that the absence of child properties infers a parent type. That is, every deducible subtype MUST have some unique properties and the input data MUST contain said unique properties to provide a positive match.

[drekbour]

Please assign to me. I'm happy to support my addition :)

[cowtowncoder]

Great! Sounds like this could go in 2.12 branch (might make it in 2.12.2 depending on timing), if it feels like low-risk addition (seems to me that'd be the case). Otherwise should go in 2.13 which would get bit longer to get out.

[drekbour]

See PR #3057. However

To discuss: Quality of feedback is reduced
(a) with no defaultImpl, we now report only Cannot deduce unique subtype and not even the number of candidates. This is a bit weak but hard to correct without a larger change to the inherited error handling.
(b) with successful defaultImpl, nobody mentions that deduction failed. I think this is expected
(c) with unsuccessful defaultImpl, nobody mentions that deduction failed. I think this is poor.

I stumbled on (c) as my superclass was abstract (and after that because I had not disabled FAIL_ON_UNKNOWN_PROPERTIES). I can quite imagine users doing similar things and never testing that their defaultImpl works (DEDUCTION mode is after all a little bit of a "lazy model").

I predict confusion as to why Jackson is complaining about a secondary issue and completely masking the original cause (deduction failure).

@cowtowncoder next step is your call but I'd rather include what we have here as there's other work to be done in this area

[cowtowncoder]

Hmmh. Yes, I see; while fixing the issue wrt defaultImpl is good, I really don't like losing contextual information. And although it is likewise good to allow graceful handling via handleXxx() methods (instead of directly failing), handleMissingTypeId() was not design with deduction in mind.
Handler itself does get TypeIdResolver, but handleMissingTypeId() is designed around missing info and not... incomplete/ambiguous one.

For 2.12 this may be best we could do; for 2.13 callbacks could possibly be improved. For example, could add alternative handleFailedTypeIdDeduction() which in turn could either call either existing handleMissingTypeId() in DeserializationProblemHandler or add a new method there as well.

I suspect that we might be able to solve some of these (esp (a)) with more changes to base impl TypeDeserializerBase... but that does run risk of regression for anyone subclassing with custom implementation (although that is not part of intended public API, not a supported extension point).

So with that, @drekbour, do you think splitting improvements b/w 2.12 and 2.13 makes sense?
I can definitely live with that.

[drekbour]

I think defaultImpl for both (b) and (c) works the same for other subtype modes does it not?
The loss in (a) is minor (the count of candidates) as it already doesnt include anything concrete that would help fix a problem much. That should be fixed but yes, not here

[cowtowncoder]

@drekbour Yes, defaultImpl is used similarly in that if the type cannot be resolved for whatever reason, it will be used; my concern is not with the logic. But the exception message should better indicate the reason for failure when defaultImpl is missing: and in that case it is wrong to say "type id is missing" for Deduction since type id is never provided (nor could or should be). But there is important contextual information provided up until now about potential types.

So my concern is only about actual fail messages. Those should be adjustable based on "type id type" (resolution mechanism), possibly just having special logic for Deduction type.

[drekbour]

Have updated this PR with a simple change to allow contextual failure reports - reinstating the original text. I have not attempted to write a better report as that would be a separate PR with new discussion (enough here already!).

[cowtowncoder]

@drekbour thanks! That sounds good. Agree, first things first; and for 2.12.x patches fixing missing defaultImpl handling sounds good. I'll have a look at PR.