Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
princejha95 opened this issue Sep 5, 2018 · 8 comments · Fixed by #833
Open

Comments

@princejha95
Copy link

princejha95 commented Sep 5, 2018

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.

@johanbrandhorst
Copy link
Collaborator

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.

@princejha95
Copy link
Author

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
Copy link
Author

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 ?

@carrelld
Copy link

carrelld commented Sep 7, 2018

@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.

@princejha95
Copy link
Author

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
Copy link
Author

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

will be set to a.proto and you can see that in
if err := renderServices(p.Services, s.Paths, p.reg, requestResponseRefs, customRefs); err != nil {
, 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
Copy link
Author

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

@johanbrandhorst
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants