@JsonEnumDefaultValue should take precedence over FAIL_ON_NUMBERS_FOR_ENUMS

[Stephan202]

Consider the following ObjectMapper definition:

ObjectMapper m =
        new ObjectMapper()
                .enable(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)
                .enable(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE);

With this ObjectMapper, when one attempts to deserialize an enum value V for an enum with an @JsonEnumDefaultValue element, the deserialization will:

• Pass, if V is a valid enum value. ✅
• Pass, if V is an invalid non-integer value. ✅
• Fail, if V is an invalid integer value. ❌

To me this seems highly unintuitive. I would have expected the READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE feature to take precedence over the FAIL_ON_NUMBERS_FOR_ENUMS feature in those cases where it applies (i.e., when deserializing an enum with a default element).

I've put together a test class enumerating the relevant cases. See the inline comments towards the bottom. In four cases I feel that Jackson's current behavior is not as one might expect.

import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS;
import static com.fasterxml.jackson.databind.DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;

import com.fasterxml.jackson.annotation.JsonEnumDefaultValue;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.databind.ObjectMapper;
import javax.annotation.Nullable;
import org.testng.annotations.Test;

public final class JacksonEnumTest {
    @Test
    public void testWithoutCustomFeatures() {
        final ObjectMapper m = new ObjectMapper();

        verifyDeserialization(m, "ZERO", SimpleEnum.class, SimpleEnum.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnum.class, SimpleEnum.ONE);
        verifyDeserialization(m, "TWO", SimpleEnum.class, null);
        verifyDeserialization(m, "0", SimpleEnum.class, SimpleEnum.ZERO);
        verifyDeserialization(m, "1", SimpleEnum.class, SimpleEnum.ONE);
        verifyDeserialization(m, "2", SimpleEnum.class, null);

        verifyDeserialization(m, "ZERO", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ONE);
        verifyDeserialization(m, "TWO", SimpleEnumWithDefault.class, null);
        verifyDeserialization(m, "0", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ONE);
        verifyDeserialization(m, "2", SimpleEnumWithDefault.class, null);

        verifyDeserialization(m, "ZERO", CustomEnum.class, null);
        verifyDeserialization(m, "ONE", CustomEnum.class, null);
        verifyDeserialization(m, "TWO", CustomEnum.class, null);
        verifyDeserialization(m, "0", CustomEnum.class, CustomEnum.ZERO);
        verifyDeserialization(m, "1", CustomEnum.class, CustomEnum.ONE);
        verifyDeserialization(m, "2", CustomEnum.class, null);

        verifyDeserialization(m, "ZERO", CustomEnumWithDefault.class, null);
        verifyDeserialization(m, "ONE", CustomEnumWithDefault.class, null);
        verifyDeserialization(m, "TWO", CustomEnumWithDefault.class, null);
        verifyDeserialization(m, "0", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", CustomEnumWithDefault.class, CustomEnumWithDefault.ONE);
        verifyDeserialization(m, "2", CustomEnumWithDefault.class, null);
    }

    @Test
    public void testWithFailOnNumbersForEnums() {
        final ObjectMapper m = new ObjectMapper().enable(FAIL_ON_NUMBERS_FOR_ENUMS);

        verifyDeserialization(m, "ZERO", SimpleEnum.class, SimpleEnum.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnum.class, SimpleEnum.ONE);
        verifyDeserialization(m, "TWO", SimpleEnum.class, null);
        verifyDeserialization(m, "0", SimpleEnum.class, null);
        verifyDeserialization(m, "1", SimpleEnum.class, null);
        verifyDeserialization(m, "2", SimpleEnum.class, null);

        verifyDeserialization(m, "ZERO", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ONE);
        verifyDeserialization(m, "TWO", SimpleEnumWithDefault.class, null);
        verifyDeserialization(m, "0", SimpleEnumWithDefault.class, null);
        verifyDeserialization(m, "1", SimpleEnumWithDefault.class, null);
        verifyDeserialization(m, "2", SimpleEnumWithDefault.class, null);

        verifyDeserialization(m, "ZERO", CustomEnum.class, null);
        verifyDeserialization(m, "ONE", CustomEnum.class, null);
        verifyDeserialization(m, "TWO", CustomEnum.class, null);
        verifyDeserialization(m, "0", CustomEnum.class, CustomEnum.ZERO);
        verifyDeserialization(m, "1", CustomEnum.class, CustomEnum.ONE);
        verifyDeserialization(m, "2", CustomEnum.class, null);

        verifyDeserialization(m, "ZERO", CustomEnumWithDefault.class, null);
        verifyDeserialization(m, "ONE", CustomEnumWithDefault.class, null);
        verifyDeserialization(m, "TWO", CustomEnumWithDefault.class, null);
        verifyDeserialization(m, "0", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", CustomEnumWithDefault.class, CustomEnumWithDefault.ONE);
        verifyDeserialization(m, "2", CustomEnumWithDefault.class, null);
    }

    @Test
    public void testWithReadUnknownEnumValuesUsingDefaultValue() {
        final ObjectMapper m =
                new ObjectMapper().enable(READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE);

        verifyDeserialization(m, "ZERO", SimpleEnum.class, SimpleEnum.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnum.class, SimpleEnum.ONE);
        verifyDeserialization(m, "TWO", SimpleEnum.class, null);
        verifyDeserialization(m, "0", SimpleEnum.class, SimpleEnum.ZERO);
        verifyDeserialization(m, "1", SimpleEnum.class, SimpleEnum.ONE);
        verifyDeserialization(m, "2", SimpleEnum.class, null);

        verifyDeserialization(m, "ZERO", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ONE);
        verifyDeserialization(m, "TWO", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "0", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ONE);
        verifyDeserialization(m, "2", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);

        verifyDeserialization(m, "ZERO", CustomEnum.class, null);
        verifyDeserialization(m, "ONE", CustomEnum.class, null);
        verifyDeserialization(m, "TWO", CustomEnum.class, null);
        verifyDeserialization(m, "0", CustomEnum.class, CustomEnum.ZERO);
        verifyDeserialization(m, "1", CustomEnum.class, CustomEnum.ONE);
        verifyDeserialization(m, "2", CustomEnum.class, null);

        verifyDeserialization(m, "ZERO", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "ONE", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "TWO", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "0", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", CustomEnumWithDefault.class, CustomEnumWithDefault.ONE);
        verifyDeserialization(m, "2", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
    }

    @Test
    public void testWithFailOnNumbersForEnumsAndReadUnknownEnumValuesUsingDefaultValue() {
ObjectMapper m = new ObjectMapper()
		                .enable(FAIL_ON_NUMBERS_FOR_ENUMS)
		                .enable(READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE);

        verifyDeserialization(m, "ZERO", SimpleEnum.class, SimpleEnum.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnum.class, SimpleEnum.ONE);
        verifyDeserialization(m, "TWO", SimpleEnum.class, null);
        verifyDeserialization(m, "0", SimpleEnum.class, null);
        verifyDeserialization(m, "1", SimpleEnum.class, null);
        verifyDeserialization(m, "2", SimpleEnum.class, null);

        verifyDeserialization(m, "ZERO", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "ONE", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ONE);
        verifyDeserialization(m, "TWO", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        // The three tests below fail; Jackson throws an exception on the basis that
        // "FAIL_ON_NUMBERS_FOR_ENUMS" is enabled. I claim the default value should be returned instead.
        verifyDeserialization(m, "0", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);
        verifyDeserialization(m, "2", SimpleEnumWithDefault.class, SimpleEnumWithDefault.ZERO);

        verifyDeserialization(m, "ZERO", CustomEnum.class, null);
        verifyDeserialization(m, "ONE", CustomEnum.class, null);
        verifyDeserialization(m, "TWO", CustomEnum.class, null);
        verifyDeserialization(m, "0", CustomEnum.class, CustomEnum.ZERO);
        verifyDeserialization(m, "1", CustomEnum.class, CustomEnum.ONE);
        verifyDeserialization(m, "2", CustomEnum.class, null);

        verifyDeserialization(m, "ZERO", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "ONE", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "TWO", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "0", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
        verifyDeserialization(m, "1", CustomEnumWithDefault.class, CustomEnumWithDefault.ONE);
        // Fails. Jackson throws an exception on the basis that "FAIL_ON_NUMBERS_FOR_ENUMS"
        // is enabled, but the default value should have been returned instead.
        verifyDeserialization(m, "2", CustomEnumWithDefault.class, CustomEnumWithDefault.ZERO);
    }

    private <T> void verifyDeserialization(
            final ObjectMapper objectMapper,
            final String fromValue,
            final Class<T> toValueType,
            @Nullable final T toValue) {
        if (toValue != null) {
            assertEquals(objectMapper.convertValue(fromValue, toValueType), toValue);
        } else {
            try {
                objectMapper.convertValue(fromValue, toValueType);
                fail("Deserialization should have failed");
            } catch (final IllegalArgumentException e) {
                /* Expected. */
            }
        }
    }

    enum SimpleEnum {
        ZERO,
        ONE;
    }

    enum SimpleEnumWithDefault {
        @JsonEnumDefaultValue
        ZERO,
        ONE;
    }

    enum CustomEnum {
        ZERO(0),
        ONE(1);

        private final int number;

        CustomEnum(final int number) {
            this.number = number;
        }

        @JsonValue
        int getNumber() {
            return this.number;
        }
    }

    enum CustomEnumWithDefault {
        @JsonEnumDefaultValue
        ZERO(0),
        ONE(1);

        private final int number;

        CustomEnumWithDefault(final int number) {
            this.number = number;
        }

        @JsonValue
        int getNumber() {
            return this.number;
        }
    }
}

Would love to hear your thoughts on this. (If there is some other feature I should enable to get the behavior I'm looking for, let me know :).)

[cowtowncoder]

Thank you for reporting this, providing comprehensive tests! I'll have a look; I assume you are right unless I find something to suggest otherwise, and see if I can change behavior as requested.

[cowtowncoder]

Yes, I agree. Handling of "number as string" is bit odd altogether, so I changed it to only be tried unless failure would occur; either way given tests pass and behavior is improved, I agree.

[Stephan202]

Super. Thanks for the rapid fix! I'll keep in mind to use the test format in EnumDefaultReadTest.java next time around. (Though hopefully there'll be no next time around ;).)