-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
OpenAPI spec is generated with duplicated operation IDs. #442
Comments
Can you elaborate on the circumstances that lead to this class of errors? |
It seems to happen when you define two different services and the same method name in both. |
Short self-contained example would help. |
The self-contained example is a proto file with two services having one method named the same. Pass that through protoc-gen-swagger and you will get a broken Swagger spec. syntax = "proto3";
message TeamsListRequest {
string org_id = 1;
}
message TeamsListResponse {}
message OrgListRequest {}
message OrgListResponse {}
service Orgs {
rpc List(OrgListRequest) returns (OrgListResponse) {
option (google.api.http) = {
get: "/organizations"
};
}
}
service Teams {
rpc List(TeamsListRequest) returns (TeamsListResponse) {
option (google.api.http) = {
get: "/teams"
};
}
} |
@c4milo, as an FYI everyone who is helping out here is doing it on volunteer time. This will probably get fixed some day but it is a low priority relative to other things in my life. One of the things that makes bug reports easier to act on is a simple way for me to jump into the problem without having to set up an environment where I can get expected outputs. One of the ways to make this easier is to create a pull request with a failing test case. We have a folder where you can put an example and add it as a testcase. If you do that then the next person who runs into it will have an easier time to fix the issue. In the meantime, have you considered splitting your proto up into 2 files and generating multiple swagger definitions or changing the name of your rpc methods to be ListOrg and ListTeam? |
@achew22, I've been an open source contributor myself for a while and I believe I understand the dynamics involved. I'm also volunteering my time to report the issue. Whenever possible, I contribute the fix myself as well. Please don't take it personal. Very often, people don't have enough time to do perfect issue reports and issues are usually closed due to lack of information to reproduce. In this particular case, I considered it easy to reproduce by a project maintainer, given a proto file. Apologies if I made anybody feel uncomfortable, it was not my intention. I created the issue with the sole purpose of keeping track of it. I didn't mean to demand an immediate fix.
I did consider splitting the file in two, but it quickly became unmanageable from the frontend side. I also tried to fix it by namespacing the operation ID. However, Swagger's JS code generator produced ugly code. I'll keep researching how we can fix this in the best way possible. |
So, after some more digging, it turns out SwaggerJS is already dealing with duplicated operation IDs by appending a number: I have not look into other code generators, I imagine some of them will choke when encountering duplicated operation IDs. However, Swagger's spec v2 doesn't provide a lot of options for us to joggle from grpc-gateway side. That said, I'm closing this issue since the main reason to use grpc-gateway is to be able to use our gRPC services from a browser. For other programming languages, people should rely on the corresponding gRPC client code generator instead of Swagger. |
A possible solution is to prefix them with the service's name.
The text was updated successfully, but these errors were encountered: