prost icon indicating copy to clipboard operation
prost copied to clipboard

Are proto2 extensions supported?

Open im-0 opened this issue 6 years ago • 10 comments

https://developers.google.com/protocol-buffers/docs/proto#extensions

I have existing proto2 protocol definition which uses extensions. And I'm unable to understand how to work with such protocol using prost. Am I looking into wrong direction or this feature is not supported?

im-0 avatar May 18 '18 08:05 im-0

I found a workaround. This protocol definition:

syntax = "proto2";

message Header {
        required uint64 msg_id = 1;
        extensions 100 to max;
}

message FooMessage {
        required string some_parameter = 1;
        extend Header {
                optional FooMessage msg_foo = 101;
        }
}

message BarMessage {
        required string another_parameter = 1;
        extend Header {
                optional BarMessage msg_bar = 102;
        }
}

can be rewritten as this:

syntax = "proto2";

message FooMessage {
        required string some_parameter = 1;
}

message BarMessage {
        required string another_parameter = 1;
}

message Header {
        required uint64 msg_id = 1;

        optional FooMessage msg_foo = 101;
        optional BarMessage msg_bar = 102;
}

This does not change the binary format, so both definitions are equal. And the latter one works out of the box with prost.

im-0 avatar May 18 '18 16:05 im-0

Glad you found a workaround. Extensions aren't supported, since I haven't personally needed them, and they are more-or-less deprecated. I'm not against adding support in prost, but obviously it will be low down on the priority list. If you want to hack on it then please feel free.

danburkert avatar May 18 '18 18:05 danburkert

If you want to hack on it then please feel free.

I already have a sufficient workaround and it is very unlikely that I will need this feature again. So, you may close this issue if you want.

im-0 avatar May 18 '18 19:05 im-0

Keep it open :-)

Custom options are implemented as extensions, even in proto3: https://developers.google.com/protocol-buffers/docs/proto3#custom_options

Seems conceivable that prost could allow customizing generated code via options.

kamalmarhubi avatar May 26 '18 02:05 kamalmarhubi

I find myself in need of extensions for the reason mentioned above. :)

I'm working on a Rust port of https://github.com/Xorlev/grpc-jersey using tonic and tower-web, but I'll need to parse options:

service TestService {
    rpc TestMethod (TestRequest) returns (TestResponse) {
        option (google.api.http).get = "/users/{string}/{uint32}/{nested_type.field_f64}";
    }
}

If you're still open to the idea @danburkert I'll start up a draft PR. I suspect it should be reasonably similar to Java protobufs + prost-build would gain an ExtensionRegistry argument such that the parsed MethodDescriptorProto would have an extensions field populated.

It wouldn't be a bad time to drag #117 along for the ride either, but they can be separate.

Xorlev avatar Feb 08 '20 19:02 Xorlev

I'm also interested in this.

@Xorlev I think it would require changing the generated code to carry around unknown fields, so that they may be accessed by looking them up via extension number. Or are you thinking of something else?

tiziano88 avatar Mar 04 '20 21:03 tiziano88

In the Java version of protos, extension registries are declared during deserialization, and those fields are eagerly extracted out. If an extension isn't recognized, it goes to unknown fields. This has the advantage of checking validity at deserialization time rather than extension access time.

They're mechanisms that work well together but don't necessarily need to both be implemented.

On Wed, Mar 4, 2020, 11:24 Tiziano Santoro [email protected] wrote:

I'm also interested in this.

@Xorlev https://github.com/Xorlev I think it would require changing the generated code to carry around unknown fields, so that they may be accessed by looking them up via extension number. Or are you thinking of something else?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/danburkert/prost/issues/100?email_source=notifications&email_token=AACVDSQ24BROZKSISY4OZCLRF3BHTA5CNFSM4FARIA3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN2MALA#issuecomment-594853932, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACVDSTOOBC5UWOQJG4JF53RF3BHTANCNFSM4FARIA3A .

Xorlev avatar Mar 05 '20 03:03 Xorlev

@Xorlev did you manage to start working on this? we're porting a lot of gRPC stuff to Rust at work and need this so just wanted to make sure we aren't duplicating work. happy to help as well

amilkov3 avatar Mar 31 '20 01:03 amilkov3

Proto2 Extensions are also required by the GTFS (General Transit Feed Specification) realtime spec. For testing purposes, I will try to use the workaround proposed by @im-0, but it would be great if we could follow the official way for our gtfs-rt extension for our project Dystonse.

lenaschimmel avatar Jun 30 '20 20:06 lenaschimmel

@Xorlev I'd love to understand your proposed solution better, so I could try to implement it. We need the google.http.api extension at work.

One thing in particular I didn't understand was this:

In the Java version of protos, extension registries are declared during deserialization

What deserialization are you referring to? The deserialization of the literal value on the right-hand-side of option(my_option) = { foo: "bar"};

imalsogreg avatar Aug 16 '21 02:08 imalsogreg