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

Field mix-ins do not work for `Enum`s

Open kostousov-ds opened this issue 5 years ago • 3 comments

  1. create java enum like this
public enum SomeEnum {
    none,
    tax10,
    tax20
}
  1. create mixin for enum
public enum  SomeEnumMixin {
    @JsonProperty("zero")
    none,
    @JsonProperty("TypTyp")
    tax10,
    @JsonProperty("PytPyt")
    tax20
}
  1. register mixin via .addMixIn(SomeEnum.class, SomeEnumMixin.class)

  2. try to deserialize sometithing

ObjectMapper throws NullPointerException

java.lang.NullPointerException
	at com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector._addFieldMixIns(AnnotatedFieldCollector.java:117)
	at com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector._findFields(AnnotatedFieldCollector.java:94)
	at com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector.collect(AnnotatedFieldCollector.java:48)
	at com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector.collectFields(AnnotatedFieldCollector.java:43)
	at com.fasterxml.jackson.databind.introspect.AnnotatedClass._fields(AnnotatedClass.java:366)
	at com.fasterxml.jackson.databind.introspect.AnnotatedClass.fields(AnnotatedClass.java:338)
	at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector._addFields(POJOPropertiesCollector.java:393)
	at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.collectAll(POJOPropertiesCollector.java:322)
	at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.getPropertyMap(POJOPropertiesCollector.java:287)
	at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.getProperties(POJOPropertiesCollector.java:186)
	at com.fasterxml.jackson.databind.introspect.BasicBeanDescription._properties(BasicBeanDescription.java:164)
	at com.fasterxml.jackson.databind.introspect.BasicBeanDescription.findProperties(BasicBeanDescription.java:239)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._findCreatorsFromProperties(BasicDeserializerFactory.java:292)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._constructDefaultValueInstantiator(BasicDeserializerFactory.java:276)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.createEnumDeserializer(BasicDeserializerFactory.java:1472)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:371)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:349)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
	at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:481)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:497)
	at com.fasterxml.jackson.databind.deser.std.DelegatingDeserializer.resolve(DelegatingDeserializer.java:58)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:293)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:491)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4669)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4478)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3434)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3402)

I've tested on 2.11.0 and 2.11.1

kostousov-ds avatar Jul 06 '20 05:07 kostousov-ds

Ok I can reproduce this.

Also checked same failure occurs on 2.10 at least.

cowtowncoder avatar Jul 08 '20 03:07 cowtowncoder

So, as per commit message, added failing test; and fixed NPE part.

However, functionality will not work as expected, most likely because of the way JDK implements Enums under the hood; entries may look like fields or perhaps instance methods but are neither if I remember correctly -- so mix-in handling functionality may not be able to attach annotations as expected to them yet (class annotations work fine and are tested).

I hope to resolve the second problem too, hence leaving this issue open.

And the problem itself is that when buffering content that can not yet be used (both for polymorphic subtype handling and for dealing with unwrapped content), it is not known that type will be needed as BigDecimal -- so it will be buffered as Double (with somewhat lower overhead). While it would be possible to force storage of all floating-point values as BigDecimal, in theory, one nasty consequence but that binary formats with efficient storage format for 32- and 64-bit values would be heavily penalized by conversions between 2- and 10-based FP numbers. So it would be good to figure out something better; in case of textual format it might even make sense to defer number parsing.

But I do not know a good way yet; and changes likely need to go in a new minor version anyway (2.12.0 at earliest).

cowtowncoder avatar Jul 08 '20 20:07 cowtowncoder

Ah. The problem is that unlike POJO properties that are discovered using AnnotatedField / -Method and so on -- on which mix-ins are applied -- enum names are detected directly from fields that Enum declares, by JacksonAnnotationIntrospector. This means mix-ins are not indeed applied.

This should be fixed but will be bit bigger undertaking as it requires changes to AnnotationIntrospector interface, for one. That means it can not be fixed for versions earlier than 2.12.0.

cowtowncoder avatar Jul 11 '20 18:07 cowtowncoder

It looks like 2.15 will bring support for lowercasing of serialized enums per #3053, which is one use case I have for enum mix-ins. I'd rather configure the output case on a per-mix-in basis, so I'd still very much appreciate this feature.

ianbrandt avatar Mar 14 '23 22:03 ianbrandt

It would be great to get this fixed but right now there is no good plan to do that, unfortunately.

But if anyone wants to tackle it, I'd be happy to help get PR ready.

/cc @JooHyukKim this would be another Enum-related challenge. :)

cowtowncoder avatar Mar 14 '23 23:03 cowtowncoder

@cowtowncoder thankssss for the mention! Like you thought, I did try tackling already 🤣.

That time I got caught up with other PR, but now that you mention it, I will go back to it now. Do you think below method would be the right place to start?

https://github.com/FasterXML/jackson-databind/blob/95f29d2818634be6e7a396d5d2cb1d9dcb12997e/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java#L1664

And possibly somewhere in that method, do introspection like...

Class<?> mixInClass = ctxt.getConfig().findMixInClassFor(type.getRawClass());
JavaType mixInType = ctxt.constructType(mixInClass);

then pass in additional lookup for the construction of an EnumDeserializer?

JooHyukKim avatar Mar 14 '23 23:03 JooHyukKim

@JooHyukKim Ideally it would all work through standard mix-in handling and not enum-specific handling. But then again, that may be difficult in its own way. FWTW with POJOs AnnotatedClass already has all mix-ins mixed in.

I think a starting challenge is figuring out what Enum values look like, to find how mix-ins apply. I don't remember exactly what entries are (methods?), esp. in case of method overrides but they were not quite what I expected.

cowtowncoder avatar Mar 15 '23 00:03 cowtowncoder

I realized I have a need to sometimes de-snake-case and rename in addition to lowercase my enum values, so it's really exciting to see the pending PR for this. Mix-ins should prove a lot nicer than writing custom serializers and deserializers. 🎉

ianbrandt avatar Mar 21 '23 23:03 ianbrandt

@ianbrandt Thank you for the support! 🙏🏼 Though probably we might have to write code over again, you could say it's in progress.

JooHyukKim avatar Mar 21 '23 23:03 JooHyukKim

If we could figure out how to match Enum class structure for annotation flattening, that'd be great. I forget exact way Enum values map to "regular" class constructs but it was somewhat non-intuitive (i.e. they had to sort of hack it back in Java .... 1.4? or whenever they were added)

cowtowncoder avatar Mar 22 '23 15:03 cowtowncoder

I will go check. 🙆🏽‍♂️🙆🏽‍♂️ As far as I know,

  1. Each Enums values are compiled as ‘public static final SOME_VALUE’.
  2. Since they are fields, annotation flattening should be done in ‘AnnotatedFieldCollector’.
  3. “Annotation Flattening” means apply all annotations of matching fields from mixin class to target class. This is what you mean right? @cowtowncoder

JooHyukKim avatar Mar 22 '23 21:03 JooHyukKim

@JooHyukKim correct, that's the idea. Change existing machinery to support mix-ins for Enums in general way (both for Enum class and enum values).

Main concerns/questions are just that:

  1. When using overrides in enum definition, do definitions change (there's some sub-classing involved as I recall, but maybe it won't affect annotation handling)
  2. From users POV, how should mix-ins look like? Do they have to create throw-away Enum types; or just know to specify static Fields with matching names? Basically, not trying to specify, say, Methods as those would not match (or should they? I guess with enough work it's possible but could get ugly).

cowtowncoder avatar Mar 23 '23 18:03 cowtowncoder

@jinwookh correct, that's t

I think someone else is tagged 😅here.

JooHyukKim avatar Mar 24 '23 05:03 JooHyukKim

... correct, that's t

I think someone else is tagged 😅here.

Sorry. Auto-completion for the win. :-/

cowtowncoder avatar Mar 24 '23 16:03 cowtowncoder

I realized I have a need to sometimes de-snake-case and rename in addition to lowercase my enum values, so it's really exciting to see the pending PR for this. Mix-ins should prove a lot nicer than writing custom serializers and deserializers. 🎉

I just realized there might be a feature that might satisfy your needs, @ianbrandt ! 👍🏻

There is a recent PR https://github.com/FasterXML/jackson-databind/pull/3792 that features naming strategy for enum's. Check below code for example usage. If your desired naming conversion is not supported, but is general enough, I think we can discuss a new EnumNamingStrategy in a new issue.

@EnumNaming(EnumNamingStrategies.CamelCaseStrategy.class)
    static enum EnumFlavorA {
        PEANUT_BUTTER, // handled as peanutButter
        SALTED_CARAMEL, // as saltedCaramel
        @JsonEnumDefaultValue
        VANILLA; // 
    }

JooHyukKim avatar Apr 09 '23 05:04 JooHyukKim

It seems this issue can be closed now, addressed by #3990 ?

JooHyukKim avatar Nov 05 '23 14:11 JooHyukKim

Yes, included as fixed in 2.16.0-rc1 release notes, just forgot to close issue itself.

cowtowncoder avatar Nov 06 '23 02:11 cowtowncoder