Added unit test for Issue-4214 #4215
Conversation
| if (ser == null || (value instanceof EnumSet && ser.getClass() != EnumSetSerializer.class)) { | ||
| // if value instance of EnumSet, use EnumSetSerializer for EnumSet instead of | ||
| // generic CollectionSerializer, since de-serialization uses EnumSetDeserializer, | ||
| // if property value is of type EnumSet |
There was a problem hiding this comment.
I am sorry, but this is not a valid solution. The solution does not seem to fit overall and class design in any way. Is there simliar implementation in BeanPropertyWriter? Or any other pattern that you refered to?
There was a problem hiding this comment.
i agree that the fix is not fitting in overall design of this class, just added this workaround here to explain the problem better. I tried to set the serializer while initializing the instance of this class, but the runtime type information of the property value is not available at the time of initializing of this class's object.
|
|
||
| public void testSerialization() throws Exception { | ||
| ObjectMapper mapper = jsonMapperBuilder().build() | ||
| .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) |
There was a problem hiding this comment.
Never disable this for unit tests (except when testing feature itself) -- it has a nasty habit of hiding real problems, and developers wrongly enable it out of habit (there are good reasons to enable it too, just not always).
There was a problem hiding this comment.
sure, will remove this configuration.
| .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) | ||
| // this type information is needed in json string, for this test | ||
| .activateDefaultTyping(BasicPolymorphicTypeValidator.builder().allowIfBaseType(Object.class).build(), | ||
| DefaultTyping.EVERYTHING); |
There was a problem hiding this comment.
Ah. And of course it's EVERYTHING that causes problems. :)
|
@dvhvsekhar Would it be possible to change the PR to just have failing test? It should be moved under |
@cowtowncoder , can you please check now? I incorporated all suggestions. |
dvhvsekhar
changed the title
See #4214 for more details.