add DeserializationFeature.FAIL_ON_SUBTYPE_CLASS_NOT_REGISTERED #5027
Conversation
pjfanning
changed the title
|
@pjfanning Subtypes need not be listed when using class name as type id since, well, class is found directly. It should indeed fail when using "type name" option (which defaults to "simple" class name). |
I can confirm that the |
|
Is this PR still needed? |
src/test/java/com/fasterxml/jackson/databind/deser/PolymorphicIdClassDeserTest.java
Show resolved
Hide resolved
| FooClass res1 = MAPPER.readValue(foo1, FooClass.class); | ||
| assertTrue(res1 instanceof FooClassImpl); | ||
| // next bit should in theory fail because FooClassImpl2 is not listed as a subtype | ||
| FooClass res2 = MAPPER.readValue(foo2, FooClass.class); |
There was a problem hiding this comment.
No. @JsonSubTypes is not used for any checking; it is only used for location subtypes when using "type name" (not class) as id, to find typeName-to-class mappings for subtypes on deserialization. This is not needed for class names.
Otherwise only Java language level subtype limitations are enforced (or checked).
EDIT: based on offline discussions, realize this is sort of RFE, asking to support such checks (optionally)
|
@cowtowncoder I've added a DeserializationFeature.FAIL_ON_POLYMORPHIC_SUBTYPE_CLASS_NOT_EXPLICITLY_REGISTERED and the new check only happens if this is enabled. I still need to work out how to support this for Id.MINIMAL_CLASS type too. |
src/main/java/com/fasterxml/jackson/databind/DeserializationFeature.java
Outdated
Show resolved
Hide resolved
pjfanning
changed the title
|
@cowtowncoder does this look like it's enough to merge? |
|
@pjfanning I hope to look into this soon (started 3.0.0-rc2 release) |
| @@ -26,6 +27,8 @@ public class ClassNameIdResolver | |||
|
|
|||
| protected final PolymorphicTypeValidator _subTypeValidator; | |||
|
|
|||
| private Set<String> _allowedSubtypes; | |||
There was a problem hiding this comment.
I'll change code a bit to make this final.
| if (ctxt instanceof DeserializationContext) { | ||
| deserializationContext = (DeserializationContext) ctxt; | ||
| } | ||
| if (_allowedSubtypes != null && deserializationContext != null |
There was a problem hiding this comment.
Without @JsonSubTypes, wouldn't _allowedSubtypes end up being null... ?
There was a problem hiding this comment.
I guess in the current impl, you can omit the JsonSubTypes annotation and then all subtypes are valid. Maybe I should change that.
There was a problem hiding this comment.
@cowtowncoder I changed the code to enforce the check if enabled even when JsonSubTyoes are not defined
| @@ -0,0 +1,61 @@ | |||
| package com.fasterxml.jackson.databind.deser; | |||
There was a problem hiding this comment.
There's different package for polymorphic deser... let me see.
The test is about polymorphic types. FooClass has 2 defined subclasses but I only register one (via a JsonSubTypes annotation).
So in theory, when I deserialize, I should be an exception when I try the 2nd unregisered class.
With Id.NAME, the subtypes are check at deserialization time. With Id.CLASS and Id.MINIMAL_CLASS, the subtypes are not checked.
DeserializationFeature.FAIL_ON_SUBTYPE_CLASS_NOT_REGISTERED defaults to false but when enabled, Id.CLASS and Id.MINIMAL_CLASS deserialization for subtypes will also fail if the classes in the inputs don't match the subtypes listed for the JsonTypeInfo annotation (via a JsonSubTypes annotation).
If you enable this feature, you should carefully test that your deserialization works. You will run into trouble if you have base types annotated with
@JsonTypeInfobut don't have a@JsonSubTypesannotation that lists all the possible subtypes.