jackson-dataformats-binary icon indicating copy to clipboard operation
jackson-dataformats-binary copied to clipboard

Honor `READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE` feature with Protobuf

Open itsmoonrack opened this issue 9 months ago • 5 comments

The wire plugin uses a mechanism of fall-back to default enum value (index 0) when the index is above the one in the schema, this ensure we don't break compatibility when they add a new field upstream, our system still can deserialize the message by fall-back on the default value. Of course some systems want that an exception be thrown when encountering an unknown value.

This behaviour could be setup (enabled or disabled) through the standard DeserializationFeature: READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE

if true we fall-back on the default enum value (index 0) and if false we throw an exception.

itsmoonrack avatar Sep 22 '23 08:09 itsmoonrack

This makes sense from functionality perspective, +1.

One challenge is that of exposing "unknown" value: parser has no way to access information at databind level so there'd have to be some convention of passing maybe marker value (empty String? -1?), or perhaps JsonToken.VALUE_NULL would work?

As with other ideas this could be configurable with a new ProtobufParser.Feature feature flag.

It might also possible to change some of handling of EnumDeserializer in jackson-databind to recognize certain "default value" or "unknown enum" marker.

cowtowncoder avatar Sep 23 '23 22:09 cowtowncoder

Thanks for your reply.

The way the wire plugin resolve this is that they default on the position 0 when the enum not found.

Let's say you have the following schema:

message Test {
    enum TestEnum {
        DEFAULT_VALUE = 0;
        VALUE_A = 1;
        VALUE_C = 3;
    }
}

With no option to fall-back if the upstream server send the value at index 2 or 4 and above it will fail as expected with a com.fasterxml.jackson.core.JsonParseException: Unknown id 2 (for enum field type) for instance.

With the feature READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE to fall-back instead of the Unknown id exception, we would have the value mapped at index 0 (DEFAULT_VALUE String if enable READ_ENUMS_USING_TO_STRING feature).

itsmoonrack avatar Sep 25 '23 08:09 itsmoonrack

@itsmoonrack That makes sense to me, yes.

cowtowncoder avatar Sep 26 '23 23:09 cowtowncoder

Also realized now that DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE cannot really be used since Protobuf decoder (ProtobufParser) is implemented at streaming (jackson-core) level and has no access to databind concepts or configuration. So we would need ProtobufParser.Feature here. Doable, I think.

cowtowncoder avatar Nov 11 '23 02:11 cowtowncoder

Ha you are right, good point.

itsmoonrack avatar Nov 12 '23 17:11 itsmoonrack