Error when using oneof name in response body selector

[jefferai]

As asked from my question in #585 I'm filing a new bug here.

I'm trying to have the response_body in the HTTP options use the oneof field, so that we can have alternate schemas in the response. However, this causes an error. Here is a minimal proto:

syntax = "proto3";

package services;

option go_package = "services;services";

import "google/api/annotations.proto";

service HostService {
  // This retrieves the host specified in the request using the basic view.
  rpc GetHost(GetHostRequest) returns (GetHostResponse) {
    option (google.api.http) = {
      get: "/hosts/{id}"
      response_body: "item"
    };
  }
}

message HostA {
  string foo = 1;
}

message HostB {
  string bar = 1;
}

message GetHostRequest {
  string id = 1;
}

message GetHostResponse {
  oneof item {
    HostA a = 1;
    HostB b = 2;
  }
}

Here is tree output:

.
├── gen
│   └── service
└── service
    ├── service.proto
    └── third_party
        ├── google
        │   └── api
        │       ├── annotations.proto
        │       ├── http.proto
        │       └── httpbody.proto
        └── protoc-gen-swagger
            └── options
                ├── annotations.proto
                └── openapiv2.proto

And here is my invocation:
protoc -I. -I service/third_party --go_out=plugins=grpc,paths=source_relative:./gen --grpc-gateway_out=logtostderr=true,paths=source_relative:./gen service/service.proto

When I run the command I get:
--grpc-gateway_out: no field "item" found in GetHostResponse

Note that if I change the response body def to select either "a" or "b" -- not actually what I want, but just to try it out -- I get a different error (which I think is already captured in a different ticket):
--grpc-gateway_out: 224:9: expected operand, found 'if'

[johanbrandhorst]

Thanks for raising the issue. The reponse_body handling is all in this file: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-grpc-gateway/descriptor/services.go#L146. I expect we may need to add some logic to handle the case where a oneof is used. What can I do to help you bring this fix in?

[jefferai]

Well, having never dealt with either underlying proto handling code or grpc-gateway code, any pointers you may have would be useful :-)

[jefferai]

Closing -- it may be a bug but we're actually going to structure our API differently (for different reasons than this). If you want to keep it open to track, feel free!

[johanbrandhorst]

Okay! Feel free to reopen again if you need it later. If you need any other support for the gateway, I'm usually available in #grpc-gateway on Gophers slack.

[jlewi]

This seems like a useful feature.

I investigated a bit and it looks like we'd want to modify the request handler template

grpc-gateway/protoc-gen-grpc-gateway/internal/gengateway/template.go

Line 412 in 9ec6238

msg, err := client.{{.Method.GetName}}(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD))

To test the oneof and return the value that is actually set. When I hacked my generated code the following seemed to do the correct thing.

	msg, err := client.Get(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD))

	var result proto.Message
	switch x := msg.GetResult().(type) {
	case *GetResponse_Document:
		result = x.Document
		break
	case *GetResponse_Status:
		result = x.Status
	case nil:
	default:
		result = nil;
	}

where my proto looked like

message GetResponse {  
  oneof result {
    Document document = 1;
    ResponseStatus status = 2;
  }
}

No immediate plans to submit a fix for this. We are currently evaluating the use of gRPC and gRPC-gateway. If we end up adopting it we may try to tackle this.

[johanbrandhorst]

Thanks for your input, I'll reopen this bug report then :).

[stale]

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.

[stale]

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.