protoc-gen-swagger: Support discriminator in oneof definitions

[inotnako]

Hi! You have just field discriminator in openapiv2.proto. But field not used for generating spec. It can be useful for show oneof message. discriminator example here on petType

[achew22]

Hey @antonikonovalov, thanks for your interest in the project. You're right that we don't currently have support for it :frowning_face:. Fortunately, the feature is just a PR away!

If you wanted to take a stab at this, I would be more than happy to help. I'll even start by pointing you to the right-ish place. To add this in you could either parse the opts to handle the case where the user explicitly adds it, or you could parse the type information to detect when a oneof has been used and automatically add the descriptor field.

Good luck and please comment if you have any questions or problems.

[inotnako]

Ok, I will try do it. Looks clear

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tete17]

I wouldn't mind giving a try myself if https://github.com/grpc-ecosystem/grpc-gateway/pull/1059 is not planning to add it.

As long as I can maybe get a bit of a support from @achew22 as he volunteer before to offer help

[johanbrandhorst]

Hi @tete17. We'd be happy to help you get this in. Most of the code changes would be in and around https://github.com/grpc-ecosystem/grpc-gateway/tree/master/protoc-gen-swagger/genswagger. You can see how other fields are implemented here: https://github.com/grpc-ecosystem/grpc-gateway/blob/f7120437bb4f6c71f7f5076ad65a45310de2c009/protoc-gen-swagger/genswagger/types.go#L57.

[achew22]

I'm also happy to help out.If you have any questions, please mention me and I'll do what I can

[tete17]

Ok after a couple of days of investigating I think I have a better idea of what is required to achieve this.

I also think it is a bit more complicated than what I initially expected. Looking at the generated code of one of the examples https://github.com/grpc-ecosystem/grpc-gateway/blob/f7120437bb4f6c71f7f5076ad65a45310de2c009/examples/proto/examplepb/echo_service.swagger.json#L458 Whenever there is a oneof in the protobuf definition all the keys are simply added to the object as if the oneof keyword never existed.

I am also a bit confused on where this annotations should be applied. Feels like that feature was build for a more inheritance-like system rather than unions-like types in protobuf.

Maybe you guys could help me out to map the Pet example to protobuf because It seemed simple to me at the begging but turns out to be non trivial.

Another thing I managed to do is to easily add the struct swaggerDiscriminatorObject, but I wonder if it is mandatory to add something to https://github.com/grpc-ecosystem/grpc-gateway/tree/master/protoc-gen-swagger/options since I am not really sure where in the code the annotations to the protobuf files are being read.

I hope I made myself clear, if not please let me know so I can make a better effort explaining myself

[johanbrandhorst]

I also think it is a bit more complicated than what I initially expected.

Every time :joy:.

Maybe you guys could help me out to map the Pet example to protobuf because It seemed simple to me at the begging but turns out to be non trivial.

Not sure about this.

Another thing I managed to do is to easily add the struct swaggerDiscriminatorObject, but I wonder if it is mandatory to add something to https://github.com/grpc-ecosystem/grpc-gateway/tree/master/protoc-gen-swagger/options since I am not really sure where in the code the annotations to the protobuf files are being read.

If we're auto-generating the discriminator object from oneof declarations, we shouldn't need to expose it to the user (e.g., add it here). At least not for a first pass.

Maybe you could share some code and we could gauge whats left?

[vtolstov]

any chance to get this done?

[achew22]

There definitely is, @vtolstov. What do you need in order to give it a try with @tete17's design?

[jefferai]

Hi there,

I'm looking to be able to do this:

service FooService {
  rpc foo(FooRequest) returns (FooResponse) {
    option (google.api.http) = {
      post: "/foo-message"
      body: "payload"
    };
  }
}

message FooRequest {
  oneof payload {
    string msg = 1;
    google.protobuf.Struct str = 2;
  }
}

That is, not reference to a oneof member field in body, but reference the oneof field itself. Is that what this issue is tackling? It's kind of unclear -- I've found some related issues/PRs (like https://github.com/grpc-ecosystem/grpc-gateway/issues/413) but those seem specific to using the fields in the oneof spec rather than the oneof field itself.

I could probably dedicate some time to helping if so, but would like to figure out if what I'm asking for is this (so that the advice in this thread would apply) or if not, if what I'm asking for is even feasible.

[johanbrandhorst]

I think this issue is with regards to supporting some extra information in our swagger generator, not extra functionality in the annotation parser. With regards to your request, if it looks like it should be supported by https://github.com/googleapis/googleapis/blob/master/google/api/http.proto and we don't already support it (have you tried? What's the error?), please file a separate issue and we can discuss it there.

[jefferai]

Sure, I'll file a separate issue with the error.

[casimcdaniels]

@tete17 Do you have a working fork you can push up? Would love to continue where you left off if you had any substantial work towards getting this feature out.