Merging swagger specs fails to use rpc comments #664

lukasmalkmus · 2018-06-02T00:55:01Z

I have two proto files:

a.proto:

syntax = "proto3";

package api.v1;

import "google/api/annotations.proto";

// A is a service for handling Foo stuff.
service A {
  // List all Foos
  //
  // Returns a (possibly paginated) list of all foo resources on success.
  rpc ListFoos (ListFoosRequest) returns (ListFoosResponse) {
    option (google.api.http) = { get: "/v1/foos" };
  }
}

// The ListFoos request message.
message ListFoosRequest {}

// The ListFoos response message.
message ListFoosResponse {}

b.proto:

syntax = "proto3";

package api.v1;

import "google/api/annotations.proto";

// B is a service for handling Bar stuff.
service B {
  // List all Bars
  //
  // Returns a (possibly paginated) list of all bar resources on success.
  rpc ListBars (ListBarsRequest) returns (ListBarsResponse) {
    option (google.api.http) = { get: "/v1/bars" };
  }
}

// The ListBars request message.
message ListBarsRequest {}

// The ListBars response message.
message ListBarsResponse {}

I tried to create a single swagger spec by using the allow_merge option. However, the generator removes some of the protobuf comments which get transformed into the endpoint description.

Steps you follow to reproduce the error:

Grab the provided protobuf file a.proto and b.proto

Generate the swagger spec without merging:

protoc -I=. \
        -I=$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway \
        -I=$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
        --swagger_out=logtostderr=true:. \
        a.proto b.proto

Generate the swagger spec with merging:

protoc -I=. \
        -I=$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway \
        -I=$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
        --swagger_out=logtostderr=true,allow_merge=true:. \
        a.proto b.proto

What did you expect to happen instead:

Take a look at the separate specifications, the protobuf comments got transformed into endpoint descriptions:

a.swagger.json:

{
  "swagger": "2.0",
  "info": {
    "title": "a.proto",
    "version": "version not set"
  },
  "schemes": [
    "http",
    "https"
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/v1/foos": {
      "get": {
        "summary": "List all Foos",
        "description": "Returns a (possibly paginated) list of all foo resources on success.",
        "operationId": "ListFoos",
        "responses": {
          "200": {
            "description": "",
            "schema": {
              "$ref": "#/definitions/v1ListFoosResponse"
            }
          }
        },
        "tags": [
          "A"
        ]
      }
    }
  },
  "definitions": {
    "v1ListFoosResponse": {
      "type": "object",
      "description": "The ListFoos response message."
    }
  }
}

b.swagger.json:

{
  "swagger": "2.0",
  "info": {
    "title": "b.proto",
    "version": "version not set"
  },
  "schemes": [
    "http",
    "https"
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/v1/bars": {
      "get": {
        "summary": "List all Bars",
        "description": "Returns a (possibly paginated) list of all bar resources on success.",
        "operationId": "ListBars",
        "responses": {
          "200": {
            "description": "",
            "schema": {
              "$ref": "#/definitions/v1ListBarsResponse"
            }
          }
        },
        "tags": [
          "B"
        ]
      }
    }
  },
  "definitions": {
    "v1ListBarsResponse": {
      "type": "object",
      "description": "The ListBars response message."
    }
  }
}

This isn't true for the merged specification:

apidocs.swagger.json:

{
  "swagger": "2.0",
  "info": {
    "title": "a.proto",
    "version": "version not set"
  },
  "schemes": [
    "http",
    "https"
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/v1/bars": {
      "get": {
        "operationId": "ListBars",
        "responses": {
          "200": {
            "description": "",
            "schema": {
              "$ref": "#/definitions/v1ListBarsResponse"
            }
          }
        },
        "tags": [
          "B"
        ]
      }
    },
    "/v1/foos": {
      "get": {
        "summary": "List all Foos",
        "description": "Returns a (possibly paginated) list of all foo resources on success.",
        "operationId": "ListFoos",
        "responses": {
          "200": {
            "description": "",
            "schema": {
              "$ref": "#/definitions/v1ListFoosResponse"
            }
          }
        },
        "tags": [
          "A"
        ]
      }
    }
  },
  "definitions": {
    "v1ListBarsResponse": {
      "type": "object",
      "description": "The ListBars response message."
    },
    "v1ListFoosResponse": {
      "type": "object",
      "description": "The ListFoos response message."
    }
  }
}

This spec misses the description of the /v1/bars endpoint.

What's your theory on why it isn't working:

There seems to be a bug in the code which merges the two specs. I this its located around here:

grpc-gateway/protoc-gen-swagger/genswagger/generator.go

Lines 93 to 97 in a5b66c1

    
           } else { 
        
           	mergedTarget.Enums = append(mergedTarget.Enums, f.Enums...) 
        
           	mergedTarget.Messages = append(mergedTarget.Messages, f.Messages...) 
        
           	mergedTarget.Services = append(mergedTarget.Services, f.Services...) 
        
           }

If merged to the leader, the code does not copy the comments.

The text was updated successfully, but these errors were encountered:

emcfarlane · 2018-06-14T09:43:11Z

Have found the same issue.

lukasmalkmus · 2018-06-25T09:41:58Z

Looks like this was fixed in latest master with 11bef10. Can you confirm @afking ?

emcfarlane · 2018-06-28T16:00:12Z

@lukasmalkmus fixed, thanks!

princejha95 · 2018-09-05T07:04:43Z

I have a proto file which contains two services. The comments of the rpc methods present in second service gets copied from first service on swagger UI. Can somebody please tell me how to fix it ?

johanbrandhorst · 2018-09-05T08:22:15Z

@princejha95 That sounds like a bug, have you got your protofile and generated swagger file so we can see if there's an error?

princejha95 · 2018-09-05T11:23:25Z

syntax = "proto3";

import "google/api/annotations.proto";

package test;

service SampleService1{
    // Say Hello
    rpc SayHello (HelloRequest) returns (HelloResponse){
        option (google.api.http) = {
            get : "/api/hello"
        };
    }
}

message HelloRequest{
    // Hello message
    string msg = 1;
}

message HelloResponse{
    //Hello response
    string res = 1;
}

service SampleService2{
    //Say Bye
    rpc SayBye (ByeRequest) returns (ByeResponse){
        option (google.api.http) = {
            get : "/api/bye"
        };
    }
}

message ByeRequest{
    //Bye message
    string msg = 1;
}

message ByeResponse{
    //Bye response
    string res = 1;
}

This is the proto file.

princejha95 · 2018-09-05T11:24:53Z

{
    "paths": {
        "/api/bye": {
            "get": {
                "tags": [
                    "SampleService2"
                ],
                "operationId": "SayBye",
                "responses": {
                    "200": {
                        "description": "",
                        "schema": {
                            "$ref": "#/definitions/testByeResponse"
                        }
                    },
                    "404": {
                        "description": "Not Found",
                        "schema": {}
                    }
                },
                "parameters": [
                    {
                        "required": false,
                        "type": "string",
                        "name": "msg",
                        "description": "Bye message."
                    }
                ],
                "summary": "Say Hello"
            }
        },
        "/api/hello": {
            "get": {
                "tags": [
                    "SampleService1"
                ],
                "operationId": "SayHello",
                "responses": {
                    "200": {
                        "description": "",
                        "schema": {
                            "$ref": "#/definitions/testHelloResponse"
                        }
                    },
                    "404": {
                        "description": "Not Found",
                        "schema": {}
                    }
                },
                "parameters": [
                    {
                        "required": false,
                        "type": "string",
                        "name": "msg",
                        "description": "Hello message."
                    }
                ],
                "summary": "Say Hello"
            }
        }
    },
    "schemes": [
        "http",
        "https"
    ],
    "tags": [
        {
            "name": "SampleService2",
            "description": "Description"
        },
        {
            "name": "SampleService1",
            "description": "Description"
        }
    ],
    "basePath": "/",
    "host": "localhost:3000",
    "definitions": {
        "testHelloResponse": {
            "type": "object",
            "properties": {
                "res": {
                    "type": "string",
                    "title": "Hello response"
                }
            }
        },
        "testByeResponse": {
            "type": "object",
            "properties": {
                "res": {
                    "type": "string",
                    "title": "Bye response"
                }
            }
        }
    },
    "swagger": "2.0"
}

This is the generated swagger json file.

johanbrandhorst · 2018-09-05T13:11:15Z

Yeah, that looks wrong. Could you open a separate issue with a minimal protofile which reproduces the error? I think something like the following should do:

syntax = "proto3";

import "google/api/annotations.proto";

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

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

message Empty {}

princejha95 · 2018-09-05T13:25:46Z

sure

princejha95 · 2018-09-05T13:59:44Z

I have raised a new issue. Here's the link:

#746

bsakweson · 2019-01-06T06:14:46Z

I ran into an issue similar to this, would love to open a new issue if needed but thought I should start here since they related.

I have a setup with multiple micro services each defined in their own .proto file. Code generation works well until I tried to combine my openAPI documentation using protoc-gen-swagger from grpc-gateway.

Using this command protoc --proto_path=api/v1 --proto_path=$GOPATH/src --proto_path=third_party --swagger_out=logtostderr=true,allow_merge=true:third_party/OpenAPI a/a.proto b/b.proto to generate a merged apidocs.swagger.json file containing all services defined in a and b respectively so that I can serve it as a single page throws this error --swagger_out: inconsistent package names: a b . Notice that a and b are all in different packages a and b.

service a:

syntax = "proto3";

package com.somecompany.a;

option go_package = "a";

import "google/api/annotations.proto";

// A is a service for handling Foo stuff.
service A {
  // List all Foos
  //
  // Returns a (possibly paginated) list of all foo resources on success.
  rpc ListFoos (ListFoosRequest) returns (ListFoosResponse) {
    option (google.api.http) = { get: "/v1/foos" };
  }
}

// The ListFoos request message.
message ListFoosRequest {}

// The ListFoos response message.
message ListFoosResponse {}

service b:

syntax = "proto3";

package com.somecompany.b;

option go_package = "b";
import "google/api/annotations.proto";

// B is a service for handling Bar stuff.
service B {
  // List all Bars
  //
  // Returns a (possibly paginated) list of all bar resources on success.
  rpc ListBars (ListBarsRequest) returns (ListBarsResponse) {
    option (google.api.http) = { get: "/v1/bars" };
  }
}

// The ListBars request message.
message ListBarsRequest {}

// The ListBars response message.
message ListBarsResponse {}

johanbrandhorst · 2019-01-06T08:40:07Z

Hi @bsakweson. Unfortunately, allow_merge will almost certainly only work within protofiles from the sake package. Yours would be a feature request, as I believe it is working as intended here. Would you like to open another issue to track this?

johanbrandhorst · 2020-05-24T20:49:04Z

This is apparently still broken

ashutshkumr · 2020-05-24T20:55:46Z

Steps for reproduction on latest release: #1387

kentdotn · 2020-06-06T18:15:28Z

Hi, I have same problem and this patch works for me.

diff --git a/protoc-gen-swagger/genswagger/generator.go b/protoc-gen-swagger/genswagger/generator.go
index ecf1135..30fb7a7 100644
--- a/protoc-gen-swagger/genswagger/generator.go
+++ b/protoc-gen-swagger/genswagger/generator.go
@@ -168,7 +168,7 @@ func (g *generator) Generate(targets []*descriptor.File) ([]*plugin.CodeGenerato
                for _, f := range targets {
                        if mergedTarget == nil {
                                mergedTarget = f
-                       } else {
+                       } else if mergedTarget != f {
                                mergedTarget.Enums = append(mergedTarget.Enums, f.Enums...)
                                mergedTarget.Messages = append(mergedTarget.Messages, f.Messages...)
                                mergedTarget.Services = append(mergedTarget.Services, f.Services...)

johanbrandhorst · 2020-06-07T07:18:17Z

Hi @kentdotn, would you be willing to submit a PR with this fix? We can test it against our existing examples to make sure it doesn't break anything and hopefully finally fix this bug.

Thanks!

kentdotn · 2020-06-07T09:18:54Z

Sure! Here it is.

* fix rpc comment generation against allow_merge * pass proper typeIndex base ond source file * add an example for merged output * update BUILD.bazel Fixes #664

ghost · 2020-08-11T14:38:13Z

@johanbrandhorst any chances this to be released in stable version soon?

johanbrandhorst · 2020-08-12T08:32:15Z

I'll tag a new patch release on v1. Please consider migrating to v2.

johanbrandhorst · 2020-08-12T08:53:51Z

https://github.com/grpc-ecosystem/grpc-gateway/releases/tag/v1.14.7

tmc added openapi and removed openapi labels Jun 19, 2018

lukasmalkmus closed this as completed Jul 3, 2018

co3k mentioned this issue Apr 15, 2019

Merging swagger specs fails to use rpc comments (again) #923

Closed

ashutshkumr mentioned this issue May 24, 2020

Merging swagger specs (for multiple protos belonging to same package) fails to emit summary for some RPCs #1387

Closed

johanbrandhorst reopened this May 24, 2020

kentdotn mentioned this issue Jun 7, 2020

fix rpc comment generation against allow_merge #1445

Merged

johanbrandhorst closed this as completed in #1445 Jun 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merging swagger specs fails to use rpc comments #664

Merging swagger specs fails to use rpc comments #664

lukasmalkmus commented Jun 2, 2018 •

edited

Loading

emcfarlane commented Jun 14, 2018

lukasmalkmus commented Jun 25, 2018 •

edited

Loading

emcfarlane commented Jun 28, 2018

princejha95 commented Sep 5, 2018

johanbrandhorst commented Sep 5, 2018

princejha95 commented Sep 5, 2018 •

edited by johanbrandhorst

Loading

princejha95 commented Sep 5, 2018 •

edited by johanbrandhorst

Loading

johanbrandhorst commented Sep 5, 2018

princejha95 commented Sep 5, 2018

princejha95 commented Sep 5, 2018

bsakweson commented Jan 6, 2019

johanbrandhorst commented Jan 6, 2019 •

edited

Loading

johanbrandhorst commented May 24, 2020

ashutshkumr commented May 24, 2020

kentdotn commented Jun 6, 2020

johanbrandhorst commented Jun 7, 2020

kentdotn commented Jun 7, 2020

ghost commented Aug 11, 2020

johanbrandhorst commented Aug 12, 2020

johanbrandhorst commented Aug 12, 2020

Merging swagger specs fails to use rpc comments #664

Merging swagger specs fails to use rpc comments #664

Comments

lukasmalkmus commented Jun 2, 2018 • edited Loading

emcfarlane commented Jun 14, 2018

lukasmalkmus commented Jun 25, 2018 • edited Loading

emcfarlane commented Jun 28, 2018

princejha95 commented Sep 5, 2018

johanbrandhorst commented Sep 5, 2018

princejha95 commented Sep 5, 2018 • edited by johanbrandhorst Loading

princejha95 commented Sep 5, 2018 • edited by johanbrandhorst Loading

johanbrandhorst commented Sep 5, 2018

princejha95 commented Sep 5, 2018

princejha95 commented Sep 5, 2018

bsakweson commented Jan 6, 2019

johanbrandhorst commented Jan 6, 2019 • edited Loading

johanbrandhorst commented May 24, 2020

ashutshkumr commented May 24, 2020

kentdotn commented Jun 6, 2020

johanbrandhorst commented Jun 7, 2020

kentdotn commented Jun 7, 2020

ghost commented Aug 11, 2020

johanbrandhorst commented Aug 12, 2020

johanbrandhorst commented Aug 12, 2020

lukasmalkmus commented Jun 2, 2018 •

edited

Loading

lukasmalkmus commented Jun 25, 2018 •

edited

Loading

princejha95 commented Sep 5, 2018 •

edited by johanbrandhorst

Loading

princejha95 commented Sep 5, 2018 •

edited by johanbrandhorst

Loading

johanbrandhorst commented Jan 6, 2019 •

edited

Loading