Deduction deserialization - add support for pojo-hierarchies such that the absence of child fields infers a supertype

[pakorcz]

Currently, deduction deserialization requires all values to have unique field. Because of that, there is no way to deserialize values to superclass and subclasses, when defaultImpl is different than superclass.

Example:

For json:

 "name": "Italy",
 "places": {
   "mountains": {
     "name": "The Alps"
   },
   "cites": [
     {
       "name": "Rome",
       "places": {
         "colosseum": {
           "name": "The Colosseum"
         },
         "romanForum": {
           "name": "The Roman Forum"
         }
       }
     },
     {
       "name": "Venice",
       "places": {
         "bridges": []
       }
     }
   ]
 }
}

and pojo-hierarchy:

public static class Place implements WorthSeeing {  
  public String name;  
}  
  
public static class CompositePlace extends Place implements WorthSeeing {  
  
  public Map<String, WorthSeeing> places;  
}  
  
static class ListOfPlaces extends ArrayList<WorthSeeing> implements WorthSeeing {  
}

and deduction deserialization with defaultImpl differ then supertype:

@JsonTypeInfo(use = DEDUCTION, defaultImpl = ListOfPlaces.class)  
@JsonSubTypes( {@Type(ListOfPlaces.class), @Type(CompositePlace.class), @Type(Place.class)})  
interface WorthSeeing {}

the MismatchedInputException is thrown, because the values with only name fields are not classified to Place .

Desired behaviour:

• value contains only fields from superclass -> it's mapped to an object of superclass
• value contains fields from superclass and a subclass -> it's mapped to an object of the supclass
• value contains fields from superclass and fields that don't exist in any of subclasses -> the UnrecognizedPropertyException is thrown

[cowtowncoder]

I am not sure whether this is a valid idea, will have to defer to @drekbour.

[drekbour]

Acknowledged, yet to dig and and respond.

[drekbour]

Hi @pakorcz. I refer you to #2976 (comment) and the comments which follow it. The given example (great to see something not involving animals!) makes assumptions on a vanilla usecase that mean it's ok to infer the supertype. Those assumptions will not always hold true and the logic will be chaotic in other configurations.

@cowtowncoder Much as it pains me to say, I don't think the change is stable to include.

I really opened a can o' worms with this didn't I! We should keep collecting these requirements (I think they are all the same requirement really but expressed in usefully different ways as users try to hack around the limitations) and perhaps they will weave a clearer picture of where a general-case solution can safely add value.

I take from this issue the reasonable expectation that supertypes might be ok when they are explicitly included in the set of JsonSubTypes.

PS. There exists an alternative implementation of the deduction code which I wrote after the initial PR. It should be faster and would better handle terminal conditions - though nothing can get away from the fact that deser code doesn't why a field is absent given the many options. I'll PR it so it's not lost.

[cowtowncoder]

Yeah I was guessing it would not be something we'd want to do for general usage -- there tends to lots of logic that needs to be added to cover all kinds of cases that "make sense" to one user or use case.

+1 for collecting requirements if anyone wants to volunteer -- one possible place would

https://github.com/FasterXML/jackson-future-ideas/wiki

either new JSTEP-x for "improved deduction", or just another wiki page linked.
Or an issue.

Ideally I think there should be alternative way to add Type Id detection, given JsonNode (which does require full buffering but it is what it is), and let users implement more advanced logic.
But that's not easy given existing API design.

[drekbour]

I'll raise a new feature because I think I have a workable solution in mind now. It justs needs some meditation before I raise

[cowtowncoder]

Creating an issue as Work-in-Progress sounds reasonable; can always edit/change/close/re-create if need be.

[pakorcz]

Thank you guys for explanation and taking an interest in this case!

The point of it is exactly what @drekbour wrote:
“the expectation that supertypes might be ok when they are explicitly included in the set of JsonSubTypes”.

I would really appreciate this feature, because thanks to it – if set of possible json data reflects some kind of hierarchy – then it could be deserialized out of the box to proper class of pojo-hierarchy ( without further mapping and checking of null values).

[barkefors]

👋 I wonder if it would be an option to allow opting in to this behavior by explicitly declaring these disambiguating expectations. (Let me first say that while I've been using Jackson since at least as long as I can remember, I've never ventured out here before so apologies if this idea has already been discussed or is fundamentally broken in some way!)

In my understanding the presence of a property is in a natural way a fulfillment of the requiredness of a property, so what if we could resolve cases like the example here by putting the onus on the user, letting them declare CompositePlace as

class CompositePlace extends Place implements WorthSeeing {  
  @JsonProperty(required = true)
  public Map<String, WorthSeeing> places;  
}

// or
public CompositePlace(@JsonProperty(value="places", required=true) Map<String, WorthSeeing> places) {...}

// or
record CompositePlace(@JsonProperty(required=true) Map<String, WorthSeeing> places) implements WorthSeeing {}

which given {"name": "BarkingDuck"} would then discard CompositePlace as a candidate since places is missing. I'd argue that doing it this way would to some extent address concerns re. Include.NON_NULL, by requiring the user to decide whether an X without a Y is allowed to be an X or not. (Bird/Wingspan, CompositePlace/Place, OncallEngineer/PagerNumber...)

I believe that this could be implemented in a quite similar way to what #3290 does:

0. When building fingerprints, also construct a Map<BitSet, BitSet> of required properties per type keyed by fingerprint
1. Keep track of seen, known properties while iterating over fields in BitSet actual
2. After iterating over fields, iterate over candidates removing if required.andNot(actual) is not empty
3. Check again if candidates.size() == 1, otherwise proceed

Thanks for your great work on this amazing library! ❤️

[drekbour]

Thankyou for thinking through the scenario and code. Honouring required=true is a good addition to the deduction process and goes onto my list.

[TCke83]

+1 would love to see this in the next version, can't use the deduction now because the supertypes don't get deducted

[vivianazimbone]

+1

[rafiulahad]

I am having the same problem, Is there a solution for it?
It seems that when trying to deserialize to a POJO class, it is considering ALL fields (declared and inherited) of all classes, instead of just declared fields.
Say type T has declared fields A and B, and subtype S has declared field C. If both declared and inherited fields are considered, just seeing A and B (without C) in the JSON, it cannot infer that T should be used. It thinks that both T and S are candidates, which is wrong. If only declared fields are considered, just seeing A and B, it can deduce that an instance of T must be used.
The algorithm for DEDUCT should be something like the one shown below:
Let T be the set of types mentioned in JsonTypeInfo (the type itself and its subtypes that we want to deserialize to)
For a type t, let declaredFields(t) be the set of all fields declared in type t, and allFileds(t) be the set of declared and inherited fields. Let J be the set of fields the Json document.

1. compute R1 = {t in T such that intersection of J and declaredFields(t) in not empty }
2. If R1 has only one type in it use that type else
3. compute R2 = {t in R1 such that J is a subset of allFields(t)}
4. if there is one type in R2 use it, else raise error
   Am I missing something?
   if it is already solved, please let me know the solution.

[JooHyukKim]

@rafiulahad Is it possible to share a reproducible test case with your configuration(includin JsonTypeInfo and Mapper) against Jackson 2.15.2 version? It will be easier to provide help 👍🏻👍🏻😄

[rafiulahad]

@JooHyukKim, below is a minimal reproducible case using Jackson 2.15.2
@DaTa
@JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION)
@JsonSubTypes({ @JsonSubTypes.Type(SuperClass.class),@JsonSubTypes.Type(SubClass.class)})
public class SuperClass {
int a;
}
@DaTa
public class SubClass extends SuperClass {
int b;
}
public class TestMapper {
public static SuperClass deserialize(String json) {
ObjectMapper objectMapper = new ObjectMapper();
try {
return objectMapper.readValue(json, SuperClass.class);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
}
}

test code
String json = "{"a":1}";
TestMapper.deserialize(json);
result
Could not resolve subtype of [simple type, xxxxxxxx.SuperClass]: Cannot deduce unique subtype of xxxxxxxx.SuperClass (2 candidates match)
It should use SuperClass in this case.
BTW, it works for json = "{"a":1, "b":2}";
My algorithm will work for both cases.
(I am not sure why this editor is converting Data to DaTa and backslash double quotes to double quotes :)

[rafiulahad]

Hi Joo Hyuk, I included a simple test case in my comment. Thanks. -Rafiul

…

On Sat, Jun 17, 2023 at 8:04 AM Kim, Joo Hyuk ***@***.***> wrote: @rafiulahad <https://github.com/rafiulahad> Is it possible to share a reproducible test case with your configuration(includin JsonTypeInfo and Mapper) against Jackson 2.15.2 version? It will be easier to provide help 👍🏻👍🏻😄 — Reply to this email directly, view it on GitHub <#3289 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFPXVTSGEZ2LWAAQRVPPVTTXLXBQDANCNFSM5FASPSXA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cowtowncoder]

@rafiulahad Please read the whole chain for the current status. There has been no work on this, and I do not that without you or someone working on it anything will happen.

Personally I am not sure there should even be lots of more advanced functionality here -- perhaps a better approach would be to allow registering a handler that would be given JsonNode of all contents, to infer the subtype.
I think it is impossible/impractical to cover all possible cases users would want to use inference on.

But that would be a new feature for someone to implement. I do not have time or actual itch to work on this but maybe someone else does.

[rafiulahad]

I worked around the issue by introducing a field in the super type and not using deduction.
Thank you for your insightful feedback as an (the) inventor of this library. To see if I got it right, below are additional details I want to add to your proposal on the handler approach.
Suppose we have an interface: Class resolveClass(JsonNode json, Set<Class> classes) throws .... which when called, will find one class, if it exists, among the classes to use for deserialization. ObjectMapper will call this to get the class (and for fields of type POJO class) to deserialize the given JsonNode. Is this inline with what you are thinking?
if so, I can provide a default implementation of the interface. In fact, I have a PoC implementation of it with no effort put in to optimize it yet. But I won't have time to understand the ObjectMapper code to make changes.

[cowtowncoder]

@rafiulahad Something like that, yes, although whether Set<Class> is passed is an open question (resolver need to limit itself to that set, at any rate).

But the tricky part is how to make it all work with @JsonTypeInfo and existing routing, which assumes different things (like existence of type id, inclusion mechanism); piping through TypeDeserializer (and possibly TypeSerializer).
Not so much any implementation of the interace.

[rafiulahad]

The set of classes are the ones mentioned in @JsonSubTypes

[cowtowncoder]

@rafiulahad Yes I was guessing that'd be the case. Note tho that these are typically passed during construction of TypeDeserializer and not for each resolution call -- this set does not really change so resolver does not need it dynamically passed. But that's a minor detail in grand scheme of things.