grpc-gateway icon indicating copy to clipboard operation
grpc-gateway copied to clipboard

Comments of rpc method gets copied if multiple services are present in a proto file.

Open princejha95 opened this issue 5 years ago • 8 comments

I have a proto file which consists of multiple services. When I generate the swagger json file from it, the comment which are mentioned for the first service gets copied down to the second service in the json file.

Here is the proto file:

syntax = "proto3";

import "google/api/annotations.proto";

service SampleService1{
    // Comment 1
    rpc Method (Empty) returns (Empty){
        option (google.api.http) = {
            get : "/api/method1"
        };
    }
}

service SampleService2{
    // Comment 2
    rpc Method (Empty) returns (Empty){
        option (google.api.http) = {
            get : "/api/method2"
        };
    }
}

message Empty {}

Here is the generated swagger json file:

{
    "paths": {
        "/api/method1": {
            "get": {
                "operationId": "Method",
                "responses": {
                    "200": {
                        "description": "",
                        "schema": {
                            "$ref": "#/definitions/Empty"
                        }
                    },
                    "404": {
                        "description": "Not Found",
                        "schema": {}
                    }
                },
                "tags": [
                    "SampleService1"
                ],
                "summary": "Comment 1"
            }
        },
        "/api/method2": {
            "get": {
                "operationId": "Method",
                "responses": {
                    "200": {
                        "description": "",
                        "schema": {
                            "$ref": "#/definitions/Empty"
                        }
                    },
                    "404": {
                        "description": "Not Found",
                        "schema": {}
                    }
                },
                "tags": [
                    "SampleService2"
                ],
                "summary": "Comment 1"
            }
        }
    },
    "schemes": [
        "http",
        "https"
    ],
    "tags": [
        {
            "name": "SampleService1",
            "description": "Description"
        },
        {
            "name": "SampleService2",
            "description": "Description"
        }
    ],
    "basePath": "/",
    "host": "localhost:3000",
    "definitions": {
        "Empty": {
            "type": "object"
        }
    },
    "swagger": "2.0"
}

Can anyone help in fixing this issue ?

I don't want the comments to get copied. The comments should be present as it is in json file for their respective rpc methods under their respective services.

princejha95 avatar Sep 05 '18 13:09 princejha95

Thanks for raising this issue @princejha95. I'm guessing there's either some variable being overwritten incorrectly or an index not being used here. I predict that if we add a third service with the same method name it will also give that the comment from the first service.

We'd be happy to review a PR to fix this. I think the first place to look would be https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/main.go.

johanbrandhorst avatar Sep 05 '18 14:09 johanbrandhorst

I think we need to look at https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template.go because this is where the comments part is dealt.

Please have a look to this file.

princejha95 avatar Sep 05 '18 20:09 princejha95

Can anyone tell me how can i debug files in grpc-gateway using a debugger ?

Which file should be the starting point to start debugging ?

princejha95 avatar Sep 07 '18 02:09 princejha95

@princejha95 What I did to start debugging was to set-up a failing test, but that was a bit harder than expected given I wan't sure how to construct a valid File object. Here's my best guess though at a fix:

The index of the service gets passed to https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template.go#L743 the values being 0 on the first pass and 1 on the second pass. Within that function, there's https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template.go#L1238 which truncates the path from int32{6, a, 2, b} to int32{2, b}. This will end up returning true for any SourceCodeInfo_Location that is has a path matching Service -> Method (with matching name). It will match the first one and return the comments ignoring the second one.

I think the missing piece is the a that gets truncated from the path. I think a represents the service index. So at https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template.go#L1208, the line should instead be

if paths[0] != serviceProtoPath || paths[2] != methodProtoPath || paths[1] != typeIndex { 

You should be able to make this change and test it against your proto file. I'd do it myself, but I'm having trouble getting code generation working at all.

carrelld avatar Sep 07 '18 03:09 carrelld

I have done and checked what you said and it's correct but this is working only for a single proto file. If i have multiple proto files in a directory, then it is displaying the comments of rpc methods of all the services only for a single proto file. The comments of rpc methods of remaining proto files is not getting displayed.

princejha95 avatar Sep 07 '18 05:09 princejha95

https://github.com/grpc-ecosystem/grpc-gateway/blob/ab0345bb328757bfef2f3d7d4e642e182eb985b9/protoc-gen-swagger/genswagger/template.go#L841

What i observed is that when we have multiple proto files, then the title of the generated file is set to the name of that file which comes first in alphabetical order.

So, in the above link, *p.File.Name in title represents the name of file coming first alphabetically due to which it traverses the rpc methods of services of only that file.

Example: If i have two proto files say a.proto and b.proto. When i will generate swagger json file via merging both the proto files, then the title present in https://github.com/grpc-ecosystem/grpc-gateway/blob/ab0345bb328757bfef2f3d7d4e642e182eb985b9/protoc-gen-swagger/genswagger/template.go#L841 will be set to a.proto and you can see that in https://github.com/grpc-ecosystem/grpc-gateway/blob/ab0345bb328757bfef2f3d7d4e642e182eb985b9/protoc-gen-swagger/genswagger/template.go#L850, the services for only that proto file is passed.

So can somebody tell me how to pass services of all the proto files instead of single proto file ?

princejha95 avatar Sep 07 '18 05:09 princejha95

Can anyone provide the solution for the above issue ? I tried but i was not able to figure it out.

princejha95 avatar Sep 18 '18 06:09 princejha95

Reopening this as I've had to back out the latest change. @hexfusion I'd love to work together to get a fix in for this that maintains the comment merging behaviour.

johanbrandhorst avatar Jun 13 '19 20:06 johanbrandhorst