Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sub type mapping to same class also programmatically #2950

Closed
JoergHeinicke5005 opened this issue Nov 20, 2020 · 8 comments
Closed
Labels
duplicate Duplicate of an existing (usually earlier) issue

Comments

@JoergHeinicke5005
Copy link

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 JoergHeinicke5005 added the to-evaluate Issue that has been received but not yet evaluated label Nov 20, 2020
@JoergHeinicke5005
Copy link
Author

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.

@cowtowncoder
Copy link
Member

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 cowtowncoder added 2.13 and removed to-evaluate Issue that has been received but not yet evaluated labels Nov 25, 2020
@JoergHeinicke5005
Copy link
Author

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.

@cowtowncoder cowtowncoder added 2.14 and removed 2.13 labels Jul 15, 2021
@cowtowncoder cowtowncoder removed the 2.14 label Aug 1, 2022
@JoergHeinicke5005
Copy link
Author

@cowtowncoder, it actually looks like this issue has been fixed ages ago: #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...

@JooHyukKim
Copy link
Member

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

@JoergHeinicke5005
Copy link
Author

In #2515 commit 3ac186c is referenced.

@JooHyukKim
Copy link
Member

@JoergHeinicke5005 Thank you! Good to have references around 😄

@cowtowncoder
Copy link
Member

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

@cowtowncoder cowtowncoder added the duplicate Duplicate of an existing (usually earlier) issue label May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Duplicate of an existing (usually earlier) issue
Projects
None yet
Development

No branches or pull requests

3 participants