Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE feature with Protobuf #398

Open
itsmoonrack opened this issue Sep 22, 2023 · 5 comments
Labels

Comments

@itsmoonrack
Copy link

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.

@cowtowncoder cowtowncoder changed the title Honor READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE feature Honor READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE feature with Protobuf Sep 23, 2023
@cowtowncoder cowtowncoder changed the title Honor READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE feature with Protobuf Honor READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE feature with Protobuf Sep 23, 2023
@cowtowncoder
Copy link
Member

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.

@itsmoonrack
Copy link
Author

itsmoonrack commented Sep 25, 2023

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).

@cowtowncoder
Copy link
Member

@itsmoonrack That makes sense to me, yes.

@cowtowncoder
Copy link
Member

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.

@itsmoonrack
Copy link
Author

Ha you are right, good point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants