Problem deserializing some type of Enums when using PropertyNamingStrategy

[Badbond]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

When using a mapper with a PropertyNamingStrategy configured, the following exception is thrown when trying to deserialize an enum that contains a field with the same name as one of the enum constants:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Multiple fields representing property "foo": tech.picnic.config.util.EnumDeserializationTest$SomeEnum#FOO vs tech.picnic.config.util.EnumDeserializationTest$SomeEnum#foo
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1887)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:289)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:265)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:163)
[...]

It seems that now enum constants are also considered fields, which can clash with an enum's field when they are renamed. See also 2134584.

Version Information

2.16.1

Reproduction

  @Test
  void shouldDeserialize() throws IOException {
    var objectMapper =
            JsonMapper.builder()
              .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
              .build();
    assertThat(objectMapper.readValue("\"FOO\"", SomeEnum.class))
            .isEqualTo(SomeEnum.FOO);
  }

  enum SomeEnum {
    FOO(0);

    public final int foo;

    SomeEnum(int foo) {
      this.foo = foo;
    }
  }

Expected behavior

Similar to Jackson 2.15.3, I would expect this enum to be deserializable given we don't specify any mixins on the constants.

Additional context

The reproduction case above has a public field, but the issue is also apparent if the field is private and the following visibility is configured:

  .visibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
  .visibility(PropertyAccessor.CREATOR, JsonAutoDetect.Visibility.ANY)
  .visibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY)

[cowtowncoder]

Ok that does sound like a bug. Thank you for reporting it.

And special thank you for including reproduction (unit test).

[cowtowncoder]

@JooHyukKim Probably due to refactoring, hopefully there is an easy fix given it is easy to reproduce.

[JooHyukKim]

By intution, I thought we would just skip renaming if target field is enum.
Probably, inside POJOPropertiesCollector._renameUsing(), but I am not 100% yet if it would cover all cases.

[cowtowncoder]

By intuition, I thought we would just skip renaming if target field is enum. Probably, inside POJOPropertiesCollector._renameUsing(), but I am not 100% yet if it would cover all cases.

Not ideal (conceptually), but yes that sound like a straight-forward way to do it.
Fundamental problem being that Enums are really very different from POJOs, so general POJO handling can't really work. Except if/when "enums as POJOs" serialization is enabled, in which case some of processing is needed.
So because of this change like you suggested probably is the way to go.

[JooHyukKim]

PTAL at #4311, @cowtowncoder ?

[cowtowncoder]

@JooHyukKim Fix works here, but I wonder if it might have caused a new unit test failure for Ion backend:

FasterXML/jackson-dataformats-binary#454

but I haven't yet had time to dig into whether test is at fault or change here.

It could also be something completely different, but reference to Enum made me wonder.

[JooHyukKim]

FasterXML/jackson-dataformats-binary#454

WIll look into it later today. After work, thank you for letting me know! @cowtowncoder

[JooHyukKim]

The fix for this issue was incomplete... Made #4313. Could you please have a look @cowtowncoder ?

[cowtowncoder]

Will look soon -- also realized the approach of the fix is wrong; I didn't think about it enough. I don't think it's super difficult to fix so when I get a chance I can tackle it too (didn't have time last night).