curioswitch / protobuf-jackson

High performance protobuf JSON marshaler based on Jackson.
MIT License
20 stars 4 forks source link

Enums defined in `proto2` throws an exception #8

Open jrhee17 opened 2 years ago

jrhee17 commented 2 years ago

When one tries to deserialize json into proto2 with enums defined, an exception seems to be thrown. This behavior seems to differ from upstream, since only proto2 specific features aren't supported. I haven't gone through the code so I'm unsure how trivial the fix will be, but wanted to file an issue to record my findings.


If one defines an enum in proto2

syntax = "proto2";

...

enum Foo {
  A = 1;
  B = 2;
}

message Proto2Response {
  required string message = 1;
  optional Foo foo = 2;
}

And tries to parse json into proto

JsonParser parser = new ObjectMapper().getFactory().createParser("{\"message\": \"hello\", \"foo\": 1}");
MessageMarshaller marshaller =
    MessageMarshaller.builder().register(Proto2Test.Proto2Response.getDefaultInstance()).build();
marshaller.mergeValue(parser, Proto2Test.Proto2Response.newBuilder());
assertThat(parser.isClosed()).isFalse();

the following exception is thrown

Could not find setter.
java.lang.IllegalStateException: Could not find setter.
    at org.curioswitch.common.protobuf.json.ProtoFieldInfo.setValueMethod(ProtoFieldInfo.java:213)
    at org.curioswitch.common.protobuf.json.DoParse.setFieldValue(DoParse.java:433)
    at org.curioswitch.common.protobuf.json.DoParse.apply(DoParse.java:391)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$MethodPool$Record$ForDefinedMethod$WithBody.applyCode(TypeWriter.java:719)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$MethodPool$Record$ForDefinedMethod$WithBody.applyBody(TypeWriter.java:704)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$MethodPool$Record$ForDefinedMethod.apply(TypeWriter.java:611)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForCreation.create(TypeWriter.java:5959)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default.make(TypeWriter.java:2213)
    at net.bytebuddy.dynamic.scaffold.subclass.SubclassDynamicTypeBuilder.make(SubclassDynamicTypeBuilder.java:232)
    at net.bytebuddy.dynamic.scaffold.subclass.SubclassDynamicTypeBuilder.make(SubclassDynamicTypeBuilder.java:204)
    at net.bytebuddy.dynamic.DynamicType$Builder$AbstractBase.make(DynamicType.java:3661)
    at net.bytebuddy.dynamic.DynamicType$Builder$AbstractBase$Delegator.make(DynamicType.java:3899)
    at org.curioswitch.common.protobuf.json.TypeSpecificMarshaller.buildOrFindMarshaller(TypeSpecificMarshaller.java:257)
    at org.curioswitch.common.protobuf.json.TypeSpecificMarshaller.buildAndAdd(TypeSpecificMarshaller.java:140)
    at org.curioswitch.common.protobuf.json.MessageMarshaller$Builder.build(MessageMarshaller.java:407)
    at org.curioswitch.common.protobuf.json.MessageMarshallerTest.proto2Enum(MessageMarshallerTest.java:1021)
    ...

It seems like proto2 generates stubs differently for enums

...
public Builder setFoo(org.curioswitch.common.protobuf.json.test.Proto2Test.Foo value) {
        if (value == null) {
...
jrhee17 commented 2 years ago

reproducer: https://github.com/jrhee17/protobuf-jackson/tree/bugfix/proto2_enum

chokoswitch commented 2 years ago

Hi @jrhee17 - upstream defined JSON serialization for protobuf only in proto3, not proto2

https://developers.google.com/protocol-buffers/docs/proto3#json

So this project only targets proto3 primarily and proto2 support is best effort. It was definitely quite intentional to use only numeric value setting for enums from the byte-buddy generated code because having to reference the actual Java class for the enum makes things much trickier.