Add support for extracting extensions from options

[htdvisser]

ping @pseudomuto

What is Changing?

This Pull Request adds support for extension-defined options for messages, fields, methods, etc. It's the first step in resolving pseudomuto/protoc-gen-doc#202 and pseudomuto/protoc-gen-doc#233. The use case is an RPC service with custom options, such as the (google.api.http) MethodOption in the following example:

import "google/api/annotations.proto";

service Demo {
  rpc Unary (Foo) returns (Bar) {
    option (google.api.http) = {
      post: "/unary"
    };
  }
}

and the (validator.field) FieldOption in this one:

import "github.com/mwitkow/go-proto-validators/validator.proto";

message Foo {
  int32 percentage = 1 [(validator.field) = {int_gte: 0, int_lte: 100}];
}

How is it Changing?

I can imagine we wouldn't make everyone happy by importing all possible extensions that are out there, so plugins will have to register the ones they use to protokit by importing the packages that define the extensions:

import _ "google.golang.org/genproto/googleapis/api/annotations"
import _ "github.com/mwitkow/go-proto-validators"

This works because the generated .pb.go files in those packages contain something like:

func init() {
	proto.RegisterExtension(E_Http)
}

and we simply range over the registered extensions to match them with the ones in the options.

What Could Go Wrong?

Not much. The setExtensions func is only called if there are options to begin with, it only looks at extensions that are registered for the given options, and skips anything that errors. I also added a test case to make sure this work as intended.

[htdvisser]

Please wait with review/merge until I've added some more test coverage to make codecov happy

[htdvisser]

@pseudomuto looks like codecov is satisfied now. Please review when you have time 😃

[pseudomuto]

Overall this looks great! Thanks for taking the time to put a PR together!

plugins will have to register the ones they use to protokit by importing the packages that define the extensions

This probably deserves a note in the README. Would also be good to use this feature in the jsonator example.

I'd also like to ensure that this works for proto3 if possible.

We'll also need to update the CHANGELOG (under Unreleased).

[pseudomuto]

Seems like this is only used in parser_test.go. Should we just move it there?

[htdvisser]

Fine with me. Will do

[pseudomuto]

Rather than doing this here, can we make newCommon initialize the map? That way downstream apps don't need to check for nil before accessing a value

[htdvisser]

Accessing (reading) fields in a nil map has the same results as accessing them from an empty map. The only problem would be with writing, but that shouldn't happen downstream.

[htdvisser]

Thanks for the review. I'll update the README, example and CHANGELOG 👍

Unfortunately proto3 doesn't have the concept of extensions, the docs also refer to proto2 when talking about custom options. I could define the extensions for testing in a separate proto2 file, and import that from the other testing proto files. That would take some time though.

[pseudomuto]

I understand extensions aren't available, but options are. For example, you can use google.api.options in an RPC defined in a proto3 file, right?

[htdvisser]

Hi @pseudomuto, I'm back with the requested changes.

• Removed parser_extensions_test.go
• Added (google.api.http) to README and jsonator example
• Added tests for proto3 (instead of only proto2)
• Updated CHANGELOG

The jsonator example now depends on google/api/annotations.proto for the (google.api.http) option. As those protos aren't installed with protoc by default, I made the generator fetch those from Github.

The other changes are pretty straightforward.

[htdvisser]

@pseudomuto: missed the notification perhaps?

[arturgspb]

Thanks a lot!
Jsonator for api.google.http It's really useful patch for me! I'm looking forward to!

[rogchap]

Is this any closer to being merged?

[pseudomuto]

Sorry for the delay, this LGTM 👍