jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

Support sub type mapping to same class also programmatically

Open JoergHeinicke5005 opened this issue 3 years ago • 3 comments

Describe the bug With #312 sub type mapping to same class was fixed/ enabled for declaring @JsonSubTypes. As already mentioned in a later comment in that issue, the same does not work for specifying sub type mapping programmatically using ObjectMapper.registerSubtypes(NamedType... types). Reason is that equals(..) and hashCode(..) of NamedType only do consider the class field, but not the name field. StdSubtypeResolver uses a LinkedHashSet to which only the first sub type is being added.

Version information Which Jackson version(s) was this for? 2.10.5

To Reproduce

final ObjectMapper objectMapper = new ObjectMapper();
// this sub type is being added successfully
objectMapper.registerSubtypes(new NamedType(MyObject.class, "myObject1"));
// this one is not being added
objectMapper.registerSubtypes(new NamedType(MyObject.class, "myObject2"));

While the same worked with @JsonSubTypes:

@JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
        property = "type",
        defaultImpl = java.lang.Void.class)
@JsonSubTypes({
        @JsonSubTypes.Type(value = MyObject.class, name = "myObject1"),
        @JsonSubTypes.Type(value = MyObject.class, name = "myObject2")
})

Expected behavior ObjectMapper should successfully register all sub types even if they map to same class.

Additional context Background for changing from declarative (annotations) to programmatic approach is a refactoring in a way that the generic (and so far annotated) class is being moved into a central lib and must no longer be aware of all the specific sub types.

JoergHeinicke5005 avatar Nov 20 '20 00:11 JoergHeinicke5005

While debugging I found that sub types added via @JsonSubTypes are stored entirely differently than the ones registered programmatically. This itself makes me a little bit suspicious about the correct way to fix it.

One way could be to provide a different/ modified implementation of the SubtypeResolver (i.e. not using LinkedHashSet in StdSubtypeResolver. The easiest way to fix it seems to be to change equals(..) and hashCode(..) implementation in NamedType, i.e. considering name field as well besides _class field. The latter approach we had already working successfully for our use cases but I'm not aware of other possible side effects.

Currently we work around the issue by creating 2 dummy extension classes MyObject1 extends MyObject and MyObject2 extends MyObject.

JoergHeinicke5005 avatar Nov 20 '20 00:11 JoergHeinicke5005

One possible challenge is that whereas with annotations, access, it is easier to see which direction mapping is needed, same is not true for simple registration which is expected to apply to both serialization and deserialization. But then again access is by ser/deser type so maybe this is not problematic after all.

I think I would be open to solutions here: it seems valuable to be able to store mappings with multiple name mapping to same class. Case of conflicts (wrt serialization) should be resolved in a well-defined manner; I guess there are 3 main choices:

  1. Use first, ignore remaining
  2. Use last, ignore prior
  3. Throw exception in case of ambiguity.

I'll tag this with 2.13 as timing-wise 2.12 is feature-locked at this point.

cowtowncoder avatar Nov 25 '20 01:11 cowtowncoder

Not sure what kind of input, feedback or support is required.

On registering the first is currently considered, additional ones are ignored. I would also be fine with throwing exception.

JoergHeinicke5005 avatar Jan 08 '21 10:01 JoergHeinicke5005

@cowtowncoder, it actually looks like this issue has been fixed ages ago: https://github.com/FasterXML/jackson-databind/issues/2515, timewise even 1 yr before I reported this issue. Back then we were still using 2.10.5 though while the fix was only included in 2.11.x.

The fix implemented is the one I suggested back then as well:

The easiest way to fix it seems to be to change equals(..) and hashCode(..) implementation in NamedType, i.e. considering name field as well besides _class field.

I have noticed it now when reviewing some code: assuming this issue is still pending, I was wondering why the code is working. So I checked it in more detail...

JoergHeinicke5005 avatar May 09 '23 22:05 JoergHeinicke5005

@JoergHeinicke5005 Hmmm, strange there is no commit or PR related to this issue.

JooHyukKim avatar May 10 '23 12:05 JooHyukKim

In https://github.com/FasterXML/jackson-databind/issues/2515 commit https://github.com/FasterXML/jackson-databind/commit/3ac186c506f4180e22ac4e75b7cfd5bc5e6c6c2c is referenced.

JoergHeinicke5005 avatar May 10 '23 13:05 JoergHeinicke5005

@JoergHeinicke5005 Thank you! Good to have references around 😄

JooHyukKim avatar May 10 '23 13:05 JooHyukKim

Ok just to make: @JoergHeinicke5005 can this now be closed as as duplicate of #2515? I assume this is so and close this.

cowtowncoder avatar May 15 '23 00:05 cowtowncoder