Fixes additional paths generated when many methods have the same resource path

[aethanol]

References to other Issues or PRs

Fixes #2489

Have you read the Contributing Guidelines?

Yes

Brief description of what is fixed or changed

This adds tests and fixes the small regression introduced in #2461.

Other comments

More tests should probably be written, and specifically a test with multiple services and multiple same paths.

[aethanol]

Ah looks like I raised this PR preemptively, it regresses TestTemplateWithDuplicateHttp1Operations.

[aethanol]

This should be correct now! though I think I noticed another bug where when methods with the same generated path are not in order, it will generate a valid, but not ideal schema.
Eg.

syntax = "proto3";

package test;

option go_package = "example.com/test";

import "google/api/annotations.proto";
import "google/protobuf/empty.proto";

service Service1 {
  rpc DeleteBar(DeleteBarRequest) returns (google.protobuf.Empty) {
    option (google.api.http) = {
      delete: "/v1/{name=bars/*}"
    };
  }
  rpc GetFoo(GetFooRequest) returns (Foo) {
    option (google.api.http) = {
      get: "/v1/{name=foos/*}",
    };
  }
  rpc DeleteFoo(DeleteFooRequest) returns (google.protobuf.Empty) {
    option (google.api.http) = {
      delete: "/v1/{name=foos/*}"
    };
  }
  rpc GetBar(GetBarRequest) returns (Bar) {
    option (google.api.http) = {
      get: "/v1/{name=bars/*}"
    };
  }
}

message GetFooRequest {
  string name = 1;
}

message DeleteFooRequest {
  string name = 1;
}

message Foo { }


message GetBarRequest {
  string name = 1;
}

message DeleteBarRequest {
  string name = 1;
}

message Bar { }

will generate a schema with path /v1/{name} having DeleteBar and GetFoo and /v1/{name_1} having DeleteFoo GetBar. This is technically valid, but will probably make generated clients weird.

The potential solution would be to check that the param pattern is the same when adding to existing objects.

[oyvindwe]

The potential solution would be to check that the param pattern is the same when adding to existing objects.

Yes, I think that should work as well. But the path problem sticks even deeper as discussed in issue #720.

[johanbrandhorst]

Thanks for the quick fix, and thanks @oyvindwe for confirming the fix!

[johanbrandhorst]

Closing and reopening PR to see if I can trigger CI again 🙄

[codecov-commenter]

Codecov Report

Merging #2496 (a3407f2) into master (32c92f1) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2496      +/-   ##
==========================================
+ Coverage   60.31%   60.41%   +0.09%     
==========================================
  Files          36       36              
  Lines        4720     4721       +1     
==========================================
+ Hits         2847     2852       +5     
+ Misses       1577     1573       -4     
  Partials      296      296              

Impacted Files	Coverage Δ
...otoc-gen-openapiv2/internal/genopenapi/template.go	64.63% <100.00%> (+0.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32c92f1...a3407f2. Read the comment docs.

[johanbrandhorst]

Thanks for your contribution!

[aethanol]

Thanks for your contribution!

Glad to help! also what is the release schedule for grpc-gateway like? Or if you had a rough eta? I am contributing this change back to https://github.com/grafeas/grafeas

[johanbrandhorst]

I can cut a new release :). There's no particular schedule.

[johanbrandhorst]

https://github.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.7.3