Deserialization of enums with name defined with different cases leads to InvalidDefinitionException: Multiple fields representing property

[sbailliez]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

I'm using a thirdparty client that is deserializing json where an enum is made of variations like RGBA and RGBa. When upgrading our dependencies from Jackson 2.14.1 to 2.16.1 this API started failing with

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Multiple fields representing property "rgba" ...

Version Information

2.16.1

Looks like the bug was introduced in 2.16.0 as it works with 2.15.4

Reproduction

    public static class Bug {
        public enum ColorMode {
            RGB, RGBa, RGBA
        }
        public ColorMode colorMode;
    }

    @Test
    public void enumFails() throws Exception {
        String json = "{ \"color_mode\": \"RGBa\"}";
        ObjectMapper mapper = new ObjectMapper();
        mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE);
        mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
        Bug bug = mapper.readValue(json, Bug.class);
    }

Expected behavior

This should succeed.

Additional context

For this particular, I worked around this issue right now by patching the objectmapper created within the client and adding a custom deserializer for that enum:

    public static class EnumDeserializer extends StdDeserializer<File.ColorMode> {
        public EnumDeserializer() {
            this(null);
        }

        public EnumDeserializer(Class<?> vc) {
            super(vc);
        }

        @Override
        public File.ColorMode deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException {
            JsonNode node = p.getCodec().readTree(p);
            String value = node.asText();
            return value != null ? File.ColorMode.valueOf(value) : null;
        }
    }
    
    // .... reflective access to find the objectmapper in the thirdparty client...
    mapper.registerModule(new SimpleModule("patch-enum")
         .addDeserializer(File.ColorMode.class, new EnumDeserializer()));

[cowtowncoder]

Thank you for reporting this, @sbailliez ! It definitely looks like a bug.

Two immediate guesses for possible triggering part (will try to see if they are related).

1. Use of naming strategy would trigger bogus conflict
2. Case-insensitivity incorrectly enabled by some processing resulting in bogus conflict.

[cowtowncoder]

Yes, looks like PropertyNamingStrategy is triggering the issue.

Possibly related:

• Problem deserializing some type of Enums when using PropertyNamingStrategy #4302
• Fix Enum deserialization to use @JsonProperty, @JsonAlias even if EnumNamingStrategy used #4037 / Use @JsonProperty over EnumNamingStrategy for Enum serialization #4040
• Add @EnumNaming, EnumNamingStrategy to allow use of naming strategies for Enums #2667 (@EnumNaming)

[cowtowncoder]

Quick note: I haven't dig into why "RGBA" and '"RGBa"are translated by convention, but you can work around the issue by adding@JsonProperty` into one of conflicting values:

    enum ColorMode {
        RGB, RGBa,
        @JsonProperty("RGBA")
        RGBA
    }

[cowtowncoder]

Ok the problem is that names do translate both to rgba as per SNAKE_CASE_STRATEGY:

https://github.com/FasterXML/jackson-databind/blob/2.17/src/main/java/com/fasterxml/jackson/databind/PropertyNamingStrategies.java#L171

So this is a problem that was not detected until 2.15 (I think) -- behavior in 2.16 is as expected going forward.

In case where naming strategy converts value into conflicts, annotation is needed, or possible @EnumNaming override: however, adding:

@EnumNaming(EnumNamingStrategies.CamelCaseStrategy.class)

does not seem to help in this case (assuming it will not translate relevant values into separate ones)

[cowtowncoder]

Closing as wont-fix; solution is to apply annotation on one of conflicting values.

[cowtowncoder]

/cc @JooHyukKim you might be interested in this -- I had forgotten that the per-mapper default PropertyNamingStrategy is applied to Enum values.

[JooHyukKim]

Right, from users could feel awkward who are used to having Enums unaffected by PropertyNamingStrategy.
I should've thought this through wrt PropertyNameStrategy 🥲.

WRT, keeping backward compatibility either...

1. Keep the Enums behave as they are?
2. Or have EnumFeature as....

    // name yet TBD
    EnumFeature.APPLY_PROPERTY_NAMING_STRATEGY_TO_ENUMS

...such to keep the PropertyNamingStrategy away from Enums unless configured so.

[sbailliez]

This is probably the first time that I see that the naming strategy was "supposed" to be applied to enums values. I thought there were expected to be serialized/deserialized as is and you would use valueOf or fromString to impact it. So I would vote for keeping backward compatibility.

In my case this is a thirdparty library, so I cannot add annotations but will open a bug for trying to annotate it explicitly to make it compatible for all jackson versions.

[cowtowncoder]

I must say I did not remember that PropertyNamingStrategy was to have worked on enum values. But at the same time, behavior that 2.16 has out is, by know, the behavioral baseline.

I think EnumFeature to control this would be useful; that'd need to go in 2.17, and should have default settings that reflect what 2.16 does.

This could mean that strategy should, by default, apply, but may be disabled by the new EnumFeature.

[JooHyukKim]

Okay, so will modify PR #4414 to implement the new EnumFeature accordingly (probably later today).

But WRT version, shouldn't we have the feature provided in 2.16? So users upgrading from 2.15 can easily fix things, like case here.

[cowtowncoder]

No, as per SemVer no features may be added in patch releases: all patch releases should have same API -- additions may be done in minor releases.

But let's think this through first, before providing more solutions before understanding the problem.

What I'd want to know is the actual scope of problem; both wrt version in which behavior changes (wrt PropertyNamingStrategy applying to Fields of Enum types), and whether it actually changes serialization of regular Enums -- or is it just some special case. Given that we haven't had many reports, I wonder how commonly this actually changes behavior.

[cowtowncoder]

@sbailliez I may have spoken too soon on this change being intentional wrt PropertyNamingStrategy. It may well be unintentional change, due to somewhat aggressive rewriting of Enum handling in 2.16 (and some in 2.15 I think).

[JooHyukKim]

Did some digging and made tests against 2.15 and 2.16

• 2.15🔗
  • PropertyNamingStrategy NOT applied to both Serialization and Deserialization
• 2.16 🔗
  • PropertyNamingStrategy applied to ONLY deserialization

[sbailliez]

@JooHyukKim do you mean for 2.15 "NOT applied to both Serialization and Deserialization"?

[sbailliez]

@cowtowncoder I believe the reason there has not been any report related to this behavior in 2.16 is that it is pretty unconventional to have enums with values that have a case difference. In the API I refer to it is probably there for backward compatibility.

[cowtowncoder]

@sbailliez True, that makes it less common. Enums often have underscores and some naming conventions change those so... but either way, it is interesting how few problems have been reported

[cowtowncoder]

@JooHyukKim Given this, I feel comfortable disabling application of PropertyNamingStrategy defaults for Enum types generally.

[cowtowncoder]

Ok so it's EnumFormatShapeTest that fails if skipping for all Enum types (wrt #2365 as per comment).
Everything else passes fwtw.

[cowtowncoder]

Ok. The trick is to check what BasicBeanDescription.findExpectedFormat() does, but from POJOPropertiesCollector -- and if we do that, I think ideally it'd be collected by POJOPropertiesCollector and then handed to BasicBeanDescription (to avoid multiple introspection calls).

But in short term (2.17) duplication would be fine.
This lookup is needed in 2.17 just for enum types, so we'd have something like:

    protected void _renameUsing(Map<String, POJOPropertyBuilder> propMap,
            PropertyNamingStrategy naming)
    {
        // _findFormatShape() needs to what `BasicBeanDescription.findExpectedFormat()`
        // (but need not take or use "defValue"; there's no defaulting)
        if (_type.isEnumType() && _findFormatShape() != Shape.OBJECT) {
            return;
        }
        ...
}

I might have time to look into this tomorrow, but if not I hope above helps.

[JooHyukKim]

Thanks @sbailliez for reproduction and all!

[cowtowncoder]

Fixed for 2.16.2